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