Re: RFA: valgrind and the test suite, take 2

[Pedro Alves <pedro@codesourcery.com>]

On Friday 19 February 2010 18:09:45, Joel Brobecker wrote:
> > > 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.
> > 
> > Couldn't this be done with a board file?
> 
> It seems to be that it would be much more convenient to just be able
> to defined VALGRIND to activate this feature.  board files are the only
> way we have to provide setup-specific information (such as target name,
> etc), but the stuff to get testing under valgrind should be the same
> for everyone...
> 

Except when it won't, no?  E.g, I don't see a way to point at a
different valgrind binary than the one in the PATH, or a way to 
change the parameters that are hardcoded.

I took Tom's patch a converted it to the board file pasted
below.  I placed it amongst my other board files in the board
dir pointed at by DEJAGNU/site.exp, and ran it with

make check RUNTESTFLAGS="--target_board=valgrind.exp"

It has the same effect, it seems.  This technique can be
reused for other random ways to wrap gdb, not just valgrind.

Maybe we need a dir where we can place random board
files.  The board file used to test gdbserver locally could
got there as well.  "gdb/contrib", or a subdirectory within,
or somesuch.

> Similarly, I have always found unfortunate that gdb.reverse testing
> is conditioned on a setting that gets set in a board file.  As a result
> of this, I have never run the gdb.reverse testcases even though I could
> have done so on my laptop.

That is different.  Please do not mix things up.  That's about
testing a GDB functionality, _what_ to test, not about _how_ to test
GDB.  It is a deficiency of the testsuite that it doesn't detect
that on native testing on some targets it could use precord
for testing.  It has non complicated fix, and it has been
discussed before.  It just requires someone moving forward
fixing it.

-- 
Pedro Alves

========== valgrind.exp ============

set board_info(localhost,isremote) 0
load_generic_config "unix"

# The default compiler for this target.
set_board_info compiler "[find_gcc]"

global outdir vg_basename
set vg_basename "$outdir/valgrind."
global vg_count
set vg_count "0"

global GDB
set GDB "valgrind --quiet --trace-children=no --child-silent-after-fork=yes --log-file=${vg_basename}%q{VGCOUNT} $GDB"

# Support for running gdb under valgrind and checking the results.

rename remote_spawn vg_remote_spawn
proc remote_spawn {args} {
    global env vg_basename vg_count

    incr vg_count
    set env(VGCOUNT) $vg_count

    file delete -force ${vg_basename}$vg_count
    set res [uplevel vg_remote_spawn $args]

    return $res
}

rename gdb_exit vg_gdb_exit
proc gdb_exit {args} {
    global vg_basename vg_count 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($vg_count)]} {
	set vg_reported($vg_count) 1

	# FIXME: how can we name this test consistently across
	# runs?
	if {[file exists ${vg_basename}$vg_count]
	    && [file size ${vg_basename}$vg_count] > 0} {
	    send_log "Valgrind output:\n"
	    set fd [open ${vg_basename}$vg_count]
	    send_log [read $fd]
	    close $fd
	    send_log "\n"
	    fail "valgrind check $vg_count"
	} else {
	    pass "valgrind check $vg_count"
	}
    }

    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}]