Re: [PATCH] warn-access: Fix up check_pointer_uses [PR104715]

[Richard Biener <rguenther@suse.de>]

> Am 01.03.2022 um 20:08 schrieb Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org>:
> 
> On 3/1/22 11:41, Jakub Jelinek wrote:
>> Hi!
>> The following testcase emits bogus -Wdangling-pointer warnings.
>> The bug is that when it sees that ptr immediate use is a call that
>> returns one of its arguments, it will assume that the return value
>> is based on ptr, but that is the case only if ptr is passed to the
>> argument that is actually returned (so e.g. for memcpy the first argument,
>> etc.).  When the builtins guarantee e.g. that the result is based on the
>> first argument (either ERF_RETURNS_ARG 0 in which case it will always
>> just returns the first argument as is, or when it is something like
>> strstr or strpbrk or mempcpy that it returns some pointer based on the
>> first argument), it means the result is not based on second or following
>> argument if any.  The second hunk fixes this.
>> The first hunk just removes an unnecessary TREE_CODE check, the code only
>> pushes SSA_NAMEs into the pointers vector and if it didn't, it uses
>>       FOR_EACH_IMM_USE_FAST (use_p, iter, ptr)
>> a few lines below this, which of course requires that ptr is a SSA_NAME.
>> Tree checking on SSA_NAME_VERSION will already ensure that if it wasn't
>> a SSA_NAME, we'd ICE.
>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok

Richard 

> Thanks for the fix.  It makes sense to me.  Besides the test for
> the false positives I would suggest to add one to verify that using
> the first argument to a strstr() call is diagnosed if it's dangling
> (both as is, as well as with an offset from the first element).
> There are tests for memchr and strchr in the -Wdangling-pointer
> test suite but none for strstr.
> 
> Martin
> 
>> 2022-03-01  Jakub Jelinek  <jakub@redhat.com>
>>    PR tree-optimization/104715
>>    * gimple-ssa-warn-access.cc (pass_waccess::check_pointer_uses): Don't
>>    unnecessarily test if ptr is a SSA_NAME, it has to be.  Only push lhs
>>    of a call if gimple_call_return_arg is equal to ptr, not just when it
>>    is non-NULL.
>>    * c-c++-common/Wdangling-pointer-7.c: New test.
>> --- gcc/gimple-ssa-warn-access.cc.jj    2022-02-28 16:22:40.860520930 +0100
>> +++ gcc/gimple-ssa-warn-access.cc    2022-02-28 16:55:01.242272499 +0100
>> @@ -4169,8 +4169,7 @@ pass_waccess::check_pointer_uses (gimple
>>    for (unsigned i = 0; i != pointers.length (); ++i)
>>      {
>>        tree ptr = pointers[i];
>> -      if (TREE_CODE (ptr) == SSA_NAME
>> -      && !bitmap_set_bit (visited, SSA_NAME_VERSION (ptr)))
>> +      if (!bitmap_set_bit (visited, SSA_NAME_VERSION (ptr)))
>>      /* Avoid revisiting the same pointer.  */
>>      continue;
>>  @@ -4267,7 +4266,7 @@ pass_waccess::check_pointer_uses (gimple
>>          if (gcall *call = dyn_cast <gcall *>(use_stmt))
>>          {
>> -          if (gimple_call_return_arg (call))
>> +          if (gimple_call_return_arg (call) == ptr)
>>          if (tree lhs = gimple_call_lhs (call))
>>            if (TREE_CODE (lhs) == SSA_NAME)
>>              pointers.safe_push (lhs);
>> --- gcc/testsuite/c-c++-common/Wdangling-pointer-7.c.jj    2022-02-28 17:09:09.906355082 +0100
>> +++ gcc/testsuite/c-c++-common/Wdangling-pointer-7.c    2022-02-28 17:03:50.533839892 +0100
>> @@ -0,0 +1,36 @@
>> +/* PR tree-optimization/104715 */
>> +/* { dg-do compile } */
>> +/* { dg-options "-Wdangling-pointer" } */
>> +
>> +char *
>> +foo (char *p)
>> +{
>> +  {
>> +    char q[61] = "012345678901234567890123456789012345678901234567890123456789";
>> +    char *r = q;
>> +    p = __builtin_strcat (p, r);
>> +  }
>> +  return p;    /* { dg-bogus "using dangling pointer" } */
>> +}
>> +
>> +char *
>> +bar (char *p)
>> +{
>> +  {
>> +    char q[] = "0123456789";
>> +    char *r = q;
>> +    p = __builtin_strstr (p, r);
>> +  }
>> +  return p;    /* { dg-bogus "using dangling pointer" } */
>> +}
>> +
>> +char *
>> +baz (char *p)
>> +{
>> +  {
>> +    char q[] = "0123456789";
>> +    char *r = q;
>> +    p = __builtin_strpbrk (p, r);
>> +  }
>> +  return p;    /* { dg-bogus "using dangling pointer" } */
>> +}
>>    Jakub
>