[PATCH] algol68: Add runtime bounds checks for row displays
Kanishka Solanki
kanishkasolanki456s@gmail.com
Thu Aug 27 21:11:32 GMT 2026
Hi Jose,
No problem! Here is the patch.
As mentioned in my previous email, the patch is not working as expected
because the variable dim is equal to 1 even for:
begin [,]int foo = ((1,2,3),
(4,5));
skip
end
I am having some trouble understanding why this is happening. Could you
please help me understand this?
Thanks,
Kanishka
On Thu, Aug 27, 2026 at 11:41 PM Jose E. Marchesi <jemarch@gnu.org> wrote:
>
> Hello Kanishka.
>
> I apologize, but I think I have misplaced an email you sent with a
> subsequent version of a patch for this, along with some questions. I
> remember seeing it during my time off and made a note of replying to it,
> but now I cannot find it :(
>
> Could you please resend the patch to the list?
> Thanks!
>
> > 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.
> >
> > 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;
> >>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/algol68/attachments/20260828/9aa618e8/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-algol68-Add-runtime-bounds-checks-for-row-displays.patch
Type: text/x-patch
Size: 18773 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/algol68/attachments/20260828/9aa618e8/attachment-0001.bin>
More information about the Algol68
mailing list