From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28558 invoked by alias); 28 Nov 2012 10:14:43 -0000 Received: (qmail 28548 invoked by uid 22791); 28 Nov 2012 10:14:42 -0000 X-SWARE-Spam-Status: No, hits=-6.5 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_CP,TW_LV X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 28 Nov 2012 10:14:24 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qASAENZ9029257 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 28 Nov 2012 05:14:23 -0500 Received: from zalov.redhat.com (vpn1-7-172.ams2.redhat.com [10.36.7.172]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id qASAELcl029985 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 28 Nov 2012 05:14:23 -0500 Received: from zalov.cz (localhost [127.0.0.1]) by zalov.redhat.com (8.14.5/8.14.5) with ESMTP id qASAELET004019; Wed, 28 Nov 2012 11:14:21 +0100 Received: (from jakub@localhost) by zalov.cz (8.14.5/8.14.5/Submit) id qASAEKxU004018; Wed, 28 Nov 2012 11:14:20 +0100 Date: Wed, 28 Nov 2012 10:14:00 -0000 From: Jakub Jelinek To: Wei Mi Cc: GCC Patches , David Li , Diego Novillo , Kostya Serebryany , Dodji Seketeli Subject: Re: [PATCH] asan unit tests from llvm lit-test Message-ID: <20121128101420.GG2315@tucnak.redhat.com> Reply-To: Jakub Jelinek References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2012-11/txt/msg02300.txt.bz2 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 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 > +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 > +#include > +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