Hi!
This is just a small improvement for Dodji's work. We can flush the hash
table with memory references known to be instrumented only at extended basic
block boundaries, no need to flush it at every bb (of course, for 4.9 we
want to do something better). And, now that we have nonfreeing_call_p
predicate, we can avoid also flushing at builtin calls that are known not to
free memory or use callbacks etc.
The first 5 hunks are there because I've noticed that we were creating a
fallthru edge from basic blocks with __asan_report_* call (which is
noreturn) - we shouldn't have any edge there.
Where these change make a difference is e.g.:
void baz (long *, int);
long
foo (long *p, int q)
{
long a = *p;
if (q)
{
*p = 6;
baz (p, 7);
}
else
{
*p = 25;
baz (p + 1, 17);
}
return a;
}
long
bar (long *p, double q, double r)
{
long a = *p;
a += __builtin_pow (q, r);
*p = 6;
return a;
}
Without the patch, in asan1 pass there are two load instrumentations
(one in each fn) and 3 store insturmentations, with the patch 2 load
and one store instrumentation (only one of then and else bbs are
considered extended bb). For foo later optimizations actually optimize
away all of store instrumentations, but it doesn't have to happen always.
2013-02-13 Jakub Jelinek <ja...@redhat.com>
* asan.c (create_cond_insert_point): Add create_then_fallthru_edge
argument. If it is false, don't create edge from then_bb to
fallthru_bb.
(insert_if_then_before_iter): Pass true to it.
(build_check_stmt): Pass false to it.
(transform_statements): Flush hash table only on extended basic
block boundaries, rather than at the beginning of every bb.
Don't flush hash table on nonfreeing_call_p calls.
* tree-flow.h (nonfreeing_call_p): New prototype.
* tree-ssa-phiopt.c (nonfreeing_call_p): No longer static.
--- gcc/asan.c.jj 2013-02-13 11:53:42.000000000 +0100
+++ gcc/asan.c 2013-02-13 19:46:20.033555655 +0100
@@ -1185,6 +1185,9 @@ report_error_func (bool is_store, int si
'then block' of the condition statement to be inserted by the
caller.
+ If CREATE_THEN_FALLTHRU_EDGE is false, no edge will be created from
+ *THEN_BLOCK to *FALLTHROUGH_BLOCK.
+
Similarly, the function will set *FALLTRHOUGH_BLOCK to the 'else
block' of the condition statement to be inserted by the caller.
@@ -1201,6 +1204,7 @@ static gimple_stmt_iterator
create_cond_insert_point (gimple_stmt_iterator *iter,
bool before_p,
bool then_more_likely_p,
+ bool create_then_fallthru_edge,
basic_block *then_block,
basic_block *fallthrough_block)
{
@@ -1226,7 +1230,8 @@ create_cond_insert_point (gimple_stmt_it
? PROB_VERY_UNLIKELY
: PROB_ALWAYS - PROB_VERY_UNLIKELY;
e->probability = PROB_ALWAYS - fallthrough_probability;
- make_single_succ_edge (then_bb, fallthru_bb, EDGE_FALLTHRU);
+ if (create_then_fallthru_edge)
+ make_single_succ_edge (then_bb, fallthru_bb, EDGE_FALLTHRU);
/* Set up the fallthrough basic block. */
e = find_edge (cond_bb, fallthru_bb);
@@ -1277,6 +1282,7 @@ insert_if_then_before_iter (gimple cond,
create_cond_insert_point (iter,
/*before_p=*/true,
then_more_likely_p,
+ /*create_then_fallthru_edge=*/true,
then_bb,
fallthrough_bb);
gsi_insert_after (&cond_insert_point, cond, GSI_NEW_STMT);
@@ -1314,6 +1320,7 @@ build_check_stmt (location_t location, t
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);
@@ -1883,15 +1890,31 @@ maybe_instrument_call (gimple_stmt_itera
static void
transform_statements (void)
{
- basic_block bb;
+ basic_block bb, last_bb = NULL;
gimple_stmt_iterator i;
int saved_last_basic_block = last_basic_block;
FOR_EACH_BB (bb)
{
- empty_mem_ref_hash_table ();
+ basic_block prev_bb = bb;
if (bb->index >= saved_last_basic_block) continue;
+
+ /* Flush the mem ref hash table, if current bb doesn't have
+ exactly one predecessor, or if that predecessor (skipping
+ over asan created basic blocks) isn't the last processed
+ basic block. Thus we effectively flush on extended basic
+ block boundaries. */
+ while (single_pred_p (prev_bb))
+ {
+ prev_bb = single_pred (prev_bb);
+ if (prev_bb->index < saved_last_basic_block)
+ break;
+ }
+ if (prev_bb != last_bb)
+ empty_mem_ref_hash_table ();
+ last_bb = bb;
+
for (i = gsi_start_bb (bb); !gsi_end_p (i);)
{
gimple s = gsi_stmt (i);
@@ -1909,11 +1932,11 @@ transform_statements (void)
{
/* No instrumentation happened.
- If the current instruction is a function call, let's
- forget about the memory references that got
- instrumented. Otherwise we might miss some
- instrumentation opportunities. */
- if (is_gimple_call (s))
+ If the current instruction is a function call that
+ might free something, let's forget about the memory
+ references that got instrumented. Otherwise we might
+ miss some instrumentation opportunities. */
+ if (is_gimple_call (s) && !nonfreeing_call_p (s))
empty_mem_ref_hash_table ();
gsi_next (&i);
--- gcc/tree-flow.h.jj 2013-01-16 17:04:20.000000000 +0100
+++ gcc/tree-flow.h 2013-02-13 17:29:33.512538537 +0100
@@ -609,6 +609,7 @@ struct tree_niter_desc
/* In tree-ssa-phiopt.c */
bool empty_block_p (basic_block);
basic_block *blocks_in_phiopt_order (void);
+bool nonfreeing_call_p (gimple);
/* In tree-ssa-loop*.c */
--- gcc/tree-ssa-phiopt.c.jj 2013-02-08 16:05:14.000000000 +0100
+++ gcc/tree-ssa-phiopt.c 2013-02-13 17:28:47.442803866 +0100
@@ -1339,7 +1339,7 @@ add_or_mark_expr (basic_block bb, tree e
/* Return true when CALL is a call stmt that definitely doesn't
free any memory or makes it unavailable otherwise. */
-static bool
+bool
nonfreeing_call_p (gimple call)
{
if (gimple_call_builtin_p (call, BUILT_IN_NORMAL)
Jakub
