Checks
Commit Message
From: Alexandra Hájková <ahajkova@redhat.com>
Check if valgrind is present during the configure time and
run smoke tests with valgrind to verify dynamic loader.
---
Makefile | 4 +++
configure | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++
configure.ac | 1 +
elf/Makefile | 8 +++++
4 files changed, 106 insertions(+)
Comments
On 5/24/21 8:15 AM, Alexandra Hájková via Libc-alpha wrote:
> From: Alexandra Hájková <ahajkova@redhat.com>
>
> Check if valgrind is present during the configure time and
> run smoke tests with valgrind to verify dynamic loader.
Thanks for proposing this. We have been running similar valgrind smoke tests
in Fedora Rawhide and it is very beneficial.
> ---
> Makefile | 4 +++
> configure | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> configure.ac | 1 +
> elf/Makefile | 8 +++++
> 4 files changed, 106 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 50f99ca611..a9ae15d65e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -146,6 +146,7 @@ GCONV_PATH="$${builddir}/iconvdata"
> usage () {
> echo "usage: $$0 [--tool=strace] PROGRAM [ARGUMENTS...]" 2>&1
> echo " $$0 --tool=valgrind PROGRAM [ARGUMENTS...]" 2>&1
> + echo " $$0 --tool=valgrind-test PROGRAM [ARGUMENTS...]" 2>&1
OK. Add valgrind-test to testrun.sh.
> exit 1
> }
>
> @@ -181,6 +182,9 @@ case "$$toolname" in
> valgrind)
> exec env $(run-program-env) valgrind $(test-via-rtld-prefix) $${1+"$$@"}
> ;;
> + valgrind-test)
> + exec env $(run-program-env) valgrind -q --error-exitcode=1 $(test-via-rtld-prefix) $${1+"$$@"}
> + ;;
OK.
> container)
> exec env $(run-program-env) $(test-via-rtld-prefix) \
> $(common-objdir)/support/test-container \
> diff --git a/configure b/configure
> index 5dde2ba355..769341ef05 100755
> --- a/configure
> +++ b/configure
OK.
> --- a/configure.ac
> +++ b/configure.ac
> @@ -52,6 +52,7 @@ fi
> AC_SUBST(cross_compiling)
> AC_PROG_CPP
> AC_CHECK_TOOL(READELF, readelf, false)
> +AC_CHECK_TOOL([VALGRIND], [valgrind], [false])
OK.
>
> # We need the C++ compiler only for testing.
> AC_PROG_CXX
> diff --git a/elf/Makefile b/elf/Makefile
> index 834ec858a8..5b72cc76f9 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -49,6 +49,14 @@ ifeq (yesyes,$(build-shared)$(run-built-tests))
> tests-special += $(objpfx)list-tunables.out
> endif
>
> +# 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?
Please see scripts/cross-test-ssh.sh.
> +
> # Make sure that the compiler does not insert any library calls in tunables
> # code paths.
> ifeq (yes,$(have-loop-to-function))
>
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.
There are problems even in native-like cases when no test wrapper is being
used. What if glibc is being built and tested for 32-bit x86 but
/usr/bin/true is a 64-bit binary? A native build doesn't mean that any
particular installed program uses the same ABI as the glibc being built.
It's not safe to run /usr/bin/true with the newly built glibc at all (and
some systems have /bin/true instead of /usr/bin/true). (That issue could
presumably be addressed by building a test program against the newly built
glibc and running that in place of /usr/bin/true. So you end up needing to
build two test programs against the new glibc, one to check for valgrind
and run it if available, and the other to run under valgrind.)
* Joseph Myers:
> 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.
Would it be okay to run these tests only if $(run-built-tests) is yes,
$(cross-compiling) is no, and configure had found a valgrind binary?
I think even a minimal test which is does not work for cross-compilation
would be valuable.
Thanks,
Florian
On Mon, 28 Jun 2021, Florian Weimer via Libc-alpha wrote:
> Would it be okay to run these tests only if $(run-built-tests) is yes,
> $(cross-compiling) is no, and configure had found a valgrind binary?
We've got rid of the vast bulk of $(cross-compiling) conditionals that
actually change what's built, tested or installed; I don't like the idea
of adding more, when we already have an example of a test that deals with
checking whether a given command (gdb in that case) is available before
running it.
(And you can't avoid the issue of /usr/bin/true possibly not existing or
having the wrong ABI in any case.)
---------- 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
On Fri, 2 Jul 2021, Alexandra Petlanova Hajkova via Libc-alpha wrote:
> So I should not be using
> +AC_CHECK_TOOL([VALGRIND], [valgrind], [false])
> at all in my case?
Indeed.
@@ -146,6 +146,7 @@ GCONV_PATH="$${builddir}/iconvdata"
usage () {
echo "usage: $$0 [--tool=strace] PROGRAM [ARGUMENTS...]" 2>&1
echo " $$0 --tool=valgrind PROGRAM [ARGUMENTS...]" 2>&1
+ echo " $$0 --tool=valgrind-test PROGRAM [ARGUMENTS...]" 2>&1
exit 1
}
@@ -181,6 +182,9 @@ case "$$toolname" in
valgrind)
exec env $(run-program-env) valgrind $(test-via-rtld-prefix) $${1+"$$@"}
;;
+ valgrind-test)
+ exec env $(run-program-env) valgrind -q --error-exitcode=1 $(test-via-rtld-prefix) $${1+"$$@"}
+ ;;
container)
exec env $(run-program-env) $(test-via-rtld-prefix) \
$(common-objdir)/support/test-container \
@@ -690,6 +690,7 @@ sysheaders
ac_ct_CXX
CXXFLAGS
CXX
+VALGRIND
READELF
CPP
cross_compiling
@@ -2922,6 +2923,98 @@ else
READELF="$ac_cv_prog_READELF"
fi
+if test -n "$ac_tool_prefix"; then
+ # Extract the first word of "${ac_tool_prefix}valgrind", so it can be a program name with args.
+set dummy ${ac_tool_prefix}valgrind; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_prog_VALGRIND+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ if test -n "$VALGRIND"; then
+ ac_cv_prog_VALGRIND="$VALGRIND" # Let the user override the test.
+else
+as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+ IFS=$as_save_IFS
+ test -z "$as_dir" && as_dir=.
+ for ac_exec_ext in '' $ac_executable_extensions; do
+ if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+ ac_cv_prog_VALGRIND="${ac_tool_prefix}valgrind"
+ $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+ break 2
+ fi
+done
+ done
+IFS=$as_save_IFS
+
+fi
+fi
+VALGRIND=$ac_cv_prog_VALGRIND
+if test -n "$VALGRIND"; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $VALGRIND" >&5
+$as_echo "$VALGRIND" >&6; }
+else
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+
+fi
+if test -z "$ac_cv_prog_VALGRIND"; then
+ ac_ct_VALGRIND=$VALGRIND
+ # Extract the first word of "valgrind", so it can be a program name with args.
+set dummy valgrind; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_prog_ac_ct_VALGRIND+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ if test -n "$ac_ct_VALGRIND"; then
+ ac_cv_prog_ac_ct_VALGRIND="$ac_ct_VALGRIND" # Let the user override the test.
+else
+as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+ IFS=$as_save_IFS
+ test -z "$as_dir" && as_dir=.
+ for ac_exec_ext in '' $ac_executable_extensions; do
+ if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+ ac_cv_prog_ac_ct_VALGRIND="valgrind"
+ $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+ break 2
+ fi
+done
+ done
+IFS=$as_save_IFS
+
+fi
+fi
+ac_ct_VALGRIND=$ac_cv_prog_ac_ct_VALGRIND
+if test -n "$ac_ct_VALGRIND"; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_ct_VALGRIND" >&5
+$as_echo "$ac_ct_VALGRIND" >&6; }
+else
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+ if test "x$ac_ct_VALGRIND" = x; then
+ VALGRIND="false"
+ else
+ case $cross_compiling:$ac_tool_warned in
+yes:)
+{ $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: using cross tools not prefixed with host triplet" >&5
+$as_echo "$as_me: WARNING: using cross tools not prefixed with host triplet" >&2;}
+ac_tool_warned=yes ;;
+esac
+ VALGRIND=$ac_ct_VALGRIND
+ fi
+else
+ VALGRIND="$ac_cv_prog_VALGRIND"
+fi
+
# We need the C++ compiler only for testing.
ac_ext=cpp
@@ -52,6 +52,7 @@ fi
AC_SUBST(cross_compiling)
AC_PROG_CPP
AC_CHECK_TOOL(READELF, readelf, false)
+AC_CHECK_TOOL([VALGRIND], [valgrind], [false])
# We need the C++ compiler only for testing.
AC_PROG_CXX
@@ -49,6 +49,14 @@ ifeq (yesyes,$(build-shared)$(run-built-tests))
tests-special += $(objpfx)list-tunables.out
endif
+# 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
+
# Make sure that the compiler does not insert any library calls in tunables
# code paths.
ifeq (yes,$(have-loop-to-function))