On 10/06/2017 03:33 PM, Jakub Jelinek wrote:
> On Fri, Oct 06, 2017 at 02:46:05PM +0200, Martin Liška wrote:
>> + 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)))
>
Hello.
> Isn't it better to test is_pointer_compare_opcode right after
> is_gimple_assign and leave the gimple_assign_rhs_class test out?
Yes, it is :)
>
>> + {
>> + 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;
>> + }
>
> You don't handle the case when there is a COND_EXPR with pointer comparison
> in the condition.
Good point, fixed.
>
>> + }
>> +
>> + if (sanitize_subtraction_p
>> + && is_gimple_assign (s)
>> + && gimple_assign_rhs_class (s) == GIMPLE_BINARY_RHS
>
> The above isn't really needed.
>
>> + && 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))))
>
> Better add temporaries for rhs1/2 of def1, then you don't have issues with
> too long lines.
Yes.
>
>> --- 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.
>
> Poiinter-compare doesn't instrument -, so ", -" should be left out.
That's typo.
>
>> +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
>
> -fsanitize=pointer-subtract
>
>> +@opindex fsanitize=pointer-compare
>
> s/compare/subtract/
Likewise.
>
>> +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.
>
> Conceptually, these two instrumentations rely on address sanitization,
> not really sure if we should supporting them for kernel sanitization (but I
> bet it is just going to be too costly for kernel).
> So, we also need to make sure at least parts of SANITIZE_ADDRESS is enabled
> when these options are on.
> That can be done by erroring out if -fsanitize=pointer-compare is requested
> without -fsanitize=address, or by implicitly enabling -fsanitize=address for
> these, or by adding yet another SANITIZE_* bit which would cover
> sanitization of memory accesses for asan, that bit would be set by
> -fsanitize={address,kernel-address} in addition to the current 2 bits, but
> pointer-{subtract,compare} would set its own bit and SANITIZE_ADDRESS and
> SANITIZE_USER_ADDRESS only. Without the new bit we'd emit red zones,
> function prologue/epilogue asan changes, registraction of global variables,
> but not actual instrumentation of memory accesses (and probably not
> instrumentation of C++ ctor ordering).
Agree, would be much easier to just enable SANITIZE_ADDRESS with there 2
options.
Question is how to make it also possible with -fsanitize=kernel-address:
$ ./xgcc -B.
/home/marxin/Programming/gcc/gcc/testsuite/gcc.dg/asan/pointer-compare-2.c
-fsanitize=pointer-compare,kernel-address
cc1: error: ‘-fsanitize=address’ is incompatible with
‘-fsanitize=kernel-address’
Ideas?
>
>> --- 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
>
> Depending on the above, I wonder if sanitize(address) shouldn't be implied
> by the new -fsanitize=pointer-{subtract,compare}, rather than polluting
> specs for it everywhere.
Agree.
>
>
>> - if (flag_sanitize & SANITIZE_ADDRESS)
>> + if (flag_sanitize & SANITIZE_ADDRESS
>> + || flag_sanitize & SANITIZE_POINTER_COMPARE
>> + || flag_sanitize & SANITIZE_POINTER_SUBTRACT)
>
> Style nit, that would be better written as flag_sanitize & (A | B | C).
> But depending on the above it might be simpler.
Will not be needed as now we enable SANITIZE_ADDRESS.
>
> In any case, I'm not sure if we want this patch in GCC until the library
> side isn't in so sorry state, because as we've discussed in the past, it
> isn't really usable. At least my reading of it is that whenever you
> do one of these pointer comparisons or pointer subtractions and one or both
> of the pointers aren't heap allocated, you'll get a runtime error.
Fully agree, I will send libsanitizer bits to LLVM as soon as we're happy with
the shape of the patch.
Martin
>
> Jakub
>
>From e43a7424cacde7f3cb7d3c1a481f9642d9cc8d95 Mon Sep 17 00:00:00 2001
From: marxin <mli...@suse.cz>
Date: Thu, 5 Oct 2017 12:14:25 +0200
Subject: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
gcc/ChangeLog:
2017-10-06 Martin Liska <mli...@suse.cz>
* asan.c (is_pointer_compare_opcode): New function.
(instrument_pointer_comparison): Likewise.
(asan_instrument): Handle SANITIZE_POINTER_COMPARE and
SANITIZE_POINTER_SUBTRACT.
* doc/invoke.texi: Document the options.
* flag-types.h (enum sanitize_code): Add
SANITIZE_POINTER_COMPARE and SANITIZE_POINTER_SUBTRACT.
* opts.c: Define new sanitizer options.
* sanitizer.def (BUILT_IN_ASAN_POINTER_COMPARE):
(BUILT_IN_ASAN_POINTER_SUBTRACT): Likewise.
gcc/testsuite/ChangeLog:
2017-10-06 Martin Liska <mli...@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 | 119 +++++++++++++++++++++++++
gcc/doc/invoke.texi | 18 ++++
gcc/flag-types.h | 2 +
gcc/opts.c | 4 +
gcc/sanitizer.def | 4 +
gcc/testsuite/gcc.dg/asan/pointer-compare-1.c | 40 +++++++++
gcc/testsuite/gcc.dg/asan/pointer-compare-2.c | 41 +++++++++
gcc/testsuite/gcc.dg/asan/pointer-subtract-1.c | 41 +++++++++
gcc/testsuite/gcc.dg/asan/pointer-subtract-2.c | 32 +++++++
libsanitizer/asan/asan_descriptions.cc | 9 ++
libsanitizer/asan/asan_descriptions.h | 1 +
libsanitizer/asan/asan_globals.cc | 25 ++++++
libsanitizer/asan/asan_report.cc | 52 +++++++++--
libsanitizer/asan/asan_report.h | 1 +
libsanitizer/asan/asan_thread.cc | 23 +++++
libsanitizer/asan/asan_thread.h | 1 +
16 files changed, 406 insertions(+), 7 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
diff --git a/gcc/asan.c b/gcc/asan.c
index 2aa0a795af2..6bd437e0228 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -2370,6 +2370,122 @@ 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)
+ {
+ tree cond_expr, rhs1, rhs2, lhs, rhs;
+
+ if (is_gimple_assign (s)
+ && is_pointer_compare_opcode (gimple_assign_rhs_code (s))
+ && (rhs1 = gimple_assign_rhs1 (s))
+ && (rhs2 = gimple_assign_rhs2 (s))
+ && POINTER_TYPE_P (TREE_TYPE (rhs1))
+ && POINTER_TYPE_P (TREE_TYPE (rhs2)))
+ {
+ ptr1 = rhs1;
+ ptr2 = rhs2;
+ fn = BUILT_IN_ASAN_POINTER_COMPARE;
+ }
+ else if (is_gimple_assign (s)
+ && gimple_assign_rhs_code (s) == COND_EXPR
+ && (cond_expr = gimple_assign_rhs1 (s))
+ && is_pointer_compare_opcode (TREE_CODE (cond_expr))
+ && (rhs1 = TREE_OPERAND (cond_expr, 0))
+ && (rhs2 = TREE_OPERAND (cond_expr, 1))
+ && POINTER_TYPE_P (TREE_TYPE (rhs1))
+ && POINTER_TYPE_P (TREE_TYPE (rhs2)))
+ {
+ ptr1 = rhs1;
+ ptr2 = rhs2;
+ fn = BUILT_IN_ASAN_POINTER_COMPARE;
+ }
+ else if (gimple_code (s) == GIMPLE_COND
+ && (lhs = gimple_cond_lhs (s))
+ && (rhs = gimple_cond_rhs (s))
+ && POINTER_TYPE_P (TREE_TYPE (lhs))
+ && POINTER_TYPE_P (TREE_TYPE (rhs))
+ && is_pointer_compare_opcode (gimple_cond_code (s)))
+ {
+ ptr1 = rhs;
+ ptr2 = rhs;
+ 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 +3548,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;
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 9ad1fb339ba..e49aef5f6ba 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10955,6 +10955,24 @@ 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}.
+
+@item -fsanitize=pointer-subtract
+@opindex fsanitize=pointer-subtract
+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}.
+
@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/opts.c b/gcc/opts.c
index 5aa5d066dbe..f544484cf26 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1496,6 +1496,10 @@ 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 | SANITIZE_ADDRESS
+ | SANITIZE_USER_ADDRESS), true),
+ SANITIZER_OPT (pointer-subtract, (SANITIZE_POINTER_SUBTRACT | SANITIZE_ADDRESS
+ | SANITIZE_USER_ADDRESS), 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..8b842c5567e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/pointer-compare-1.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=0" }
+// { dg-options "-fsanitize=pointer-compare -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. */
+ // { 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]);
+ return 1;
+}
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..0e0d3589a30
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/pointer-compare-2.c
@@ -0,0 +1,41 @@
+// { dg-do run }
+// { dg-set-target-env-var ASAN_OPTIONS "detect_invalid_pointer_pairs=1 halt_on_error=1" }
+// { dg-options "-fsanitize=pointer-compare -O0" }
+
+int foo(char *p)
+{
+ char *p2 = p + 20;
+ return p > p2;
+}
+
+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 = foo(p);
+ __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[9000]);
+ bar(&global[500], &global[10]);
+
+ /* 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/gcc.dg/asan/pointer-subtract-1.c b/gcc/testsuite/gcc.dg/asan/pointer-subtract-1.c
new file mode 100644
index 00000000000..10792264f2e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/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=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. */
+ // { 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]);
+ return 1;
+}
+
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..17cf0e6711d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/pointer-subtract-2.c
@@ -0,0 +1,32 @@
+// { dg-do run }
+// { dg-set-target-env-var ASAN_OPTIONS "detect_invalid_pointer_pairs=1 halt_on_error=1" }
+// { dg-options "-fsanitize=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]);
+
+ /* 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/libsanitizer/asan/asan_descriptions.cc b/libsanitizer/asan/asan_descriptions.cc
index 35d1619f2d9..eaa04378c2c 100644
--- a/libsanitizer/asan/asan_descriptions.cc
+++ b/libsanitizer/asan/asan_descriptions.cc
@@ -220,6 +220,15 @@ bool GetStackAddressInformation(uptr addr, uptr access_size,
return true;
}
+bool GetStackVariableBeginning(uptr addr, uptr *shadow_addr)
+{
+ AsanThread *t = FindThreadByStackAddress(addr);
+ if (!t) return false;
+
+ *shadow_addr = t->GetStackFrameVariableBeginning (addr);
+ return true;
+}
+
static void PrintAccessAndVarIntersection(const StackVarDescr &var, uptr addr,
uptr access_size, uptr prev_var_end,
uptr next_var_beg) {
diff --git a/libsanitizer/asan/asan_descriptions.h b/libsanitizer/asan/asan_descriptions.h
index 584b9ba6491..b7f23b1a71b 100644
--- a/libsanitizer/asan/asan_descriptions.h
+++ b/libsanitizer/asan/asan_descriptions.h
@@ -138,6 +138,7 @@ struct StackAddressDescription {
bool GetStackAddressInformation(uptr addr, uptr access_size,
StackAddressDescription *descr);
+bool GetStackVariableBeginning(uptr addr, uptr *shadow_addr);
struct GlobalAddressDescription {
uptr addr;
diff --git a/libsanitizer/asan/asan_globals.cc b/libsanitizer/asan/asan_globals.cc
index f2292926e6a..ed707c0ca01 100644
--- a/libsanitizer/asan/asan_globals.cc
+++ b/libsanitizer/asan/asan_globals.cc
@@ -122,6 +122,31 @@ int GetGlobalsForAddress(uptr addr, Global *globals, u32 *reg_sites,
return res;
}
+bool AreGlobalVariablesSame(uptr addr1, uptr addr2)
+{
+ if (addr1 > addr2)
+ {
+ uptr tmp = addr1;
+ addr1 = addr2;
+ addr2 = tmp;
+ }
+
+ BlockingMutexLock lock(&mu_for_globals);
+
+ uptr aligned_addr1 = addr1 & ~(SANITIZER_WORDSIZE/8 - 1); // align addr.
+ uptr aligned_addr2 = addr2 & ~(SANITIZER_WORDSIZE/8 - 1); // align addr.
+
+ u8 *shadow_ptr1 = (u8*)MemToShadow(aligned_addr1);
+ u8 *shadow_ptr2 = (u8*)MemToShadow(aligned_addr2);
+
+ while (shadow_ptr1 <= shadow_ptr2
+ && *shadow_ptr1 != kAsanGlobalRedzoneMagic) {
+ shadow_ptr1++;
+ }
+
+ return shadow_ptr1 == shadow_ptr2;
+}
+
enum GlobalSymbolState {
UNREGISTERED = 0,
REGISTERED = 1
diff --git a/libsanitizer/asan/asan_report.cc b/libsanitizer/asan/asan_report.cc
index 84d67646b40..44fe95e163d 100644
--- a/libsanitizer/asan/asan_report.cc
+++ b/libsanitizer/asan/asan_report.cc
@@ -344,14 +344,52 @@ 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)) {
- GET_CALLER_PC_BP_SP;
- return ReportInvalidPointerPair(pc, bp, sp, a1, a2);
+
+ if (a1 == a2)
+ return;
+
+ uptr shadow_offset1, shadow_offset2;
+ bool valid1, valid2;
+ {
+ ThreadRegistryLock l(&asanThreadRegistry());
+
+ valid1 = GetStackVariableBeginning(a1, &shadow_offset1);
+ valid2 = GetStackVariableBeginning(a2, &shadow_offset2);
+ }
+
+ if (valid1 && valid2) {
+ if (shadow_offset1 == shadow_offset2)
+ return;
}
+ else if (!valid1 && !valid2) {
+ AsanChunkView chunk1 = FindHeapChunkByAddress(a1);
+ AsanChunkView chunk2 = FindHeapChunkByAddress(a2);
+ valid1 = chunk1.IsAllocated();
+ valid2 = chunk2.IsAllocated();
+
+ if (valid1 && valid2) {
+ if (chunk1.Eq(chunk2))
+ return;
+ }
+ else if (!valid1 && !valid2) {
+ uptr offset = a1 < a2 ? a2 - a1 : a1 - a2;
+ if (offset <= 2048) {
+ if (AreGlobalVariablesSame (a1, a2))
+ return;
+ }
+
+ GlobalAddressDescription gdesc1, gdesc2;
+ valid1 = GetGlobalAddressInformation(a1, 1, &gdesc1);
+ valid2 = GetGlobalAddressInformation(a2, 1, &gdesc2);
+
+ if (valid1 && valid2
+ && gdesc1.globals[0].beg == gdesc2.globals[0].beg)
+ return;
+ }
+ }
+
+ GET_CALLER_PC_BP_SP;
+ ReportInvalidPointerPair(pc, bp, sp, a1, a2);
}
// ----------------------- Mac-specific reports ----------------- {{{1
diff --git a/libsanitizer/asan/asan_report.h b/libsanitizer/asan/asan_report.h
index 111b8400153..2b4c9d29fda 100644
--- a/libsanitizer/asan/asan_report.h
+++ b/libsanitizer/asan/asan_report.h
@@ -27,6 +27,7 @@ struct StackVarDescr {
// them to "globals" array.
int GetGlobalsForAddress(uptr addr, __asan_global *globals, u32 *reg_sites,
int max_globals);
+bool AreGlobalVariablesSame(uptr addr1, uptr addr2);
const char *MaybeDemangleGlobalName(const char *name);
void PrintGlobalNameIfASCII(InternalScopedString *str, const __asan_global &g);
diff --git a/libsanitizer/asan/asan_thread.cc b/libsanitizer/asan/asan_thread.cc
index 818e1261400..d6bd051a493 100644
--- a/libsanitizer/asan/asan_thread.cc
+++ b/libsanitizer/asan/asan_thread.cc
@@ -322,6 +322,29 @@ bool AsanThread::GetStackFrameAccessByAddr(uptr addr,
return true;
}
+uptr AsanThread::GetStackFrameVariableBeginning(uptr addr)
+{
+ uptr bottom = 0;
+ if (AddrIsInStack(addr)) {
+ bottom = stack_bottom();
+ } else if (has_fake_stack()) {
+ bottom = fake_stack()->AddrIsInFakeStack(addr);
+ CHECK(bottom);
+ }
+ uptr aligned_addr = addr & ~(SANITIZER_WORDSIZE/8 - 1); // 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 c51a58ad0bb..c5adecacad4 100644
--- a/libsanitizer/asan/asan_thread.h
+++ b/libsanitizer/asan/asan_thread.h
@@ -80,6 +80,7 @@ class AsanThread {
const char *frame_descr;
};
bool GetStackFrameAccessByAddr(uptr addr, StackFrameAccess *access);
+ uptr GetStackFrameVariableBeginning(uptr addr);
bool AddrIsInStack(uptr addr);
--
2.14.2
