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

[Jakub Jelinek <jakub at redhat dot com>]

On Mon, Dec 03, 2012 at 10:32:52AM -0800, Wei Mi wrote:
> Jakub, thank you for your so detailed comments! I will fix them
> according to your comments. About the lto options, llvm test does't
> include it too so I skipped it in torture options. Is it because most
> cases we only use asan under O1/O2? Kostya, could you tell us is there
> any reason to not test lto+asan in llvm side?

The former lit-tests are usually single source file anyway, so I think
lto doesn't change much (and by testing also with lto (or -g) we actually
test that those option combinations work with asan too).  For the gtest
based tests it matters more, but those are also much more test time
intensive (at least asan_test.C is), so that one is for -O2 only.

> >> --- gcc/testsuite/c-c++-common/asan/strip-path-prefix-1.c     (revision 0)
> >> +++ gcc/testsuite/c-c++-common/asan/strip-path-prefix-1.c     (revision 0)
> >> @@ -0,0 +1,14 @@
> >> +/* { dg-do run } */
> >> +/* { dg-skip-if "" { *-*-* }  { "*" } { "-O2 -m64" } } */
> >
> > The -m64 here is just wrong.  If you want to run the test only
> > for -O2 and x86_64-linux compilation (why?, what is so specific
> > about it to that combination?), then you'd do
> > /* { dg-do run { target { { i?86-*-* x86_64-*-* } && lp64 } } } */
> > /* { dg-skip-if "" { *-*-* }  { "*" } { "-O2" } } */
> > or so.  But again, why?
> >
> 
> I copied it from llvm test. I think it just think -m64 test is enough
> to check the feature.

Yeah, I could understand it wants to check somewhere, and with
FILECHECK/llvm that is the way to do that.  The above dg-skip-if
will mean though that if you test say on i?86-linux target rather than
x86_64-linux, then it won't be tested at all, and I guess on x86_64-linux
when not using RUNTESTFLAGS='--target_board=unix\{-m32,-m64\}' it won't be
tested either, because -m64 is then not explicitly passed (it is the
default).
> 
> >> --- gcc/testsuite/c-c++-common/asan/force-inline-opt0-1.c     (revision 0)
> >> +++ gcc/testsuite/c-c++-common/asan/force-inline-opt0-1.c     (revision 0)
> >> @@ -0,0 +1,16 @@
> >> +/* This test checks that we are no instrumenting a memory access twice
> >> +   (before and after inlining) */
> >> +
> >> +/* { dg-do run } */
> >> +/* { dg-options "-Wno-attributes" } */
> >> +/* { dg-skip-if "" { *-*-* }  { "*" } { "-O0 -m64" "-O1 -m64" } } */
> >
> > As I said above.  Why is this not tested for 32-bit testing?
> > From the name, -O0/-O1 limit could make sense, but then even for -O2 and
> > above it should do the same.
> >
> 
> I also copied it from llvm.

As unlike the gtest based tests, these tests are copied + modified, I think
we should just do what makes sense for GCC testing.  And, please do
something about always_inline here too (== no -Wno-attributes).  Best if you
could for each comment of mine grep for similar things elsewhere in the
patch, I've commented only on some of the occurences, some things happen
in lots of testcases.

	Jakub