| Message ID | 20200522210301.10824-1-jason@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [pushed] c++: -fsanitize=vptr and -fstrong-eval-order. [PR95221] | expand |
Hi Jason! On 2020-05-22T17:03:01-0400, Jason Merrill via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > [...] > > This issue suggests that we should be running the ubsan tests in multiple > standard modes like the rest of the G++ testsuite, so I've made that change > as well. > --- a/gcc/testsuite/g++.dg/ubsan/ubsan.exp > +++ b/gcc/testsuite/g++.dg/ubsan/ubsan.exp > @@ -26,7 +26,7 @@ ubsan_init > > # Main loop. > if [check_effective_target_fsanitize_undefined] { > - gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C $srcdir/c-c++-common/ubsan/*.c]] "" "" > + g++-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C $srcdir/c-c++-common/ubsan/*.c]] "" "" > } Hmm, but that means that testing is now no longer running the "optimization options torture testing": Running [...]/source-gcc/gcc/testsuite/g++.dg/ubsan/ubsan.exp ... -PASS: c-c++-common/ubsan/align-1.c -O0 (test for excess errors) -PASS: c-c++-common/ubsan/align-1.c -O0 execution test -PASS: c-c++-common/ubsan/align-1.c -O1 (test for excess errors) -PASS: c-c++-common/ubsan/align-1.c -O1 execution test -PASS: c-c++-common/ubsan/align-1.c -O2 (test for excess errors) -PASS: c-c++-common/ubsan/align-1.c -O2 execution test -PASS: c-c++-common/ubsan/align-1.c -O2 -flto -fno-use-linker-plugin -flto-partition=none (test for excess errors) -PASS: c-c++-common/ubsan/align-1.c -O2 -flto -fno-use-linker-plugin -flto-partition=none execution test -PASS: c-c++-common/ubsan/align-1.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects (test for excess errors) -PASS: c-c++-common/ubsan/align-1.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects execution test -PASS: c-c++-common/ubsan/align-1.c -O3 -g (test for excess errors) -PASS: c-c++-common/ubsan/align-1.c -O3 -g execution test -PASS: c-c++-common/ubsan/align-1.c -Os (test for excess errors) -PASS: c-c++-common/ubsan/align-1.c -Os execution test +PASS: c-c++-common/ubsan/align-1.c -std=gnu++14 (test for excess errors) +PASS: c-c++-common/ubsan/align-1.c -std=gnu++14 execution test +PASS: c-c++-common/ubsan/align-1.c -std=gnu++17 (test for excess errors) +PASS: c-c++-common/ubsan/align-1.c -std=gnu++17 execution test +PASS: c-c++-common/ubsan/align-1.c -std=gnu++2a (test for excess errors) +PASS: c-c++-common/ubsan/align-1.c -std=gnu++2a execution test +PASS: c-c++-common/ubsan/align-1.c -std=gnu++98 (test for excess errors) +PASS: c-c++-common/ubsan/align-1.c -std=gnu++98 execution test Etc. Not sure if that was intentional? I suppose this removed way more testsuite coverage compared to what different C++ '-std=[...]' add? > The testcase changes are all to accommodate that. > /* { dg-options "-fsanitize=bounds -Wno-array-bounds" } */ > +/* { dg-options "-fsanitize=bounds -Wno-array-bounds -Wno-volatile" { target c++ } } */ Simpler would've been (untested): +/* { dg-additional-options "-Wno-volatile" { target c++ } } */ Etc. ;-) Grüße Thomas ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
Hi! On 2020-06-23T13:21:05+0200, I wrote: > On 2020-05-22T17:03:01-0400, Jason Merrill via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: >> [...] >> >> This issue suggests that we should be running the ubsan tests in multiple >> standard modes like the rest of the G++ testsuite, so I've made that change >> as well. > >> --- a/gcc/testsuite/g++.dg/ubsan/ubsan.exp >> +++ b/gcc/testsuite/g++.dg/ubsan/ubsan.exp >> @@ -26,7 +26,7 @@ ubsan_init >> >> # Main loop. >> if [check_effective_target_fsanitize_undefined] { >> - gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C $srcdir/c-c++-common/ubsan/*.c]] "" "" >> + g++-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C $srcdir/c-c++-common/ubsan/*.c]] "" "" >> } > > Hmm, but that means that testing is now no longer running the > "optimization options torture testing": > > Running [...]/source-gcc/gcc/testsuite/g++.dg/ubsan/ubsan.exp ... > -PASS: c-c++-common/ubsan/align-1.c -O0 (test for excess errors) > -PASS: c-c++-common/ubsan/align-1.c -O0 execution test > -PASS: c-c++-common/ubsan/align-1.c -O1 (test for excess errors) > -PASS: c-c++-common/ubsan/align-1.c -O1 execution test > -PASS: c-c++-common/ubsan/align-1.c -O2 (test for excess errors) > -PASS: c-c++-common/ubsan/align-1.c -O2 execution test > -PASS: c-c++-common/ubsan/align-1.c -O2 -flto -fno-use-linker-plugin -flto-partition=none (test for excess errors) > -PASS: c-c++-common/ubsan/align-1.c -O2 -flto -fno-use-linker-plugin -flto-partition=none execution test > -PASS: c-c++-common/ubsan/align-1.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects (test for excess errors) > -PASS: c-c++-common/ubsan/align-1.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects execution test > -PASS: c-c++-common/ubsan/align-1.c -O3 -g (test for excess errors) > -PASS: c-c++-common/ubsan/align-1.c -O3 -g execution test > -PASS: c-c++-common/ubsan/align-1.c -Os (test for excess errors) > -PASS: c-c++-common/ubsan/align-1.c -Os execution test > +PASS: c-c++-common/ubsan/align-1.c -std=gnu++14 (test for excess errors) > +PASS: c-c++-common/ubsan/align-1.c -std=gnu++14 execution test > +PASS: c-c++-common/ubsan/align-1.c -std=gnu++17 (test for excess errors) > +PASS: c-c++-common/ubsan/align-1.c -std=gnu++17 execution test > +PASS: c-c++-common/ubsan/align-1.c -std=gnu++2a (test for excess errors) > +PASS: c-c++-common/ubsan/align-1.c -std=gnu++2a execution test > +PASS: c-c++-common/ubsan/align-1.c -std=gnu++98 (test for excess errors) > +PASS: c-c++-common/ubsan/align-1.c -std=gnu++98 execution test > > Etc. > > Not sure if that was intentional? I suppose this removed way more > testsuite coverage compared to what different C++ '-std=[...]' add? Any comments/ideas here? Note that 'g++.dg/asan/asan.exp', 'g++.dg/tsan/tsan.exp' also use 'gcc-dg-runtest' ("optimization options" variation), not 'g++-dg-runtest' ("C++ '-std=[...]'" variation). Grüße Thomas >> The testcase changes are all to accommodate that. > >> /* { dg-options "-fsanitize=bounds -Wno-array-bounds" } */ >> +/* { dg-options "-fsanitize=bounds -Wno-array-bounds -Wno-volatile" { target c++ } } */ > > Simpler would've been (untested): > > +/* { dg-additional-options "-Wno-volatile" { target c++ } } */ > > Etc. > > ;-) > > > Grüße > Thomas ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
diff --git a/gcc/cp/cp-ubsan.c b/gcc/cp/cp-ubsan.c index b064a98202a..c40dac72b42 100644 --- a/gcc/cp/cp-ubsan.c +++ b/gcc/cp/cp-ubsan.c @@ -118,14 +118,33 @@ cp_ubsan_maybe_instrument_member_call (tree stmt) { if (call_expr_nargs (stmt) == 0) return; - tree *opp = &CALL_EXPR_ARG (stmt, 0); - tree op = *opp; - if (op == error_mark_node - || !INDIRECT_TYPE_P (TREE_TYPE (op))) - return; - while (TREE_CODE (op) == COMPOUND_EXPR) + tree op, *opp; + + tree fn = CALL_EXPR_FN (stmt); + if (fn && TREE_CODE (fn) == OBJ_TYPE_REF) { - opp = &TREE_OPERAND (op, 1); + /* Virtual function call: Sanitize the use of the object pointer in the + OBJ_TYPE_REF, since the vtable reference will SEGV otherwise (95221). + OBJ_TYPE_REF_EXPR is ptr->vptr[N] and OBJ_TYPE_REF_OBJECT is ptr. */ + opp = &OBJ_TYPE_REF_EXPR (fn); + op = OBJ_TYPE_REF_OBJECT (fn); + while (*opp != op) + { + if (TREE_CODE (*opp) == COMPOUND_EXPR) + opp = &TREE_OPERAND (*opp, 1); + else + opp = &TREE_OPERAND (*opp, 0); + } + } + else + { + /* Non-virtual call: Sanitize the 'this' argument. */ + opp = &CALL_EXPR_ARG (stmt, 0); + if (*opp == error_mark_node + || !INDIRECT_TYPE_P (TREE_TYPE (*opp))) + return; + while (TREE_CODE (*opp) == COMPOUND_EXPR) + opp = &TREE_OPERAND (*opp, 1); op = *opp; } op = cp_ubsan_maybe_instrument_vptr (EXPR_LOCATION (stmt), op, diff --git a/gcc/testsuite/c-c++-common/ubsan/bounds-13.c b/gcc/testsuite/c-c++-common/ubsan/bounds-13.c index 25b0467ec67..a8bce370f8f 100644 --- a/gcc/testsuite/c-c++-common/ubsan/bounds-13.c +++ b/gcc/testsuite/c-c++-common/ubsan/bounds-13.c @@ -1,6 +1,7 @@ /* PR sanitizer/71498 */ /* { dg-do run } */ /* { dg-options "-fsanitize=bounds -Wno-array-bounds" } */ +/* { dg-options "-fsanitize=bounds -Wno-array-bounds -Wno-volatile" { target c++ } } */ struct S { int a[100]; int b, c; } s; diff --git a/gcc/testsuite/c-c++-common/ubsan/bounds-2.c b/gcc/testsuite/c-c++-common/ubsan/bounds-2.c index 56654486f65..ec4ef7eed20 100644 --- a/gcc/testsuite/c-c++-common/ubsan/bounds-2.c +++ b/gcc/testsuite/c-c++-common/ubsan/bounds-2.c @@ -1,5 +1,6 @@ /* { dg-do run } */ /* { dg-options "-fsanitize=bounds -Wall -Wextra -Wno-unused -Wno-array-bounds -Wno-uninitialized" } */ +/* { dg-options "-fsanitize=bounds -Wall -Wextra -Wno-unused -Wno-array-bounds -Wno-uninitialized -Wno-volatile" { target c++ } } */ /* Test runtime errors. */ diff --git a/gcc/testsuite/c-c++-common/ubsan/div-by-zero-1.c b/gcc/testsuite/c-c++-common/ubsan/div-by-zero-1.c index 479ced038fb..8b62ae743d8 100644 --- a/gcc/testsuite/c-c++-common/ubsan/div-by-zero-1.c +++ b/gcc/testsuite/c-c++-common/ubsan/div-by-zero-1.c @@ -1,5 +1,6 @@ /* { dg-do run } */ /* { dg-options "-fsanitize=integer-divide-by-zero -Wno-div-by-zero" } */ +/* { dg-options "-fsanitize=integer-divide-by-zero -Wno-div-by-zero -Wno-volatile" { target c++ } } */ int main (void) diff --git a/gcc/testsuite/c-c++-common/ubsan/div-by-zero-6.c b/gcc/testsuite/c-c++-common/ubsan/div-by-zero-6.c index 27a18bb096e..be017ad8179 100644 --- a/gcc/testsuite/c-c++-common/ubsan/div-by-zero-6.c +++ b/gcc/testsuite/c-c++-common/ubsan/div-by-zero-6.c @@ -1,5 +1,6 @@ /* { dg-do run } */ /* { dg-options "-fsanitize=integer-divide-by-zero -Wno-div-by-zero" } */ +/* { dg-options "-fsanitize=integer-divide-by-zero -Wno-div-by-zero -Wno-volatile" { target c++ } } */ #include <stdio.h> diff --git a/gcc/testsuite/c-c++-common/ubsan/div-by-zero-7.c b/gcc/testsuite/c-c++-common/ubsan/div-by-zero-7.c index 5f53bef74ea..643b76af143 100644 --- a/gcc/testsuite/c-c++-common/ubsan/div-by-zero-7.c +++ b/gcc/testsuite/c-c++-common/ubsan/div-by-zero-7.c @@ -1,5 +1,6 @@ /* { dg-do run } */ /* { dg-options "-fsanitize=integer-divide-by-zero -Wno-div-by-zero -fno-sanitize-recover=integer-divide-by-zero" } */ +/* { dg-options "-fsanitize=integer-divide-by-zero -Wno-div-by-zero -fno-sanitize-recover=integer-divide-by-zero -Wno-volatile" { target c++ } } */ /* { dg-shouldfail "ubsan" } */ #include <stdio.h> diff --git a/gcc/testsuite/c-c++-common/ubsan/overflow-add-1.c b/gcc/testsuite/c-c++-common/ubsan/overflow-add-1.c index 960f1b0afaf..3c0a17edb85 100644 --- a/gcc/testsuite/c-c++-common/ubsan/overflow-add-1.c +++ b/gcc/testsuite/c-c++-common/ubsan/overflow-add-1.c @@ -1,5 +1,6 @@ /* { dg-do run } */ /* { dg-options "-fsanitize=signed-integer-overflow -Wno-unused-variable -fno-sanitize-recover=signed-integer-overflow" } */ +/* { dg-options "-fsanitize=signed-integer-overflow -Wno-unused-variable -fno-sanitize-recover=signed-integer-overflow -Wno-volatile" { target c++ } } */ #define SCHAR_MAX __SCHAR_MAX__ #define SHRT_MAX __SHRT_MAX__ diff --git a/gcc/testsuite/c-c++-common/ubsan/overflow-add-2.c b/gcc/testsuite/c-c++-common/ubsan/overflow-add-2.c index b104d6158fe..8fa9074647a 100644 --- a/gcc/testsuite/c-c++-common/ubsan/overflow-add-2.c +++ b/gcc/testsuite/c-c++-common/ubsan/overflow-add-2.c @@ -1,5 +1,6 @@ /* { dg-do run } */ /* { dg-options "-fsanitize=signed-integer-overflow -Wno-unused-variable" } */ +/* { dg-options "-fsanitize=signed-integer-overflow -Wno-unused-variable -Wno-volatile" { target c++ } } */ #define INT_MAX __INT_MAX__ #define INT_MIN (-__INT_MAX__ - 1) diff --git a/gcc/testsuite/c-c++-common/ubsan/overflow-int128.c b/gcc/testsuite/c-c++-common/ubsan/overflow-int128.c index 400f25b01e1..10ccc79d0fe 100644 --- a/gcc/testsuite/c-c++-common/ubsan/overflow-int128.c +++ b/gcc/testsuite/c-c++-common/ubsan/overflow-int128.c @@ -1,6 +1,7 @@ /* { dg-do run } */ /* { dg-require-effective-target int128 } */ /* { dg-options "-fsanitize=signed-integer-overflow" } */ +/* { dg-options "-fsanitize=signed-integer-overflow -Wno-volatile" { target c++ } } */ /* 2^127 - 1 */ #define INT128_MAX (__int128) (((unsigned __int128) 1 << ((__SIZEOF_INT128__ * __CHAR_BIT__) - 1)) - 1) diff --git a/gcc/testsuite/c-c++-common/ubsan/overflow-sub-1.c b/gcc/testsuite/c-c++-common/ubsan/overflow-sub-1.c index e92aaf4ce33..7ceb4ec60a7 100644 --- a/gcc/testsuite/c-c++-common/ubsan/overflow-sub-1.c +++ b/gcc/testsuite/c-c++-common/ubsan/overflow-sub-1.c @@ -1,5 +1,6 @@ /* { dg-do run } */ /* { dg-options "-fsanitize=signed-integer-overflow -Wno-unused-variable -fno-sanitize-recover=signed-integer-overflow" } */ +/* { dg-options "-fsanitize=signed-integer-overflow -Wno-unused-variable -fno-sanitize-recover=signed-integer-overflow -Wno-volatile" { target c++ } } */ #define SCHAR_MAX __SCHAR_MAX__ #define SCHAR_MIN (-__SCHAR_MAX__ - 1) diff --git a/gcc/testsuite/c-c++-common/ubsan/overflow-sub-2.c b/gcc/testsuite/c-c++-common/ubsan/overflow-sub-2.c index cc94061634e..92a2ea28af2 100644 --- a/gcc/testsuite/c-c++-common/ubsan/overflow-sub-2.c +++ b/gcc/testsuite/c-c++-common/ubsan/overflow-sub-2.c @@ -1,5 +1,6 @@ /* { dg-do run } */ /* { dg-options "-fsanitize=signed-integer-overflow -Wno-unused-variable" } */ +/* { dg-options "-fsanitize=signed-integer-overflow -Wno-unused-variable -Wno-volatile" { target c++ } } */ #define INT_MAX __INT_MAX__ #define INT_MIN (-__INT_MAX__ - 1) diff --git a/gcc/testsuite/g++.dg/ubsan/pr85029.C b/gcc/testsuite/g++.dg/ubsan/pr85029.C index 07472af16a5..836ce69cc15 100644 --- a/gcc/testsuite/g++.dg/ubsan/pr85029.C +++ b/gcc/testsuite/g++.dg/ubsan/pr85029.C @@ -1,7 +1,7 @@ // PR sanitizer/85029 // { dg-do compile } // { dg-skip-if "" { *-*-* } { "-flto -fno-fat-lto-objects" } } -// { dg-options "-fsanitize=undefined" } +// { dg-options "-fsanitize=undefined -Wno-register" } struct B { virtual B bar (); diff --git a/gcc/testsuite/g++.dg/ubsan/vptr-14.C b/gcc/testsuite/g++.dg/ubsan/vptr-14.C index 2247ad99fcc..06cd66347d4 100644 --- a/gcc/testsuite/g++.dg/ubsan/vptr-14.C +++ b/gcc/testsuite/g++.dg/ubsan/vptr-14.C @@ -1,5 +1,5 @@ // PR sanitizer/89869 -// { dg-do run } +// { dg-do run { target c++11 } } // { dg-options "-fsanitize=vptr -fno-sanitize-recover=vptr" } struct S { S *s = 0; virtual ~S () {} }; diff --git a/gcc/testsuite/g++.dg/ubsan/ubsan.exp b/gcc/testsuite/g++.dg/ubsan/ubsan.exp index 11b92bff980..2bc98831d2e 100644 --- a/gcc/testsuite/g++.dg/ubsan/ubsan.exp +++ b/gcc/testsuite/g++.dg/ubsan/ubsan.exp @@ -26,7 +26,7 @@ ubsan_init # Main loop. if [check_effective_target_fsanitize_undefined] { - gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C $srcdir/c-c++-common/ubsan/*.c]] "" "" + g++-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C $srcdir/c-c++-common/ubsan/*.c]] "" "" } # All done.