Re: [PATCH] Small asan improvement

[Dodji Seketeli <dodji at redhat dot com>]

Jakub Jelinek <jakub@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  <jakub@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