On Mon, May 26, 2014 at 07:58:17PM +0400, Yury Gribov wrote: > This patch adds support for outline Asan instrumentation (i.e. > function calls instead of inline checks). This 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 > > Implementation is a bit different from LLVM: GCC starts generating > function calls after overflowing the threshold whereas LLVM replaces > all instrumentations once total number is estimated to be larger > than treshold. Making implementations more similar would require > bigger rewrite of Asan which I would prefer to avoid. > > The patch consists of two parts: > * asan_negative_params_1.diff - support for negative parameters
I don't like this. Why do you need it? Negative params look like a bad idea to me. If you want to have different values for none, all and some, then you can use e.g. 0 as a special value and number of accesses >= param for the other one, or one can be 0, another INT_MAX, etc., -1, 0 and positive doesn't make things any clearer. > * asan_calls_1.diff - the proper Note, the patch conflicts with the recently posted asan patches, please wait until those are reviewed. > --- 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; I don't see this variable ever changed, so don't see how the patch can actually work. > @@ -1476,17 +1506,30 @@ 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; > + bool use_calls = asan_num_accesses > > ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD; Too long line. > @@ -1687,6 +1798,8 @@ 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; Likewise. > @@ -2265,6 +2386,9 @@ initialize_sanitizer_builtins (void) > tree BT_FN_VOID_PTR_PTR > = build_function_type_list (void_type_node, ptr_type_node, > ptr_type_node, NULL_TREE); > + tree BT_FN_VOID_PTR_SIZE > + = build_function_type_list (void_type_node, ptr_type_node, > + size_type_node, NULL_TREE); I thought the functions use uptr arguments, aka BT_FN_VOID_PTR_PTRMODE. Jakub