From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18191 invoked by alias); 3 Dec 2012 18:49:30 -0000 Received: (qmail 18182 invoked by uid 22791); 3 Dec 2012 18:49:29 -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_KC,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; Mon, 03 Dec 2012 18:49:16 +0000 Received: by mail-lb0-f175.google.com with SMTP id gg13so2553211lbb.20 for ; Mon, 03 Dec 2012 10:49:15 -0800 (PST) Received: by 10.152.103.18 with SMTP id fs18mr10344169lab.32.1354560554905; Mon, 03 Dec 2012 10:49:14 -0800 (PST) MIME-Version: 1.0 Received: by 10.112.135.105 with HTTP; Mon, 3 Dec 2012 10:48:54 -0800 (PST) In-Reply-To: References: <20121128101420.GG2315@tucnak.redhat.com> <20121203110018.GR2315@tucnak.redhat.com> From: Konstantin Serebryany Date: Mon, 03 Dec 2012 18:49:00 -0000 Message-ID: Subject: Re: [PATCH] asan unit tests from llvm lit-test To: Wei Mi Cc: Jakub Jelinek , Mike Stump , 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-12/txt/msg00153.txt.bz2 On Mon, Dec 3, 2012 at 10:32 PM, Wei Mi wrote: > Hi, > > 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 existing tests may fail with lto because lto does more aggressive optimizations. No other reasons. --kcc > > Thanks, > Wei. > > On Mon, Dec 3, 2012 at 3:00 AM, Jakub Jelinek wrote: >> Hi! >> >> Mike, CCing you especially on the proposed lib/gcc-dg.exp dg-env-var >> changes and I have one question about cleanup of files (file delete >> vs. remote_file target (or is that host or build) delete). >> But of course if you could eyeball the rest and comment, I'd be even happier. >> >> On Fri, Nov 30, 2012 at 12:35:35PM -0800, Wei Mi wrote: >>> 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))). >> >>> --- gcc/testsuite/gcc.dg/asan/asan.exp (revision 194002) >>> +++ gcc/testsuite/gcc.dg/asan/asan.exp (working copy) >>> @@ -30,6 +30,10 @@ if ![check_effective_target_faddress_san >>> dg-init >>> asan_init >>> >>> +# Set default torture options >>> +set default_asan_torture_options [list { -O0 } { -O1 } { -O2 } { -O3 }] >>> +set-torture-options $default_asan_torture_options >> >> Why this? What is undesirable on the default torture options? >> Do those tests fail with lto or similar? >> > > tests on llvm side don't contain lto option so I do the same. Some > tests fail with lto because more aggressive inline. > >>> Index: gcc/testsuite/c-c++-common/asan/strip-path-prefix-1.c >>> =================================================================== >>> --- 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. > >>> --- 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. > >>> --- gcc/testsuite/g++.dg/asan/deep-stack-uaf-1.C (revision 0) >>> +++ gcc/testsuite/g++.dg/asan/deep-stack-uaf-1.C (revision 0) >>> @@ -0,0 +1,33 @@ >>> +// Check that we can store lots of stack frames if asked to. >>> + >>> +// { dg-do run } >>> +// { dg-env-var ASAN_OPTIONS "malloc_context_size=120:redzone=512" } >>> +// { dg-shouldfail "asan" } >> >> Can you please replace the two spaces after // with just one? >> Dejagnu directives are often quite long, and thus it is IMHO better to make >> the lines longer than necessary. >> For this test, don't you need >> // { dg-options "-fno-optimize-sibling-calls" } >> and __attribute__((noinline)) on the free method? Otherwise I'd expect >> that either at least at -O3 it could be all inlined, or if not inlined, then >> at least tail call optimized (and thus not showing up in the backtrace >> either). >> > > Ok, will fix it. > >>> +#include >>> +#include >>> + >>> +template >>> +struct DeepFree { >>> + static void free(char *x) { >>> + DeepFree::free(x); >>> + } >>> +}; >>> + >>> +template<> >>> +struct DeepFree<0> { >>> + static void free(char *x) { >>> + ::free(x); >>> + } >>> +}; >>> + >>> +int main() { >>> + char *x = new char[10]; >>> + // deep_free(x); >>> + DeepFree<200>::free(x); >>> + return x[5]; >>> +} >>> + >>> +// { dg-output "ERROR: AddressSanitizer:? heap-use-after-free on address.*(\n|\r\n|\r)" } >>> +// { dg-output " #37 0x\[0-9a-f\]+ (in \[^\n\r]*DeepFree\[^\n\r]*36|\[(\]).*(\n|\r\n|\r)" } >>> +// { dg-output " #99 0x\[0-9a-f\]+ (in \[^\n\r]*DeepFree\[^\n\r]*98|\[(\]).*(\n|\r\n|\r)" } >>> +// { dg-output " #116 0x\[0-9a-f\]+ (in \[^\n\r]*DeepFree\[^\n\r]*115|\[(\])\[^\n\r]*(\n|\r\n|\r)" } >> >>> --- gcc/testsuite/g++.dg/asan/deep-tail-call-1.C (revision 0) >>> +++ gcc/testsuite/g++.dg/asan/deep-tail-call-1.C (revision 0) >>> @@ -0,0 +1,20 @@ >>> +// { dg-do run } >>> +// { dg-options "-mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls" } >> >> -mno-omit-leaf-frame-pointer is i?86/x86_64 options, so you need to leave it >> from dg-options and add >> // { dg-additional-options "-mno-omit-leaf-frame-pointer" { target { i?86-*-* x86_64-*-* } } } >> > > Ok, will fix it. > >>> --- gcc/testsuite/g++.dg/asan/asan.exp (revision 194002) >>> +++ gcc/testsuite/g++.dg/asan/asan.exp (working copy) >>> @@ -28,9 +28,15 @@ if ![check_effective_target_faddress_san >>> dg-init >>> asan_init >>> >>> +# Set default torture options >>> +set default_asan_torture_options [list { -O0 } { -O1 } { -O2 } { -O3 }] >>> +set-torture-options $default_asan_torture_options >> >> Again, like I asked earlier. >> >>> + >>> # Main loop. >>> gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C $srcdir/c-c++-common/asan/*.c]] "" >>> >>> +source $srcdir/$subdir/special/special.exp >> >> Won't this cause double testing of the special tests? AFAIK dejagnu is >> looking recursively for all *.exp files, so once you'd source it when >> running asan.exp and again when dejagnu finds special.exp on its own. >> If that is the case, then you shouldn't source it here, and rename >> special.exp to say asan-special.exp, so that one can test all asan >> tests with just RUNTESTFLAGS="--target_board=unix\{-m32,-m64\} asan.exp asan-special.exp" >> but also make check will DTRT. Or perhaps name it also asan.exp, see if >> RUNTESTFLAGS="--target_board=unix\{-m32,-m64\} asan.exp" >> then will DTRT and also make check? >> > > Ok, I will try that. > >>> --- gcc/testsuite/g++.dg/asan/interception-test-1.C (revision 0) >>> +++ gcc/testsuite/g++.dg/asan/interception-test-1.C (revision 0) >>> @@ -0,0 +1,22 @@ >>> +// ASan interceptor can be accessed with __interceptor_ prefix. >>> + >>> +// { dg-do run } >>> +// { dg-shouldfail "asan" } >>> + >>> +#include >>> +#include >>> + >>> +extern "C" long __interceptor_strtol(const char *nptr, char **endptr, int base); >>> +extern "C" long strtol(const char *nptr, char **endptr, int base) { >>> + fprintf(stderr, "my_strtol_interceptor\n"); >>> + return __interceptor_strtol(nptr, endptr, base); >>> +} >>> + >>> +int main() { >>> + char *x = (char*)malloc(10 * sizeof(char)); >> >> Ugh, why the * sizeof(char)? That is completely pointless... > > I will fix it. > >> >>> --- gcc/testsuite/g++.dg/asan/large-func-test-1.C (revision 0) >>> +++ gcc/testsuite/g++.dg/asan/large-func-test-1.C (revision 0) >>> @@ -0,0 +1,47 @@ >>> +// { dg-do run } >>> +// { dg-shouldfail "asan" } >>> + >>> +#include >>> +__attribute__((noinline)) >>> +static void LargeFunction(int *x, int zero) { >>> + x[0]++; >>> + x[1]++; >>> + x[2]++; >>> + x[3]++; >>> + x[4]++; >>> + x[5]++; >>> + x[6]++; >>> + x[7]++; >>> + x[8]++; >>> + x[9]++; >>> + >>> + x[zero + 111]++; // we should report this exact line >>> + >>> + x[10]++; >>> + x[11]++; >>> + x[12]++; >>> + x[13]++; >>> + x[14]++; >>> + x[15]++; >>> + x[16]++; >>> + x[17]++; >>> + x[18]++; >>> + x[19]++; >>> +} >>> + >>> +int main(int argc, char **argv) { >>> + int *x = new int[100]; >>> + LargeFunction(x, argc - 1); >>> + delete x; >>> +} >>> + >>> +// { dg-output "ERROR: AddressSanitizer:? heap-buffer-overflow on address\[^\n\r]*" } >>> +// { dg-output "0x\[0-9a-f\]+ at pc 0x\[0-9a-f\]+ bp 0x\[0-9a-f\]+ sp 0x\[0-9a-f\]+\[^\n\r]*(\n|\r\n|\r)" } >>> +// { dg-output "READ of size 4 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } >>> +// { dg-output " #0 0x\[0-9a-f\]+ (in \[^\n\r]*LargeFunction\[^\n\r]*(large-func-test-1.C:18|\[?\]\[?\]:0)|\[(\]).*(\n|\r\n|\r)" } >>> +// { dg-output "0x\[0-9a-f\]+ is located 44 bytes to the right of 400-byte region.*(\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 (operator new|_Znwm)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } >>> +// { dg-output " #2 0x\[0-9a-f\]+ (in (operator new|_Znam)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } >>> +// { dg-output " #3 0x\[0-9a-f\]+ (in _*main\[^\n\r]*(large-func-test-1.C:33|\[?\]\[?\]:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } >>> Index: gcc/testsuite/g++.dg/asan/dlclose-test-1-so.C >>> =================================================================== >>> --- gcc/testsuite/g++.dg/asan/dlclose-test-1-so.C (revision 0) >>> +++ gcc/testsuite/g++.dg/asan/dlclose-test-1-so.C (revision 0) >> >> Name it dlclose-test-1.so.cc instead? > > Ok, will fix it. > >> >>> +// { dg-skip-if "" { *-*-* } { "*" } { "" } } >> >>> --- gcc/testsuite/g++.dg/asan/special/dlclose-test-1.C (revision 0) >>> +++ gcc/testsuite/g++.dg/asan/special/dlclose-test-1.C (revision 0) >>> @@ -0,0 +1,69 @@ >>> +// Regression test for >>> +// http://code.google.com/p/address-sanitizer/issues/detail?id=19 >>> +// Bug description: >>> +// 1. application dlopens foo.so >>> +// 2. asan registers all globals from foo.so >>> +// 3. application dlcloses foo.so >>> +// 4. application mmaps some memory to the location where foo.so was before >>> +// 5. application starts using this mmaped memory, but asan still thinks there >>> +// are globals. >>> +// 6. BOOM >>> + >>> +// { dg-do run } >>> +// { dg-require-effective-target "dlopen" } >>> +// { dg-require-effective-target "mmap" } >> >> My preference would be // { dg-do run { target { dlopen && mmap } } } >> In any case, no need for "s around the dlopen/mmap/pthread etc. > > Ok, I will fix it. > >>> + >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> + >>> +#include >>> + >>> +using std::string; >>> + >>> +static const int kPageSize = 4096; >>> + >>> +typedef int *(fun_t)(); >>> + >>> +int main(int argc, char *argv[]) { >>> + string path = string(argv[0]) + "-so.so"; >>> + printf("opening %s ... \n", path.c_str()); >>> + void *lib = dlopen(path.c_str(), RTLD_NOW); >>> + if (!lib) { >>> + printf("error in dlopen(): %s\n", dlerror()); >>> + return 1; >>> + } >>> + fun_t *get = (fun_t*)dlsym(lib, "get_address_of_static_var"); >>> + if (!get) { >>> + printf("failed dlsym\n"); >>> + return 1; >>> + } >>> + int *addr = get(); >>> + //assert(((size_t)addr % 32) == 0); // should be 32-byte aligned. >>> + printf("addr: %p\n", addr); >>> + addr[0] = 1; // make sure we can write there. >>> + >>> + // Now dlclose the shared library. >>> + printf("attempting to dlclose\n"); >>> + if (dlclose(lib)) { >>> + printf("failed to dlclose\n"); >>> + return 1; >>> + } >>> + // Now, the page where 'addr' is unmapped. Map it. >>> + size_t page_beg = ((size_t)addr) & ~(kPageSize - 1); >>> + void *res = mmap((void*)(page_beg), kPageSize, >>> + PROT_READ | PROT_WRITE, >>> + MAP_PRIVATE | MAP_ANON | MAP_FIXED | MAP_NORESERVE, 0, 0); >>> + if (res == (char*)-1L) { >>> + printf("failed to mmap\n"); >>> + return 1; >>> + } >>> + addr[1] = 2; // BOOM (if the bug is not fixed). >>> + printf("PASS\n"); >>> + // CHECK: PASS >>> + return 0; >>> +} >>> + >>> +// { dg-output "PASS" } >> >> Isn't printf("PASS\n"); and dg-output completely unnecessary here? >> If the test doesn't reach the return 0, the test will fail (the canonical >> way of failing is abort ();, but for asan I agree it is better to exit with >> non-zero status, because the asan multi-terrabyte mappings cause slowdowns >> e.g. with abrt or if cores are enabled) the execution test part, if it >> reaches there, it will pass the execution test, by testing dg-output you >> are adding another dejagnu accounted test (another pass/fail/unsupported >> item), but it tests exactly what has been tested before already. > > Ok, I will fix it. > >>> Index: gcc/testsuite/g++.dg/asan/special/shared-lib-test-1.C >>> =================================================================== >>> --- gcc/testsuite/g++.dg/asan/special/shared-lib-test-1.C (revision 0) >>> +++ gcc/testsuite/g++.dg/asan/special/shared-lib-test-1.C (revision 0) >>> @@ -0,0 +1,34 @@ >>> +// { dg-do run } >>> +// { dg-require-effective-target "dlopen" } >>> +// { dg-shouldfail "asan" } >>> + >>> +#include >>> +#include >>> +#include >>> + >>> +#include >>> + >>> +using std::string; >>> + >>> +typedef void (fun_t)(int x); >>> + >>> +int main(int argc, char *argv[]) { >>> + string path = string(argv[0]) + "-so.so"; >>> + printf("opening %s ... \n", path.c_str()); >>> + void *lib = dlopen(path.c_str(), RTLD_NOW); >>> + if (!lib) { >>> + printf("error in dlopen(): %s\n", dlerror()); >>> + return 1; >>> + } >>> + fun_t *inc = (fun_t*)dlsym(lib, "inc"); >>> + if (!inc) return 1; >>> + printf("ok\n"); >>> + inc(1); >>> + inc(-1); // BOOM >>> + return 0; >>> +} >>> + >>> +// { dg-output "ERROR: AddressSanitizer:? global-buffer-overflow\[^\n\r]*(\n|\r\n|\r)" } >>> +// { dg-output "READ of size 4 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } >>> +// { dg-output " #0 0x\[0-9a-f\]+\[^\n\r]*(\n|\r\n|\r)" } >>> +// { dg-output " #1 0x\[0-9a-f\]+ (in _*main \[^\n\r]*(shared-lib-test-1.C:27|\[?\]\[?\]:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } >>> Index: gcc/testsuite/g++.dg/asan/special/special.exp >>> =================================================================== >>> --- gcc/testsuite/g++.dg/asan/special/special.exp (revision 0) >>> +++ gcc/testsuite/g++.dg/asan/special/special.exp (revision 0) >>> @@ -0,0 +1,59 @@ >>> +# Copyright (C) 2012 Free Software Foundation, Inc. >>> +# >>> +# This file is part of GCC. >>> +# >>> +# GCC is free software; you can redistribute it and/or modify >>> +# it under the terms of the GNU General Public License as published by >>> +# the Free Software Foundation; either version 3, or (at your option) >>> +# any later version. >>> +# >>> +# GCC is distributed in the hope that it will be useful, >>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of >>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> +# GNU General Public License for more details. >>> +# >>> +# You should have received a copy of the GNU General Public License >>> +# along with GCC; see the file COPYING3. If not see >>> +# . >>> + >>> +# Handle special tests >>> +if { [info procs target_compile] != [list] \ >>> + && [info procs saved_asan_target_compile] == [list] } { >>> + rename target_compile saved_asan_target_compile >>> + >>> + proc target_compile { source dest type options } { >>> + global srcdir subdir >>> + >>> + if { [string match "*dlclose-test-1.C" $source] } { >>> + set dlclose_so_options $options >>> + lappend dlclose_so_options "additional_flags=-fPIC -shared" >>> + set auxfile [glob $srcdir/$subdir/dlclose-test-1-so.C] >>> + set result [eval [list saved_asan_target_compile \ >>> + $auxfile \ >>> + "dlclose-test-1.exe-so.so" \ >>> + "executable" $dlclose_so_options]] >>> + } elseif { [string match "*shared-lib-test-1.C" $source] } { >>> + set shared_lib_so_options $options >>> + lappend shared_lib_so_options "additional_flags=-fPIC -shared" >>> + set auxfile [glob $srcdir/$subdir/shared-lib-test-1-so.C] >>> + set result [eval [list saved_asan_target_compile \ >>> + $auxfile \ >>> + "shared-lib-test-1.exe-so.so" \ >>> + "executable" $shared_lib_so_options]] >>> + } >>> + set result [eval [list saved_asan_target_compile $source $dest $type $options]] >>> + return $result >> >> I'm missing hre cleaning up of the created shared libraries, are you sure >> they aren't kept in the g++/testsuite/g++/ directory after make check? >> >> Plus, if this *.exp file is renamed to asan.exp or asan-special.exp and >> not sourced in from the upper directory asan.exp, it needs to start/end with >> what asan.exp does. >> >>> +if { [info procs saved_asan_target_compile] != [list] } { >>> + rename target_compile "" >>> + rename saved_asan_target_compile target_compile >>> +} >>> + >>> +# Clean .so generated by special tests. >>> +file delete dlclose-test-1.exe-so.so >>> +file delete shared-lib-test-1.exe-so.so >> >> Ah, it is here, but wonder what it will do for cross testing. >> Shouldn't that be remove_file ? delete where ? is either target, or host, or >> build (not sure which one). Mike? >> >>> --- gcc/testsuite/g++.dg/asan/shared-lib-test-1-so.C (revision 0) >>> +++ gcc/testsuite/g++.dg/asan/shared-lib-test-1-so.C (revision 0) >> >> Again, *-so.cc ? > > Ok. > >> >>> +// { dg-skip-if "" { *-*-* } { "*" } { "" } } >> >>> --- gcc/testsuite/lib/gcc-dg.exp (revision 194002) >>> +++ gcc/testsuite/lib/gcc-dg.exp (working copy) >>> @@ -254,7 +254,16 @@ if { [info procs ${tool}_load] != [list] >>> proc ${tool}_load { program args } { >>> global tool >>> global shouldfail >>> + global set_env_var >>> + >>> + set saved_env_var [list] >>> + if { [llength $set_env_var] != 0 } { >>> + set-env-var >>> + } >>> set result [eval [list saved_${tool}_load $program] $args] >>> + if { [llength $set_env_var] != 0 } { >>> + restore-env-var >>> + } >>> if { $shouldfail != 0 } { >>> switch [lindex $result 0] { >>> "pass" { set status "fail" } >>> @@ -266,6 +275,37 @@ if { [info procs ${tool}_load] != [list] >>> } >>> } >>> >>> +proc dg-env-var { args } { >>> + global set_env_var >>> + if { [llength $args] != 3 } { >>> + error "[lindex $args 1]: need two arguments" >>> + return >>> + } >>> + lappend set_env_var [list [lindex $args 1] [lindex $args 2]] >>> +} >>> + >>> +proc set-env-var { } { >>> + global set_env_var >>> + upvar 1 saved_env_var saved_env_var >>> + foreach env_var $set_env_var { >>> + set var [lindex $env_var 0] >>> + set value [lindex $env_var 1] >>> + if [info exists env($var)] { >>> + lappend saved_env_var [list $var $env($var)] >>> + } >>> + setenv $var $value >>> + } >>> +} >>> + >>> +proc restore-env-var { } { >>> + upvar 1 saved_env_var saved_env_var >>> + foreach env_var $saved_env_var { >>> + set var [lindex $env_var 0] >>> + set value [lindex $env_var 1] >>> + unsetenv $var $value >>> + } >>> +} >>> + >>> # Utility routines. >>> >>> # >>> @@ -287,6 +327,10 @@ proc search_for { file pattern } { >>> # as c-torture does. >>> proc gcc-dg-runtest { testcases default-extra-flags } { >>> global runtests >>> + global set_env_var >>> + >>> + # Init set_env_var >>> + set set_env_var [list] >>> >>> # Some callers set torture options themselves; don't override those. >>> set existing_torture_options [torture-options-exist] >> >> For this, I'd appreciate Mike's input. If it is useful for all tests >> generally (I'd say it is, we could use it e.g. for testing some of the >> libgomp env vars), then it should stay here or so, otherwise it would need >> to be moved into asan-dg.exp and have asan in the name. >> >> More importantly, I'm wondering about dg-env-var vs. cross testing, I guess >> env var is set on host only, not remotely set on the target. So, either >> we should mark all tests that use dg-env-var with some special effective >> target that would be basically [is_native] - or what is the way to limit >> tests to native testing only, or dg-evn-var itself should arrange to just >> make the whole test unsupported if not native (don't call ${tool}_load >> at all and return something else?). >> > >>> +/* { dg-env-var ASAN_OPTIONS "strip_path_prefix='/'" } */ >>> +/* { dg-shouldfail "asan" } */ >>> + >>> +#include >>> +int main() { >>> + char *x = (char*)malloc(10 * sizeof(char)); >>> + free(x); >>> + return x[5]; >>> +} >>> + >>> +/* { dg-output "heap-use-after-free.*(\n|\r\n|\r)" } */ >>> +/* { dg-output " #0 0x\[0-9a-f\]+ \[(\]\[^/\]\[^\n\r]*(\n|\r\n|\r)" } */ >> > > >>> +__attribute__((always_inline)) >> >> Please drop -Wno-attributes above, and instead DTRT, i.e. >> together with __attribute__((always_inline)) always use also inline keyword. >> always_inline attribute alone is invalid on functions not marked as inline. >> > > Ok. > >>> +void foo(int *x) { >>> + *x = 0; >>> +} >>> + >>> +int main() { >>> + int x; >>> + foo(&x); >>> + return x; >>> +} >> >> But of course, the test actually doesn't test anything at all, there is >> no check for it not being instrumented twice, you'd use >> dg-do compile test for it instead, and test assembly in dg-final or similar. >> Except that there are no memory accesses at all, at least for -O1 >> by the time this reaches the asan pass I'm pretty sure it will be just >> int main() { return 0; } >> (perhaps with DEBUG x => 0 for -g). >> Then it will be very dependent on whether the foo function is emitted >> or not (which depends on C vs. C++ and for C on 89 vs 99 vs. >> -fgnu89-inline). So, for -O1 main won't contain any instrumented accesses, >> and foo either won't be emitted at all, or will contain one store. >> For -O0 for main it will contain one insturmented store, and for foo the >> same as for -O1. So you could >> /* { dg-final { scan-assembler-not "__asan_report_load" } } */ > > Ok, thanks. > >> >>> Index: gcc/testsuite/c-c++-common/asan/swapcontext-test-1.c >>> =================================================================== >>> --- gcc/testsuite/c-c++-common/asan/swapcontext-test-1.c (revision 0) >>> +++ gcc/testsuite/c-c++-common/asan/swapcontext-test-1.c (revision 0) >>> @@ -0,0 +1,62 @@ >>> +/* Check that ASan plays well with easy cases of makecontext/swapcontext. */ >>> + >>> +/* { dg-do run } */ >>> +/* { dg-require-effective-target "swapcontext" } */ >>> + >>> +#include >>> +#include >>> +#include >>> + >>> +ucontext_t orig_context; >>> +ucontext_t child_context; >>> + >>> +void Child(int mode) { >>> + char x[32] = {0}; /* Stack gets poisoned. */ >>> + printf("Child: %p\n", x); >>> + /* (a) Do nothing, just return to parent function. >>> + (b) Jump into the original function. Stack remains poisoned unless we do >>> + something. */ >>> + if (mode == 1) { >>> + if (swapcontext(&child_context, &orig_context) < 0) { >>> + perror("swapcontext"); >>> + _exit(0); >>> + } >>> + } >>> +} >>> + >>> +int Run(int arg, int mode) { >>> + int i; >>> + const int kStackSize = 1 << 20; >>> + char child_stack[kStackSize + 1]; >>> + printf("Child stack: %p\n", child_stack); >>> + /* Setup child context. */ >>> + getcontext(&child_context); >>> + child_context.uc_stack.ss_sp = child_stack; >>> + child_context.uc_stack.ss_size = kStackSize / 2; >>> + if (mode == 0) { >>> + child_context.uc_link = &orig_context; >>> + } >>> + makecontext(&child_context, (void (*)())Child, 1, mode); >>> + if (swapcontext(&orig_context, &child_context) < 0) { >>> + perror("swapcontext"); >>> + return 0; >>> + } >>> + /* Touch childs's stack to make sure it's unpoisoned. */ >>> + for (i = 0; i < kStackSize; i++) { >>> + child_stack[i] = i; >>> + } >>> + return child_stack[arg]; >>> +} >>> + >>> +int main(int argc, char **argv) { >>> + int ret = 0; >>> + ret += Run(argc - 1, 0); >>> + printf("Test1 passed\n"); >>> + ret += Run(argc - 1, 1); >>> + printf("Test2 passed\n"); >>> + return ret; >>> +} >>> + >>> +/* { dg-output "WARNING: ASan doesn't fully support makecontext/swapcontext.*" } */ >>> +/* { dg-output "Test1 passed.*" } */ >>> +/* { dg-output "Test2 passed.*" } */ >>> Index: gcc/testsuite/c-c++-common/asan/null-deref-1.c >>> =================================================================== >>> --- gcc/testsuite/c-c++-common/asan/null-deref-1.c (revision 0) >>> +++ gcc/testsuite/c-c++-common/asan/null-deref-1.c (revision 0) >>> @@ -0,0 +1,16 @@ >>> +/* { dg-do run } */ >>> +/* { dg-shouldfail "asan" } */ >>> + >>> +__attribute__((noinline)) >> >> For GCC you need >> __attribute__((noinline, noclone)) >> here, otherwise GCC could very well clone the function to >> NullDeref.isra.0 or similar, taking no arguments and doing >> the NULL dereference or __builtin_unreachable directly. > > I will add it. > >> >>> +static void NullDeref(int *ptr) { >>> + ptr[10]++; >>> +} >>> +int main() { >>> + NullDeref((int*)0); >>> +} >>> + >>> +/* { dg-output "ERROR: AddressSanitizer:? SEGV on unknown address.*" } */ >>> +/* { dg-output "0x\[0-9a-f\]+ .*pc 0x\[0-9a-f\]+\[^\n\r]*(\n|\r\n|\r)" } */ >>> +/* { dg-output "AddressSanitizer can not provide additional info.*(\n|\r\n|\r)" } */ >>> +/* { dg-output " #0 0x\[0-9a-f\]+ (in \[^\n\r]*NullDeref\[^\n\r]*(null-deref-1.c:6|\[?\]\[?\]:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */ >>> +/* { dg-output " #1 0x\[0-9a-f\]+ (in \[^\n\r]*main\[^\n\r]*(null-deref-1.c:9|\[?\]\[?\]:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */ >>> Index: gcc/testsuite/c-c++-common/asan/global-overflow-1.c >>> =================================================================== >>> --- gcc/testsuite/c-c++-common/asan/global-overflow-1.c (revision 0) >>> +++ gcc/testsuite/c-c++-common/asan/global-overflow-1.c (revision 0) >>> @@ -0,0 +1,22 @@ >>> +/* { dg-do run } */ >>> +/* { dg-shouldfail "asan" } */ >>> + >>> +#include >>> +volatile int one = 1; >>> + >>> +int main() { >>> + 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[one * 10]; /* BOOOM */ >> >> I'd expect the compiler could eventually be smart enough to figure >> out the only valid access of YYY[something * 10] would be if something >> is 0 and thus optimize (one would be read before and forgotten) the >> access to YYY[0]. I'd write the test instead with volatile int ten = 10; >> and s/one/ten/g plus YYY[ten]; and XXX[ten / 10]; and similarly for ZZZ. > > Ok, I will change it. > >> >>> + res += XXX[one] + ZZZ[one]; >>> + return res; >> >>> --- gcc/testsuite/c-c++-common/asan/strncpy-overflow-1.c (revision 0) >>> +++ gcc/testsuite/c-c++-common/asan/strncpy-overflow-1.c (revision 0) >>> @@ -0,0 +1,23 @@ >>> +/* { dg-do run } */ >>> +/* { dg-options "-fno-builtin-strncpy" } */ >>> +/* { 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:11|\[?]\[?]: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:10|\[?]\[?]:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */ >>> + >>> + >>> Index: gcc/testsuite/c-c++-common/asan/rlimit-mmap-test-1.c >>> =================================================================== >>> --- gcc/testsuite/c-c++-common/asan/rlimit-mmap-test-1.c (revision 0) >>> +++ gcc/testsuite/c-c++-common/asan/rlimit-mmap-test-1.c (revision 0) >>> @@ -0,0 +1,22 @@ >>> +/* Check that we properly report mmap failure. */ >>> + >>> +/* { dg-do run } */ >>> +/* { dg-skip-if "" { *-*-* } { "*" } { "-O0 -m64" } } */ >> >> Again, what is 64-bit specific on this test? If you want to run >> it just once, not iterate over all torture options, just do >> /* { dg-skip-if "" { *-*-* } { "*" } { "-O0" } } */ >> >>> +/* { dg-require-effective-target "setrlimit" } */ >>> +/* { dg-shouldfail "asan" } */ >>> + >>> +#include >>> +#include >>> +#include >>> +#include >>> + >>> +static volatile void *x; >>> + >>> +int main(int argc, char **argv) { >>> + struct rlimit mmap_resource_limit = { 0, 0 }; >>> + assert(0 == setrlimit(RLIMIT_AS, &mmap_resource_limit)); >> >> Assert is too expensive with asan (see above). >> Just do >> if (setrlimit(RLIMIT_AS, &mmap_resource_limit)) return 1; >> >> return 0; wouldn't help here, as the output test would then fail. >> > > Ok, I will fix it. > >> >>> --- gcc/testsuite/c-c++-common/asan/use-after-free-1.c (revision 0) >>> +++ gcc/testsuite/c-c++-common/asan/use-after-free-1.c (revision 0) >>> @@ -0,0 +1,22 @@ >>> +/* { dg-do run } */ >>> +/* { dg-options "-fno-builtin-malloc" } */ >>> +/* { dg-shouldfail "asan" } */ >>> + >>> +#include >>> +int main() { >>> + char *x = (char*)malloc(10 * sizeof(char)); >> >> Again, why the sizeof(char)? It is always 1. > > Ok, I will fix it. > >> >>> + free(x); >>> + return x[5]; >>> +} >>> + >>> +/* { dg-output "ERROR: AddressSanitizer:? heap-use-after-free on address\[^\n\r]*" } */ >>> +/* { dg-output "0x\[0-9a-f\]+ at pc 0x\[0-9a-f\]+ bp 0x\[0-9a-f\]+ sp 0x\[0-9a-f\]+\[^\n\r]*(\n|\r\n|\r)" } */ >>> +/* { dg-output "READ of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ >>> +/* { dg-output " #0 0x\[0-9a-f\]+ (in _*main \[^\n\r]*(use-after-free-1.c:9|\[?]\[?]:0)|\[(\]).*(\n|\r\n|\r)" } */ >>> +/* { dg-output "0x\[0-9a-f\]+ is located 5 bytes inside of 10-byte region .0x\[0-9a-f\]+,0x\[0-9a-f\]+\[^\n\r]*(\n|\r\n|\r)" } */ >>> +/* { dg-output "freed by thread T0 here:\[^\n\r]*(\n|\r\n|\r)" } */ >>> +/* { dg-output " #0 0x\[0-9a-f\]+ (in \[^\n\r]*free|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */ >>> +/* { dg-output " #1 0x\[0-9a-f\]+ (in \[^\n\r]*main \[^\n\r]*(use-after-free-1.c:8|\[?]\[?]:0)|\[(\]).*(\n|\r\n|\r)" } */ >>> +/* { dg-output "previously allocated by thread T0 here:\[^\n\r]*(\n|\r\n|\r)" } */ >>> +/* { dg-output " #0 0x\[0-9a-f\]+ (in \[^\n\r]*malloc|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */ >>> +/* { dg-output " #1 0x\[0-9a-f\]+ (in \[^\n\r]*main \[^\n\r]*(use-after-free-1.c:7|\[?]\[?]:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */ >>> Index: gcc/testsuite/c-c++-common/asan/stack-use-after-return-1.c >>> =================================================================== >>> --- gcc/testsuite/c-c++-common/asan/stack-use-after-return-1.c (revision 0) >>> +++ gcc/testsuite/c-c++-common/asan/stack-use-after-return-1.c (revision 0) >>> @@ -0,0 +1,31 @@ >>> +/* { dg-do run } */ >>> +/* { dg-shouldfail "asan" } */ >>> +#include >>> + >>> +__attribute__((noinline)) >>> +char *Ident(char *x) { >>> + fprintf(stderr, "1: %p\n", x); >>> + return x; >>> +} >>> + >>> +__attribute__((noinline)) >>> +char *Func1() { >>> + char local; >>> + return Ident(&local); >>> +} >>> + >>> +__attribute__((noinline)) >>> +void Func2(char *x) { >>> + fprintf(stderr, "2: %p\n", x); >>> + *x = 1; >>> +} >>> + >>> +int main(int argc, char **argv) { >>> + Func2(Func1()); >>> + return 0; >>> +} >>> + >>> +/* { dg-output "WRITE of size 1 \[^\n\r]* thread T0" } */ >>> +/* { dg-output " #0\[^\n\r]*Func2\[^\n\r]*(stack-use-after-return.cc:24|\[?\]\[?\]:)" } */ >>> +/* { dg-output "is located in frame <\[^\n\r]*Func1\[^\n\r]*> of T0's stack" } */ >> >> Doesn't this test in LLVM start with >> // XFAIL: * >> ? It does need the (for LLVM non-default?, for GCC not implemented yet) >> expensive use-after-return mode where all stack vars are malloced/freed, >> right? >> So, I'd just /* { dg-do run { xfail *-*-* } } */ it for now or not add at >> all. >> > > Ok, I will not add it for now. > >>> --- gcc/testsuite/c-c++-common/asan/sanity-check-pure-c-1.c (revision 0) >>> +++ gcc/testsuite/c-c++-common/asan/sanity-check-pure-c-1.c (revision 0) >>> @@ -0,0 +1,16 @@ >>> +/* { dg-do run } */ >> >> -fno-builtin-malloc at least to dg-options? > > Ok. > >> >>> +/* { dg-shouldfail "asan" } */ >>> + >>> +#include >>> +int main() { >>> + char *x = (char*)malloc(10 * sizeof(char)); >>> + free(x); >>> + return x[5]; >>> +} >>> + >>> +/* { dg-output "heap-use-after-free.*(\n|\r\n|\r)" } */ >>> +/* { dg-output " #0 \[^\n\r]*free\[^\n\r]*(\n|\r\n|\r)" } */ >>> +/* { dg-output " #1 \[^\n\r]*(in main\[^\n\r]*(sanity-check-pure-c-1.c:7|\[?\]\[?\]:0)|\[(\]).*(\n|\r\n|\r)" } */ >>> +/* { dg-output " #0 \[^\n\r]*(interceptor_|)malloc\[^\n\r]*(\n|\r\n|\r)" } */ >>> +/* { dg-output " #1 \[^\n\r]*(in main\[^\n\r]*(sanity-check-pure-c-1.c:6|\[?\]\[?\]:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */ >>> + >>> Index: gcc/testsuite/c-c++-common/asan/clone-test-1.c >>> =================================================================== >>> --- gcc/testsuite/c-c++-common/asan/clone-test-1.c (revision 0) >>> +++ gcc/testsuite/c-c++-common/asan/clone-test-1.c (revision 0) >>> @@ -0,0 +1,47 @@ >>> +/* Regression test for: >>> + http://code.google.com/p/address-sanitizer/issues/detail?id=37 */ >>> + >>> +/* { dg-do run } */ >> >> Please use /* { dg-do run { target *-*-linux* } } */ above too. >> The test is really very Linux specific. >> > > Ok. > >>> --- gcc/testsuite/c-c++-common/asan/heap-overflow-1.c (revision 0) >>> +++ gcc/testsuite/c-c++-common/asan/heap-overflow-1.c (revision 0) >>> @@ -0,0 +1,20 @@ >>> +/* { dg-do run } */ >>> +/* { dg-shouldfail "asan" } */ >>> + >>> +#include >>> +#include >>> +int main(int argc, char **argv) { >>> + char *x = (char*)malloc(10 * sizeof(char)); >>> + memset(x, 0, 10); >>> + int res = x[argc * 10]; /* BOOOM */ >>> + free(x); >>> + return res; >>> +} >> >> What has been said earlier about argc used in tests... >> Plus -fno-builtin-malloc (and add -fno-builtin-free to all uses >> of -fno-builtin-malloc too for all tests). >> > > Ok. > >>> --- gcc/testsuite/c-c++-common/asan/sleep-before-dying-1.c (revision 0) >>> +++ gcc/testsuite/c-c++-common/asan/sleep-before-dying-1.c (revision 0) >>> @@ -0,0 +1,13 @@ >>> +/* { dg-do run } */ >>> +/* { dg-env-var ASAN_OPTIONS "sleep_before_dying=1" } */ >>> +/* { dg-skip-if "" { *-*-* } { "*" } { "-O2 -m64" } } */ >> >> As has been said several times. Fine to do it at one torture >> option instead of iterating, but don't limit that to -m64 (and if yes, not >> this way). Plus again dg-options -fno-builtin-malloc -fno-builtin-free. >> > > Ok. > >> Jakub