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

[Kostya Serebryany]

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.


--kcc



>
>
> Rietveld review split into two:
> llvm: http://codereview.appspot.com/6643048/
> clang: http://codereview.appspot.com/6635055/
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121015/23342361/attachment.html>