diff mbox

Fix PR 58867: asan and ubsan tests not run for installed testing

Message ID CA+=Sn1=P+4JwkBam26C9zm2ZTB2Z9PTyhDzLZ1tGmJjQZjCk7Q@mail.gmail.com
State New
Headers show

Commit Message

Andrew Pinski Oct. 29, 2013, 2:56 p.m. UTC 
Hi,
  The problem here is that both asan and ubsan testsuite test if we
have set a library path before running the testsuite.  This is
incorrect when running the already installed testing as there is no
path to set.

This patch changes it so it tests if the executable is able to be
built/linked before running the tests.  This also fixes a
warning/error in restoring the CXXFLAGS which shows up in the already
installed testing case.

OK?  Bootstrapped and tested on x86_64-linux-gnu.  Also manually
looked into the log file to make sure both asan and ubsan were still
being tested.

Thanks,
Andrew Pinski

ChangeLog:
* lib/ubsan-dg.exp (check_effective_target_fundefined_sanitizer): New function.
(ubsan_init): Save off ALWAYS_CXXFLAGS.
(ubsan_finish): Restore ALWAYS_CXXFLAGS correctly.
* lib/asan-dg.exp (check_effective_target_faddress_sanitizer): Change
to creating an executable.
(asan_init): Save off ALWAYS_CXXFLAGS.
(asan_finish): Restore ALWAYS_CXXFLAGS correctly.
* gcc.dg/ubsan/ubsan.exp: Don't check the return value of ubsan_init.
Check check_effective_target_fundefined_sanitizer before running the
tests.
* g++.dg/ubsan/ubsan.exp: Likewise.
* testsuite/gcc.dg/asan/asan.exp: Don't check
check_effective_target_faddress_sanitizer too early. Don't check the
return value of asan_init.
* g++.dg/asan/asan.exp: Likewise.

Comments

Andrew Pinski Dec. 14, 2013, 7:56 p.m. UTC | #1 
On Tue, Oct 29, 2013 at 7:56 AM, Andrew Pinski
<andrew.pinski@caviumnetworks.com> wrote:
> Hi,
>   The problem here is that both asan and ubsan testsuite test if we
> have set a library path before running the testsuite.  This is
> incorrect when running the already installed testing as there is no
> path to set.
>
> This patch changes it so it tests if the executable is able to be
> built/linked before running the tests.  This also fixes a
> warning/error in restoring the CXXFLAGS which shows up in the already
> installed testing case.
>
> OK?  Bootstrapped and tested on x86_64-linux-gnu.  Also manually
> looked into the log file to make sure both asan and ubsan were still
> being tested.

Ping.  This patch has been outstanding for over a month now.

Thanks,
Andrew


>
> Thanks,
> Andrew Pinski
>
> ChangeLog:
> * lib/ubsan-dg.exp (check_effective_target_fundefined_sanitizer): New function.
> (ubsan_init): Save off ALWAYS_CXXFLAGS.
> (ubsan_finish): Restore ALWAYS_CXXFLAGS correctly.
> * lib/asan-dg.exp (check_effective_target_faddress_sanitizer): Change
> to creating an executable.
> (asan_init): Save off ALWAYS_CXXFLAGS.
> (asan_finish): Restore ALWAYS_CXXFLAGS correctly.
> * gcc.dg/ubsan/ubsan.exp: Don't check the return value of ubsan_init.
> Check check_effective_target_fundefined_sanitizer before running the
> tests.
> * g++.dg/ubsan/ubsan.exp: Likewise.
> * testsuite/gcc.dg/asan/asan.exp: Don't check
> check_effective_target_faddress_sanitizer too early. Don't check the
> return value of asan_init.
> * g++.dg/asan/asan.exp: Likewise.
Jakub Jelinek Dec. 17, 2013, 6:16 p.m. UTC | #2 
On Tue, Oct 29, 2013 at 07:56:32AM -0700, Andrew Pinski wrote:
> * lib/ubsan-dg.exp (check_effective_target_fundefined_sanitizer): New function.
> (ubsan_init): Save off ALWAYS_CXXFLAGS.
> (ubsan_finish): Restore ALWAYS_CXXFLAGS correctly.
> * lib/asan-dg.exp (check_effective_target_faddress_sanitizer): Change
> to creating an executable.
> (asan_init): Save off ALWAYS_CXXFLAGS.
> (asan_finish): Restore ALWAYS_CXXFLAGS correctly.
> * gcc.dg/ubsan/ubsan.exp: Don't check the return value of ubsan_init.
> Check check_effective_target_fundefined_sanitizer before running the
> tests.
> * g++.dg/ubsan/ubsan.exp: Likewise.
> * testsuite/gcc.dg/asan/asan.exp: Don't check
> check_effective_target_faddress_sanitizer too early. Don't check the
> return value of asan_init.
> * g++.dg/asan/asan.exp: Likewise.

Try to find out some mailer that doesn't eat tabs?  Once you have tabs,
some of the lines (at least the first one) will be too long.

> --- testsuite/lib/ubsan-dg.exp	(revision 204138)
> +++ testsuite/lib/ubsan-dg.exp	(working copy)
> @@ -14,6 +14,15 @@
>  # along with GCC; see the file COPYING3.  If not see
>  # <http://www.gnu.org/licenses/>.
>  
> +# Return 1 if compilation with -fsanitize=undefined is error-free for trivial
> +# code, 0 otherwise.
> +
> +proc check_effective_target_fundefined_sanitizer {} {

Please rename this to ..._fsanitize_undefined everywhere, and
also *_faddress_sanitizer to *_fsanitize_address.  The history behind
the latter is that the option I think was initially -faddress-sanitizer
and only later got renamed to the current form.

> @@ -60,6 +69,7 @@ proc ubsan_init { args } {
>      global ALWAYS_CXXFLAGS
>      global TOOL_OPTIONS
>      global ubsan_saved_TEST_ALWAYS_FLAGS
> +    global usan_saved_ALWAYS_CXXFLAGS

Please spell it as ubsan* instead of usan*.

> --- testsuite/gcc.dg/asan/asan.exp	(revision 204138)
> +++ testsuite/gcc.dg/asan/asan.exp	(working copy)
> @@ -1,4 +1,4 @@
> -# Copyright (C) 2012-2013 Free Software Foundation, Inc.
> +# Copyright (C) 2012 Free Software Foundation, Inc.

Don't do this.

> --- testsuite/g++.dg/asan/asan.exp	(revision 204138)
> +++ testsuite/g++.dg/asan/asan.exp	(working copy)
> @@ -1,4 +1,4 @@
> -# Copyright (C) 2012-2013 Free Software Foundation, Inc.
> +# Copyright (C) 2012 Free Software Foundation, Inc.

Nor this.

Otherwise it looks good to me.  There is tsan.exp that will probably need
similar treatment though now.

	Jakub
diff mbox

Patch

Index: testsuite/lib/ubsan-dg.exp
===================================================================
--- testsuite/lib/ubsan-dg.exp	(revision 204138)
+++ testsuite/lib/ubsan-dg.exp	(working copy)
@@ -14,6 +14,15 @@ 
 # along with GCC; see the file COPYING3.  If not see
 # <http://www.gnu.org/licenses/>.
 
+# Return 1 if compilation with -fsanitize=undefined is error-free for trivial
+# code, 0 otherwise.
+
+proc check_effective_target_fundefined_sanitizer {} {
+    return [check_no_compiler_messages fundefined_sanitizer executable {
+	int main (void) { return 0; }
+    } "-fsanitize=undefined"]
+}
+
 #
 # ubsan_link_flags -- compute library path and flags to find libubsan.
 # (originally from g++.exp)
@@ -60,6 +69,7 @@  proc ubsan_init { args } {
     global ALWAYS_CXXFLAGS
     global TOOL_OPTIONS
     global ubsan_saved_TEST_ALWAYS_FLAGS
+    global usan_saved_ALWAYS_CXXFLAGS
 
     set link_flags ""
     if ![is_remote host] {
@@ -74,6 +84,7 @@  proc ubsan_init { args } {
 	set ubsan_saved_TEST_ALWAYS_FLAGS $TEST_ALWAYS_FLAGS
     }
     if [info exists ALWAYS_CXXFLAGS] {
+	set usan_saved_ALWAYS_CXXFLAGS $ALWAYS_CXXFLAGS
 	set ALWAYS_CXXFLAGS [concat "{ldflags=$link_flags}" $ALWAYS_CXXFLAGS]
     } else {
 	if [info exists TEST_ALWAYS_FLAGS] {
@@ -95,10 +106,15 @@  proc ubsan_init { args } {
 proc ubsan_finish { args } {
     global TEST_ALWAYS_FLAGS
     global ubsan_saved_TEST_ALWAYS_FLAGS
+    global asan_saved_ALWAYS_CXXFLAGS
 
-    if [info exists ubsan_saved_TEST_ALWAYS_FLAGS] {
-	set TEST_ALWAYS_FLAGS $ubsan_saved_TEST_ALWAYS_FLAGS
+    if [info exists asan_saved_ALWAYS_CXXFLAGS ] {
+	set ALWAYS_CXXFLAGS $asan_saved_ALWAYS_CXXFLAGS
     } else {
-	unset TEST_ALWAYS_FLAGS
+	if [info exists ubsan_saved_TEST_ALWAYS_FLAGS] {
+	    set TEST_ALWAYS_FLAGS $ubsan_saved_TEST_ALWAYS_FLAGS
+	} else {
+	    unset TEST_ALWAYS_FLAGS
+	}
     }
 }
Index: testsuite/lib/asan-dg.exp
===================================================================
--- testsuite/lib/asan-dg.exp	(revision 204138)
+++ testsuite/lib/asan-dg.exp	(working copy)
@@ -18,8 +18,8 @@ 
 # code, 0 otherwise.
 
 proc check_effective_target_faddress_sanitizer {} {
-    return [check_no_compiler_messages faddress_sanitizer object {
-	void foo (void) { }
+    return [check_no_compiler_messages faddress_sanitizer executable {
+	int main (void) { return 0; }
     } "-fsanitize=address"]
 }
 
@@ -69,6 +69,7 @@  proc asan_init { args } {
     global ALWAYS_CXXFLAGS
     global TOOL_OPTIONS
     global asan_saved_TEST_ALWAYS_FLAGS
+    global asan_saved_ALWAYS_CXXFLAGS
 
     set link_flags ""
     if ![is_remote host] {
@@ -83,6 +84,7 @@  proc asan_init { args } {
 	set asan_saved_TEST_ALWAYS_FLAGS $TEST_ALWAYS_FLAGS
     }
     if [info exists ALWAYS_CXXFLAGS] {
+	set asan_saved_ALWAYS_CXXFLAGS $ALWAYS_CXXFLAGS
 	set ALWAYS_CXXFLAGS [concat "{ldflags=$link_flags}" $ALWAYS_CXXFLAGS]
 	set ALWAYS_CXXFLAGS [concat "{additional_flags=-fsanitize=address -g}" $ALWAYS_CXXFLAGS]
     } else {
@@ -105,11 +107,16 @@  proc asan_init { args } {
 proc asan_finish { args } {
     global TEST_ALWAYS_FLAGS
     global asan_saved_TEST_ALWAYS_FLAGS
+    global asan_saved_ALWAYS_CXXFLAGS
 
-    if [info exists asan_saved_TEST_ALWAYS_FLAGS] {
-	set TEST_ALWAYS_FLAGS $asan_saved_TEST_ALWAYS_FLAGS
+    if [info exists asan_saved_ALWAYS_CXXFLAGS ] {
+	set ALWAYS_CXXFLAGS $asan_saved_ALWAYS_CXXFLAGS
     } else {
-	unset TEST_ALWAYS_FLAGS
+	if [info exists asan_saved_TEST_ALWAYS_FLAGS] {
+	    set TEST_ALWAYS_FLAGS $asan_saved_TEST_ALWAYS_FLAGS
+	} else {
+	    unset TEST_ALWAYS_FLAGS
+	}
     }
 }
 
Index: testsuite/gcc.dg/ubsan/ubsan.exp
===================================================================
--- testsuite/gcc.dg/ubsan/ubsan.exp	(revision 204138)
+++ testsuite/gcc.dg/ubsan/ubsan.exp	(working copy)
@@ -24,11 +24,11 @@  load_lib ubsan-dg.exp
 
 # Initialize `dg'.
 dg-init
-if [ubsan_init] {
+ubsan_init
 
 # Main loop.
-gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.c $srcdir/c-c++-common/ubsan/*.c]] ""
-
+if [check_effective_target_fundefined_sanitizer] {
+  gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.c $srcdir/c-c++-common/ubsan/*.c]] ""
 }
 
 # All done.
Index: testsuite/gcc.dg/asan/asan.exp
===================================================================
--- testsuite/gcc.dg/asan/asan.exp	(revision 204138)
+++ testsuite/gcc.dg/asan/asan.exp	(working copy)
@@ -1,4 +1,4 @@ 
-# Copyright (C) 2012-2013 Free Software Foundation, Inc.
+# Copyright (C) 2012 Free Software Foundation, Inc.
 #
 # This file is part of GCC.
 #
@@ -22,17 +22,13 @@ 
 load_lib gcc-dg.exp
 load_lib asan-dg.exp
 
-if ![check_effective_target_faddress_sanitizer] {
-  return
-}
-
 # Initialize `dg'.
 dg-init
-if [asan_init] {
+asan_init
 
 # Main loop.
-gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.c $srcdir/c-c++-common/asan/*.c]] ""
-
+if [check_effective_target_faddress_sanitizer] {
+  gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.c $srcdir/c-c++-common/asan/*.c]] ""
 }
 
 # All done.
Index: testsuite/g++.dg/ubsan/ubsan.exp
===================================================================
--- testsuite/g++.dg/ubsan/ubsan.exp	(revision 204138)
+++ testsuite/g++.dg/ubsan/ubsan.exp	(working copy)
@@ -22,11 +22,11 @@  load_lib ubsan-dg.exp
 
 # Initialize `dg'.
 dg-init
-if [ubsan_init] {
+ubsan_init
 
 # Main loop.
-gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C $srcdir/c-c++-common/ubsan/*.c]] ""
-
+if [check_effective_target_fundefined_sanitizer] {
+  gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C $srcdir/c-c++-common/ubsan/*.c]] ""
 }
 
 # All done.
Index: testsuite/g++.dg/asan/asan.exp
===================================================================
--- testsuite/g++.dg/asan/asan.exp	(revision 204138)
+++ testsuite/g++.dg/asan/asan.exp	(working copy)
@@ -1,4 +1,4 @@ 
-# Copyright (C) 2012-2013 Free Software Foundation, Inc.
+# Copyright (C) 2012 Free Software Foundation, Inc.
 #
 # This file is part of GCC.
 #
@@ -20,17 +20,14 @@ 
 load_lib g++-dg.exp
 load_lib asan-dg.exp
 
-if ![check_effective_target_faddress_sanitizer] {
-  return
-}
-
 # Initialize `dg'.
 dg-init
-if [asan_init] {
+asan_init
 
-# Main loop.
-gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C $srcdir/c-c++-common/asan/*.c]] ""
 
+# Main loop.
+if [check_effective_target_faddress_sanitizer] {
+  gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C $srcdir/c-c++-common/asan/*.c]] ""
 }
 
 # All done.