Message ID | ZBlpWhnv6NXCrESh@tucnak |
---|---|
State | New |
Headers | show |
Series | tree: Fix up component_ref_sam_type handling of arrays of 0 sized elements [PR109215] | expand |
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 <jakub@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 > >
--- 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 *-*-* } } */ +}