This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR sanitizer/PR69276
- From: Martin LiÅka <mliska at suse dot cz>
- To: gcc-patches at gcc dot gnu dot org
- Date: Wed, 27 Jan 2016 14:45:31 +0100
- Subject: Re: [PATCH] Fix PR sanitizer/PR69276
- Authentication-results: sourceware.org; auth=none
- References: <56A8BC4E dot 4060507 at suse dot cz> <20160127125838 dot GP3017 at tucnak dot redhat dot com>
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 <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.
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 <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 | 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