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

[Qing Zhao]

Hi, Richard,

thanks a lot for your review.

> 
>>                {
>>                  /* __copy is always the same for characters.
>>                     Check to see if copy function already exists.  */
>> -                 sprintf (name, "__copy_character_%d", ts->kind);
>> +                 name = xasprintf ("__copy_character_%d", ts->kind);
>>                  contained = ns->contained;
>>                  for (; contained; contained = contained->sibling)
>>                    if (contained->proc_name
>> @@ -2796,6 +2802,7 @@ find_intrinsic_vtab (gfc_typespec *ts)
>>          vtab->ts.u.derived = vtype;
>>          vtab->value = gfc_default_initializer (&vtab->ts);
>>        }
>> +      free (name);
> 
> It looks like this might be in a performance critical lookup path
> where we'd really
> like to avoid allocating/freeing memory.  Why's GFC_MAX_SYMBOL_LEN+1 not
> enough?  Leaving for fortran folks to comment - note you should CC
> fort...@gcc.gnu.org <mailto:fort...@gcc.gnu.org>
> for fortran patches (done).

I have sent this patch to fort...@gcc.gnu.org <mailto:fort...@gcc.gnu.org> per 
Thomas’s suggestion last week, and got approval by fortran team on last Friday:

https://gcc.gnu.org/ml/fortran/2017-12/msg00027.html

> 
>>    }
>> 
>>  found_sym = vtab;
>> diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
>> index 353a46e..fef1969 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 || integer_zerop (val))
> 
> val might be non-constant so you also need to check TREE_CODE (val) !=
> INTEGER_CST here.

Okay.
> 
>> +                return false;
>> +              val = fold_build2 (MINUS_EXPR, TREE_TYPE (val), val,
>> +                                 integer_one_node);
> 
>   val = wide_int_to_tree (TREE_TYPE (val), wi::minus (wi::to_wide (val), 1));
> 
> you pass a possibly bogus type of 1 to fold_build2 above so the wide-int 
> variant
> is prefered.
> 
> The gimple-fold.c changes are ok with that change.

Per your comments, the updated gimple-fold.c is like the following:

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