RFA: valgrind and the test suite, take 2

[Tom Tromey <tromey at redhat dot com>]

This needs a documentation review.  I would also appreciate other
comments, as usual.

This patch adds support for valgrind to the test suite.  Unlike my last
patch along these lines, this one adds value above just setting GDB.

In particular, it adds a new test at each point where the tested gdb
exits.  If that gdb run caused valgrind to log any errors, the test
fails.

I ran the entire test suite this way on the compile farm and found a
number of problems (perhaps not all of our causing ... I'm still sorting
through the logs).

Tom

2010-02-19  Tom Tromey  <tromey@redhat.com>

	* gdbint.texinfo (Testsuite): Document use of VALGRIND setting.

2010-02-19  Tom Tromey  <tromey@redhat.com>

	* lib/gdb.exp: Recognize VALGRIND variable.
	(remote_spawn): New wrapper proc.
	(gdb_exit): Likewise.

diff --git a/gdb/doc/gdbint.texinfo b/gdb/doc/gdbint.texinfo
index 7741855..8b02ed8 100644
--- a/gdb/doc/gdbint.texinfo
+++ b/gdb/doc/gdbint.texinfo
@@ -7674,6 +7674,19 @@ HOME=`pwd` runtest \
   INTERNAL_GDBFLAGS=-nw
 @end smallexample
 
+@item @code{VALGRIND}
+
+It is useful to occasionally run @value{GDBN} under @code{valgrind},
+to find bugs that do not always result in an immediate failure.  This
+can be accomplished by setting @code{VALGRIND}.  The contents of this
+will be appended to the internal @code{valgrind} command line that the
+test suite computes.  @code{VALGRIND} can be empty.
+
+Setting this will add an additional check; each time @value{GDBN}
+exits, the contents of a the @code{valgrind} log file are examined
+and, if there was any output, the @samp{valgrind check} test will
+fail.
+
 @end itemize
 
 There are two ways to run the testsuite and pass additional parameters
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index a42d551..7bf1ea9 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -42,6 +42,14 @@ if ![info exists GDB] {
 }
 verbose "using GDB = $GDB" 2
 
+# If VALGRIND is set, we run gdb under valgrind.
+if {[info exists VALGRIND]} {
+    global outdir env vg_basename
+    set env(VGNAME) 0
+    set vg_basename "$outdir/valgrind."
+    set GDB "valgrind --quiet --trace-children=no --child-silent-after-fork=yes --log-file=${vg_basename}%q{VGNAME} $VALGRIND $GDB"
+}
+
 # GDBFLAGS is available for the user to set on the command line.
 # E.g. make check RUNTESTFLAGS=GDBFLAGS=mumble
 # Testcases may use it to add additional flags, but they must:
@@ -3139,6 +3147,50 @@ if {[info exists TRANSCRIPT]} {
   }
 }
 
+# Support for running gdb under valgrind and checking the results.
+if {[info exists VALGRIND]} {
+    rename remote_spawn vg_remote_spawn
+    rename gdb_exit vg_gdb_exit
+
+    proc remote_spawn {args} {
+	global env vg_basename
+	incr env(VGNAME)
+	file delete -force ${vg_basename}$env(VGNAME)
+	return [uplevel vg_remote_spawn $args]
+    }
+
+    proc gdb_exit {args} {
+	global env vg_basename vg_reported
+
+	# We can sometimes see multiple consecutive calls to gdb_exit
+	# without an intervening spawn.  In this case we only want to
+	# report the valgrind results once.
+	if {![info exists vg_reported($env(VGNAME))]} {
+	    set vg_reported($env(VGNAME)) 1
+
+	    # FIXME: how can we name this test consistently across
+	    # runs?
+	    if {[file exists ${vg_basename}$env(VGNAME)]
+		&& [file size ${vg_basename}$env(VGNAME)] > 0} {
+		send_log "Valgrind output:\n"
+		set fd [open ${vg_basename}$env(VGNAME)]
+		send_log [read $fd]
+		close $fd
+		send_log "\n"
+		fail "valgrind check $env(VGNAME)"
+	    } else {
+		pass "valgrind check $env(VGNAME)"
+	    }
+	}
+
+	return [uplevel vg_gdb_exit $args]
+    }
+
+    # Valgrind slows everything down, so boost the default timeout.
+    global gdb_test_timeout
+    set gdb_test_timeout [expr {20 * $gdb_test_timeout}]
+}
+
 proc core_find {binfile {deletefiles {}} {arg ""}} {
     global objdir subdir