On 01/27/2016 01:58 PM, Jakub Jelinek wrote: > On Wed, Jan 27, 2016 at 01:47:10PM +0100, Martin Liška wrote: >> Following patch was kind of pre-approved by Jakub in: >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69276#c4 >> >> Patch can bootstrap in x86_64-linux-gnu and survives regression tests. >> I also verified that newly added test-case works with the patch. >> >> Ready for trunk? > > Ok, with nits. > >> >From 4e4575cfef5d06d8e8477716ce2f4d7e28ae66f0 Mon Sep 17 00:00:00 2001 >> From: marxin <mli...@suse.cz> >> Date: Thu, 14 Jan 2016 18:15:04 +0100 >> Subject: [PATCH] Fix PR sanitizer/PR69276 >> >> gcc/testsuite/ChangeLog: >> >> 2016-01-14 Martin Liska <mli...@suse.cz> >> >> * g++.dg/asan/pr69276.C: New test. >> >> gcc/ChangeLog: >> >> 2016-01-14 Martin Liska <mli...@suse.cz> >> >> PR sanitizer/PR69276 >> * asan.c (has_stmt_been_instrumented_p): Instrument gimple calls >> that are gimple_store_p. >> (maybe_instrument_call): Likewise. >> --- >> gcc/asan.c | 21 ++++++++++++++++++++ >> gcc/testsuite/g++.dg/asan/pr69276.C | 38 >> +++++++++++++++++++++++++++++++++++++ >> 2 files changed, 59 insertions(+) >> create mode 100644 gcc/testsuite/g++.dg/asan/pr69276.C >> >> diff --git a/gcc/asan.c b/gcc/asan.c >> index 2f9f92f..1747e90 100644 >> --- a/gcc/asan.c >> +++ b/gcc/asan.c >> @@ -2038,6 +2048,17 @@ maybe_instrument_call (gimple_stmt_iterator *iter) >> gimple_set_location (g, gimple_location (stmt)); >> gsi_insert_before (iter, g, GSI_SAME_STMT); >> } >> + else if (gimple_store_p (stmt)) > > I'd remove the "else " here, I believe if a noreturn call returns aggregate > the lhs is not removed and the store can still (partially) happen, if it is > returned by invisible reference.
Hi Jakub. Ah, you are right, good nit! Do you instrument it before the call or > after btw? Before the call might be premature, the call might not return > and not store anything into the result, after the call might be too late > (store happened already). The instrumentation is applied before the call. Hope we do not introduce a new false positives. Can I apply the v2? Thanks, Martin > > Jakub >
>From d78539fdd0a0d3c3275eb0cdbdd50d7b6ddf9c4c Mon Sep 17 00:00:00 2001 From: marxin <mli...@suse.cz> Date: Thu, 14 Jan 2016 18:15:04 +0100 Subject: [PATCH] Fix PR sanitizer/PR69276 gcc/testsuite/ChangeLog: 2016-01-14 Martin Liska <mli...@suse.cz> * g++.dg/asan/pr69276.C: New test. gcc/ChangeLog: 2016-01-14 Martin Liska <mli...@suse.cz> PR sanitizer/PR69276 * asan.c (has_stmt_been_instrumented_p): Instrument gimple calls that are gimple_store_p. (maybe_instrument_call): Likewise. --- gcc/asan.c | 22 +++++++++++++++++++++ gcc/testsuite/g++.dg/asan/pr69276.C | 38 +++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 gcc/testsuite/g++.dg/asan/pr69276.C diff --git a/gcc/asan.c b/gcc/asan.c index 2f9f92f..aeb840d 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -897,6 +897,16 @@ has_stmt_been_instrumented_p (gimple *stmt) return true; } } + else if (is_gimple_call (stmt) && gimple_store_p (stmt)) + { + asan_mem_ref r; + asan_mem_ref_init (&r, NULL, 1); + + r.start = gimple_call_lhs (stmt); + r.access_size = int_size_in_bytes (TREE_TYPE (r.start)); + return has_mem_ref_been_instrumented (&r); + } + return false; } @@ -2038,6 +2048,18 @@ maybe_instrument_call (gimple_stmt_iterator *iter) gimple_set_location (g, gimple_location (stmt)); gsi_insert_before (iter, g, GSI_SAME_STMT); } + + if (gimple_store_p (stmt)) + { + tree ref_expr = gimple_call_lhs (stmt); + instrument_derefs (iter, ref_expr, + gimple_location (stmt), + /*is_store=*/true); + + gsi_next (iter); + return true; + } + return false; } diff --git a/gcc/testsuite/g++.dg/asan/pr69276.C b/gcc/testsuite/g++.dg/asan/pr69276.C new file mode 100644 index 0000000..ff43650 --- /dev/null +++ b/gcc/testsuite/g++.dg/asan/pr69276.C @@ -0,0 +1,38 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ +/* { dg-additional-options "-O0 -fno-lto" } */ + +#include <stdlib.h> + +typedef __SIZE_TYPE__ size_t; +inline void * operator new (size_t, void *p) { return p; } + + +struct vec +{ + int size; +}; + +struct vnull +{ + operator vec() { return vec(); } +}; +vnull vNULL; + +struct A +{ + A(): value2 (vNULL), value3 (vNULL) {} + int value; + vec value2; + vec value3; +}; + +int main() +{ + int *array = (int *)malloc (sizeof (int) * 1); + A *a = new (array) A (); + free (array); +} + +/* { dg-output "ERROR: AddressSanitizer: heap-buffer-overflow.*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +in A::A()" } */ -- 2.7.0