On 10/11/2017 04:22 PM, Jakub Jelinek wrote: > On Wed, Oct 11, 2017 at 03:36:40PM +0200, Martin Liška wrote: >>> 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. > > Ok. > >>> 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? > > First of all, why are you aligning anything? >> + uptr aligned_addr1 = addr1 & ~(SANITIZER_WORDSIZE/8 - 1); // align addr. >> + uptr aligned_addr2 = addr2 & ~(SANITIZER_WORDSIZE/8 - 1); // align addr. > You want to compare what the pointers point to, not what the aligned pointer > points to. > > Actually, looking around, there already is __sanitizer::mem_is_zero > which does what we want. > > Or even __asan_region_is_poisoned(addr1, addr2 - addr1).
Hi. Thanks, the function works fine. I added various tests for global variables and found that my check with GetGlobalAddressInformation was wrong Thus I'm adding bool GlobalAddressDescription::PointsInsideASameVariable. Another change I did is: gcc -fsanitize=pointer-compare /tmp/main.c cc1: error: ‘-fsanitize=pointer-compare’ must be combined with ‘-fsanitize=address’ or ‘-fsanitize=kernel-address’ Which would make life easier in gcc.c, where one would have to distinguish between -fsanitize=pointer-compare and fsanitize=pointer-compare,kernel-address and according to -lasan will be added. Would it be possible to require it explicitly? I'm adding some numbers for postgres: 1 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/backend/utils/mmgr/freepage.c:673 in FreePageBtreeCleanup 1 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/port/qsort_arg.c:168 in qsort_arg 2 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/backend/regex/rege_dfa.c:316 in matchuntil 3 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/backend/utils/mmgr/freepage.c:1543 in FreePageManagerPutInternal 4 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/backend/access/gin/gindatapage.c:155 in GinDataLeafPageGetItems 4 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/common/base64.c:160 in pg_b64_decode 4 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/common/keywords.c:103 in ScanKeywordLookup 14 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/backend/access/hash/hashovfl.c:768 in _hash_initbitmapbuffer 18 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/backend/utils/mmgr/freepage.c:187 in FreePageManagerInitialize 26 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/port/qsort.c:188 in pg_qsort 40 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/backend/utils/mmgr/dsa.c:1358 in init_span 43 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/backend/utils/mmgr/freepage.c:1388 in FreePageManagerGetInternal 87 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/pl/plpgsql/src/pl_scanner.c:666 in plpgsql_location_to_lineno 92 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/port/qsort.c:168 in pg_qsort 154 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/backend/storage/buffer/bufmgr.c:385 in ForgetPrivateRefCountEntry 273 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/backend/storage/ipc/shm_toc.c:182 in shm_toc_insert 1158 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/backend/regex/rege_dfa.c:153 in longest 3906 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/backend/nodes/tidbitmap.c:840 in tbm_prepare_shared_iterate 6545 SUMMARY: AddressSanitizer: invalid-pointer-pair /home/marxin/Programming/postgres/src/backend/utils/adt/formatting.c:4582 in NUM_numpart_to_char There are some comparisons with NULL: ==28245==ERROR: AddressSanitizer: invalid-pointer-pair: 0x625000e42139 0x000000000000 and quite some: Address 0x1465a178e5e0 is a wild pointer. Which is quite interesting reason, I'll investigate where a memory comes from. Thanks, Martin > > Jakub >
>From c83ea69668cc6c153024d648fb8ca565f8f16025 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. * ipa-inline.c (sanitize_attrs_match_for_inline_p): Add handling of SANITIZE_POINTER_COMPARE and SANITIZE_POINTER_SUBTRACT. 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 | 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 | 64 +++++++++++++ gcc/testsuite/gcc.dg/asan/pointer-compare-2.c | 47 ++++++++++ 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 | 27 ++++++ libsanitizer/asan/asan_descriptions.h | 5 ++ libsanitizer/asan/asan_report.cc | 58 ++++++++++-- libsanitizer/asan/asan_thread.cc | 23 +++++ libsanitizer/asan/asan_thread.h | 3 + 15 files changed, 461 insertions(+), 9 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..669adeb38b6 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -10955,6 +10955,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 5aa5d066dbe..36aa7056195 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..854f6853193 --- /dev/null +++ b/gcc/testsuite/gcc.dg/asan/pointer-compare-1.c @@ -0,0 +1,64 @@ +// { 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); + + /* 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..87ff8bd7fbb --- /dev/null +++ b/gcc/testsuite/gcc.dg/asan/pointer-compare-2.c @@ -0,0 +1,47 @@ +// { 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] = {}; +char small_global[7] = {}; + +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]); + bar(&small_global[0], &small_global[1]); + bar(&small_global[0], &small_global[7]); + bar(&small_global[7], &small_global[1]); + bar(&small_global[6], &small_global[7]); + bar(&small_global[7], &small_global[7]); + + /* 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..004cd6aca18 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,24 @@ void GlobalAddressDescription::Print(const char *bug_type) const { } } +bool GlobalAddressDescription::PointsInsideASameVariable + (const GlobalAddressDescription &other) const +{ + // we must have just a single variables candidate + if (size != 1 || other.size != 1) + return false; + + if (globals[0].beg != other.globals[0].beg) + return false; + + // check that both descriptions are inside a same global variable + return (globals[0].beg <= addr + && other.globals[0].beg <= other.addr + && (addr + access_size) <= (globals[0].beg + globals[0].size) + && ((other.addr + other.access_size) + <= (other.globals[0].beg + other.globals[0].size))); +} + 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..bf10e6944f1 100644 --- a/libsanitizer/asan/asan_report.cc +++ b/libsanitizer/asan/asan_report.cc @@ -344,14 +344,58 @@ 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; + 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; + } + } + + GlobalAddressDescription gdesc1, gdesc2; + valid1 = GetGlobalAddressInformation(a1, 0, &gdesc1); + valid2 = GetGlobalAddressInformation(a2, 0, &gdesc2); + + if (valid1 && valid2 + && gdesc1.PointsInsideASameVariable (gdesc2)) + return; + } + } + + 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