This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]