[PATCH] algol68: Add runtime bounds checks for row displays
Jose E. Marchesi
jemarch@gnu.org
Fri Jul 17 09:43:29 GMT 2026
Hello Kanishka.
> Hi Jose,
> I am learning compiler internals in depth as of now so I can work on the
> patches more efficiently and I plan to pick them back up in a week.
Sure, no rush, thank you :)
Have fun!
>
> Thanks,
> Kanishka
>
> On Sat, 4 Jul, 2026, 18:28 Jose E. Marchesi, <jemarch@gnu.org> wrote:
>
>>
>> > Hello Kanishka!
>> >
>> > Thank you for the patch.
>> >
>> >> Hi,
>> >>
>> >> I've implemented the runtime bounds checks for row displays.
>> >>
>> >> The patch compares the lower bound, upper bound, and stride of each
>> >> sub-multiple against those of the first sub-multiple. If they differ,
>> >> it reports a runtime error instead of constructing an invalid row
>> >> display.
>> >>
>> >> I've rebuilt GCC and run the Algol68 testsuite successfully.
>> >>
>> >> I also tried to write a standalone test to trigger the new runtime
>> >> error, but I wasn't able to construct a valid Algol 68 program that
>> >> reaches this code path. If there is an existing test or a simple way to
>> >> exercise this case, I'd really appreciate any pointers. I'd be happy to
>> >> add a test case if possible.
>> >
>> > This should do it:
>> >
>> > begin [,]int foo = ((1,2,3),
>> > (4,5));
>> > skip
>> > end
>> >
>> > And indeed it works:
>> >
>> > $ ./a.out
>> > foo.a68:1: runtime error: row display bounds mismatch: [1:2] /= [1:3]
>> > Aborted
>> >
>> > However, note how the line number of the run-time error is 1, and I
>> > would expect it to be 2. More about this in comments below.
>> >
>> >>
>> >> I'd appreciate any feedback or suggestions for improvement.
>> >>
>> >> Thanks,
>> >> Kanishka
>> >> From 37dbfff72bedf6c1959a2c90e41f6b01bf9bb793 Mon Sep 17 00:00:00 2001
>> >> From: Kanishka Solanki <kanishkasolanki456s@gmail.com>
>> >> Date: Sat, 4 Jul 2026 02:33:42 +0530
>> >> Subject: [PATCH] algol68: Add runtime bounds checks for row displays
>> >>
>> >> Compare the descriptors of each sub-multiple in a row display
>> >> against those of the first sub-multiple. Report a runtime error
>> >> if the lower bound, upper bound, or stride differs.
>> >>
>> >> Signed-off-by: Kanishka Solanki <kanishkasolanki456s@gmail.com>
>> >>
>> >> Compare the descriptors of each sub-multiple in a row display
>> >> against those of the first sub-multiple. Report a runtime error
>> >> if the lower bound, upper bound, or stride differs.
>> >>
>> >> gcc/algol68/
>> >> * a68-low-clauses.cc (a68_lower_collateral_clause):
>> >> Add runtime bounds checks for row displays.
>> >> * a68-low-runtime.def
>> >> (A68_LIBCALL_ROWDISPLAYBOUNDSMISMATCH): New libcall.
>> >>
>> >> libga68/
>> >> * ga68-error.c (libga68_rowdisplayboundsmismatch): New.
>> >> * ga68.h: Declare it.
>> >> * ga68.map: Export it.
>> >> ---
>> >> gcc/algol68/a68-low-clauses.cc | 71 +++++++++++++++++++++++++++------
>> >> gcc/algol68/a68-low-runtime.def | 2 +
>> >> libga68/ga68-error.c | 13 ++++++
>> >> libga68/ga68.h | 3 ++
>> >> libga68/ga68.map | 1 +
>> >> 5 files changed, 78 insertions(+), 12 deletions(-)
>> >>
>> >> diff --git a/gcc/algol68/a68-low-clauses.cc
>> b/gcc/algol68/a68-low-clauses.cc
>> >> index 29ccc43b076..8da0209b35b 100644
>> >> --- a/gcc/algol68/a68-low-clauses.cc
>> >> +++ b/gcc/algol68/a68-low-clauses.cc
>> >> @@ -1209,9 +1209,9 @@ a68_lower_collateral_clause (NODE_T *p
>> ATTRIBUTE_UNUSED,
>> >> tree multiple_elements = NULL_TREE;
>> >> tree multiple_elements_size = NULL_TREE;
>> >> tree sub_multiple = NULL_TREE;
>> >> - // tree sub_multiple_lb = NULL_TREE;
>> >> - // tree sub_multiple_ub = NULL_TREE;
>> >> - // tree sub_multiple_stride = NULL_TREE;
>> >> + tree sub_multiple_lb = NULL_TREE;
>> >> + tree sub_multiple_ub = NULL_TREE;
>> >> + tree sub_multiple_stride = NULL_TREE;
>> >> tree index = a68_lower_tmpvar ("index%", sizetype,
>> size_zero_node);
>> >> for (tree_stmt_iterator si = tsi_start (units); !tsi_end_p (si);
>> tsi_next (&si))
>> >> {
>> >> @@ -1227,7 +1227,6 @@ a68_lower_collateral_clause (NODE_T *p
>> ATTRIBUTE_UNUSED,
>> >>
>> >> if (si == tsi_start (units))
>> >> {
>> >> -#if 0
>> >> tree ssize_zero_node = fold_convert (ssizetype,
>> size_zero_node);
>> >> /* The first sub-multiple establishes the bounds that all
>> >> subsequent sub-multiples shall match. */
>> >> @@ -1243,7 +1242,7 @@ a68_lower_collateral_clause (NODE_T *p
>> ATTRIBUTE_UNUSED,
>> >> sizetype,
>> >>
>> a68_multiple_stride (sub_multiple,
>> >>
>> size_zero_node));
>> >> -#endif
>> >> +
>> >> /* Now we have enough information to calculate the size
>> of
>> >> the elements of the new multiple and allocate
>> >> multiple_elements. */
>> >> @@ -1280,13 +1279,61 @@ a68_lower_collateral_clause (NODE_T *p
>> ATTRIBUTE_UNUSED,
>> >> }
>> >> else
>> >> {
>> >> - /* Check bounds of this sub-multiple. Note that this is
>> >> - always done at run-time, since the interpretation of
>> a row
>> >> - display depens on the target type, whether it is a
>> row row
>> >> - or a row of rows, for example. */
>> >> - // XXX use sub_multiple_lb, sub_multiple_ub and
>> sub_multiple_stride
>> >
>> > Please keep a comment here like
>> >
>> > /* Check bounds of this sub-multiple. */
>> >
>> > for clarity :)
>> >
>> >> + tree current_lb = a68_lower_tmpvar ("current_lb%",
>> >> + ssizetype,
>> >> +
>> a68_multiple_lower_bound (sub_multiple,
>> >> +
>> size_zero_node));
>> >> + tree current_ub = a68_lower_tmpvar ("current_ub%",
>> >> + ssizetype,
>> >> +
>> a68_multiple_upper_bound (sub_multiple,
>> >> +
>> size_zero_node));
>> >> + tree current_stride = a68_lower_tmpvar
>> ("current_stride%",
>> >> + sizetype,
>> >> +
>> a68_multiple_stride (sub_multiple,
>> >> + size_zero_node));
>> >> + tree lb_equal = fold_build2 (EQ_EXPR,
>> >> + boolean_type_node,
>> >> + current_lb,
>> >> + sub_multiple_lb);
>> >> + tree ub_equal = fold_build2 (EQ_EXPR,
>> >> + boolean_type_node,
>> >> + current_ub,
>> >> + sub_multiple_ub);
>> >> + tree stride_equal = fold_build2 (EQ_EXPR,
>> >> + boolean_type_node,
>> >> + current_stride,
>> >> + sub_multiple_stride);
>> >> + tree bounds_equal = fold_build2 (TRUTH_AND_EXPR,
>> >> + boolean_type_node,
>> >> + lb_equal,
>> >> + ub_equal);
>> >> + tree descriptors_equal = fold_build2 (TRUTH_AND_EXPR,
>> >> + boolean_type_node,
>> >> + bounds_equal,
>> >> + stride_equal);
>> >> + unsigned int lineno = NUMBER (LINE (INFO (p)));
>> >
>> > We would want the location of the offending entry here, not the location
>> > of the entire row display.
>> >
>> > But it is not clear how to do this with the existing strategy though, as
>> > we are first collecting units:
>> >
>> > /* Lower the constituent units into a statements list. */
>> > a68_push_stmt_list (mode);
>> > if (!clause_is_empty)
>> > {
>> > if (a68_lower_tree (NEXT (SUB (p)), ctx) != NULL_TREE)
>> > /* unit lists always lower to NULL_TREE and, as a side-effect,
>> > append the units to the current statements list. */
>> > gcc_unreachable ();
>> > }
>> > tree units = a68_pop_stmt_list ();
>> >
>> > and then operating on the resulting stmt_list.
>> >
>> > This is no good. I was too clumsy and silly taking that approach, back
>> > when I wanted to get something working ASAP 8-)
>> >
>> > The current approach also makes compile-time error checking more
>> > difficult.
>> >
>> > I wonder whether it wouldn't be better to change the approach and
>> > dispense with the `units' stmt list. Instead, we could have a lowerer
>> > function for the units of a row display, and use it like:
>> >
>> > else
>> > {
>> > /* this is a row display. */
>> > gcc_assert (dim > 1);
>> > return a68_lower_row_display (p, ctx);
>> > }
>> >
>> > Where a68_lower_row_display would:
>> >
>> > 1. Traverse and lowering the constituent units, detecting compile-time
>> > mismatches and reporting them.
>> >
>> > 2. Generate run-time checks as well.
>> >
>> > WDYT?
>>
>> Elaborating a little bit more.
>>
>> In what I am picturing a68_lower_row_display would look like this:
>>
>>
>> static tree
>> a68_lower_row_display (NODE_T *row, NODE_T *p, LOW_CTX_T ctx)
>> {
>> /* Lower each entry in the row display. */
>>
>> tree elems_length = NULL_TREE; /* NULL_TREE means unknown. May be
>> an INTEGER_CST */
>>
>> for (; p != NO_NODE; FORWARD (p))
>> {
>> /* The lenght of the first element determines the length that
>> the rest of the elements must have.
>>
>> Note that the constant folding (_fold calls) will reduce the
>> length to an INTEGER_CST if the length of the unit is known
>> at compile time, like when the element is a row-display
>> itself. */
>>
>> if (elems_length == NULL_TREE)
>> {
>> /* Processing first element. */
>> elems_length = ...;
>> }
>> else
>> {
>> /* Processing a subsequent element. */
>>
>> tree elem = a68_lower_row_display (row, p, ctx);
>>
>> /* If the element is a row display itself, then we can check
>> its length. The length of the first row-display element
>> sets the length all other elements should
>>
>> if (IS (p, ENCLOSED_CLAUSE))
>> {
>> NODE_T *collateral_clause = SUB (p);
>> MOID_T *clause_mode = MOID (collateral_clause);
>>
>> if (IS_FLEXETY_ROW (clause_mode) || clause_mode ==
>> M_STRING)
>> {
>>
>> }
>> }
>> else
>> {
>> /* Element is now a row display, so check that its
>> length is elems_length at run-time */
>> ...
>> }
>> }
>> }
>>
>> >
>> >> + const char *filename_str = FILENAME (LINE (INFO (p)));
>> >> + tree filename = build_string_literal (strlen
>> (filename_str) + 1,
>> >> + filename_str);
>> >> + tree call = a68_build_libcall
>> (A68_LIBCALL_ROWDISPLAYBOUNDSMISMATCH,
>> >> + void_type_node, 6,
>> >> + filename,
>> >> + build_int_cst
>> (unsigned_type_node, lineno),
>> >> +
>> fold_convert (ssizetype, current_lb),
>> >> + fold_convert (ssizetype,
>> current_ub),
>> >> + fold_convert (ssizetype,
>> sub_multiple_lb),
>> >> + fold_convert (ssizetype,
>> sub_multiple_ub));
>> >> + call = fold_build2 (COMPOUND_EXPR,
>> >> + a68_bool_type,
>> >> + call,
>> >> + boolean_false_node);
>> >> + tree check = fold_build2 (TRUTH_ORIF_EXPR,
>> >> + boolean_type_node,
>> >> + descriptors_equal,
>> >> + call);
>> >> +
>> >> + a68_add_stmt (check);
>> >> }
>> >> -
>> >> /* Copy the elements of a copy of the sub-multiple in the
>> >> elements of the multiple. */
>> >> tree sub_multiple_elements = a68_multiple_elements
>> (sub_multiple);
>> >> @@ -1327,7 +1374,7 @@ a68_lower_collateral_clause (NODE_T *p
>> ATTRIBUTE_UNUSED,
>> >> a68_add_stmt (multiple);
>> >> return a68_pop_range ();
>> >> }
>> >> - }
>> >> + }
>> >> else if (IS_STRUCT (mode))
>> >> {
>> >> /* This is a struct display. There are as many units in the
>> clause as
>> >> diff --git a/gcc/algol68/a68-low-runtime.def
>> b/gcc/algol68/a68-low-runtime.def
>> >> index 5f12906a0ce..a3a0ab505aa 100644
>> >> --- a/gcc/algol68/a68-low-runtime.def
>> >> +++ b/gcc/algol68/a68-low-runtime.def
>> >> @@ -56,6 +56,8 @@ DEF_A68_RUNTIME (ARRAYBOUNDS, "_libga68_bounds",
>> RT(VOID),
>> >> P5(CONSTCHARPTR, UINT, SSIZE, SSIZE, SSIZE), ECF_NORETURN)
>> >> DEF_A68_RUNTIME (ARRAYBOUNDSMISMATCH, "_libga68_bounds_mismatch",
>> RT(VOID),
>> >> P7(CONSTCHARPTR, UINT, SIZE, SSIZE, SSIZE, SSIZE, SSIZE),
>> ECF_NORETURN)
>> >> +DEF_A68_RUNTIME (ROWDISPLAYBOUNDSMISMATCH,
>> "_libga68_row_display_bounds_mismatch", RT(VOID),
>> >> + P6(CONSTCHARPTR, UINT, SSIZE, SSIZE, SSIZE, SSIZE),
>> ECF_NORETURN)
>> >> DEF_A68_RUNTIME (ARRAYDIM, "_libga68_dim", RT(VOID),
>> >> P4(CONSTCHARPTR, UINT, SIZE, SIZE), ECF_NORETURN)
>> >> DEF_A68_RUNTIME (RANDOM, "_libga68_random", RT(FLOAT), P0(), 0)
>> >> diff --git a/libga68/ga68-error.c b/libga68/ga68-error.c
>> >> index 1bb0530ffdc..d3b80f67ab5 100644
>> >> --- a/libga68/ga68-error.c
>> >> +++ b/libga68/ga68-error.c
>> >> @@ -150,3 +150,16 @@ _libga68_bounds_mismatch (const char *filename,
>> unsigned int lineno,
>> >> assignation: dim %zu: [%zd:%zd] /= [%zd:%zd]\n",
>> >> filename, lineno, dim, lb1, ub1, lb2, ub2);
>> >> }
>> >> +
>> >> +/* Row display sub-multiples have different bounds. */
>> >> +
>> >> +void
>> >> +_libga68_row_display_bounds_mismatch (const char *filename,
>> >> + unsigned int lineno,
>> >> + ssize_t lb1, ssize_t ub1,
>> >> + ssize_t lb2, ssize_t ub2)
>> >> +{
>> >> + _libga68_abort ("%s:%u: runtime error: row display bounds
>> mismatch: "
>> >> + "[%zd:%zd] /= [%zd:%zd]\n",
>> >> + filename, lineno, lb1, ub1, lb2, ub2);
>> >> +}
>> >> \ No newline at end of file
>> >> diff --git a/libga68/ga68.h b/libga68/ga68.h
>> >> index 9c104e60437..0571ce53ee9 100644
>> >> --- a/libga68/ga68.h
>> >> +++ b/libga68/ga68.h
>> >> @@ -65,6 +65,9 @@ void _libga68_dim (const char *filename, unsigned int
>> lineno,
>> >> void _libga68_bounds_mismatch (const char *filename, unsigned int
>> lineno,
>> >> size_t dim, ssize_t lb1, ssize_t ub1,
>> >> ssize_t lb2, ssize_t ub2);
>> >> +void _libga68_row_display_bounds_mismatch (const char *filename,
>> unsigned int lineno,
>> >> + ssize_t lb1, ssize_t ub1,
>> >> + ssize_t lb2, ssize_t ub2);
>> >>
>> >> /* ga68-alloc.c */
>> >>
>> >> diff --git a/libga68/ga68.map b/libga68/ga68.map
>> >> index 57610931961..44d2619157c 100644
>> >> --- a/libga68/ga68.map
>> >> +++ b/libga68/ga68.map
>> >> @@ -4,6 +4,7 @@ LIBGA68_2.0 {
>> >> _libga68_bitsboundserror;
>> >> _libga68_bounds;
>> >> _libga68_bounds_mismatch;
>> >> + _libga68_row_display_bounds_mismatch;
>> >> _libga68_derefnil;
>> >> _libga68_dim;
>> >> _libga68_invalidcharerror;
>>
More information about the Algol68
mailing list