This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Mon, Dec 3, 2012 at 10:32 PM, Wei Mi <wmi@google.com> 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 <jakub@redhat.com> 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 <stdlib.h>
>>> +#include <stdio.h>
>>> +
>>> +template <int depth>
>>> +struct DeepFree {
>>> +  static void free(char *x) {
>>> +    DeepFree<depth - 1>::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 <stdlib.h>
>>> +#include <stdio.h>
>>> +
>>> +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 <stdlib.h>
>>> +__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 <assert.h>
>>> +#include <dlfcn.h>
>>> +#include <stdio.h>
>>> +#include <string.h>
>>> +#include <sys/mman.h>
>>> +
>>> +#include <string>
>>> +
>>> +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 <dlfcn.h>
>>> +#include <stdio.h>
>>> +#include <string.h>
>>> +
>>> +#include <string>
>>> +
>>> +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
>>> +# <http://www.gnu.org/licenses/>.
>>> +
>>> +# 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 <stdlib.h>
>>> +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 <stdio.h>
>>> +#include <ucontext.h>
>>> +#include <unistd.h>
>>> +
>>> +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 <string.h>
>>> +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 <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: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 <stdlib.h>
>>> +#include <assert.h>
>>> +#include <sys/time.h>
>>> +#include <sys/resource.h>
>>> +
>>> +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 <stdlib.h>
>>> +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 <stdio.h>
>>> +
>>> +__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 <stdlib.h>
>>> +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 <stdlib.h>
>>> +#include <string.h>
>>> +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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]