This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
On 10/13/2017 03:13 PM, Jakub Jelinek wrote:
> On Fri, Oct 13, 2017 at 02:53:50PM +0200, Martin Liška wrote:
>> @@ -3826,6 +3827,19 @@ 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 = 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, c_fully_fold (op0, false, NULL),
>> + c_fully_fold (op1, false, NULL));
>> + }
>
Hello Jakub.
> Why the c_fully_fold? Can't that be deferred until it actually is
> folded all together later?
Yes, now it's not needed as I use save_expr. I hit some ICE before I switched
to use save_expr. That's why I put it there.
>
>> + ret = pointer_diff (location, op0, op1, &instrument_expr);
>> goto return_build_binary_op;
>> }
>> /* Handle pointer minus int. Just like pointer plus int. */
>> @@ -11707,6 +11721,18 @@ 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);
>> +
>> + 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);
>> + }
>
> Is this the right spot for this? I mean then you don't handle
> ptr >= 0 or ptr > 0 and similar or ptr >= 0x12345678.
> I know we have warnings or pedwarns for those, still I think it would be
> better to handle the above e.g. before
> if ((TREE_CODE (TREE_TYPE (orig_op0)) == BOOLEAN_TYPE
> || truth_value_p (TREE_CODE (orig_op0)))
> ^ (TREE_CODE (TREE_TYPE (orig_op1)) == BOOLEAN_TYPE
> || truth_value_p (TREE_CODE (orig_op1))))
> maybe_warn_bool_compare (location, code, orig_op0, orig_op1);
> by testing if ((code0 == POINTER_TYPE || code1 == POINTER_TYPE)
> && sanitize_flags_p (SANITIZE_POINTER_COMPARE))
Agree.
>
> What about the C++ FE? Or is pointer comparison well defined there?
> What about pointer subtraction? My memory is fuzzy.
No, you're right, I basically forgot:
```
>From § 5.9 of the C++11 standard:
If two pointers p and q of the same type point to different objects that
are not members of the same object or elements of the same array or to
different functions, or if only one of them is null, the results
of p<q, p>q, p<=q, and p>=q are unspecified.
```
I also moved the tests to c-c++-common/asan/ subfolder.
>
>> +// { dg-options "-fsanitize=pointer-compare -O0" }
>
> Please use -fsanitize=address,pointer-compare
> etc. in the testcases, so that it is an example to users who don't know
> we have implicit -fsanitize=address for these tests.
>> + if (offset <= 2048) {
>> + if (__asan_region_is_poisoned (left, offset) == 0)
>
> I think the LLVM coding conventions don't want a space before ( above.
>
>> + // check whether left is a stack memory pointer
>> + if (GetStackVariableBeginning(left, &shadow_offset1)) {
>> + if (GetStackVariableBeginning(right - 1, &shadow_offset2)
>> + && shadow_offset1 == shadow_offset2)
>
> Nor && at the beginning of the line (they want it at the end of previous
> one).
>
>> + return;
>> + else
>> + goto do_error;
>> + }
>
> If you have goto do_error; for all cases, then you don't need to indent
> further stuff into else ... further and further.
>
>> + // check whether left is a heap memory address
>> + else {
>> + HeapAddressDescription hdesc1, hdesc2;
>> + if (GetHeapAddressInformation(left, 0, &hdesc1)
>> + && hdesc1.chunk_access.access_type == kAccessTypeInside) {
>> + if (GetHeapAddressInformation(right, 0, &hdesc2)
>> + && hdesc2.chunk_access.access_type == kAccessTypeInside
>> + && (hdesc1.chunk_access.chunk_begin
>> + == hdesc2.chunk_access.chunk_begin))
>> + return;
>
> So, here one is a heap object and the other is not. That should be an
> do_error, right?
Yes, coding style should be fixed.
>
>> + } else {
>> + // check whether left is an address of a global variable
>> + GlobalAddressDescription gdesc1, gdesc2;
>> + if (GetGlobalAddressInformation(left, 0, &gdesc1)) {
>> + if (GetGlobalAddressInformation(right - 1, 0, &gdesc2)
>> + && gdesc1.PointsInsideASameVariable (gdesc2))
>> + return;
>> + } else {
>> + // TODO
>
> Either goto do_error; here too, or do the if (offset <= 16384) case here.
> Guess upstream wouldn't like it with a TODO spot.
Agree. Do you feel that it's right moment to trigger review process of libsanitizer
changes?
Thanks,
Martin
>
> Jakub
>
>From 92cff481bc39feb9e17075d80e11774ea3085719 Mon Sep 17 00:00:00 2001
From: marxin <mliska@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 <mliska@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 <mliska@suse.cz>
* c-typeck.c (pointer_diff): Add new argument and instrument
pointer subtraction.
(build_binary_op): Similar for pointer comparison.
gcc/cp/ChangeLog:
2017-10-16 Martin Liska <mliska@suse.cz>
* typeck.c (pointer_diff): Add new argument and instrument
pointer subtraction.
(cp_build_binary_op): Create compound expression if doing an
instrumentation.
gcc/testsuite/ChangeLog:
2017-10-16 Martin Liska <mliska@suse.cz>
* 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.
---
gcc/c/c-typeck.c | 33 +++++++--
gcc/cp/typeck.c | 43 +++++++++--
gcc/doc/invoke.texi | 22 ++++++
gcc/flag-types.h | 2 +
gcc/ipa-inline.c | 8 ++-
gcc/opts.c | 15 ++++
gcc/sanitizer.def | 4 ++
.../c-c++-common/asan/pointer-compare-1.c | 83 ++++++++++++++++++++++
.../c-c++-common/asan/pointer-compare-2.c | 76 ++++++++++++++++++++
.../c-c++-common/asan/pointer-subtract-1.c | 41 +++++++++++
.../c-c++-common/asan/pointer-subtract-2.c | 32 +++++++++
libsanitizer/asan/asan_descriptions.cc | 29 ++++++++
libsanitizer/asan/asan_descriptions.h | 5 ++
libsanitizer/asan/asan_report.cc | 57 +++++++++++++--
libsanitizer/asan/asan_thread.cc | 23 ++++++
libsanitizer/asan/asan_thread.h | 3 +
16 files changed, 458 insertions(+), 18 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-compare-1.c
create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-compare-2.c
create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-1.c
create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-2.c
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index cb9c589e061..d623418288f 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,17 @@ 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 = 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 as integers;
then drop through to build the divide operator.
Do not do default conversions on the minus operator
@@ -11188,7 +11200,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. */
@@ -11738,6 +11750,19 @@ build_binary_op (location_t location, enum tree_code code,
result_type = type1;
pedwarn (location, 0, "comparison between pointer and integer");
}
+
+ if ((code0 == POINTER_TYPE || code1 == POINTER_TYPE)
+ && sanitize_flags_p (SANITIZE_POINTER_COMPARE))
+ {
+ gcc_assert (current_function_decl != NULL_TREE);
+
+ 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
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 08b2ae555e6..25087934910 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -54,7 +54,7 @@ static tree rationalize_conditional_expr (enum tree_code, tree,
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);
@@ -4318,8 +4318,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))
@@ -5008,6 +5016,19 @@ 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))
+ {
+ gcc_assert (current_function_decl != NULL_TREE);
+
+ 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:
@@ -5363,11 +5384,12 @@ cp_pointer_int_sum (location_t loc, enum tree_code resultcode, tree ptrop,
}
/* 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;
tree restype = ptrdiff_type_node;
@@ -5401,6 +5423,17 @@ pointer_diff (location_t loc, tree op0, tree op1, tree ptrtype,
return error_mark_node;
}
+ 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 as integers;
then drop through to build the divide operator. */
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/c-c++-common/asan/pointer-compare-1.c b/gcc/testsuite/c-c++-common/asan/pointer-compare-1.c
new file mode 100644
index 00000000000..5f97d954a37
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/pointer-compare-1.c
@@ -0,0 +1,83 @@
+// { 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);
+ // { 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];
+ 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]);
+ // { 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);
+ return 1;
+}
diff --git a/gcc/testsuite/c-c++-common/asan/pointer-compare-2.c b/gcc/testsuite/c-c++-common/asan/pointer-compare-2.c
new file mode 100644
index 00000000000..4967064634f
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/pointer-compare-2.c
@@ -0,0 +1,76 @@
+// { 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);
+ 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;
+}
diff --git a/gcc/testsuite/c-c++-common/asan/pointer-subtract-1.c b/gcc/testsuite/c-c++-common/asan/pointer-subtract-1.c
new file mode 100644
index 00000000000..10792264f2e
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/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/c-c++-common/asan/pointer-subtract-2.c b/gcc/testsuite/c-c++-common/asan/pointer-subtract-2.c
new file mode 100644
index 00000000000..17cf0e6711d
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/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..8901772e0da 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..7eea02de04a 100644
--- a/libsanitizer/asan/asan_report.cc
+++ b/libsanitizer/asan/asan_report.cc
@@ -344,14 +344,57 @@ 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;
+ else
+ goto do_error;
+ }
+
+ {
+ uptr shadow_offset1, shadow_offset2;
+ ThreadRegistryLock l(&asanThreadRegistry());
+
+ // check whether left is a stack memory pointer
+ if (GetStackVariableBeginning(left, &shadow_offset1)) {
+ if (GetStackVariableBeginning(right - 1, &shadow_offset2) &&
+ shadow_offset1 == shadow_offset2)
+ return;
+ else
+ goto do_error;
+ }
+ // check whether left is a heap memory address
+ HeapAddressDescription hdesc1, hdesc2;
+ if (GetHeapAddressInformation(left, 0, &hdesc1) &&
+ hdesc1.chunk_access.access_type == kAccessTypeInside) {
+ if (GetHeapAddressInformation(right, 0, &hdesc2) &&
+ hdesc2.chunk_access.access_type == kAccessTypeInside &&
+ (hdesc1.chunk_access.chunk_begin
+ == hdesc2.chunk_access.chunk_begin))
+ return;
+ else
+ goto do_error;
+ }
+ // check whether left is an address of a global variable
+ GlobalAddressDescription gdesc1, gdesc2;
+ if (GetGlobalAddressInformation(left, 0, &gdesc1)) {
+ if (GetGlobalAddressInformation(right - 1, 0, &gdesc2) &&
+ gdesc1.PointsInsideASameVariable (gdesc2))
+ return;
+ } else
+ goto do_error;
}
+
+do_error:
+ 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..eb6db52905d 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
- References:
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.
- Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.