From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5467 invoked by alias); 30 Nov 2012 10:43:40 -0000 Received: (qmail 5457 invoked by uid 22791); 30 Nov 2012 10:43:40 -0000 X-SWARE-Spam-Status: No, hits=-5.4 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,TW_LV X-Spam-Check-By: sourceware.org Received: from mail-lb0-f175.google.com (HELO mail-lb0-f175.google.com) (209.85.217.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 30 Nov 2012 10:43:33 +0000 Received: by mail-lb0-f175.google.com with SMTP id gg13so339585lbb.20 for ; Fri, 30 Nov 2012 02:43:32 -0800 (PST) Received: by 10.152.128.9 with SMTP id nk9mr872046lab.17.1354272212702; Fri, 30 Nov 2012 02:43:32 -0800 (PST) MIME-Version: 1.0 Received: by 10.112.135.105 with HTTP; Fri, 30 Nov 2012 02:43:12 -0800 (PST) In-Reply-To: <20121130101420.GE2315@tucnak.redhat.com> References: <20121128102445.GH2315@tucnak.redhat.com> <20121128110327.GI2315@tucnak.redhat.com> <20121129204636.GC2315@tucnak.redhat.com> <20121130101420.GE2315@tucnak.redhat.com> From: Konstantin Serebryany Date: Fri, 30 Nov 2012 10:55:00 -0000 Message-ID: Subject: Re: [PATCH] asan_test.cc from llvm To: Jakub Jelinek Cc: Wei Mi , GCC Patches , David Li , Diego Novillo , Kostya Serebryany , Dodji Seketeli Content-Type: text/plain; charset=ISO-8859-1 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/msg02533.txt.bz2 I've upstreamed most of your changes, please take a look. Looks like we will be able to use libsanitizer/merge.sh to update the test code as well. On Fri, Nov 30, 2012 at 2:14 PM, Jakub Jelinek wrote: > On Fri, Nov 30, 2012 at 01:32:52PM +0400, Konstantin Serebryany wrote: >> Ideally, I would like to limit the differences from upstream. >> I'll put some of your changes upstream, for others I'd ask you to >> consider other choices. >> >> -#include "gtest/gtest.h" >> +#include "dejagnu-gtest.h" >> >> Maybe like this? >> >> #if ASAN_USE_DEJAGNU_GTEST >> #include "dejagnu-gtest.h" >> #else >> #include "gtest/gtest.h" >> #endif > > Sure, I'm fine with that. > >> Can we have gcc_asan_test.C which will #include the unchanged (modulo >> the comment header) asan_test.cc >> and have dejagnu lines there? >> >> Like this: >> // { dg-do run { target { mmap && pthread } } } >> ... >> #include asan_test.cc > > Yeah, I can do that, advantage will be that the testcase is named the same, > asan.exp currently runs only *.C testcases (so that *.cc is left to helper > files etc.). So there will be a small asan_test.C with the dg markup. > >> +#elif defined(__SANITIZE_ADDRESS__) >> + bool asan = 1; >> >> I'll put this upstream. > > Thanks, if GCC ever introduces the __has_feature magic macro, it can be > dropped, but for now... Note elsewhere I'm using the __SANITIZE_ADDRESS__ > define as a test whether this is for GCC or other compilers, because > I believe clang defines __GNUC__ macro or similar, I'd need to > #if defined (__GNUC__) && !defined (__clang__) or something? I'd prefer to avoid such hairy ifdefs... > >> +#ifdef __SANITIZE_ADDRESS__ >> + // Avoid this test during dejagnu testing, it is too expensive >> + if (getenv ("GCC_TEST_RUN_EXPENSIVE") == NULL) >> + return; >> +#endif >> >> I'd prefer to simply put this w/o ifdef. > > Then even in llvm the expensive test won't run unless you put > GCC_TEST_RUN_EXPENSIVE=1 into environment. That would be weird > for a llvm testcase to use GCC in the name. Alternative would be > to use some other env var name, like > ASAN_TEST_RUN_EXPENSIVE, and I could adjust asan.exp to set that > env var, or it could be a macro instead, guarding the whole test, > #ifndef ASAN_AVOID_EXPENSIVE_TESTS > (TEST(AddressSanitizer, ManyThreadsTest) { > ... > } > #endif > and I could in asan_test.C add: > // { dg-additional-options "-DASAN_AVOID_EXPENSIVE_TESTS" { target { ! run_expensive_tests } } } I can add ASAN_ALLOW_SLOW_TESTS to asan_test_config.h and then #if ASAN_ALLOW_SLOW_TESTS around ManyThreadsTest. I did not do it yet, because I would like to understand why it's slow. On my box (with clang+gtest) it runs in 0.6 seconds, and the entire test suite takes 60 seconds. Note that when running gtest tests in llvm test harness the tests are sharded, so on a multicore machine the test takes < 10 seconds. > > Or, if you want even upstream to avoid it by default, it could > be a positive test, #ifdef ASAN_RUN_EXPENSIVE_TESTS or similar. > >> >> -# error "please define ASAN_HAS_BLACKLIST" >> +//# error "please define ASAN_HAS_BLACKLIST" >> >> You can add -DASAN_HAS_BLACKLIST=0 to the command line. >> If/when gcc gets blacklist support, we'll redefine it to 1 > > Okay. > >> +#if __has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__) >> >> this is upstreamable >> >> +#ifdef __GNUC__ >> +# define break_optimization(arg) __asm__ __volatile__ ("" : : "r" >> (arg) : "memory") >> +#endif >> + >> >> That's a nice piece of magic, let me use this too. > > For pointers/integers it can be even as simple as > __asm__ ("" : "+g" (val)); > making compiler forget about val, obviously that doesn't work for > aggregates. Anyway, the reason for the macro is both that I wanted to > avoid compiling in another file, and for LTO it wouldn't work anyway. > > BTW, I had to add -Wno-unused-but-set-variable option because without > that there was a warning about one of the variables, I think it was > stack_string in StrLenOOBTest. Adding (void) stack_string; somewhere > would shut it up. I added break_optimization instead. > > Jakub