[PATCH] fold constant flexarrays to strings (PR 91490)

[Martin Sebor]

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