Jakub Jelinek <ja...@redhat.com> writes:
> 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).
This makes a lot of sense, thank you for doing this.
> And, now that we have nonfreeing_call_p predicate,
I didn't even know this existed ...
> we can avoid also flushing at builtin calls that are known not to
> free memory or use callbacks etc.
Right.
> 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.
Good catch.
> 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.
Maybe we could have this added to he GCC test suite?
> 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.
This looks OK to me. Thanks.
--
Dodji
