diff mbox

[0/17] KASan 4.9 backport

Message ID 20141017134946.GX10376@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Oct. 17, 2014, 1:49 p.m. UTC 
On Fri, Oct 17, 2014 at 05:45:17PM +0400, Yury Gribov wrote:
> On 10/17/2014 04:12 PM, Jakub Jelinek wrote:
> >I had a brief look at what ended up on the branch in the end, and
> >from what I understand, the 4.9 libasan.so has
> >__asan_report_store_n and __asan_report_load_n entry points, but does
> >not have any __asan_loadN/__asan_reportN entrypoints (neither 1/2/4/8/16,
> >nor variable).
> >So, what the branch does seems to not match what the library provides.
> 
> I agree, __asan_report_loadN is indeed there and misalign tests seem to pass
> fine. Probably I should have examined 4.9 libasan closer.
> 
> >With -fsanitize=address -O2 --param asan-instrumentation-with-call-threshold=0
> >foo is again unexpectedly not instrumented, and bar is instrumented
> >with __asan_load8, which looks wrong to me, because the library does not
> >provide any such entry point.
> 
> By default asan-instrumentation-with-call-threshold is INT_MAX which means
> that compiler will never generate __asan_load*/__asan_store* calls unless
> forced by the user (e.g. for Kasan).
> 
> >But, in execute_sanopt force !use_calls
> >for (flag_sanitize & SANITIZE_USER_ADDRESS).
> 
> Do you think above limitation is not enough?

Yeah, even if the default is that it doesn't make the non-existing calls,
anyone who uses the parameter will get code that doesn't link.

> 
> >Thus, IMHO the:
> >   if ((flag_sanitize & SANITIZE_USER_ADDRESS) != 0
> >       && ((size_in_bytes & (size_in_bytes - 1)) != 0
> >	  || (unsigned HOST_WIDE_INT) size_in_bytes - 1 >= 16))
> >     return;
> >should be nuked from 4.9, we can do unaligned/non-{1,2,4,8,16}
> >accesses fine.
> 
> Right. I'd also import misalign tests.
> 
> >Or were there any bugfixes needed for __asan_report_{store,load}_n
> >on the library side?
> 
> I don't think so.

So, what about this?  Just checked that with
make -k check-g{cc,++} RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} asan.exp tsan.exp ubsan.exp'
so far.  Plus if you add misalign tests...

2014-10-17  Jakub Jelinek  <jakub@redhat.com>

	* asan.c (instrument_derefs): Allow instrumentation of odd-sized
	accesses even for -fsanitize=address.
	(execute_sanopt): Only allow use_calls for -fsanitize=kernel-address.

	* c-c++-common/asan/instrument-with-calls-1.c: Add
	-fno-sanitize=address -fsanitize=kernel-address to dg-options.
	* c-c++-common/asan/instrument-with-calls-2.c: Likewise.



	Jakub

Comments

Yury Gribov Oct. 17, 2014, 2:15 p.m. UTC | #1 
On 10/17/2014 05:49 PM, Jakub Jelinek wrote:
 > Plus if you add misalign tests...

Sure, can do this on Monday.

 > -  bool use_calls = ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD < INT_MAX
> -    && asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
> +  bool use_calls
> +    = ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD < INT_MAX
> +      && (flag_sanitize & SANITIZE_KERNEL_ADDRESS)
> +      && asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;

I agree that original code didn't quite match GNU conventions but can we 
avoid reformatting it to make future backports easier? So e.g.

  bool use_calls = ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD < INT_MAX
+    && (flag_sanitize & SANITIZE_KERNEL_ADDRESS)
    && asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;

-Y
diff mbox

Patch

--- gcc/asan.c.jj	2014-10-17 12:51:27.000000000 +0200
+++ gcc/asan.c	2014-10-17 15:21:29.921495259 +0200
@@ -1707,10 +1707,6 @@  instrument_derefs (gimple_stmt_iterator
   size_in_bytes = int_size_in_bytes (type);
   if (size_in_bytes <= 0)
     return;
-  if ((flag_sanitize & SANITIZE_USER_ADDRESS) != 0
-      && ((size_in_bytes & (size_in_bytes - 1)) != 0
-	  || (unsigned HOST_WIDE_INT) size_in_bytes - 1 >= 16))
-    return;
 
   HOST_WIDE_INT bitsize, bitpos;
   tree offset;
@@ -2780,8 +2776,10 @@  execute_sanopt (void)
 	  }
     }
 
-  bool use_calls = ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD < INT_MAX
-    && asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
+  bool use_calls
+    = ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD < INT_MAX
+      && (flag_sanitize & SANITIZE_KERNEL_ADDRESS)
+      && asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
 
   FOR_EACH_BB_FN (bb, cfun)
     {
--- gcc/testsuite/c-c++-common/asan/instrument-with-calls-1.c.jj	2014-10-17 12:51:27.000000000 +0200
+++ gcc/testsuite/c-c++-common/asan/instrument-with-calls-1.c	2014-10-17 15:34:06.679627168 +0200
@@ -1,5 +1,5 @@ 
 /* { dg-do assemble } */
-/* { dg-options "--param asan-instrumentation-with-call-threshold=0 -save-temps" } */
+/* { dg-options "-fno-sanitize=address -fsanitize=kernel-address --param asan-instrumentation-with-call-threshold=0 -save-temps" } */
 
 void f(char *a, int *b) {
   *b = *a;
--- gcc/testsuite/c-c++-common/asan/instrument-with-calls-2.c.jj	2014-10-17 12:51:27.000000000 +0200
+++ gcc/testsuite/c-c++-common/asan/instrument-with-calls-2.c	2014-10-17 15:34:15.569472032 +0200
@@ -1,5 +1,5 @@ 
 /* { dg-do assemble } */
-/* { dg-options "--param asan-instrumentation-with-call-threshold=1 -save-temps" } */
+/* { dg-options "-fno-sanitize=address -fsanitize=kernel-address --param asan-instrumentation-with-call-threshold=1 -save-temps" } */
 
 int x;