On Tue, Feb 12, 2013 at 03:19:37PM +0100, Dodji Seketeli wrote:
> gcc/
> * Makefile.in (asan.o): Add new dependency on hash-table.h
> * asan.c (struct asan_mem_ref, struct mem_ref_hasher): New types.
> (asan_mem_ref_init, asan_mem_ref_get_end, get_mem_ref_hash_table)
> (has_stmt_been_instrumented_p, empty_mem_ref_hash_table)
> (free_mem_ref_resources, has_mem_ref_been_instrumented)
> (has_stmt_been_instrumented_p, update_mem_ref_hash_table)
> (get_mem_ref_of_assignment): New functions.
> (get_mem_refs_of_builtin_call): Extract from
> instrument_builtin_call and tweak a little bit to make it fit with
> the new signature.
> (instrument_builtin_call): Use the new
> get_mem_refs_of_builtin_call. Use gimple_call_builtin_p instead
> of is_gimple_builtin_call.
> (instrument_derefs, instrument_mem_region_access): Insert the
> instrumented memory reference into the hash table.
> (maybe_instrument_assignment): Renamed instrument_assignment into
> this, and change it to advance the iterator when instrumentation
> actually happened and return true in that case. This makes it
> homogeneous with maybe_instrument_assignment, and thus give a
> chance to callers to be more 'regular'.
> (transform_statements): Clear the memory reference hash table
> whenever we enter a new BB, when we cross a function call, or when
> we are done transforming statements. Use
> maybe_instrument_assignment instead of instrumentation. No more
> need to special case maybe_instrument_assignment and advance the
> iterator after calling it; it's now handled just like
> maybe_instrument_call. Update comment.
Ok. Just some testsuite nits.
> gcc/testsuite/
>
> * c-c++-common/asan/no-redundant-instrumentation-1.c: New test.
> * testsuite/c-c++-common/asan/no-redundant-instrumentation-2.c:
> Likewise.
> * testsuite/c-c++-common/asan/no-redundant-instrumentation-3.c:
> Likewise.
> * testsuite/c-c++-common/asan/inc.c: Likewise.
> diff --git a/gcc/testsuite/c-c++-common/asan/inc.c
> b/gcc/testsuite/c-c++-common/asan/inc.c
> new file mode 100644
> index 0000000..09ad176
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/asan/inc.c
> @@ -0,0 +1,20 @@
> +/* { dg-options "-fdump-tree-asan0" }
> + { dg-do compile } */
I think generally the style used in the gcc testsuite is for C tests:
/* { dg-options "..." } */
/* { dg-do compile } */
and for C++ // comments.
I.e. don't merge multiple lines into one comments, no extra spaces.
> +
> +/* { dg-final { scan-tree-dump-times "__builtin___asan_report" 1 "asan0" } }
> */
> +
> +/* { dg-final { scan-tree-dump "__builtin___asan_report_load4" "asan0" } }
> */
No need for the empty line in between that.
> 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
> new file mode 100644
> index 0000000..8c6cca4
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c
> @@ -0,0 +1,67 @@
> +/* This tests that when faced with two references to the same memory
> + location in the same basic block, the second reference should not
> + be instrumented by the Address Sanitizer
Here I'd end with a full stop, i.e. Address Sanitizer. */
> +
> + { dg-options "-fdump-tree-asan0" }
> + { dg-do compile }
> + */
And the above would be one comment per one line style again.
> +
> +/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store1" 7
> "asan0" } }
> +
> + { dg-final { scan-tree-dump-times "__builtin___asan_report_load" 2
> "asan0" } }
> + */
Again, both one line comments, and no empty line in between.
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-2.c
> @@ -0,0 +1,26 @@
> +/* This tests that when faced with two references to the same memory
> + location in the same basic block, the second reference should not
> + be instrumented by the Address Sanitizer. But in case of access to
> + overlapping regions we must be precise.
> +
> + { dg-options "-fdump-tree-asan0" }
> + { dg-do compile }
> + */
Ditto.
> +
> +int
> +main ()
> +{
> + char tab[5];
> +
> + /* Here, we instrument the access at offset 0 and access at offset
> + 4. */
> + __builtin_memset (tab, 1, sizeof (tab));
> + /* We instrumented access at offset 0 above already, so only access
> + at offset 3 is instrumented. */
> + __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" } }
> + */
Ditto.
...
Jakub
