diff mbox

Fix PR sanitizer/PR69276

Message ID 56A8BC4E.4060507@suse.cz
State New
Headers show

Commit Message

Martin Liška Jan. 27, 2016, 12:47 p.m. UTC 
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?
Thanks,
Martin

Comments

Jakub Jelinek Jan. 27, 2016, 12:58 p.m. UTC | #1 
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 <mliska@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  <mliska@suse.cz>
> 
> 	* g++.dg/asan/pr69276.C: New test.
> 
> gcc/ChangeLog:
> 
> 2016-01-14  Martin Liska  <mliska@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
diff mbox

Patch

From 4e4575cfef5d06d8e8477716ce2f4d7e28ae66f0 Mon Sep 17 00:00:00 2001
From: marxin <mliska@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  <mliska@suse.cz>

	* g++.dg/asan/pr69276.C: New test.

gcc/ChangeLog:

2016-01-14  Martin Liska  <mliska@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
@@ -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,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))
+    {
+      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