Re: [PATCH] asan unit tests from llvm lit-test incremental changes

[Jakub Jelinek <jakub@redhat.com>]

On Wed, Dec 12, 2012 at 10:16:49PM +0100, Dodji Seketeli wrote:
> Independently of this review, I think it's be interesting to hear
> Kostya's voice on:
> 
> Jakub Jelinek <jakub@redhat.com> writes:
> 
> > 2) In large-func-test-1.C, I had to stop matching the backtrace after
> > _Znw[jm], because libasan is using the fast but inaccurate backtrace,
> > and while the tests can be easily tweaked to compile with
> > -fno-omit-frame-pointer, we definitely can't rely on libstdc++.so to be
> > built with that option.  Most likely it isn't.  I repeat that I think
> > that at least for Linux libasan should use the _Unwind* based backtrace
> > at least for the fatal functions (__asan_report* etc.), and perhaps for
> > these malloc wrappers like ::operator new, ::operator new[] and their
> > const std::nothrow_t& variants libasan could intercept them, call
> > malloc and if that returns NULL, call the original corresponding function
> > so that it deals with exceptions, new handler etc.

Yeah, I'd appreciate that too.

> and on:
> 
> > 3) deep-thread-stack-1.C fails for me right now with some libasan assertion,
> > Kostya, can you please look at that?
> >   AsanThread *t = asanThreadRegistry().GetCurrent();
> >   CHECK(t);
> > where it failed on the CHECK, because t was NULL.  I've skipped the test for
> > now.
> 
> [...]

This one is for the testcase solved right now already by the -lasan -lpthread 
linking instead of just -lpthread (and driver adding -lasan afterwards).
We'll need to think about how to tweak the driver to add -lasan early on the
command line, before user passed -l* options.
> 
> > --- gcc/testsuite/g++.dg/asan/deep-tail-call-1.C.jj	2012-12-04 20:24:10.000000000 +0100
> > +++ gcc/testsuite/g++.dg/asan/deep-tail-call-1.C	2012-12-05 11:01:48.600443834 +0100
> > @@ -1,21 +1,22 @@
> > -// { dg-do run } 
> > +// { dg-do run }
> >  // { dg-options "-fno-omit-frame-pointer -fno-optimize-sibling-calls" }
> >  // { dg-additional-options "-mno-omit-leaf-frame-pointer" { target { i?86-*-* x86_64-*-* } } }
> > -// { dg-shouldfail "asan" } 
> > +// { dg-shouldfail "asan" }
> >  
> >  int global[10];
> >  void __attribute__((noinline)) call4(int i) { global[i+10]++; }
> >  void __attribute__((noinline)) call3(int i) { call4(i); }
> >  void __attribute__((noinline)) call2(int i) { call3(i); }
> >  void __attribute__((noinline)) call1(int i) { call2(i); }
> > -int main(int argc, char **argv) {
> > -  call1(argc);
> > +volatile int one = 1;
> 
> Just curious, why do we need this variable to be volatile, especially
> since the test is compiled without optimization?

asan.exp tests are torture tests, they iterate over several -O* options,
unless explicitly dg-skip-if skipped.  It could be non-volatile with
asm volatile ("" : : : "memory");
or asm volatile ("" : "+m" (one)); or similar too, sure.
I just don't want to rely on argc being one, and the compiler shouldn't know
that one is 1 in the test.

> [...]
> 
> The patch looks OK to me in any case.

Thanks.

	Jakub