Fwd: [PATCH] Add valgrind smoke test

[Alexandra Petlanova Hajkova <ahajkova@redhat.com>]

---------- Forwarded message ---------
From: Alexandra Petlanova Hajkova <ahajkova@redhat.com>
Date: Fri, Jul 2, 2021 at 3:17 PM
Subject: Re: [PATCH] Add valgrind smoke test
To: Joseph Myers <joseph@codesourcery.com>




On Mon, May 24, 2021 at 9:29 PM Joseph Myers <joseph@codesourcery.com>
wrote:

> On Mon, 24 May 2021, Carlos O'Donell via Libc-alpha wrote:
>
> > > +# Run smoke tests with valgrind to verify dynamic loader
> > > +ifneq ($(VALGRIND),false)
> > > +tests-special += $(objpfx)valgrind-smoke-test.out
> > > +$(objpfx)valgrind-smoke-test.out: $(objpfx)ld.so
> > > +   $(common-objpfx)testrun.sh --tool=valgrind-test /usr/bin/true > $@
> > > +   $(common-objpfx)testrun.sh --tool=valgrind-test /usr/bin/true
> --help >> $@
> > > +endif
> > Does this work for cross-compiling?
> >
> > Do you correctly detect the target valgrind and does the test wrapper
> > allow the correct execution of valgrind on the target system?
>
> My expectation is that <host>-valgrind won't exist - but AC_CHECK_TOOL is
> documented as falling back to the tool without a host prefix if a prefixed
> version doesn't exist, which is a problem.  Logically, this test should
> run if valgrind is available on the test system (which would need to be
> determined when the tests are run, test-wrapper isn't available when glibc
> is built), but the test-wrapper interface doesn't strictly support running
> arbitrary installed programs like that.  So you might need a test program
> (run on the test system via test-wrapper) that itself checks for valgrind
> being available in the PATH and runs it or returns UNSUPPORTED if not
> available.  That is, something like nptl/tst-pthread-gdb-attach.c, which
> handles checking for whether gdb is available.
>

 So I should not be using
+AC_CHECK_TOOL([VALGRIND], [valgrind], [false])
at all in my case?
I consider this patch rejected then and will try to come with something like
nptl/tst-pthread-gdb-attach.c - for example elf/tst-valgrind-smoke.c.

Thank you for the review,
Alexandra