diff mbox

RFC Asan instrumentation control

Message ID 52B2E067.2050308@partner.samsung.com
State New
Headers show

Commit Message

max Dec. 19, 2013, 12:02 p.m. UTC
Sorry, ChangeLog and patch, of course.

-Maxim.

Comments

Jakub Jelinek Dec. 19, 2013, 12:27 p.m. UTC | #1
On Thu, Dec 19, 2013 at 04:02:47PM +0400, Maxim Ostapenko wrote:
> Sorry, ChangeLog and patch, of course.

> 2013-12-19  Max Ostapenko  <m.ostapenko@partner.samsung.com>
> 
> 	* cfgexpand.c (expand_stack_vars): Optionally disable asan stack protection.

Too long lines in ChangeLog, wrap to 80 columns.

> 	(expand_used_vars): Likewise.
> 	(partition_stack_vars): Likewise.
> 	* asan.c (asan_emit_stack_protection): Optionally disable after return stack usage.

Ditto.

> 	(instrument_derefs): Optionally disable memory access instrumentation.
> 	(instrument_builtin_call): Likewise.
> 	(instrument_strlen_call): Likewise.
> 	(asan_protect_global): Optionally disable global variables protection.
> 	* doc/invoke.texi: Added doc for new options.
> 	* params.def: Added new options.
> 	* params.h: Likewise.
> 
> 2013-12-19  Max Ostapenko  <m.ostapenko@partner.samsung.com>
> 	* c-c++-common/asan/global-overflow-2.c: New test.

Missing vertical space between date/name/mail and first entry.

> @@ -1003,7 +1004,7 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
>    str_cst = asan_pp_string (&asan_pp);
>  
>    /* Emit the prologue sequence.  */
> -  if (asan_frame_size > 32 && asan_frame_size <= 65536 && pbase)
> +  if (asan_frame_size > 32 && asan_frame_size <= 65536 && pbase && ASAN_USE_AFTER_RETURN)
>      {
>        use_after_return_class = floor_log2 (asan_frame_size - 1) - 5;
>        /* __asan_stack_malloc_N guarantees alignment

Please wrap this.

> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -798,7 +798,7 @@ partition_stack_vars (void)
>  	     sizes, as the shorter vars wouldn't be adequately protected.
>  	     Don't do that for "large" (unsupported) alignment objects,
>  	     those aren't protected anyway.  */
> -	  if ((flag_sanitize & SANITIZE_ADDRESS) && isize != jsize
> +	  if ((flag_sanitize & SANITIZE_ADDRESS) && ASAN_STACK  && isize != jsize

Replace the two spaces with just one.

> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -10037,6 +10037,36 @@ The default choice depends on the target.
>  Set the maximum number of existing candidates that will be considered when
>  seeking a basis for a new straight-line strength reduction candidate.
>  
> +@item asan-globals
> +Enable buffer overflow detection for global objects. This kind of protection
> +is enabled by default if you are using @option{-fsanitize=address} option.
> +To disable global objects protection use @option{--param asan-globals=0} option.

Too long lines (several times).
> +To disable memory reads instructions protection use @option{--param asan-instrument-reads=0} option.
> +
> +@item asan-instrument-writes
> +Enable buffer overflow detection for memory writes instructions. This kind of protection
> +is enabled by default if you are using @option{-fsanitize=address} option.
> +To disable memory writes instructions protection use @option{--param asan-instrument-writes=0} option.
> +
> +@item asan-memintrin
> +Enable detection for builtin functions. This kind of protection

I think for docs it should be "built-in functions".

As for the tests, I'm afraid I don't like them at all.
If anything, it ought to be dg-do compile tests where you say
scan assembly or some dump, but having runtime testcases that
trigger undefined behavior that isn't detected by the instrumentation
library at all and expect them to "pass" is simply wrong.

	Jakub
diff mbox

Patch

2013-12-19  Max Ostapenko  <m.ostapenko@partner.samsung.com>

	* cfgexpand.c (expand_stack_vars): Optionally disable asan stack protection.
	(expand_used_vars): Likewise.
	(partition_stack_vars): Likewise.
	* asan.c (asan_emit_stack_protection): Optionally disable after return stack usage.
	(instrument_derefs): Optionally disable memory access instrumentation.
	(instrument_builtin_call): Likewise.
	(instrument_strlen_call): Likewise.
	(asan_protect_global): Optionally disable global variables protection.
	* doc/invoke.texi: Added doc for new options.
	* params.def: Added new options.
	* params.h: Likewise.

2013-12-19  Max Ostapenko  <m.ostapenko@partner.samsung.com>
	* c-c++-common/asan/global-overflow-2.c: New test.
	* c-c++-common/asan/memcmp-3.c: Likewise.
	* c-c++-common/asan/no-instrument-reads.c: Likewise.
	* c-c++-common/asan/no-instrument-writes.c: Likewise.
	* c-c++-common/asan/stack-use-after-return.c: Likewise
	* c-c++-common/asan/no-stack-use-after-return.c: Likewise.

diff --git a/gcc/asan.c b/gcc/asan.c
index 1394e13..005db18 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -53,6 +53,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "gimple-builder.h"
 #include "ubsan.h"
 #include "predict.h"
+#include "params.h"
 
 /* AddressSanitizer finds out-of-bounds and use-after-free bugs
    with <2x slowdown on average.
@@ -1003,7 +1004,7 @@  asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
   str_cst = asan_pp_string (&asan_pp);
 
   /* Emit the prologue sequence.  */
-  if (asan_frame_size > 32 && asan_frame_size <= 65536 && pbase)
+  if (asan_frame_size > 32 && asan_frame_size <= 65536 && pbase && ASAN_USE_AFTER_RETURN)
     {
       use_after_return_class = floor_log2 (asan_frame_size - 1) - 5;
       /* __asan_stack_malloc_N guarantees alignment
@@ -1239,6 +1240,9 @@  asan_needs_local_alias (tree decl)
 bool
 asan_protect_global (tree decl)
 {
+  if (!ASAN_GLOBALS)
+    return false;
+
   rtx rtl, symbol;
 
   if (TREE_CODE (decl) == STRING_CST)
@@ -1568,6 +1572,11 @@  static void
 instrument_derefs (gimple_stmt_iterator *iter, tree t,
 		   location_t location, bool is_store)
 {
+  if (is_store && !ASAN_INSTRUMENT_WRITES)
+    return;
+  if (!is_store && !ASAN_INSTRUMENT_READS)
+    return;
+
   tree type, base;
   HOST_WIDE_INT size_in_bytes;
 
@@ -1897,6 +1906,9 @@  instrument_strlen_call (gimple_stmt_iterator *iter)
 static bool
 instrument_builtin_call (gimple_stmt_iterator *iter)
 {
+  if (!ASAN_MEMINTRIN)
+    return false;
+
   bool iter_advanced_p = false;
   gimple call = gsi_stmt (*iter);
 
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 7a93975..55708eb 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -798,7 +798,7 @@  partition_stack_vars (void)
 	     sizes, as the shorter vars wouldn't be adequately protected.
 	     Don't do that for "large" (unsupported) alignment objects,
 	     those aren't protected anyway.  */
-	  if ((flag_sanitize & SANITIZE_ADDRESS) && isize != jsize
+	  if ((flag_sanitize & SANITIZE_ADDRESS) && ASAN_STACK  && isize != jsize
 	      && ialign * BITS_PER_UNIT <= MAX_SUPPORTED_STACK_ALIGNMENT)
 	    break;
 
@@ -981,7 +981,7 @@  expand_stack_vars (bool (*pred) (size_t), struct stack_vars_data *data)
       if (alignb * BITS_PER_UNIT <= MAX_SUPPORTED_STACK_ALIGNMENT)
 	{
 	  base = virtual_stack_vars_rtx;
-	  if ((flag_sanitize & SANITIZE_ADDRESS) && pred)
+	  if ((flag_sanitize & SANITIZE_ADDRESS) && ASAN_STACK && pred)
 	    {
 	      HOST_WIDE_INT prev_offset = frame_offset;
 	      tree repr_decl = NULL_TREE;
@@ -1160,7 +1160,7 @@  defer_stack_allocation (tree var, bool toplevel)
   /* If stack protection is enabled, *all* stack variables must be deferred,
      so that we can re-order the strings to the top of the frame.
      Similarly for Address Sanitizer.  */
-  if (flag_stack_protect || (flag_sanitize & SANITIZE_ADDRESS))
+  if (flag_stack_protect || ((flag_sanitize & SANITIZE_ADDRESS) && ASAN_STACK))
     return true;
 
   /* We handle "large" alignment via dynamic allocation.  We want to handle
@@ -1820,7 +1820,7 @@  expand_used_vars (void)
 	    expand_stack_vars (stack_protect_decl_phase_2, &data);
 	}
 
-      if (flag_sanitize & SANITIZE_ADDRESS)
+      if ((flag_sanitize & SANITIZE_ADDRESS) && ASAN_STACK)
 	/* Phase 3, any partitions that need asan protection
 	   in addition to phase 1 and 2.  */
 	expand_stack_vars (asan_decl_phase_3, &data);
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 1a6d815..f25958b 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10037,6 +10037,36 @@  The default choice depends on the target.
 Set the maximum number of existing candidates that will be considered when
 seeking a basis for a new straight-line strength reduction candidate.
 
+@item asan-globals
+Enable buffer overflow detection for global objects. This kind of protection
+is enabled by default if you are using @option{-fsanitize=address} option.
+To disable global objects protection use @option{--param asan-globals=0} option.
+
+@item asan-stack
+Enable buffer overflow detection for stack objects. This kind of protection
+is enabled by default if you are using @option{-fsanitize=address} option.
+To disable stack protection use @option{--param asan-stack=0} option.
+
+@item asan-instrument-reads
+Enable buffer overflow detection for memory reads instructions. This kind of protection
+is enabled by default if you are using @option{-fsanitize=address} option.
+To disable memory reads instructions protection use @option{--param asan-instrument-reads=0} option.
+
+@item asan-instrument-writes
+Enable buffer overflow detection for memory writes instructions. This kind of protection
+is enabled by default if you are using @option{-fsanitize=address} option.
+To disable memory writes instructions protection use @option{--param asan-instrument-writes=0} option.
+
+@item asan-memintrin
+Enable detection for builtin functions. This kind of protection
+is enabled by default if you are using @option{-fsanitize=address} option.
+To disable builtin functions protection use @option{--param asan-memintrin=0} option.
+
+@item asan-use-after-return
+Enable detection of use-after-return. This kind of protection is enabled by default if you are using
+@option{-fsanitize=address} option.
+To disable builtin functions protection use @option{--param asan-use-after-return=0} option.
+
 @end table
 @end table
 
diff --git a/gcc/params.def b/gcc/params.def
index c0f9622..60ce2df 100644
--- a/gcc/params.def
+++ b/gcc/params.def
@@ -1049,7 +1049,37 @@  DEFPARAM (PARAM_MAX_SLSR_CANDIDATE_SCAN,
 	  "strength reduction",
 	  50, 1, 999999)
 
+DEFPARAM(PARAM_ASAN_STACK,
+         "asan-stack",
+         "Enable asan stack protection",
+         1, 0, 1)
+
+DEFPARAM(PARAM_ASAN_GLOBALS,
+         "asan-globals",
+         "Enable asan globals protection",
+         1, 0, 1)
+
+DEFPARAM(PARAM_ASAN_INSTRUMENT_WRITES,
+         "asan-instrument-writes",
+         "Enable asan store operations protection",
+         1, 0, 1)
+
+DEFPARAM(PARAM_ASAN_INSTRUMENT_READS,
+         "asan-instrument-reads",
+         "Enable asan load operations protection",
+         1, 0, 1)
+
+DEFPARAM(PARAM_ASAN_MEMINTRIN,
+         "asan-memintrin",
+         "Enable asan builtin functions protection",
+         1, 0, 1)
+
+DEFPARAM(PARAM_ASAN_USE_AFTER_RETURN,
+         "asan-use-after-return",
+         "Enable asan builtin functions protection",
+         1, 0, 1)
 /*
+
 Local variables:
 mode:c
 End:
diff --git a/gcc/params.h b/gcc/params.h
index f137e9e..49c1a2d 100644
--- a/gcc/params.h
+++ b/gcc/params.h
@@ -218,5 +218,17 @@  extern void init_param_values (int *params);
   PARAM_VALUE (PARAM_ALLOW_PACKED_LOAD_DATA_RACES)
 #define ALLOW_PACKED_STORE_DATA_RACES \
   PARAM_VALUE (PARAM_ALLOW_PACKED_STORE_DATA_RACES)
+#define ASAN_STACK \
+  PARAM_VALUE (PARAM_ASAN_STACK)
+#define ASAN_GLOBALS \
+  PARAM_VALUE (PARAM_ASAN_GLOBALS)
+#define ASAN_INSTRUMENT_READS \
+  PARAM_VALUE (PARAM_ASAN_INSTRUMENT_READS)
+#define ASAN_INSTRUMENT_WRITES \
+  PARAM_VALUE (PARAM_ASAN_INSTRUMENT_WRITES)
+#define ASAN_MEMINTRIN \
+  PARAM_VALUE (PARAM_ASAN_MEMINTRIN)
+#define ASAN_USE_AFTER_RETURN \
+  PARAM_VALUE (PARAM_ASAN_USE_AFTER_RETURN)
 
 #endif /* ! GCC_PARAMS_H */
diff --git a/gcc/testsuite/c-c++-common/asan/global-overflow-2.c b/gcc/testsuite/c-c++-common/asan/global-overflow-2.c
new file mode 100644
index 0000000..5fe7ba3
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/global-overflow-2.c
@@ -0,0 +1,22 @@ 
+/* { dg-do run } */
+/* { dg-options "--param asan-globals=0" } */
+#include <stdio.h>
+extern
+#ifdef __cplusplus
+"C"
+#endif
+void *memset (void *, int, __SIZE_TYPE__);
+
+volatile int ten = 10;
+
+int main() {
+  static char XXX[10];
+  static char YYY[10];
+  static char ZZZ[10];
+  memset(XXX, 0, 10);
+  memset(YYY, 0, 10);
+  memset(ZZZ, 0, 10);
+  int res = YYY[ten];  /* BOOOM */
+  res += XXX[ten/10] + ZZZ[ten/10];
+  return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/asan/memcmp-3.c b/gcc/testsuite/c-c++-common/asan/memcmp-3.c
new file mode 100644
index 0000000..5b28155
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/memcmp-3.c
@@ -0,0 +1,14 @@ 
+/* { dg-do run } */
+/* { dg-options "--param asan-stack=0" } */
+#include <string.h>
+
+volatile int one = 1;
+
+int
+main ()
+{
+  volatile char a1[] = {one, 2, 3, 4};
+  volatile char a2[] = {1, 2*one, 3, 4};
+  int res = memcmp ((void*)a1,(void*) a2, 5 + one);
+  return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/asan/no-instrument-reads.c b/gcc/testsuite/c-c++-common/asan/no-instrument-reads.c
new file mode 100644
index 0000000..6098545
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/no-instrument-reads.c
@@ -0,0 +1,18 @@ 
+/* { dg-do run } */
+/* { dg-options "--param asan-instrument-reads=0" } */
+
+extern
+#ifdef __cplusplus
+"C"
+#endif
+void *memset (void *, int, __SIZE_TYPE__);
+
+volatile int ten = 10;
+
+int main() {
+  volatile char x[10];
+  memset((void*) x, 0, 10);
+  volatile int a = x[ten];  /* BOOOM */
+  return 0;
+}
+
diff --git a/gcc/testsuite/c-c++-common/asan/no-instrument-writes.c b/gcc/testsuite/c-c++-common/asan/no-instrument-writes.c
new file mode 100644
index 0000000..2569f39
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/no-instrument-writes.c
@@ -0,0 +1,19 @@ 
+/* { dg-do run } */
+/* { dg-options "--param asan-instrument-writes=0" } */
+
+extern
+#ifdef __cplusplus
+"C"
+#endif
+void *memset (void *, int, __SIZE_TYPE__);
+
+volatile int ten = 10;
+
+int main() {
+  volatile char x[10];
+  volatile int y;
+  memset((void *)x, 0, 10);
+  x[ten] = 1;  /* BOOOM */
+  return 0;
+}
+
diff --git a/gcc/testsuite/c-c++-common/asan/no-stack-use-after-return.c b/gcc/testsuite/c-c++-common/asan/no-stack-use-after-return.c
new file mode 100644
index 0000000..4f9461e
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/no-stack-use-after-return.c
@@ -0,0 +1,49 @@ 
+/* { dg-set-target-env-var ASAN_OPTIONS "detect_stack_use_after_return=1" } */
+/* { dg-options "--param asan-use-after-return=0" } */
+/* { dg-do run } */
+
+#include <stdio.h>
+#include <pthread.h>
+
+#ifndef kSize
+# define kSize 1
+#endif
+
+#ifndef UseThread
+# define UseThread 0
+#endif
+
+__attribute__((noinline))
+char *Ident(char *x) {
+  fprintf(stderr, "1: %p\n", x);
+  return x;
+}
+
+__attribute__((noinline))
+char *Func1() {
+  char local[kSize];
+  return Ident(local);
+}
+
+__attribute__((noinline))
+void Func2(char *x) {
+  fprintf(stderr, "2: %p\n", x);
+  *x = 1;
+}
+
+void *Thread(void *unused)  {
+  Func2(Func1());
+  return NULL;
+}
+
+int main(int argc, char **argv) {
+#if UseThread
+  pthread_t t;
+  pthread_create(&t, 0, Thread, 0);
+  pthread_join(t, 0);
+#else
+  Func2(Func1());
+#endif
+  return 0;
+}
+
diff --git a/gcc/testsuite/c-c++-common/asan/stack-use-after-return.c b/gcc/testsuite/c-c++-common/asan/stack-use-after-return.c
new file mode 100644
index 0000000..9194d83
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/stack-use-after-return.c
@@ -0,0 +1,53 @@ 
+/* { dg-set-target-env-var ASAN_OPTIONS "detect_stack_use_after_return=1" } */
+/* { dg-do run } */
+/* { dg-shouldfail "asan" } */
+
+#include <stdio.h>
+#include <pthread.h>
+
+#ifndef kSize
+# define kSize 1
+#endif
+
+#ifndef UseThread
+# define UseThread 0
+#endif
+
+__attribute__((noinline))
+char *Ident(char *x) {
+  fprintf(stderr, "1: %p\n", x);
+  return x;
+}
+
+__attribute__((noinline))
+char *Func1() {
+  char local[kSize];
+  return Ident(local);
+}
+
+__attribute__((noinline))
+void Func2(char *x) {
+  fprintf(stderr, "2: %p\n", x);
+  *x = 1;
+}
+
+void *Thread(void *unused)  {
+  Func2(Func1());
+  return NULL;
+}
+
+int main(int argc, char **argv) {
+#if UseThread
+  pthread_t t;
+  pthread_create(&t, 0, Thread, 0);
+  pthread_join(t, 0);
+#else
+  Func2(Func1());
+#endif
+  return 0;
+}
+
+/* { dg-output "WRITE of size 1 .* thread T0(\n|\r\n|\r).*" } */
+/* { dg-output "    #0 .* Func2.*stack-use-after-return.c:31.*" }*/
+/* { dg-output "is located in stack of thread T0 at offset.*" }*/
+/* { dg-output "\'local\' <== Memory access at offset 32 is inside this variable.*" }*/