https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70541
--- Comment #4 from Maxim Ostapenko <m.ostapenko at samsung dot com> ---
(In reply to Jakub Jelinek from comment #3)
> (In reply to Maxim Ostapenko from comment #1)
> > @@ -2060,7 +2067,20 @@ maybe_instrument_call (gimple_stmt_iterator *iter)
> > return true;
> > }
>
> If the function call returns a struct, then your patch wouldn't instrument
> it.
> You need the bool instrumented = false; already above
> if (gimple_store_p (stmt))
> and set instrumented = true; there instead of gsi_next (iter); return true;
>
> > - return false;
> > + bool instrumented = false;
> > + HOST_WIDE_INT args_num = gimple_call_num_args (stmt);
> > + for (int i = 0; i < args_num; ++i)
> > + {
> > + if (is_arg_deref_p (TREE_CODE (gimple_call_arg (stmt, i))))
>
> I'm not aware of any is_arg_deref_p predicate.
> IMHO you should test:
> if (!is_gimple_reg (gimple_call_arg (stmt, i)))
>
> > + {
> > + instrument_derefs (iter, gimple_call_arg (stmt, i),
> > + gimple_location (stmt), false);
> > + instrumented = true;
> > + }
> > + }
> > + if (instrumented)
> > + gsi_next (iter);
> > + return instrumented;
>
Thanks, looks better now?
diff --git a/gcc/asan.c b/gcc/asan.c
index 47bfdcd..c51e629 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1766,6 +1766,8 @@ instrument_derefs (gimple_stmt_iterator *iter, tree t,
tree type, base;
HOST_WIDE_INT size_in_bytes;
+ if (location == UNKNOWN_LOCATION)
+ location = EXPR_LOCATION (t);
type = TREE_TYPE (t);
switch (TREE_CODE (t))
@@ -2049,6 +2051,7 @@ maybe_instrument_call (gimple_stmt_iterator *iter)
gsi_insert_before (iter, g, GSI_SAME_STMT);
}
+ bool instrumented = false;
if (gimple_store_p (stmt))
{
tree ref_expr = gimple_call_lhs (stmt);
@@ -2056,11 +2059,22 @@ maybe_instrument_call (gimple_stmt_iterator *iter)
gimple_location (stmt),
/*is_store=*/true);
- gsi_next (iter);
- return true;
+ instrumented = true;
}
- return false;
+ HOST_WIDE_INT args_num = gimple_call_num_args (stmt);
+ for (int i = 0; i < args_num; ++i)
+ {
+ if (!is_gimple_reg (gimple_call_arg (stmt, i)))
+ {
+ instrument_derefs (iter, gimple_call_arg (stmt, i),
+ gimple_location (stmt), false);
+ instrumented = true;
+ }
+ }
+ if (instrumented)
+ gsi_next (iter);
+ return instrumented;
}
> As for the location_t thing, the fix would be to do in instrument_derefs
> something like:
> if (location == UNKNOWN_LOCATION)
> location = EXPR_LOCATION (t);
> after the early bail outs.
Hm, even with if (location == UNKNOWN_LOCATION) location = EXPR_LOCATION (t); I
don't see reasonable line in -O2 case:
1 #include <stdio.h>
2 #include <stdlib.h>
3
4 struct Simple {
5 int value;
6 };
7
8 int f(struct Simple simple) {
9 return simple.value;
10 }
11
12 int g(int value) {
13 return value;
14 }
15
16 int main() {
17 struct Simple *psimple = (struct Simple *) malloc(sizeof(struct Simple));
18 psimple->value = 42;
19 free(psimple);
20 printf("%d\n", f(*psimple));
21 return 0;
22 }
$ ./a.out
==29898==ERROR: AddressSanitizer: heap-use-after-free on address 0x60200000eff0
at pc 0x0000004055a6 bp 0x7ffc6b5632c0 sp 0x7ffc6b5632a0
READ of size 4 at 0x60200000eff0 thread T0
#0 0x4055a5 in main /tmp/test2.c:22
#1 0x7f8e6df32ec4 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)
#2 0x4055f9 (/tmp/a.out+0x4055f9)
$ cat test2.c.126t.asan1
main ()
{
int simple$value;
struct Simple * psimple;
<bb 2>:
[test2.c:17:18] psimple_3 = malloc (4);
[test2.c:17:18] # DEBUG psimple => psimple_3
[test2.c:19:3] free (psimple_3);
ASAN_CHECK (6, psimple_3, 4, 8);
simple$value_6 = MEM[(struct Simple *)psimple_3];
# DEBUG simple$value => simple$value_6
[test2.c:20:3] printf ([test2.c:20:10] "%d\n", simple$value_6);
return 0;
}
Perhaps we indeed should look at the inliner.
