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

[Qing Zhao <qing dot zhao at oracle dot com>]

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.

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