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]

[PATCH,RFC] Introduce RUN_UNDER_VALGRIND in test-suite


Hello.

In last two weeks I've removed couple of memory leaks, mainly tight to middle-end.
Currently, a user of the GCC compiler can pass '--enable-checking=valgrind' configure option
that will run all commands within valgrind environment, but as the valgrind runs just with '-q' option,
the result is not very helpful.

I would like to start with another approach, where we can run all tests in test-suite
within the valgrind sandbox and return an exit code if there's an error seen by the tool.
That unfortunately leads to many latent (maybe false positives, FE issues, ...) that can
be efficiently ignored by valgrind suppressions file (the file is part of suggested patch).

The first version of the valgrind.supp can survive running compilation of tramp3d with -O2
and majority of tests in test-suite can successfully finish. Most of memory leaks
mentioned in the file can be eventually fixed.

As I noticed in results log files, most of remaining issues are connected to gcc.c and
lto-wrapper.c files. gcc.c heavily manipulates with strings and it would probably require
usage of a string pool, that can easily eventually removed (just in case of --enable-valgrind-annotations).
The second source file tends to produce memory leaks because of fork/exec constructs. However both
can be improved during next stage1.

Apart from aforementioned issues, the compiler does not contain so many issues and I think it's
doable to prune them and rely on reported valgrind errors.

Patch touches many .exp files, but basically does just couple of modifications:

1) gcc-defs.exp introduces new global variable run_under_valgrind
2) new procedure dg-run-valgrind distinguishes between just passing options to 'gd-test',
   or runs 'dg-test' with additional flags that enable valgrind (using -wrapper)
3) new procedure dg-runtest-valgrind does the similar
4) many changes in corresponding *.exp files that utilize these procedures

The patch should be definitely part of next stage1, but I would appreciate any thoughts
about the described approach?

Thank you,
Martin

>From 61286c72b747a1543fc08bad87566afb3656067c Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Wed, 18 Nov 2015 17:49:03 +0100
Subject: [PATCH 1/2] Introduce RUN_UNDER_VALGRIND support in dg-test procedure

contrib/ChangeLog:

2015-11-19  Martin Liska  <mliska@suse.cz>

	* valgrind.supp: New file.

gcc/testsuite/ChangeLog:

2015-11-19  Martin Liska  <mliska@suse.cz>

	* jit.dg/jit.exp: Use global run_under_valgrind instead
	of the local one.
	* lib/g++-dg.exp: Replace dg-test with dg-test-valgrind.
	* lib/gcc-defs.exp: Introduce global variable
	run_under_valgrind.
	* lib/gcc-dg.exp: Add -wrapper that runs a test in valgrind.
	* lib/gfortran-dg.exp: Replace dg-test with dg-test-valgrind.
	* lib/obj-c++-dg.exp: Replace dg-test with dg-test-valgrind.
	* lib/objc-dg.exp: Replace dg-test with dg-test-valgrind.
---
 contrib/valgrind.supp             | 108 ++++++++++++++++++++++++++++++++++++++
 gcc/testsuite/jit.dg/jit.exp      |   2 +-
 gcc/testsuite/lib/g++-dg.exp      |   2 +-
 gcc/testsuite/lib/gcc-defs.exp    |   3 ++
 gcc/testsuite/lib/gcc-dg.exp      |  30 ++++++++++-
 gcc/testsuite/lib/gfortran-dg.exp |   4 +-
 gcc/testsuite/lib/obj-c++-dg.exp  |   4 +-
 gcc/testsuite/lib/objc-dg.exp     |   2 +-
 8 files changed, 146 insertions(+), 9 deletions(-)
 create mode 100644 contrib/valgrind.supp

diff --git a/contrib/valgrind.supp b/contrib/valgrind.supp
new file mode 100644
index 0000000..deefb28
--- /dev/null
+++ b/contrib/valgrind.supp
@@ -0,0 +1,108 @@
+{
+   cpp_get_buff
+   Memcheck:Leak
+   match-leak-kinds: possible
+   fun:malloc
+   fun:xmalloc
+   fun:new_buff
+   fun:_cpp_get_buff
+   ...
+}
+{
+   gnu-as
+   Memcheck:Leak
+   match-leak-kinds: definite,possible
+   fun:malloc
+   ...
+   obj:/usr/bin/as
+   ...
+}
+{
+   gnu-as
+   Memcheck:Leak
+   match-leak-kinds: definite,possible
+   fun:calloc
+   fun:xcalloc
+   ...
+   obj:/usr/bin/as
+   ...
+}
+{
+   todo-fix-mpfr
+   Memcheck:Leak
+   match-leak-kinds: possible
+   fun:malloc
+   fun:__gmp_default_allocate
+   fun:mpfr_init2
+   fun:mpfr_cache
+   fun:mpfr_log
+   ...
+}
+{
+   cpp-front-end
+   Memcheck:Leak
+   match-leak-kinds: definite
+   fun:malloc
+   fun:xmalloc
+   fun:_ZL22cp_literal_operator_idPKc
+   fun:cp_parser_template_name
+   ...
+}
+{
+   todo-fix-options1
+   Memcheck:Leak
+   match-leak-kinds: definite
+   fun:calloc
+   fun:xcalloc
+   fun:_Z20lang_specific_driverPP17cl_decoded_optionPjPi
+   fun:_ZL15process_commandjP17cl_decoded_option
+   fun:_ZNK6driver12set_up_specsEv
+   fun:_ZN6driver4mainEiPPc
+   fun:main
+}
+{
+   todo-fix-options2
+   Memcheck:Leak
+   match-leak-kinds: definite
+   fun:malloc
+   fun:xmalloc
+   fun:_Z20lang_specific_driverPP17cl_decoded_optionPjPi
+   fun:_ZL15process_commandjP17cl_decoded_option
+   fun:_ZNK6driver12set_up_specsEv
+   fun:_ZN6driver4mainEiPPc
+   fun:main
+}
+{
+   ld.bfd-alloc
+   Memcheck:Leak
+   match-leak-kinds: definite,possible
+   fun:malloc
+   ...
+   obj:/usr/bin/ld.bfd
+   ...
+}
+{
+   ld.bfd-realloc
+   Memcheck:Leak
+   match-leak-kinds: definite,possible
+   fun:realloc
+   ...
+   obj:/usr/bin/ld.bfd
+   ...
+}
+{
+   <insert_a_suppression_name_here>
+   Memcheck:Leak
+   match-leak-kinds: definite
+   fun:calloc
+   fun:xcalloc
+   fun:main
+}
+{
+   collect2
+   Memcheck:Leak
+   match-leak-kinds: definite
+   fun:calloc
+   fun:xcalloc
+   fun:main
+}
diff --git a/gcc/testsuite/jit.dg/jit.exp b/gcc/testsuite/jit.dg/jit.exp
index 39e37c2..155a4cf 100644
--- a/gcc/testsuite/jit.dg/jit.exp
+++ b/gcc/testsuite/jit.dg/jit.exp
@@ -138,6 +138,7 @@ proc fixed_host_execute {args} {
     global env
     global text
     global spawn_id
+    global run_under_valgrind
 
     verbose "fixed_host_execute: $args"
 
@@ -169,7 +170,6 @@ proc fixed_host_execute {args} {
     # Run under valgrind if RUN_UNDER_VALGRIND is present in the environment.
     # Note that it's best to configure gcc with --enable-valgrind-annotations
     # when testing under valgrind.
-    set run_under_valgrind [info exists env(RUN_UNDER_VALGRIND)]
     if $run_under_valgrind {
 	set valgrind_logfile "${executable}.valgrind.txt"
 	set valgrind_params {"valgrind"}
diff --git a/gcc/testsuite/lib/g++-dg.exp b/gcc/testsuite/lib/g++-dg.exp
index 421f8b6..7b2f89c 100644
--- a/gcc/testsuite/lib/g++-dg.exp
+++ b/gcc/testsuite/lib/g++-dg.exp
@@ -55,7 +55,7 @@ proc g++-dg-runtest { testcases flags default-extra-flags } {
 
 	foreach flags_t $option_list {
 	    verbose "Testing $nshort, $flags $flags_t" 1
-	    dg-test $test "$flags $flags_t" ${default-extra-flags}
+	    dg-test-valgrind $test "$flags $flags_t" ${default-extra-flags}
 	}
     }
 }
diff --git a/gcc/testsuite/lib/gcc-defs.exp b/gcc/testsuite/lib/gcc-defs.exp
index a30b176..7d4a0c5 100644
--- a/gcc/testsuite/lib/gcc-defs.exp
+++ b/gcc/testsuite/lib/gcc-defs.exp
@@ -352,3 +352,6 @@ proc gcc-set-multilib-library-path { compiler } {
 
     return $libpath
 }
+
+global run_under_valgrind
+set run_under_valgrind [info exists env(RUN_UNDER_VALGRIND)]
diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
index 8cc1d87..93a8db2 100644
--- a/gcc/testsuite/lib/gcc-dg.exp
+++ b/gcc/testsuite/lib/gcc-dg.exp
@@ -310,6 +310,19 @@ proc gcc-dg-test { prog do_what extra_tool_flags } {
     return [gcc-dg-test-1 gcc_target_compile $prog $do_what $extra_tool_flags]
 }
 
+proc dg-test-valgrind { test flags default-extra-flags } {
+    global srcdir
+    global run_under_valgrind
+
+    set valgrind_command "valgrind,--leak-check=yes,--trace-children=yes,--suppressions=${srcdir}/../../contrib/valgrind.supp,--error-exitcode=111,-q"
+
+    if $run_under_valgrind {
+	dg-test $test "$flags -wrapper $valgrind_command" ${default-extra-flags}
+    } else {
+        dg-test $test $flags ${default-extra-flags}
+    }
+}
+
 proc gcc-dg-prune { system text } {
     global additional_prunes
 
@@ -443,6 +456,19 @@ proc search_for { file pattern } {
     return 0
 }
 
+proc dg-runtest-valgrind { testcases flags default-extra-flags } {
+    global runtests
+
+    foreach testcase $testcases {
+	# If we're only testing specific files and this isn't one of them, skip it.
+	if {![runtest_file_p $runtests $testcase]} {
+	    continue
+	}
+	verbose "Testing [file tail [file dirname $testcase]]/[file tail $testcase]"
+	dg-test-valgrind $testcase $flags ${default-extra-flags}
+    }
+}
+
 # Modified dg-runtest that can cycle through a list of optimization options
 # as c-torture does.
 proc gcc-dg-runtest { testcases flags default-extra-flags } {
@@ -477,7 +503,7 @@ proc gcc-dg-runtest { testcases flags default-extra-flags } {
 
 	foreach flags_t $option_list {
 	    verbose "Testing $nshort, $flags $flags_t" 1
-	    dg-test $test "$flags $flags_t" ${default-extra-flags}
+	    dg-test-valgrind $test "$flags $flags_t" ${default-extra-flags}
 	}
     }
 
@@ -556,7 +582,7 @@ proc gcc-dg-debug-runtest { target_compile trivial opt_opts testcases } {
 
 	    if { $doit } {
 		verbose -log "Testing $nshort, $flags" 1
-		dg-test $test $flags ""
+		dg-test-valgrind $test $flags ""
 	    }
 	}
     }
diff --git a/gcc/testsuite/lib/gfortran-dg.exp b/gcc/testsuite/lib/gfortran-dg.exp
index ddf8f22..075586c 100644
--- a/gcc/testsuite/lib/gfortran-dg.exp
+++ b/gcc/testsuite/lib/gfortran-dg.exp
@@ -134,7 +134,7 @@ proc gfortran-dg-runtest { testcases flags default-extra-flags } {
 
 	foreach flags_t $option_list {
 	    verbose "Testing $nshort, $flags $flags_t" 1
-	    dg-test $test "$flags $flags_t" ${default-extra-flags}
+	    dg-test-valgrind $test "$flags $flags_t" ${default-extra-flags}
 	    cleanup-modules ""
 	}
     }
@@ -200,7 +200,7 @@ proc gfortran-dg-debug-runtest { target_compile trivial opt_opts testcases } {
 
            if { $doit } {
                verbose -log "Testing $nshort, $flags" 1
-               dg-test $test $flags ""
+               dg-test-valgrind $test $flags ""
 		cleanup-modules ""
            }
        }
diff --git a/gcc/testsuite/lib/obj-c++-dg.exp b/gcc/testsuite/lib/obj-c++-dg.exp
index 7ba10f1..3a8dfb5 100644
--- a/gcc/testsuite/lib/obj-c++-dg.exp
+++ b/gcc/testsuite/lib/obj-c++-dg.exp
@@ -63,11 +63,11 @@ proc obj-c++-dg-runtest { testcases flags default-extra-flags } {
 	    # combine flags so that dg-skip & xfail will see the extras.
 	    set combined_flags "$flags $flags_t ${default-extra-flags}"
 	    verbose "Testing $nshort, $combined_flags" 1
-	    dg-test $test $combined_flags ""
+	    dg-test-valgrind $test $combined_flags ""
 	}
     }
 
     if { $existing_torture_options == 0 } {
 	torture-finish
     }
-}
\ No newline at end of file
+}
diff --git a/gcc/testsuite/lib/objc-dg.exp b/gcc/testsuite/lib/objc-dg.exp
index 5782593..9c1173f 100644
--- a/gcc/testsuite/lib/objc-dg.exp
+++ b/gcc/testsuite/lib/objc-dg.exp
@@ -64,7 +64,7 @@ proc objc-dg-runtest { testcases flags default-extra-flags } {
 	    # combine flags so that dg-skip & xfail will see the extras.
 	    set combined_flags "$flags $flags_t ${default-extra-flags}"
 	    verbose "Testing $nshort, $combined_flags" 1
-	    dg-test $test $combined_flags ""
+	    dg-test-valgrind $test $combined_flags ""
 	}
     }
 
-- 
2.6.3

Attachment: 0002-Replace-dg-runtest-with-dg-runtest-valgrind.patch.bz2
Description: application/bzip


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