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
