public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Add -fsanitize=pointer-{compare,subtract}.
@ 2017-10-06 12:46 Martin Liška
  2017-10-06 13:34 ` Jakub Jelinek
  0 siblings, 1 reply; 23+ messages in thread
From: Martin Liška @ 2017-10-06 12:46 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jakub Jelinek

[-- Attachment #1: Type: text/plain, Size: 2558 bytes --]

Hi.

Adding a missing functionality mentioned and explained here:
https://github.com/google/sanitizers/wiki/AddressSanitizerClangVsGCC-(5.0-vs-7.1)#feature-8

Currently it only works for heap allocated variables. I'm working on support for stack and global
variables.

The functionality is not included in -fsanitize=address by default, one needs to explicitly ask
for both instrumentation and enabling in run-time. It's expensive check.

Ready to be installed?
Martin

gcc/ChangeLog:

2017-10-06  Martin Liska  <mliska@suse.cz>

	* asan.c (is_pointer_compare_opcode): New function.
	(instrument_pointer_comparison): Likewise.
	(asan_instrument): Handle SANITIZE_POINTER_COMPARE and
	SANITIZE_POINTER_SUBTRACT.
	(gate_asan): Likewise.
	* doc/invoke.texi: Document the options.
	* flag-types.h (enum sanitize_code): Add
	SANITIZE_POINTER_COMPARE and SANITIZE_POINTER_SUBTRACT.
	* gcc.c (SANITIZER_EARLY_SPEC): Handle
	-fsanitize=pointer-compare and -fsanitize=pointer-subtract.
	(SANITIZER_SPEC): Likewise.
	(sanitize_spec_function): Likewise.
	* opts.c (finish_options): Add checking for the options.
	* sanitizer.def (BUILT_IN_ASAN_POINTER_COMPARE): New builtin.
	(BUILT_IN_ASAN_POINTER_SUBTRACT): Likewise.
	* toplev.c (process_options):  Handle SANITIZE_POINTER_COMPARE and
	SANITIZE_POINTER_SUBTRACT.

gcc/testsuite/ChangeLog:

2017-10-06  Martin Liska  <mliska@suse.cz>

	* gcc.dg/asan/pointer-compare-1.c: New test.
	* gcc.dg/asan/pointer-compare-2.c: New test.
	* gcc.dg/asan/pointer-subtract-1.c: New test.
	* gcc.dg/asan/pointer-subtract-2.c: New test.
---
 gcc/asan.c                                     | 104 ++++++++++++++++++++++++-
 gcc/doc/invoke.texi                            |  20 +++++
 gcc/flag-types.h                               |   2 +
 gcc/gcc.c                                      |  12 ++-
 gcc/opts.c                                     |  10 +++
 gcc/sanitizer.def                              |   4 +
 gcc/testsuite/gcc.dg/asan/pointer-compare-1.c  |  31 ++++++++
 gcc/testsuite/gcc.dg/asan/pointer-compare-2.c  |  19 +++++
 gcc/testsuite/gcc.dg/asan/pointer-subtract-1.c |  31 ++++++++
 gcc/testsuite/gcc.dg/asan/pointer-subtract-2.c |  19 +++++
 gcc/toplev.c                                   |   4 +-
 11 files changed, 253 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/asan/pointer-compare-1.c
 create mode 100644 gcc/testsuite/gcc.dg/asan/pointer-compare-2.c
 create mode 100644 gcc/testsuite/gcc.dg/asan/pointer-subtract-1.c
 create mode 100644 gcc/testsuite/gcc.dg/asan/pointer-subtract-2.c



[-- Attachment #2: 0001-Add-fsanitize-pointer-compare-subtract.patch --]
[-- Type: text/x-patch, Size: 13384 bytes --]

diff --git a/gcc/asan.c b/gcc/asan.c
index 2aa0a795af2..6feea017795 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -2370,6 +2370,104 @@ maybe_instrument_call (gimple_stmt_iterator *iter)
   return instrumented;
 }
 
+/* Return true if a given opcode CODE is potentially a non-valid comparison
+   of pointer types.  */
+
+static bool
+is_pointer_compare_opcode (tree_code code)
+{
+  return (code == LE_EXPR || code == LT_EXPR || code == GE_EXPR
+	  || code == GT_EXPR);
+}
+
+/* Instrument potential invalid operation executed on pointer types:
+   comparison different from != and == and subtraction of pointers.  */
+
+static void
+instrument_pointer_comparison (void)
+{
+  basic_block bb;
+  gimple_stmt_iterator i;
+
+  bool sanitize_comparison_p = sanitize_flags_p (SANITIZE_POINTER_COMPARE);
+  bool sanitize_subtraction_p = sanitize_flags_p (SANITIZE_POINTER_SUBTRACT);
+
+  FOR_EACH_BB_FN (bb, cfun)
+    {
+      for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
+	{
+	  gimple *s = gsi_stmt (i);
+
+	  tree ptr1 = NULL_TREE;
+	  tree ptr2 = NULL_TREE;
+	  enum built_in_function fn = BUILT_IN_NONE;
+
+	  if (sanitize_comparison_p)
+	    {
+	      if (is_gimple_assign (s)
+		  && gimple_assign_rhs_class (s) == GIMPLE_BINARY_RHS
+		  && POINTER_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (s)))
+		  && POINTER_TYPE_P (TREE_TYPE (gimple_assign_rhs2 (s)))
+		  && is_pointer_compare_opcode (gimple_assign_rhs_code (s)))
+		{
+		  ptr1 = gimple_assign_rhs1 (s);
+		  ptr2 = gimple_assign_rhs2 (s);
+		  fn = BUILT_IN_ASAN_POINTER_COMPARE;
+		}
+	      else if (gimple_code (s) == GIMPLE_COND
+		       && POINTER_TYPE_P (TREE_TYPE (gimple_cond_lhs (s)))
+		       && POINTER_TYPE_P (TREE_TYPE (gimple_cond_rhs (s)))
+		       && is_pointer_compare_opcode (gimple_cond_code (s)))
+		{
+		  ptr1 = gimple_cond_lhs (s);
+		  ptr2 = gimple_cond_rhs (s);
+		  fn = BUILT_IN_ASAN_POINTER_COMPARE;
+		}
+	    }
+
+	  if (sanitize_subtraction_p
+	      && is_gimple_assign (s)
+	      && gimple_assign_rhs_class (s) == GIMPLE_BINARY_RHS
+	      && gimple_assign_rhs_code (s) == MINUS_EXPR)
+	    {
+	      tree rhs1 = gimple_assign_rhs1 (s);
+	      tree rhs2 = gimple_assign_rhs2 (s);
+
+	      if (TREE_CODE (rhs1) == SSA_NAME
+		  || TREE_CODE (rhs2) == SSA_NAME)
+		{
+		  gassign *def1
+		    = dyn_cast<gassign *>(SSA_NAME_DEF_STMT (rhs1));
+		  gassign *def2
+		    = dyn_cast<gassign *>(SSA_NAME_DEF_STMT (rhs2));
+
+		  if (def1 && def2
+		      && gimple_assign_rhs_class (def1) == GIMPLE_UNARY_RHS
+		      && gimple_assign_rhs_class (def2) == GIMPLE_UNARY_RHS)
+		    {
+		      if (POINTER_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (def1)))
+			  && POINTER_TYPE_P
+			  (TREE_TYPE (gimple_assign_rhs1 (def2))))
+			{
+			  ptr1 = rhs1;
+			  ptr2 = rhs2;
+			  fn = BUILT_IN_ASAN_POINTER_SUBTRACT;
+			}
+		    }
+		}
+	    }
+
+	  if (ptr1 != NULL_TREE && ptr2 != NULL_TREE)
+	    {
+	      tree decl = builtin_decl_implicit (fn);
+	      gimple *g = gimple_build_call (decl, 2, ptr1, ptr2);
+	      gimple_set_location (g, gimple_location (s));
+	      gsi_insert_before (&i, g, GSI_SAME_STMT);
+	    }
+	}
+    }
+}
+
 /* Walk each instruction of all basic block and instrument those that
    represent memory references: loads, stores, or function calls.
    In a given basic block, this function avoids instrumenting memory
@@ -3432,6 +3530,9 @@ asan_instrument (void)
 {
   if (shadow_ptr_types[0] == NULL_TREE)
     asan_init_shadow_ptr_types ();
+
+  if (sanitize_flags_p (SANITIZE_POINTER_COMPARE | SANITIZE_POINTER_SUBTRACT))
+    instrument_pointer_comparison ();
   transform_statements ();
   last_alloca_addr = NULL_TREE;
   return 0;
@@ -3440,7 +3541,8 @@ asan_instrument (void)
 static bool
 gate_asan (void)
 {
-  return sanitize_flags_p (SANITIZE_ADDRESS);
+  return sanitize_flags_p (SANITIZE_ADDRESS | SANITIZE_POINTER_COMPARE
+			   | SANITIZE_POINTER_SUBTRACT);
 }
 
 namespace {
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index f862b7f8c99..f481c29fbb9 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10935,6 +10935,26 @@ Enable AddressSanitizer for Linux kernel.
 See @uref{https://github.com/google/kasan/wiki} for more details.
 The option cannot be combined with @option{-fcheck-pointer-bounds}.
 
+@item -fsanitize=pointer-compare
+@opindex fsanitize=pointer-compare
+Instrument comparison operation (<, <=, >, >=, -) with pointer operands.
+The option cannot be combined with @option{-fsanitize=thread}
+and/or @option{-fcheck-pointer-bounds}.
+Note: By default the check is disabled at run time.  To enable it,
+add @code{detect_invalid_pointer_pairs=1} to the environment variable
+@env{ASAN_OPTIONS}.  The checking currently works only for pointers allocated
+on heap.
+
+@item -fsanitize=subtract
+@opindex fsanitize=pointer-compare
+Instrument subtraction with pointer operands.
+The option cannot be combined with @option{-fsanitize=thread}
+and/or @option{-fcheck-pointer-bounds}.
+Note: By default the check is disabled at run time.  To enable it,
+add @code{detect_invalid_pointer_pairs=1} to the environment variable
+@env{ASAN_OPTIONS}.  The checking currently works only for pointers allocated
+on heap.
+
 @item -fsanitize=thread
 @opindex fsanitize=thread
 Enable ThreadSanitizer, a fast data race detector.
diff --git a/gcc/flag-types.h b/gcc/flag-types.h
index 1f439d35b07..74464651e00 100644
--- a/gcc/flag-types.h
+++ b/gcc/flag-types.h
@@ -246,6 +246,8 @@ enum sanitize_code {
   SANITIZE_VPTR = 1UL << 22,
   SANITIZE_BOUNDS_STRICT = 1UL << 23,
   SANITIZE_POINTER_OVERFLOW = 1UL << 24,
+  SANITIZE_POINTER_COMPARE = 1UL << 25,
+  SANITIZE_POINTER_SUBTRACT = 1UL << 26,
   SANITIZE_SHIFT = SANITIZE_SHIFT_BASE | SANITIZE_SHIFT_EXPONENT,
   SANITIZE_UNDEFINED = SANITIZE_SHIFT | SANITIZE_DIVIDE | SANITIZE_UNREACHABLE
 		       | SANITIZE_VLA | SANITIZE_NULL | SANITIZE_RETURN
diff --git a/gcc/gcc.c b/gcc/gcc.c
index cec3ed5be5f..20ac35ac03d 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -971,7 +971,9 @@ proper position among the other output files.  */
 /* Linker command line options for -fsanitize= early on the command line.  */
 #ifndef SANITIZER_EARLY_SPEC
 #define SANITIZER_EARLY_SPEC "\
-%{!nostdlib:%{!nodefaultlibs:%{%:sanitize(address):" LIBASAN_EARLY_SPEC "} \
+%{!nostdlib:%{!nodefaultlibs:%{%:sanitize(address)\
+    |%:sanitize(pointer-compare)\
+    |%:sanitize(pointer-subtract):" LIBASAN_EARLY_SPEC "} \
     %{%:sanitize(thread):" LIBTSAN_EARLY_SPEC "} \
     %{%:sanitize(leak):" LIBLSAN_EARLY_SPEC "}}}"
 #endif
@@ -981,6 +983,10 @@ proper position among the other output files.  */
 #define SANITIZER_SPEC "\
 %{!nostdlib:%{!nodefaultlibs:%{%:sanitize(address):" LIBASAN_SPEC "\
     %{static:%ecannot specify -static with -fsanitize=address}}\
+    %{%:sanitize(pointer-compare):" LIBASAN_SPEC "\
+    %{static:%ecannot specify -static with -fsanitize=pointer-compare}}\
+    %{%:sanitize(pointer-subtract):" LIBASAN_SPEC "\
+    %{static:%ecannot specify -static with -fsanitize=pointer-subtract}}\
     %{%:sanitize(thread):" LIBTSAN_SPEC "\
     %{static:%ecannot specify -static with -fsanitize=thread}}\
     %{%:sanitize(undefined):" LIBUBSAN_SPEC "}\
@@ -9421,6 +9427,10 @@ sanitize_spec_function (int argc, const char **argv)
     return (flag_sanitize & SANITIZE_USER_ADDRESS) ? "" : NULL;
   if (strcmp (argv[0], "kernel-address") == 0)
     return (flag_sanitize & SANITIZE_KERNEL_ADDRESS) ? "" : NULL;
+  if (strcmp (argv[0], "pointer-compare") == 0)
+    return (flag_sanitize & SANITIZE_POINTER_COMPARE) ? "" : NULL;
+  if (strcmp (argv[0], "pointer-subtract") == 0)
+    return (flag_sanitize & SANITIZE_POINTER_SUBTRACT) ? "" : NULL;
   if (strcmp (argv[0], "thread") == 0)
     return (flag_sanitize & SANITIZE_THREAD) ? "" : NULL;
   if (strcmp (argv[0], "undefined") == 0)
diff --git a/gcc/opts.c b/gcc/opts.c
index 5aa5d066dbe..6d45ddf574c 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -966,6 +966,14 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
 	      "%<-fsanitize=address%> and %<-fsanitize=kernel-address%> "
 	      "are incompatible with %<-fsanitize=thread%>");
 
+  if ((opts->x_flag_sanitize & SANITIZE_POINTER_COMPARE
+       || opts->x_flag_sanitize & SANITIZE_POINTER_SUBTRACT)
+      && (opts->x_flag_sanitize & SANITIZE_THREAD))
+    error_at (loc,
+	      "%<-fsanitize=pointer-compare%> and "
+	      "%<-fsanitize=pointer-subtract%> "
+	      "are incompatible with %<-fsanitize=thread%>");
+
   if ((opts->x_flag_sanitize & SANITIZE_LEAK)
       && (opts->x_flag_sanitize & SANITIZE_THREAD))
     error_at (loc,
@@ -1496,6 +1504,8 @@ const struct sanitizer_opts_s sanitizer_opts[] =
   SANITIZER_OPT (address, (SANITIZE_ADDRESS | SANITIZE_USER_ADDRESS), true),
   SANITIZER_OPT (kernel-address, (SANITIZE_ADDRESS | SANITIZE_KERNEL_ADDRESS),
 		 true),
+  SANITIZER_OPT (pointer-compare, SANITIZE_POINTER_COMPARE, true),
+  SANITIZER_OPT (pointer-subtract, SANITIZE_POINTER_SUBTRACT, true),
   SANITIZER_OPT (thread, SANITIZE_THREAD, false),
   SANITIZER_OPT (leak, SANITIZE_LEAK, false),
   SANITIZER_OPT (shift, SANITIZE_SHIFT, true),
diff --git a/gcc/sanitizer.def b/gcc/sanitizer.def
index 9d963f05c21..d06f68ba66e 100644
--- a/gcc/sanitizer.def
+++ b/gcc/sanitizer.def
@@ -175,6 +175,10 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_ALLOCA_POISON, "__asan_alloca_poison",
 		      BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST)
 DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_ALLOCAS_UNPOISON, "__asan_allocas_unpoison",
 		      BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_POINTER_COMPARE, "__sanitizer_ptr_cmp",
+		      BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_POINTER_SUBTRACT, "__sanitizer_ptr_sub",
+		      BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST)
 
 /* Thread Sanitizer */
 DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_INIT, "__tsan_init", 
diff --git a/gcc/testsuite/gcc.dg/asan/pointer-compare-1.c b/gcc/testsuite/gcc.dg/asan/pointer-compare-1.c
new file mode 100644
index 00000000000..3d8ab19c9f9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/pointer-compare-1.c
@@ -0,0 +1,31 @@
+// { dg-do run }
+// { dg-shouldfail "asan" }
+// { dg-set-target-env-var ASAN_OPTIONS "detect_invalid_pointer_pairs=1 halt_on_error=0" }
+// { dg-options "-fsanitize=pointer-compare -O0" }
+
+int f(char *p, char *q)
+{
+  return p > q;
+}
+
+int f2(char *p)
+{
+  char *p2 = p + 20;
+  __builtin_free(p);
+  return p > p2;
+}
+
+int
+main ()
+{
+  char *p = (char *)__builtin_malloc(42);
+  char *q = (char *)__builtin_malloc(42);
+
+  int r = f(p, q) + f2(p);
+  __builtin_free (q);
+
+  return 1;
+}
+
+// { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*(\n|\r\n|\r)*" }
+// { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*(\n|\r\n|\r)" }
diff --git a/gcc/testsuite/gcc.dg/asan/pointer-compare-2.c b/gcc/testsuite/gcc.dg/asan/pointer-compare-2.c
new file mode 100644
index 00000000000..fe6ff9bc7e1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/pointer-compare-2.c
@@ -0,0 +1,19 @@
+// { dg-do run }
+// { dg-set-target-env-var ASAN_OPTIONS "detect_invalid_pointer_pairs=1 halt_on_error=0" }
+// { dg-options "-fsanitize=pointer-compare -O0" }
+
+int f(char *p)
+{
+  char *p2 = p + 20;
+  return p > p2;
+}
+
+int
+main ()
+{
+  char *p = (char *)__builtin_malloc(42);
+
+  int r = f(p);
+  __builtin_free (p);
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/asan/pointer-subtract-1.c b/gcc/testsuite/gcc.dg/asan/pointer-subtract-1.c
new file mode 100644
index 00000000000..b69969c3091
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/pointer-subtract-1.c
@@ -0,0 +1,31 @@
+// { dg-do run }
+// { dg-shouldfail "asan" }
+// { dg-set-target-env-var ASAN_OPTIONS "detect_invalid_pointer_pairs=1 halt_on_error=0" }
+// { dg-options "-fsanitize=pointer-subtract -O0" }
+
+int f(char *p, char *q)
+{
+  return p - q;
+}
+
+int f2(char *p)
+{
+  char *p2 = p + 20;
+  __builtin_free(p);
+  return p2 - p;
+}
+
+int
+main ()
+{
+  char *p = (char *)__builtin_malloc(42);
+  char *q = (char *)__builtin_malloc(42);
+
+  int r = f(p, q) + f2(p);
+  __builtin_free (q);
+
+  return 1;
+}
+
+// { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*(\n|\r\n|\r)*" }
+// { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*(\n|\r\n|\r)" }
diff --git a/gcc/testsuite/gcc.dg/asan/pointer-subtract-2.c b/gcc/testsuite/gcc.dg/asan/pointer-subtract-2.c
new file mode 100644
index 00000000000..be4a3c5f2c2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/pointer-subtract-2.c
@@ -0,0 +1,19 @@
+// { dg-do run }
+// { dg-set-target-env-var ASAN_OPTIONS "detect_invalid_pointer_pairs=1 halt_on_error=0" }
+// { dg-options "-fsanitize=pointer-subtract -O0" }
+
+int f(char *p)
+{
+  char *p2 = p + 20;
+  return p - p2;
+}
+
+int
+main ()
+{
+  char *p = (char *)__builtin_malloc(42);
+
+  int r = f(p);
+  __builtin_free (p);
+  return 0;
+}
diff --git a/gcc/toplev.c b/gcc/toplev.c
index bee79d313b2..cd52f8776d1 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1302,7 +1302,9 @@ process_options (void)
 	  flag_check_pointer_bounds = 0;
 	}
 
-      if (flag_sanitize & SANITIZE_ADDRESS)
+      if (flag_sanitize & SANITIZE_ADDRESS
+	  || flag_sanitize & SANITIZE_POINTER_COMPARE
+	  || flag_sanitize & SANITIZE_POINTER_SUBTRACT)
 	{
 	  error_at (UNKNOWN_LOCATION,
 		    "%<-fcheck-pointer-bounds%> is not supported with "


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2017-12-21  8:37 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-06 12:46 [PATCH] Add -fsanitize=pointer-{compare,subtract} Martin Liška
2017-10-06 13:34 ` Jakub Jelinek
2017-10-11  6:13   ` Martin Liška
2017-10-11  7:39     ` Jakub Jelinek
2017-10-11 13:50       ` Martin Liška
2017-10-11 14:31         ` Jakub Jelinek
2017-10-12 11:30           ` Martin Liška
2017-10-12 11:34             ` Martin Liška
2017-10-12 11:38               ` Jakub Jelinek
2017-10-12 11:38             ` Jakub Jelinek
2017-10-13 11:17               ` Martin Liška
2017-10-13 11:20                 ` Jakub Jelinek
2017-10-13 13:02                   ` Martin Liška
2017-10-13 13:16                     ` Jakub Jelinek
2017-10-16 12:03                       ` Martin Liška
2017-10-16 12:34                         ` Jakub Jelinek
2017-10-16 13:41                           ` Martin Liška
2017-10-16 13:45                             ` Jakub Jelinek
2017-10-16 20:45                               ` Martin Liška
2017-11-21 12:42                                 ` Martin Liška
2017-12-05  9:28                                   ` Jakub Jelinek
2017-12-18  8:25                                     ` Martin Liška
2017-12-21  8:37                                     ` Martin Liška

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).