This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
On 10/13/2017 01:17 PM, Jakub Jelinek wrote:
> On Fri, Oct 13, 2017 at 01:01:37PM +0200, Martin Liška wrote:
>> @@ -3826,6 +3827,18 @@ 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 = c_fully_fold (op0, false, NULL);
>> + op1 = c_fully_fold (op1, false, NULL);
>> +
>> + tree tt = builtin_decl_explicit (BUILT_IN_ASAN_POINTER_SUBTRACT);
>> + *instrument_expr
>> + = build_call_expr_loc (loc, tt, 2, op0, op1);
>> + }
>
> If op0 or op1 have some embedded side-effects, won't you evaluate them
> multiple times? I'd expect you need to c_save_expr them and make sure the
> actual pointer difference expression uses the result of the save expr too.
Good point, fixed.
>
>> --- a/libsanitizer/asan/asan_report.cc
>> +++ b/libsanitizer/asan/asan_report.cc
>> @@ -344,14 +344,68 @@ 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 offset = a1 < a2 ? a2 - a1 : a1 - a2;
>> + uptr left = a1 < a2 ? a1 : a2;
>> + uptr right = a1 < a2 ? a2 : a1;
>> + if (offset <= 2048) {
>> + if (__asan_region_is_poisoned (left, offset) == 0)
>> + return;
>> + }
>
> If offset <= 2048 and something is poisoned in between, then it is
> clearly a failure, so you should either goto the error or duplicate
> the two lines inside of the outer if above.
Done that.
>
>> +
>> + uptr shadow_offset1, shadow_offset2;
>> + bool valid1, valid2;
>> + {
>> + ThreadRegistryLock l(&asanThreadRegistry());
>> + valid1 = GetStackVariableBeginning(left, &shadow_offset1);
>> }
>> +
>> + // check whether a1 is a stack memory pointer
>> + if (valid1) {
>> + {
>> + ThreadRegistryLock l(&asanThreadRegistry());
>> + valid2 = GetStackVariableBeginning(right - 1, &shadow_offset2);
>
> Why do you take the registery lock twice? Just do:
Yep, once is OK. However, we need to have the lock in a different scope than:
GET_CALLER_PC_BP_SP;
ReportInvalidPointerPair(pc, bp, sp, a1, a2);
Otherwise we'll reach a deadlock.
> {
> ThreadRegistryLock l(&asanThreadRegistry());
> if (GetStackVariableBeginning(left, &shadow_offset1))
> {
> if (GetStackVariableBeginning(right - 1, &shadow_offset2) &&
> shadow_offset1 == shadow_offset2)
> return;
> // goto do_error; or:
> GET_CALLER_PC_BP_SP;
> ReportInvalidPointerPair(pc, bp, sp, a1, a2);
> return;
> }
> }
>
>> + }
>> +
>> + if (valid2 && shadow_offset1 == shadow_offset2)
>> + return;
>> + }
>> + // check whether a1 is a heap memory address
>> + else {
>> + HeapAddressDescription hdesc1;
>> + valid1 = GetHeapAddressInformation(a1, 0, &hdesc1);
>> +
>> + if (valid1 && hdesc1.chunk_access.access_type == kAccessTypeInside) {
>> + HeapAddressDescription hdesc2;
>> + valid2 = GetHeapAddressInformation(a2, 0, &hdesc2);
>
> Again, no need for valid1/valid2. Why do you use above left and right - 1
> and here a1 and a2? Shouldn't it be always left and right - 1?
valid{1,2} removed. Now using a{1,2} just for error report purpose.
Martin
>
> Jakub
>
>From 68710d309cedf737ef333e82f33a8f2c9fd43c25 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 5 Oct 2017 12:14:25 +0200
Subject: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
gcc/ChangeLog:
2017-10-13 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-10-13 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/testsuite/ChangeLog:
2017-10-13 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/c/c-typeck.c | 34 ++++++++++--
gcc/doc/invoke.texi | 22 ++++++++
gcc/flag-types.h | 2 +
gcc/ipa-inline.c | 8 ++-
gcc/opts.c | 15 +++++
gcc/sanitizer.def | 4 ++
gcc/testsuite/gcc.dg/asan/pointer-compare-1.c | 77 ++++++++++++++++++++++++++
gcc/testsuite/gcc.dg/asan/pointer-compare-2.c | 76 +++++++++++++++++++++++++
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 | 29 ++++++++++
libsanitizer/asan/asan_descriptions.h | 5 ++
libsanitizer/asan/asan_report.cc | 59 +++++++++++++++++---
libsanitizer/asan/asan_thread.cc | 23 ++++++++
libsanitizer/asan/asan_thread.h | 3 +
15 files changed, 417 insertions(+), 13 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/c/c-typeck.c b/gcc/c/c-typeck.c
index cb9c589e061..ef0818ce994 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);
@@ -3779,10 +3779,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;
@@ -3826,6 +3827,19 @@ 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, c_fully_fold (op0, false, NULL),
+ c_fully_fold (op1, false, NULL));
+ }
+
/* First do the subtraction as integers;
then drop through to build the divide operator.
Do not do default conversions on the minus operator
@@ -11188,7 +11202,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. */
@@ -11707,6 +11721,18 @@ build_binary_op (location_t location, enum tree_code code,
pedwarn (location, 0,
"comparison of distinct pointer types lacks a cast");
}
+
+ if (sanitize_flags_p (SANITIZE_POINTER_COMPARE))
+ {
+ gcc_assert (current_function_decl != NULL_TREE);
+
+ 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);
+ }
}
else if (code0 == POINTER_TYPE && null_pointer_constant_p (orig_op1))
{
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4e7dfb33c31..6cec1fea033 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10960,6 +10960,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 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/ipa-inline.c b/gcc/ipa-inline.c
index dd46cb61362..d60432cded3 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 adf3d89851d..f39ead7e1cf 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 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..0e448cf2a3a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/pointer-compare-1.c
@@ -0,0 +1,77 @@
+// { 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] = {};
+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);
+
+ /* 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]);
+
+ /* 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..4967064634f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/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=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/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..934618c3181 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) {
@@ -330,6 +339,26 @@ void GlobalAddressDescription::Print(const char *bug_type) const {
}
}
+bool GlobalAddressDescription::PointsInsideASameVariable
+ (const GlobalAddressDescription &other) const
+{
+ if (size == 0 || other.size == 0)
+ return false;
+
+ for (unsigned i = 0; i < size; i++)
+ for (unsigned j = 0; j < other.size; j++) {
+ if (globals[i].beg == other.globals[j].beg
+ && globals[i].beg <= addr
+ && other.globals[j].beg <= other.addr
+ && (addr + access_size) < (globals[i].beg + globals[i].size)
+ && ((other.addr + other.access_size)
+ < (other.globals[j].beg + other.globals[j].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 584b9ba6491..35baec6f2f9 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;
@@ -149,6 +150,10 @@ struct GlobalAddressDescription {
u8 size;
void Print(const char *bug_type = "") const;
+
+ // Return true when this descriptions points inside a same global variable
+ // as other. Descriptions can have different address within the variable
+ bool PointsInsideASameVariable (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 84d67646b40..c062c422a15 100644
--- a/libsanitizer/asan/asan_report.cc
+++ b/libsanitizer/asan/asan_report.cc
@@ -344,14 +344,59 @@ 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 offset = a1 < a2 ? a2 - a1 : a1 - a2;
+ uptr left = a1 < a2 ? a1 : a2;
+ uptr right = a1 < a2 ? a2 : a1;
+ if (offset <= 2048) {
+ if (__asan_region_is_poisoned (left, offset) == 0)
+ return;
+ else
+ goto do_error;
+ }
+
+ {
+ uptr shadow_offset1, shadow_offset2;
+ ThreadRegistryLock l(&asanThreadRegistry());
+
+ // check whether left is a stack memory pointer
+ if (GetStackVariableBeginning(left, &shadow_offset1)) {
+ if (GetStackVariableBeginning(right - 1, &shadow_offset2)
+ && shadow_offset1 == shadow_offset2)
+ return;
+ else
+ goto do_error;
+ }
+ // check whether left is a heap memory address
+ else {
+ HeapAddressDescription hdesc1, hdesc2;
+ if (GetHeapAddressInformation(left, 0, &hdesc1)
+ && hdesc1.chunk_access.access_type == kAccessTypeInside) {
+ if (GetHeapAddressInformation(right, 0, &hdesc2)
+ && hdesc2.chunk_access.access_type == kAccessTypeInside
+ && (hdesc1.chunk_access.chunk_begin
+ == hdesc2.chunk_access.chunk_begin))
+ return;
+ } else {
+ // check whether left is an address of a global variable
+ GlobalAddressDescription gdesc1, gdesc2;
+ if (GetGlobalAddressInformation(left, 0, &gdesc1)) {
+ if (GetGlobalAddressInformation(right - 1, 0, &gdesc2)
+ && gdesc1.PointsInsideASameVariable (gdesc2))
+ return;
+ } else {
+ // TODO
+ }
+ }
+ }
}
+
+do_error:
+ GET_CALLER_PC_BP_SP;
+ 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 818e1261400..0ffca7af35e 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..edd1d815632 100644
--- a/libsanitizer/asan/asan_thread.h
+++ b/libsanitizer/asan/asan_thread.h
@@ -81,6 +81,9 @@ class AsanThread {
};
bool GetStackFrameAccessByAddr(uptr addr, StackFrameAccess *access);
+ // Return beginning of a stack variable in shadow memory
+ uptr GetStackFrameVariableBeginning(uptr addr);
+
bool AddrIsInStack(uptr addr);
void DeleteFakeStack(int tid) {
--
2.14.2
- References:
- [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.