Re: [PATCH 13/21] PR jit/63854: Add support for running "make check-jit" under valgrind

[David Malcolm <dmalcolm at redhat dot com>]

On Wed, 2014-11-19 at 09:57 -0700, Jeff Law wrote:
> On 11/19/14 03:46, David Malcolm wrote:
> > This commit updates jit.exp so that if RUN_UNDER_VALGRIND is present
> > in the environment, all of the built client code using libgccjit.so is
> > run under valgrind, with --leak-check=full.
> >
> > Hence:
> >    RUN_UNDER_VALGRIND= make check-jit
> > will run all jit testcases under valgrind (taking 27 mins on my
> > machine).
> >
> > Results are written to testsuite/jit/test-FOO.exe.valgrind.txt
> >
> > jit.exp automatically parses these result file, looking for lines of
> > the form
> >    definitely lost: 11,316 bytes in 235 blocks
> >    indirectly lost: 352 bytes in 4 blocks
> > in the valgrind log's summary footer, adding PASSes if they are zero
> > bytes, and, for now generating XFAILs for non-zero bytes.
> >
> > Sadly this diverges jit.exp's fixed_host_execute further from DejaGnu's
> > host_execute, but I don't see a clean way to fix that.
> >
> > This currently adds 63 PASSes and 49 XFAILs to jit.sum, giving:
> >    # of expected passes   2481
> >    # of expected failures 49
> >
> > gcc/testsuite/ChangeLog:
> > 	PR jit/63854
> > 	* jit.dg/jit.exp (report_leak): New.
> > 	(parse_valgrind_logfile): New.
> > 	(fixed_host_execute): Detect if RUN_UNDER_VALGRIND is present
> > 	in the environment, and if so, run the executable under
> > 	valgrind, capturing valgrind's output to a logfile.  Parse the
> > 	log file, generating PASSes and XFAILs for the summary of leaks.
> OK for the trunk.  

Thanks - though the patch I posted uses the contrib/valgrind.supp file,
which I added before seeing the --enable-valgrind-annotations configure
option that does a better job of this.  So I'll revise it to remove that
suppression file (and add some usage notes to the internals/index.rst
document).

> FWIW, I'd love to see a mode where we can easily do 
> this for the other testsuites as well.

I suspect that the functions report_leak and parse_valgrind_logfile
could be moved into a lib/valgrind.exp at some point if someone wants to
reuse them - maybe adding a param to specify if we expect it to be clean
(PASS) vs leaky (XFAIL) - I'm close to having most of the jit testcases
be valgrind-clean.

> All this work is definitely appreciated -- Jakub usually does similar 
> stuff later in the release process, so you're offloading some stuff from 
> him, which definitely helps.

Yeah - leaks are a much bigger issue for libgccjit.so than for e.g. cc1:
people embedding it into long-running processes like, say, an X server
aren't going to be happy about slow leaks.

Dave