Re: [PATCH] Add valgrind smoke test

[Mark Wielaard <mark@klomp.org>]

Hi DJ,

On Tue, 2021-12-07 at 15:32 -0500, DJ Delorie wrote:
> > +# Test whether valgrind is available in the test
> > +# environment. If not, skip the test.
> > +${test_wrapper_env} \
> > +${run_program_env} \
> > +$rtld --library-path "$library_path" \
> > +  /bin/sh -c 'command -v valgrind' || exit 77
> 
> Tested on bash, sh, and ksh - although 1003.2-1992 doesn't list
> "command" as a built-in.  Not a problem; we don't support systems
> that old ;-)
> 
> We run this script via $(SHELL), we should use $(SHELL) here too, not
> /bin/sh

Can we rely on SHELL being defined here or should we pass that to the
test script too?

> > +${test_wrapper_env} \
> > +${run_program_env} \
> > +$rtld --library-path "$library_path" \
> > +/bin/sh -c "valgrind -q --error-exitcode=1 $test_prog"
> 
> Same $(SHELL) here.
> 
> A bit of inconsistency with $x vs ${x} vs "$x" but that doesn't
> affect
> functionality here.  However, it looks like we're running /bin/sh
> with
> the just-built glibc, and the test program with the system's glibc?
> If you look in $(builddir)/testrun.sh you'll see how we run valgrind
> to
> test the built glibc.
> 
> I.e. (in short):
> 
> ${...} /bin/sh valgrind foo  <- runs /bin/sh with new glibc
> /bin/sh ${...} valgrind foo  <- runs valgrind with new glibc
> /bin/sh valgrind ${...} foo  <- runs foo with new glibc
> 
> Also, if it's your *intent* to test the system valgrind against the
> just-built glibc[*], that needs a comment, and should be tested in a
> cross-compile environment.
> [*] or both valgrind and the test binary

The intent is to test the valgrind available in the test environment
against the just-built glibc (that is both valgrind and the test binary
valgrind-test, which are really the same process)

So we want option 2 from your list. What exactly is ${...} in this
case?

> (in general, testing in a cross-compiled case is an excellent way to
> see
> if you got the rtld stuff in the right place ;-)

I never did a cross-compiled case. Is there a description of that
process?

> > diff --git a/elf/valgrind-test.c b/elf/valgrind-test.c
> > +int
> > +main (void)
> > +{
> > +    setlocale (LC_ALL, "");
> > +    bindtextdomain ("translit", "");
> > +    textdomain ("translit");
> > +
> > +  return 0;
> > +}
> 
> Any reason why these particular calls were chosen, rather than (for
> example) a malloc/free pair?  Perhaps a bit of commentary for future
> readers about what valgrind is expected to be testing here...

See also my other comment in 
https://sourceware.org/pipermail/libc-alpha/2021-December/133714.html

This tests not just a simple malloc/free pair, but also various
string/path comparisons, dlopen, etc. which in the past have seen
issues with valgrind.

Thanks,

Mark