[Bug sanitizer/69276] Address sanitizer does not handle heap overflow

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69276

Martin Liška  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #6 from Martin Liška  ---
Fixed in trunk.

[Bug sanitizer/69276] Address sanitizer does not handle heap overflow

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69276

--- Comment #5 from Martin Liška  ---
Author: marxin
Date: Thu Feb  4 11:50:40 2016
New Revision: 233137

URL: https://gcc.gnu.org/viewcvs?rev=233137=gcc=rev
Log:
Fix PR sanitizer/69276

* g++.dg/asan/pr69276.C: New test.
PR sanitizer/PR69276
* asan.c (has_stmt_been_instrumented_p): Instrument gimple calls
that are gimple_store_p.
(maybe_instrument_call): Likewise.

Added:
trunk/gcc/testsuite/g++.dg/asan/pr69276.C
Modified:
trunk/gcc/ChangeLog
trunk/gcc/asan.c
trunk/gcc/testsuite/ChangeLog

[Bug sanitizer/69276] Address sanitizer does not handle heap overflow

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69276

--- Comment #3 from Martin Liška  ---
(In reply to Jakub Jelinek from comment #2)
> Comment on attachment 37341 [details]
> suggested patch
> 
> +  else if (is_gimple_call (stmt) && gimple_store_p (stmt)
> +&& gimple_clobber_p (stmt))
> +{
> +  asan_mem_ref r;
> +  asan_mem_ref_init (, 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 ();
> +}
> +
> 
> This condition is never true, did you mean !gimple_clobber_p instead?
> But obviously calls are never clobbers, so there is no need to test that.

Sure, that was a typo, yeah it can be removed from the condition ;)

[Bug sanitizer/69276] Address sanitizer does not handle heap overflow

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69276

Jakub Jelinek  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2016-01-14
 Ever confirmed|0   |1

--- Comment #4 from Jakub Jelinek  ---
Otherwise it LGTM, so please remove the gimple_clobber_p stuff from the patch,
add a testcase to the testsuite, verify it fails before the patch and passes
after it and submit to gcc-patches.  Thanks.

[Bug sanitizer/69276] Address sanitizer does not handle heap overflow

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69276

--- Comment #1 from Jakub Jelinek  ---
Comment on attachment 37341
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=37341
suggested patch

+  else if (is_gimple_call (stmt) && gimple_store_p (stmt)
+  && gimple_clobber_p (stmt))
+{
+  asan_mem_ref r;
+  asan_mem_ref_init (, 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 ();
+}
+

This condition is never true, did you mean !gimple_clobber_p instead?
But obviously calls are never clobbers, so there is no need to test that.

[Bug sanitizer/69276] Address sanitizer does not handle heap overflow

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69276

--- Comment #2 from Jakub Jelinek  ---
Comment on attachment 37341
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=37341
suggested patch

+  else if (is_gimple_call (stmt) && gimple_store_p (stmt)
+  && gimple_clobber_p (stmt))
+{
+  asan_mem_ref r;
+  asan_mem_ref_init (, 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 ();
+}
+

This condition is never true, did you mean !gimple_clobber_p instead?
But obviously calls are never clobbers, so there is no need to test that.