[PATCH] Add -fsanitize=pointer-{compare,subtract}.

Martin Liška mliska@suse.cz
Mon Dec 18 08:25:00 GMT 2017


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  <mliska@suse.cz>
> 	    Jakub Jelinek  <jakub@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
> 



More information about the Gcc-patches mailing list