[PATCH] algol68: Add runtime bounds checks for row displays
Kanishka Solanki
kanishkasolanki456s@gmail.com
Thu Jul 16 20:36:16 GMT 2026
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/20260717/13ad5cb0/attachment-0001.htm>
More information about the Algol68
mailing list