Re: How does address sanitizer handle read-modify-write memory access?

[Jakub Jelinek <jakub at redhat dot com>]

On Mon, Dec 17, 2012 at 11:12:43AM +0100, Dodji Seketeli wrote:
> Konstantin Serebryany <konstantin.s.serebryany@gmail.com> a écrit:
> 
> > When we have a code like X++ (either RMW, or a regular increment) it
> > is enough for asan to instrument it just once (either as a read or a
> > write, doesn't matter).
> > LLVM implementation does this optimization for regular increments,
> > while GCC does not (yet).
> >
> > % cat inc.cc
> > void foo(int *a) {
> >   (*a)++;
> > }
> > % clang -O2 -fsanitize=address -S -o -  inc.cc | grep __asan_report
> > 	callq	__asan_report_load4
> > % gcc -O2 -fsanitize=address -S -o -  inc.cc | grep __asan_report
> > 	call	__asan_report_load4
> > 	call	__asan_report_store4
> >
> > Doing two __asan_report* calls here is not a correctness bug, but a
> > performance problem.
> > I think we saw ~3%-5% performance gain due to this optimization in
> > LLVM, i.e. this is nice to have, but not critical.
> 
> Right.  I plan to work on this kind of optimizations on asan soonish.  I
> guess I should file a bug to track this in the mean time.

Yeah, the plan was first to introduce a new builtin, that is around only
between the asan/asan0 passes and some sanopt pass in between vectorizer and
expansion, teach alias oracle that the new builtin (which would stand for
the shadow memory address computation, test + noreturn call if load/store is
bad; probably with arguments of address, size and whether read/write) - that
the address doesn't really escape, and that it doesn't clobber any memory
the current TU is interested in, but can't be moved across calls that might
call malloc/free and similar, and then finally the sanopt pass to use
ssa_propagate or similar to find out what memory accesses have been already
instrumented in dominating place and we don't need to test them again, what
checks could be merged together (load + store into store check, wider size
check with narrower one), etc., then finally folding the remaining builtins
into the shadow memory load + noreturn call (perhaps that could be done
during expansion instead).

	Jakub