This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.


On 10/16/2017 10:39 PM, Martin Liška wrote:
> Hi.
> 
> All nits included in mainline review request I've just done:
> https://reviews.llvm.org/D38971
> 
> Martin

Hi.

There's updated version of patch where I added new test-cases and it's rebased
with latest version of libsanitizer changes. This is subject for libsanitizer review process.

Martin
>From 100b723b9b7fb10dedb2154f30e1ebd6ef885ab4 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Wed, 8 Nov 2017 13:16:17 +0100
Subject: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

gcc/ChangeLog:

2017-11-21  Martin Liska  <mliska@suse.cz>

	* doc/invoke.texi: Document the options.
	* flag-types.h (enum sanitize_code): Add
	SANITIZE_POINTER_COMPARE and SANITIZE_POINTER_SUBTRACT.
	* ipa-inline.c (sanitize_attrs_match_for_inline_p): Add handling
	of SANITIZE_POINTER_COMPARE and SANITIZE_POINTER_SUBTRACT.
	* opts.c: Define new sanitizer options.
	* sanitizer.def (BUILT_IN_ASAN_POINTER_COMPARE): Likewise.
	(BUILT_IN_ASAN_POINTER_SUBTRACT): Likewise.

gcc/c/ChangeLog:

2017-11-21  Martin Liska  <mliska@suse.cz>

	* c-typeck.c (pointer_diff): Add new argument and instrument
	pointer subtraction.
	(build_binary_op): Similar for pointer comparison.

gcc/cp/ChangeLog:

2017-11-21  Martin Liska  <mliska@suse.cz>

	* typeck.c (pointer_diff): Add new argument and instrument
	pointer subtraction.
	(cp_build_binary_op): Create compound expression if doing an
	instrumentation.

gcc/testsuite/ChangeLog:

2017-11-21  Martin Liska  <mliska@suse.cz>

	* c-c++-common/asan/pointer-compare-1.c: New test.
	* c-c++-common/asan/pointer-compare-2.c: New test.
	* c-c++-common/asan/pointer-subtract-1.c: New test.
	* c-c++-common/asan/pointer-subtract-2.c: New test.
	* c-c++-common/asan/pointer-subtract-3.c: New test.
	* c-c++-common/asan/pointer-subtract-4.c: New test.
---
 gcc/c/c-typeck.c                                   | 31 ++++++--
 gcc/cp/typeck.c                                    | 39 ++++++++--
 gcc/doc/invoke.texi                                | 22 ++++++
 gcc/flag-types.h                                   |  2 +
 gcc/ipa-inline.c                                   |  8 ++-
 gcc/opts.c                                         | 15 ++++
 gcc/sanitizer.def                                  |  4 ++
 .../c-c++-common/asan/pointer-compare-1.c          | 83 ++++++++++++++++++++++
 .../c-c++-common/asan/pointer-compare-2.c          | 76 ++++++++++++++++++++
 .../c-c++-common/asan/pointer-subtract-1.c         | 41 +++++++++++
 .../c-c++-common/asan/pointer-subtract-2.c         | 33 +++++++++
 .../c-c++-common/asan/pointer-subtract-3.c         | 40 +++++++++++
 .../c-c++-common/asan/pointer-subtract-4.c         | 40 +++++++++++
 libsanitizer/asan/asan_descriptions.cc             | 20 ++++++
 libsanitizer/asan/asan_descriptions.h              |  4 ++
 libsanitizer/asan/asan_report.cc                   | 53 ++++++++++++--
 libsanitizer/asan/asan_thread.cc                   | 25 ++++++-
 libsanitizer/asan/asan_thread.h                    |  3 +
 18 files changed, 521 insertions(+), 18 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-compare-1.c
 create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-compare-2.c
 create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-1.c
 create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-2.c
 create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-3.c
 create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-4.c

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 4bdc48a9ea3..5dac9bdf08b 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -96,7 +96,7 @@ static tree lookup_field (tree, tree);
 static int convert_arguments (location_t, vec<location_t>, tree,
 			      vec<tree, va_gc> *, vec<tree, va_gc> *, tree,
 			      tree);
-static tree pointer_diff (location_t, tree, tree);
+static tree pointer_diff (location_t, tree, tree, tree *);
 static tree convert_for_assignment (location_t, location_t, tree, tree, tree,
 				    enum impl_conv, bool, tree, tree, int);
 static tree valid_compound_expr_initializer (tree, tree);
@@ -3778,10 +3778,11 @@ parser_build_binary_op (location_t location, enum tree_code code,
 }
 
 /* Return a tree for the difference of pointers OP0 and OP1.
-   The resulting tree has type int.  */
+   The resulting tree has type int.  If POINTER_SUBTRACT sanitization is
+   enabled, assign to INSTRUMENT_EXPR call to libsanitizer.  */
 
 static tree
-pointer_diff (location_t loc, tree op0, tree op1)
+pointer_diff (location_t loc, tree op0, tree op1, tree *instrument_expr)
 {
   tree restype = ptrdiff_type_node;
   tree result, inttype;
@@ -3825,6 +3826,17 @@ pointer_diff (location_t loc, tree op0, tree op1)
     pedwarn (loc, OPT_Wpointer_arith,
 	     "pointer to a function used in subtraction");
 
+  if (sanitize_flags_p (SANITIZE_POINTER_SUBTRACT))
+    {
+      gcc_assert (current_function_decl != NULL_TREE);
+
+      op0 = save_expr (op0);
+      op1 = save_expr (op1);
+
+      tree tt = builtin_decl_explicit (BUILT_IN_ASAN_POINTER_SUBTRACT);
+      *instrument_expr = build_call_expr_loc (loc, tt, 2, op0, op1);
+    }
+
   /* First do the subtraction as integers;
      then drop through to build the divide operator.
      Do not do default conversions on the minus operator
@@ -11185,7 +11197,7 @@ build_binary_op (location_t location, enum tree_code code,
       if (code0 == POINTER_TYPE && code1 == POINTER_TYPE
 	  && comp_target_types (location, type0, type1))
 	{
-	  ret = pointer_diff (location, op0, op1);
+	  ret = pointer_diff (location, op0, op1, &instrument_expr);
 	  goto return_build_binary_op;
 	}
       /* Handle pointer minus int.  Just like pointer plus int.  */
@@ -11735,6 +11747,17 @@ build_binary_op (location_t location, enum tree_code code,
 	  result_type = type1;
 	  pedwarn (location, 0, "comparison between pointer and integer");
 	}
+
+      if ((code0 == POINTER_TYPE || code1 == POINTER_TYPE)
+	  && sanitize_flags_p (SANITIZE_POINTER_COMPARE))
+	{
+	  op0 = save_expr (op0);
+	  op1 = save_expr (op1);
+
+	  tree tt = builtin_decl_explicit (BUILT_IN_ASAN_POINTER_COMPARE);
+	  instrument_expr = build_call_expr_loc (location, tt, 2, op0, op1);
+	}
+
       if ((TREE_CODE (TREE_TYPE (orig_op0)) == BOOLEAN_TYPE
 	   || truth_value_p (TREE_CODE (orig_op0)))
 	  ^ (TREE_CODE (TREE_TYPE (orig_op1)) == BOOLEAN_TYPE
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 9130c10f390..e2474267436 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -54,7 +54,7 @@ static tree rationalize_conditional_expr (enum tree_code, tree,
 static int comp_ptr_ttypes_real (tree, tree, int);
 static bool comp_except_types (tree, tree, bool);
 static bool comp_array_types (const_tree, const_tree, bool);
-static tree pointer_diff (location_t, tree, tree, tree, tsubst_flags_t);
+static tree pointer_diff (location_t, tree, tree, tree, tsubst_flags_t, tree *);
 static tree get_delta_difference (tree, tree, bool, bool, tsubst_flags_t);
 static void casts_away_constness_r (tree *, tree *, tsubst_flags_t);
 static bool casts_away_constness (tree, tree, tsubst_flags_t);
@@ -4318,8 +4318,16 @@ cp_build_binary_op (location_t location,
       if (code0 == POINTER_TYPE && code1 == POINTER_TYPE
 	  && same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (type0),
 							TREE_TYPE (type1)))
-	return pointer_diff (location, op0, op1,
-			     common_pointer_type (type0, type1), complain);
+	{
+	  result = pointer_diff (location, op0, op1,
+				 common_pointer_type (type0, type1), complain,
+				 &instrument_expr);
+	  if (instrument_expr != NULL)
+	    result = build2 (COMPOUND_EXPR, TREE_TYPE (result),
+			     instrument_expr, result);
+
+	  return result;
+	}
       /* In all other cases except pointer - int, the usual arithmetic
 	 rules apply.  */
       else if (!(code0 == POINTER_TYPE && code1 == INTEGER_TYPE))
@@ -5008,6 +5016,17 @@ cp_build_binary_op (location_t location,
           else
             return error_mark_node;
 	}
+
+      if ((code0 == POINTER_TYPE || code1 == POINTER_TYPE)
+	  && sanitize_flags_p (SANITIZE_POINTER_COMPARE))
+	{
+	  op0 = save_expr (op0);
+	  op1 = save_expr (op1);
+
+	  tree tt = builtin_decl_explicit (BUILT_IN_ASAN_POINTER_COMPARE);
+	  instrument_expr = build_call_expr_loc (location, tt, 2, op0, op1);
+	}
+
       break;
 
     case UNORDERED_EXPR:
@@ -5363,11 +5382,12 @@ cp_pointer_int_sum (location_t loc, enum tree_code resultcode, tree ptrop,
 }
 
 /* Return a tree for the difference of pointers OP0 and OP1.
-   The resulting tree has type int.  */
+   The resulting tree has type int.  If POINTER_SUBTRACT sanitization is
+   enabled, assign to INSTRUMENT_EXPR call to libsanitizer.  */
 
 static tree
 pointer_diff (location_t loc, tree op0, tree op1, tree ptrtype,
-	      tsubst_flags_t complain)
+	      tsubst_flags_t complain, tree *instrument_expr)
 {
   tree result;
   tree restype = ptrdiff_type_node;
@@ -5401,6 +5421,15 @@ pointer_diff (location_t loc, tree op0, tree op1, tree ptrtype,
 	return error_mark_node;
     }
 
+  if (sanitize_flags_p (SANITIZE_POINTER_SUBTRACT))
+    {
+      op0 = save_expr (op0);
+      op1 = save_expr (op1);
+
+      tree tt = builtin_decl_explicit (BUILT_IN_ASAN_POINTER_SUBTRACT);
+      *instrument_expr = build_call_expr_loc (loc, tt, 2, op0, op1);
+    }
+
   /* First do the subtraction as integers;
      then drop through to build the divide operator.  */
 
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 2ef88e081f9..4f6f6cd30ce 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10984,6 +10984,28 @@ 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 must be combined with either @option{-fsanitize=kernel-address} or
+@option{-fsanitize=address}
+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}.
+
+@item -fsanitize=pointer-subtract
+@opindex fsanitize=pointer-subtract
+Instrument subtraction with pointer operands.
+The option must be combined with either @option{-fsanitize=kernel-address} or
+@option{-fsanitize=address}
+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}.
+
 @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 591b74457cd..3073c661872 100644
--- a/gcc/flag-types.h
+++ b/gcc/flag-types.h
@@ -246,6 +246,8 @@ enum sanitize_code {
   SANITIZE_BOUNDS_STRICT = 1UL << 23,
   SANITIZE_POINTER_OVERFLOW = 1UL << 24,
   SANITIZE_BUILTIN = 1UL << 25,
+  SANITIZE_POINTER_COMPARE = 1UL << 26,
+  SANITIZE_POINTER_SUBTRACT = 1UL << 27,
   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/ipa-inline.c b/gcc/ipa-inline.c
index 687996876ce..6969b5365af 100644
--- a/gcc/ipa-inline.c
+++ b/gcc/ipa-inline.c
@@ -263,8 +263,12 @@ sanitize_attrs_match_for_inline_p (const_tree caller, const_tree callee)
   if (!caller || !callee)
     return true;
 
-  return sanitize_flags_p (SANITIZE_ADDRESS, caller)
-    == sanitize_flags_p (SANITIZE_ADDRESS, callee);
+  return ((sanitize_flags_p (SANITIZE_ADDRESS, caller)
+	   == sanitize_flags_p (SANITIZE_ADDRESS, callee))
+	  && (sanitize_flags_p (SANITIZE_POINTER_COMPARE, caller)
+	      == sanitize_flags_p (SANITIZE_POINTER_COMPARE, callee))
+	  && (sanitize_flags_p (SANITIZE_POINTER_SUBTRACT, caller)
+	      == sanitize_flags_p (SANITIZE_POINTER_SUBTRACT, callee)));
 }
 
 /* Used for flags where it is safe to inline when caller's value is
diff --git a/gcc/opts.c b/gcc/opts.c
index ac383d48ec1..c4dded7ff55 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -952,6 +952,19 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
   if (opts->x_dwarf_split_debug_info)
     opts->x_debug_generate_pub_sections = 2;
 
+  if ((opts->x_flag_sanitize
+       & (SANITIZE_USER_ADDRESS | SANITIZE_KERNEL_ADDRESS)) == 0)
+    {
+      if (opts->x_flag_sanitize & SANITIZE_POINTER_COMPARE)
+	error_at (loc,
+		  "%<-fsanitize=pointer-compare%> must be combined with "
+		  "%<-fsanitize=address%> or %<-fsanitize=kernel-address%>");
+      if (opts->x_flag_sanitize & SANITIZE_POINTER_SUBTRACT)
+	error_at (loc,
+		  "%<-fsanitize=pointer-subtract%> must be combined with "
+		  "%<-fsanitize=address%> or %<-fsanitize=kernel-address%>");
+    }
+
   /* Userspace and kernel ASan conflict with each other.  */
   if ((opts->x_flag_sanitize & SANITIZE_USER_ADDRESS)
       && (opts->x_flag_sanitize & SANITIZE_KERNEL_ADDRESS))
@@ -1496,6 +1509,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 00e7ae031e6..7d224fa081e 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/c-c++-common/asan/pointer-compare-1.c b/gcc/testsuite/c-c++-common/asan/pointer-compare-1.c
new file mode 100644
index 00000000000..742a6a26f07
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/pointer-compare-1.c
@@ -0,0 +1,83 @@
+// { 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=address,pointer-compare -O0" }
+
+int foo(char *p, char *q)
+{
+  return p > q;
+}
+
+char global1[100] = {}, global2[100] = {};
+char small_global[7] = {};
+char large_global[5000] = {};
+
+int
+main ()
+{
+  /* Heap allocated memory.  */
+  char *heap1 = (char *)__builtin_malloc(42);
+  char *heap2 = (char *)__builtin_malloc(42);
+
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo(heap1, heap2);
+  __builtin_free (heap1);
+  __builtin_free (heap2);
+
+  heap1 = (char *)__builtin_malloc(1024);
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo (heap1, heap1 + 1025);
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo (heap1 + 1024, heap1 + 1025);
+  __builtin_free (heap1);
+
+  heap1 = (char *)__builtin_malloc(4096);
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo (heap1, heap1 + 4097);
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo (heap1, 0);
+
+  /* Global variables.  */
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo(&global1[0], &global2[10]);
+
+  char *p = &small_global[0];
+  foo (p, p); // OK
+  foo (p, p + 7); // OK
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo (p, p + 8);
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo (p - 1, p);
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo (p, p - 1);
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo (p - 1, p + 8);
+
+  p = &large_global[0];
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo (p - 1, p);
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo (p, p - 1);
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo (p, &global1[0]);
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo (p, &small_global[0]);
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo (p, 0);
+
+  /* Stack variables.  */
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  char stack1, stack2;
+  foo(&stack1, &stack2);
+
+  /* Mixtures.  */
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo(heap1, &stack1);
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo(heap1, &global1[0]);
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo(&stack1, &global1[0]);
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair" }
+  foo(&stack1, 0);
+  return 1;
+}
diff --git a/gcc/testsuite/c-c++-common/asan/pointer-compare-2.c b/gcc/testsuite/c-c++-common/asan/pointer-compare-2.c
new file mode 100644
index 00000000000..996c4812d41
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/pointer-compare-2.c
@@ -0,0 +1,76 @@
+// { dg-do run }
+// { dg-set-target-env-var ASAN_OPTIONS "detect_invalid_pointer_pairs=1 halt_on_error=1" }
+// { dg-options "-fsanitize=address,pointer-compare -O0" }
+
+int foo(char *p)
+{
+  char *p2 = p + 20;
+  return p > p2;
+}
+
+int bar(char *p, char *q)
+{
+  return p <= q;
+}
+
+int baz(char *p, char *q)
+{
+  return p != 0 && p < q;
+}
+
+char global[8192] = {};
+char small_global[7] = {};
+
+int
+main ()
+{
+  /* Heap allocated memory.  */
+  char *p = (char *)__builtin_malloc(42);
+  int r = foo(p);
+  __builtin_free (p);
+
+  p = (char *)__builtin_malloc(1024);
+  bar(p, p + 1024);
+  bar(p + 1024, p + 1023);
+  bar(p + 1, p + 1023);
+  __builtin_free (p);
+
+  p = (char *)__builtin_malloc(4096);
+  bar(p, p + 4096);
+  bar(p + 10, p + 100);
+  bar(p + 1024, p + 4096);
+  bar(p + 4095, p + 4096);
+  bar(p + 4095, p + 4094);
+  bar(p + 100, p + 4096);
+  bar(p + 100, p + 4094);
+  __builtin_free (p);
+
+  /* Global variable.  */
+  bar(&global[0], &global[1]);
+  bar(&global[1], &global[2]);
+  bar(&global[2], &global[1]);
+  bar(&global[0], &global[100]);
+  bar(&global[1000], &global[7000]);
+  bar(&global[500], &global[10]);
+  p = &global[0];
+  bar(p, p + 8192);
+  p = &global[8000];
+  bar(p, p + 192);
+
+  p = &small_global[0];
+  bar (p, p + 1);
+  bar (p, p + 7);
+  bar (p + 7, p + 1);
+  bar (p + 6, p + 7);
+  bar (p + 7, p + 7);
+
+  /* Stack variable.  */
+  char stack[10000];
+  bar(&stack[0], &stack[100]);
+  bar(&stack[1000], &stack[9000]);
+  bar(&stack[500], &stack[10]);
+
+  baz(0, &stack[10]);
+
+  return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/asan/pointer-subtract-1.c b/gcc/testsuite/c-c++-common/asan/pointer-subtract-1.c
new file mode 100644
index 00000000000..6d8adebf783
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/pointer-subtract-1.c
@@ -0,0 +1,41 @@
+// { 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=address,pointer-subtract -O0" }
+
+int foo(char *p, char *q)
+{
+  return p - q;
+}
+
+char global1[100] = {}, global2[100] = {};
+
+int
+main ()
+{
+  /* Heap allocated memory.  */
+  char *heap1 = (char *)__builtin_malloc(42);
+  char *heap2 = (char *)__builtin_malloc(42);
+
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo(heap1, heap2);
+
+  /* Global variables.  */
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo(&global1[0], &global2[10]);
+
+  /* Stack variables.  */
+  char stack1, stack2;
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo(&stack1, &stack2);
+
+  /* Mixtures.  */
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo(heap1, &stack1);
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" }
+  foo(heap1, &global1[0]);
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair" }
+  foo(&stack1, &global1[0]);
+  return 1;
+}
+
diff --git a/gcc/testsuite/c-c++-common/asan/pointer-subtract-2.c b/gcc/testsuite/c-c++-common/asan/pointer-subtract-2.c
new file mode 100644
index 00000000000..50bd62993fa
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/pointer-subtract-2.c
@@ -0,0 +1,33 @@
+// { dg-do run }
+// { dg-set-target-env-var ASAN_OPTIONS "detect_invalid_pointer_pairs=1 halt_on_error=1" }
+// { dg-options "-fsanitize=address,pointer-subtract -O0" }
+
+int bar(char *p, char *q)
+{
+  return p <= q;
+}
+
+char global[10000] = {};
+
+int
+main ()
+{
+  /* Heap allocated memory.  */
+  char *p = (char *)__builtin_malloc(42);
+  int r = bar(p, p - 20);
+  __builtin_free (p);
+
+  /* Global variable.  */
+  bar(&global[0], &global[100]);
+  bar(&global[1000], &global[9000]);
+  bar(&global[500], &global[10]);
+  bar(&global[0], &global[10000]);
+
+  /* Stack variable.  */
+  char stack[10000];
+  bar(&stack[0], &stack[100]);
+  bar(&stack[1000], &stack[9000]);
+  bar(&stack[500], &stack[10]);
+
+  return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/asan/pointer-subtract-3.c b/gcc/testsuite/c-c++-common/asan/pointer-subtract-3.c
new file mode 100644
index 00000000000..422a9a125e4
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/pointer-subtract-3.c
@@ -0,0 +1,40 @@
+// { dg-do run }
+// { dg-set-target-env-var ASAN_OPTIONS "detect_invalid_pointer_pairs=1:halt_on_error=1" }
+// { dg-options "-fsanitize=address,pointer-subtract -O0" }
+
+#include <unistd.h>
+#include <pthread.h>
+
+char *pointers[2];
+
+void *
+thread_main (void *n)
+{
+  char local;
+
+  unsigned long id = (unsigned long) n;
+  pointers[id] = &local;
+  sleep (1);
+
+  return 0;
+}
+
+int
+main (int argc, char **argv)
+{
+  pthread_t threads[2];
+  pthread_create (&threads[0], 0, thread_main, (void *) 0);
+  pthread_create (&threads[1], 0, thread_main, (void *) 1);
+
+  do
+    {
+    }
+  while (pointers[0] == 0 || pointers[1] == 0);
+
+  unsigned r = pointers[0] - pointers[1];
+
+  pthread_join(threads[0], 0);
+  pthread_join(threads[1], 0);
+
+  return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/asan/pointer-subtract-4.c b/gcc/testsuite/c-c++-common/asan/pointer-subtract-4.c
new file mode 100644
index 00000000000..f64f289c487
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/pointer-subtract-4.c
@@ -0,0 +1,40 @@
+// { dg-do run }
+// { dg-shouldfail "asan" }
+// { dg-set-target-env-var ASAN_OPTIONS "detect_invalid_pointer_pairs=1:halt_on_error=1" }
+// { dg-options "-fsanitize=address,pointer-subtract -O0" }
+
+#include <unistd.h>
+#include <pthread.h>
+
+char *pointer;
+
+void *
+thread_main (void *n)
+{
+  char local;
+  pointer = &local;
+  sleep (1);
+
+  return 0;
+}
+
+int
+main (int argc, char **argv)
+{
+  pthread_t thread;
+  pthread_create (&thread, 0, thread_main, (void *) 0);
+
+  do
+    {
+    }
+  while (pointer == 0);
+
+  char local;
+  char *parent_pointer = &local;
+
+  // { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair" }
+  unsigned r = parent_pointer - pointer;
+  pthread_join(thread, 0);
+
+  return 0;
+}
diff --git a/libsanitizer/asan/asan_descriptions.cc b/libsanitizer/asan/asan_descriptions.cc
index d46962adf27..c856a653742 100644
--- a/libsanitizer/asan/asan_descriptions.cc
+++ b/libsanitizer/asan/asan_descriptions.cc
@@ -333,6 +333,26 @@ void GlobalAddressDescription::Print(const char *bug_type) const {
   }
 }
 
+bool GlobalAddressDescription::PointsInsideTheSameVariable(
+    const GlobalAddressDescription &other) const {
+  if (size == 0 || other.size == 0) return false;
+
+  for (uptr i = 0; i < size; i++) {
+    const __asan_global &a = globals[i];
+    for (uptr j = 0; j < other.size; j++) {
+      const __asan_global &b = other.globals[j];
+      if (a.beg == b.beg &&
+          a.beg <= addr &&
+          b.beg <= other.addr &&
+          (addr + access_size) < (a.beg + a.size) &&
+          (other.addr + other.access_size) < (b.beg + b.size))
+        return true;
+    }
+  }
+
+  return false;
+}
+
 void StackAddressDescription::Print() const {
   Decorator d;
   char tname[128];
diff --git a/libsanitizer/asan/asan_descriptions.h b/libsanitizer/asan/asan_descriptions.h
index 0fbb531492a..12d9ac5f70b 100644
--- a/libsanitizer/asan/asan_descriptions.h
+++ b/libsanitizer/asan/asan_descriptions.h
@@ -143,6 +143,10 @@ struct GlobalAddressDescription {
   u8 size;
 
   void Print(const char *bug_type = "") const;
+
+  // Returns true when this descriptions points inside the same global variable
+  // as other. Descriptions can have different address within the variable
+  bool PointsInsideTheSameVariable(const GlobalAddressDescription &other) const;
 };
 
 bool GetGlobalAddressInformation(uptr addr, uptr access_size,
diff --git a/libsanitizer/asan/asan_report.cc b/libsanitizer/asan/asan_report.cc
index 51bad6e45e3..1f19e726e1e 100644
--- a/libsanitizer/asan/asan_report.cc
+++ b/libsanitizer/asan/asan_report.cc
@@ -295,17 +295,58 @@ static NOINLINE void ReportInvalidPointerPair(uptr pc, uptr bp, uptr sp,
   in_report.ReportError(error);
 }
 
+static bool IsInvalidPointerPair(uptr a1, uptr a2) {
+  if (a1 == a2)
+    return false;
+
+  // 256B in shadow memory can be iterated quite fast
+  static const uptr kMaxOffset = 2048;
+
+  uptr left = a1 < a2 ? a1 : a2;
+  uptr right = a1 < a2 ? a2 : a1;
+  uptr offset = right - left;
+  if (offset <= kMaxOffset)
+    return __asan_region_is_poisoned(left, offset);
+
+  AsanThread *t = GetCurrentThread();
+
+  // check whether left is a stack memory pointer
+  if (uptr shadow_offset1 = t->GetStackVariableShadowStart(left)) {
+    uptr shadow_offset2 = t->GetStackVariableShadowStart(right);
+    return shadow_offset2 == 0 || shadow_offset1 != shadow_offset2;
+  }
+
+  // check whether left is a heap memory address
+  HeapAddressDescription hdesc1, hdesc2;
+  if (GetHeapAddressInformation(left, 0, &hdesc1) &&
+      hdesc1.chunk_access.access_type == kAccessTypeInside)
+    return !GetHeapAddressInformation(right, 0, &hdesc2) ||
+        hdesc2.chunk_access.access_type != kAccessTypeInside ||
+        hdesc1.chunk_access.chunk_begin != hdesc2.chunk_access.chunk_begin;
+
+  // check whether left is an address of a global variable
+  GlobalAddressDescription gdesc1, gdesc2;
+  if (GetGlobalAddressInformation(left, 0, &gdesc1))
+    return !GetGlobalAddressInformation(right - 1, 0, &gdesc2) ||
+        !gdesc1.PointsInsideTheSameVariable(gdesc2);
+
+  if (t->GetStackVariableShadowStart(right) ||
+      GetHeapAddressInformation(right, 0, &hdesc2) ||
+      GetGlobalAddressInformation(right - 1, 0, &gdesc2))
+    return true;
+
+  /* At this point we know nothing about both a1 and a2 addresses.  */
+  return false;
+}
+
 static INLINE void CheckForInvalidPointerPair(void *p1, void *p2) {
   if (!flags()->detect_invalid_pointer_pairs) return;
   uptr a1 = reinterpret_cast<uptr>(p1);
   uptr a2 = reinterpret_cast<uptr>(p2);
-  AsanChunkView chunk1 = FindHeapChunkByAddress(a1);
-  AsanChunkView chunk2 = FindHeapChunkByAddress(a2);
-  bool valid1 = chunk1.IsAllocated();
-  bool valid2 = chunk2.IsAllocated();
-  if (!valid1 || !valid2 || !chunk1.Eq(chunk2)) {
+
+  if (IsInvalidPointerPair(a1, a2)) {
     GET_CALLER_PC_BP_SP;
-    return ReportInvalidPointerPair(pc, bp, sp, a1, a2);
+    ReportInvalidPointerPair(pc, bp, sp, a1, a2);
   }
 }
 // ----------------------- Mac-specific reports ----------------- {{{1
diff --git a/libsanitizer/asan/asan_thread.cc b/libsanitizer/asan/asan_thread.cc
index d0fdf6e9847..178909a52b4 100644
--- a/libsanitizer/asan/asan_thread.cc
+++ b/libsanitizer/asan/asan_thread.cc
@@ -315,7 +315,7 @@ bool AsanThread::GetStackFrameAccessByAddr(uptr addr,
     access->frame_descr = (const char *)((uptr*)bottom)[1];
     return true;
   }
-  uptr aligned_addr = addr & ~(SANITIZER_WORDSIZE/8 - 1);  // align addr.
+  uptr aligned_addr = RoundDownTo(addr, SANITIZER_WORDSIZE / 8);  // align addr.
   uptr mem_ptr = RoundDownTo(aligned_addr, SHADOW_GRANULARITY);
   u8 *shadow_ptr = (u8*)MemToShadow(aligned_addr);
   u8 *shadow_bottom = (u8*)MemToShadow(bottom);
@@ -344,6 +344,29 @@ bool AsanThread::GetStackFrameAccessByAddr(uptr addr,
   return true;
 }
 
+uptr AsanThread::GetStackVariableShadowStart(uptr addr) {
+  uptr bottom = 0;
+  if (AddrIsInStack(addr)) {
+    bottom = stack_bottom();
+  } else if (has_fake_stack()) {
+    bottom = fake_stack()->AddrIsInFakeStack(addr);
+    CHECK(bottom);
+  } else
+    return 0;
+
+  uptr aligned_addr = RoundDownTo(addr, SANITIZER_WORDSIZE / 8);  // align addr.
+  u8 *shadow_ptr = (u8*)MemToShadow(aligned_addr);
+  u8 *shadow_bottom = (u8*)MemToShadow(bottom);
+
+  while (shadow_ptr >= shadow_bottom &&
+         (*shadow_ptr != kAsanStackLeftRedzoneMagic &&
+          *shadow_ptr != kAsanStackMidRedzoneMagic &&
+          *shadow_ptr != kAsanStackRightRedzoneMagic))
+    shadow_ptr--;
+
+  return (uptr)shadow_ptr;
+}
+
 bool AsanThread::AddrIsInStack(uptr addr) {
   const auto bounds = GetStackBounds();
   return addr >= bounds.bottom && addr < bounds.top;
diff --git a/libsanitizer/asan/asan_thread.h b/libsanitizer/asan/asan_thread.h
index f7a91f3e73b..3195edb9f6f 100644
--- a/libsanitizer/asan/asan_thread.h
+++ b/libsanitizer/asan/asan_thread.h
@@ -88,6 +88,9 @@ class AsanThread {
   };
   bool GetStackFrameAccessByAddr(uptr addr, StackFrameAccess *access);
 
+  // Return beginning of a stack variable in shadow memory
+  uptr GetStackVariableShadowStart(uptr addr);
+
   bool AddrIsInStack(uptr addr);
 
   void DeleteFakeStack(int tid) {
-- 
2.14.3


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]