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.  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).

        Jakub

Reply via email to