| Message ID | 84ea8466-a686-102e-43e7-d1eb1db01c76@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | fold constant flexarrays to strings (PR 91490) | expand |
On 8/21/19 2:50 PM, Martin Sebor wrote: > This patch is a subset of the solution for PR 91457 whose main > goal is to eliminate inconsistencies in warnings issued for > out-of-bounds accesses to the various flavors of flexible array > members of constant objects. That patch was posted here: > https://gcc.gnu.org/ml/gcc-patches/2019-08/msg01202.html > > Like PR 91457, this patch also relies on improving optimization > to issue better quality warnings. (I.e., with the latter being > what motivated it.) > > The optimization enhances string_constant to recognize empty > CONSTRUCTORs returned by fold_ctor_reference for references > to members of constant objects with either "insufficient" > initializers (e.g., given struct S { char n, a[]; } s = { 0 };) > or with braced-list initializers (e.g., given > struct S s = { 3 { 1, 2, 3, 0 } }; The patch lets string_constant > convert the CONSTRUCTOR for s.a into a STRING_CST, which in turn > enables the folding of calls to built-ins like strlen, strchr, or > strcmp with such arguments. > > Exposing the strings to the folder then also lets it detect and > issue warnings for out-of-bounds offsets in more instances of > such references than before. > > The remaining changes in the patch mostly enhance the places > that use the no-warning bit to prevent redundant diagnostics. > > Tested on x86_64-linux. > > Martin > > gcc-91490.diff > > PR middle-end/91490 - bogus argument missing terminating nul warning on strlen of a flexible array member > > gcc/c-family/ChangeLog: > > PR middle-end/91490 > * c-common.c (braced_list_to_string): Add argument and overload. > Handle flexible length arrays. > > gcc/testsuite/ChangeLog: > > PR middle-end/91490 > * c-c++-common/Warray-bounds-7.c: New test. > * gcc.dg/Warray-bounds-39.c: Expect either -Warray-bounds or > -Wstringop-overflow. > * gcc.dg/strlenopt-78.c: New test. > > gcc/ChangeLog: > > PR middle-end/91490 > * builtins.c (c_strlen): Rename argument and introduce new local. > Set no-warning bit on original argument. > * expr.c (string_constant): Pass argument type to fold_ctor_reference. > * gimple-fold.c (fold_nonarray_ctor_reference): Return a STRING_CST > for missing initializers. > * tree.c (build_string_literal): Handle optional argument. > * tree.h (build_string_literal): Add defaulted argument. > * gimple-ssa-warn-restrict.c (maybe_diag_access_bounds): Check > no-warning bit on original expression. > > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c > index 2810867235d..ef942ffce57 100644 > --- a/gcc/c-family/c-common.c > +++ b/gcc/c-family/c-common.c > @@ -8868,7 +8876,7 @@ braced_lists_to_strings (tree type, tree ctor) > unsigned HOST_WIDE_INT idx; > FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (ctor), idx, val) > { > - val = braced_lists_to_strings (ttp, val); > + val = braced_lists_to_strings (ttp, val, code == RECORD_TYPE); Do you need to handle UNION_TYPE or QUAL_UNION_TYPE here too? If so, RECORD_OR_UNION_TYPE_P is a better test. > diff --git a/gcc/expr.c b/gcc/expr.c > index 92979289e83..d16e0982dcf 100644 > --- a/gcc/expr.c > +++ b/gcc/expr.c > @@ -11402,6 +11402,16 @@ is_aligning_offset (const_tree offset, const_tree exp) > tree > string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree *decl) > { > + tree dummy = NULL_TREE;; > + if (!mem_size) > + mem_size = &dummy; > + > + tree chartype = TREE_TYPE (arg); > + if (POINTER_TYPE_P (chartype)) > + chartype = TREE_TYPE (chartype); > + while (TREE_CODE (chartype) == ARRAY_TYPE) > + chartype = TREE_TYPE (chartype); I'm hesitant to ACK this bit. I can understand that if we're given an ADDR_EXPR, which should be a POINTER_TYPE_P, that we'll want to look at what it's pointing to. But shouldn't we verify that it's an ADDR_EXPR before just blindly stripping away the outer type? I also wonder if this (assuming we keep it in some form) belongs after the STRIP_NOPs. > @@ -11602,17 +11605,39 @@ string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree *decl) > int len = native_encode_expr (init, charbuf, sizeof charbuf, 0); > if (len > 0) > { > - /* Construct a string literal with elements of ELTYPE and > + /* Construct a string literal with elements of INITTYPE and > the representation above. Then strip > the ADDR_EXPR (ARRAY_REF (...)) around the STRING_CST. */ > - init = build_string_literal (len, (char *)charbuf, eltype); > + init = build_string_literal (len, (char *)charbuf, inittype); > init = TREE_OPERAND (TREE_OPERAND (init, 0), 0); > } > } > > + tree initsize = TYPE_SIZE_UNIT (inittype); > + > + if (TREE_CODE (init) == CONSTRUCTOR && !CONSTRUCTOR_ELTS (init)) Would initializer_zerop be better here? That would catch zero-initialized constructors as well. If not, then you want to make sure to check !TREE_CLOBBER_P (init) since that's something completely different than zero initialization. > > return init; > diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c > index d576b0842f9..fcffb9802b7 100644 > --- a/gcc/gimple-fold.c > +++ b/gcc/gimple-fold.c [ ... ] None of the gimple-fold stuff looked strictly necessary for this patch. But gimple-fold changes are OK. I think you need to double-check your ChangeLog as well. There's changes in expr.c that aren't reflected in the ChangeLog. The changes to fold_nonarray_ctor_reference don't seem to match the ChangeLog either. Mostly this looks OK. I think we close on the issues above and this will be ready for the trunk. jeff
On 8/21/19 2:50 PM, Martin Sebor wrote: > This patch is a subset of the solution for PR 91457 whose main > goal is to eliminate inconsistencies in warnings issued for > out-of-bounds accesses to the various flavors of flexible array > members of constant objects. That patch was posted here: > https://gcc.gnu.org/ml/gcc-patches/2019-08/msg01202.html > > Like PR 91457, this patch also relies on improving optimization > to issue better quality warnings. (I.e., with the latter being > what motivated it.) > > The optimization enhances string_constant to recognize empty > CONSTRUCTORs returned by fold_ctor_reference for references > to members of constant objects with either "insufficient" > initializers (e.g., given struct S { char n, a[]; } s = { 0 };) > or with braced-list initializers (e.g., given > struct S s = { 3 { 1, 2, 3, 0 } }; The patch lets string_constant > convert the CONSTRUCTOR for s.a into a STRING_CST, which in turn > enables the folding of calls to built-ins like strlen, strchr, or > strcmp with such arguments. > > Exposing the strings to the folder then also lets it detect and > issue warnings for out-of-bounds offsets in more instances of > such references than before. > > The remaining changes in the patch mostly enhance the places > that use the no-warning bit to prevent redundant diagnostics. > > Tested on x86_64-linux. > > Martin > > gcc-91490.diff > > PR middle-end/91490 - bogus argument missing terminating nul warning on strlen of a flexible array member > > gcc/c-family/ChangeLog: > > PR middle-end/91490 > * c-common.c (braced_list_to_string): Add argument and overload. > Handle flexible length arrays. > > gcc/testsuite/ChangeLog: > > PR middle-end/91490 > * c-c++-common/Warray-bounds-7.c: New test. > * gcc.dg/Warray-bounds-39.c: Expect either -Warray-bounds or > -Wstringop-overflow. > * gcc.dg/strlenopt-78.c: New test. > > gcc/ChangeLog: > > PR middle-end/91490 > * builtins.c (c_strlen): Rename argument and introduce new local. > Set no-warning bit on original argument. > * expr.c (string_constant): Pass argument type to fold_ctor_reference. > * gimple-fold.c (fold_nonarray_ctor_reference): Return a STRING_CST > for missing initializers. > * tree.c (build_string_literal): Handle optional argument. > * tree.h (build_string_literal): Add defaulted argument. > * gimple-ssa-warn-restrict.c (maybe_diag_access_bounds): Check > no-warning bit on original expression. > > diff --git a/gcc/builtins.c b/gcc/builtins.c > --- a/gcc/expr.c > +++ b/gcc/expr.c > @@ -11402,6 +11402,16 @@ is_aligning_offset (const_tree offset, const_tree exp) > tree > string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree *decl) > { > + tree dummy = NULL_TREE;; > + if (!mem_size) > + mem_size = &dummy; > + > + tree chartype = TREE_TYPE (arg); > + if (POINTER_TYPE_P (chartype)) > + chartype = TREE_TYPE (chartype); > + while (TREE_CODE (chartype) == ARRAY_TYPE) > + chartype = TREE_TYPE (chartype); > + > tree array; > STRIP_NOPS (arg); So per our conversation today, I took a closer look at this. As you noted CHARTYPE is only used for the empty constructor code you're adding as a part of this patch. Rather than stripping away types like this to compute chartype, couldn't we just use char_type_node instead of chartype in this code below? > + tree initsize = TYPE_SIZE_UNIT (inittype); > + > + if (TREE_CODE (init) == CONSTRUCTOR && !CONSTRUCTOR_ELTS (init)) > + { > + /* Convert a char array to an empty STRING_CST having an array > + of the expected type. */ > + if (!initsize) > + initsize = integer_zero_node; > + > + unsigned HOST_WIDE_INT size = tree_to_uhwi (initsize); > + init = build_string_literal (size ? 1 : 0, "", chartype, size); > + init = TREE_OPERAND (init, 0); > + init = TREE_OPERAND (init, 0); > + > + *ptr_offset = integer_zero_node; > + } Per my prior message, we do need to update that test to be something like if (TREE_CODE (init) == CONSTRUCTOR && !CONSTRUCTOR_ELTS && !TREE_CLOBBER_P (init)) While I don't think we're likely to see a TREE_CLOBBER_P here, it's best to be safe since those have totally different meanings, > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c > index 2810867235d..ef942ffce57 100644 > --- a/gcc/c-family/c-common.c > +++ b/gcc/c-family/c-common.c > @@ -8868,7 +8876,7 @@ braced_lists_to_strings (tree type, tree ctor) > unsigned HOST_WIDE_INT idx; > FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (ctor), idx, val) > { > - val = braced_lists_to_strings (ttp, val); > + val = braced_lists_to_strings (ttp, val, code == RECORD_TYPE); Also per our discussion, I think a comment that we might want to test RECORD_OR_UNION_TYPE_P here instead of just testing for RECORD_TYPE is all we need. With those changes and a bootstrap/regression test, this is OK. jeff
On 8/21/19 4:28 PM, Jeff Law wrote: > On 8/21/19 2:50 PM, Martin Sebor wrote: >> This patch is a subset of the solution for PR 91457 whose main >> goal is to eliminate inconsistencies in warnings issued for >> out-of-bounds accesses to the various flavors of flexible array >> members of constant objects. That patch was posted here: >> https://gcc.gnu.org/ml/gcc-patches/2019-08/msg01202.html >> >> Like PR 91457, this patch also relies on improving optimization >> to issue better quality warnings. (I.e., with the latter being >> what motivated it.) >> >> The optimization enhances string_constant to recognize empty >> CONSTRUCTORs returned by fold_ctor_reference for references >> to members of constant objects with either "insufficient" >> initializers (e.g., given struct S { char n, a[]; } s = { 0 };) >> or with braced-list initializers (e.g., given >> struct S s = { 3 { 1, 2, 3, 0 } }; The patch lets string_constant >> convert the CONSTRUCTOR for s.a into a STRING_CST, which in turn >> enables the folding of calls to built-ins like strlen, strchr, or >> strcmp with such arguments. >> >> Exposing the strings to the folder then also lets it detect and >> issue warnings for out-of-bounds offsets in more instances of >> such references than before. >> >> The remaining changes in the patch mostly enhance the places >> that use the no-warning bit to prevent redundant diagnostics. >> >> Tested on x86_64-linux. >> >> Martin >> >> gcc-91490.diff >> >> PR middle-end/91490 - bogus argument missing terminating nul warning on strlen of a flexible array member >> >> gcc/c-family/ChangeLog: >> >> PR middle-end/91490 >> * c-common.c (braced_list_to_string): Add argument and overload. >> Handle flexible length arrays. >> >> gcc/testsuite/ChangeLog: >> >> PR middle-end/91490 >> * c-c++-common/Warray-bounds-7.c: New test. >> * gcc.dg/Warray-bounds-39.c: Expect either -Warray-bounds or >> -Wstringop-overflow. >> * gcc.dg/strlenopt-78.c: New test. >> >> gcc/ChangeLog: >> >> PR middle-end/91490 >> * builtins.c (c_strlen): Rename argument and introduce new local. >> Set no-warning bit on original argument. >> * expr.c (string_constant): Pass argument type to fold_ctor_reference. >> * gimple-fold.c (fold_nonarray_ctor_reference): Return a STRING_CST >> for missing initializers. >> * tree.c (build_string_literal): Handle optional argument. >> * tree.h (build_string_literal): Add defaulted argument. >> * gimple-ssa-warn-restrict.c (maybe_diag_access_bounds): Check >> no-warning bit on original expression. >> > >> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c >> index 2810867235d..ef942ffce57 100644 >> --- a/gcc/c-family/c-common.c >> +++ b/gcc/c-family/c-common.c >> @@ -8868,7 +8876,7 @@ braced_lists_to_strings (tree type, tree ctor) >> unsigned HOST_WIDE_INT idx; >> FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (ctor), idx, val) >> { >> - val = braced_lists_to_strings (ttp, val); >> + val = braced_lists_to_strings (ttp, val, code == RECORD_TYPE); > Do you need to handle UNION_TYPE or QUAL_UNION_TYPE here too? If so, > RECORD_OR_UNION_TYPE_P is a better test. I confess I didn't even think about unions. I wouldn't expect to see the definition of a const union but I also can't think of a reason to exclude them from here so I've made the change. > > >> diff --git a/gcc/expr.c b/gcc/expr.c >> index 92979289e83..d16e0982dcf 100644 >> --- a/gcc/expr.c >> +++ b/gcc/expr.c >> @@ -11402,6 +11402,16 @@ is_aligning_offset (const_tree offset, const_tree exp) >> tree >> string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree *decl) >> { >> + tree dummy = NULL_TREE;; >> + if (!mem_size) >> + mem_size = &dummy; >> + >> + tree chartype = TREE_TYPE (arg); >> + if (POINTER_TYPE_P (chartype)) >> + chartype = TREE_TYPE (chartype); >> + while (TREE_CODE (chartype) == ARRAY_TYPE) >> + chartype = TREE_TYPE (chartype); > I'm hesitant to ACK this bit. I can understand that if we're given an > ADDR_EXPR, which should be a POINTER_TYPE_P, that we'll want to look at > what it's pointing to. But shouldn't we verify that it's an ADDR_EXPR > before just blindly stripping away the outer type? The character type is that of the expression, before any casts are stripped. For example, given: struct S { char n, a[4]; } s[1] = { 0 }; and strlen (s->a), ARG is the POINTER_PLUS_EXPR '(const char *) &s + 1' The type of the operand is a pointer to the struct S. > > I also wonder if this (assuming we keep it in some form) belongs after > the STRIP_NOPs. In other expressions, after STRIP_NOPS the type also may not reflect the actual character type. I don't know how else get at the character type when the type of the CONSTRUCTOR is that of an enclosing object. I've moved the code closer to where it's needed but otherwise left it unchanged. >> @@ -11602,17 +11605,39 @@ string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree *decl) >> int len = native_encode_expr (init, charbuf, sizeof charbuf, 0); >> if (len > 0) >> { >> - /* Construct a string literal with elements of ELTYPE and >> + /* Construct a string literal with elements of INITTYPE and >> the representation above. Then strip >> the ADDR_EXPR (ARRAY_REF (...)) around the STRING_CST. */ >> - init = build_string_literal (len, (char *)charbuf, eltype); >> + init = build_string_literal (len, (char *)charbuf, inittype); >> init = TREE_OPERAND (TREE_OPERAND (init, 0), 0); >> } >> } >> >> + tree initsize = TYPE_SIZE_UNIT (inittype); >> + >> + if (TREE_CODE (init) == CONSTRUCTOR && !CONSTRUCTOR_ELTS (init)) > Would initializer_zerop be better here? That would catch > zero-initialized constructors as well. I originally had initializer_zerop there and removed it at the last minute it because none of my tests needed it. But my tests only exercised the flexible array members (which have to be initialized in constant objects) and not the case where initializer_zerop does make a difference such as: struct A { char a[4]; }; struct B { struct A a; }; const struct B b[2] = { 0 }; void f (void) { if (__builtin_strlen (b->a.a) == 3) __builtin_abort (); } Here, fold_ctor_reference returns a non-empty CONSTRUCTOR that the code doesn't handle. Adding initializer_zerop lets it create the empty string and the caller fold the call. But it's just a special case of the more general problem where the CONSTRUCTOR is both non-empty and non-zero for the parts of the object before the array we're interested in, such as in const struct B b[1] = { 1 }; Here, strlen (b->a.a) again isn't folded. Ironically, the equivalent strlen (b[0].a.a) is folded. The difference between the two is that b[0].a.a is represented as itself (i.e., NOP (char*, ADDR (b.a.a)) while b->a.a as (const char *) &b + 1. In the former case fold_ctor_reference returns an empty CONSTRUCTOR for the char array. In the latter, it returns the non-empty, non-zero CTOR for all of b the we don't know how to extract the empty string from. Incidentally, it's only the C front-end that "bastardizes" the expression like this. The C++ FE preserves the original form of the expression and so it's able to fold the call. (Uncovering and trying to fix these problems, by the way, is what I meant the other day when I said how patches in this area have a tendency to snownball into "projects." Folding empty ctors of constant objects probably isn't terribly important but the lack of consistency is what makes writing tests that behave the same way across different front-ends and back-ends challenging.) > If not, then you want to make sure to check !TREE_CLOBBER_P (init) since > that's something completely different than zero initialization. I went with the initializer_zerop since it improves things, if negligibly, and added tests for it. If you or Richard have a test case that I could add to the patch showing when TREE_CLOBBER_P is set on a CTOR and that not checking would cause trouble. >> return init; >> diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c >> index d576b0842f9..fcffb9802b7 100644 >> --- a/gcc/gimple-fold.c >> +++ b/gcc/gimple-fold.c > [ ... ] > None of the gimple-fold stuff looked strictly necessary for this patch. > But gimple-fold changes are OK. > > > I think you need to double-check your ChangeLog as well. There's > changes in expr.c that aren't reflected in the ChangeLog. The changes > to fold_nonarray_ctor_reference don't seem to match the ChangeLog either. I've updated it. > > Mostly this looks OK. I think we close on the issues above and this > will be ready for the trunk. Attached is the updated/retested patch. Martin
On 8/22/19 3:55 PM, Martin Sebor wrote: >>> diff --git a/gcc/expr.c b/gcc/expr.c >>> index 92979289e83..d16e0982dcf 100644 >>> --- a/gcc/expr.c >>> +++ b/gcc/expr.c >>> @@ -11402,6 +11402,16 @@ is_aligning_offset (const_tree offset, >>> const_tree exp) >>> tree >>> string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree >>> *decl) >>> { >>> + tree dummy = NULL_TREE;; >>> + if (!mem_size) >>> + mem_size = &dummy; >>> + >>> + tree chartype = TREE_TYPE (arg); >>> + if (POINTER_TYPE_P (chartype)) >>> + chartype = TREE_TYPE (chartype); >>> + while (TREE_CODE (chartype) == ARRAY_TYPE) >>> + chartype = TREE_TYPE (chartype); >> I'm hesitant to ACK this bit. I can understand that if we're given an >> ADDR_EXPR, which should be a POINTER_TYPE_P, that we'll want to look at >> what it's pointing to. But shouldn't we verify that it's an ADDR_EXPR >> before just blindly stripping away the outer type? > > The character type is that of the expression, before any casts > are stripped. For example, given: > > struct S { char n, a[4]; } s[1] = { 0 }; > > and strlen (s->a), ARG is the POINTER_PLUS_EXPR > '(const char *) &s + 1' The type of the operand is a pointer to > the struct S. Given "chartype" is only used in the CONSTRUCTOR handling you're adding, couldn't we just use "char_type_node" in there instead and drop the bits noted above? >>> + if (TREE_CODE (init) == CONSTRUCTOR && !CONSTRUCTOR_ELTS (init)) >> Would initializer_zerop be better here? That would catch >> zero-initialized constructors as well. > > I originally had initializer_zerop there and removed it at the last > minute it because none of my tests needed it. But my tests only > exercised the flexible array members (which have to be initialized > in constant objects) and not the case where initializer_zerop does > make a difference such as: > > struct A { char a[4]; }; > struct B { struct A a; }; > > const struct B b[2] = { 0 }; > > void f (void) > { > if (__builtin_strlen (b->a.a) == 3) > __builtin_abort (); > } > > Here, fold_ctor_reference returns a non-empty CONSTRUCTOR that > the code doesn't handle. Adding initializer_zerop lets it create > the empty string and the caller fold the call. But it's just > a special case of the more general problem where the CONSTRUCTOR > is both non-empty and non-zero for the parts of the object before > the array we're interested in, such as in > > const struct B b[1] = { 1 }; > > Here, strlen (b->a.a) again isn't folded. Ironically, the equivalent > strlen (b[0].a.a) is folded. The difference between the two is that > b[0].a.a is represented as itself (i.e., NOP (char*, ADDR (b.a.a)) > while b->a.a as (const char *) &b + 1. In the former case > fold_ctor_reference returns an empty CONSTRUCTOR for the char array. > In the latter, it returns the non-empty, non-zero CTOR for all of > b the we don't know how to extract the empty string from. > > Incidentally, it's only the C front-end that "bastardizes" > the expression like this. The C++ FE preserves the original form > of the expression and so it's able to fold the call. > > (Uncovering and trying to fix these problems, by the way, is what > I meant the other day when I said how patches in this area have > a tendency to snownball into "projects." Folding empty ctors of > constant objects probably isn't terribly important but the lack > of consistency is what makes writing tests that behave the same > way across different front-ends and back-ends challenging.) Yup. I understand. When presented with these kinds of snowballing issues, the best advice I can give is to break things down into logical units and make patches which are a series rather than a single unified patch to address multiple issues. git rebase can really help. > >> If not, then you want to make sure to check !TREE_CLOBBER_P (init) since >> that's something completely different than zero initialization. > > I went with the initializer_zerop since it improves things, if > negligibly, and added tests for it. If you or Richard have a test > case that I could add to the patch showing when TREE_CLOBBER_P is > set on a CTOR and that not checking would cause trouble. Note that initializer_zerop checks TREE_CLOBBER internally. So if you're using initializer_zerop, then you're good. So all that's left is the "chartype" stuff in string_constant. If we can use char_type_node instead of trying to extract a character type, then the problematical code would just go away. jeff
On 8/22/19 3:27 PM, Jeff Law wrote: > On 8/21/19 2:50 PM, Martin Sebor wrote: >> This patch is a subset of the solution for PR 91457 whose main >> goal is to eliminate inconsistencies in warnings issued for >> out-of-bounds accesses to the various flavors of flexible array >> members of constant objects. That patch was posted here: >> https://gcc.gnu.org/ml/gcc-patches/2019-08/msg01202.html >> >> Like PR 91457, this patch also relies on improving optimization >> to issue better quality warnings. (I.e., with the latter being >> what motivated it.) >> >> The optimization enhances string_constant to recognize empty >> CONSTRUCTORs returned by fold_ctor_reference for references >> to members of constant objects with either "insufficient" >> initializers (e.g., given struct S { char n, a[]; } s = { 0 };) >> or with braced-list initializers (e.g., given >> struct S s = { 3 { 1, 2, 3, 0 } }; The patch lets string_constant >> convert the CONSTRUCTOR for s.a into a STRING_CST, which in turn >> enables the folding of calls to built-ins like strlen, strchr, or >> strcmp with such arguments. >> >> Exposing the strings to the folder then also lets it detect and >> issue warnings for out-of-bounds offsets in more instances of >> such references than before. >> >> The remaining changes in the patch mostly enhance the places >> that use the no-warning bit to prevent redundant diagnostics. >> >> Tested on x86_64-linux. >> >> Martin >> >> gcc-91490.diff >> >> PR middle-end/91490 - bogus argument missing terminating nul warning on strlen of a flexible array member >> >> gcc/c-family/ChangeLog: >> >> PR middle-end/91490 >> * c-common.c (braced_list_to_string): Add argument and overload. >> Handle flexible length arrays. >> >> gcc/testsuite/ChangeLog: >> >> PR middle-end/91490 >> * c-c++-common/Warray-bounds-7.c: New test. >> * gcc.dg/Warray-bounds-39.c: Expect either -Warray-bounds or >> -Wstringop-overflow. >> * gcc.dg/strlenopt-78.c: New test. >> >> gcc/ChangeLog: >> >> PR middle-end/91490 >> * builtins.c (c_strlen): Rename argument and introduce new local. >> Set no-warning bit on original argument. >> * expr.c (string_constant): Pass argument type to fold_ctor_reference. >> * gimple-fold.c (fold_nonarray_ctor_reference): Return a STRING_CST >> for missing initializers. >> * tree.c (build_string_literal): Handle optional argument. >> * tree.h (build_string_literal): Add defaulted argument. >> * gimple-ssa-warn-restrict.c (maybe_diag_access_bounds): Check >> no-warning bit on original expression. >> >> diff --git a/gcc/builtins.c b/gcc/builtins.c >> --- a/gcc/expr.c >> +++ b/gcc/expr.c >> @@ -11402,6 +11402,16 @@ is_aligning_offset (const_tree offset, const_tree exp) >> tree >> string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree *decl) >> { >> + tree dummy = NULL_TREE;; >> + if (!mem_size) >> + mem_size = &dummy; >> + >> + tree chartype = TREE_TYPE (arg); >> + if (POINTER_TYPE_P (chartype)) >> + chartype = TREE_TYPE (chartype); >> + while (TREE_CODE (chartype) == ARRAY_TYPE) >> + chartype = TREE_TYPE (chartype); >> + >> tree array; >> STRIP_NOPS (arg); > So per our conversation today, I took a closer look at this. As you > noted CHARTYPE is only used for the empty constructor code you're adding > as a part of this patch. > > Rather than stripping away types like this to compute chartype, couldn't > we just use char_type_node instead of chartype in this code below? We can't. string_constant is also called for wide strings (e.g., by the sprintf pass). Returning a narrow string when the caller asks for a wide one breaks the sprintf stuff. >> + tree initsize = TYPE_SIZE_UNIT (inittype); >> + >> + if (TREE_CODE (init) == CONSTRUCTOR && !CONSTRUCTOR_ELTS (init)) >> + { >> + /* Convert a char array to an empty STRING_CST having an array >> + of the expected type. */ >> + if (!initsize) >> + initsize = integer_zero_node; >> + >> + unsigned HOST_WIDE_INT size = tree_to_uhwi (initsize); >> + init = build_string_literal (size ? 1 : 0, "", chartype, size); >> + init = TREE_OPERAND (init, 0); >> + init = TREE_OPERAND (init, 0); >> + >> + *ptr_offset = integer_zero_node; >> + } > Per my prior message, we do need to update that test to be something like > > if (TREE_CODE (init) == CONSTRUCTOR > && !CONSTRUCTOR_ELTS > && !TREE_CLOBBER_P (init)) > > While I don't think we're likely to see a TREE_CLOBBER_P here, it's best > to be safe since those have totally different meanings, I went with the other alternative you mentioned and used initializer_zerop as I explained in my reply. Let me know if that's not what you meant. > >> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c >> index 2810867235d..ef942ffce57 100644 >> --- a/gcc/c-family/c-common.c >> +++ b/gcc/c-family/c-common.c >> @@ -8868,7 +8876,7 @@ braced_lists_to_strings (tree type, tree ctor) >> unsigned HOST_WIDE_INT idx; >> FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (ctor), idx, val) >> { >> - val = braced_lists_to_strings (ttp, val); >> + val = braced_lists_to_strings (ttp, val, code == RECORD_TYPE); > Also per our discussion, I think a comment that we might want to test > RECORD_OR_UNION_TYPE_P here instead of just testing for RECORD_TYPE is > all we need. > > With those changes and a bootstrap/regression test, this is OK. I just sent an updated patch with most of these changes. I'll wait to until tomorrow to commit it in cases there are further comments. Thanks Martin
On 8/22/19 4:23 PM, Martin Sebor wrote: > On 8/22/19 3:27 PM, Jeff Law wrote: >> On 8/21/19 2:50 PM, Martin Sebor wrote: >>> This patch is a subset of the solution for PR 91457 whose main >>> goal is to eliminate inconsistencies in warnings issued for >>> out-of-bounds accesses to the various flavors of flexible array >>> members of constant objects. That patch was posted here: >>> https://gcc.gnu.org/ml/gcc-patches/2019-08/msg01202.html >>> >>> Like PR 91457, this patch also relies on improving optimization >>> to issue better quality warnings. (I.e., with the latter being >>> what motivated it.) >>> >>> The optimization enhances string_constant to recognize empty >>> CONSTRUCTORs returned by fold_ctor_reference for references >>> to members of constant objects with either "insufficient" >>> initializers (e.g., given struct S { char n, a[]; } s = { 0 };) >>> or with braced-list initializers (e.g., given >>> struct S s = { 3 { 1, 2, 3, 0 } }; The patch lets string_constant >>> convert the CONSTRUCTOR for s.a into a STRING_CST, which in turn >>> enables the folding of calls to built-ins like strlen, strchr, or >>> strcmp with such arguments. >>> >>> Exposing the strings to the folder then also lets it detect and >>> issue warnings for out-of-bounds offsets in more instances of >>> such references than before. >>> >>> The remaining changes in the patch mostly enhance the places >>> that use the no-warning bit to prevent redundant diagnostics. >>> >>> Tested on x86_64-linux. >>> >>> Martin >>> >>> gcc-91490.diff >>> >>> PR middle-end/91490 - bogus argument missing terminating nul warning >>> on strlen of a flexible array member >>> >>> gcc/c-family/ChangeLog: >>> >>> PR middle-end/91490 >>> * c-common.c (braced_list_to_string): Add argument and overload. >>> Handle flexible length arrays. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR middle-end/91490 >>> * c-c++-common/Warray-bounds-7.c: New test. >>> * gcc.dg/Warray-bounds-39.c: Expect either -Warray-bounds or >>> -Wstringop-overflow. >>> * gcc.dg/strlenopt-78.c: New test. >>> >>> gcc/ChangeLog: >>> >>> PR middle-end/91490 >>> * builtins.c (c_strlen): Rename argument and introduce new local. >>> Set no-warning bit on original argument. >>> * expr.c (string_constant): Pass argument type to >>> fold_ctor_reference. >>> * gimple-fold.c (fold_nonarray_ctor_reference): Return a STRING_CST >>> for missing initializers. >>> * tree.c (build_string_literal): Handle optional argument. >>> * tree.h (build_string_literal): Add defaulted argument. >>> * gimple-ssa-warn-restrict.c (maybe_diag_access_bounds): Check >>> no-warning bit on original expression. >>> >>> diff --git a/gcc/builtins.c b/gcc/builtins.c >>> --- a/gcc/expr.c >>> +++ b/gcc/expr.c >>> @@ -11402,6 +11402,16 @@ is_aligning_offset (const_tree offset, >>> const_tree exp) >>> tree >>> string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree >>> *decl) >>> { >>> + tree dummy = NULL_TREE;; >>> + if (!mem_size) >>> + mem_size = &dummy; >>> + >>> + tree chartype = TREE_TYPE (arg); >>> + if (POINTER_TYPE_P (chartype)) >>> + chartype = TREE_TYPE (chartype); >>> + while (TREE_CODE (chartype) == ARRAY_TYPE) >>> + chartype = TREE_TYPE (chartype); >>> + >>> tree array; >>> STRIP_NOPS (arg); >> So per our conversation today, I took a closer look at this. As you >> noted CHARTYPE is only used for the empty constructor code you're adding >> as a part of this patch. >> >> Rather than stripping away types like this to compute chartype, couldn't >> we just use char_type_node instead of chartype in this code below? > > We can't. string_constant is also called for wide strings (e.g., > by the sprintf pass). Returning a narrow string when the caller > asks for a wide one breaks the sprintf stuff. Sigh. And presumably we can't just move the block down because other bits in string_constant modify ARG. So I think a quick comment before that fragment about its purpose and we're good to go for the patch as a whole based on the one you posted an hour or so ago. Thanks, jeff
On 8/22/19 4:37 PM, Jeff Law wrote: > On 8/22/19 4:23 PM, Martin Sebor wrote: >> On 8/22/19 3:27 PM, Jeff Law wrote: >>> On 8/21/19 2:50 PM, Martin Sebor wrote: >>>> This patch is a subset of the solution for PR 91457 whose main >>>> goal is to eliminate inconsistencies in warnings issued for >>>> out-of-bounds accesses to the various flavors of flexible array >>>> members of constant objects. That patch was posted here: >>>> https://gcc.gnu.org/ml/gcc-patches/2019-08/msg01202.html >>>> >>>> Like PR 91457, this patch also relies on improving optimization >>>> to issue better quality warnings. (I.e., with the latter being >>>> what motivated it.) >>>> >>>> The optimization enhances string_constant to recognize empty >>>> CONSTRUCTORs returned by fold_ctor_reference for references >>>> to members of constant objects with either "insufficient" >>>> initializers (e.g., given struct S { char n, a[]; } s = { 0 };) >>>> or with braced-list initializers (e.g., given >>>> struct S s = { 3 { 1, 2, 3, 0 } }; The patch lets string_constant >>>> convert the CONSTRUCTOR for s.a into a STRING_CST, which in turn >>>> enables the folding of calls to built-ins like strlen, strchr, or >>>> strcmp with such arguments. >>>> >>>> Exposing the strings to the folder then also lets it detect and >>>> issue warnings for out-of-bounds offsets in more instances of >>>> such references than before. >>>> >>>> The remaining changes in the patch mostly enhance the places >>>> that use the no-warning bit to prevent redundant diagnostics. >>>> >>>> Tested on x86_64-linux. >>>> >>>> Martin >>>> >>>> gcc-91490.diff >>>> >>>> PR middle-end/91490 - bogus argument missing terminating nul warning >>>> on strlen of a flexible array member >>>> >>>> gcc/c-family/ChangeLog: >>>> >>>> PR middle-end/91490 >>>> * c-common.c (braced_list_to_string): Add argument and overload. >>>> Handle flexible length arrays. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> PR middle-end/91490 >>>> * c-c++-common/Warray-bounds-7.c: New test. >>>> * gcc.dg/Warray-bounds-39.c: Expect either -Warray-bounds or >>>> -Wstringop-overflow. >>>> * gcc.dg/strlenopt-78.c: New test. >>>> >>>> gcc/ChangeLog: >>>> >>>> PR middle-end/91490 >>>> * builtins.c (c_strlen): Rename argument and introduce new local. >>>> Set no-warning bit on original argument. >>>> * expr.c (string_constant): Pass argument type to >>>> fold_ctor_reference. >>>> * gimple-fold.c (fold_nonarray_ctor_reference): Return a STRING_CST >>>> for missing initializers. >>>> * tree.c (build_string_literal): Handle optional argument. >>>> * tree.h (build_string_literal): Add defaulted argument. >>>> * gimple-ssa-warn-restrict.c (maybe_diag_access_bounds): Check >>>> no-warning bit on original expression. >>>> >>>> diff --git a/gcc/builtins.c b/gcc/builtins.c >>>> --- a/gcc/expr.c >>>> +++ b/gcc/expr.c >>>> @@ -11402,6 +11402,16 @@ is_aligning_offset (const_tree offset, >>>> const_tree exp) >>>> tree >>>> string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree >>>> *decl) >>>> { >>>> + tree dummy = NULL_TREE;; >>>> + if (!mem_size) >>>> + mem_size = &dummy; >>>> + >>>> + tree chartype = TREE_TYPE (arg); >>>> + if (POINTER_TYPE_P (chartype)) >>>> + chartype = TREE_TYPE (chartype); >>>> + while (TREE_CODE (chartype) == ARRAY_TYPE) >>>> + chartype = TREE_TYPE (chartype); >>>> + >>>> tree array; >>>> STRIP_NOPS (arg); >>> So per our conversation today, I took a closer look at this. As you >>> noted CHARTYPE is only used for the empty constructor code you're adding >>> as a part of this patch. >>> >>> Rather than stripping away types like this to compute chartype, couldn't >>> we just use char_type_node instead of chartype in this code below? >> >> We can't. string_constant is also called for wide strings (e.g., >> by the sprintf pass). Returning a narrow string when the caller >> asks for a wide one breaks the sprintf stuff. > Sigh. And presumably we can't just move the block down because other > bits in string_constant modify ARG. > > So I think a quick comment before that fragment about its purpose and > we're good to go for the patch as a whole based on the one you posted an > hour or so ago. I did move it down into the block with the STRING_CST transform. A comment is also there so I committed the patch in r274837. Thanks Martin
PR middle-end/91490 - bogus argument missing terminating nul warning on strlen of a flexible array member gcc/c-family/ChangeLog: PR middle-end/91490 * c-common.c (braced_list_to_string): Add argument and overload. Handle flexible length arrays. gcc/testsuite/ChangeLog: PR middle-end/91490 * c-c++-common/Warray-bounds-7.c: New test. * gcc.dg/Warray-bounds-39.c: Expect either -Warray-bounds or -Wstringop-overflow. * gcc.dg/strlenopt-78.c: New test. gcc/ChangeLog: PR middle-end/91490 * builtins.c (c_strlen): Rename argument and introduce new local. Set no-warning bit on original argument. * expr.c (string_constant): Pass argument type to fold_ctor_reference. * gimple-fold.c (fold_nonarray_ctor_reference): Return a STRING_CST for missing initializers. * tree.c (build_string_literal): Handle optional argument. * tree.h (build_string_literal): Add defaulted argument. * gimple-ssa-warn-restrict.c (maybe_diag_access_bounds): Check no-warning bit on original expression. diff --git a/gcc/builtins.c b/gcc/builtins.c index 9a766e4ad63..073b92a1dc9 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -620,7 +620,7 @@ unterminated_array (tree exp, tree *size /* = NULL */, bool *exact /* = NULL */) into the instruction stream and zero if it is going to be expanded. E.g. with i++ ? "foo" : "bar", if ONLY_VALUE is nonzero, constant 3 is returned, otherwise NULL, since - len = c_strlen (src, 1); if (len) expand_expr (len, ...); would not + len = c_strlen (ARG, 1); if (len) expand_expr (len, ...); would not evaluate the side-effects. If ONLY_VALUE is two then we do not emit warnings about out-of-bound @@ -628,7 +628,7 @@ unterminated_array (tree exp, tree *size /* = NULL */, bool *exact /* = NULL */) into the instruction stream. Additional information about the string accessed may be recorded - in DATA. For example, if SRC references an unterminated string, + in DATA. For example, if ARG references an unterminated string, then the declaration will be stored in the DECL field. If the length of the unterminated string can be determined, it'll be stored in the LEN field. Note this length could well be different @@ -640,7 +640,7 @@ unterminated_array (tree exp, tree *size /* = NULL */, bool *exact /* = NULL */) The value returned is of type `ssizetype'. */ tree -c_strlen (tree src, int only_value, c_strlen_data *data, unsigned eltsize) +c_strlen (tree arg, int only_value, c_strlen_data *data, unsigned eltsize) { /* If we were not passed a DATA pointer, then get one to a local structure. That avoids having to check DATA for NULL before @@ -650,7 +650,8 @@ c_strlen (tree src, int only_value, c_strlen_data *data, unsigned eltsize) data = &local_strlen_data; gcc_checking_assert (eltsize == 1 || eltsize == 2 || eltsize == 4); - STRIP_NOPS (src); + + tree src = STRIP_NOPS (arg); if (TREE_CODE (src) == COND_EXPR && (only_value || !TREE_SIDE_EFFECTS (TREE_OPERAND (src, 0)))) { @@ -762,11 +763,15 @@ c_strlen (tree src, int only_value, c_strlen_data *data, unsigned eltsize) { /* Suppress multiple warnings for propagated constant strings. */ if (only_value != 2 - && !TREE_NO_WARNING (src) + && !TREE_NO_WARNING (arg) && warning_at (loc, OPT_Warray_bounds, "offset %qwi outside bounds of constant string", eltoff)) - TREE_NO_WARNING (src) = 1; + { + if (decl) + inform (DECL_SOURCE_LOCATION (decl), "%qE declared here", decl); + TREE_NO_WARNING (arg) = 1; + } return NULL_TREE; } diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 2810867235d..ef942ffce57 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -8747,9 +8747,12 @@ maybe_add_include_fixit (rich_location *richloc, const char *header, the converted string on success or the original ctor on failure. */ static tree -braced_list_to_string (tree type, tree ctor) +braced_list_to_string (tree type, tree ctor, bool member) { - if (!tree_fits_uhwi_p (TYPE_SIZE_UNIT (type))) + /* Ignore non-members with unknown size like arrays with unspecified + bound. */ + tree typesize = TYPE_SIZE_UNIT (type); + if (!member && !tree_fits_uhwi_p (typesize)) return ctor; /* If the array has an explicit bound, use it to constrain the size @@ -8757,10 +8760,17 @@ braced_list_to_string (tree type, tree ctor) as long as implied by the index of the last zero specified via a designator, as in: const char a[] = { [7] = 0 }; */ - unsigned HOST_WIDE_INT maxelts = tree_to_uhwi (TYPE_SIZE_UNIT (type)); - maxelts /= tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (type))); + unsigned HOST_WIDE_INT maxelts; + if (typesize) + { + maxelts = tree_to_uhwi (typesize); + maxelts /= tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (type))); + } + else + maxelts = HOST_WIDE_INT_M1U; - /* Avoid converting initializers for zero-length arrays. */ + /* Avoid converting initializers for zero-length arrays (but do + create them for flexible array members). */ if (!maxelts) return ctor; @@ -8818,7 +8828,7 @@ braced_list_to_string (tree type, tree ctor) } /* Append a nul string termination. */ - if (str.length () < maxelts) + if (maxelts != HOST_WIDE_INT_M1U && str.length () < maxelts) str.safe_push (0); /* Build a STRING_CST with the same type as the array. */ @@ -8827,14 +8837,12 @@ braced_list_to_string (tree type, tree ctor) return res; } -/* Attempt to convert a CTOR containing braced array initializer lists - for array TYPE into one containing STRING_CSTs, for convenience and - efficiency. Recurse for arrays of arrays and member initializers. - Return the converted CTOR or STRING_CST on success or the original - CTOR otherwise. */ +/* Implementation of the two-argument braced_lists_to_string withe + the same arguments plus MEMBER which is set for struct members + to allow initializers for flexible member arrays. */ -tree -braced_lists_to_strings (tree type, tree ctor) +static tree +braced_lists_to_strings (tree type, tree ctor, bool member) { if (TREE_CODE (ctor) != CONSTRUCTOR) return ctor; @@ -8858,7 +8866,7 @@ braced_lists_to_strings (tree type, tree ctor) if ((TREE_CODE (ttp) == ARRAY_TYPE || TREE_CODE (ttp) == INTEGER_TYPE) && TYPE_STRING_FLAG (ttp)) - return braced_list_to_string (type, ctor); + return braced_list_to_string (type, ctor, member); code = TREE_CODE (ttp); if (code == ARRAY_TYPE || code == RECORD_TYPE) @@ -8868,7 +8876,7 @@ braced_lists_to_strings (tree type, tree ctor) unsigned HOST_WIDE_INT idx; FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (ctor), idx, val) { - val = braced_lists_to_strings (ttp, val); + val = braced_lists_to_strings (ttp, val, code == RECORD_TYPE); CONSTRUCTOR_ELT (ctor, idx)->value = val; } } @@ -8876,4 +8884,16 @@ braced_lists_to_strings (tree type, tree ctor) return ctor; } +/* Attempt to convert a CTOR containing braced array initializer lists + for array TYPE into one containing STRING_CSTs, for convenience and + efficiency. Recurse for arrays of arrays and member initializers. + Return the converted CTOR or STRING_CST on success or the original + CTOR otherwise. */ + +tree +braced_lists_to_strings (tree type, tree ctor) +{ + return braced_lists_to_strings (type, ctor, false); +} + #include "gt-c-family-c-common.h" diff --git a/gcc/expr.c b/gcc/expr.c index 92979289e83..d16e0982dcf 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -11402,6 +11402,16 @@ is_aligning_offset (const_tree offset, const_tree exp) tree string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree *decl) { + tree dummy = NULL_TREE;; + if (!mem_size) + mem_size = &dummy; + + tree chartype = TREE_TYPE (arg); + if (POINTER_TYPE_P (chartype)) + chartype = TREE_TYPE (chartype); + while (TREE_CODE (chartype) == ARRAY_TYPE) + chartype = TREE_TYPE (chartype); + tree array; STRIP_NOPS (arg); @@ -11464,7 +11474,7 @@ string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree *decl) && TREE_CODE (TREE_TYPE (TREE_TYPE (arg))) == ARRAY_TYPE && !(decl && !*decl) && !(decl && tree_fits_uhwi_p (DECL_SIZE_UNIT (*decl)) - && mem_size && tree_fits_uhwi_p (*mem_size) + && tree_fits_uhwi_p (*mem_size) && tree_int_cst_equal (*mem_size, DECL_SIZE_UNIT (*decl)))) return NULL_TREE; @@ -11496,7 +11506,7 @@ string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree *decl) && TREE_CODE (TREE_TYPE (TREE_TYPE (rhs1))) == ARRAY_TYPE && !(decl && !*decl) && !(decl && tree_fits_uhwi_p (DECL_SIZE_UNIT (*decl)) - && mem_size && tree_fits_uhwi_p (*mem_size) + && tree_fits_uhwi_p (*mem_size) && tree_int_cst_equal (*mem_size, DECL_SIZE_UNIT (*decl)))) return NULL_TREE; @@ -11530,8 +11540,7 @@ string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree *decl) if (TREE_CODE (array) == STRING_CST) { *ptr_offset = fold_convert (sizetype, offset); - if (mem_size) - *mem_size = TYPE_SIZE_UNIT (TREE_TYPE (array)); + *mem_size = TYPE_SIZE_UNIT (TREE_TYPE (array)); if (decl) *decl = NULL_TREE; gcc_checking_assert (tree_to_shwi (TYPE_SIZE_UNIT (TREE_TYPE (array))) @@ -11561,7 +11570,7 @@ string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree *decl) base_off = wioff.to_uhwi (); unsigned HOST_WIDE_INT fieldoff = 0; - init = fold_ctor_reference (NULL_TREE, init, base_off, 0, array, + init = fold_ctor_reference (TREE_TYPE (arg), init, base_off, 0, array, &fieldoff); HOST_WIDE_INT cstoff; if (!base_off.is_constant (&cstoff)) @@ -11580,17 +11589,11 @@ string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree *decl) *ptr_offset = offset; - tree eltype = TREE_TYPE (init); - tree initsize = TYPE_SIZE_UNIT (eltype); - if (mem_size) - *mem_size = initsize; - - if (decl) - *decl = array; + tree inittype = TREE_TYPE (init); if (TREE_CODE (init) == INTEGER_CST && (TREE_CODE (TREE_TYPE (array)) == INTEGER_TYPE - || TYPE_MAIN_VARIANT (eltype) == char_type_node)) + || TYPE_MAIN_VARIANT (inittype) == char_type_node)) { /* For a reference to (address of) a single constant character, store the native representation of the character in CHARBUF. @@ -11602,17 +11605,39 @@ string_constant (tree arg, tree *ptr_offset, tree *mem_size, tree *decl) int len = native_encode_expr (init, charbuf, sizeof charbuf, 0); if (len > 0) { - /* Construct a string literal with elements of ELTYPE and + /* Construct a string literal with elements of INITTYPE and the representation above. Then strip the ADDR_EXPR (ARRAY_REF (...)) around the STRING_CST. */ - init = build_string_literal (len, (char *)charbuf, eltype); + init = build_string_literal (len, (char *)charbuf, inittype); init = TREE_OPERAND (TREE_OPERAND (init, 0), 0); } } + tree initsize = TYPE_SIZE_UNIT (inittype); + + if (TREE_CODE (init) == CONSTRUCTOR && !CONSTRUCTOR_ELTS (init)) + { + /* Convert a char array to an empty STRING_CST having an array + of the expected type. */ + if (!initsize) + initsize = integer_zero_node; + + unsigned HOST_WIDE_INT size = tree_to_uhwi (initsize); + init = build_string_literal (size ? 1 : 0, "", chartype, size); + init = TREE_OPERAND (init, 0); + init = TREE_OPERAND (init, 0); + + *ptr_offset = integer_zero_node; + } + + if (decl) + *decl = array; + if (TREE_CODE (init) != STRING_CST) return NULL_TREE; + *mem_size = initsize; + gcc_checking_assert (tree_to_shwi (initsize) >= TREE_STRING_LENGTH (init)); return init; diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index d576b0842f9..fcffb9802b7 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -759,11 +759,13 @@ gimple_fold_builtin_memory_op (gimple_stmt_iterator *gsi, dest_align = get_pointer_alignment (dest); if (tree_fits_uhwi_p (len) && compare_tree_int (len, MOVE_MAX) <= 0 - /* ??? Don't transform copies from strings with known length this - confuses the tree-ssa-strlen.c. This doesn't handle - the case in gcc.dg/strlenopt-8.c which is XFAILed for that - reason. */ - && !c_strlen (src, 2) + /* FIXME: Don't transform copies from strings with known length. + Until GCC 9 this prevented a case in gcc.dg/strlenopt-8.c + from being handled, and the case was XFAILed for that reason. + Now that it is handled and the XFAIL removed, as soon as other + strlenopt tests that rely on it for passing are adjusted, this + hack can be removed. */ + && !c_strlen (src, 1) && !((tmp_str = c_getstr (src, &tmp_len)) != NULL && memchr (tmp_str, 0, tmp_len) == NULL)) { @@ -6969,12 +6971,15 @@ fold_nonarray_ctor_reference (tree type, tree ctor, from_decl, suboff); } } - /* Memory not explicitly mentioned in constructor is 0. */ - return type ? build_zero_cst (type) : NULL_TREE; + + if (!type) + return NULL_TREE; + + return build_zero_cst (type); } /* CTOR is value initializing memory. Fold a reference of TYPE and - bit size POLY_SIZE to the memory at bit POLY_OFFSET. When SIZE + bit size POLY_SIZE to the memory at bit POLY_OFFSET. When POLY_SIZE is zero, attempt to fold a reference to the entire subobject which OFFSET refers to. This is used when folding accesses to string members of aggregates. When non-null, set *SUBOFF to diff --git a/gcc/gimple-ssa-warn-restrict.c b/gcc/gimple-ssa-warn-restrict.c index 64175f258ae..cbfc47894dc 100644 --- a/gcc/gimple-ssa-warn-restrict.c +++ b/gcc/gimple-ssa-warn-restrict.c @@ -1678,7 +1678,8 @@ maybe_diag_access_bounds (location_t loc, gimple *call, tree func, int strict, if (!warn_array_bounds) return false; - if (ref.ref && TREE_NO_WARNING (ref.ref)) + if (TREE_NO_WARNING (ref.ptr) + || (ref.ref && TREE_NO_WARNING (ref.ref))) return false; if (EXPR_HAS_LOCATION (ref.ptr)) diff --git a/gcc/testsuite/c-c++-common/Warray-bounds-7.c b/gcc/testsuite/c-c++-common/Warray-bounds-7.c new file mode 100644 index 00000000000..668e8096200 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Warray-bounds-7.c @@ -0,0 +1,107 @@ +/* PR middle-end/91490 - bogus argument missing terminating nul warning + on strlen of a flexible array member + { dg-do compile } + { dg-options "-Wall -ftrack-macro-expansion=0" } */ + +#define INT_MAX __INT_MAX__ +#define PTRDIFF_MAX __PTRDIFF_MAX__ +#define SIZE_MAX __SIZE_MAX__ + +struct A0 { char n, a[0]; }; +struct A1 { char n, a[1]; }; +struct Ax { char n, a[]; }; + +const struct A0 a0 = { }; +const struct A0 a0_0 = { 0 }; +const struct A0 a0_0_ = { 0, { } }; + +const struct A0 a1 = { }; +const struct A0 a1_0 = { 0 }; +const struct A0 a1_0_ = { 0, { } }; + +const struct Ax ax= { }; +const struct Ax ax_0 = { 0 }; +const struct Ax ax_0_ = { 0, { } }; + +void sink (unsigned); + +#define T(x) sink (__builtin_strlen (x)) + +void test_zero_length_array (void) +{ + T (a0.a); // { dg-warning "\\\[-Warray-bounds" } + T (a0.a - 1); // { dg-warning "\\\[-Warray-bounds" } + T (a0.a + 1); // { dg-warning "\\\[-Warray-bounds" } + T (a0.a + 9); // { dg-warning "\\\[-Warray-bounds" } + T (a0.a + INT_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (a0.a + PTRDIFF_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (a0.a + SIZE_MAX); // { dg-warning "\\\[-Warray-bounds" } + + T (a0_0.a); // { dg-warning "\\\[-Warray-bounds" } + T (a0_0.a - 1); // { dg-warning "\\\[-Warray-bounds" } + T (a0_0.a + 1); // { dg-warning "\\\[-Warray-bounds" } + T (a0_0.a + 9); // { dg-warning "\\\[-Warray-bounds" } + T (a0_0.a + INT_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (a0_0.a + PTRDIFF_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (a0_0.a + SIZE_MAX); // { dg-warning "\\\[-Warray-bounds" } + + T (a0_0_.a); // { dg-warning "\\\[-Warray-bounds" } + T (a0_0_.a - 1); // { dg-warning "\\\[-Warray-bounds" } + T (a0_0_.a + 1); // { dg-warning "\\\[-Warray-bounds" } + T (a0_0_.a + 9); // { dg-warning "\\\[-Warray-bounds" } + T (a0_0_.a + INT_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (a0_0_.a + PTRDIFF_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (a0_0_.a + SIZE_MAX); // { dg-warning "\\\[-Warray-bounds" } +} + +void test_one_element_array (void) +{ + T (a1.a - 1); // { dg-warning "\\\[-Warray-bounds" } + T (a1.a + 1); // { dg-warning "\\\[-Warray-bounds" } + T (a1.a + 9); // { dg-warning "\\\[-Warray-bounds" } + T (a1.a + INT_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (a1.a + PTRDIFF_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (a1.a + SIZE_MAX); // { dg-warning "\\\[-Warray-bounds" } + + T (a1_0.a - 1); // { dg-warning "\\\[-Warray-bounds" } + T (a1_0.a + 1); // { dg-warning "\\\[-Warray-bounds" } + T (a1_0.a + 9); // { dg-warning "\\\[-Warray-bounds" } + T (a1_0.a + INT_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (a1_0.a + PTRDIFF_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (a1_0.a + SIZE_MAX); // { dg-warning "\\\[-Warray-bounds" } + + T (a1_0_.a - 1); // { dg-warning "\\\[-Warray-bounds" } + T (a1_0_.a + 1); // { dg-warning "\\\[-Warray-bounds" } + T (a1_0_.a + 9); // { dg-warning "\\\[-Warray-bounds" } + T (a1_0_.a + INT_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (a1_0_.a + PTRDIFF_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (a1_0_.a + SIZE_MAX); // { dg-warning "\\\[-Warray-bounds" } +} + +void test_flexible_array_member (void) +{ + T (ax.a); // { dg-warning "\\\[-Warray-bounds" } + T (ax.a - 1); // { dg-warning "\\\[-Warray-bounds" } + T (ax.a + 1); // { dg-warning "\\\[-Warray-bounds" } + T (ax.a + 9); // { dg-warning "\\\[-Warray-bounds" } + T (ax.a + INT_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (ax.a + PTRDIFF_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (ax.a + SIZE_MAX); // { dg-warning "\\\[-Warray-bounds" } + + T (ax_0.a); // { dg-warning "\\\[-Warray-bounds" } + T (ax_0.a - 1); // { dg-warning "\\\[-Warray-bounds" } + T (ax_0.a + 1); // { dg-warning "\\\[-Warray-bounds" } + T (ax_0.a + 9); // { dg-warning "\\\[-Warray-bounds" } + T (ax_0.a + INT_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (ax_0.a + PTRDIFF_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (ax_0.a + SIZE_MAX); // { dg-warning "\\\[-Warray-bounds" } + + T (ax_0_.a); // { dg-warning "\\\[-Warray-bounds" } + T (ax_0_.a - 1); // { dg-warning "\\\[-Warray-bounds" } + T (ax_0_.a + 1); // { dg-warning "\\\[-Warray-bounds" } + T (ax_0_.a + 9); // { dg-warning "\\\[-Warray-bounds" } + T (ax_0_.a + INT_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (ax_0_.a + PTRDIFF_MAX); // { dg-warning "\\\[-Warray-bounds" } + T (ax_0_.a + SIZE_MAX); // { dg-warning "\\\[-Warray-bounds" } +} + diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-39.c b/gcc/testsuite/gcc.dg/Warray-bounds-39.c index 6a441c73e01..ca42af86cd4 100644 --- a/gcc/testsuite/gcc.dg/Warray-bounds-39.c +++ b/gcc/testsuite/gcc.dg/Warray-bounds-39.c @@ -21,65 +21,65 @@ char d[4]; void* test_memcpy_s0_1 (void *d) { - return memcpy (d, s0, 1); /* { dg-warning "\\\[-Warray-bounds" } */ + return memcpy (d, s0, 1); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } void* test_memcpy_s0_2 (void *d) { - return memcpy (d, s0, 2); /* { dg-warning "\\\[-Warray-bounds" } */ + return memcpy (d, s0, 2); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } void* test_memcpy_s0_0_1 (void *d) { - return memcpy (d, s0_0, 1); /* { dg-warning "\\\[-Warray-bounds" } */ + return memcpy (d, s0_0, 1); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } void* test_memcpy_s0_0_2 (void *d) { - return memcpy (d, s0_0, 2); /* { dg-warning "\\\[-Warray-bounds" } */ + return memcpy (d, s0_0, 2); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } void* test_memcpy_s0_1_1 (void *d) { - return memcpy (d, s0_1, 1); /* { dg-warning "\\\[-Warray-bounds" } */ + return memcpy (d, s0_1, 1); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } void* test_memcpy_s0_1_2 (void *d) { - return memcpy (d, s0_1, 2); /* { dg-warning "\\\[-Warray-bounds" } */ + return memcpy (d, s0_1, 2); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } void* test_memcpy_s1_0_1 (void *d) { - return memcpy (d, s1_0, 1); /* { dg-warning "\\\[-Warray-bounds" } */ + return memcpy (d, s1_0, 1); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } void* test_memcpy_s1_0_2 (void *d) { - return memcpy (d, s1_0, 2); /* { dg-warning "\\\[-Warray-bounds" } */ + return memcpy (d, s1_0, 2); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } void* test_memmove_s0_1 (void *d) { - return memmove (d, s0, 1); /* { dg-warning "\\\[-Warray-bounds" } */ + return memmove (d, s0, 1); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } void* test_memmove_s0_2 (void *d) { - return memmove (d, s0, 2); /* { dg-warning "\\\[-Warray-bounds" } */ + return memmove (d, s0, 2); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } void* test_memmove_s0_0_1 (void *d) { - return memmove (d, s0_0, 1); /* { dg-warning "\\\[-Warray-bounds" } */ + return memmove (d, s0_0, 1); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } void* test_memmove_s0_0_2 (void *d) { - return memmove (d, s0_0, 2); /* { dg-warning "\\\[-Warray-bounds" } */ + return memmove (d, s0_0, 2); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } @@ -92,57 +92,57 @@ const struct Empty e1_0[1][0] = { }; void* test_memcpy_e_1 (void *d) { - return memcpy (d, &e, 1); /* { dg-warning "\\\[-Warray-bounds" } */ + return memcpy (d, &e, 1); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } void* test_memcpy_e0_1 (void *d) { - return memcpy (d, e0, 1); /* { dg-warning "\\\[-Warray-bounds" } */ + return memcpy (d, e0, 1); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } void* test_memcpy_e0_0_1 (void *d) { - return memcpy (d, e0_0, 1); /* { dg-warning "\\\[-Warray-bounds" } */ + return memcpy (d, e0_0, 1); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } void* test_memcpy_e0_1_1 (void *d) { - return memcpy (d, e0_1, 1); /* { dg-warning "\\\[-Warray-bounds" } */ + return memcpy (d, e0_1, 1); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } void* test_memcpy_e1_0_1 (void *d) { - return memcpy (d, e1_0, 1); /* { dg-warning "\\\[-Warray-bounds" } */ + return memcpy (d, e1_0, 1); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } char* test_strcpy_s0 (char *d) { - return strcpy (d, s0); /* { dg-warning "\\\[-Warray-bounds" "pr88991" { xfail *-*-* } } */ + return strcpy (d, s0); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } char* test_strcpy_s0_0 (char *d) { - return strcpy (d, s0_0[0]); /* { dg-warning "\\\[-Warray-bounds" "pr88991" { xfail *-*-* } } */ + return strcpy (d, s0_0[0]); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" "pr88991" { xfail *-*-* } } */ } char* test_strncpy_s0_1 (char *d) { - return strncpy (d, s0, 1); /* { dg-warning "\\\[-Warray-bounds" "pr88991" { xfail *-*-* } } */ + return strncpy (d, s0, 1); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } char* test_strncpy_s0_2 (char *d) { - return strncpy (d, s0, 2); /* { dg-warning "\\\[-Warray-bounds" "pr88991" { xfail *-*-* } } */ + return strncpy (d, s0, 2); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" } */ } char* test_strncpy_s0_0_1 (char *d) { - return strncpy (d, s0_0[0], 1); /* { dg-warning "\\\[-Warray-bounds" "pr88991" { xfail *-*-* } } */ + return strncpy (d, s0_0[0], 1); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" "pr88991" { xfail *-*-* } } */ } char* test_strncpy_s0_0_2 (char *d) { - return strncpy (d, s0_0[0], 2); /* { dg-warning "\\\[-Warray-bounds" "pr88991" { xfail *-*-* } } */ + return strncpy (d, s0_0[0], 2); /* { dg-warning "\\\[-Warray-bounds|-Wstringop-overflow" "pr88991" { xfail *-*-* } } */ } diff --git a/gcc/testsuite/gcc.dg/strlenopt-78.c b/gcc/testsuite/gcc.dg/strlenopt-78.c new file mode 100644 index 00000000000..4fdfe844cdc --- /dev/null +++ b/gcc/testsuite/gcc.dg/strlenopt-78.c @@ -0,0 +1,126 @@ +/* PR middle-end/91490 - bogus argument missing terminating nul warning + on strlen of a flexible array member + Verify that strlen calls with a flexible array member (and its common + forms) of declaraed objects are folded. + { dg-do compile } + { dg-options "-Wall -fdump-tree-gimple" } */ + +#include "strlenopt.h" + +extern void* memchr (const void*, int, size_t); + +struct A1 { char n, a[1]; }; +struct A2 { char n, a[2]; }; +struct Ax { char n, a[]; }; + +const struct A1 a1 = { 0 }; +const struct A1 a1__ = { 0, { } }; +const struct A1 a1_0 = { 0, { 0 } }; + +const struct A2 a2 = { 1, { } }; +const struct A2 a2_1 = { 1, { 1 } }; +const struct A2 a2_1_0 = { 1, { 1, 0 } }; + +const struct Ax ax = { 3, { 3, 2, 1, 0 } }; + +struct BxA1 { int n; struct A1 a[]; }; +struct BxA2 { int n; struct A2 a[]; }; + +const struct BxA2 bx = { 2, { { 2, { 2, 1 } }, { 2, { 1, 0 } } } }; + +int mchr1, mchr1__, mchr1_0, mchr2_1, mchr2, mchr2_1_0, mchrax, mchrbx; +int schr1, schr1__, schr1_0, schr2_1, schr2, schr2_1_0, schrax, schrbx; +int scmp1, scmp1__, scmp1_0, scmp2_1, scmp2, scmp2_1_0, scmpax, scmpbx; +int len1, len1__, len1_0, len2_1, len2, len2_1_0, lenax, lenbx; + +#if 0 + +// Not implemented yet. + +void test_memchr_flexarray (void) +{ + mchr1 = 0 != memchr (&a1, '1', sizeof a1); + mchr1__ = 0 != memchr (&a1__, '2', sizeof a1__); + mchr1_0 = 0 != memchr (&a1_0, '3', sizeof a1_0); + + mchr2 = 0 != memchr (&a2, '4', sizeof a2); + mchr2_1 = 0 != memchr (&a2_1, '\001', sizeof a2_1); + mchr2_1_0 = 0 != memchr (&a2_1_0, '\001', sizeof a2_1_0); + + mchrax = (const char*)&ax + sizeof ax - 1 == memchr (&ax, '\001', sizeof ax); + mchrbx = (const char*)&bx + sizeof bx - 1 == memchr (&bx, '\001', sizeof bx); +} +#endif + +void test_strchr_flexarray (void) +{ + schr1 = 0 != strchr (a1.a, '1'); + schr1__ = 0 != strchr (a1__.a, '2'); + schr1_0 = 0 != strchr (a1_0.a, '3'); + + schr2 = 0 != strchr (a2.a, '4'); + schr2_1 = 0 != strchr (a2_1.a, '\001'); + schr2_1_0 = 0 != strchr (a2_1_0.a, '\001'); + + schrax = 0 != strchr (ax.a, '\001'); + schrbx = 0 != strchr (bx.a[1].a, '\0'); +} + +void test_strcmp_flexarray (void) +{ + scmp1 = 0 == strcmp (a1.a, "1"); + scmp1__ = 0 == strcmp (a1__.a, "2"); + scmp1_0 = 0 == strcmp (a1_0.a, "3"); + + scmp2 = 0 == strcmp (a2.a, "4"); + scmp2_1 = 0 == strcmp (a2_1.a, "\001"); + scmp2_1_0 = 0 == strcmp (a2_1_0.a, "\001"); + + scmpax = 0 == strcmp (ax.a, "\003\002\001"); + scmpbx = 0 == strcmp (bx.a[1].a, "\001"); +} + +void test_strlen_flexarray (void) +{ + len1 = strlen (a1.a); + len1__ = strlen (a1__.a); + len1_0 = strlen (a1_0.a); + + len2 = strlen (a2.a); + len2_1 = strlen (a2_1.a); + len2_1_0 = strlen (a2_1_0.a); + + lenax = strlen (ax.a); + lenbx = strlen (bx.a[1].a); +} + +/* { dg-final { scan-tree-dump-not "strchr *\\(" "gimple" } } + { dg-final { scan-tree-dump-not "strcmp *\\(" "gimple" } } + { dg-final { scan-tree-dump-not "strlen *\\(" "gimple" } } + + { dg-final { scan-tree-dump "schr1 = 0;" "gimple" } } + { dg-final { scan-tree-dump "schr1__ = 0;" "gimple" } } + { dg-final { scan-tree-dump "schr1_0 = 0;" "gimple" } } + { dg-final { scan-tree-dump "schr2 = 0;" "gimple" } } + { dg-final { scan-tree-dump "schr2_1 = 1;" "gimple" } } + { dg-final { scan-tree-dump "schr2_1_0 = 1;" "gimple" } } + { dg-final { scan-tree-dump "schrax = 1;" "gimple" } } + { dg-final { scan-tree-dump "schrbx = 1;" "gimple" } } + + { dg-final { scan-tree-dump "scmp1 = 0;" "gimple" } } + { dg-final { scan-tree-dump "scmp1__ = 0;" "gimple" } } + { dg-final { scan-tree-dump "scmp1_0 = 0;" "gimple" } } + { dg-final { scan-tree-dump "scmp2 = 0;" "gimple" } } + { dg-final { scan-tree-dump "scmp2_1 = 1;" "gimple" } } + { dg-final { scan-tree-dump "scmp2_1_0 = 1;" "gimple" } } + { dg-final { scan-tree-dump "scmpax = 1;" "gimple" } } + { dg-final { scan-tree-dump "scmpbx = 1;" "gimple" } } + + { dg-final { scan-tree-dump "len1 = 0;" "gimple" } } + { dg-final { scan-tree-dump "len1__ = 0;" "gimple" } } + { dg-final { scan-tree-dump "len1_0 = 0;" "gimple" } } + { dg-final { scan-tree-dump "len2 = 0;" "gimple" } } + { dg-final { scan-tree-dump "len2_1 = 1;" "gimple" } } + { dg-final { scan-tree-dump "len2_1_0 = 1;" "gimple" } } + { dg-final { scan-tree-dump "lenax = 3;" "gimple" } } + { dg-final { scan-tree-dump "lenbx = 1;" "gimple" } } */ diff --git a/gcc/tree.c b/gcc/tree.c index ae292281b1f..613efa5f2b0 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -11872,18 +11872,23 @@ build_alloca_call_expr (tree size, unsigned int align, HOST_WIDE_INT max_size) } } -/* Create a new constant string literal consisting of elements of type - ELTYPE and return a tree node representing char* pointer to it as - an ADDR_EXPR (ARRAY_REF (ELTYPE, ...)). The STRING_CST value is - the LEN bytes at STR (the representation of the string, which may +/* Create a new constant string literal of type ELTYPE[SIZE] (or LEN + if SIZE == -1) and return a tree node representing char* pointer to + it as an ADDR_EXPR (ARRAY_REF (ELTYPE, ...)). The STRING_CST value + is the LEN bytes at STR (the representation of the string, which may be wide). */ tree build_string_literal (int len, const char *str, - tree eltype /* = char_type_node */) + tree eltype /* = char_type_node */, + unsigned HOST_WIDE_INT size /* = -1 */) { tree t = build_string (len, str); - tree index = build_index_type (size_int (len - 1)); + /* Set the maximum valid index based on the string length or SIZE. */ + unsigned HOST_WIDE_INT maxidx + = (size == HOST_WIDE_INT_M1U ? len : size) - 1; + + tree index = build_index_type (size_int (maxidx)); eltype = build_type_variant (eltype, 1, 0); tree type = build_array_type (eltype, index); TREE_TYPE (t) = type; diff --git a/gcc/tree.h b/gcc/tree.h index 37f6d57c034..fc85572dffa 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -4418,7 +4418,8 @@ extern tree build_call_expr_internal_loc_array (location_t, enum internal_fn, extern tree maybe_build_call_expr_loc (location_t, combined_fn, tree, int, ...); extern tree build_alloca_call_expr (tree, unsigned int, HOST_WIDE_INT); -extern tree build_string_literal (int, const char *, tree = char_type_node); +extern tree build_string_literal (int, const char *, tree = char_type_node, + unsigned HOST_WIDE_INT = HOST_WIDE_INT_M1U); /* Construct various nodes representing data types. */
This patch is a subset of the solution for PR 91457 whose main goal is to eliminate inconsistencies in warnings issued for out-of-bounds accesses to the various flavors of flexible array members of constant objects. That patch was posted here: https://gcc.gnu.org/ml/gcc-patches/2019-08/msg01202.html Like PR 91457, this patch also relies on improving optimization to issue better quality warnings. (I.e., with the latter being what motivated it.) The optimization enhances string_constant to recognize empty CONSTRUCTORs returned by fold_ctor_reference for references to members of constant objects with either "insufficient" initializers (e.g., given struct S { char n, a[]; } s = { 0 };) or with braced-list initializers (e.g., given struct S s = { 3 { 1, 2, 3, 0 } }; The patch lets string_constant convert the CONSTRUCTOR for s.a into a STRING_CST, which in turn enables the folding of calls to built-ins like strlen, strchr, or strcmp with such arguments. Exposing the strings to the folder then also lets it detect and issue warnings for out-of-bounds offsets in more instances of such references than before. The remaining changes in the patch mostly enhance the places that use the no-warning bit to prevent redundant diagnostics. Tested on x86_64-linux. Martin