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 <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 | 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