asan unit tests from llvm lit-test

Thanks for the comments! Here is the second version patch. Please see
if it is ok.
(-Wno-attributes is kept or else we will get a warning because of
__attribute__((always_inline))).

These tests are excluded for now because unsupported features or needs
some twists to be included.
blacklist-1.c                             (use -asan-blacklist=)
initialization-blacklist-1.C         (use -asan-blacklist=)
initialization-bug-1.c                (use -asan-initialization-order)
initialization-nobug-1.C           (use -asan-initialization-order)
initialization-bug-any-order.cc (use -asan-initialization-order)
interface-symbols-1.c              (needs to be added to
libsanitizer/Makefile.am)
log-path_test.cc                       (needs to twist dg-final
commands, skip for now)

Test results:
gcc summary
# of expected passes		248
# of unexpected failures	        39
# of unsupported tests		27

g++ summary
# of expected passes            468
# of unexpected failures        56
# of unsupported tests          50

Thanks,
Wei.

On Wed, Nov 28, 2012 at 2:14 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> On Wed, Nov 28, 2012 at 01:15:20AM -0800, Wei Mi wrote:
>> I try to migrate the left asan lit-tests from llvm (class3). This is a
>> preliminary version patch. Please forgive it has many mistakes.
>
> Thanks for working on it.
>
>> A known problems: I hardcoded -m32 in (set link_flags
>> "[asan_link_flags [get_multilibs -m32]] $link_flags") in
>> gcc/testsuite/lib/asan-dg.exp to make 32 bit library path included in
>> ld_library_path. I don't know the elegant way to fix it.
>
> That is wrong, no *.exp file should do anything with -m32/-m64.
> If user wants to test both -m32 and -m64, it should be done through
> RUNTESTFLAGS='--target_board=unix\{-m32,-m64\}'
> on the command line of make check if desired (or any other options deemed
> necessary to test).  Not all targets support both -m32 and -m64 (e.g. even
> i686-linux doesn't), some targets have other ABI options (e.g. -m31/-m64 on
> s390x, mips has more variants, etc.).  It must be user's choice what he
> wants to test for what multilibs.
>
>>         * g++.dg/asan/linux: New, migrate from llvm asan lit-test.
>>         * g++.dg/asan/linux/interception-test-1.C: Likewise.
>>         * g++.dg/asan/linux/interception-failure-test-1.C: Likewise.
>>         * g++.dg/asan/linux/interception-malloc-test-1.C: Likewise.
>
> Why the linux/ subdirectories (which you seem to run for all targets
> anyway)?  That doesn't make any sense.  All tests that won't run on certain
> targets because of some required features (whether it is e.g. dlopen, mmap,
> pthreads) should be guarded, e.g.
> // { dg-require-effective-target pthread }
> or
> /* { dg-run { target pthread } } */
> and similar.  If some check_effective_target_* tcl test is missing, it can
> be always added (e.g. dlopen doesn't have any, and you can't assume dlopen
> works everywhere).
>
>>         * g++.dg/asan/Helpers: Likewise.
>>         * g++.dg/asan/Helpers/initialization-blacklist-1.tmp: Likewise.
>>         * g++.dg/asan/Helpers/initialization-blacklist-extra-1.C: Likewise.
>
> We aren't a CamelCase shop, I'd strongly prefer if we could avoid that
> ugliness.  Ditto for SharedLibs/ etc. subdirs.  And why you need the subdirs
> at all?  The usual way how to handle e.g. the dg-additional-sources is just
> make sure the additional sources are either named in a way that doesn't
> match the normal wildcard (for C++ e.g. *.cc instead of *.C) or add some dg
> directive in there that it won't run, or be dg-do compile only test etc.
>
>> +    if { [string match "*blacklist-1.c" $source] } {
>> +      set blacklist_options $options
>> +      set blist_tmp [glob $srcdir/c-c++-common/asan/Helpers/blacklist-1.tmp]
>> +      lappend blacklist_options "additional_flags=-asan-blacklist=$blist_tmp"
>> +      set result [eval [list saved_asan_target_compile $source $dest $type $blacklist_options]]
>> +      return $result
>> +    } elseif { [string match "*interface-symbols-1.c" $source] } {
>> +      set result [eval [list saved_asan_target_compile \
>> +                        $source "interface-symbols-1.exe" \
>> +                        "executable" $options]]
>> +      if { [string match "" $result] } {
>> +        set exefile [glob interface-symbols-1.exe]
>> +        set asan_interface_h [glob $srcdir/../../libsanitizer/include/sanitizer/asan_interface.h]
>> +        set script [glob $srcdir/c-c++-common/asan/Helpers/interface_symbols.sh]
>> +        set diff_result [exec sh $script $exefile $asan_interface_h]
>> +        if { ![string match "" $diff_result] } {
>> +          fail "$source -- diff result not empty: $diff_result"
>> +        }
>> +      }
>> +    } elseif { [string match "*initialization-bug-any-order-1.c" $source] } {
>> +      set auxfile [glob $srcdir/c-c++-common/asan/Helpers/initialization-bug-extra-1.c]
>> +      global subtest
>> +      if { [string match "subtest1" $subtest] } {
>> +        set source "$source $auxfile"
>> +      } else {
>> +        set source "$auxfile $source"
>> +      }
>> +      set result [eval [list saved_asan_target_compile $source $dest $type $options]]
>> +    } else {
>> +      set result [eval [list saved_asan_target_compile $source $dest $type $options]]
>> +    }
>
> This is too ugly.  asan.exp shouldn't turn into yet another vect.exp, the
> ideal is that for adding new tests you don't need to tweak any *.exp and add
> exceptions for that, unless there is no other way.  So, preferrably in asan/
> dir should stay tests that can be just handled the standard way, and there
> can be some extra subdirectory that will handle hard to handle tests.
> Say g++.dg/asan/special/ could have its own asan-special.exp or similar.
> Note that e.g. for building shared libraries you really need to guard it
> with appropriate target checks.
>
>> +foreach srcfile [lsort [glob -nocomplain \
>> +                        $srcdir/$subdir/*.c \
>> +                        $srcdir/c-c++-common/asan/*.c \
>> +                        $srcdir/c-c++-common/asan/linux/*.c]] {
>> +  set asan_torture_options $default_asan_torture_options
>> +  if { [string match "*force-inline-opt0-1.c" $srcfile] } {
>> +    set asan_torture_options [list { -O0 -m64 } { -O1 -m64 }]
>
> As said earlier, no -m64/-m32 here, and if at all possible, no special
> casing of tests in *.exp.  If you want to change the set of options
> at which some test is run, i.e. you don't want to iterate over all the
> options, just use dg-skip-if.  See
> http://gcc.gnu.org/onlinedocs/gccint/Directives.html
> for details about it.
>
>> +  } elseif { [string match "*sleep-before-dying-1.c" $srcfile] } {
>> +    setenv ASAN_OPTIONS "sleep_before_dying=1"
>> +    set asan_torture_options [list { -O2 }]
>
> For env options, I believe we don't have any dg directive right now to
> set env vars for runtime tests, but the best way would be to add it,
> dg-env-var or similar.  Or better yet, does libasan have a way to set
> the options through some function call?  Then you wouldn't have to
> set env vars...
>
>> --- gcc/testsuite/g++.dg/asan/deep-thread-stack-1.C   (revision 0)
>> +++ gcc/testsuite/g++.dg/asan/deep-thread-stack-1.C   (revision 0)
>> @@ -0,0 +1,55 @@
>> +/* { dg-do run } */
>> +/* { dg-shouldfail "asan" } */
>
> In C++ only tests, just use // comments instead of /* ... */
>> +
>> +#include <pthread.h>
>
> That is exactly where you need to require effective target pthread...
>
>> +/* { dg-options "-asan-initialization-order" } */
>
> AFAIK gcc doesn't have that option, and if it had, it wouldn't be of this
> form.  -fasan-initialization-order, --param asan-initialization-order=1 or
> similar, perhaps, but not -asan-initialization-order.
> But more generally, adding tests into GCC testsuite for unimplemented
> features is undesirable, you can prepare the tests and post a rough patch
> how they could look like, but it shouldn't be committed until the feature
> is implemented.
>
>> +/* { dg-additional-sources "Helpers/initialization-blacklist-extra-1.C" } */
>
>> +//
>> +//                     The LLVM Compiler Infrastructure
>
> I believe we've been removing the above two lines from libsanitizer, so they
> should probably be removed also from the tests?
>
>> --- gcc/testsuite/lib/asan-dg.exp     (revision 193881)
>> +++ gcc/testsuite/lib/asan-dg.exp     (working copy)
>> @@ -75,6 +75,7 @@ proc asan_init { args } {
>>           set link_flags "[asan_link_flags [get_multilibs ${TOOL_OPTIONS}]]"
>>       } else {
>>           set link_flags "[asan_link_flags [get_multilibs]]"
>> +         set link_flags "[asan_link_flags [get_multilibs -m32]] $link_flags"
>
> As has been said earlier, please don't do this.
>
>> --- gcc/testsuite/c-c++-common/asan/interface-symbols-1.c     (revision 0)
>> +++ gcc/testsuite/c-c++-common/asan/interface-symbols-1.c     (revision 0)
>> @@ -0,0 +1,24 @@
>> +// Check the presense of interface symbols in compiled file.
>> +
>> +// RUN: %clang -fsanitize=address -dead_strip -O2 %s -o %t.exe
>> +// RUN: nm %t.exe | egrep " [TW] " | sed "s/.* T //" | sed "s/.* W //" \
>> +// RUN:    | grep "__asan_" | sed "s/___asan_/__asan_/" > %t.symbols
>> +// RUN: cat %p/../../../include/sanitizer/asan_interface.h \
>> +// RUN:    | sed "s/\/\/.*//" | sed "s/typedef.*//" \
>> +// RUN:    | grep "__asan_.*(" | sed "s/.* __asan_/__asan_/;s/(.*//" \
>> +// RUN:    > %t.interface
>> +// RUN: echo __asan_report_load1 >> %t.interface
>> +// RUN: echo __asan_report_load2 >> %t.interface
>> +// RUN: echo __asan_report_load4 >> %t.interface
>> +// RUN: echo __asan_report_load8 >> %t.interface
>> +// RUN: echo __asan_report_load16 >> %t.interface
>> +// RUN: echo __asan_report_store1 >> %t.interface
>> +// RUN: echo __asan_report_store2 >> %t.interface
>> +// RUN: echo __asan_report_store4 >> %t.interface
>> +// RUN: echo __asan_report_store8 >> %t.interface
>> +// RUN: echo __asan_report_store16 >> %t.interface
>> +// RUN: cat %t.interface | sort -u | diff %t.symbols -
>> +
>> +/* { dg-options "-static-libasan -lpthread -ldl" } */
>> +
>> +int main() { return 0; }
>
> This kind of test IMHO doesn't belong to the dejagnu testsuite,
> if you really want to do it, it should be done somewhere in
> libsanitizer/asan/ Makefile.am as part of building the library.
>
>> --- 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,15 @@
>> +// This test checks that we are no instrumenting a memory access twice
>> +// (before and after inlining)
>> +
>> +/* { dg-do run } */
>> +/* { dg-options "-Wno-attributes" } */
>> +__attribute__((always_inline))
>
> Why -Wno-attributes?
>
>> +#include <string.h>
>> +int main(int argc, char **argv) {
>> +  static char XXX[10];
>> +  static char YYY[10];
>> +  static char ZZZ[10];
>> +  memset(XXX, 0, 10);
>> +  memset(YYY, 0, 10);
>> +  memset(ZZZ, 0, 10);
>> +  int res = YYY[argc * 10];  // BOOOM
>> +  res += XXX[argc] + ZZZ[argc];
>
> argc/argv using tests are not portable to all targets, you can't rely
> argc isn't e.g. zero.  Better just have some global variable, say,
> int one = 1;
> and at the beginning of main do asm volatile ("" : : : "memory");
> to let compiler forget about the value it has, or just make the variable
> volatile int one = 1;
>
>> @@ -0,0 +1,22 @@
>> +/* { dg-do run } */
>
> I'd expect you want /* { dg-options "-fno-builtin-strncpy" } */
> here, otherwise it is reported inside of main directly, rather than in the
> strncpy interceptor.
>
>> +/* { dg-shouldfail "asan" } */
>> +
>> +#include <string.h>
>> +#include <stdlib.h>
>> +int main(int argc, char **argv) {
>> +  char *hello = (char*)malloc(6);
>> +  strcpy(hello, "hello");
>> +  char *short_buffer = (char*)malloc(9);
>> +  strncpy(short_buffer, hello, 10);  // BOOM
>> +  return short_buffer[8];
>> +}
>> +
>> +/* { dg-output "WRITE of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */
>> +/* { dg-output "    #0 0x\[0-9a-f\]+ (in _*(interceptor_|)strncpy|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */
>> +/* { dg-output "    #1 0x\[0-9a-f\]+ (in _*main \[^\n\r]*(strncpy-overflow-1.c:10|\[?]\[?]:0)|\[(\]).*(\n|\r\n|\r)" } */
>> +/* { dg-output "0x\[0-9a-f\]+ is located 0 bytes to the right of 9-byte region\[^\n\r]*(\n|\r\n|\r)" } */
>> +/* { dg-output "allocated by thread T0 here:\[^\n\r]*(\n|\r\n|\r)" } */
>> +/* { dg-output "    #0 0x\[0-9a-f\]+ (in _*(interceptor_|)malloc|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */
>> +/* { dg-output "    #1 0x\[0-9a-f\]+ (in _*main \[^\n\r]*(strncpy-overflow-1.c:9|\[?]\[?]:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */
>
>         Jakub

asan unit tests from llvm lit-test

Commit Message

Comments

Patch