On 12/05/2017 10:27 AM, Jakub Jelinek wrote: > On Tue, Nov 21, 2017 at 12:59:46PM +0100, Martin Liška wrote: >> On 10/16/2017 10:39 PM, Martin Liška wrote: >>> Hi. >>> >>> All nits included in mainline review request I've just done: >>> https://reviews.llvm.org/D38971 >>> >>> Martin >> >> Hi. >> >> There's updated version of patch where I added new test-cases and it's >> rebased >> with latest version of libsanitizer changes. This is subject for >> libsanitizer review process. > > A slightly modified version has been finally accepted upstream, here is the > updated patch with that final version cherry-picked, plus small changes to > make it apply after the POINTER_DIFF_EXPR changes, and a lot of testsuite > tweaks, so that we don't run it 7 times with -O0, but with different > optimization levels, cleanups etc.
Hi Jakub. Thanks for finalizing the patch review and for the clean up you've done. > The most important change I've done in the testsuite was pointer-subtract-2.c > used -fsanitize=address,pointer-subtract, but the function was actually > doing pointer comparison. Guess that needs to be propagated upstream at > some point. Another thing is that in pointer-compare-1.c where you test > p - 1, p and p, p - 1 on the global variables, we need to ensure there is > some other array before it, otherwise we run into the issue that there is no > red zone before the first global (and when optimizing, global objects seems > to be sorted by decreasing size). I'll add there minor issues to my TODO list and I'll create mainline patch review. Martin > > Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk. > > 2017-12-05 Martin Liska <mli...@suse.cz> > Jakub Jelinek <ja...@redhat.com> > > gcc/ > * 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/ > * c-typeck.c (pointer_diff): Add new argument and instrument > pointer subtraction. > (build_binary_op): Similar for pointer comparison. > gcc/cp/ > * typeck.c (pointer_diff): Add new argument and instrument > pointer subtraction. > (cp_build_binary_op): Create compound expression if doing an > instrumentation. > gcc/testsuite/ > * c-c++-common/asan/pointer-compare-1.c: New test. > * c-c++-common/asan/pointer-compare-2.c: New test. > * c-c++-common/asan/pointer-subtract-1.c: New test. > * c-c++-common/asan/pointer-subtract-2.c: New test. > * c-c++-common/asan/pointer-subtract-3.c: New test. > * c-c++-common/asan/pointer-subtract-4.c: New test. > libsanitizer/ > * asan/asan_descriptions.cc: Cherry-pick upstream r319668. > * asan/asan_descriptions.h: Likewise. > * asan/asan_report.cc: Likewise. > * asan/asan_thread.cc: Likewise. > * asan/asan_thread.h: Likewise. > > --- gcc/c/c-typeck.c.jj 2017-12-01 19:42:09.504222186 +0100 > +++ gcc/c/c-typeck.c 2017-12-04 22:41:50.680290866 +0100 > @@ -95,7 +95,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); > @@ -3768,10 +3768,11 @@ parser_build_binary_op (location_t locat > } > > /* Return a tree for the difference of pointers OP0 and OP1. > - The resulting tree has type ptrdiff_t. */ > + The resulting tree has type ptrdiff_t. 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; > @@ -3815,6 +3816,17 @@ pointer_diff (location_t loc, tree op0, > pedwarn (loc, OPT_Wpointer_arith, > "pointer to a function used in subtraction"); > > + if (sanitize_flags_p (SANITIZE_POINTER_SUBTRACT)) > + { > + gcc_assert (current_function_decl != NULL_TREE); > + > + op0 = save_expr (op0); > + op1 = save_expr (op1); > + > + tree tt = builtin_decl_explicit (BUILT_IN_ASAN_POINTER_SUBTRACT); > + *instrument_expr = build_call_expr_loc (loc, tt, 2, op0, op1); > + } > + > /* First do the subtraction, then build the divide operator > and only convert at the very end. > Do not do default conversions in case restype is a short type. */ > @@ -3825,8 +3837,8 @@ pointer_diff (location_t loc, tree op0, > space, cast the pointers to some larger integer type and do the > computations in that type. */ > if (TYPE_PRECISION (inttype) > TYPE_PRECISION (TREE_TYPE (op0))) > - op0 = build_binary_op (loc, MINUS_EXPR, convert (inttype, op0), > - convert (inttype, op1), false); > + op0 = build_binary_op (loc, MINUS_EXPR, convert (inttype, op0), > + convert (inttype, op1), false); > else > op0 = build2_loc (loc, POINTER_DIFF_EXPR, inttype, op0, op1); > > @@ -11113,7 +11125,7 @@ build_binary_op (location_t location, en > 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. */ > @@ -11663,6 +11675,17 @@ build_binary_op (location_t location, en > result_type = type1; > pedwarn (location, 0, "comparison between pointer and integer"); > } > + > + if ((code0 == POINTER_TYPE || code1 == POINTER_TYPE) > + && sanitize_flags_p (SANITIZE_POINTER_COMPARE)) > + { > + op0 = save_expr (op0); > + op1 = save_expr (op1); > + > + tree tt = builtin_decl_explicit (BUILT_IN_ASAN_POINTER_COMPARE); > + instrument_expr = build_call_expr_loc (location, tt, 2, op0, op1); > + } > + > if ((TREE_CODE (TREE_TYPE (orig_op0)) == BOOLEAN_TYPE > || truth_value_p (TREE_CODE (orig_op0))) > ^ (TREE_CODE (TREE_TYPE (orig_op1)) == BOOLEAN_TYPE > --- gcc/cp/typeck.c.jj 2017-11-28 18:16:13.663825436 +0100 > +++ gcc/cp/typeck.c 2017-12-04 22:43:29.597071458 +0100 > @@ -54,7 +54,7 @@ static tree rationalize_conditional_expr > static int comp_ptr_ttypes_real (tree, tree, int); > static bool comp_except_types (tree, tree, bool); > static bool comp_array_types (const_tree, const_tree, bool); > -static tree pointer_diff (location_t, tree, tree, tree, tsubst_flags_t); > +static tree pointer_diff (location_t, tree, tree, tree, tsubst_flags_t, tree > *); > static tree get_delta_difference (tree, tree, bool, bool, tsubst_flags_t); > static void casts_away_constness_r (tree *, tree *, tsubst_flags_t); > static bool casts_away_constness (tree, tree, tsubst_flags_t); > @@ -4329,8 +4329,16 @@ cp_build_binary_op (location_t location, > if (code0 == POINTER_TYPE && code1 == POINTER_TYPE > && same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (type0), > TREE_TYPE (type1))) > - return pointer_diff (location, op0, op1, > - common_pointer_type (type0, type1), complain); > + { > + result = pointer_diff (location, op0, op1, > + common_pointer_type (type0, type1), complain, > + &instrument_expr); > + if (instrument_expr != NULL) > + result = build2 (COMPOUND_EXPR, TREE_TYPE (result), > + instrument_expr, result); > + > + return result; > + } > /* In all other cases except pointer - int, the usual arithmetic > rules apply. */ > else if (!(code0 == POINTER_TYPE && code1 == INTEGER_TYPE)) > @@ -5019,6 +5027,17 @@ cp_build_binary_op (location_t location, > else > return error_mark_node; > } > + > + if ((code0 == POINTER_TYPE || code1 == POINTER_TYPE) > + && sanitize_flags_p (SANITIZE_POINTER_COMPARE)) > + { > + op0 = save_expr (op0); > + op1 = save_expr (op1); > + > + tree tt = builtin_decl_explicit (BUILT_IN_ASAN_POINTER_COMPARE); > + instrument_expr = build_call_expr_loc (location, tt, 2, op0, op1); > + } > + > break; > > case UNORDERED_EXPR: > @@ -5374,11 +5393,12 @@ cp_pointer_int_sum (location_t loc, enum > } > > /* 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, tree ptrtype, > - tsubst_flags_t complain) > + tsubst_flags_t complain, tree *instrument_expr) > { > tree result, inttype; > tree restype = ptrdiff_type_node; > @@ -5420,6 +5440,15 @@ pointer_diff (location_t loc, tree op0, > else > inttype = restype; > > + if (sanitize_flags_p (SANITIZE_POINTER_SUBTRACT)) > + { > + op0 = save_expr (op0); > + op1 = save_expr (op1); > + > + tree tt = builtin_decl_explicit (BUILT_IN_ASAN_POINTER_SUBTRACT); > + *instrument_expr = build_call_expr_loc (loc, tt, 2, op0, op1); > + } > + > /* First do the subtraction, then build the divide operator > and only convert at the very end. > Do not do default conversions in case restype is a short type. */ > --- gcc/doc/invoke.texi.jj 2017-12-04 20:10:29.232029972 +0100 > +++ gcc/doc/invoke.texi 2017-12-04 22:38:28.151787561 +0100 > @@ -11034,6 +11034,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. > --- gcc/flag-types.h.jj 2017-10-31 17:54:22.979383176 +0100 > +++ gcc/flag-types.h 2017-12-04 22:38:28.151787561 +0100 > @@ -246,6 +246,8 @@ enum sanitize_code { > SANITIZE_BOUNDS_STRICT = 1UL << 23, > SANITIZE_POINTER_OVERFLOW = 1UL << 24, > SANITIZE_BUILTIN = 1UL << 25, > + SANITIZE_POINTER_COMPARE = 1UL << 26, > + SANITIZE_POINTER_SUBTRACT = 1UL << 27, > SANITIZE_SHIFT = SANITIZE_SHIFT_BASE | SANITIZE_SHIFT_EXPONENT, > SANITIZE_UNDEFINED = SANITIZE_SHIFT | SANITIZE_DIVIDE | > SANITIZE_UNREACHABLE > | SANITIZE_VLA | SANITIZE_NULL | SANITIZE_RETURN > --- gcc/ipa-inline.c.jj 2017-11-24 20:30:42.832102733 +0100 > +++ gcc/ipa-inline.c 2017-12-04 22:38:28.152787548 +0100 > @@ -260,8 +260,12 @@ sanitize_attrs_match_for_inline_p (const > 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 > --- gcc/opts.c.jj 2017-11-21 23:17:33.245214916 +0100 > +++ gcc/opts.c 2017-12-04 22:38:28.153787536 +0100 > @@ -953,6 +953,19 @@ finish_options (struct gcc_options *opts > 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)) > @@ -1497,6 +1510,8 @@ const struct sanitizer_opts_s sanitizer_ > 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), > --- gcc/sanitizer.def.jj 2017-11-21 23:17:34.019205449 +0100 > +++ gcc/sanitizer.def 2017-12-04 22:38:28.153787536 +0100 > @@ -175,6 +175,10 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_ALLO > 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", > --- gcc/testsuite/c-c++-common/asan/pointer-compare-1.c.jj 2017-12-04 > 22:38:28.154787524 +0100 > +++ gcc/testsuite/c-c++-common/asan/pointer-compare-1.c 2017-12-05 > 10:09:02.459604949 +0100 > @@ -0,0 +1,95 @@ > +/* { dg-do run } */ > +/* { dg-set-target-env-var ASAN_OPTIONS > "detect_invalid_pointer_pairs=1:halt_on_error=0" } */ > +/* { dg-options "-fsanitize=address,pointer-compare" } */ > + > +volatile int v; > + > +__attribute__((noipa)) void > +foo (char *p, char *q) > +{ > + v = p > q; > +} > + > +char global1[100] = {}, global2[100] = {}; > +char __attribute__((used)) smallest_global[5] = {}; > +char small_global[7] = {}; > +char __attribute__((used)) little_global[10] = {}; > +char __attribute__((used)) medium_global[4000] = {}; > +char large_global[5000] = {}; > +char __attribute__((used)) largest_global[6000] = {}; > + > +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); > + __asm ("" : "+g" (heap1)); > + /* { 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); > + __asm ("" : "+g" (heap1)); > + /* { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" } */ > + foo (heap1, heap1 + 4097); > + /* { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" } */ > + foo (heap1, 0); > + > + /* Global variables. */ > + /* { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" } */ > + foo (&global1[0], &global2[10]); > + > + char *p = &small_global[0]; > + __asm ("" : "+g" (p)); > + 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]; > + __asm ("" : "+g" (p)); > + /* { 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]); > + /* { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" } */ > + foo (p, 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]); > + /* { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair" } */ > + foo (&stack1, 0); > + __builtin_free (heap1); > + > + return 0; > +} > --- gcc/testsuite/c-c++-common/asan/pointer-compare-2.c.jj 2017-12-04 > 22:38:28.154787524 +0100 > +++ gcc/testsuite/c-c++-common/asan/pointer-compare-2.c 2017-12-05 > 09:30:24.709845766 +0100 > @@ -0,0 +1,82 @@ > +/* { dg-do run } */ > +/* { dg-set-target-env-var ASAN_OPTIONS "detect_invalid_pointer_pairs=1 > halt_on_error=1" } */ > +/* { dg-options "-fsanitize=address,pointer-compare" } */ > + > +volatile int v; > + > +int > +foo (char *p) > +{ > + char *p2 = p + 20; > + v = p > p2; > + return v; > +} > + > +void > +bar (char *p, char *q) > +{ > + v = p <= q; > +} > + > +void > +baz (char *p, char *q) > +{ > + v = (p != 0 && p < q); > +} > + > +char global[8192] = {}; > +char small_global[7] = {}; > + > +int > +main () > +{ > + /* Heap allocated memory. */ > + char *p = (char *)__builtin_malloc (42); > + int r = foo (p); > + __builtin_free (p); > + > + p = (char *)__builtin_malloc (1024); > + bar (p, p + 1024); > + bar (p + 1024, p + 1023); > + bar (p + 1, p + 1023); > + __builtin_free (p); > + > + p = (char *)__builtin_malloc (4096); > + bar (p, p + 4096); > + bar (p + 10, p + 100); > + bar (p + 1024, p + 4096); > + bar (p + 4095, p + 4096); > + bar (p + 4095, p + 4094); > + bar (p + 100, p + 4096); > + bar (p + 100, p + 4094); > + __builtin_free (p); > + > + /* Global variable. */ > + bar (&global[0], &global[1]); > + bar (&global[1], &global[2]); > + bar (&global[2], &global[1]); > + bar (&global[0], &global[100]); > + bar (&global[1000], &global[7000]); > + bar (&global[500], &global[10]); > + p = &global[0]; > + bar (p, p + 8192); > + p = &global[8000]; > + bar (p, p + 192); > + > + p = &small_global[0]; > + bar (p, p + 1); > + bar (p, p + 7); > + bar (p + 7, p + 1); > + bar (p + 6, p + 7); > + bar (p + 7, p + 7); > + > + /* Stack variable. */ > + char stack[10000]; > + bar (&stack[0], &stack[100]); > + bar (&stack[1000], &stack[9000]); > + bar (&stack[500], &stack[10]); > + > + baz (0, &stack[10]); > + > + return 0; > +} > --- gcc/testsuite/c-c++-common/asan/pointer-subtract-1.c.jj 2017-12-04 > 22:38:28.154787524 +0100 > +++ gcc/testsuite/c-c++-common/asan/pointer-subtract-1.c 2017-12-05 > 09:52:10.492373364 +0100 > @@ -0,0 +1,45 @@ > +/* { dg-do run } */ > +/* { dg-set-target-env-var ASAN_OPTIONS "detect_invalid_pointer_pairs=1 > halt_on_error=0" } */ > +/* { dg-options "-fsanitize=address,pointer-subtract" } */ > + > +volatile __PTRDIFF_TYPE__ v; > + > +__attribute__((noipa)) void > +foo (char *p, char *q) > +{ > + v = 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. */ > + char stack1, stack2; > + /* { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair.*" } */ > + 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]); > + > + __builtin_free (heap1); > + __builtin_free (heap2); > + return 0; > +} > --- gcc/testsuite/c-c++-common/asan/pointer-subtract-2.c.jj 2017-12-04 > 22:38:28.154787524 +0100 > +++ gcc/testsuite/c-c++-common/asan/pointer-subtract-2.c 2017-12-05 > 09:48:55.876826988 +0100 > @@ -0,0 +1,37 @@ > +/* { dg-do run } */ > +/* { dg-set-target-env-var ASAN_OPTIONS "detect_invalid_pointer_pairs=1 > halt_on_error=1" } */ > +/* { dg-options "-fsanitize=address,pointer-subtract" } */ > + > +volatile __PTRDIFF_TYPE__ v; > + > +void > +bar (char *p, char *q) > +{ > + v = q - p; > + v = p - q; > +} > + > +char global[10000] = {}; > + > +int > +main () > +{ > + /* Heap allocated memory. */ > + char *p = (char *)__builtin_malloc (42); > + bar (p, p + 20); > + __builtin_free (p); > + > + /* Global variable. */ > + bar (&global[0], &global[100]); > + bar (&global[1000], &global[9000]); > + bar (&global[500], &global[10]); > + bar (&global[0], &global[10000]); > + > + /* Stack variable. */ > + char stack[10000]; > + bar (&stack[0], &stack[100]); > + bar (&stack[1000], &stack[9000]); > + bar (&stack[500], &stack[10]); > + > + return 0; > +} > --- gcc/testsuite/c-c++-common/asan/pointer-subtract-3.c.jj 2017-12-04 > 22:38:28.155787511 +0100 > +++ gcc/testsuite/c-c++-common/asan/pointer-subtract-3.c 2017-12-05 > 09:36:28.764246245 +0100 > @@ -0,0 +1,43 @@ > +/* { dg-do run { target pthread_h } } */ > +/* { dg-set-target-env-var ASAN_OPTIONS > "detect_invalid_pointer_pairs=1:halt_on_error=1" } */ > +/* { dg-options "-fsanitize=address,pointer-subtract" } */ > +/* { dg-additional-options "-pthread" { target pthread } } */ > + > +#include <unistd.h> > +#include <pthread.h> > + > +char *pointers[2]; > +pthread_barrier_t bar; > + > +void * > +thread_main (void *n) > +{ > + char local; > + > + __UINTPTR_TYPE__ id = (__UINTPTR_TYPE__) n; > + pointers[id] = &local; > + pthread_barrier_wait (&bar); > + pthread_barrier_wait (&bar); > + > + return 0; > +} > + > +int > +main () > +{ > + pthread_t threads[2]; > + pthread_barrier_init (&bar, NULL, 3); > + pthread_create (&threads[0], NULL, thread_main, (void *) 0); > + pthread_create (&threads[1], NULL, thread_main, (void *) 1); > + pthread_barrier_wait (&bar); > + > + /* This case is not handled yet. */ > + volatile __PTRDIFF_TYPE__ r = pointers[0] - pointers[1]; > + > + pthread_barrier_wait (&bar); > + pthread_join (threads[0], NULL); > + pthread_join (threads[1], NULL); > + pthread_barrier_destroy (&bar); > + > + return 0; > +} > --- gcc/testsuite/c-c++-common/asan/pointer-subtract-4.c.jj 2017-12-04 > 22:38:28.155787511 +0100 > +++ gcc/testsuite/c-c++-common/asan/pointer-subtract-4.c 2017-12-05 > 09:37:05.287785773 +0100 > @@ -0,0 +1,43 @@ > +/* { dg-do run { target pthread_h } } */ > +/* { dg-shouldfail "asan" } */ > +/* { dg-set-target-env-var ASAN_OPTIONS > "detect_invalid_pointer_pairs=1:halt_on_error=1" } */ > +/* { dg-options "-fsanitize=address,pointer-subtract" } */ > +/* { dg-additional-options "-pthread" { target pthread } } */ > + > +#include <unistd.h> > +#include <pthread.h> > + > +char *pointer; > +pthread_barrier_t bar; > + > +void * > +thread_main (void *n) > +{ > + char local; > + (void) n; > + pointer = &local; > + pthread_barrier_wait (&bar); > + pthread_barrier_wait (&bar); > + > + return 0; > +} > + > +int > +main () > +{ > + pthread_t thread; > + pthread_barrier_init (&bar, NULL, 2); > + pthread_create (&thread, NULL, thread_main, NULL); > + pthread_barrier_wait (&bar); > + > + char local; > + char *parent_pointer = &local; > + > + /* { dg-output "ERROR: AddressSanitizer: invalid-pointer-pair" } */ > + volatile __PTRDIFF_TYPE__ r = parent_pointer - pointer; > + pthread_barrier_wait (&bar); > + pthread_join (thread, NULL); > + pthread_barrier_destroy (&bar); > + > + return 0; > +} > --- libsanitizer/asan/asan_descriptions.cc.jj 2017-11-21 23:18:20.440637693 > +0100 > +++ libsanitizer/asan/asan_descriptions.cc 2017-12-04 22:38:28.155787511 > +0100 > @@ -333,6 +333,26 @@ void GlobalAddressDescription::Print(con > } > } > > +bool GlobalAddressDescription::PointsInsideTheSameVariable( > + const GlobalAddressDescription &other) const { > + if (size == 0 || other.size == 0) return false; > + > + for (uptr i = 0; i < size; i++) { > + const __asan_global &a = globals[i]; > + for (uptr j = 0; j < other.size; j++) { > + const __asan_global &b = other.globals[j]; > + if (a.beg == b.beg && > + a.beg <= addr && > + b.beg <= other.addr && > + (addr + access_size) < (a.beg + a.size) && > + (other.addr + other.access_size) < (b.beg + b.size)) > + return true; > + } > + } > + > + return false; > +} > + > void StackAddressDescription::Print() const { > Decorator d; > char tname[128]; > --- libsanitizer/asan/asan_descriptions.h.jj 2017-11-21 23:18:20.435637754 > +0100 > +++ libsanitizer/asan/asan_descriptions.h 2017-12-04 22:38:28.156787499 > +0100 > @@ -143,6 +143,10 @@ struct GlobalAddressDescription { > u8 size; > > void Print(const char *bug_type = "") const; > + > + // Returns true when this descriptions points inside the same global > variable > + // as other. Descriptions can have different address within the variable > + bool PointsInsideTheSameVariable(const GlobalAddressDescription &other) > const; > }; > > bool GetGlobalAddressInformation(uptr addr, uptr access_size, > --- libsanitizer/asan/asan_report.cc.jj 2017-11-21 23:18:20.435637754 > +0100 > +++ libsanitizer/asan/asan_report.cc 2017-12-04 22:38:28.156787499 +0100 > @@ -295,17 +295,58 @@ static NOINLINE void ReportInvalidPointe > in_report.ReportError(error); > } > > +static bool IsInvalidPointerPair(uptr a1, uptr a2) { > + if (a1 == a2) > + return false; > + > + // 256B in shadow memory can be iterated quite fast > + static const uptr kMaxOffset = 2048; > + > + uptr left = a1 < a2 ? a1 : a2; > + uptr right = a1 < a2 ? a2 : a1; > + uptr offset = right - left; > + if (offset <= kMaxOffset) > + return __asan_region_is_poisoned(left, offset); > + > + AsanThread *t = GetCurrentThread(); > + > + // check whether left is a stack memory pointer > + if (uptr shadow_offset1 = t->GetStackVariableShadowStart(left)) { > + uptr shadow_offset2 = t->GetStackVariableShadowStart(right); > + return shadow_offset2 == 0 || shadow_offset1 != shadow_offset2; > + } > + > + // check whether left is a heap memory address > + HeapAddressDescription hdesc1, hdesc2; > + if (GetHeapAddressInformation(left, 0, &hdesc1) && > + hdesc1.chunk_access.access_type == kAccessTypeInside) > + return !GetHeapAddressInformation(right, 0, &hdesc2) || > + hdesc2.chunk_access.access_type != kAccessTypeInside || > + hdesc1.chunk_access.chunk_begin != hdesc2.chunk_access.chunk_begin; > + > + // check whether left is an address of a global variable > + GlobalAddressDescription gdesc1, gdesc2; > + if (GetGlobalAddressInformation(left, 0, &gdesc1)) > + return !GetGlobalAddressInformation(right - 1, 0, &gdesc2) || > + !gdesc1.PointsInsideTheSameVariable(gdesc2); > + > + if (t->GetStackVariableShadowStart(right) || > + GetHeapAddressInformation(right, 0, &hdesc2) || > + GetGlobalAddressInformation(right - 1, 0, &gdesc2)) > + return true; > + > + // At this point we know nothing about both a1 and a2 addresses. > + return false; > +} > + > 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)) { > + > + if (IsInvalidPointerPair(a1, a2)) { > GET_CALLER_PC_BP_SP; > - return ReportInvalidPointerPair(pc, bp, sp, a1, a2); > + ReportInvalidPointerPair(pc, bp, sp, a1, a2); > } > } > // ----------------------- Mac-specific reports ----------------- {{{1 > --- libsanitizer/asan/asan_thread.cc.jj 2017-11-21 23:18:20.434637767 > +0100 > +++ libsanitizer/asan/asan_thread.cc 2017-12-04 22:38:28.156787499 +0100 > @@ -315,7 +315,7 @@ bool AsanThread::GetStackFrameAccessByAd > access->frame_descr = (const char *)((uptr*)bottom)[1]; > return true; > } > - uptr aligned_addr = addr & ~(SANITIZER_WORDSIZE/8 - 1); // align addr. > + uptr aligned_addr = RoundDownTo(addr, SANITIZER_WORDSIZE / 8); // align > addr. > uptr mem_ptr = RoundDownTo(aligned_addr, SHADOW_GRANULARITY); > u8 *shadow_ptr = (u8*)MemToShadow(aligned_addr); > u8 *shadow_bottom = (u8*)MemToShadow(bottom); > @@ -344,6 +344,29 @@ bool AsanThread::GetStackFrameAccessByAd > return true; > } > > +uptr AsanThread::GetStackVariableShadowStart(uptr addr) { > + uptr bottom = 0; > + if (AddrIsInStack(addr)) { > + bottom = stack_bottom(); > + } else if (has_fake_stack()) { > + bottom = fake_stack()->AddrIsInFakeStack(addr); > + CHECK(bottom); > + } else > + return 0; > + > + uptr aligned_addr = RoundDownTo(addr, SANITIZER_WORDSIZE / 8); // 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 + 1; > +} > + > bool AsanThread::AddrIsInStack(uptr addr) { > const auto bounds = GetStackBounds(); > return addr >= bounds.bottom && addr < bounds.top; > --- libsanitizer/asan/asan_thread.h.jj 2017-11-21 23:18:20.435637754 > +0100 > +++ libsanitizer/asan/asan_thread.h 2017-12-04 22:38:28.157787487 +0100 > @@ -88,6 +88,9 @@ class AsanThread { > }; > bool GetStackFrameAccessByAddr(uptr addr, StackFrameAccess *access); > > + // Returns a pointer to the start of the stack variable's shadow memory. > + uptr GetStackVariableShadowStart(uptr addr); > + > bool AddrIsInStack(uptr addr); > > void DeleteFakeStack(int tid) { > > > Jakub >