On 10/12/2017 01:34 PM, Jakub Jelinek wrote: > On Thu, Oct 12, 2017 at 01:13:56PM +0200, Martin Liška wrote: >> + 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; >> + uptr left = a1 < a2 ? a1 : a2; >> + if (offset <= 2048) { >> + if (__asan_region_is_poisoned (left, offset) == 0) >> + return; >> + else { >> + GET_CALLER_PC_BP_SP; >> + ReportInvalidPointerPair(pc, bp, sp, a1, a2); >> + return; >> + } > > My assumption was that this offset/left stuff would be done > right after the if (a1 == a2) above, i.e. after the most common > case - equality comparison, handle the case of pointers close to each other > without any expensive locking and data structure lookup (also, you only need > to compute left if offset is <= 2048). Only for larger distances, you'd > go through the other cases. I.e. see if one or both pointers point > into a stack variable, or heap, or global registered variable. > > For those 3 cases, I wonder if pairs of valid1 = ...; valid2 = ...; > is what is most efficient. What we really want is to error out if > one pointer is valid in any of the categories, but the other is > not. So, isn't the best general algorithm something like: > if (a1 == a2) > return; > if (distance <= 2048) > { > if (not poisoned area) > return; > } > else if (heap (a1)) > { > if (heap (a2) && same_heap_object) > return; > } > else if (stackvar (a1)) > { > if (stackvar (a2) && same_stackvar) > return; > } > else if (globalvar (a1)) > { > if (globalvar (a2) && same_globalvar) > return; > } > else > return; /* ??? We don't know what the pointers point to, punt. > Or perhaps try the not poisoned area check even for > slightly larger distances (like 16K) and only punt > for larger? */ > error; > > ? What order of the a1 tests should be done depends on how expensive > those tests are and perhaps some data gathering on real-world apps > on what pointers in the comparisons/subtracts are most common. > > Jakub >
Thanks Jakub for valuable feedback. I'm sending new version where I implemented what you pointed. I also moved builtin created to FE and fixed quite some bugs I seen on postgres database. I guess I should slowly start with review process of libsanitizer changes. They are quite some. Martin
>From 28401b05e8da21e57c8c0388fcc71fcbf34e0002 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-13 Martin Liska <mli...@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 <mli...@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 <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/c/c-typeck.c | 31 +++++++++-- 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 | 72 ++++++++++++++++++++++++ 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 | 68 ++++++++++++++++++++--- libsanitizer/asan/asan_thread.cc | 23 ++++++++ libsanitizer/asan/asan_thread.h | 3 + 15 files changed, 419 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..e4dc2833c91 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,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); + } + /* 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 +11201,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 +11720,16 @@ 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); + + tree tt = builtin_decl_explicit (BUILT_IN_ASAN_POINTER_COMPARE); + instrument_expr + = build_call_expr_loc (location, tt, 2, unshare_expr (op0), + unshare_expr (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..c4829ad2b83 --- /dev/null +++ b/gcc/testsuite/gcc.dg/asan/pointer-compare-2.c @@ -0,0 +1,72 @@ +// { 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); + __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..2704f3c8095 100644 --- 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; + } + + 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); + } + + 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); + + if (valid2 + && hdesc2.chunk_access.access_type == kAccessTypeInside + && (hdesc1.chunk_access.chunk_begin + == hdesc2.chunk_access.chunk_begin)) + return; + } else { + // check whether a1 is an address of a global variable + GlobalAddressDescription gdesc1; + valid1 = GetGlobalAddressInformation(left, 0, &gdesc1); + + if (valid1) { + GlobalAddressDescription gdesc2; + valid2 = GetGlobalAddressInformation(right - 1, 0, &gdesc2); + + if (valid2 && gdesc1.PointsInsideASameVariable (gdesc2)) + return; + } else { + // TODO + } + } + } + + 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