* [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
@ 2014-06-03 8:02 Yury Gribov
2014-06-03 8:38 ` Jakub Jelinek
0 siblings, 1 reply; 22+ messages in thread
From: Yury Gribov @ 2014-06-03 8:02 UTC (permalink / raw)
To: GCC Patches, Jakub Jelinek, Konstantin Serebryany,
Viacheslav Garbuzov, Dmitry Vyukov
[-- Attachment #1: Type: text/plain, Size: 513 bytes --]
Hi all,
This is a second try on outline Asan instrumentation (original patch:
https://gcc.gnu.org/ml/gcc-patches/2014-05/msg02194.html).
This patch adds support for replacing Asan inline checks with function
calls. This feature has been recently added to LLVM to
* reduce long compiler runtimes on large functions with huge (over 10K)
number of memory accesses (http://llvm.org/bugs/show_bug.cgi?id=12653)
* a step to full Kasan support in GCC
* reduce code size overhead
Bootstrapped/regtested on x64.
-Y
[-- Attachment #2: asan_calls_2.diff --]
[-- Type: text/x-diff, Size: 14512 bytes --]
2014-06-03 Yury Gribov <y.gribov@samsung.com>
New asan-instrumentation-with-call-threshold parameter.
gcc/
* asan.c (check_func): New function.
(build_check_stmt_sized): Likewise.
(build_check_stmt): Add support for new parameter.
(instrument_mem_region_access): Likewise.
(asan_instrument): Likewise.
* cfgcleanup.c (old_insns_match_p): Add support for new
functions.
* doc/invoke.texi: Describe new parameter.
* params.def: Define new parameter.
* params.h: Likewise.
* sanitizer.def: Describe new builtins.
gcc/testsuite/
* c-c++-common/asan/instrument-with-calls-1.c: New test.
* c-c++-common/asan/instrument-with-calls-2.c: Likewise.
* c-c++-common/asan/instrument-with-calls-3.c: Likewise.
diff --git a/gcc/asan.c b/gcc/asan.c
index 3397655..1677c51 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -257,6 +257,8 @@ struct asan_mem_ref
static alloc_pool asan_mem_ref_alloc_pool;
+static int asan_num_accesses;
+
/* This creates the alloc pool used to store the instances of
asan_mem_ref that are stored in the hash table asan_mem_ref_ht. */
@@ -1335,6 +1337,26 @@ report_error_func (bool is_store, HOST_WIDE_INT size_in_bytes, bool slow_p)
return builtin_decl_implicit (report[is_store][exact_log2 (size_in_bytes)]);
}
+/* Construct a function tree for __asan_{load,store}{1,2,4,8,16,_n}.
+ IS_STORE is either 1 (for a store) or 0 (for a load). */
+
+static tree
+check_func (bool is_store, int size_in_bytes, int slow_p)
+{
+ static enum built_in_function check[2][6]
+ = { { BUILT_IN_ASAN_CHECK_LOAD1, BUILT_IN_ASAN_CHECK_LOAD2,
+ BUILT_IN_ASAN_CHECK_LOAD4, BUILT_IN_ASAN_CHECK_LOAD8,
+ BUILT_IN_ASAN_CHECK_LOAD16, BUILT_IN_ASAN_CHECK_LOAD_N },
+ { BUILT_IN_ASAN_CHECK_STORE1, BUILT_IN_ASAN_CHECK_STORE2,
+ BUILT_IN_ASAN_CHECK_STORE4, BUILT_IN_ASAN_CHECK_STORE8,
+ BUILT_IN_ASAN_CHECK_STORE16, BUILT_IN_ASAN_CHECK_STORE_N } };
+ if ((size_in_bytes & (size_in_bytes - 1)) != 0
+ || size_in_bytes > 16
+ || slow_p)
+ return builtin_decl_implicit (check[is_store][5]);
+ return builtin_decl_implicit (check[is_store][exact_log2 (size_in_bytes)]);
+}
+
/* Split the current basic block and create a condition statement
insertion point right before or after the statement pointed to by
ITER. Return an iterator to the point at which the caller might
@@ -1520,9 +1542,11 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
tree shadow_type = TREE_TYPE (shadow_ptr_type);
tree uintptr_type
= build_nonstandard_integer_type (TYPE_PRECISION (TREE_TYPE (base)), 1);
- tree base_ssa = base;
+ tree base_ssa;
HOST_WIDE_INT real_size_in_bytes = size_in_bytes;
tree sz_arg = NULL_TREE;
+ bool use_calls =
+ asan_num_accesses++ >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
if (size_in_bytes == 1)
slow_p = false;
@@ -1534,15 +1558,27 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
slow_p = true;
}
- /* Get an iterator on the point where we can add the condition
- statement for the instrumentation. */
- gsi = create_cond_insert_point (iter, before_p,
- /*then_more_likely_p=*/false,
- /*create_then_fallthru_edge=*/false,
- &then_bb,
- &else_bb);
+ base_ssa = base = unshare_expr (base);
- base = unshare_expr (base);
+ if (use_calls)
+ {
+ gsi = *iter;
+ g = gimple_build_nop ();
+ if (before_p)
+ gsi_insert_before (&gsi, g, GSI_NEW_STMT);
+ else
+ gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+ }
+ else
+ {
+ /* Get an iterator on the point where we can add the condition
+ statement for the instrumentation. */
+ gsi = create_cond_insert_point (iter, before_p,
+ /*then_more_likely_p=*/false,
+ /*create_then_fallthru_edge=*/false,
+ &then_bb,
+ &else_bb);
+ }
/* BASE can already be an SSA_NAME; in that case, do not create a
new SSA_NAME for it. */
@@ -1563,6 +1599,22 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
gsi_insert_after (&gsi, g, GSI_NEW_STMT);
base_addr = gimple_assign_lhs (g);
+ if (use_calls)
+ {
+ tree fun = check_func (is_store, size_in_bytes, slow_p);
+ g = slow_p
+ ? gimple_build_call (fun, 1, base_addr)
+ : gimple_build_call (fun, 2, base_addr,
+ build_int_cst (pointer_sized_int_node, size_in_bytes));
+ gimple_set_location (g, location);
+ gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+
+ if (!before_p)
+ *iter = gsi;
+
+ return;
+ }
+
/* Build
(base_addr >> ASAN_SHADOW_SHIFT) + targetm.asan_shadow_offset (). */
shadow = build_shadow_mem_access (&gsi, location, base_addr,
@@ -1642,6 +1694,61 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
*iter = gsi_start_bb (else_bb);
}
+static void
+build_check_stmt_sized (location_t location, tree base, tree len,
+ gimple_stmt_iterator *iter, bool is_store)
+{
+ gimple g;
+ tree t;
+ gimple_stmt_iterator gsi = *iter;
+ tree uintptr_type
+ = build_nonstandard_integer_type (TYPE_PRECISION (TREE_TYPE (base)), 1);
+ tree base_ssa;
+ tree base_addr;
+
+ base_ssa = base = unshare_expr (base);
+
+ if (TREE_CODE (len) != SSA_NAME)
+ {
+ t = make_ssa_name (TREE_TYPE (len), NULL);
+ g = gimple_build_assign_with_ops (TREE_CODE (len), t, len, NULL);
+ gimple_set_location (g, location);
+ gsi_insert_before (&gsi, g, GSI_SAME_STMT);
+ len = t;
+ }
+
+ if (!useless_type_conversion_p (size_type_node, TREE_TYPE (len)))
+ {
+ t = make_ssa_name (size_type_node, NULL);
+ g = gimple_build_assign_with_ops (NOP_EXPR, t, len, NULL);
+ gimple_set_location (g, location);
+ gsi_insert_before (&gsi, g, GSI_SAME_STMT);
+ len = t;
+ }
+
+ if (TREE_CODE (base) != SSA_NAME)
+ {
+ g = gimple_build_assign_with_ops (TREE_CODE (base),
+ make_ssa_name (TREE_TYPE (base), NULL),
+ base, NULL_TREE);
+ gimple_set_location (g, location);
+ gsi_insert_before (&gsi, g, GSI_SAME_STMT);
+ base_ssa = gimple_assign_lhs (g);
+ }
+
+ g = gimple_build_assign_with_ops (NOP_EXPR,
+ make_ssa_name (uintptr_type, NULL),
+ base_ssa, NULL_TREE);
+ gimple_set_location (g, location);
+ gsi_insert_before (&gsi, g, GSI_SAME_STMT);
+ base_addr = gimple_assign_lhs (g);
+
+ g = gimple_build_call (check_func (is_store, 0, 1),
+ 2, base_addr, len);
+ gimple_set_location (g, location);
+ gsi_insert_before (&gsi, g, GSI_NEW_STMT);
+}
+
/* If T represents a memory access, add instrumentation code before ITER.
LOCATION is source code location.
IS_STORE is either TRUE (for a store) or FALSE (for a load). */
@@ -1774,6 +1881,9 @@ instrument_mem_region_access (tree base, tree len,
gimple_stmt_iterator *iter,
location_t location, bool is_store)
{
+ bool use_calls =
+ asan_num_accesses++ >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
+
if (!POINTER_TYPE_P (TREE_TYPE (base))
|| !INTEGRAL_TYPE_P (TREE_TYPE (len))
|| integer_zerop (len))
@@ -1795,6 +1905,14 @@ instrument_mem_region_access (tree base, tree len,
if (start_instrumented && end_instrumented)
return;
+ if (use_calls)
+ {
+ build_check_stmt_sized (location, base, len, iter, is_store);
+ update_mem_ref_hash_table (base, 1);
+ update_mem_ref_hash_table (end, 1);
+ return;
+ }
+
if (!is_gimple_constant (len))
{
/* So, the length of the memory area to asan-protect is
@@ -2568,6 +2686,7 @@ asan_instrument (void)
{
if (shadow_ptr_types[0] == NULL_TREE)
asan_init_shadow_ptr_types ();
+ asan_num_accesses = 0;
transform_statements ();
return 0;
}
diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c
index 7c24a6d..cfdd059 100644
--- a/gcc/cfgcleanup.c
+++ b/gcc/cfgcleanup.c
@@ -1175,7 +1175,7 @@ old_insns_match_p (int mode ATTRIBUTE_UNUSED, rtx i1, rtx i2)
&& DECL_FUNCTION_CODE (SYMBOL_REF_DECL (symbol))
>= BUILT_IN_ASAN_REPORT_LOAD1
&& DECL_FUNCTION_CODE (SYMBOL_REF_DECL (symbol))
- <= BUILT_IN_ASAN_REPORT_STORE16)
+ <= BUILT_IN_ASAN_CHECK_STORE_N)
return dir_none;
}
}
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 9475594..96d6895 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10230,6 +10230,12 @@ is enabled by default when using @option{-fsanitize=address} option.
To disable use-after-return detection use
@option{--param asan-use-after-return=0}.
+@item asan-instrumentation-with-call-threshold
+Once number of memory accesses in function becomes greater
+or equal than this number, use callbacks instead of
+generating inline code. E.g. to disable inline code use
+@option{--param asan-instrumentation-with-call-threshold=0}.
+
@end table
@end table
diff --git a/gcc/params.def b/gcc/params.def
index c3a8797..28ef79a 100644
--- a/gcc/params.def
+++ b/gcc/params.def
@@ -1090,6 +1090,12 @@ DEFPARAM (PARAM_ASAN_USE_AFTER_RETURN,
"Enable asan builtin functions protection",
1, 0, 1)
+DEFPARAM (PARAM_ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD,
+ "asan-instrumentation-with-call-threshold",
+ "Use callbacks instead of inline code once number of accesses "
+ " in function becomes greater or equal than this threshold",
+ 10000, 0, INT_MAX)
+
DEFPARAM (PARAM_UNINIT_CONTROL_DEP_ATTEMPTS,
"uninit-control-dep-attempts",
"Maximum number of nested calls to search for control dependencies "
diff --git a/gcc/params.h b/gcc/params.h
index 0d6daa2..d488e32 100644
--- a/gcc/params.h
+++ b/gcc/params.h
@@ -232,5 +232,7 @@ extern void init_param_values (int *params);
PARAM_VALUE (PARAM_ASAN_MEMINTRIN)
#define ASAN_USE_AFTER_RETURN \
PARAM_VALUE (PARAM_ASAN_USE_AFTER_RETURN)
+#define ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD \
+ PARAM_VALUE (PARAM_ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD)
#endif /* ! GCC_PARAMS_H */
diff --git a/gcc/sanitizer.def b/gcc/sanitizer.def
index 4016fc5..5649c6d 100644
--- a/gcc/sanitizer.def
+++ b/gcc/sanitizer.def
@@ -29,7 +29,7 @@ along with GCC; see the file COPYING3. If not see
/* Address Sanitizer */
DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_INIT, "__asan_init_v3",
BT_FN_VOID, ATTR_NOTHROW_LEAF_LIST)
-/* Do not reorder the BUILT_IN_ASAN_REPORT* builtins, e.g. cfgcleanup.c
+/* Do not reorder the BUILT_IN_ASAN_{REPORT,CHECK}* builtins, e.g. cfgcleanup.c
relies on this order. */
DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REPORT_LOAD1, "__asan_report_load1",
BT_FN_VOID_PTR, ATTR_TMPURE_NORETURN_NOTHROW_LEAF_LIST)
@@ -57,6 +57,30 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REPORT_STORE16, "__asan_report_store16",
DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REPORT_STORE_N, "__asan_report_store_n",
BT_FN_VOID_PTR_PTRMODE,
ATTR_TMPURE_NORETURN_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_CHECK_LOAD1, "__asan_load1",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_CHECK_LOAD2, "__asan_load2",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_CHECK_LOAD4, "__asan_load4",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_CHECK_LOAD8, "__asan_load8",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_CHECK_LOAD16, "__asan_load16",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_CHECK_LOAD_N, "__asan_loadN",
+ BT_FN_VOID_PTR_PTRMODE, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_CHECK_STORE1, "__asan_store1",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_CHECK_STORE2, "__asan_store2",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_CHECK_STORE4, "__asan_store4",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_CHECK_STORE8, "__asan_store8",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_CHECK_STORE16, "__asan_store16",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_CHECK_STORE_N, "__asan_storeN",
+ BT_FN_VOID_PTR_PTRMODE, ATTR_TMPURE_NOTHROW_LEAF_LIST)
DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REGISTER_GLOBALS,
"__asan_register_globals",
BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST)
diff --git a/gcc/testsuite/c-c++-common/asan/instrument-with-calls-1.c b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-1.c
new file mode 100644
index 0000000..80f7620
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-1.c
@@ -0,0 +1,10 @@
+/* { dg-do assemble } */
+/* { dg-options "--param asan-instrumentation-with-call-threshold=0 -save-temps" } */
+
+void f(char *a, int *b) {
+ *b = *a;
+}
+
+/* { dg-final { scan-assembler "__asan_load1" } } */
+/* { dg-final { scan-assembler "__asan_store4" } } */
+/* { dg-final { cleanup-saved-temps } } */
diff --git a/gcc/testsuite/c-c++-common/asan/instrument-with-calls-2.c b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-2.c
new file mode 100644
index 0000000..570f796
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-2.c
@@ -0,0 +1,16 @@
+/* { dg-do assemble } */
+/* { dg-options "--param asan-instrumentation-with-call-threshold=1 -save-temps" } */
+
+int x;
+
+void f(int *a, int *b) {
+ *a = 0;
+ asm volatile ("" ::: "memory");
+ x = *b;
+}
+
+/* { dg-final { scan-assembler-not "__asan_store4" } } */
+/* { dg-final { scan-assembler "__asan_report_store4" } } */
+/* { dg-final { scan-assembler "__asan_load4" } } */
+/* { dg-final { scan-assembler-not "__asan_report_load4" } } */
+/* { dg-final { cleanup-saved-temps } } */
diff --git a/gcc/testsuite/c-c++-common/asan/instrument-with-calls-3.c b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-3.c
new file mode 100644
index 0000000..3712c7a
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-3.c
@@ -0,0 +1,15 @@
+/* { dg-do assemble } */
+/* { dg-options "--param asan-instrumentation-with-call-threshold=0 -save-temps" } */
+
+struct A {
+ char x[7];
+};
+
+void f(struct A *x, struct A *y) {
+ *x = *y;
+}
+
+/* { dg-final { scan-assembler "__asan_loadN" } } */
+/* { dg-final { scan-assembler "__asan_storeN" } } */
+/* { dg-final { cleanup-saved-temps } } */
+
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-03 8:02 [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try) Yury Gribov
@ 2014-06-03 8:38 ` Jakub Jelinek
2014-06-03 9:33 ` Yury Gribov
2014-06-06 5:08 ` Yury Gribov
0 siblings, 2 replies; 22+ messages in thread
From: Jakub Jelinek @ 2014-06-03 8:38 UTC (permalink / raw)
To: Yury Gribov
Cc: GCC Patches, Konstantin Serebryany, Viacheslav Garbuzov, Dmitry Vyukov
On Tue, Jun 03, 2014 at 12:03:02PM +0400, Yury Gribov wrote:
> --- a/gcc/asan.c
> +++ b/gcc/asan.c
> @@ -257,6 +257,8 @@ struct asan_mem_ref
>
> static alloc_pool asan_mem_ref_alloc_pool;
>
> +static int asan_num_accesses;
> +
> /* This creates the alloc pool used to store the instances of
> asan_mem_ref that are stored in the hash table asan_mem_ref_ht. */
>
> @@ -1335,6 +1337,26 @@ report_error_func (bool is_store, HOST_WIDE_INT size_in_bytes, bool slow_p)
> return builtin_decl_implicit (report[is_store][exact_log2 (size_in_bytes)]);
> }
>
> +/* Construct a function tree for __asan_{load,store}{1,2,4,8,16,_n}.
> + IS_STORE is either 1 (for a store) or 0 (for a load). */
> +
> +static tree
> +check_func (bool is_store, int size_in_bytes, int slow_p)
> +{
> + static enum built_in_function check[2][6]
> + = { { BUILT_IN_ASAN_CHECK_LOAD1, BUILT_IN_ASAN_CHECK_LOAD2,
> + BUILT_IN_ASAN_CHECK_LOAD4, BUILT_IN_ASAN_CHECK_LOAD8,
> + BUILT_IN_ASAN_CHECK_LOAD16, BUILT_IN_ASAN_CHECK_LOAD_N },
> + { BUILT_IN_ASAN_CHECK_STORE1, BUILT_IN_ASAN_CHECK_STORE2,
> + BUILT_IN_ASAN_CHECK_STORE4, BUILT_IN_ASAN_CHECK_STORE8,
> + BUILT_IN_ASAN_CHECK_STORE16, BUILT_IN_ASAN_CHECK_STORE_N } };
Any reason why the BUILT_IN_* names so differ from the actual function
names? I.e. why not use BUILT_IN_ASAN_{LOAD,STORE}{1,2,4,8,16,N}
(no underscore before N, no CHECK_)?
> @@ -1520,9 +1542,11 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
> tree shadow_type = TREE_TYPE (shadow_ptr_type);
> tree uintptr_type
> = build_nonstandard_integer_type (TYPE_PRECISION (TREE_TYPE (base)), 1);
> - tree base_ssa = base;
> + tree base_ssa;
> HOST_WIDE_INT real_size_in_bytes = size_in_bytes;
> tree sz_arg = NULL_TREE;
> + bool use_calls =
> + asan_num_accesses++ >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
Wouldn't it be better to do
bool use_calls
= asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
(note, = on next line per GNU Coding Conventions), and then
if (!use_calls)
asan_num_accesses++;
so that you don't risk signed overflow? Maybe also
if (ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD == INT_MAX)
use_calls = false;
before that, so that there is a way to turn the calls off completely.
> @@ -1534,15 +1558,27 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
> slow_p = true;
> }
>
> - /* Get an iterator on the point where we can add the condition
> - statement for the instrumentation. */
> - gsi = create_cond_insert_point (iter, before_p,
> - /*then_more_likely_p=*/false,
> - /*create_then_fallthru_edge=*/false,
> - &then_bb,
> - &else_bb);
> + base_ssa = base = unshare_expr (base);
>
> - base = unshare_expr (base);
> + if (use_calls)
> + {
> + gsi = *iter;
> + g = gimple_build_nop ();
> + if (before_p)
> + gsi_insert_before (&gsi, g, GSI_NEW_STMT);
> + else
> + gsi_insert_after (&gsi, g, GSI_NEW_STMT);
This looks ugly. I don't like adding GIMPLE_NOP, it is going
to take many passes before it is going to be DCEd.
Just handle the use_calls && before_p case in the two spots
where a new stmt is inserted.
Also note (but, already preexisting issue) is that the
__asan_report* and __asan_{load,store}* calls are declared in
sanitizer.def as having 1st argument PTR type, while in the library
it is uptr (aka PTRMODE). So, either we should fix up sanitizer.def
to match that (we were passing already base_addr which is integral,
not pointer), or if we keep sanitizer.def as is, we should pass
to the calls base_ssa instead of base_addr.
> @@ -1642,6 +1694,61 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
> *iter = gsi_start_bb (else_bb);
> }
>
> +static void
> +build_check_stmt_sized (location_t location, tree base, tree len,
> + gimple_stmt_iterator *iter, bool is_store)
Formatting, gimple_stmt_iterator needs to be below location_t.
Furthermore, this looks just like a partial transition, I don't see
why we should emit one __asan_loadN call but two separate
__asan_report_load_1 for the instrumented stringops.
Instead, build_check_stmt should be changed, so that it is possible to pass
it a variable length (checked by the caller for non-zero already),
and in that case force using the slow_p stuff, but instead of adding a
constant size add the variable length - 1.
> @@ -1774,6 +1881,9 @@ instrument_mem_region_access (tree base, tree len,
> gimple_stmt_iterator *iter,
> location_t location, bool is_store)
> {
> + bool use_calls =
> + asan_num_accesses++ >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
> +
See above.
> @@ -1795,6 +1905,14 @@ instrument_mem_region_access (tree base, tree len,
> if (start_instrumented && end_instrumented)
> return;
>
> + if (use_calls)
> + {
> + build_check_stmt_sized (location, base, len, iter, is_store);
> + update_mem_ref_hash_table (base, 1);
> + update_mem_ref_hash_table (end, 1);
> + return;
> + }
> +
See above, here we should not handle use_calls any differently from
!use_calls.
Jakub
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-03 8:38 ` Jakub Jelinek
@ 2014-06-03 9:33 ` Yury Gribov
2014-06-03 9:37 ` Konstantin Serebryany
2014-06-03 9:37 ` Jakub Jelinek
2014-06-06 5:08 ` Yury Gribov
1 sibling, 2 replies; 22+ messages in thread
From: Yury Gribov @ 2014-06-03 9:33 UTC (permalink / raw)
To: Jakub Jelinek
Cc: GCC Patches, Konstantin Serebryany, Viacheslav Garbuzov, Dmitry Vyukov
> Any reason why the BUILT_IN_* names so differ from the actual function
> names? I.e. why not use BUILT_IN_ASAN_{LOAD,STORE}{1,2,4,8,16,N}
> (no underscore before N, no CHECK_)?
Makes sense.
> Wouldn't it be better to do
> ...
> so that you don't risk signed overflow? Maybe also
Yeah, that looks cleaner.
> Also note (but, already preexisting issue) is that the
> __asan_report* and __asan_{load,store}* calls are declared in
> sanitizer.def as having 1st argument PTR type, while in the library
> it is uptr (aka PTRMODE). So, either we should fix up sanitizer.def
> to match that (we were passing already base_addr which is integral,
> not pointer), or if we keep sanitizer.def as is, we should pass
> to the calls base_ssa instead of base_addr.
Which solution is better for middle-end? Would casting to uptr
complicate alias analysis or somehow damage optimization capabilities of
further passes?
Frankly I don't understand why libsanitizer prefers uptr to void*'s and
size_t's...
> Furthermore, this looks just like a partial transition, I don't see
> why we should emit one __asan_loadN call but two separate
> __asan_report_load_1 for the instrumented stringops.
I was too lazy to bother with strlen. But ok, I'll optimize this piece.
> Instead, build_check_stmt should be changed, so that it is possible to pass
> it a variable length (checked by the caller for non-zero already),
> and in that case force using the slow_p stuff, but instead of adding a
> constant size add the variable length - 1.
+1, this will simplify code.
-Y
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-03 9:33 ` Yury Gribov
2014-06-03 9:37 ` Konstantin Serebryany
@ 2014-06-03 9:37 ` Jakub Jelinek
1 sibling, 0 replies; 22+ messages in thread
From: Jakub Jelinek @ 2014-06-03 9:37 UTC (permalink / raw)
To: Yury Gribov
Cc: GCC Patches, Konstantin Serebryany, Viacheslav Garbuzov, Dmitry Vyukov
On Tue, Jun 03, 2014 at 01:33:39PM +0400, Yury Gribov wrote:
> >Also note (but, already preexisting issue) is that the
> >__asan_report* and __asan_{load,store}* calls are declared in
> >sanitizer.def as having 1st argument PTR type, while in the library
> >it is uptr (aka PTRMODE). So, either we should fix up sanitizer.def
> >to match that (we were passing already base_addr which is integral,
> >not pointer), or if we keep sanitizer.def as is, we should pass
> >to the calls base_ssa instead of base_addr.
>
> Which solution is better for middle-end? Would casting to uptr
> complicate alias analysis or somehow damage optimization
> capabilities of further passes?
Supposedly at least for use_calls case pretending the argument is
void * instead of uptr would result in smaller IL (no need to cast it
to integral type).
> Frankly I don't understand why libsanitizer prefers uptr to void*'s
> and size_t's...
Supposedly because then you don't have to cast it on the library side
if you want to perform masking/shifting etc. on it. But that doesn't sound
like a very strong argument.
Jakub
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-03 9:33 ` Yury Gribov
@ 2014-06-03 9:37 ` Konstantin Serebryany
2014-06-03 9:37 ` Jakub Jelinek
1 sibling, 0 replies; 22+ messages in thread
From: Konstantin Serebryany @ 2014-06-03 9:37 UTC (permalink / raw)
To: Yury Gribov
Cc: Jakub Jelinek, GCC Patches, Konstantin Serebryany,
Viacheslav Garbuzov, Dmitry Vyukov
On Tue, Jun 3, 2014 at 1:33 PM, Yury Gribov <y.gribov@samsung.com> wrote:
>> Any reason why the BUILT_IN_* names so differ from the actual function
>> names? I.e. why not use BUILT_IN_ASAN_{LOAD,STORE}{1,2,4,8,16,N}
>> (no underscore before N, no CHECK_)?
>
>
> Makes sense.
>
>> Wouldn't it be better to do
>> ...
>>
>> so that you don't risk signed overflow? Maybe also
>
>
> Yeah, that looks cleaner.
>
>
>> Also note (but, already preexisting issue) is that the
>> __asan_report* and __asan_{load,store}* calls are declared in
>> sanitizer.def as having 1st argument PTR type, while in the library
>> it is uptr (aka PTRMODE). So, either we should fix up sanitizer.def
>> to match that (we were passing already base_addr which is integral,
>> not pointer), or if we keep sanitizer.def as is, we should pass
>> to the calls base_ssa instead of base_addr.
>
>
> Which solution is better for middle-end? Would casting to uptr complicate
> alias analysis or somehow damage optimization capabilities of further
> passes?
>
> Frankly I don't understand why libsanitizer prefers uptr to void*'s and
> size_t's...
We can not use size_t because we do not include system headers.
>
>
>> Furthermore, this looks just like a partial transition, I don't see
>> why we should emit one __asan_loadN call but two separate
>> __asan_report_load_1 for the instrumented stringops.
>
>
> I was too lazy to bother with strlen. But ok, I'll optimize this piece.
Note that in clang we stopped instrumenting stringops in compiler and
fully rely on interceptors
({memset,memcpy,memmove} are replaced with
__asan_{memset,memcpy,memmove} to avoid late inlining)
>
>
>> Instead, build_check_stmt should be changed, so that it is possible to
>> pass
>> it a variable length (checked by the caller for non-zero already),
>> and in that case force using the slow_p stuff, but instead of adding a
>> constant size add the variable length - 1.
>
>
> +1, this will simplify code.
>
> -Y
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-03 8:38 ` Jakub Jelinek
2014-06-03 9:33 ` Yury Gribov
@ 2014-06-06 5:08 ` Yury Gribov
2014-06-06 6:02 ` Jakub Jelinek
1 sibling, 1 reply; 22+ messages in thread
From: Yury Gribov @ 2014-06-06 5:08 UTC (permalink / raw)
To: Jakub Jelinek
Cc: GCC Patches, Konstantin Serebryany, Viacheslav Garbuzov, Dmitry Vyukov
[-- Attachment #1: Type: text/plain, Size: 1156 bytes --]
Jakub,
Here is an updated patch. I have not addressed some of the points:
> Also note (but, already preexisting issue) is that the
> __asan_report* and __asan_{load,store}* calls are declared in
> sanitizer.def as having 1st argument PTR type, while in the library
> it is uptr (aka PTRMODE). So, either we should fix up sanitizer.def
> to match that (we were passing already base_addr which is integral,
> not pointer), or if we keep sanitizer.def as is, we should pass
> to the calls base_ssa instead of base_addr.
Can I do this with a separate patch? The current one is already quite big.
>> + if (use_calls)
>> + {
>> + build_check_stmt_sized (location, base, len, iter, is_store);
>> + update_mem_ref_hash_table (base, 1);
>> + update_mem_ref_hash_table (end, 1);
>> + return;
>> + }
>> +
>
> See above, here we should not handle use_calls any differently from
> !use_calls.
Build_check_stmt is now unified for const/variable lengths
but I'd still prefer to treat the use_calls case specially
because calling single __asan_loadN is more space-efficient
than emitting two separate calls with a length check.
-Y
[-- Attachment #2: asan_calls_3.diff --]
[-- Type: text/x-diff, Size: 22299 bytes --]
2014-06-06 Yury Gribov <y.gribov@samsung.com>
New asan-instrumentation-with-call-threshold parameter.
gcc/
* asan.c (check_func): New function.
(use_calls_p): Likewise.
(report_error_func): Change interface.
(build_check_stmt): Allow non-integer lengths; add support
for new parameter.
(instrument_mem_region_access): Likewise.
(asan_instrument): Likewise.
(instrument_derefs): Moved code to build_check_stmt.
* cfgcleanup.c (old_insns_match_p): Add support for new
functions.
* doc/invoke.texi: Describe new parameter.
* params.def: Define new parameter.
* params.h: Likewise.
* sanitizer.def: Describe new builtins.
gcc/testsuite/
* c-c++-common/asan/instrument-with-calls-1.c: New test.
* c-c++-common/asan/instrument-with-calls-2.c: Likewise.
* c-c++-common/asan/instrument-with-calls-3.c: Likewise.
diff --git a/gcc/asan.c b/gcc/asan.c
index 3397655..acaf110 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -257,6 +257,15 @@ struct asan_mem_ref
static alloc_pool asan_mem_ref_alloc_pool;
+static int asan_num_accesses;
+
+static inline bool
+use_calls_p ()
+{
+ return ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD < INT_MAX
+ && asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
+}
+
/* This creates the alloc pool used to store the instances of
asan_mem_ref that are stored in the hash table asan_mem_ref_ht. */
@@ -1319,7 +1328,7 @@ asan_protect_global (tree decl)
IS_STORE is either 1 (for a store) or 0 (for a load). */
static tree
-report_error_func (bool is_store, HOST_WIDE_INT size_in_bytes, bool slow_p)
+report_error_func (bool is_store, HOST_WIDE_INT size_in_bytes, int *nargs)
{
static enum built_in_function report[2][6]
= { { BUILT_IN_ASAN_REPORT_LOAD1, BUILT_IN_ASAN_REPORT_LOAD2,
@@ -1328,13 +1337,37 @@ report_error_func (bool is_store, HOST_WIDE_INT size_in_bytes, bool slow_p)
{ BUILT_IN_ASAN_REPORT_STORE1, BUILT_IN_ASAN_REPORT_STORE2,
BUILT_IN_ASAN_REPORT_STORE4, BUILT_IN_ASAN_REPORT_STORE8,
BUILT_IN_ASAN_REPORT_STORE16, BUILT_IN_ASAN_REPORT_STORE_N } };
- if ((size_in_bytes & (size_in_bytes - 1)) != 0
- || size_in_bytes > 16
- || slow_p)
- return builtin_decl_implicit (report[is_store][5]);
+ if (size_in_bytes == -1)
+ {
+ *nargs = 2;
+ return builtin_decl_implicit (report[is_store][5]);
+ }
+ *nargs = 1;
return builtin_decl_implicit (report[is_store][exact_log2 (size_in_bytes)]);
}
+/* Construct a function tree for __asan_{load,store}{1,2,4,8,16,_n}.
+ IS_STORE is either 1 (for a store) or 0 (for a load). */
+
+static tree
+check_func (bool is_store, int size_in_bytes, int *nargs)
+{
+ static enum built_in_function check[2][6]
+ = { { BUILT_IN_ASAN_LOAD1, BUILT_IN_ASAN_LOAD2,
+ BUILT_IN_ASAN_LOAD4, BUILT_IN_ASAN_LOAD8,
+ BUILT_IN_ASAN_LOAD16, BUILT_IN_ASAN_LOADN },
+ { BUILT_IN_ASAN_STORE1, BUILT_IN_ASAN_STORE2,
+ BUILT_IN_ASAN_STORE4, BUILT_IN_ASAN_STORE8,
+ BUILT_IN_ASAN_STORE16, BUILT_IN_ASAN_STOREN } };
+ if (size_in_bytes == -1)
+ {
+ *nargs = 2;
+ return builtin_decl_implicit (check[is_store][5]);
+ }
+ *nargs = 1;
+ return builtin_decl_implicit (check[is_store][exact_log2 (size_in_bytes)]);
+}
+
/* Split the current basic block and create a condition statement
insertion point right before or after the statement pointed to by
ITER. Return an iterator to the point at which the caller might
@@ -1508,41 +1541,72 @@ build_shadow_mem_access (gimple_stmt_iterator *gsi, location_t location,
otherwise, it points to the statement logically following it. */
static void
-build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
- bool before_p, bool is_store, HOST_WIDE_INT size_in_bytes,
- bool slow_p = false)
+build_check_stmt (location_t location, tree base, tree len,
+ gimple_stmt_iterator *iter, bool before_p, bool is_store,
+ unsigned int align = 0)
{
gimple_stmt_iterator gsi;
basic_block then_bb, else_bb;
tree t, base_addr, shadow;
gimple g;
- tree shadow_ptr_type = shadow_ptr_types[size_in_bytes == 16 ? 1 : 0];
- tree shadow_type = TREE_TYPE (shadow_ptr_type);
tree uintptr_type
= build_nonstandard_integer_type (TYPE_PRECISION (TREE_TYPE (base)), 1);
- tree base_ssa = base;
+ tree base_ssa;
+ tree sz_arg;
+ bool use_calls = use_calls_p ();
+
+ HOST_WIDE_INT size_in_bytes
+ = len && tree_fits_uhwi_p (len) ? TREE_INT_CST_LOW (len) : -1;
HOST_WIDE_INT real_size_in_bytes = size_in_bytes;
- tree sz_arg = NULL_TREE;
- if (size_in_bytes == 1)
- slow_p = false;
- else if ((size_in_bytes & (size_in_bytes - 1)) != 0
- || size_in_bytes > 16
- || slow_p)
+ tree shadow_ptr_type = shadow_ptr_types[size_in_bytes == 16 ? 1 : 0];
+ tree shadow_type = TREE_TYPE (shadow_ptr_type);
+
+ if (size_in_bytes > 1)
{
- real_size_in_bytes = 1;
- slow_p = true;
+ if ((size_in_bytes & (size_in_bytes - 1)) != 0
+ || size_in_bytes > 16)
+ {
+ size_in_bytes = -1;
+ real_size_in_bytes = 1;
+ }
+ else
+ {
+ if (align && align < size_in_bytes * BITS_PER_UNIT)
+ {
+ /* On non-strict alignment targets, if
+ 16-byte access is just 8-byte aligned,
+ this will result in misaligned shadow
+ memory 2 byte load, but otherwise can
+ be handled using one read. */
+ if (size_in_bytes != 16
+ || STRICT_ALIGNMENT
+ || align < 8 * BITS_PER_UNIT)
+ {
+ size_in_bytes = -1;
+ real_size_in_bytes = 1;
+ }
+ }
+ }
}
- /* Get an iterator on the point where we can add the condition
- statement for the instrumentation. */
- gsi = create_cond_insert_point (iter, before_p,
- /*then_more_likely_p=*/false,
- /*create_then_fallthru_edge=*/false,
- &then_bb,
- &else_bb);
+ if (!use_calls)
+ ++asan_num_accesses;
- base = unshare_expr (base);
+ base_ssa = base = unshare_expr (base);
+
+ if (use_calls)
+ gsi = *iter;
+ else
+ {
+ /* Get an iterator on the point where we can add the condition
+ statement for the instrumentation. */
+ gsi = create_cond_insert_point (iter, before_p,
+ /*then_more_likely_p=*/false,
+ /*create_then_fallthru_edge=*/false,
+ &then_bb,
+ &else_bb);
+ }
/* BASE can already be an SSA_NAME; in that case, do not create a
new SSA_NAME for it. */
@@ -1552,7 +1616,10 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
make_ssa_name (TREE_TYPE (base), NULL),
base, NULL_TREE);
gimple_set_location (g, location);
- gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+ if (before_p && use_calls)
+ gsi_insert_before (&gsi, g, GSI_SAME_STMT);
+ else
+ gsi_insert_after (&gsi, g, GSI_NEW_STMT);
base_ssa = gimple_assign_lhs (g);
}
@@ -1560,9 +1627,39 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
make_ssa_name (uintptr_type, NULL),
base_ssa, NULL_TREE);
gimple_set_location (g, location);
- gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+ if (before_p && use_calls)
+ gsi_insert_before (&gsi, g, GSI_SAME_STMT);
+ else
+ gsi_insert_after (&gsi, g, GSI_NEW_STMT);
base_addr = gimple_assign_lhs (g);
+ g = gimple_build_assign_with_ops (NOP_EXPR,
+ make_ssa_name (uintptr_type, NULL),
+ len, NULL_TREE);
+ gimple_set_location (g, location);
+ if (before_p && use_calls)
+ gsi_insert_before (&gsi, g, GSI_SAME_STMT);
+ else
+ gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+ sz_arg = gimple_assign_lhs (g);
+
+ if (use_calls)
+ {
+ int nargs;
+ tree fun = check_func (is_store, size_in_bytes, &nargs);
+ g = gimple_build_call (fun, nargs, base_addr, sz_arg);
+ gimple_set_location (g, location);
+ if (before_p)
+ gsi_insert_before (&gsi, g, GSI_SAME_STMT);
+ else
+ {
+ gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+ gsi_next (&gsi);
+ *iter = gsi;
+ }
+ return;
+ }
+
/* Build
(base_addr >> ASAN_SHADOW_SHIFT) + targetm.asan_shadow_offset (). */
shadow = build_shadow_mem_access (&gsi, location, base_addr,
@@ -1591,13 +1688,19 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
gimple_seq_set_location (seq, location);
gsi_insert_seq_after (&gsi, seq, GSI_CONTINUE_LINKING);
/* For weird access sizes or misaligned, check first and last byte. */
- if (slow_p)
+ if (size_in_bytes == -1)
{
+ g = gimple_build_assign_with_ops (MINUS_EXPR,
+ make_ssa_name (uintptr_type, NULL),
+ sz_arg,
+ build_int_cst (uintptr_type, 1));
+ gimple_set_location (g, location);
+ gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+ tree last = gimple_assign_lhs (g);
g = gimple_build_assign_with_ops (PLUS_EXPR,
make_ssa_name (uintptr_type, NULL),
base_addr,
- build_int_cst (uintptr_type,
- size_in_bytes - 1));
+ last);
gimple_set_location (g, location);
gsi_insert_after (&gsi, g, GSI_NEW_STMT);
tree base_end_addr = gimple_assign_lhs (g);
@@ -1621,7 +1724,6 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
t = gimple_assign_lhs (gimple_seq_last (seq));
gimple_seq_set_location (seq, location);
gsi_insert_seq_after (&gsi, seq, GSI_CONTINUE_LINKING);
- sz_arg = build_int_cst (pointer_sized_int_node, size_in_bytes);
}
}
else
@@ -1634,8 +1736,9 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
/* Generate call to the run-time library (e.g. __asan_report_load8). */
gsi = gsi_start_bb (then_bb);
- g = gimple_build_call (report_error_func (is_store, size_in_bytes, slow_p),
- sz_arg ? 2 : 1, base_addr, sz_arg);
+ int nargs;
+ tree fun = report_error_func (is_store, size_in_bytes, &nargs);
+ g = gimple_build_call (fun, nargs, base_addr, sz_arg);
gimple_set_location (g, location);
gsi_insert_after (&gsi, g, GSI_NEW_STMT);
@@ -1656,7 +1759,7 @@ instrument_derefs (gimple_stmt_iterator *iter, tree t,
return;
tree type, base;
- HOST_WIDE_INT size_in_bytes;
+ HOST_WIDE_INT isize;
type = TREE_TYPE (t);
switch (TREE_CODE (t))
@@ -1672,8 +1775,8 @@ instrument_derefs (gimple_stmt_iterator *iter, tree t,
return;
}
- size_in_bytes = int_size_in_bytes (type);
- if (size_in_bytes <= 0)
+ isize = int_size_in_bytes (type);
+ if (isize <= 0)
return;
HOST_WIDE_INT bitsize, bitpos;
@@ -1682,9 +1785,9 @@ instrument_derefs (gimple_stmt_iterator *iter, tree t,
int volatilep = 0, unsignedp = 0;
tree inner = get_inner_reference (t, &bitsize, &bitpos, &offset,
&mode, &unsignedp, &volatilep, false);
- if (((size_in_bytes & (size_in_bytes - 1)) == 0
- && (bitpos % (size_in_bytes * BITS_PER_UNIT)))
- || bitsize != size_in_bytes * BITS_PER_UNIT)
+ if (((isize & (isize - 1)) == 0
+ && (bitpos % (isize * BITS_PER_UNIT)))
+ || bitsize != isize * BITS_PER_UNIT)
{
if (TREE_CODE (t) == COMPONENT_REF
&& DECL_BIT_FIELD_REPRESENTATIVE (TREE_OPERAND (t, 1)) != NULL_TREE)
@@ -1728,35 +1831,13 @@ instrument_derefs (gimple_stmt_iterator *iter, tree t,
}
base = build_fold_addr_expr (t);
- if (!has_mem_ref_been_instrumented (base, size_in_bytes))
+ if (!has_mem_ref_been_instrumented (base, isize))
{
- bool slow_p = false;
- if (size_in_bytes > 1)
- {
- if ((size_in_bytes & (size_in_bytes - 1)) != 0
- || size_in_bytes > 16)
- slow_p = true;
- else
- {
- unsigned int align = get_object_alignment (t);
- if (align < size_in_bytes * BITS_PER_UNIT)
- {
- /* On non-strict alignment targets, if
- 16-byte access is just 8-byte aligned,
- this will result in misaligned shadow
- memory 2 byte load, but otherwise can
- be handled using one read. */
- if (size_in_bytes != 16
- || STRICT_ALIGNMENT
- || align < 8 * BITS_PER_UNIT)
- slow_p = true;
- }
- }
- }
- build_check_stmt (location, base, iter, /*before_p=*/true,
- is_store, size_in_bytes, slow_p);
- update_mem_ref_hash_table (base, size_in_bytes);
- update_mem_ref_hash_table (t, size_in_bytes);
+ unsigned int align = get_object_alignment (t);
+ build_check_stmt (location, base, size_in_bytes (type), iter,
+ /*before_p=*/true, is_store, align);
+ update_mem_ref_hash_table (base, isize);
+ update_mem_ref_hash_table (t, isize);
}
}
@@ -1774,6 +1855,8 @@ instrument_mem_region_access (tree base, tree len,
gimple_stmt_iterator *iter,
location_t location, bool is_store)
{
+ bool use_calls = use_calls_p ();
+
if (!POINTER_TYPE_P (TREE_TYPE (base))
|| !INTEGRAL_TYPE_P (TREE_TYPE (len))
|| integer_zerop (len))
@@ -1795,6 +1878,16 @@ instrument_mem_region_access (tree base, tree len,
if (start_instrumented && end_instrumented)
return;
+ if (use_calls)
+ {
+ /* It is faster to simply call __asan_(load|store)N instead of
+ calculating length and checking bytes independently. */
+ build_check_stmt (location, base, len, iter, /*before_p*/true, is_store);
+ update_mem_ref_hash_table (base, 1);
+ update_mem_ref_hash_table (end, 1);
+ return;
+ }
+
if (!is_gimple_constant (len))
{
/* So, the length of the memory area to asan-protect is
@@ -1827,7 +1920,8 @@ instrument_mem_region_access (tree base, tree len,
/* Instrument the beginning of the memory region to be accessed,
and arrange for the rest of the intrumentation code to be
inserted in the then block *after* the current gsi. */
- build_check_stmt (location, base, &gsi, /*before_p=*/true, is_store, 1);
+ build_check_stmt (location, base, build_int_cst (size_type_node, 1), &gsi,
+ /*before_p=*/true, is_store);
if (then_bb)
/* We are in the case where the length of the region is not
@@ -1912,7 +2006,8 @@ instrument_mem_region_access (tree base, tree len,
/* instrument access at _2; */
gsi = gsi_for_stmt (region_end);
build_check_stmt (location, gimple_assign_lhs (region_end),
- &gsi, /*before_p=*/false, is_store, 1);
+ build_int_cst (size_type_node, 1),
+ &gsi, /*before_p=*/false, is_store);
if (then_bb == NULL)
update_mem_ref_hash_table (end, 1);
@@ -1967,8 +2062,9 @@ instrument_strlen_call (gimple_stmt_iterator *iter)
gimple_set_location (str_arg_ssa, loc);
gimple_stmt_iterator gsi = *iter;
gsi_insert_before (&gsi, str_arg_ssa, GSI_NEW_STMT);
- build_check_stmt (loc, gimple_assign_lhs (str_arg_ssa), &gsi,
- /*before_p=*/false, /*is_store=*/false, 1);
+ build_check_stmt (loc, gimple_assign_lhs (str_arg_ssa),
+ build_int_cst (size_type_node, 1), &gsi,
+ /*before_p=*/false, /*is_store=*/false);
/* If we initially had an instruction like:
@@ -1987,8 +2083,9 @@ instrument_strlen_call (gimple_stmt_iterator *iter)
gimple_set_location (stmt, loc);
gsi_insert_after (&gsi, stmt, GSI_NEW_STMT);
- build_check_stmt (loc, gimple_assign_lhs (stmt), &gsi,
- /*before_p=*/false, /*is_store=*/false, 1);
+ build_check_stmt (loc, gimple_assign_lhs (stmt),
+ build_int_cst (size_type_node, 1), &gsi,
+ /*before_p=*/false, /*is_store=*/false);
/* Ensure that iter points to the statement logically following the
one it was initially pointing to. */
@@ -2568,6 +2665,7 @@ asan_instrument (void)
{
if (shadow_ptr_types[0] == NULL_TREE)
asan_init_shadow_ptr_types ();
+ asan_num_accesses = 0;
transform_statements ();
return 0;
}
diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c
index 7c24a6d..e857543 100644
--- a/gcc/cfgcleanup.c
+++ b/gcc/cfgcleanup.c
@@ -1175,7 +1175,7 @@ old_insns_match_p (int mode ATTRIBUTE_UNUSED, rtx i1, rtx i2)
&& DECL_FUNCTION_CODE (SYMBOL_REF_DECL (symbol))
>= BUILT_IN_ASAN_REPORT_LOAD1
&& DECL_FUNCTION_CODE (SYMBOL_REF_DECL (symbol))
- <= BUILT_IN_ASAN_REPORT_STORE16)
+ <= BUILT_IN_ASAN_STOREN)
return dir_none;
}
}
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 9475594..96d6895 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10230,6 +10230,12 @@ is enabled by default when using @option{-fsanitize=address} option.
To disable use-after-return detection use
@option{--param asan-use-after-return=0}.
+@item asan-instrumentation-with-call-threshold
+Once number of memory accesses in function becomes greater
+or equal than this number, use callbacks instead of
+generating inline code. E.g. to disable inline code use
+@option{--param asan-instrumentation-with-call-threshold=0}.
+
@end table
@end table
diff --git a/gcc/params.def b/gcc/params.def
index c3a8797..28ef79a 100644
--- a/gcc/params.def
+++ b/gcc/params.def
@@ -1090,6 +1090,12 @@ DEFPARAM (PARAM_ASAN_USE_AFTER_RETURN,
"Enable asan builtin functions protection",
1, 0, 1)
+DEFPARAM (PARAM_ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD,
+ "asan-instrumentation-with-call-threshold",
+ "Use callbacks instead of inline code once number of accesses "
+ " in function becomes greater or equal than this threshold",
+ 10000, 0, INT_MAX)
+
DEFPARAM (PARAM_UNINIT_CONTROL_DEP_ATTEMPTS,
"uninit-control-dep-attempts",
"Maximum number of nested calls to search for control dependencies "
diff --git a/gcc/params.h b/gcc/params.h
index 0d6daa2..d488e32 100644
--- a/gcc/params.h
+++ b/gcc/params.h
@@ -232,5 +232,7 @@ extern void init_param_values (int *params);
PARAM_VALUE (PARAM_ASAN_MEMINTRIN)
#define ASAN_USE_AFTER_RETURN \
PARAM_VALUE (PARAM_ASAN_USE_AFTER_RETURN)
+#define ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD \
+ PARAM_VALUE (PARAM_ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD)
#endif /* ! GCC_PARAMS_H */
diff --git a/gcc/sanitizer.def b/gcc/sanitizer.def
index 4016fc5..b4af164 100644
--- a/gcc/sanitizer.def
+++ b/gcc/sanitizer.def
@@ -29,7 +29,7 @@ along with GCC; see the file COPYING3. If not see
/* Address Sanitizer */
DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_INIT, "__asan_init_v3",
BT_FN_VOID, ATTR_NOTHROW_LEAF_LIST)
-/* Do not reorder the BUILT_IN_ASAN_REPORT* builtins, e.g. cfgcleanup.c
+/* Do not reorder the BUILT_IN_ASAN_{REPORT,CHECK}* builtins, e.g. cfgcleanup.c
relies on this order. */
DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REPORT_LOAD1, "__asan_report_load1",
BT_FN_VOID_PTR, ATTR_TMPURE_NORETURN_NOTHROW_LEAF_LIST)
@@ -57,6 +57,30 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REPORT_STORE16, "__asan_report_store16",
DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REPORT_STORE_N, "__asan_report_store_n",
BT_FN_VOID_PTR_PTRMODE,
ATTR_TMPURE_NORETURN_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOAD1, "__asan_load1",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOAD2, "__asan_load2",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOAD4, "__asan_load4",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOAD8, "__asan_load8",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOAD16, "__asan_load16",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOADN, "__asan_loadN",
+ BT_FN_VOID_PTR_PTRMODE, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STORE1, "__asan_store1",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STORE2, "__asan_store2",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STORE4, "__asan_store4",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STORE8, "__asan_store8",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STORE16, "__asan_store16",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STOREN, "__asan_storeN",
+ BT_FN_VOID_PTR_PTRMODE, ATTR_TMPURE_NOTHROW_LEAF_LIST)
DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REGISTER_GLOBALS,
"__asan_register_globals",
BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST)
diff --git a/gcc/testsuite/c-c++-common/asan/instrument-with-calls-1.c b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-1.c
new file mode 100644
index 0000000..80f7620
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-1.c
@@ -0,0 +1,10 @@
+/* { dg-do assemble } */
+/* { dg-options "--param asan-instrumentation-with-call-threshold=0 -save-temps" } */
+
+void f(char *a, int *b) {
+ *b = *a;
+}
+
+/* { dg-final { scan-assembler "__asan_load1" } } */
+/* { dg-final { scan-assembler "__asan_store4" } } */
+/* { dg-final { cleanup-saved-temps } } */
diff --git a/gcc/testsuite/c-c++-common/asan/instrument-with-calls-2.c b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-2.c
new file mode 100644
index 0000000..570f796
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-2.c
@@ -0,0 +1,16 @@
+/* { dg-do assemble } */
+/* { dg-options "--param asan-instrumentation-with-call-threshold=1 -save-temps" } */
+
+int x;
+
+void f(int *a, int *b) {
+ *a = 0;
+ asm volatile ("" ::: "memory");
+ x = *b;
+}
+
+/* { dg-final { scan-assembler-not "__asan_store4" } } */
+/* { dg-final { scan-assembler "__asan_report_store4" } } */
+/* { dg-final { scan-assembler "__asan_load4" } } */
+/* { dg-final { scan-assembler-not "__asan_report_load4" } } */
+/* { dg-final { cleanup-saved-temps } } */
diff --git a/gcc/testsuite/c-c++-common/asan/instrument-with-calls-3.c b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-3.c
new file mode 100644
index 0000000..3712c7a
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-3.c
@@ -0,0 +1,15 @@
+/* { dg-do assemble } */
+/* { dg-options "--param asan-instrumentation-with-call-threshold=0 -save-temps" } */
+
+struct A {
+ char x[7];
+};
+
+void f(struct A *x, struct A *y) {
+ *x = *y;
+}
+
+/* { dg-final { scan-assembler "__asan_loadN" } } */
+/* { dg-final { scan-assembler "__asan_storeN" } } */
+/* { dg-final { cleanup-saved-temps } } */
+
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-06 5:08 ` Yury Gribov
@ 2014-06-06 6:02 ` Jakub Jelinek
2014-06-09 4:53 ` Yury Gribov
2014-06-09 15:48 ` Yury Gribov
0 siblings, 2 replies; 22+ messages in thread
From: Jakub Jelinek @ 2014-06-06 6:02 UTC (permalink / raw)
To: Yury Gribov
Cc: GCC Patches, Konstantin Serebryany, Viacheslav Garbuzov, Dmitry Vyukov
On Fri, Jun 06, 2014 at 09:08:17AM +0400, Yury Gribov wrote:
> Build_check_stmt is now unified for const/variable lengths
> but I'd still prefer to treat the use_calls case specially
> because calling single __asan_loadN is more space-efficient
> than emitting two separate calls with a length check.
That is not what I meant.
1) instrument_mem_region_access/instrument_strlen should use a single
build_check_stmt call instead of two that they do now,
both for use_calls case and !use_calls case, the difference
is just that instrument_mem_region_access needs to guard
it against len == 0, while for use_calls it doesn't have to,
it seems __asan_{load,store}N handles length of 0 correctly
(still, the caller shouldn't update_mem_ref_hash_table
if length might be zero, even in the use_calls case)
2) passing always a tree length to build_check_stmt and then
recreating size_in_bytes out of it is ugly, just add a new
tree parameter, if it is non-NULL, the length is variable,
and real_size_in_bytes should be 1, and the code additionally
has to compute length - 1 at runtime and add the result,
otherwise it just keeps adding precomputed constant
Jakub
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-06 6:02 ` Jakub Jelinek
@ 2014-06-09 4:53 ` Yury Gribov
2014-06-09 15:48 ` Yury Gribov
1 sibling, 0 replies; 22+ messages in thread
From: Yury Gribov @ 2014-06-09 4:53 UTC (permalink / raw)
To: Jakub Jelinek
Cc: GCC Patches, Konstantin Serebryany, Viacheslav Garbuzov, Dmitry Vyukov
> 1) instrument_mem_region_access/instrument_strlen should use a single
> build_check_stmt call instead of two that they do now,
I agree with instrument_mem_region_access but the strlen case is quite
different because checks are inserted at different sides of instrumented
insn. Adding yet more flags and code paths to build_check_stmt would
IMHO overcomplicate it.
-Y
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-06 6:02 ` Jakub Jelinek
2014-06-09 4:53 ` Yury Gribov
@ 2014-06-09 15:48 ` Yury Gribov
2014-06-11 9:32 ` Jakub Jelinek
1 sibling, 1 reply; 22+ messages in thread
From: Yury Gribov @ 2014-06-09 15:48 UTC (permalink / raw)
To: Jakub Jelinek
Cc: GCC Patches, Konstantin Serebryany, Viacheslav Garbuzov, Dmitry Vyukov
[-- Attachment #1: Type: text/plain, Size: 1956 bytes --]
>> Build_check_stmt is now unified for const/variable lengths
>> but I'd still prefer to treat the use_calls case specially
>> because calling single __asan_loadN is more space-efficient
>> than emitting two separate calls with a length check.
>
> That is not what I meant.
> 1) instrument_mem_region_access/instrument_strlen should use a single
> build_check_stmt call instead of two that they do now,
> both for use_calls case and !use_calls case, the difference
> is just that instrument_mem_region_access needs to guard
> it against len == 0, while for use_calls it doesn't have to,
> it seems __asan_{load,store}N handles length of 0 correctly
> (still, the caller shouldn't update_mem_ref_hash_table
> if length might be zero, even in the use_calls case)
>2) passing always a tree length to build_check_stmt and then
> recreating size_in_bytes out of it is ugly, just add a new
> tree parameter, if it is non-NULL, the length is variable,
> and real_size_in_bytes should be 1, and the code additionally
> has to compute length - 1 at runtime and add the result,
> otherwise it just keeps adding precomputed constant
Let's check if I'm on the right way. I've attached current version. It
may have some rough edges but at least all regtests pass on x64.
Some obvious problems:
1) build_check_stmt argument list has become really long and messy;
(this could be simplified to some extent if we get rid of size_of_bytes
and non_zero_len_p and recompute them from len instead
but all other parameters seem really unavoidable)
2) we can't easily check number of instrumentations in
no-redundant-instrumentation-*.c family of tests
because memory region accesses now have single error handler
(__asan_report_load_n/store_n);
the best solution I came up with is counting all 'X & 7' patterns.
3) strlen instrumentation is still using two separate calls to
build_check_stmt (due to reasons I mentioned above).
-Y
[-- Attachment #2: asan_calls_4.diff --]
[-- Type: text/x-diff, Size: 42382 bytes --]
diff --git a/gcc/asan.c b/gcc/asan.c
index 3397655..fb1ddb9 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -242,6 +242,19 @@ static GTY(()) tree shadow_ptr_types[2];
/* Decl for __asan_option_detect_stack_use_after_return. */
static GTY(()) tree asan_detect_stack_use_after_return;
+/* Number of instrumentations in current function so far. */
+
+static int asan_num_accesses;
+
+/* Check whether we should replace inline instrumentation with calls. */
+
+static inline bool
+use_calls_p ()
+{
+ return ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD < INT_MAX
+ && asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
+}
+
/* Hashtable support for memory references used by gimple
statements. */
@@ -1319,7 +1332,7 @@ asan_protect_global (tree decl)
IS_STORE is either 1 (for a store) or 0 (for a load). */
static tree
-report_error_func (bool is_store, HOST_WIDE_INT size_in_bytes, bool slow_p)
+report_error_func (bool is_store, HOST_WIDE_INT size_in_bytes, int *nargs)
{
static enum built_in_function report[2][6]
= { { BUILT_IN_ASAN_REPORT_LOAD1, BUILT_IN_ASAN_REPORT_LOAD2,
@@ -1328,13 +1341,37 @@ report_error_func (bool is_store, HOST_WIDE_INT size_in_bytes, bool slow_p)
{ BUILT_IN_ASAN_REPORT_STORE1, BUILT_IN_ASAN_REPORT_STORE2,
BUILT_IN_ASAN_REPORT_STORE4, BUILT_IN_ASAN_REPORT_STORE8,
BUILT_IN_ASAN_REPORT_STORE16, BUILT_IN_ASAN_REPORT_STORE_N } };
- if ((size_in_bytes & (size_in_bytes - 1)) != 0
- || size_in_bytes > 16
- || slow_p)
- return builtin_decl_implicit (report[is_store][5]);
+ if (size_in_bytes == -1)
+ {
+ *nargs = 2;
+ return builtin_decl_implicit (report[is_store][5]);
+ }
+ *nargs = 1;
return builtin_decl_implicit (report[is_store][exact_log2 (size_in_bytes)]);
}
+/* Construct a function tree for __asan_{load,store}{1,2,4,8,16,_n}.
+ IS_STORE is either 1 (for a store) or 0 (for a load). */
+
+static tree
+check_func (bool is_store, int size_in_bytes, int *nargs)
+{
+ static enum built_in_function check[2][6]
+ = { { BUILT_IN_ASAN_LOAD1, BUILT_IN_ASAN_LOAD2,
+ BUILT_IN_ASAN_LOAD4, BUILT_IN_ASAN_LOAD8,
+ BUILT_IN_ASAN_LOAD16, BUILT_IN_ASAN_LOADN },
+ { BUILT_IN_ASAN_STORE1, BUILT_IN_ASAN_STORE2,
+ BUILT_IN_ASAN_STORE4, BUILT_IN_ASAN_STORE8,
+ BUILT_IN_ASAN_STORE16, BUILT_IN_ASAN_STOREN } };
+ if (size_in_bytes == -1)
+ {
+ *nargs = 2;
+ return builtin_decl_implicit (check[is_store][5]);
+ }
+ *nargs = 1;
+ return builtin_decl_implicit (check[is_store][exact_log2 (size_in_bytes)]);
+}
+
/* Split the current basic block and create a condition statement
insertion point right before or after the statement pointed to by
ITER. Return an iterator to the point at which the caller might
@@ -1494,6 +1531,76 @@ build_shadow_mem_access (gimple_stmt_iterator *gsi, location_t location,
return gimple_assign_lhs (g);
}
+/* BASE can already be an SSA_NAME; in that case, do not create a
+ new SSA_NAME for it. */
+
+static tree
+maybe_create_ssa_name (location_t loc, tree base, gimple_stmt_iterator *iter,
+ bool before_p)
+{
+ if (TREE_CODE (base) == SSA_NAME)
+ return base;
+ gimple g
+ = gimple_build_assign_with_ops (TREE_CODE (base),
+ make_ssa_name (TREE_TYPE (base), NULL),
+ base, NULL_TREE);
+ gimple_set_location (g, loc);
+ if (before_p)
+ gsi_insert_before (iter, g, GSI_SAME_STMT);
+ else
+ gsi_insert_after (iter, g, GSI_NEW_STMT);
+ return gimple_assign_lhs (g);
+}
+
+/* Instrument the memory access instruction using callbacks.
+ Parameters are similar to BUILD_CHECK_STMT. */
+
+static void
+build_check_stmt_with_calls (location_t loc, tree base, tree len,
+ HOST_WIDE_INT size_in_bytes, gimple_stmt_iterator *iter,
+ bool before_p, bool is_store, bool is_scalar_access)
+{
+ gimple_stmt_iterator gsi = *iter;
+ tree base_ssa = maybe_create_ssa_name (loc, base, &gsi, before_p);
+
+ gimple g
+ = gimple_build_assign_with_ops (NOP_EXPR,
+ make_ssa_name (pointer_sized_int_node, NULL),
+ base_ssa, NULL_TREE);
+ gimple_set_location (g, loc);
+ if (before_p)
+ gsi_insert_before (&gsi, g, GSI_NEW_STMT);
+ else
+ gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+ tree base_addr = gimple_assign_lhs (g);
+
+ int nargs;
+ tree fun
+ = check_func (is_store, is_scalar_access ? size_in_bytes : -1, &nargs);
+ if (nargs == 1)
+ g = gimple_build_call (fun, 1, base_addr);
+ else
+ {
+ gcc_assert (nargs == 2);
+ g = gimple_build_assign_with_ops (NOP_EXPR,
+ make_ssa_name (pointer_sized_int_node,
+ NULL),
+ len, NULL_TREE);
+ gimple_set_location (g, loc);
+ gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+ tree sz_arg = gimple_assign_lhs (g);
+ g = gimple_build_call (fun, nargs, base_addr, sz_arg);
+ }
+ gimple_set_location (g, loc);
+ gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+
+ if (!before_p)
+ {
+ gsi_next (&gsi);
+ *iter = gsi;
+ }
+}
+
/* Instrument the memory access instruction BASE. Insert new
statements before or after ITER.
@@ -1501,111 +1608,198 @@ build_shadow_mem_access (gimple_stmt_iterator *gsi, location_t location,
SSA_NAME, or a non-SSA expression. LOCATION is the source code
location. IS_STORE is TRUE for a store, FALSE for a load.
BEFORE_P is TRUE for inserting the instrumentation code before
- ITER, FALSE for inserting it after ITER.
+ ITER, FALSE for inserting it after ITER. IS_SCALAR_ACCESS is TRUE
+ for a scalar memory access and FALSE for memory region access.
+ NON_ZERO_P is TRUE if memory region is guaranteed to have non-zero
+ length. ALIGN tells alignment of accessed memory object.
+
+ START_INSTRUMENTED and END_INSTRUMENTED are TRUE if start/end of
+ memory region have already been instrumented.
If BEFORE_P is TRUE, *ITER is arranged to still point to the
statement it was pointing to prior to calling this function,
otherwise, it points to the statement logically following it. */
static void
-build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
- bool before_p, bool is_store, HOST_WIDE_INT size_in_bytes,
- bool slow_p = false)
+build_check_stmt (location_t location, tree base, tree len,
+ HOST_WIDE_INT size_in_bytes, gimple_stmt_iterator *iter,
+ bool non_zero_len_p, bool before_p, bool is_store,
+ bool is_scalar_access, unsigned int align = 0,
+ bool start_instrumented = false,
+ bool end_instrumented = false)
{
- gimple_stmt_iterator gsi;
- basic_block then_bb, else_bb;
- tree t, base_addr, shadow;
+ gimple_stmt_iterator gsi = *iter;
gimple g;
- tree shadow_ptr_type = shadow_ptr_types[size_in_bytes == 16 ? 1 : 0];
- tree shadow_type = TREE_TYPE (shadow_ptr_type);
tree uintptr_type
= build_nonstandard_integer_type (TYPE_PRECISION (TREE_TYPE (base)), 1);
- tree base_ssa = base;
- HOST_WIDE_INT real_size_in_bytes = size_in_bytes;
- tree sz_arg = NULL_TREE;
-
- if (size_in_bytes == 1)
- slow_p = false;
- else if ((size_in_bytes & (size_in_bytes - 1)) != 0
- || size_in_bytes > 16
- || slow_p)
+
+ gcc_assert (!(size_in_bytes > 0 && !non_zero_len_p));
+
+ if (len)
+ {
+ len = unshare_expr (len);
+ }
+ else
{
- real_size_in_bytes = 1;
- slow_p = true;
+ gcc_assert (size_in_bytes != -1);
+ len = build_int_cst (pointer_sized_int_node, size_in_bytes);
+ }
+
+ if (size_in_bytes > 1)
+ {
+ if ((size_in_bytes & (size_in_bytes - 1)) != 0
+ || size_in_bytes > 16)
+ {
+ size_in_bytes = -1;
+ }
+ else if (align && align < size_in_bytes * BITS_PER_UNIT)
+ {
+ /* On non-strict alignment targets, if
+ 16-byte access is just 8-byte aligned,
+ this will result in misaligned shadow
+ memory 2 byte load, but otherwise can
+ be handled using one read. */
+ if (size_in_bytes != 16
+ || STRICT_ALIGNMENT
+ || align < 8 * BITS_PER_UNIT)
+ {
+ size_in_bytes = -1;
+ }
+ }
+ }
+
+ HOST_WIDE_INT real_size_in_bytes = size_in_bytes == -1 ? 1 : size_in_bytes;
+
+ tree shadow_ptr_type = shadow_ptr_types[real_size_in_bytes == 16 ? 1 : 0];
+ tree shadow_type = TREE_TYPE (shadow_ptr_type);
+
+ base = unshare_expr (base);
+
+ if (use_calls_p ())
+ {
+ gsi = *iter;
+ build_check_stmt_with_calls (location, base, len, size_in_bytes, iter,
+ before_p, is_store, is_scalar_access);
+ return;
+ }
+
+ ++asan_num_accesses;
+
+ if (!non_zero_len_p)
+ {
+ gcc_assert (before_p);
+
+ /* So, the length of the memory area to asan-protect is
+ non-constant. Let's guard the generated instrumentation code
+ like:
+
+ if (len != 0)
+ {
+ //asan instrumentation code goes here.
+ }
+ // falltrough instructions, starting with *ITER. */
+
+ g = gimple_build_cond (NE_EXPR,
+ len,
+ build_int_cst (TREE_TYPE (len), 0),
+ NULL_TREE, NULL_TREE);
+ gimple_set_location (g, location);
+
+ basic_block then_bb, fallthrough_bb;
+ insert_if_then_before_iter (g, iter, /*then_more_likely_p=*/true,
+ &then_bb, &fallthrough_bb);
+ /* Note that fallthrough_bb starts with the statement that was
+ pointed to by ITER. */
+
+ /* The 'then block' of the 'if (len != 0) condition is where
+ we'll generate the asan instrumentation code now. */
+ gsi = gsi_last_bb (then_bb);
+ build_check_stmt (location, base, len, size_in_bytes, &gsi,
+ /*non_zero_len_p*/true, /*before_p*/true, is_store,
+ is_scalar_access, align,
+ start_instrumented, end_instrumented);
+ return;
}
/* Get an iterator on the point where we can add the condition
statement for the instrumentation. */
- gsi = create_cond_insert_point (iter, before_p,
+ basic_block then_bb, else_bb;
+ gsi = create_cond_insert_point (&gsi, before_p,
/*then_more_likely_p=*/false,
/*create_then_fallthru_edge=*/false,
&then_bb,
&else_bb);
- base = unshare_expr (base);
-
- /* BASE can already be an SSA_NAME; in that case, do not create a
- new SSA_NAME for it. */
- if (TREE_CODE (base) != SSA_NAME)
- {
- g = gimple_build_assign_with_ops (TREE_CODE (base),
- make_ssa_name (TREE_TYPE (base), NULL),
- base, NULL_TREE);
- gimple_set_location (g, location);
- gsi_insert_after (&gsi, g, GSI_NEW_STMT);
- base_ssa = gimple_assign_lhs (g);
- }
+ tree base_ssa = maybe_create_ssa_name (location, base, &gsi,
+ /*before_p*/false);
g = gimple_build_assign_with_ops (NOP_EXPR,
make_ssa_name (uintptr_type, NULL),
base_ssa, NULL_TREE);
gimple_set_location (g, location);
gsi_insert_after (&gsi, g, GSI_NEW_STMT);
- base_addr = gimple_assign_lhs (g);
-
- /* Build
- (base_addr >> ASAN_SHADOW_SHIFT) + targetm.asan_shadow_offset (). */
- shadow = build_shadow_mem_access (&gsi, location, base_addr,
- shadow_ptr_type);
+ tree base_addr = gimple_assign_lhs (g);
- if (real_size_in_bytes < 8)
+ tree t;
+ if (real_size_in_bytes >= 8)
{
- /* Slow path for 1, 2 and 4 byte accesses.
- Test (shadow != 0)
- & ((base_addr & 7) + (real_size_in_bytes - 1)) >= shadow). */
- gimple_seq seq = NULL;
- gimple shadow_test = build_assign (NE_EXPR, shadow, 0);
- gimple_seq_add_stmt (&seq, shadow_test);
- gimple_seq_add_stmt (&seq, build_assign (BIT_AND_EXPR, base_addr, 7));
- gimple_seq_add_stmt (&seq, build_type_cast (shadow_type,
- gimple_seq_last (seq)));
- if (real_size_in_bytes > 1)
- gimple_seq_add_stmt (&seq,
- build_assign (PLUS_EXPR, gimple_seq_last (seq),
- real_size_in_bytes - 1));
- gimple_seq_add_stmt (&seq, build_assign (GE_EXPR, gimple_seq_last (seq),
- shadow));
- gimple_seq_add_stmt (&seq, build_assign (BIT_AND_EXPR, shadow_test,
- gimple_seq_last (seq)));
- t = gimple_assign_lhs (gimple_seq_last (seq));
- gimple_seq_set_location (seq, location);
- gsi_insert_seq_after (&gsi, seq, GSI_CONTINUE_LINKING);
- /* For weird access sizes or misaligned, check first and last byte. */
- if (slow_p)
+ tree shadow = build_shadow_mem_access (&gsi, location, base_addr,
+ shadow_ptr_type);
+ t = shadow;
+ }
+ else
+ {
+ /* Slow path for 1, 2 and 4 byte accesses. */
+
+ if (!start_instrumented)
+ {
+ /* Test (shadow != 0)
+ & ((base_addr & 7) + (real_size_in_bytes - 1)) >= shadow). */
+ tree shadow = build_shadow_mem_access (&gsi, location, base_addr,
+ shadow_ptr_type);
+ gimple shadow_test = build_assign (NE_EXPR, shadow, 0);
+ gimple_seq seq = NULL;
+ gimple_seq_add_stmt (&seq, shadow_test);
+ gimple_seq_add_stmt (&seq, build_assign (BIT_AND_EXPR, base_addr, 7));
+ gimple_seq_add_stmt (&seq, build_type_cast (shadow_type,
+ gimple_seq_last (seq)));
+ if (real_size_in_bytes > 1)
+ gimple_seq_add_stmt (&seq,
+ build_assign (PLUS_EXPR, gimple_seq_last (seq),
+ real_size_in_bytes - 1));
+ gimple_seq_add_stmt (&seq, build_assign (GE_EXPR,
+ gimple_seq_last (seq),
+ shadow));
+ gimple_seq_add_stmt (&seq, build_assign (BIT_AND_EXPR, shadow_test,
+ gimple_seq_last (seq)));
+ t = gimple_assign_lhs (gimple_seq_last (seq));
+ gimple_seq_set_location (seq, location);
+ gsi_insert_seq_after (&gsi, seq, GSI_CONTINUE_LINKING);
+ }
+
+ /* For non-constant, misaligned or otherwise weird access sizes,
+ check first and last byte. */
+ if (size_in_bytes == -1 && !end_instrumented)
{
+ g = gimple_build_assign_with_ops (MINUS_EXPR,
+ make_ssa_name (uintptr_type, NULL),
+ len,
+ build_int_cst (uintptr_type, 1));
+ gimple_set_location (g, location);
+ gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+ tree last = gimple_assign_lhs (g);
g = gimple_build_assign_with_ops (PLUS_EXPR,
make_ssa_name (uintptr_type, NULL),
base_addr,
- build_int_cst (uintptr_type,
- size_in_bytes - 1));
+ last);
gimple_set_location (g, location);
gsi_insert_after (&gsi, g, GSI_NEW_STMT);
tree base_end_addr = gimple_assign_lhs (g);
- shadow = build_shadow_mem_access (&gsi, location, base_end_addr,
+ tree shadow = build_shadow_mem_access (&gsi, location, base_end_addr,
shadow_ptr_type);
- seq = NULL;
- shadow_test = build_assign (NE_EXPR, shadow, 0);
+ gimple shadow_test = build_assign (NE_EXPR, shadow, 0);
+ gimple_seq seq = NULL;
gimple_seq_add_stmt (&seq, shadow_test);
gimple_seq_add_stmt (&seq, build_assign (BIT_AND_EXPR,
base_end_addr, 7));
@@ -1616,16 +1810,14 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
shadow));
gimple_seq_add_stmt (&seq, build_assign (BIT_AND_EXPR, shadow_test,
gimple_seq_last (seq)));
- gimple_seq_add_stmt (&seq, build_assign (BIT_IOR_EXPR, t,
- gimple_seq_last (seq)));
+ if (!start_instrumented)
+ gimple_seq_add_stmt (&seq, build_assign (BIT_IOR_EXPR, t,
+ gimple_seq_last (seq)));
t = gimple_assign_lhs (gimple_seq_last (seq));
gimple_seq_set_location (seq, location);
gsi_insert_seq_after (&gsi, seq, GSI_CONTINUE_LINKING);
- sz_arg = build_int_cst (pointer_sized_int_node, size_in_bytes);
}
}
- else
- t = shadow;
g = gimple_build_cond (NE_EXPR, t, build_int_cst (TREE_TYPE (t), 0),
NULL_TREE, NULL_TREE);
@@ -1634,8 +1826,23 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
/* Generate call to the run-time library (e.g. __asan_report_load8). */
gsi = gsi_start_bb (then_bb);
- g = gimple_build_call (report_error_func (is_store, size_in_bytes, slow_p),
- sz_arg ? 2 : 1, base_addr, sz_arg);
+ int nargs;
+ tree fun = report_error_func (is_store, is_scalar_access ? size_in_bytes : -1,
+ &nargs);
+ if (nargs == 1)
+ g = gimple_build_call (fun, 1, base_addr);
+ else
+ {
+ gcc_assert (nargs == 2);
+ g = gimple_build_assign_with_ops (NOP_EXPR,
+ make_ssa_name (pointer_sized_int_node,
+ NULL),
+ len, NULL_TREE);
+ gimple_set_location (g, location);
+ gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+ tree sz_arg = gimple_assign_lhs (g);
+ g = gimple_build_call (fun, nargs, base_addr, sz_arg);
+ }
gimple_set_location (g, location);
gsi_insert_after (&gsi, g, GSI_NEW_STMT);
@@ -1730,31 +1937,10 @@ instrument_derefs (gimple_stmt_iterator *iter, tree t,
base = build_fold_addr_expr (t);
if (!has_mem_ref_been_instrumented (base, size_in_bytes))
{
- bool slow_p = false;
- if (size_in_bytes > 1)
- {
- if ((size_in_bytes & (size_in_bytes - 1)) != 0
- || size_in_bytes > 16)
- slow_p = true;
- else
- {
- unsigned int align = get_object_alignment (t);
- if (align < size_in_bytes * BITS_PER_UNIT)
- {
- /* On non-strict alignment targets, if
- 16-byte access is just 8-byte aligned,
- this will result in misaligned shadow
- memory 2 byte load, but otherwise can
- be handled using one read. */
- if (size_in_bytes != 16
- || STRICT_ALIGNMENT
- || align < 8 * BITS_PER_UNIT)
- slow_p = true;
- }
- }
- }
- build_check_stmt (location, base, iter, /*before_p=*/true,
- is_store, size_in_bytes, slow_p);
+ unsigned int align = get_object_alignment (t);
+ build_check_stmt (location, base, NULL_TREE, size_in_bytes, iter,
+ /*non_zero_len_p*/size_in_bytes > 0, /*before_p=*/true,
+ is_store, /*is_scalar_access*/true, align);
update_mem_ref_hash_table (base, size_in_bytes);
update_mem_ref_hash_table (t, size_in_bytes);
}
@@ -1779,10 +1965,6 @@ instrument_mem_region_access (tree base, tree len,
|| integer_zerop (len))
return;
- gimple_stmt_iterator gsi = *iter;
-
- basic_block fallthrough_bb = NULL, then_bb = NULL;
-
/* If the beginning of the memory region has already been
instrumented, do not instrument it. */
bool start_instrumented = has_mem_ref_been_instrumented (base, 1);
@@ -1792,130 +1974,15 @@ instrument_mem_region_access (tree base, tree len,
tree end = asan_mem_ref_get_end (base, len);
bool end_instrumented = has_mem_ref_been_instrumented (end, 1);
- if (start_instrumented && end_instrumented)
- return;
+ HOST_WIDE_INT size_in_bytes = tree_fits_shwi_p (len) ? tree_to_shwi (len) : -1;
- if (!is_gimple_constant (len))
- {
- /* So, the length of the memory area to asan-protect is
- non-constant. Let's guard the generated instrumentation code
- like:
+ build_check_stmt (location, base, len, size_in_bytes, iter,
+ /*non_zero_len_p*/size_in_bytes > 0, /*before_p*/true,
+ is_store, /*is_scalar_access*/false, /*align*/0,
+ start_instrumented, end_instrumented);
- if (len != 0)
- {
- //asan instrumentation code goes here.
- }
- // falltrough instructions, starting with *ITER. */
-
- gimple g = gimple_build_cond (NE_EXPR,
- len,
- build_int_cst (TREE_TYPE (len), 0),
- NULL_TREE, NULL_TREE);
- gimple_set_location (g, location);
- insert_if_then_before_iter (g, iter, /*then_more_likely_p=*/true,
- &then_bb, &fallthrough_bb);
- /* Note that fallthrough_bb starts with the statement that was
- pointed to by ITER. */
-
- /* The 'then block' of the 'if (len != 0) condition is where
- we'll generate the asan instrumentation code now. */
- gsi = gsi_last_bb (then_bb);
- }
-
- if (!start_instrumented)
- {
- /* Instrument the beginning of the memory region to be accessed,
- and arrange for the rest of the intrumentation code to be
- inserted in the then block *after* the current gsi. */
- build_check_stmt (location, base, &gsi, /*before_p=*/true, is_store, 1);
-
- if (then_bb)
- /* We are in the case where the length of the region is not
- constant; so instrumentation code is being generated in the
- 'then block' of the 'if (len != 0) condition. Let's arrange
- for the subsequent instrumentation statements to go in the
- 'then block'. */
- gsi = gsi_last_bb (then_bb);
- else
- {
- *iter = gsi;
- /* Don't remember this access as instrumented, if length
- is unknown. It might be zero and not being actually
- instrumented, so we can't rely on it being instrumented. */
- update_mem_ref_hash_table (base, 1);
- }
- }
-
- if (end_instrumented)
- return;
-
- /* We want to instrument the access at the end of the memory region,
- which is at (base + len - 1). */
-
- /* offset = len - 1; */
- len = unshare_expr (len);
- tree offset;
- gimple_seq seq = NULL;
- if (TREE_CODE (len) == INTEGER_CST)
- offset = fold_build2 (MINUS_EXPR, size_type_node,
- fold_convert (size_type_node, len),
- build_int_cst (size_type_node, 1));
- else
- {
- gimple g;
- tree t;
-
- if (TREE_CODE (len) != SSA_NAME)
- {
- t = make_ssa_name (TREE_TYPE (len), NULL);
- g = gimple_build_assign_with_ops (TREE_CODE (len), t, len, NULL);
- gimple_set_location (g, location);
- gimple_seq_add_stmt_without_update (&seq, g);
- len = t;
- }
- if (!useless_type_conversion_p (size_type_node, TREE_TYPE (len)))
- {
- t = make_ssa_name (size_type_node, NULL);
- g = gimple_build_assign_with_ops (NOP_EXPR, t, len, NULL);
- gimple_set_location (g, location);
- gimple_seq_add_stmt_without_update (&seq, g);
- len = t;
- }
-
- t = make_ssa_name (size_type_node, NULL);
- g = gimple_build_assign_with_ops (MINUS_EXPR, t, len,
- build_int_cst (size_type_node, 1));
- gimple_set_location (g, location);
- gimple_seq_add_stmt_without_update (&seq, g);
- offset = gimple_assign_lhs (g);
- }
-
- /* _1 = base; */
- base = unshare_expr (base);
- gimple region_end =
- gimple_build_assign_with_ops (TREE_CODE (base),
- make_ssa_name (TREE_TYPE (base), NULL),
- base, NULL);
- gimple_set_location (region_end, location);
- gimple_seq_add_stmt_without_update (&seq, region_end);
-
- /* _2 = _1 + offset; */
- region_end =
- gimple_build_assign_with_ops (POINTER_PLUS_EXPR,
- make_ssa_name (TREE_TYPE (base), NULL),
- gimple_assign_lhs (region_end),
- offset);
- gimple_set_location (region_end, location);
- gimple_seq_add_stmt_without_update (&seq, region_end);
- gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
-
- /* instrument access at _2; */
- gsi = gsi_for_stmt (region_end);
- build_check_stmt (location, gimple_assign_lhs (region_end),
- &gsi, /*before_p=*/false, is_store, 1);
-
- if (then_bb == NULL)
- update_mem_ref_hash_table (end, 1);
+ update_mem_ref_hash_table (base, 1);
+ update_mem_ref_hash_table (end, 1);
*iter = gsi_for_stmt (gsi_stmt (*iter));
}
@@ -1956,47 +2023,30 @@ instrument_strlen_call (gimple_stmt_iterator *iter)
location_t loc = gimple_location (call);
tree str_arg = gimple_call_arg (call, 0);
- /* Instrument the access to the first byte of str_arg. i.e:
-
- _1 = str_arg; instrument (_1); */
tree cptr_type = build_pointer_type (char_type_node);
gimple str_arg_ssa =
gimple_build_assign_with_ops (NOP_EXPR,
- make_ssa_name (cptr_type, NULL),
- str_arg, NULL);
+ make_ssa_name (cptr_type, NULL),
+ str_arg, NULL);
gimple_set_location (str_arg_ssa, loc);
- gimple_stmt_iterator gsi = *iter;
- gsi_insert_before (&gsi, str_arg_ssa, GSI_NEW_STMT);
- build_check_stmt (loc, gimple_assign_lhs (str_arg_ssa), &gsi,
- /*before_p=*/false, /*is_store=*/false, 1);
-
- /* If we initially had an instruction like:
-
- int n = strlen (str)
+ gsi_insert_before (iter, str_arg_ssa, GSI_SAME_STMT);
- we now want to instrument the access to str[n], after the
- instruction above.*/
+ build_check_stmt (loc, gimple_assign_lhs (str_arg_ssa), NULL_TREE, 1, iter,
+ /*non_zero_len_p*/true, /*before_p=*/true,
+ /*is_store=*/false, /*is_scalar_access*/false, /*align*/0);
- /* So let's build the access to str[n] that is, access through the
- pointer_plus expr: (_1 + len). */
gimple stmt =
- gimple_build_assign_with_ops (POINTER_PLUS_EXPR,
- make_ssa_name (cptr_type, NULL),
- gimple_assign_lhs (str_arg_ssa),
- len);
+ gimple_build_assign_with_ops (PLUS_EXPR,
+ make_ssa_name (TREE_TYPE (len), NULL),
+ len,
+ build_int_cst (TREE_TYPE (len), 1));
gimple_set_location (stmt, loc);
- gsi_insert_after (&gsi, stmt, GSI_NEW_STMT);
-
- build_check_stmt (loc, gimple_assign_lhs (stmt), &gsi,
- /*before_p=*/false, /*is_store=*/false, 1);
-
- /* Ensure that iter points to the statement logically following the
- one it was initially pointing to. */
- *iter = gsi;
- /* As *ITER has been advanced to point to the next statement, let's
- return true to inform transform_statements that it shouldn't
- advance *ITER anymore; otherwises it will skip that next
- statement, which wouldn't be instrumented. */
+ gsi_insert_after (iter, stmt, GSI_NEW_STMT);
+
+ build_check_stmt (loc, gimple_assign_lhs (stmt), len, 1, iter,
+ /*non_zero_len_p*/true, /*before_p=*/false,
+ /*is_store=*/false, /*is_scalar_access*/false, /*align*/0);
+
return true;
}
@@ -2568,6 +2618,7 @@ asan_instrument (void)
{
if (shadow_ptr_types[0] == NULL_TREE)
asan_init_shadow_ptr_types ();
+ asan_num_accesses = 0;
transform_statements ();
return 0;
}
diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c
index 7c24a6d..e857543 100644
--- a/gcc/cfgcleanup.c
+++ b/gcc/cfgcleanup.c
@@ -1175,7 +1175,7 @@ old_insns_match_p (int mode ATTRIBUTE_UNUSED, rtx i1, rtx i2)
&& DECL_FUNCTION_CODE (SYMBOL_REF_DECL (symbol))
>= BUILT_IN_ASAN_REPORT_LOAD1
&& DECL_FUNCTION_CODE (SYMBOL_REF_DECL (symbol))
- <= BUILT_IN_ASAN_REPORT_STORE16)
+ <= BUILT_IN_ASAN_STOREN)
return dir_none;
}
}
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 9475594..96d6895 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10230,6 +10230,12 @@ is enabled by default when using @option{-fsanitize=address} option.
To disable use-after-return detection use
@option{--param asan-use-after-return=0}.
+@item asan-instrumentation-with-call-threshold
+Once number of memory accesses in function becomes greater
+or equal than this number, use callbacks instead of
+generating inline code. E.g. to disable inline code use
+@option{--param asan-instrumentation-with-call-threshold=0}.
+
@end table
@end table
diff --git a/gcc/params.def b/gcc/params.def
index c3a8797..28ef79a 100644
--- a/gcc/params.def
+++ b/gcc/params.def
@@ -1090,6 +1090,12 @@ DEFPARAM (PARAM_ASAN_USE_AFTER_RETURN,
"Enable asan builtin functions protection",
1, 0, 1)
+DEFPARAM (PARAM_ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD,
+ "asan-instrumentation-with-call-threshold",
+ "Use callbacks instead of inline code once number of accesses "
+ " in function becomes greater or equal than this threshold",
+ 10000, 0, INT_MAX)
+
DEFPARAM (PARAM_UNINIT_CONTROL_DEP_ATTEMPTS,
"uninit-control-dep-attempts",
"Maximum number of nested calls to search for control dependencies "
diff --git a/gcc/params.h b/gcc/params.h
index 0d6daa2..d488e32 100644
--- a/gcc/params.h
+++ b/gcc/params.h
@@ -232,5 +232,7 @@ extern void init_param_values (int *params);
PARAM_VALUE (PARAM_ASAN_MEMINTRIN)
#define ASAN_USE_AFTER_RETURN \
PARAM_VALUE (PARAM_ASAN_USE_AFTER_RETURN)
+#define ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD \
+ PARAM_VALUE (PARAM_ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD)
#endif /* ! GCC_PARAMS_H */
diff --git a/gcc/sanitizer.def b/gcc/sanitizer.def
index 4016fc5..b4af164 100644
--- a/gcc/sanitizer.def
+++ b/gcc/sanitizer.def
@@ -29,7 +29,7 @@ along with GCC; see the file COPYING3. If not see
/* Address Sanitizer */
DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_INIT, "__asan_init_v3",
BT_FN_VOID, ATTR_NOTHROW_LEAF_LIST)
-/* Do not reorder the BUILT_IN_ASAN_REPORT* builtins, e.g. cfgcleanup.c
+/* Do not reorder the BUILT_IN_ASAN_{REPORT,CHECK}* builtins, e.g. cfgcleanup.c
relies on this order. */
DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REPORT_LOAD1, "__asan_report_load1",
BT_FN_VOID_PTR, ATTR_TMPURE_NORETURN_NOTHROW_LEAF_LIST)
@@ -57,6 +57,30 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REPORT_STORE16, "__asan_report_store16",
DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REPORT_STORE_N, "__asan_report_store_n",
BT_FN_VOID_PTR_PTRMODE,
ATTR_TMPURE_NORETURN_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOAD1, "__asan_load1",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOAD2, "__asan_load2",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOAD4, "__asan_load4",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOAD8, "__asan_load8",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOAD16, "__asan_load16",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOADN, "__asan_loadN",
+ BT_FN_VOID_PTR_PTRMODE, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STORE1, "__asan_store1",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STORE2, "__asan_store2",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STORE4, "__asan_store4",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STORE8, "__asan_store8",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STORE16, "__asan_store16",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STOREN, "__asan_storeN",
+ BT_FN_VOID_PTR_PTRMODE, ATTR_TMPURE_NOTHROW_LEAF_LIST)
DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REGISTER_GLOBALS,
"__asan_register_globals",
BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST)
diff --git a/gcc/testsuite/c-c++-common/asan/instrument-with-calls-1.c b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-1.c
new file mode 100644
index 0000000..80f7620
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-1.c
@@ -0,0 +1,10 @@
+/* { dg-do assemble } */
+/* { dg-options "--param asan-instrumentation-with-call-threshold=0 -save-temps" } */
+
+void f(char *a, int *b) {
+ *b = *a;
+}
+
+/* { dg-final { scan-assembler "__asan_load1" } } */
+/* { dg-final { scan-assembler "__asan_store4" } } */
+/* { dg-final { cleanup-saved-temps } } */
diff --git a/gcc/testsuite/c-c++-common/asan/instrument-with-calls-2.c b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-2.c
new file mode 100644
index 0000000..570f796
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-2.c
@@ -0,0 +1,16 @@
+/* { dg-do assemble } */
+/* { dg-options "--param asan-instrumentation-with-call-threshold=1 -save-temps" } */
+
+int x;
+
+void f(int *a, int *b) {
+ *a = 0;
+ asm volatile ("" ::: "memory");
+ x = *b;
+}
+
+/* { dg-final { scan-assembler-not "__asan_store4" } } */
+/* { dg-final { scan-assembler "__asan_report_store4" } } */
+/* { dg-final { scan-assembler "__asan_load4" } } */
+/* { dg-final { scan-assembler-not "__asan_report_load4" } } */
+/* { dg-final { cleanup-saved-temps } } */
diff --git a/gcc/testsuite/c-c++-common/asan/instrument-with-calls-3.c b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-3.c
new file mode 100644
index 0000000..3712c7a
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-3.c
@@ -0,0 +1,15 @@
+/* { dg-do assemble } */
+/* { dg-options "--param asan-instrumentation-with-call-threshold=0 -save-temps" } */
+
+struct A {
+ char x[7];
+};
+
+void f(struct A *x, struct A *y) {
+ *x = *y;
+}
+
+/* { dg-final { scan-assembler "__asan_loadN" } } */
+/* { dg-final { scan-assembler "__asan_storeN" } } */
+/* { dg-final { cleanup-saved-temps } } */
+
diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c
index fa52e0c..c7c594e 100644
--- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c
+++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c
@@ -16,12 +16,11 @@ test0 ()
tab[0] = 1;
tab[1] = 2;
- /* __builtin___asan_report_load1 called 1 time for the store
- below. */
- char t0 = tab[1];
-
/* This load should not be instrumented because it is to the same
memory location as above. */
+ char t0 = tab[1];
+
+ /* Likewise. */
char t1 = tab[1];
return t0 + t1;
@@ -36,7 +35,7 @@ test1 (int i)
the initialization. */
foo[i] = 1;
- /*__builtin___asan_report_store1 called 2 times here to instrument
+ /*__builtin___asan_report_store_n called once here to instrument
the store to the memory region of tab. */
__builtin_memset (tab, 3, sizeof (tab));
@@ -44,8 +43,8 @@ test1 (int i)
__builtin_memset (tab, 4, sizeof (tab));
__builtin_memset (tab, 5, sizeof (tab));
- /* There are 2 calls to __builtin___asan_report_store1 and 2 calls
- to __builtin___asan_report_load1 to instrument the store to
+ /* There is a call to __builtin___asan_report_store_n and a call
+ to __builtin___asan_report_load_n to instrument the store to
(subset of) the memory region of tab. */
__builtin_memcpy (&tab[1], foo + i, 3);
@@ -53,7 +52,7 @@ test1 (int i)
the reference to tab[1] has been already instrumented above. */
return tab[1];
- /* So for these function, there should be 7 calls to
+ /* So for these functions, there should be 3 calls to
__builtin___asan_report_store1. */
}
@@ -63,6 +62,7 @@ main ()
return test0 () && test1 (0);
}
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store1" 7 "asan0" } } */
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load" 2 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store1" 3 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 2 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load" 1 "asan0" } } */
/* { dg-final { cleanup-tree-dump "asan0" } } */
diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-2.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-2.c
index 28525e0..143312f 100644
--- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-2.c
+++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-2.c
@@ -20,6 +20,7 @@ main ()
__builtin_memset (tab, 1, 3);
}
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store1" 3 "asan0" } } */
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report" 3 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "& 7" 3 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 2 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report" 2 "asan0" } } */
/* { dg-final { cleanup-tree-dump "asan0" } } */
diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-4.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-4.c
index b2e7284..da91cd5 100644
--- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-4.c
+++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-4.c
@@ -5,9 +5,13 @@
void
foo (int *a, char *b, char *c)
{
+ /* One check for c[0], one check for a[], one check for c, two checks for b. */
__builtin_memmove (c, b, a[c[0]]);
+ /* For a total of 5 checks. */
}
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 3 "asan0" } } */
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store1" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "& 7" 5 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load_n" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 1 "asan0" } } */
/* { dg-final { cleanup-tree-dump "asan0" } } */
diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-5.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-5.c
index ead3f58..134be66 100644
--- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-5.c
+++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-5.c
@@ -5,9 +5,14 @@
void
foo (int *a, char *b, char *c)
{
+ /* One check for b[0], one check for a[], 2 checks for c and one checks for b. */
__builtin_memmove (c, b, a[b[0]]);
+ /* For a total of 5 checks. */
}
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 2 "asan0" } } */
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store1" 2 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "& 7" 5 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load4" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load_n" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 1 "asan0" } } */
/* { dg-final { cleanup-tree-dump "asan0" } } */
diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-6.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-6.c
index e4691bc..6e7ed25 100644
--- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-6.c
+++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-6.c
@@ -5,10 +5,16 @@
void
foo (int *a, char *b, char *c)
{
+ /* One check for c[0], one check for a[], one check for c and 2 checks for b. */
__builtin_memmove (c, b, a[c[0]]);
+ /* One check for a[], one check for c and one check for b. */
__builtin_memmove (c, b, a[b[0]]);
+ /* For a total of 8 checks. */
}
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 5 "asan0" } } */
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store1" 2 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "& 7" 8 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load4" 2 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load_n" 2 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 2 "asan0" } } */
/* { dg-final { cleanup-tree-dump "asan0" } } */
diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c
index bf40a03..a04956d 100644
--- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c
+++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c
@@ -13,11 +13,15 @@ struct S
int
foo (int *a, char *b, char *c)
{
+ /* 2 checks for s.a, 2 checks for e. */
int d = __builtin_memcmp (s.a, e, 100);
+ /* One check for s.a and one check for e. */
d += __builtin_memcmp (s.a, e, 200);
+ /* For a total of 6 checks. */
return d;
}
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load" 6 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "& 7" 6 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load_n" 4 "asan0" } } */
/* { dg-final { scan-tree-dump-not "__builtin___asan_report_store" "asan0" } } */
/* { dg-final { cleanup-tree-dump "asan0" } } */
diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-8.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-8.c
index 38ea7a2..308a133 100644
--- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-8.c
+++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-8.c
@@ -5,10 +5,16 @@
char
foo (int *a, char *b, char *c)
{
+ /* One check for b[0], one check for a[], two checks for c and one check for b. */
__builtin_memmove (c, b, a[b[0]]);
+ /* No checks here. */
return c[0] + b[0];
+ /* For a total of 5 checks. */
}
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 3 "asan0" } } */
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store1" 2 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "& 7" 5 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load4" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load_n" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 1 "asan0" } } */
/* { dg-final { cleanup-tree-dump "asan0" } } */
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-09 15:48 ` Yury Gribov
@ 2014-06-11 9:32 ` Jakub Jelinek
2014-06-11 12:53 ` Yury Gribov
0 siblings, 1 reply; 22+ messages in thread
From: Jakub Jelinek @ 2014-06-11 9:32 UTC (permalink / raw)
To: Yury Gribov
Cc: GCC Patches, Konstantin Serebryany, Viacheslav Garbuzov, Dmitry Vyukov
On Mon, Jun 09, 2014 at 07:48:46PM +0400, Yury Gribov wrote:
> >>Build_check_stmt is now unified for const/variable lengths
> >>but I'd still prefer to treat the use_calls case specially
> >>because calling single __asan_loadN is more space-efficient
> >>than emitting two separate calls with a length check.
> >
> >That is not what I meant.
> >1) instrument_mem_region_access/instrument_strlen should use a single
> > build_check_stmt call instead of two that they do now,
> > both for use_calls case and !use_calls case, the difference
> > is just that instrument_mem_region_access needs to guard
> > it against len == 0, while for use_calls it doesn't have to,
> > it seems __asan_{load,store}N handles length of 0 correctly
> > (still, the caller shouldn't update_mem_ref_hash_table
> > if length might be zero, even in the use_calls case)
> >2) passing always a tree length to build_check_stmt and then
> > recreating size_in_bytes out of it is ugly, just add a new
> > tree parameter, if it is non-NULL, the length is variable,
> > and real_size_in_bytes should be 1, and the code additionally
> > has to compute length - 1 at runtime and add the result,
> > otherwise it just keeps adding precomputed constant
>
> Let's check if I'm on the right way. I've attached current version. It may
> have some rough edges but at least all regtests pass on x64.
The plan (we had already for 4.9, but didn't get to that yet) is in the end
not to lower the checks in asan pass that much, and lower it in sanopt
pass later on after performing some inter-bb optimizations.
The plan has been to use internal builtins, but the __builtin___asan_loadX
etc. perhaps could serve the same purpose, with the only problem being that
__asan_loadN handles zero, while the inline expansion thereof doesn't unless
it explicitly checks for zero value. But don't want to hold your patch
for that. Once we switch to that model, we'd simply "inline" the __asan_*
builtins in the sanopt pass until we hit the limit.
The reason for the plan is that it will be easier in the sanopt pass to
try to remove redundant instrumentation, e.g. when dominator bb already
checks a particular address/length and no calls that could change the
validity of the checks happen between the dominator bb and the dominated
__asan_load/storeX.
> Some obvious problems:
>
> 1) build_check_stmt argument list has become really long and messy;
> (this could be simplified to some extent if we get rid of size_of_bytes and
> non_zero_len_p and recompute them from len instead
> but all other parameters seem really unavoidable)
I think that is fine.
> 2) we can't easily check number of instrumentations in
> no-redundant-instrumentation-*.c family of tests
> because memory region accesses now have single error handler
> (__asan_report_load_n/store_n);
> the best solution I came up with is counting all 'X & 7' patterns.
Ok.
> 3) strlen instrumentation is still using two separate calls to
> build_check_stmt (due to reasons I mentioned above).
If for strlen we check before the call the first byte and after the
call the last byte, then indeed we want two calls. Or we could check
everything after the call and use one call after it.
> +static void
> +build_check_stmt_with_calls (location_t loc, tree base, tree len,
> + HOST_WIDE_INT size_in_bytes, gimple_stmt_iterator *iter,
> + bool before_p, bool is_store, bool is_scalar_access)
Whitespace. Please use tabs instead of spaces, and align the lines below
location_t.
> @@ -1501,111 +1608,198 @@ build_shadow_mem_access (gimple_stmt_iterator *gsi, location_t location,
> SSA_NAME, or a non-SSA expression. LOCATION is the source code
> location. IS_STORE is TRUE for a store, FALSE for a load.
> BEFORE_P is TRUE for inserting the instrumentation code before
> - ITER, FALSE for inserting it after ITER.
> + ITER, FALSE for inserting it after ITER. IS_SCALAR_ACCESS is TRUE
> + for a scalar memory access and FALSE for memory region access.
> + NON_ZERO_P is TRUE if memory region is guaranteed to have non-zero
> + length. ALIGN tells alignment of accessed memory object.
> +
> + START_INSTRUMENTED and END_INSTRUMENTED are TRUE if start/end of
> + memory region have already been instrumented.
>
> If BEFORE_P is TRUE, *ITER is arranged to still point to the
> statement it was pointing to prior to calling this function,
> otherwise, it points to the statement logically following it. */
>
> static void
> -build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
> - bool before_p, bool is_store, HOST_WIDE_INT size_in_bytes,
> - bool slow_p = false)
> +build_check_stmt (location_t location, tree base, tree len,
> + HOST_WIDE_INT size_in_bytes, gimple_stmt_iterator *iter,
> + bool non_zero_len_p, bool before_p, bool is_store,
> + bool is_scalar_access, unsigned int align = 0,
> + bool start_instrumented = false,
> + bool end_instrumented = false)
Whitespace again. Also, when it is one call or two minor checks in one
build_check_stmt call, we shouldn't record in the hash table start and end,
just the start, plus for NULL len also size_in_bytes, for non-NULL len
probably nothing, we don't know if len isn't zero.
> +
> + if (len)
> + {
> + len = unshare_expr (len);
> + }
Formatting. Single line blocks shouldn't have {}s around it (several
times).
Jakub
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-11 9:32 ` Jakub Jelinek
@ 2014-06-11 12:53 ` Yury Gribov
2014-06-16 8:23 ` Jakub Jelinek
0 siblings, 1 reply; 22+ messages in thread
From: Yury Gribov @ 2014-06-11 12:53 UTC (permalink / raw)
To: Jakub Jelinek
Cc: GCC Patches, Konstantin Serebryany, Viacheslav Garbuzov,
Dmitry Vyukov, Yuri Gribov
[-- Attachment #1: Type: text/plain, Size: 1653 bytes --]
On 06/11/2014 01:31 PM, Jakub Jelinek wrote:
>The plan (we had already for 4.9, but didn't get to that yet) is in the end
> not to lower the checks in asan pass that much, and lower it in sanopt
> pass later on after performing some inter-bb optimizations.
> ...
> The reason for the plan is that it will be easier in the sanopt pass to
> try to remove redundant instrumentation, e.g. when dominator bb already
> checks a particular address/length and no calls that could change the
> validity of the checks happen between the dominator bb and the dominated
> __asan_load/storeX.
Sounds great, should help get rid of many useless checks.
Do you guys need a hand with this?
>> 3) strlen instrumentation is still using two separate calls to
>> build_check_stmt (due to reasons I mentioned above).
>
> If for strlen we check before the call the first byte and after the
> call the last byte, then indeed we want two calls. Or we could check
> everything after the call and use one call after it.
Well, on one hand delaying the check risks accessing invalid memory.
But this would cause memory protection error worst case so may be fine.
> Whitespace. Please use tabs instead of spaces,
> and align the lines below location_t.
Right, I didn't bother when sending the patch because I mostly worried
about general correctness.
Attached version should have proper formatting.
> Also, when it is one call or two minor checks in one
> build_check_stmt call, we shouldn't record in the hash table start and end,
> just the start, plus for NULL len also size_in_bytes, for non-NULL len
> probably nothing, we don't know if len isn't zero.
Like this?
-Y
[-- Attachment #2: asan_calls_5.diff --]
[-- Type: text/x-diff, Size: 42706 bytes --]
2014-06-11 Yury Gribov <y.gribov@samsung.com>
New asan-instrumentation-with-call-threshold parameter.
gcc/
* asan.c (check_func): New function.
(maybe_create_ssa_name): Likewise.
(build_check_stmt_with_calls): Likewise.
(use_calls_p): Likewise.
(report_error_func): Change interface.
(build_check_stmt): Allow non-integer lengths; add support
for new parameter.
(asan_instrument): Likewise.
(instrument_mem_region_access): Moved code to
build_check_stmt.
(instrument_derefs): Likewise.
(instrument_strlen_call): Likewise.
* cfgcleanup.c (old_insns_match_p): Add support for new
functions.
* doc/invoke.texi: Describe new parameter.
* params.def: Define new parameter.
* params.h: Likewise.
* sanitizer.def: Describe new builtins.
gcc/testsuite/
* c-c++-common/asan/instrument-with-calls-1.c: New test.
* c-c++-common/asan/instrument-with-calls-2.c: Likewise.
* c-c++-common/asan/instrument-with-calls-3.c: Likewise.
* c-c++-common/asan/no-redundant-instrumentation-1.c: Update
test patterns.
* c-c++-common/asan/no-redundant-instrumentation-2.c:
Likewise.
* c-c++-common/asan/no-redundant-instrumentation-4.c:
Likewise.
* c-c++-common/asan/no-redundant-instrumentation-5.c:
Likewise.
* c-c++-common/asan/no-redundant-instrumentation-6.c:
Likewise.
* c-c++-common/asan/no-redundant-instrumentation-7.c:
Likewise.
* c-c++-common/asan/no-redundant-instrumentation-8.c:
Likewise.
diff --git a/gcc/asan.c b/gcc/asan.c
index 3397655..5f0a2de 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -242,6 +242,19 @@ static GTY(()) tree shadow_ptr_types[2];
/* Decl for __asan_option_detect_stack_use_after_return. */
static GTY(()) tree asan_detect_stack_use_after_return;
+/* Number of instrumentations in current function so far. */
+
+static int asan_num_accesses;
+
+/* Check whether we should replace inline instrumentation with calls. */
+
+static inline bool
+use_calls_p ()
+{
+ return ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD < INT_MAX
+ && asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
+}
+
/* Hashtable support for memory references used by gimple
statements. */
@@ -1319,7 +1332,7 @@ asan_protect_global (tree decl)
IS_STORE is either 1 (for a store) or 0 (for a load). */
static tree
-report_error_func (bool is_store, HOST_WIDE_INT size_in_bytes, bool slow_p)
+report_error_func (bool is_store, HOST_WIDE_INT size_in_bytes, int *nargs)
{
static enum built_in_function report[2][6]
= { { BUILT_IN_ASAN_REPORT_LOAD1, BUILT_IN_ASAN_REPORT_LOAD2,
@@ -1328,13 +1341,37 @@ report_error_func (bool is_store, HOST_WIDE_INT size_in_bytes, bool slow_p)
{ BUILT_IN_ASAN_REPORT_STORE1, BUILT_IN_ASAN_REPORT_STORE2,
BUILT_IN_ASAN_REPORT_STORE4, BUILT_IN_ASAN_REPORT_STORE8,
BUILT_IN_ASAN_REPORT_STORE16, BUILT_IN_ASAN_REPORT_STORE_N } };
- if ((size_in_bytes & (size_in_bytes - 1)) != 0
- || size_in_bytes > 16
- || slow_p)
- return builtin_decl_implicit (report[is_store][5]);
+ if (size_in_bytes == -1)
+ {
+ *nargs = 2;
+ return builtin_decl_implicit (report[is_store][5]);
+ }
+ *nargs = 1;
return builtin_decl_implicit (report[is_store][exact_log2 (size_in_bytes)]);
}
+/* Construct a function tree for __asan_{load,store}{1,2,4,8,16,_n}.
+ IS_STORE is either 1 (for a store) or 0 (for a load). */
+
+static tree
+check_func (bool is_store, int size_in_bytes, int *nargs)
+{
+ static enum built_in_function check[2][6]
+ = { { BUILT_IN_ASAN_LOAD1, BUILT_IN_ASAN_LOAD2,
+ BUILT_IN_ASAN_LOAD4, BUILT_IN_ASAN_LOAD8,
+ BUILT_IN_ASAN_LOAD16, BUILT_IN_ASAN_LOADN },
+ { BUILT_IN_ASAN_STORE1, BUILT_IN_ASAN_STORE2,
+ BUILT_IN_ASAN_STORE4, BUILT_IN_ASAN_STORE8,
+ BUILT_IN_ASAN_STORE16, BUILT_IN_ASAN_STOREN } };
+ if (size_in_bytes == -1)
+ {
+ *nargs = 2;
+ return builtin_decl_implicit (check[is_store][5]);
+ }
+ *nargs = 1;
+ return builtin_decl_implicit (check[is_store][exact_log2 (size_in_bytes)]);
+}
+
/* Split the current basic block and create a condition statement
insertion point right before or after the statement pointed to by
ITER. Return an iterator to the point at which the caller might
@@ -1494,6 +1531,76 @@ build_shadow_mem_access (gimple_stmt_iterator *gsi, location_t location,
return gimple_assign_lhs (g);
}
+/* BASE can already be an SSA_NAME; in that case, do not create a
+ new SSA_NAME for it. */
+
+static tree
+maybe_create_ssa_name (location_t loc, tree base, gimple_stmt_iterator *iter,
+ bool before_p)
+{
+ if (TREE_CODE (base) == SSA_NAME)
+ return base;
+ gimple g
+ = gimple_build_assign_with_ops (TREE_CODE (base),
+ make_ssa_name (TREE_TYPE (base), NULL),
+ base, NULL_TREE);
+ gimple_set_location (g, loc);
+ if (before_p)
+ gsi_insert_before (iter, g, GSI_SAME_STMT);
+ else
+ gsi_insert_after (iter, g, GSI_NEW_STMT);
+ return gimple_assign_lhs (g);
+}
+
+/* Instrument the memory access instruction using callbacks.
+ Parameters are similar to BUILD_CHECK_STMT. */
+
+static void
+build_check_stmt_with_calls (location_t loc, tree base, tree len,
+ HOST_WIDE_INT size_in_bytes, gimple_stmt_iterator *iter,
+ bool before_p, bool is_store, bool is_scalar_access)
+{
+ gimple_stmt_iterator gsi = *iter;
+ tree base_ssa = maybe_create_ssa_name (loc, base, &gsi, before_p);
+
+ gimple g
+ = gimple_build_assign_with_ops (NOP_EXPR,
+ make_ssa_name (pointer_sized_int_node, NULL),
+ base_ssa, NULL_TREE);
+ gimple_set_location (g, loc);
+ if (before_p)
+ gsi_insert_before (&gsi, g, GSI_NEW_STMT);
+ else
+ gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+ tree base_addr = gimple_assign_lhs (g);
+
+ int nargs;
+ tree fun
+ = check_func (is_store, is_scalar_access ? size_in_bytes : -1, &nargs);
+ if (nargs == 1)
+ g = gimple_build_call (fun, 1, base_addr);
+ else
+ {
+ gcc_assert (nargs == 2);
+ g = gimple_build_assign_with_ops (NOP_EXPR,
+ make_ssa_name (pointer_sized_int_node,
+ NULL),
+ len, NULL_TREE);
+ gimple_set_location (g, loc);
+ gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+ tree sz_arg = gimple_assign_lhs (g);
+ g = gimple_build_call (fun, nargs, base_addr, sz_arg);
+ }
+ gimple_set_location (g, loc);
+ gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+
+ if (!before_p)
+ {
+ gsi_next (&gsi);
+ *iter = gsi;
+ }
+}
+
/* Instrument the memory access instruction BASE. Insert new
statements before or after ITER.
@@ -1501,111 +1608,192 @@ build_shadow_mem_access (gimple_stmt_iterator *gsi, location_t location,
SSA_NAME, or a non-SSA expression. LOCATION is the source code
location. IS_STORE is TRUE for a store, FALSE for a load.
BEFORE_P is TRUE for inserting the instrumentation code before
- ITER, FALSE for inserting it after ITER.
+ ITER, FALSE for inserting it after ITER. IS_SCALAR_ACCESS is TRUE
+ for a scalar memory access and FALSE for memory region access.
+ NON_ZERO_P is TRUE if memory region is guaranteed to have non-zero
+ length. ALIGN tells alignment of accessed memory object.
+
+ START_INSTRUMENTED and END_INSTRUMENTED are TRUE if start/end of
+ memory region have already been instrumented.
If BEFORE_P is TRUE, *ITER is arranged to still point to the
statement it was pointing to prior to calling this function,
otherwise, it points to the statement logically following it. */
static void
-build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
- bool before_p, bool is_store, HOST_WIDE_INT size_in_bytes,
- bool slow_p = false)
+build_check_stmt (location_t location, tree base, tree len,
+ HOST_WIDE_INT size_in_bytes, gimple_stmt_iterator *iter,
+ bool non_zero_len_p, bool before_p, bool is_store,
+ bool is_scalar_access, unsigned int align = 0,
+ bool start_instrumented = false,
+ bool end_instrumented = false)
{
- gimple_stmt_iterator gsi;
- basic_block then_bb, else_bb;
- tree t, base_addr, shadow;
+ gimple_stmt_iterator gsi = *iter;
gimple g;
- tree shadow_ptr_type = shadow_ptr_types[size_in_bytes == 16 ? 1 : 0];
- tree shadow_type = TREE_TYPE (shadow_ptr_type);
tree uintptr_type
= build_nonstandard_integer_type (TYPE_PRECISION (TREE_TYPE (base)), 1);
- tree base_ssa = base;
- HOST_WIDE_INT real_size_in_bytes = size_in_bytes;
- tree sz_arg = NULL_TREE;
-
- if (size_in_bytes == 1)
- slow_p = false;
- else if ((size_in_bytes & (size_in_bytes - 1)) != 0
- || size_in_bytes > 16
- || slow_p)
+
+ gcc_assert (!(size_in_bytes > 0 && !non_zero_len_p));
+
+ if (len)
+ len = unshare_expr (len);
+ else
+ {
+ gcc_assert (size_in_bytes != -1);
+ len = build_int_cst (pointer_sized_int_node, size_in_bytes);
+ }
+
+ if (size_in_bytes > 1)
{
- real_size_in_bytes = 1;
- slow_p = true;
+ if ((size_in_bytes & (size_in_bytes - 1)) != 0
+ || size_in_bytes > 16)
+ size_in_bytes = -1;
+ else if (align && align < size_in_bytes * BITS_PER_UNIT)
+ {
+ /* On non-strict alignment targets, if
+ 16-byte access is just 8-byte aligned,
+ this will result in misaligned shadow
+ memory 2 byte load, but otherwise can
+ be handled using one read. */
+ if (size_in_bytes != 16
+ || STRICT_ALIGNMENT
+ || align < 8 * BITS_PER_UNIT)
+ size_in_bytes = -1;
+ }
+ }
+
+ HOST_WIDE_INT real_size_in_bytes = size_in_bytes == -1 ? 1 : size_in_bytes;
+
+ tree shadow_ptr_type = shadow_ptr_types[real_size_in_bytes == 16 ? 1 : 0];
+ tree shadow_type = TREE_TYPE (shadow_ptr_type);
+
+ base = unshare_expr (base);
+
+ if (use_calls_p ())
+ {
+ gsi = *iter;
+ build_check_stmt_with_calls (location, base, len, size_in_bytes, iter,
+ before_p, is_store, is_scalar_access);
+ return;
+ }
+
+ ++asan_num_accesses;
+
+ if (!non_zero_len_p)
+ {
+ gcc_assert (before_p);
+
+ /* So, the length of the memory area to asan-protect is
+ non-constant. Let's guard the generated instrumentation code
+ like:
+
+ if (len != 0)
+ {
+ //asan instrumentation code goes here.
+ }
+ // falltrough instructions, starting with *ITER. */
+
+ g = gimple_build_cond (NE_EXPR,
+ len,
+ build_int_cst (TREE_TYPE (len), 0),
+ NULL_TREE, NULL_TREE);
+ gimple_set_location (g, location);
+
+ basic_block then_bb, fallthrough_bb;
+ insert_if_then_before_iter (g, iter, /*then_more_likely_p=*/true,
+ &then_bb, &fallthrough_bb);
+ /* Note that fallthrough_bb starts with the statement that was
+ pointed to by ITER. */
+
+ /* The 'then block' of the 'if (len != 0) condition is where
+ we'll generate the asan instrumentation code now. */
+ gsi = gsi_last_bb (then_bb);
+ build_check_stmt (location, base, len, size_in_bytes, &gsi,
+ /*non_zero_len_p*/true, /*before_p*/true, is_store,
+ is_scalar_access, align,
+ start_instrumented, end_instrumented);
+ return;
}
/* Get an iterator on the point where we can add the condition
statement for the instrumentation. */
- gsi = create_cond_insert_point (iter, before_p,
+ basic_block then_bb, else_bb;
+ gsi = create_cond_insert_point (&gsi, before_p,
/*then_more_likely_p=*/false,
/*create_then_fallthru_edge=*/false,
&then_bb,
&else_bb);
- base = unshare_expr (base);
-
- /* BASE can already be an SSA_NAME; in that case, do not create a
- new SSA_NAME for it. */
- if (TREE_CODE (base) != SSA_NAME)
- {
- g = gimple_build_assign_with_ops (TREE_CODE (base),
- make_ssa_name (TREE_TYPE (base), NULL),
- base, NULL_TREE);
- gimple_set_location (g, location);
- gsi_insert_after (&gsi, g, GSI_NEW_STMT);
- base_ssa = gimple_assign_lhs (g);
- }
+ tree base_ssa = maybe_create_ssa_name (location, base, &gsi,
+ /*before_p*/false);
g = gimple_build_assign_with_ops (NOP_EXPR,
make_ssa_name (uintptr_type, NULL),
base_ssa, NULL_TREE);
gimple_set_location (g, location);
gsi_insert_after (&gsi, g, GSI_NEW_STMT);
- base_addr = gimple_assign_lhs (g);
-
- /* Build
- (base_addr >> ASAN_SHADOW_SHIFT) + targetm.asan_shadow_offset (). */
- shadow = build_shadow_mem_access (&gsi, location, base_addr,
- shadow_ptr_type);
+ tree base_addr = gimple_assign_lhs (g);
- if (real_size_in_bytes < 8)
+ tree t;
+ if (real_size_in_bytes >= 8)
+ {
+ tree shadow = build_shadow_mem_access (&gsi, location, base_addr,
+ shadow_ptr_type);
+ t = shadow;
+ }
+ else
{
- /* Slow path for 1, 2 and 4 byte accesses.
- Test (shadow != 0)
- & ((base_addr & 7) + (real_size_in_bytes - 1)) >= shadow). */
- gimple_seq seq = NULL;
- gimple shadow_test = build_assign (NE_EXPR, shadow, 0);
- gimple_seq_add_stmt (&seq, shadow_test);
- gimple_seq_add_stmt (&seq, build_assign (BIT_AND_EXPR, base_addr, 7));
- gimple_seq_add_stmt (&seq, build_type_cast (shadow_type,
- gimple_seq_last (seq)));
- if (real_size_in_bytes > 1)
- gimple_seq_add_stmt (&seq,
- build_assign (PLUS_EXPR, gimple_seq_last (seq),
- real_size_in_bytes - 1));
- gimple_seq_add_stmt (&seq, build_assign (GE_EXPR, gimple_seq_last (seq),
- shadow));
- gimple_seq_add_stmt (&seq, build_assign (BIT_AND_EXPR, shadow_test,
- gimple_seq_last (seq)));
- t = gimple_assign_lhs (gimple_seq_last (seq));
- gimple_seq_set_location (seq, location);
- gsi_insert_seq_after (&gsi, seq, GSI_CONTINUE_LINKING);
- /* For weird access sizes or misaligned, check first and last byte. */
- if (slow_p)
+ /* Slow path for 1, 2 and 4 byte accesses. */
+
+ if (!start_instrumented)
{
+ /* Test (shadow != 0)
+ & ((base_addr & 7) + (real_size_in_bytes - 1)) >= shadow). */
+ tree shadow = build_shadow_mem_access (&gsi, location, base_addr,
+ shadow_ptr_type);
+ gimple shadow_test = build_assign (NE_EXPR, shadow, 0);
+ gimple_seq seq = NULL;
+ gimple_seq_add_stmt (&seq, shadow_test);
+ gimple_seq_add_stmt (&seq, build_assign (BIT_AND_EXPR, base_addr, 7));
+ gimple_seq_add_stmt (&seq, build_type_cast (shadow_type,
+ gimple_seq_last (seq)));
+ if (real_size_in_bytes > 1)
+ gimple_seq_add_stmt (&seq,
+ build_assign (PLUS_EXPR, gimple_seq_last (seq),
+ real_size_in_bytes - 1));
+ gimple_seq_add_stmt (&seq, build_assign (GE_EXPR,
+ gimple_seq_last (seq),
+ shadow));
+ gimple_seq_add_stmt (&seq, build_assign (BIT_AND_EXPR, shadow_test,
+ gimple_seq_last (seq)));
+ t = gimple_assign_lhs (gimple_seq_last (seq));
+ gimple_seq_set_location (seq, location);
+ gsi_insert_seq_after (&gsi, seq, GSI_CONTINUE_LINKING);
+ }
+
+ /* For non-constant, misaligned or otherwise weird access sizes,
+ check first and last byte. */
+ if (size_in_bytes == -1 && !end_instrumented)
+ {
+ g = gimple_build_assign_with_ops (MINUS_EXPR,
+ make_ssa_name (uintptr_type, NULL),
+ len,
+ build_int_cst (uintptr_type, 1));
+ gimple_set_location (g, location);
+ gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+ tree last = gimple_assign_lhs (g);
g = gimple_build_assign_with_ops (PLUS_EXPR,
make_ssa_name (uintptr_type, NULL),
base_addr,
- build_int_cst (uintptr_type,
- size_in_bytes - 1));
+ last);
gimple_set_location (g, location);
gsi_insert_after (&gsi, g, GSI_NEW_STMT);
tree base_end_addr = gimple_assign_lhs (g);
- shadow = build_shadow_mem_access (&gsi, location, base_end_addr,
- shadow_ptr_type);
- seq = NULL;
- shadow_test = build_assign (NE_EXPR, shadow, 0);
+ tree shadow = build_shadow_mem_access (&gsi, location, base_end_addr,
+ shadow_ptr_type);
+ gimple shadow_test = build_assign (NE_EXPR, shadow, 0);
+ gimple_seq seq = NULL;
gimple_seq_add_stmt (&seq, shadow_test);
gimple_seq_add_stmt (&seq, build_assign (BIT_AND_EXPR,
base_end_addr, 7));
@@ -1616,16 +1804,14 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
shadow));
gimple_seq_add_stmt (&seq, build_assign (BIT_AND_EXPR, shadow_test,
gimple_seq_last (seq)));
- gimple_seq_add_stmt (&seq, build_assign (BIT_IOR_EXPR, t,
- gimple_seq_last (seq)));
+ if (!start_instrumented)
+ gimple_seq_add_stmt (&seq, build_assign (BIT_IOR_EXPR, t,
+ gimple_seq_last (seq)));
t = gimple_assign_lhs (gimple_seq_last (seq));
gimple_seq_set_location (seq, location);
gsi_insert_seq_after (&gsi, seq, GSI_CONTINUE_LINKING);
- sz_arg = build_int_cst (pointer_sized_int_node, size_in_bytes);
}
}
- else
- t = shadow;
g = gimple_build_cond (NE_EXPR, t, build_int_cst (TREE_TYPE (t), 0),
NULL_TREE, NULL_TREE);
@@ -1634,8 +1820,23 @@ build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
/* Generate call to the run-time library (e.g. __asan_report_load8). */
gsi = gsi_start_bb (then_bb);
- g = gimple_build_call (report_error_func (is_store, size_in_bytes, slow_p),
- sz_arg ? 2 : 1, base_addr, sz_arg);
+ int nargs;
+ tree fun = report_error_func (is_store, is_scalar_access ? size_in_bytes : -1,
+ &nargs);
+ if (nargs == 1)
+ g = gimple_build_call (fun, 1, base_addr);
+ else
+ {
+ gcc_assert (nargs == 2);
+ g = gimple_build_assign_with_ops (NOP_EXPR,
+ make_ssa_name (pointer_sized_int_node,
+ NULL),
+ len, NULL_TREE);
+ gimple_set_location (g, location);
+ gsi_insert_after (&gsi, g, GSI_NEW_STMT);
+ tree sz_arg = gimple_assign_lhs (g);
+ g = gimple_build_call (fun, nargs, base_addr, sz_arg);
+ }
gimple_set_location (g, location);
gsi_insert_after (&gsi, g, GSI_NEW_STMT);
@@ -1730,31 +1931,10 @@ instrument_derefs (gimple_stmt_iterator *iter, tree t,
base = build_fold_addr_expr (t);
if (!has_mem_ref_been_instrumented (base, size_in_bytes))
{
- bool slow_p = false;
- if (size_in_bytes > 1)
- {
- if ((size_in_bytes & (size_in_bytes - 1)) != 0
- || size_in_bytes > 16)
- slow_p = true;
- else
- {
- unsigned int align = get_object_alignment (t);
- if (align < size_in_bytes * BITS_PER_UNIT)
- {
- /* On non-strict alignment targets, if
- 16-byte access is just 8-byte aligned,
- this will result in misaligned shadow
- memory 2 byte load, but otherwise can
- be handled using one read. */
- if (size_in_bytes != 16
- || STRICT_ALIGNMENT
- || align < 8 * BITS_PER_UNIT)
- slow_p = true;
- }
- }
- }
- build_check_stmt (location, base, iter, /*before_p=*/true,
- is_store, size_in_bytes, slow_p);
+ unsigned int align = get_object_alignment (t);
+ build_check_stmt (location, base, NULL_TREE, size_in_bytes, iter,
+ /*non_zero_len_p*/size_in_bytes > 0, /*before_p=*/true,
+ is_store, /*is_scalar_access*/true, align);
update_mem_ref_hash_table (base, size_in_bytes);
update_mem_ref_hash_table (t, size_in_bytes);
}
@@ -1779,142 +1959,24 @@ instrument_mem_region_access (tree base, tree len,
|| integer_zerop (len))
return;
- gimple_stmt_iterator gsi = *iter;
-
- basic_block fallthrough_bb = NULL, then_bb = NULL;
-
/* If the beginning of the memory region has already been
instrumented, do not instrument it. */
bool start_instrumented = has_mem_ref_been_instrumented (base, 1);
/* If the end of the memory region has already been instrumented, do
- not instrument it. */
+ not instrument it. */
tree end = asan_mem_ref_get_end (base, len);
bool end_instrumented = has_mem_ref_been_instrumented (end, 1);
- if (start_instrumented && end_instrumented)
- return;
-
- if (!is_gimple_constant (len))
- {
- /* So, the length of the memory area to asan-protect is
- non-constant. Let's guard the generated instrumentation code
- like:
-
- if (len != 0)
- {
- //asan instrumentation code goes here.
- }
- // falltrough instructions, starting with *ITER. */
-
- gimple g = gimple_build_cond (NE_EXPR,
- len,
- build_int_cst (TREE_TYPE (len), 0),
- NULL_TREE, NULL_TREE);
- gimple_set_location (g, location);
- insert_if_then_before_iter (g, iter, /*then_more_likely_p=*/true,
- &then_bb, &fallthrough_bb);
- /* Note that fallthrough_bb starts with the statement that was
- pointed to by ITER. */
-
- /* The 'then block' of the 'if (len != 0) condition is where
- we'll generate the asan instrumentation code now. */
- gsi = gsi_last_bb (then_bb);
- }
-
- if (!start_instrumented)
- {
- /* Instrument the beginning of the memory region to be accessed,
- and arrange for the rest of the intrumentation code to be
- inserted in the then block *after* the current gsi. */
- build_check_stmt (location, base, &gsi, /*before_p=*/true, is_store, 1);
-
- if (then_bb)
- /* We are in the case where the length of the region is not
- constant; so instrumentation code is being generated in the
- 'then block' of the 'if (len != 0) condition. Let's arrange
- for the subsequent instrumentation statements to go in the
- 'then block'. */
- gsi = gsi_last_bb (then_bb);
- else
- {
- *iter = gsi;
- /* Don't remember this access as instrumented, if length
- is unknown. It might be zero and not being actually
- instrumented, so we can't rely on it being instrumented. */
- update_mem_ref_hash_table (base, 1);
- }
- }
-
- if (end_instrumented)
- return;
+ HOST_WIDE_INT size_in_bytes = tree_fits_shwi_p (len) ? tree_to_shwi (len) : -1;
- /* We want to instrument the access at the end of the memory region,
- which is at (base + len - 1). */
+ build_check_stmt (location, base, len, size_in_bytes, iter,
+ /*non_zero_len_p*/size_in_bytes > 0, /*before_p*/true,
+ is_store, /*is_scalar_access*/false, /*align*/0,
+ start_instrumented, end_instrumented);
- /* offset = len - 1; */
- len = unshare_expr (len);
- tree offset;
- gimple_seq seq = NULL;
- if (TREE_CODE (len) == INTEGER_CST)
- offset = fold_build2 (MINUS_EXPR, size_type_node,
- fold_convert (size_type_node, len),
- build_int_cst (size_type_node, 1));
- else
- {
- gimple g;
- tree t;
-
- if (TREE_CODE (len) != SSA_NAME)
- {
- t = make_ssa_name (TREE_TYPE (len), NULL);
- g = gimple_build_assign_with_ops (TREE_CODE (len), t, len, NULL);
- gimple_set_location (g, location);
- gimple_seq_add_stmt_without_update (&seq, g);
- len = t;
- }
- if (!useless_type_conversion_p (size_type_node, TREE_TYPE (len)))
- {
- t = make_ssa_name (size_type_node, NULL);
- g = gimple_build_assign_with_ops (NOP_EXPR, t, len, NULL);
- gimple_set_location (g, location);
- gimple_seq_add_stmt_without_update (&seq, g);
- len = t;
- }
-
- t = make_ssa_name (size_type_node, NULL);
- g = gimple_build_assign_with_ops (MINUS_EXPR, t, len,
- build_int_cst (size_type_node, 1));
- gimple_set_location (g, location);
- gimple_seq_add_stmt_without_update (&seq, g);
- offset = gimple_assign_lhs (g);
- }
-
- /* _1 = base; */
- base = unshare_expr (base);
- gimple region_end =
- gimple_build_assign_with_ops (TREE_CODE (base),
- make_ssa_name (TREE_TYPE (base), NULL),
- base, NULL);
- gimple_set_location (region_end, location);
- gimple_seq_add_stmt_without_update (&seq, region_end);
-
- /* _2 = _1 + offset; */
- region_end =
- gimple_build_assign_with_ops (POINTER_PLUS_EXPR,
- make_ssa_name (TREE_TYPE (base), NULL),
- gimple_assign_lhs (region_end),
- offset);
- gimple_set_location (region_end, location);
- gimple_seq_add_stmt_without_update (&seq, region_end);
- gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
-
- /* instrument access at _2; */
- gsi = gsi_for_stmt (region_end);
- build_check_stmt (location, gimple_assign_lhs (region_end),
- &gsi, /*before_p=*/false, is_store, 1);
-
- if (then_bb == NULL)
+ update_mem_ref_hash_table (base, 1);
+ if (size_in_bytes != -1)
update_mem_ref_hash_table (end, 1);
*iter = gsi_for_stmt (gsi_stmt (*iter));
@@ -1956,47 +2018,30 @@ instrument_strlen_call (gimple_stmt_iterator *iter)
location_t loc = gimple_location (call);
tree str_arg = gimple_call_arg (call, 0);
- /* Instrument the access to the first byte of str_arg. i.e:
-
- _1 = str_arg; instrument (_1); */
tree cptr_type = build_pointer_type (char_type_node);
gimple str_arg_ssa =
gimple_build_assign_with_ops (NOP_EXPR,
make_ssa_name (cptr_type, NULL),
str_arg, NULL);
gimple_set_location (str_arg_ssa, loc);
- gimple_stmt_iterator gsi = *iter;
- gsi_insert_before (&gsi, str_arg_ssa, GSI_NEW_STMT);
- build_check_stmt (loc, gimple_assign_lhs (str_arg_ssa), &gsi,
- /*before_p=*/false, /*is_store=*/false, 1);
+ gsi_insert_before (iter, str_arg_ssa, GSI_SAME_STMT);
- /* If we initially had an instruction like:
+ build_check_stmt (loc, gimple_assign_lhs (str_arg_ssa), NULL_TREE, 1, iter,
+ /*non_zero_len_p*/true, /*before_p=*/true,
+ /*is_store=*/false, /*is_scalar_access*/false, /*align*/0);
- int n = strlen (str)
-
- we now want to instrument the access to str[n], after the
- instruction above.*/
-
- /* So let's build the access to str[n] that is, access through the
- pointer_plus expr: (_1 + len). */
gimple stmt =
- gimple_build_assign_with_ops (POINTER_PLUS_EXPR,
- make_ssa_name (cptr_type, NULL),
- gimple_assign_lhs (str_arg_ssa),
- len);
+ gimple_build_assign_with_ops (PLUS_EXPR,
+ make_ssa_name (TREE_TYPE (len), NULL),
+ len,
+ build_int_cst (TREE_TYPE (len), 1));
gimple_set_location (stmt, loc);
- gsi_insert_after (&gsi, stmt, GSI_NEW_STMT);
-
- build_check_stmt (loc, gimple_assign_lhs (stmt), &gsi,
- /*before_p=*/false, /*is_store=*/false, 1);
-
- /* Ensure that iter points to the statement logically following the
- one it was initially pointing to. */
- *iter = gsi;
- /* As *ITER has been advanced to point to the next statement, let's
- return true to inform transform_statements that it shouldn't
- advance *ITER anymore; otherwises it will skip that next
- statement, which wouldn't be instrumented. */
+ gsi_insert_after (iter, stmt, GSI_NEW_STMT);
+
+ build_check_stmt (loc, gimple_assign_lhs (stmt), len, 1, iter,
+ /*non_zero_len_p*/true, /*before_p=*/false,
+ /*is_store=*/false, /*is_scalar_access*/false, /*align*/0);
+
return true;
}
@@ -2568,6 +2613,7 @@ asan_instrument (void)
{
if (shadow_ptr_types[0] == NULL_TREE)
asan_init_shadow_ptr_types ();
+ asan_num_accesses = 0;
transform_statements ();
return 0;
}
diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c
index 7c24a6d..e857543 100644
--- a/gcc/cfgcleanup.c
+++ b/gcc/cfgcleanup.c
@@ -1175,7 +1175,7 @@ old_insns_match_p (int mode ATTRIBUTE_UNUSED, rtx i1, rtx i2)
&& DECL_FUNCTION_CODE (SYMBOL_REF_DECL (symbol))
>= BUILT_IN_ASAN_REPORT_LOAD1
&& DECL_FUNCTION_CODE (SYMBOL_REF_DECL (symbol))
- <= BUILT_IN_ASAN_REPORT_STORE16)
+ <= BUILT_IN_ASAN_STOREN)
return dir_none;
}
}
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 9475594..954cd6a 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10230,6 +10230,12 @@ is enabled by default when using @option{-fsanitize=address} option.
To disable use-after-return detection use
@option{--param asan-use-after-return=0}.
+@item asan-instrumentation-with-call-threshold
+Once number of memory accesses in function becomes greater
+or equal than this number, use callbacks instead of
+generating inline code. E.g. to disable inline code use
+@option{--param asan-instrumentation-with-call-threshold=0}.
+
@end table
@end table
diff --git a/gcc/params.def b/gcc/params.def
index c3a8797..28ef79a 100644
--- a/gcc/params.def
+++ b/gcc/params.def
@@ -1090,6 +1090,12 @@ DEFPARAM (PARAM_ASAN_USE_AFTER_RETURN,
"Enable asan builtin functions protection",
1, 0, 1)
+DEFPARAM (PARAM_ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD,
+ "asan-instrumentation-with-call-threshold",
+ "Use callbacks instead of inline code once number of accesses "
+ " in function becomes greater or equal than this threshold",
+ 10000, 0, INT_MAX)
+
DEFPARAM (PARAM_UNINIT_CONTROL_DEP_ATTEMPTS,
"uninit-control-dep-attempts",
"Maximum number of nested calls to search for control dependencies "
diff --git a/gcc/params.h b/gcc/params.h
index 0d6daa2..d488e32 100644
--- a/gcc/params.h
+++ b/gcc/params.h
@@ -232,5 +232,7 @@ extern void init_param_values (int *params);
PARAM_VALUE (PARAM_ASAN_MEMINTRIN)
#define ASAN_USE_AFTER_RETURN \
PARAM_VALUE (PARAM_ASAN_USE_AFTER_RETURN)
+#define ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD \
+ PARAM_VALUE (PARAM_ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD)
#endif /* ! GCC_PARAMS_H */
diff --git a/gcc/sanitizer.def b/gcc/sanitizer.def
index 4016fc5..b4af164 100644
--- a/gcc/sanitizer.def
+++ b/gcc/sanitizer.def
@@ -29,7 +29,7 @@ along with GCC; see the file COPYING3. If not see
/* Address Sanitizer */
DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_INIT, "__asan_init_v3",
BT_FN_VOID, ATTR_NOTHROW_LEAF_LIST)
-/* Do not reorder the BUILT_IN_ASAN_REPORT* builtins, e.g. cfgcleanup.c
+/* Do not reorder the BUILT_IN_ASAN_{REPORT,CHECK}* builtins, e.g. cfgcleanup.c
relies on this order. */
DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REPORT_LOAD1, "__asan_report_load1",
BT_FN_VOID_PTR, ATTR_TMPURE_NORETURN_NOTHROW_LEAF_LIST)
@@ -57,6 +57,30 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REPORT_STORE16, "__asan_report_store16",
DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REPORT_STORE_N, "__asan_report_store_n",
BT_FN_VOID_PTR_PTRMODE,
ATTR_TMPURE_NORETURN_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOAD1, "__asan_load1",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOAD2, "__asan_load2",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOAD4, "__asan_load4",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOAD8, "__asan_load8",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOAD16, "__asan_load16",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_LOADN, "__asan_loadN",
+ BT_FN_VOID_PTR_PTRMODE, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STORE1, "__asan_store1",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STORE2, "__asan_store2",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STORE4, "__asan_store4",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STORE8, "__asan_store8",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STORE16, "__asan_store16",
+ BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_STOREN, "__asan_storeN",
+ BT_FN_VOID_PTR_PTRMODE, ATTR_TMPURE_NOTHROW_LEAF_LIST)
DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_REGISTER_GLOBALS,
"__asan_register_globals",
BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST)
diff --git a/gcc/testsuite/c-c++-common/asan/instrument-with-calls-1.c b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-1.c
new file mode 100644
index 0000000..80f7620
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-1.c
@@ -0,0 +1,10 @@
+/* { dg-do assemble } */
+/* { dg-options "--param asan-instrumentation-with-call-threshold=0 -save-temps" } */
+
+void f(char *a, int *b) {
+ *b = *a;
+}
+
+/* { dg-final { scan-assembler "__asan_load1" } } */
+/* { dg-final { scan-assembler "__asan_store4" } } */
+/* { dg-final { cleanup-saved-temps } } */
diff --git a/gcc/testsuite/c-c++-common/asan/instrument-with-calls-2.c b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-2.c
new file mode 100644
index 0000000..570f796
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-2.c
@@ -0,0 +1,16 @@
+/* { dg-do assemble } */
+/* { dg-options "--param asan-instrumentation-with-call-threshold=1 -save-temps" } */
+
+int x;
+
+void f(int *a, int *b) {
+ *a = 0;
+ asm volatile ("" ::: "memory");
+ x = *b;
+}
+
+/* { dg-final { scan-assembler-not "__asan_store4" } } */
+/* { dg-final { scan-assembler "__asan_report_store4" } } */
+/* { dg-final { scan-assembler "__asan_load4" } } */
+/* { dg-final { scan-assembler-not "__asan_report_load4" } } */
+/* { dg-final { cleanup-saved-temps } } */
diff --git a/gcc/testsuite/c-c++-common/asan/instrument-with-calls-3.c b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-3.c
new file mode 100644
index 0000000..3712c7a
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/instrument-with-calls-3.c
@@ -0,0 +1,15 @@
+/* { dg-do assemble } */
+/* { dg-options "--param asan-instrumentation-with-call-threshold=0 -save-temps" } */
+
+struct A {
+ char x[7];
+};
+
+void f(struct A *x, struct A *y) {
+ *x = *y;
+}
+
+/* { dg-final { scan-assembler "__asan_loadN" } } */
+/* { dg-final { scan-assembler "__asan_storeN" } } */
+/* { dg-final { cleanup-saved-temps } } */
+
diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c
index fa52e0c..c7c594e 100644
--- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c
+++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c
@@ -16,12 +16,11 @@ test0 ()
tab[0] = 1;
tab[1] = 2;
- /* __builtin___asan_report_load1 called 1 time for the store
- below. */
- char t0 = tab[1];
-
/* This load should not be instrumented because it is to the same
memory location as above. */
+ char t0 = tab[1];
+
+ /* Likewise. */
char t1 = tab[1];
return t0 + t1;
@@ -36,7 +35,7 @@ test1 (int i)
the initialization. */
foo[i] = 1;
- /*__builtin___asan_report_store1 called 2 times here to instrument
+ /*__builtin___asan_report_store_n called once here to instrument
the store to the memory region of tab. */
__builtin_memset (tab, 3, sizeof (tab));
@@ -44,8 +43,8 @@ test1 (int i)
__builtin_memset (tab, 4, sizeof (tab));
__builtin_memset (tab, 5, sizeof (tab));
- /* There are 2 calls to __builtin___asan_report_store1 and 2 calls
- to __builtin___asan_report_load1 to instrument the store to
+ /* There is a call to __builtin___asan_report_store_n and a call
+ to __builtin___asan_report_load_n to instrument the store to
(subset of) the memory region of tab. */
__builtin_memcpy (&tab[1], foo + i, 3);
@@ -53,7 +52,7 @@ test1 (int i)
the reference to tab[1] has been already instrumented above. */
return tab[1];
- /* So for these function, there should be 7 calls to
+ /* So for these functions, there should be 3 calls to
__builtin___asan_report_store1. */
}
@@ -63,6 +62,7 @@ main ()
return test0 () && test1 (0);
}
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store1" 7 "asan0" } } */
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load" 2 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store1" 3 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 2 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load" 1 "asan0" } } */
/* { dg-final { cleanup-tree-dump "asan0" } } */
diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-2.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-2.c
index 28525e0..143312f 100644
--- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-2.c
+++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-2.c
@@ -20,6 +20,7 @@ main ()
__builtin_memset (tab, 1, 3);
}
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store1" 3 "asan0" } } */
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report" 3 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "& 7" 3 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 2 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report" 2 "asan0" } } */
/* { dg-final { cleanup-tree-dump "asan0" } } */
diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-4.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-4.c
index b2e7284..da91cd5 100644
--- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-4.c
+++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-4.c
@@ -5,9 +5,13 @@
void
foo (int *a, char *b, char *c)
{
+ /* One check for c[0], one check for a[], one check for c, two checks for b. */
__builtin_memmove (c, b, a[c[0]]);
+ /* For a total of 5 checks. */
}
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 3 "asan0" } } */
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store1" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "& 7" 5 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load_n" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 1 "asan0" } } */
/* { dg-final { cleanup-tree-dump "asan0" } } */
diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-5.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-5.c
index ead3f58..134be66 100644
--- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-5.c
+++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-5.c
@@ -5,9 +5,14 @@
void
foo (int *a, char *b, char *c)
{
+ /* One check for b[0], one check for a[], 2 checks for c and one checks for b. */
__builtin_memmove (c, b, a[b[0]]);
+ /* For a total of 5 checks. */
}
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 2 "asan0" } } */
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store1" 2 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "& 7" 5 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load4" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load_n" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 1 "asan0" } } */
/* { dg-final { cleanup-tree-dump "asan0" } } */
diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-6.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-6.c
index e4691bc..55c8ee3 100644
--- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-6.c
+++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-6.c
@@ -5,10 +5,16 @@
void
foo (int *a, char *b, char *c)
{
+ /* One check for c[0], one check for a[], one check for c and 2 checks for b. */
__builtin_memmove (c, b, a[c[0]]);
+ /* One check for a[], one check for c and one check for b. */
__builtin_memmove (c, b, a[b[0]]);
+ /* For a total of 8 checks. */
}
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 5 "asan0" } } */
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store1" 2 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "& 7" 8 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load4" 2 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load_n" 2 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 2 "asan0" } } */
/* { dg-final { cleanup-tree-dump "asan0" } } */
diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c
index bf40a03..a04956d 100644
--- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c
+++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c
@@ -13,11 +13,15 @@ struct S
int
foo (int *a, char *b, char *c)
{
+ /* 2 checks for s.a, 2 checks for e. */
int d = __builtin_memcmp (s.a, e, 100);
+ /* One check for s.a and one check for e. */
d += __builtin_memcmp (s.a, e, 200);
+ /* For a total of 6 checks. */
return d;
}
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load" 6 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "& 7" 6 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load_n" 4 "asan0" } } */
/* { dg-final { scan-tree-dump-not "__builtin___asan_report_store" "asan0" } } */
/* { dg-final { cleanup-tree-dump "asan0" } } */
diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-8.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-8.c
index 38ea7a2..308a133 100644
--- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-8.c
+++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-8.c
@@ -5,10 +5,16 @@
char
foo (int *a, char *b, char *c)
{
+ /* One check for b[0], one check for a[], two checks for c and one check for b. */
__builtin_memmove (c, b, a[b[0]]);
+ /* No checks here. */
return c[0] + b[0];
+ /* For a total of 5 checks. */
}
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 3 "asan0" } } */
-/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store1" 2 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "& 7" 5 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load4" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load_n" 1 "asan0" } } */
+/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 1 "asan0" } } */
/* { dg-final { cleanup-tree-dump "asan0" } } */
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-11 12:53 ` Yury Gribov
@ 2014-06-16 8:23 ` Jakub Jelinek
2014-06-16 8:44 ` Yury Gribov
0 siblings, 1 reply; 22+ messages in thread
From: Jakub Jelinek @ 2014-06-16 8:23 UTC (permalink / raw)
To: Yury Gribov
Cc: GCC Patches, Konstantin Serebryany, Viacheslav Garbuzov,
Dmitry Vyukov, Yuri Gribov
On Wed, Jun 11, 2014 at 04:53:32PM +0400, Yury Gribov wrote:
> On 06/11/2014 01:31 PM, Jakub Jelinek wrote:
> >The plan (we had already for 4.9, but didn't get to that yet) is in the end
> >not to lower the checks in asan pass that much, and lower it in sanopt
> >pass later on after performing some inter-bb optimizations.
> >...
> >The reason for the plan is that it will be easier in the sanopt pass to
> >try to remove redundant instrumentation, e.g. when dominator bb already
> >checks a particular address/length and no calls that could change the
> >validity of the checks happen between the dominator bb and the dominated
> >__asan_load/storeX.
>
> Sounds great, should help get rid of many useless checks.
> Do you guys need a hand with this?
That would be appreciated.
> Like this?
>
> -Y
> 2014-06-11 Yury Gribov <y.gribov@samsung.com>
>
> New asan-instrumentation-with-call-threshold parameter.
Ok, thanks.
Jakub
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-16 8:23 ` Jakub Jelinek
@ 2014-06-16 8:44 ` Yury Gribov
0 siblings, 0 replies; 22+ messages in thread
From: Yury Gribov @ 2014-06-16 8:44 UTC (permalink / raw)
To: Jakub Jelinek
Cc: GCC Patches, Konstantin Serebryany, Viacheslav Garbuzov,
Dmitry Vyukov, Yuri Gribov
On 06/16/2014 12:23 PM, Jakub Jelinek wrote:
>> 2014-06-11 Yury Gribov <y.gribov@samsung.com>
>>
>> New asan-instrumentation-with-call-threshold parameter.
>
> Ok, thanks.
Done in r211699.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-16 18:24 ` Jakub Jelinek
@ 2014-06-16 18:31 ` Yury Gribov
0 siblings, 0 replies; 22+ messages in thread
From: Yury Gribov @ 2014-06-16 18:31 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Dominique Dhumieres, gcc-patches, v.garbuzov, kcc
>> Bootstrapped successfully on x64 with proposed patch.
>> The original commit indeed failed to bootstrap
>> (https://gcc.gnu.org/ml/gcc-testresults/2014-06/msg01419.html).
>>
>> Ok to commit fix?
>
> Ok (with proper ChangeLog entry).
r211713.
-Y
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-16 18:21 ` Yury Gribov
@ 2014-06-16 18:24 ` Jakub Jelinek
2014-06-16 18:31 ` Yury Gribov
0 siblings, 1 reply; 22+ messages in thread
From: Jakub Jelinek @ 2014-06-16 18:24 UTC (permalink / raw)
To: Yury Gribov; +Cc: Dominique Dhumieres, gcc-patches, v.garbuzov, kcc
On Mon, Jun 16, 2014 at 10:21:39PM +0400, Yury Gribov wrote:
> On 06/16/2014 04:47 PM, Yury Gribov wrote:
> >On 06/16/2014 02:34 PM, Dominique Dhumieres wrote:
> >>>Done in r211699.
> >>
> >>This breaks bootstrap on x86_64-apple-darwin13:
> >
> >Hm, perhaps I didn't run full bootstrap after last round of review.
> >Does attached patch solve the problem for you? I've started bootstrap
> >but it'll take couple of hours to complete.
>
> Bootstrapped successfully on x64 with proposed patch.
> The original commit indeed failed to bootstrap
> (https://gcc.gnu.org/ml/gcc-testresults/2014-06/msg01419.html).
>
> Ok to commit fix?
Ok (with proper ChangeLog entry).
> --- a/gcc/asan.c
> +++ b/gcc/asan.c
> @@ -1636,6 +1636,13 @@ build_check_stmt (location_t location, tree base, tree len,
>
> gcc_assert (!(size_in_bytes > 0 && !non_zero_len_p));
>
> + if (start_instrumented && end_instrumented)
> + {
> + if (!before_p)
> + gsi_next (iter);
> + return;
> + }
> +
> if (len)
> len = unshare_expr (len);
> else
> @@ -1735,7 +1742,7 @@ build_check_stmt (location_t location, tree base, tree len,
> gsi_insert_after (&gsi, g, GSI_NEW_STMT);
> tree base_addr = gimple_assign_lhs (g);
>
> - tree t;
> + tree t = NULL_TREE;
> if (real_size_in_bytes >= 8)
> {
> tree shadow = build_shadow_mem_access (&gsi, location, base_addr,
Jakub
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-16 12:47 ` Yury Gribov
2014-06-16 12:52 ` Dominique Dhumieres
@ 2014-06-16 18:21 ` Yury Gribov
2014-06-16 18:24 ` Jakub Jelinek
1 sibling, 1 reply; 22+ messages in thread
From: Yury Gribov @ 2014-06-16 18:21 UTC (permalink / raw)
To: Dominique Dhumieres, gcc-patches, Jakub Jelinek; +Cc: v.garbuzov, kcc
[-- Attachment #1: Type: text/plain, Size: 554 bytes --]
On 06/16/2014 04:47 PM, Yury Gribov wrote:
> On 06/16/2014 02:34 PM, Dominique Dhumieres wrote:
>>> Done in r211699.
>>
>> This breaks bootstrap on x86_64-apple-darwin13:
>
> Hm, perhaps I didn't run full bootstrap after last round of review.
> Does attached patch solve the problem for you? I've started bootstrap
> but it'll take couple of hours to complete.
Bootstrapped successfully on x64 with proposed patch.
The original commit indeed failed to bootstrap
(https://gcc.gnu.org/ml/gcc-testresults/2014-06/msg01419.html).
Ok to commit fix?
-Y
[-- Attachment #2: warn_fix_1.diff --]
[-- Type: text/x-diff, Size: 740 bytes --]
diff --git a/gcc/asan.c b/gcc/asan.c
index 19e1524..281a795 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1636,6 +1636,13 @@ build_check_stmt (location_t location, tree base, tree len,
gcc_assert (!(size_in_bytes > 0 && !non_zero_len_p));
+ if (start_instrumented && end_instrumented)
+ {
+ if (!before_p)
+ gsi_next (iter);
+ return;
+ }
+
if (len)
len = unshare_expr (len);
else
@@ -1735,7 +1742,7 @@ build_check_stmt (location_t location, tree base, tree len,
gsi_insert_after (&gsi, g, GSI_NEW_STMT);
tree base_addr = gimple_assign_lhs (g);
- tree t;
+ tree t = NULL_TREE;
if (real_size_in_bytes >= 8)
{
tree shadow = build_shadow_mem_access (&gsi, location, base_addr,
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-16 13:01 ` Jakub Jelinek
@ 2014-06-16 14:42 ` Dominique Dhumieres
0 siblings, 0 replies; 22+ messages in thread
From: Dominique Dhumieres @ 2014-06-16 14:42 UTC (permalink / raw)
To: y.gribov, jakub; +Cc: v.garbuzov, kcc, gcc-patches, dominiq
I have bootstrapped r211707 with the full patch (among others!).
> It does not. So the above is IMHO very likely completely unrelated,
> perhaps Darwin specific, issue.
More likely related to the fact that I only resumed bootstrap after the change
tree t = NULL_TREE; and an update from r211700 to r211701, instead of starting
a clean bootstrap.
Cheers,
Dominiaue
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-16 12:58 ` Yury Gribov
@ 2014-06-16 13:01 ` Jakub Jelinek
2014-06-16 14:42 ` Dominique Dhumieres
0 siblings, 1 reply; 22+ messages in thread
From: Jakub Jelinek @ 2014-06-16 13:01 UTC (permalink / raw)
To: Yury Gribov; +Cc: Dominique Dhumieres, gcc-patches, v.garbuzov, kcc
On Mon, Jun 16, 2014 at 04:57:54PM +0400, Yury Gribov wrote:
> On 06/16/2014 04:52 PM, Dominique Dhumieres wrote:
> >I have done the change to tree t = NULL_TREE; then bootstrap fails with
> >
> >Bootstrap comparison failure!
> >gcc/plugin.o differs
> >gcc/toplev.o differs
>
> Interesting, I didn't know that asan.c runs during normal bootstrap.
It does not. So the above is IMHO very likely completely unrelated, perhaps
Darwin specific, issue.
Jakub
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-16 12:52 ` Dominique Dhumieres
@ 2014-06-16 12:58 ` Yury Gribov
2014-06-16 13:01 ` Jakub Jelinek
0 siblings, 1 reply; 22+ messages in thread
From: Yury Gribov @ 2014-06-16 12:58 UTC (permalink / raw)
To: Dominique Dhumieres, gcc-patches; +Cc: v.garbuzov, kcc
On 06/16/2014 04:52 PM, Dominique Dhumieres wrote:
> I have done the change to tree t = NULL_TREE; then bootstrap fails with
>
> Bootstrap comparison failure!
> gcc/plugin.o differs
> gcc/toplev.o differs
Interesting, I didn't know that asan.c runs during normal bootstrap.
-Y
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-16 12:47 ` Yury Gribov
@ 2014-06-16 12:52 ` Dominique Dhumieres
2014-06-16 12:58 ` Yury Gribov
2014-06-16 18:21 ` Yury Gribov
1 sibling, 1 reply; 22+ messages in thread
From: Dominique Dhumieres @ 2014-06-16 12:52 UTC (permalink / raw)
To: y.gribov, gcc-patches, dominiq; +Cc: v.garbuzov, kcc
I have done the change to tree t = NULL_TREE; then bootstrap fails with
Bootstrap comparison failure!
gcc/plugin.o differs
gcc/toplev.o differs
I'll try your full patch later (currently bootstrapping r211698).
Thanks,
Dominique
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
2014-06-16 10:34 Dominique Dhumieres
@ 2014-06-16 12:47 ` Yury Gribov
2014-06-16 12:52 ` Dominique Dhumieres
2014-06-16 18:21 ` Yury Gribov
0 siblings, 2 replies; 22+ messages in thread
From: Yury Gribov @ 2014-06-16 12:47 UTC (permalink / raw)
To: Dominique Dhumieres, gcc-patches; +Cc: v.garbuzov, kcc
[-- Attachment #1: Type: text/plain, Size: 310 bytes --]
On 06/16/2014 02:34 PM, Dominique Dhumieres wrote:
>> Done in r211699.
>
> This breaks bootstrap on x86_64-apple-darwin13:
Hm, perhaps I didn't run full bootstrap after last round of review.
Does attached patch solve the problem for you? I've started bootstrap
but it'll take couple of hours to complete.
-Y
[-- Attachment #2: warn_fix_1.diff --]
[-- Type: text/x-diff, Size: 740 bytes --]
diff --git a/gcc/asan.c b/gcc/asan.c
index 19e1524..281a795 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1636,6 +1636,13 @@ build_check_stmt (location_t location, tree base, tree len,
gcc_assert (!(size_in_bytes > 0 && !non_zero_len_p));
+ if (start_instrumented && end_instrumented)
+ {
+ if (!before_p)
+ gsi_next (iter);
+ return;
+ }
+
if (len)
len = unshare_expr (len);
else
@@ -1735,7 +1742,7 @@ build_check_stmt (location_t location, tree base, tree len,
gsi_insert_after (&gsi, g, GSI_NEW_STMT);
tree base_addr = gimple_assign_lhs (g);
- tree t;
+ tree t = NULL_TREE;
if (real_size_in_bytes >= 8)
{
tree shadow = build_shadow_mem_access (&gsi, location, base_addr,
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try)
@ 2014-06-16 10:34 Dominique Dhumieres
2014-06-16 12:47 ` Yury Gribov
0 siblings, 1 reply; 22+ messages in thread
From: Dominique Dhumieres @ 2014-06-16 10:34 UTC (permalink / raw)
To: gcc-patches; +Cc: v.garbuzov, kcc, y.gribov
> Done in r211699.
This breaks bootstrap on x86_64-apple-darwin13:
/opt/gcc/build_w/./prev-gcc/xg++ -B/opt/gcc/build_w/./prev-gcc/ -B/opt/gcc/gcc4.10w/x86_64-apple-darwin13.2.0/bin/ -nostdinc++ -B/opt/gcc/build_w/prev-x86_64-apple-darwin13.2.0/libstdc++-v3/src/.libs -B/opt/gcc/build_w/prev-x86_64-apple-darwin13.2.0/libstdc++-v3/libsupc++/.libs -I/opt/gcc/build_w/prev-x86_64-apple-darwin13.2.0/libstdc++-v3/include/x86_64-apple-darwin13.2.0 -I/opt/gcc/build_w/prev-x86_64-apple-darwin13.2.0/libstdc++-v3/include -I/opt/gcc/work/libstdc++-v3/libsupc++ -L/opt/gcc/build_w/prev-x86_64-apple-darwin13.2.0/libstdc++-v3/src/.libs -L/opt/gcc/build_w/prev-x86_64-apple-darwin13.2.0/libstdc++-v3/libsupc++/.libs -c -g -O2 -gtoggle -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -I. -I. -I../../work/gcc -I../../work/gcc/. -I../../work/gcc/../include -I./../intl -I../../work/gcc/../libcpp/include -I/opt/mp/include -I../../work/gcc/../libdecnumber -I../../work/gcc/../libdecnumber/dpd -I../libdecnumber -I../../work/gcc/../libbacktrace -DCLOOG_INT_GMP -I/opt/mp/include -o asan.o -MT asan.o -MMD -MP -MF ./.deps/asan.TPo ../../work/gcc/asan.c
../../work/gcc/asan.c: In function 'void build_check_stmt(location_t, tree, tree, long long int, gimple_stmt_iterator*, bool, bool, bool, bool, unsigned int, bool, bool)':
../../work/gcc/asan.c:1810:34: error: 't' may be used uninitialized in this function [-Werror=maybe-uninitialized]
gimple_seq_last (seq)));
^
cc1plus: all warnings being treated as errors
TIA
Dominique
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2014-06-16 18:31 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-03 8:02 [PATCH] Support asan-instrumentation-with-call-threshold in GCC (second try) Yury Gribov
2014-06-03 8:38 ` Jakub Jelinek
2014-06-03 9:33 ` Yury Gribov
2014-06-03 9:37 ` Konstantin Serebryany
2014-06-03 9:37 ` Jakub Jelinek
2014-06-06 5:08 ` Yury Gribov
2014-06-06 6:02 ` Jakub Jelinek
2014-06-09 4:53 ` Yury Gribov
2014-06-09 15:48 ` Yury Gribov
2014-06-11 9:32 ` Jakub Jelinek
2014-06-11 12:53 ` Yury Gribov
2014-06-16 8:23 ` Jakub Jelinek
2014-06-16 8:44 ` Yury Gribov
2014-06-16 10:34 Dominique Dhumieres
2014-06-16 12:47 ` Yury Gribov
2014-06-16 12:52 ` Dominique Dhumieres
2014-06-16 12:58 ` Yury Gribov
2014-06-16 13:01 ` Jakub Jelinek
2014-06-16 14:42 ` Dominique Dhumieres
2014-06-16 18:21 ` Yury Gribov
2014-06-16 18:24 ` Jakub Jelinek
2014-06-16 18:31 ` Yury Gribov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).