[PATCH][Middle-end]79538 missing -Wformat-overflow with %s and non-member array arguments

[Martin Sebor]

On 12/12/2017 10:56 AM, Qing Zhao wrote:
> Hi, Martin,
>
> thanks for the suggestion, this might be a good enhancement for
> get_range_strlen for a future work.
>
> my understanding is,  the current get_range_strlen does not use value
> range info yet, and also does not handle VLA.
> we can improve it from both aspects in a later work.

I agree (that's also what I meant).  Your patch is a valuable
improvement in and of itself.

Martin

>
> Qing
>
>>>
>>> Per your comments, the updated gimple-fold.c is like the following:
>>
>> FWIW, I suspect Richard is thinking of VLAs with the INTEGER_CST
>> comment above.  This is the case we discussed in private.  It is
>> handled either way but the handling could be improved by determining
>> the size of the VLA from the first __builtin_alloca_with_align
>> argument and using it or its range as the minimum and maximum.
>> With that, the range of string lengths that fit in vla in
>> the following could be determined and the sprintf call could
>> be diagnosed:
>>
>>  char d[30];
>>
>>  void f (unsigned n)
>>  {
>>    if (n < 32)
>>      {
>>        char vla[n];
>>        __builtin_sprintf (d, "%s", vla);
>>      }
>>  }
>>
>> I think this would be a nice enhancement to add on top of yours,
>> not just for VLAs but for dynamically allocated arrays as well,
>> and not just for the sprintf pass but also for the strlen pass
>> to optimize cases like:
>>
>>  void f (unsigned n)
>>  {
>>    if (n < 32)
>>      {
>>        char vla[n];
>>
>>        fgets (vla, n, stdin);
>>
>>        unsigned len = strlen (vla);
>>        if (len >= n)   // cannot hold
>>          abort ();     // can be eliminated
>>       }
>>    }
>>
>> That said, at some point, it might make more sense to change
>> those passes to start tracking these things as they traverse
>> the CFG rather than having get_range_strlen() do the work.
>>
>> Martin
>>
>>>
>>> diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
>>> index 353a46e..0500fba 100644
>>> --- a/gcc/gimple-fold.c
>>> +++ b/gcc/gimple-fold.c
>>> @@ -1323,6 +1323,19 @@ get_range_strlen (tree arg, tree length[2],
>>> bitmap *visited, int type,
>>>  the array could have zero length.  */
>>>       *minlen = ssize_int (0);
>>>     }
>>> +
>>> +          if (VAR_P (arg)
>>> +              && TREE_CODE (TREE_TYPE (arg)) == ARRAY_TYPE)
>>> +            {
>>> +              val = TYPE_SIZE_UNIT (TREE_TYPE (arg));
>>> +              if (!val || TREE_CODE (val) != INTEGER_CST ||
>>> integer_zerop (val))
>>> +                return false;
>>> +              val = fold_build2 (MINUS_EXPR, TREE_TYPE (val), val,
>>> + build_int_cst (TREE_TYPE (val), 1));
>>> +              /* Set the minimum size to zero since the string in
>>> +                 the array could have zero length.  */
>>> +              *minlen = ssize_int (0);
>>> +            }
>>> }
>>>
>>>       if (!val)
>>>
>>> let me know any further issue with the above.
>>>
>>> thanks a lot.
>>>
>>> Qing
>