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}.
- From: Martin Liška <mliska at suse dot cz>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Wed, 11 Oct 2017 15:36:40 +0200
- Subject: Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Authentication-results: sourceware.org; auth=none
- References: <6113595f-93a0-7400-0800-496ceadbfb2a@suse.cz> <20171006133354.GQ18588@tucnak> <5365c5ef-7ec8-4485-e9ac-ed4dd1302902@suse.cz> <20171011073712.GI14653@tucnak>
On 10/11/2017 09:37 AM, Jakub Jelinek wrote:
> On Wed, Oct 11, 2017 at 07:55:44AM +0200, Martin Liška wrote:
>>> 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?
>
Hello.
Thanks for feedback.
> If we want to make it usable for both user and kernel address, then either
> we'll let pointer-compare/pointer-subtract implicitly enable user address,
> unless kernel-address has been enabled (that would mean set SANITIZE_ADDRESS
> bit in pointer-*, but not SANITIZE_USER_ADDRESS, and at the point where we
> diagnose option incompatibilities like -fsanitize=address,kernel-address
> check for the case (SANITIZE_ADDRESS bit set, none of SANITIZE_USER_ADDRESS
> nor SANITIZE_KERNEL_ADDRESS, and one of SANITIZE_POINTER_*) and set
> implicitly SANITIZE_USER_ADDRESS, or simply require that the user chooses,
> by erroring out if pointer-* is used without explicit address or
> kernel-address. In any case, I think this should be also something
> discussed with the upstream sanitizer folks, so that LLVM (if it ever
> decides to actually implement it) behaves compatibly.
I've added support for automatic adding of -sanitize=address if none of them is added.
Problem is that LIBASAN_SPEC is still handled in driver. Thus I guess I'll need the hunks
I sent in first version of patch. Or do I miss something?
>
>> --- 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;
>
> std::swap(addr1, addr2); ? I don't see it used in any of libsanitizer
> though, so not sure if the corresponding STL header is included.
They don't use it anywhere and I had some #include issues. That's why I did it manually.
>
>> + }
>> +
>> + BlockingMutexLock lock(&mu_for_globals);
>
> Why do you need a mutex for checking if there are no red zones in between?
>
>> + 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++;
>> + }
>
> There are many kinds of shadow memory markings. My thought was that it
> would start with a quick check, perhaps vectorized by hand (depending on if
> the arch has unaligned loads maybe without or with a short loop for
Did that, but I have no experience how to make decision about prologue that will
align the pointer? Any examples?
> alignment) where say unsigned long (perhaps may_alias?) pointer would be
> used to read 4/8 shadow bytes at a time, and just check if any of them is
> non-zero. And, if it found a non-zero byte, deal with it after the loop,
> if after the "vectorized" loop, find which byte it was, and in any case
> deal e.g. with the various special cases like when the shadow byte is 1-7
> and stands for how many bytes are accessible, etc.
Yes, can help significantly I guess.
Martin
>
>> --- 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;
>
> My thought was that you'd do the difference <= 2048 test first
> without any lock, and only for the larger stuff take locks and look stuff
> up.
>
>> +
>> + 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
>>
>
>
> Jakub
>
>From 6bcf3d0064057edf55f0f65f7899b8a4a5e7e7dc 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-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.
* 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 <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 | 119 +++++++++++++++++++++++++
gcc/doc/invoke.texi | 18 ++++
gcc/flag-types.h | 2 +
gcc/opts.c | 10 +++
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 | 29 ++++++
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, 416 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..8dee813b369 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -952,6 +952,12 @@ 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_POINTER_COMPARE | SANITIZE_POINTER_SUBTRACT))
+ && (opts->x_flag_sanitize
+ & (SANITIZE_USER_ADDRESS | SANITIZE_KERNEL_ADDRESS)) == 0)
+ opts->x_flag_sanitize |= SANITIZE_USER_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 +1502,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),
+ true),
+ SANITIZER_OPT (pointer-subtract, (SANITIZE_POINTER_SUBTRACT
+ | SANITIZE_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..9cf195adbfa 100644
--- a/libsanitizer/asan/asan_globals.cc
+++ b/libsanitizer/asan/asan_globals.cc
@@ -122,6 +122,35 @@ int GetGlobalsForAddress(uptr addr, Global *globals, u32 *reg_sites,
return res;
}
+bool AreGlobalVariablesSameFast(uptr addr1, uptr addr2) {
+ if (addr1 > addr2) {
+ uptr tmp = addr1;
+ addr1 = addr2;
+ addr2 = tmp;
+ }
+
+ 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);
+
+ // Skip fast all zero blocks.
+ unsigned int * __attribute__((may_alias)) fast1 = (unsigned int *)shadow_ptr1;
+ unsigned int * __attribute__((may_alias)) fast2 = (unsigned int *)shadow_ptr2;
+ while ((fast1 + 1) <= fast2 && *fast1 == 0)
+ fast1++;
+
+ // Check byte by byte
+ shadow_ptr1 = (u8 *)fast1;
+ 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..bcfad40bc3a 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 (AreGlobalVariablesSameFast (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..8b0a8275935 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 AreGlobalVariablesSameFast(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