2013-02-15 Jakub Jelinek PR sanitizer/56330 * asan.c (get_mem_refs_of_builtin_call): Fix up indentation. (instrument_mem_region_access): Create conditional even when the start has been already instrumented. Don't record conditional instrumentation in the hash table. Update *iter after second build_check_stmt call. (instrument_builtin_call): For consistency test != NULL_TREE in all start checks. * c-c++-common/asan/no-redundant-instrumentation-1.c: Pass 3 instead of sizeof (tab) - 1 as last argument. * c-c++-common/asan/pr56330.c: New test. --- gcc/asan.c.jj 2013-02-14 14:45:01.428038792 +0100 +++ gcc/asan.c 2013-02-15 09:37:18.614963383 +0100 @@ -747,20 +747,17 @@ get_mem_refs_of_builtin_call (const gimp got_reference_p = true; } - else - { - if (dest) - { - dst->start = dest; - dst->access_size = access_size; - *dst_len = NULL_TREE; - *dst_is_store = is_store; - *dest_is_deref = true; - got_reference_p = true; - } - } + else if (dest) + { + dst->start = dest; + dst->access_size = access_size; + *dst_len = NULL_TREE; + *dst_is_store = is_store; + *dest_is_deref = true; + got_reference_p = true; + } - return got_reference_p; + return got_reference_p; } /* Return true iff a given gimple statement has been instrumented. @@ -1535,8 +1532,15 @@ instrument_mem_region_access (tree base, /* If the beginning of the memory region has already been instrumented, do not instrument it. */ - if (has_mem_ref_been_instrumented (base, 1)) - goto after_first_instrumentation; + 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. */ + 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)) { @@ -1565,34 +1569,36 @@ instrument_mem_region_access (tree base, gsi = gsi_start_bb (then_bb); } - /* 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; - - update_mem_ref_hash_table (base, 1); + 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); + } + } - after_first_instrumentation: + if (end_instrumented) + return; /* We want to instrument the access at the end of the memory region, which is at (base + len - 1). */ - /* If the end of the memory region has already been instrumented, do - not instrument it. */ - tree end = asan_mem_ref_get_end (base, len); - if (has_mem_ref_been_instrumented (end, 1)) - return; - /* offset = len - 1; */ len = unshare_expr (len); tree offset; @@ -1640,7 +1646,10 @@ instrument_mem_region_access (tree base, gimple_set_location (region_end, location); gimple_seq_add_stmt_without_update (&seq, region_end); gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT); - gsi_prev (&gsi); + if (!start_instrumented || then_bb == NULL) + gsi_prev (&gsi); + else + gsi = gsi_last_bb (then_bb); /* _2 = _1 + offset; */ region_end = @@ -1655,7 +1664,10 @@ instrument_mem_region_access (tree base, build_check_stmt (location, gimple_assign_lhs (region_end), &gsi, /*before_p=*/false, is_store, 1); - update_mem_ref_hash_table (end, 1); + if (then_bb == NULL) + update_mem_ref_hash_table (end, 1); + + *iter = gsi_for_stmt (gsi_stmt (*iter)); } /* Instrument the call (to the builtin strlen function) pointed to by @@ -1783,7 +1795,7 @@ instrument_builtin_call (gimple_stmt_ite } else if (src0_len || src1_len || dest_len) { - if (src0.start) + if (src0.start != NULL_TREE) instrument_mem_region_access (src0.start, src0_len, iter, loc, /*is_store=*/false); if (src1.start != NULL_TREE) --- gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c.jj 2013-02-13 11:53:41.000000000 +0100 +++ gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c 2013-02-15 09:51:47.485080706 +0100 @@ -45,7 +45,7 @@ test1 () /* There are 2 calls to __builtin___asan_report_store1 and 2 calls to __builtin___asan_report_load1 to instrument the store to (subset of) the memory region of tab. */ - __builtin_memcpy (&tab[1], foo, sizeof (tab) - 1); + __builtin_memcpy (&tab[1], foo, 3); /* This should not generate a __builtin___asan_report_load1 because the reference to tab[1] has been already instrumented above. */ --- gcc/testsuite/c-c++-common/asan/pr56330.c.jj 2013-02-15 09:43:19.293845146 +0100 +++ gcc/testsuite/c-c++-common/asan/pr56330.c 2013-02-15 09:42:58.000000000 +0100 @@ -0,0 +1,23 @@ +/* PR sanitizer/56330 */ +/* { dg-do compile } */ + +char e[200]; + +struct S +{ + char a[100]; + char b[100]; +} s; + +void +foo (void) +{ + __builtin_memcmp (s.a, e, 100); + __builtin_memcmp (s.a, e, 200); +} + +void +bar (int *a, char *b, char *c) +{ + __builtin_memmove (c, b, a[b[0]]); +}