diff mbox series

gimple-ssa-warn-access: Fix up asan_test.C -Wdangling-pointer regression [PR104103]

Message ID 20220118235610.GN2646553@tucnak
State New
Headers
Series gimple-ssa-warn-access: Fix up asan_test.C -Wdangling-pointer regression [PR104103] |

Commit Message

Jakub Jelinek Jan. 18, 2022, 11:56 p.m. UTC 
  Hi!

As reported in the PR or as I've seen since the weekend, asan_test.C fails
because of many warnings like:
/home/jakub/src/gcc/gcc/testsuite/g++.dg/asan/asan_test.cc:1157:10: error: using a dangling pointer to an unnamed temporary [-Werror=dangling-pointer=]
/home/jakub/src/gcc/gcc/testsuite/g++.dg/asan/asan_test.cc:1157:10: error: using a dangling pointer to an unnamed temporary [-Werror=dangling-pointer=]
/home/jakub/src/gcc/gcc/testsuite/g++.dg/asan/asan_test.cc:1162:27: error: using a dangling pointer to an unnamed temporary [-Werror=dangling-pointer=]
...
(lots of them).
There are no dangling pointers though, the warning pass sees:
  some_automatic_var ={v} {CLOBBER};
  .ASAN_MARK (POISON, &some_automatic_var, 8);
and warns on that (both on user vars and on e.g. TARGET_EXPR temporaries).
There is nothing wrong on that, .ASAN_MARK is compiler instrumentation,
which doesn't even touch the variable in any way nor make it escaped.
What it instead does is change bytes in the shadow memory corresponding
to the variable to reflect that the variable is out of scope and make
sure that access to it would be diagnosed at runtime.
So, for all purposes of the -Wdangling-pointer and -Wuse-after-free
warnings, we should ignore this internal call.

Bootstrapped/regtested on x86_64-linux and i686-linux, fixes asan_test.C
FAIL (so no new test has been added), ok for trunk?

2022-01-18  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/104103
	* gimple-ssa-warn-access.cc (pass_waccess::check_call): Don't check
	.ASAN_MARK calls.


	Jakub

Comments

Martin Sebor Jan. 19, 2022, 12:03 a.m. UTC | #1 
On 1/18/22 16:56, Jakub Jelinek wrote:
> Hi!
> 
> As reported in the PR or as I've seen since the weekend, asan_test.C fails
> because of many warnings like:
> /home/jakub/src/gcc/gcc/testsuite/g++.dg/asan/asan_test.cc:1157:10: error: using a dangling pointer to an unnamed temporary [-Werror=dangling-pointer=]
> /home/jakub/src/gcc/gcc/testsuite/g++.dg/asan/asan_test.cc:1157:10: error: using a dangling pointer to an unnamed temporary [-Werror=dangling-pointer=]
> /home/jakub/src/gcc/gcc/testsuite/g++.dg/asan/asan_test.cc:1162:27: error: using a dangling pointer to an unnamed temporary [-Werror=dangling-pointer=]
> ...
> (lots of them).
> There are no dangling pointers though, the warning pass sees:
>    some_automatic_var ={v} {CLOBBER};
>    .ASAN_MARK (POISON, &some_automatic_var, 8);
> and warns on that (both on user vars and on e.g. TARGET_EXPR temporaries).
> There is nothing wrong on that, .ASAN_MARK is compiler instrumentation,
> which doesn't even touch the variable in any way nor make it escaped.
> What it instead does is change bytes in the shadow memory corresponding
> to the variable to reflect that the variable is out of scope and make
> sure that access to it would be diagnosed at runtime.
> So, for all purposes of the -Wdangling-pointer and -Wuse-after-free
> warnings, we should ignore this internal call.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, fixes asan_test.C
> FAIL (so no new test has been added), ok for trunk?

This is in line with what's done for -Wmaybe-uninitialized so it makes
sense to do it here as well.  -Wmaybe-uninitialized also exempts calls
to sanitizer built-ins from checking.  I don't know if they might come
up here but if it can't be ruled out, moving the code from
tree-ssa-uninit.cc into a utility helper and calling it from both
places might be a good idea.

Martin

> 
> 2022-01-18  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/104103
> 	* gimple-ssa-warn-access.cc (pass_waccess::check_call): Don't check
> 	.ASAN_MARK calls.
> 
> --- gcc/gimple-ssa-warn-access.cc.jj	2022-01-16 20:55:46.783932110 +0100
> +++ gcc/gimple-ssa-warn-access.cc	2022-01-18 20:56:13.697780325 +0100
> @@ -4232,6 +4232,11 @@ pass_waccess::check_call (gcall *stmt)
>     if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
>       check_builtin (stmt);
>   
> +  /* .ASAN_MARK doesn't access any vars, only modifies shadow memory.  */
> +  if (gimple_call_internal_p (stmt)
> +      && gimple_call_internal_fn (stmt) == IFN_ASAN_MARK)
> +    return;
> +
>     if (!m_early_checks_p)
>       if (tree callee = gimple_call_fndecl (stmt))
>         {
> 
> 	Jakub
>
Richard Biener Jan. 19, 2022, 6:29 a.m. UTC | #2 
> Am 19.01.2022 um 01:04 schrieb Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org>:
> 
> On 1/18/22 16:56, Jakub Jelinek wrote:
>> Hi!
>> As reported in the PR or as I've seen since the weekend, asan_test.C fails
>> because of many warnings like:
>> /home/jakub/src/gcc/gcc/testsuite/g++.dg/asan/asan_test.cc:1157:10: error: using a dangling pointer to an unnamed temporary [-Werror=dangling-pointer=]
>> /home/jakub/src/gcc/gcc/testsuite/g++.dg/asan/asan_test.cc:1157:10: error: using a dangling pointer to an unnamed temporary [-Werror=dangling-pointer=]
>> /home/jakub/src/gcc/gcc/testsuite/g++.dg/asan/asan_test.cc:1162:27: error: using a dangling pointer to an unnamed temporary [-Werror=dangling-pointer=]
>> ...
>> (lots of them).
>> There are no dangling pointers though, the warning pass sees:
>>   some_automatic_var ={v} {CLOBBER};
>>   .ASAN_MARK (POISON, &some_automatic_var, 8);
>> and warns on that (both on user vars and on e.g. TARGET_EXPR temporaries).
>> There is nothing wrong on that, .ASAN_MARK is compiler instrumentation,
>> which doesn't even touch the variable in any way nor make it escaped.
>> What it instead does is change bytes in the shadow memory corresponding
>> to the variable to reflect that the variable is out of scope and make
>> sure that access to it would be diagnosed at runtime.
>> So, for all purposes of the -Wdangling-pointer and -Wuse-after-free
>> warnings, we should ignore this internal call.
>> Bootstrapped/regtested on x86_64-linux and i686-linux, fixes asan_test.C
>> FAIL (so no new test has been added), ok for trunk?

Ok.

Richard 

> This is in line with what's done for -Wmaybe-uninitialized so it makes
> sense to do it here as well.  -Wmaybe-uninitialized also exempts calls
> to sanitizer built-ins from checking.  I don't know if they might come
> up here but if it can't be ruled out, moving the code from
> tree-ssa-uninit.cc into a utility helper and calling it from both
> places might be a good idea.
> 
> Martin
> 
>> 2022-01-18  Jakub Jelinek  <jakub@redhat.com>
>>    PR middle-end/104103
>>    * gimple-ssa-warn-access.cc (pass_waccess::check_call): Don't check
>>    .ASAN_MARK calls.
>> --- gcc/gimple-ssa-warn-access.cc.jj    2022-01-16 20:55:46.783932110 +0100
>> +++ gcc/gimple-ssa-warn-access.cc    2022-01-18 20:56:13.697780325 +0100
>> @@ -4232,6 +4232,11 @@ pass_waccess::check_call (gcall *stmt)
>>    if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
>>      check_builtin (stmt);
>>  +  /* .ASAN_MARK doesn't access any vars, only modifies shadow memory.  */
>> +  if (gimple_call_internal_p (stmt)
>> +      && gimple_call_internal_fn (stmt) == IFN_ASAN_MARK)
>> +    return;
>> +
>>    if (!m_early_checks_p)
>>      if (tree callee = gimple_call_fndecl (stmt))
>>        {
>>    Jakub
>
diff mbox series

Patch

--- gcc/gimple-ssa-warn-access.cc.jj	2022-01-16 20:55:46.783932110 +0100
+++ gcc/gimple-ssa-warn-access.cc	2022-01-18 20:56:13.697780325 +0100
@@ -4232,6 +4232,11 @@  pass_waccess::check_call (gcall *stmt)
   if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
     check_builtin (stmt);
 
+  /* .ASAN_MARK doesn't access any vars, only modifies shadow memory.  */
+  if (gimple_call_internal_p (stmt)
+      && gimple_call_internal_fn (stmt) == IFN_ASAN_MARK)
+    return;
+
   if (!m_early_checks_p)
     if (tree callee = gimple_call_fndecl (stmt))
       {