On Tue, 21 Mar 2023, Jakub Jelinek wrote:
> Hi!
>
> Our documentation sadly talks about elt_type arr[0]; as zero-length arrays,
> not arrays with zero elements. Unfortunately, those aren't the only arrays
> which can have zero size, the same size can be also result of zero-length
> element, like in GNU C struct whatever {} or in GNU C/C++ if the element
> type is [0] array or combination thereof (dunno if Ada doesn't allow
> something similar too). One can't do much with them, taking address of
> their elements, (no-op) copying of the elements in and out. But they
> behave differently from arr[0] arrays e.g. in that using non-zero indexes
> in them (as long as they are within bounds as for normal arrays) is valid.
>
> I think this naming inaccuracy resulted in Martin designing
> special_array_member in an inconsistent way, mixing size zero array members
> with array members of one or two or more elements and then using the
> size zero interchangeably with zero elements.
>
> The following patch changes that (but doesn't do any
> documentation/diagnostics renaming, as this is really a corner case),
> such that int_0/trail_0 for consistency is just about [0] arrays
> plus [] for the latter, not one or more zero sized elements case.
>
> The testcase has one xfailed case for where perhaps in later GCC versions
> we could add extra code to handle it, for some reason we don't diagnose
> out of bounds accesses for the zero sized elements cases. It will be
> harder because e.g. FRE will canonicalize &var.fld[0] and &var.fld[10]
> to just one of them because they are provably the same address.
> But the important thing is to fix this regression (where we warn on
> completely valid code in the Linux kernel). Anyway, for further work
> on this we don't really need any extra help from special_array_member,
> all code can just check integer_zerop (TYPE_SIZE_UNIT (TREE_TYPE (type))),
> it doesn't depend on the position of the members etc.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Richard.
> 2023-03-21 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/109215
> * tree.h (enum special_array_member): Adjust comments for int_0
> and trail_0.
> * tree.cc (component_ref_sam_type): Clear zero_elts if memtype
> has zero sized element type and the array has variable number of
> elements or constant one or more elements.
> (component_ref_size): Adjust comments, formatting fix.
>
> * gcc.dg/Wzero-length-array-bounds-3.c: New test.
>
> --- gcc/tree.h.jj 2023-03-14 19:11:52.296936422 +0100
> +++ gcc/tree.h 2023-03-20 18:48:23.068788830 +0100
> @@ -5579,8 +5579,8 @@ extern tree component_ref_field_offset (
> enum struct special_array_member
> {
> none, /* Not a special array member. */
> - int_0, /* Interior array member with size zero. */
> - trail_0, /* Trailing array member with size zero. */
> + int_0, /* Interior array member with zero elements. */
> + trail_0, /* Trailing array member with zero elements. */
> trail_1, /* Trailing array member with one element. */
> trail_n, /* Trailing array member with two or more elements. */
> int_n /* Interior array member with one or more elements. */
> --- gcc/tree.cc.jj 2023-03-10 10:38:46.551473829 +0100
> +++ gcc/tree.cc 2023-03-20 19:41:35.605580732 +0100
> @@ -13032,14 +13032,27 @@ component_ref_sam_type (tree ref)
> return sam_type;
>
> bool trailing = false;
> - (void)array_ref_flexible_size_p (ref, &trailing);
> - bool zero_length = integer_zerop (memsize);
> - if (!trailing && !zero_length)
> - /* MEMBER is an interior array with
> - more than one element. */
> + (void) array_ref_flexible_size_p (ref, &trailing);
> + bool zero_elts = integer_zerop (memsize);
> + if (zero_elts && integer_zerop (TYPE_SIZE_UNIT (TREE_TYPE (memtype))))
> + {
> + /* If array element has zero size, verify if it is a flexible
> + array member or zero length array. Clear zero_elts if
> + it has one or more members or is a VLA member. */
> + if (tree dom = TYPE_DOMAIN (memtype))
> + if (tree min = TYPE_MIN_VALUE (dom))
> + if (tree max = TYPE_MAX_VALUE (dom))
> + if (TREE_CODE (min) != INTEGER_CST
> + || TREE_CODE (max) != INTEGER_CST
> + || !((integer_zerop (min) && integer_all_onesp (max))
> + || tree_int_cst_lt (max, min)))
> + zero_elts = false;
> + }
> + if (!trailing && !zero_elts)
> + /* MEMBER is an interior array with more than one element. */
> return special_array_member::int_n;
>
> - if (zero_length)
> + if (zero_elts)
> {
> if (trailing)
> return special_array_member::trail_0;
> @@ -13047,7 +13060,7 @@ component_ref_sam_type (tree ref)
> return special_array_member::int_0;
> }
>
> - if (!zero_length)
> + if (!zero_elts)
> if (tree dom = TYPE_DOMAIN (memtype))
> if (tree min = TYPE_MIN_VALUE (dom))
> if (tree max = TYPE_MAX_VALUE (dom))
> @@ -13114,14 +13127,14 @@ component_ref_size (tree ref, special_ar
>
> tree afield_decl = TREE_OPERAND (ref, 1);
> gcc_assert (TREE_CODE (afield_decl) == FIELD_DECL);
> - /* if the trailing array is a not a flexible array member, treat it as
> + /* If the trailing array is a not a flexible array member, treat it as
> a normal array. */
> if (DECL_NOT_FLEXARRAY (afield_decl)
> && *sam != special_array_member::int_0)
> return memsize;
>
> if (*sam == special_array_member::int_0)
> - memsize = NULL_TREE;
> + memsize = NULL_TREE;
>
> /* For a reference to a flexible array member of a union
> use the size of the union instead of the size of the member. */
> @@ -13129,7 +13142,7 @@ component_ref_size (tree ref, special_ar
> memsize = TYPE_SIZE_UNIT (argtype);
> }
>
> - /* MEMBER is either a bona fide flexible array member, or a zero-length
> + /* MEMBER is either a bona fide flexible array member, or a zero-elements
> array member, or an array of length one treated as such. */
>
> /* If the reference is to a declared object and the member a true
> --- gcc/testsuite/gcc.dg/Wzero-length-array-bounds-3.c.jj 2023-03-20
> 19:17:21.290648885 +0100
> +++ gcc/testsuite/gcc.dg/Wzero-length-array-bounds-3.c 2023-03-20
> 19:16:48.494123583 +0100
> @@ -0,0 +1,19 @@
> +/* PR tree-optimization/109215 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wall" } */
> +
> +struct S {};
> +struct T { struct S s[3]; struct S t; };
> +void foo (struct S *);
> +
> +void
> +bar (struct T *t)
> +{
> + foo (&t->s[2]); /* { dg-bogus "array subscript 2 is outside the bounds
> of an interior zero-length array" } */
> +}
> +
> +void
> +baz (struct T *t)
> +{
> + foo (&t->s[3]); /* { dg-error "" "" { xfail *-*-* } } */
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
