[llvm-commits] [PATCH] Add a known bounds optimization to ASan

[Kostya Serebryany]

Minor comments in Rietveld (http://codereview.appspot.com/6643048/ and
http://codereview.appspot.com/6635055/).
In general, I think this is very close to committable.
Maybe someone who knows ScalarEvolution.h can take a quick look?

--kcc


On Tue, Oct 16, 2012 at 4:49 PM, Ott Tinn <llvm at otinn.com> wrote:

> On 15 October 2012 17:26, Kostya Serebryany <kcc at google.com> wrote:
> >
> >
> > On Tue, Oct 9, 2012 at 6:33 PM, Ott Tinn <llvm at otinn.com> wrote:
> >>
> >> On 9 October 2012 12:28, Kostya Serebryany <kcc at google.com> wrote:
> >> > Hi Ott.
> >> > In general, I like your patch and the whole idea to use an existing
> LLVM
> >> > analysis pass to optimize away redundant asan checks.
> >> > Please also send your performance and compile-time numbers here --
> they
> >> > look
> >> > nice.
> >>
> >> Performance results based on compiling chrome + browser_tests and
> >> executing chrome's browser_tests with and without the patch. The
> >> compilation was repeated 3 times (release build from scratch) and the
> >> browser_tests execution was repeated 7 times.
> >>
> >> property | without patch | with patch (reduction)
> >> run time | 2051.5 | 2016 (1.7%)
> >> compile time | 13553.4 | 13294.4 (1.9%)
> >> chrome size | 1846 MB | 1809 MB (2%)
> >> browser_tests size | 2015 MB | 1969 MB (2.3%)
> >>
> >>
> >>
> >> > On Tue, Oct 9, 2012 at 3:13 PM, Ott Tinn <llvm at otinn.com> wrote:
> >> >>
> >> >> Hi,
> >> >>
> >> >> The attached patches add an optimization to ASan that makes it avoid
> >> >> adding run-time checks to memory accesses that are always in bounds
> of
> >> >> valid (not deallocated) memory objects. The current version works on
> >> >> simple allocas, global variables, and byval arguments using the
> >> >> ScalarEvolution analysis.
> >> >>
> >> >> The included lit tests should work without the Clang patch but some
> of
> >> >> the existing ASan tests would fail without it.
> >> >>
> >> >> The ScalarEvolution and DataLayout complications seem to be caused by
> >> >> a module pass (AddressSanitizer) requiring a function pass
> >> >> (ScalarEvolution) which in turn wants to use an immutable pass
> >> >> (DataLayout).
> >> >
> >> >
> >> > Let's resolve this problem first.
> >> > Will it help if we make AddressSanitizer to be a FunctionPass?
> >> > I remember Chandler asking to do that, but don't remember the
> motivation
> >> > (other than "good for future parallelization")
> >> > (AddressSanitizer is currently a ModulePass for historical reasons,
> >> > ThreadSanitizer is already a FunctionPass).
> >> > If converting AddressSanitizer to a FunctionPass will simplify this
> >> > patch,
> >> > let's do it separately (I can do it myself, but only on the next
> week).
> >> As far as I see it would not remove the need for the clang patch.
> >>
> >> It should remove the need for ScalarEvolution.setDataLayout and
> >> definitely get rid of the per-function getAnalysis<ScalarEvolution>.
> >
> >
> > Ott,
> >
> > I've made asan to be a FunctionPass (r165936, r165937), hopefully it will
> > simplify your patch a bit.
> This change removed the need for the ScalarEvolution changes and the
> need to track the initial global variable sizes.
>
> New version of the patch attached (the Clang side still needs the same
> changes).
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121018/f1d30222/attachment.html>