* Re: [PATCH] Add valgrind smoke test
2021-12-06 14:40 [PATCH] Add valgrind smoke test Alexandra Hájková
@ 2021-12-07 11:56 ` Mark Wielaard
2021-12-07 20:32 ` DJ Delorie
` (3 subsequent siblings)
4 siblings, 0 replies; 28+ messages in thread
From: Mark Wielaard @ 2021-12-07 11:56 UTC (permalink / raw)
To: Alexandra Hájková, libc-alpha; +Cc: Alexandra Hájková
Hi,
On Mon, 2021-12-06 at 15:40 +0100, Alexandra Hájková 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.
>
> Co-authored-by: Mark Wielaard <mark@klomp.org>
Let me add some comments on the different parts to help a real review.
> ---
> elf/Makefile | 7 +++++++
> elf/tst-valgrind-smoke.sh | 38 ++++++++++++++++++++++++++++++++++++++
> elf/valgrind-test.c | 31 +++++++++++++++++++++++++++++++
> 3 files changed, 76 insertions(+)
> create mode 100644 elf/tst-valgrind-smoke.sh
> create mode 100644 elf/valgrind-test.c
>
> diff --git a/elf/Makefile b/elf/Makefile
> index ef36008673..14aab3624a 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -232,6 +232,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
> tst-dl-is_dso tst-ro-dynamic \
> tst-audit18 \
> tst-rtld-run-static \
> + valgrind-test
> # reldep9
> tests-internal += loadtest unload unload2 circleload1 \
> neededtest neededtest2 neededtest3 neededtest4 \
Adding valgrind-test to tests simply makes sure it gets build for use
with tst-valgrind-smoke.sh.
> @@ -253,6 +254,12 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
> endif
> endif
> endif
> +
> +tests-special += $(objpfx)tst-valgrind-smoke.out
> +$(objpfx)tst-valgrind-smoke.out: tst-valgrind-smoke.sh $(objpfx)ld.so
> + $(SHELL) $< $(objpfx)ld.so '$(test-wrapper-env)' '$(run-program-env)' '$(rpath-link)' $(objpfx)valgrind-test > $@; \
> + $(evaluate-test)
> +
> tests += $(tests-execstack-$(have-z-execstack))
> ifeq ($(run-built-tests),yes)
> tests-special += $(objpfx)tst-leaks1-mem.out \
We want a tst-valgrind-smoke.out file, which will show the test run.
To get tst-valgrind-smoke.out we need to have tst-valgrind-smoke.sh
(which already exists, but we'll also use it as $< in the make rule)
and ld.so build first. Question, do we also want to depend on
$(objpfx)valgrind-tst to make sure that it exists when this rule is
executed, or is having it in tests enough to express that dependency?
The rule simply passes the just build linker, test and run-program
environments and the rpath of the just build libraries, plus the actual
program to run under valgrind to the tst-valgrind-smoke.sh script. The
exit code of which will then be processed by evaluate-test.
> diff --git a/elf/tst-valgrind-smoke.sh b/elf/tst-valgrind-smoke.sh
> new file mode 100644
> index 0000000000..a78d7ff10d
> --- /dev/null
> +++ b/elf/tst-valgrind-smoke.sh
> @@ -0,0 +1,38 @@
> +#!/bin/sh
> +# Valgrind smoke test.
> +# Copyright (C) 2021 Free Software Foundation, Inc.
> +# This file is part of the GNU C Library.
> +
> +# The GNU C Library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2.1 of the License, or (at your option) any later version.
> +
> +# The GNU C Library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +# Lesser General Public License for more details.
> +
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with the GNU C Library; if not, see
> +# <https://www.gnu.org/licenses/>.
> +
> +set -e
> +
> +rtld=$1
> +test_wrapper_env=$2
> +run_program_env=$3
> +library_path=$4
> +test_prog=$5
> +
> +# 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
First we make sure that valgrind is actually available in the test
environment, if not, like in a cross-build, it will immediately exit
with 77 which produces a SKIP.
> +${test_wrapper_env} \
> +${run_program_env} \
> +$rtld --library-path "$library_path" \
> +/bin/sh -c "valgrind -q --error-exitcode=1 $test_prog"
Then we execute valgrind on the test program (valgrind-test) inside the
test environment using the just build ld.so and libraries. If valgrind
detects any error it exits with exit code 1 (producing a FAIL),
otherwise it will use the exit code (0) of the test_prog.
> diff --git a/elf/valgrind-test.c b/elf/valgrind-test.c
> new file mode 100644
> index 0000000000..606c874b68
> --- /dev/null
> +++ b/elf/valgrind-test.c
> @@ -0,0 +1,31 @@
> +/* This is the simple test intended to be called by
> + tst-valgrind-smoke to perform vagrind smoke test.
> + Copyright (C) 2021 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#include <libintl.h>
> +#include <locale.h>
> +
> +int
> +main (void)
> +{
> + setlocale (LC_ALL, "");
> + bindtextdomain ("translit", "");
> + textdomain ("translit");
> +
> + return 0;
> +}
The program to be run under valgrind in the test environment is fairly
simply, but still exercises some constructs (like string and path
lookups and dlopen) that have seen issues under valgrind in the past.
I hope this valgrind smoke test will be accepted into the glibc
testsuite because it would really help us catch issues with the
valgrind/glibc interactions early.
Cheers,
Mark
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH] Add valgrind smoke test
2021-12-06 14:40 [PATCH] Add valgrind smoke test Alexandra Hájková
2021-12-07 11:56 ` Mark Wielaard
@ 2021-12-07 20:32 ` DJ Delorie
2021-12-07 20:58 ` Florian Weimer
2021-12-10 12:56 ` Mark Wielaard
2021-12-17 18:26 ` Alexandra Hájková
` (2 subsequent siblings)
4 siblings, 2 replies; 28+ messages in thread
From: DJ Delorie @ 2021-12-07 20:32 UTC (permalink / raw)
To: Alexandra Hájková; +Cc: libc-alpha, mark, ahajkova
Alexandra Hjkov via Libc-alpha <libc-alpha@sourceware.org>
writes:
> 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.
>
> Co-authored-by: Mark Wielaard <mark@klomp.org>
Legalities are OK.
> --- a/elf/Makefile
> + valgrind-test
Standard list of tests; valgrind-test.c is the "target" despite wrapping
it in the script.
> @@ -253,6 +254,12 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
> endif
> endif
> endif
> +
> +tests-special += $(objpfx)tst-valgrind-smoke.out
> +$(objpfx)tst-valgrind-smoke.out: tst-valgrind-smoke.sh $(objpfx)ld.so
> + $(SHELL) $< $(objpfx)ld.so '$(test-wrapper-env)' '$(run-program-env)' '$(rpath-link)' $(objpfx)valgrind-test > $@; \
> + $(evaluate-test)
> +
This is our usual way of running a test, ok.
> diff --git a/elf/tst-valgrind-smoke.sh b/elf/tst-valgrind-smoke.sh
> +#!/bin/sh
Note we ignore this as we invoke it via $(SHELL) but ok
> +# Valgrind smoke test.
> +# Copyright (C) 2021 Free Software Foundation, Inc.
> +# This file is part of the GNU C Library.
> +
> +# The GNU C Library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2.1 of the License, or (at your option) any later version.
> +
> +# The GNU C Library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +# Lesser General Public License for more details.
> +
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with the GNU C Library; if not, see
> +# <https://www.gnu.org/licenses/>.
Ok.
> +set -e
> +
> +rtld=$1
> +test_wrapper_env=$2
> +run_program_env=$3
> +library_path=$4
> +test_prog=$5
The Makefile took pains to wrap these in quotes; they should be
similarly quoted here, although in reality the shell does the right
thing. Not a blocker, just my parnoia ;-)
> +# 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
> +${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.
(in general, testing in a cross-compiled case is an excellent way to see
if you got the rtld stuff in the right place ;-)
[*] or both valgrind and the test binary
> 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...
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH] Add valgrind smoke test
2021-12-07 20:32 ` DJ Delorie
@ 2021-12-07 20:58 ` Florian Weimer
2021-12-07 21:10 ` DJ Delorie
2021-12-10 12:56 ` Mark Wielaard
1 sibling, 1 reply; 28+ messages in thread
From: Florian Weimer @ 2021-12-07 20:58 UTC (permalink / raw)
To: DJ Delorie via Libc-alpha
Cc: Alexandra Hájková, DJ Delorie, mark, ahajkova
* DJ Delorie via Libc-alpha:
>> +# 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
Is it a requirement that $(SHELL) exists in the target environment? Not
sure about that. Our containers only have /bin/sh, for example.
Thanks,
Florian
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH] Add valgrind smoke test
2021-12-07 20:58 ` Florian Weimer
@ 2021-12-07 21:10 ` DJ Delorie
0 siblings, 0 replies; 28+ messages in thread
From: DJ Delorie @ 2021-12-07 21:10 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha, alexandra.khirnova, mark, ahajkova
Right, test_wrapper might ssh to a different machine.
Even then, a subtle difference between the host and build ABI could
cause problems (and has, in the past, which is why our test-container
rules are so complex), so we may not be able to run $(SHELL) or /bin/sh
via ld.so, even for a "native" build.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] Add valgrind smoke test
2021-12-07 20:32 ` DJ Delorie
2021-12-07 20:58 ` Florian Weimer
@ 2021-12-10 12:56 ` Mark Wielaard
2021-12-10 13:07 ` Florian Weimer
2021-12-10 19:15 ` DJ Delorie
1 sibling, 2 replies; 28+ messages in thread
From: Mark Wielaard @ 2021-12-10 12:56 UTC (permalink / raw)
To: DJ Delorie, Alexandra Hájková; +Cc: libc-alpha, ahajkova
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
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH] Add valgrind smoke test
2021-12-10 12:56 ` Mark Wielaard
@ 2021-12-10 13:07 ` Florian Weimer
2021-12-10 19:15 ` DJ Delorie
1 sibling, 0 replies; 28+ messages in thread
From: Florian Weimer @ 2021-12-10 13:07 UTC (permalink / raw)
To: Mark Wielaard
Cc: DJ Delorie, Alexandra Hájková, libc-alpha, ahajkova
* Mark Wielaard:
> 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?
SHELL isn't right for the test. It runs in a different environment.
Hard-coding /bin/sh is the right thing to do (like we do in the system
function, so it must already exist for many tests to work).
Thanks,
Florian
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH] Add valgrind smoke test
2021-12-10 12:56 ` Mark Wielaard
2021-12-10 13:07 ` Florian Weimer
@ 2021-12-10 19:15 ` DJ Delorie
2021-12-13 12:55 ` Mark Wielaard
1 sibling, 1 reply; 28+ messages in thread
From: DJ Delorie @ 2021-12-10 19:15 UTC (permalink / raw)
To: Mark Wielaard; +Cc: alexandra.khirnova, libc-alpha, ahajkova
Mark Wielaard <mark@klomp.org> writes:
>> 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
>
> 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)
More like a mix:
/bin/sh ${...} valgrind ${...} foo
The ${...} is all the variables needed to run in the just-built
environment, so ${test_wrapper_env} ${run_program_env} $rtld
--library-path "$library_path"
You need to include all that muck for each binary you want to run in the
just-built environment; it's not inherited.
Although you likely don't want the test_wrapper_env for the "foo" as it
might include a nested ssh, it's complicated and requires a bit of trial
and error. You definitely need the rest else the test program will use
the target's system libc.
>> (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?
You can use build-many-glibcs to do a cross build but I don't know if
that supports running the testsuite fully.
Typically you'd have a remote machine you can ssh to, which nfs mounts
your build to the same path, then set a variable to wrap all the tests
in a script that ssh's.
https://sourceware.org/glibc/wiki/Testing/Testsuite#Testing_with_a_cross-compiler
Look at glibc's scripts/cross-test-ssh.sh for instructions
> 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.
Ok.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH] Add valgrind smoke test
2021-12-10 19:15 ` DJ Delorie
@ 2021-12-13 12:55 ` Mark Wielaard
0 siblings, 0 replies; 28+ messages in thread
From: Mark Wielaard @ 2021-12-13 12:55 UTC (permalink / raw)
To: DJ Delorie; +Cc: alexandra.khirnova, libc-alpha, ahajkova
On Fri, 2021-12-10 at 14:15 -0500, DJ Delorie wrote:
> Mark Wielaard <mark@klomp.org> writes:
> > > 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
> >
> > 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)
>
> More like a mix:
>
> /bin/sh ${...} valgrind ${...} foo
>
> The ${...} is all the variables needed to run in the just-built
> environment, so ${test_wrapper_env} ${run_program_env} $rtld
> --library-path "$library_path"
>
> You need to include all that muck for each binary you want to run in the
> just-built environment; it's not inherited.
>
> Although you likely don't want the test_wrapper_env for the "foo" as it
> might include a nested ssh, it's complicated and requires a bit of trial
> and error. You definitely need the rest else the test program will use
> the target's system libc.
OK. So we would like to invoke the shell /bin/sh as is, so that is just
the system shell? Then we want to use that to invoke valgrind in the
test environment (with or without the new ld.so which doesn't really
matter because...) we then invoke valgrind on the newly build ld.so
with the new library_path, which then invokes our test binary.
One question is why /bin/sh itself doesn't need to run inside/using the
test environment. Doesn't it make things easier if the /bin/sh itself
is executing inside the test environment or does that complicate
things?
But either way should work, although running valgrind on ld.so directly
isn't really supported on some arches (e.g. ppc64 because of some odd
relocation issues). But if we can figure out what the system ld.so is
then we can use that in the first invocation to see if it works and
SKIP, exit 77 when it doesn't.
What is the correct way to extract the system ld.so?
So the tst-valgrind-smoke.sh would have the test whether valgrind is
available as is. Then an extra test to see if valgrind running on the
system ld.so actually works in the test environment. And finally the
actual test using the new ld.so and test library path.
# Test valgrind is available in the test environment
${test_wrapper_env} ${run_program_env} \
$rtld --library-path "$library_path" \
/bin/sh -c 'command -v valgrind' || exit 77
# Test valgrind works with the system ld.so in the test environment
# XXX how to get/set ${system_rtld}???
/bin/sh -c \
"${test_wrapper_env} ${run_program_env} \
valgrind -q --error-exitcode=1 \
${system_rtld} ${test_prog}" || exit 77
# Run the actual test under valgrind and the new ld.so
/bin/sh -c \
"${test_wrapper_env} ${run_program_env} \
valgrind -q --error-exitcode=1 \
${rtld} --library-path ${library_path} ${test_prog}"
> > > (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?
>
> You can use build-many-glibcs to do a cross build but I don't know if
> that supports running the testsuite fully.
Do you have an example how to invoke this script?
It starts with "This script takes as arguments a directory name
(containing a src subdirectory with sources of the relevant toolchain
components)". What are the relevant toolchain components?
> Typically you'd have a remote machine you can ssh to, which nfs mounts
> your build to the same path, then set a variable to wrap all the tests
> in a script that ssh's.
>
> https://sourceware.org/glibc/wiki/Testing/Testsuite#Testing_with_a_cross-compiler
>
> Look at glibc's scripts/cross-test-ssh.sh for instructions
OK, so we need multiple machines that can nfs mount each others source
and/or build dirs. Is there a setup like that in the gcc compile farm?
Thanks,
Mark
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH] Add valgrind smoke test
2021-12-06 14:40 [PATCH] Add valgrind smoke test Alexandra Hájková
2021-12-07 11:56 ` Mark Wielaard
2021-12-07 20:32 ` DJ Delorie
@ 2021-12-17 18:26 ` Alexandra Hájková
2021-12-17 21:07 ` DJ Delorie
2021-12-20 11:37 ` Alexandra Hájková
2022-01-12 17:15 ` Alexandra Hájková
4 siblings, 1 reply; 28+ messages in thread
From: Alexandra Hájková @ 2021-12-17 18:26 UTC (permalink / raw)
To: libc-alpha; +Cc: Alexandra Hájková, Mark Wielaard
From: Alexandra Hájková <ahajkova@redhat.com>
Check if whether valgrind is available in the test environment.
If not, skip the test. Run smoke tests with valgrind to verify dynamic loader.
First, check if algrind works with the system ld.so in the test
environment. Then run the actual test inside the test environment,
using the just build ld.so and new libraries.
Co-authored-by: Mark Wielaard <mark@klomp.org>
---
elf/Makefile | 7 ++++++
elf/tst-valgrind-smoke.sh | 46 +++++++++++++++++++++++++++++++++++++++
elf/valgrind-test.c | 44 +++++++++++++++++++++++++++++++++++++
3 files changed, 97 insertions(+)
create mode 100644 elf/tst-valgrind-smoke.sh
create mode 100644 elf/valgrind-test.c
diff --git a/elf/Makefile b/elf/Makefile
index fe42caeb0e..9c9d331311 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -230,6 +230,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
tst-glibc-hwcaps tst-glibc-hwcaps-prepend tst-glibc-hwcaps-mask \
tst-tls20 tst-tls21 tst-dlmopen-dlerror tst-dlmopen-gethostbyname \
tst-dl-is_dso tst-ro-dynamic \
+ valgrind-test \
tst-audit18 \
tst-rtld-run-static \
# reldep9
@@ -256,6 +257,12 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
endif
endif
endif
+
+tests-special += $(objpfx)tst-valgrind-smoke.out
+$(objpfx)tst-valgrind-smoke.out: tst-valgrind-smoke.sh $(objpfx)ld.so $(objpfx)valgrind-test
+ $(SHELL) $< $(objpfx)ld.so $(rtlddir)/$(rtld-installed-name) '$(test-wrapper-env)' '$(run-program-env)' '$(rpath-link)' $(objpfx)valgrind-test > $@; \
+ $(evaluate-test)
+
tests += $(tests-execstack-$(have-z-execstack))
ifeq ($(run-built-tests),yes)
tests-special += $(objpfx)tst-leaks1-mem.out \
diff --git a/elf/tst-valgrind-smoke.sh b/elf/tst-valgrind-smoke.sh
new file mode 100644
index 0000000000..b51e2907a3
--- /dev/null
+++ b/elf/tst-valgrind-smoke.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+# Valgrind smoke test.
+# Copyright (C) 2021 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+
+# The GNU C Library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <https://www.gnu.org/licenses/>.
+
+set -e
+
+rtld="$1"
+system_rtld="$2"
+test_wrapper_env="$3"
+run_program_env="$4"
+library_path="$5"
+test_prog="$6"
+
+# Test whether valgrind is available in the test
+# environment. If not, skip the test.
+${test_wrapper_env} ${run_program_env} \
+ /bin/sh -c "command -v valgrind" || exit 77
+
+# Test valgrind works with the system ld.so in the test environment
+/bin/sh -c \
+ "${test_wrapper_env} ${run_program_env} \
+ valgrind -q --error-exitcode=1 \
+ ${system_rtld} ${test_prog} ${system_rtld}" || exit 77
+
+# Finally the actual test inside the test environment,
+# using the just build ld.so and new libraries to run
+# the smoke test under valgrind.
+/bin/sh -c \
+ "${test_wrapper_env} ${run_program_env} \
+ valgrind -q --error-exitcode=1 \
+ ${rtld} --library-path ${library_path} ${test_prog} ${rtld}"
diff --git a/elf/valgrind-test.c b/elf/valgrind-test.c
new file mode 100644
index 0000000000..00c8ec1ad3
--- /dev/null
+++ b/elf/valgrind-test.c
@@ -0,0 +1,44 @@
+/* This is the simple test intended to be called by
+ tst-valgrind-smoke to perform vagrind smoke test.
+ Copyright (C) 2021 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <libintl.h>
+#include <locale.h>
+#include <limits.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+int
+main (int argc, char **argv)
+{
+ // Do some non-trivial stuff that has been known to trigger
+ // issues under valgrind in the past.
+ // Setting up the locale textdomain makes sure to test some
+ // string/path comparisons, file search and library loading.
+ setlocale (LC_ALL, "");
+ bindtextdomain ("translit", "");
+ textdomain ("translit");
+
+ // Show what we are executing and how...
+ char *me = realpath (argv[0], NULL);
+ printf ("bin: %s\n", me);
+ printf ("ld.so: %s\n", argv[1]);
+ free (me);
+
+ return 0;
+}
--
2.26.3
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH] Add valgrind smoke test
2021-12-17 18:26 ` Alexandra Hájková
@ 2021-12-17 21:07 ` DJ Delorie
2021-12-20 11:31 ` Alexandra Petlanova Hajkova
0 siblings, 1 reply; 28+ messages in thread
From: DJ Delorie @ 2021-12-17 21:07 UTC (permalink / raw)
To: Alexandra Hájková; +Cc: libc-alpha, mark, ahajkova
Alexandra Hjkov via Libc-alpha <libc-alpha@sourceware.org>
writes:
> Check if whether valgrind is available in the test environment.
> If not, skip the test. Run smoke tests with valgrind to verify dynamic loader.
> First, check if algrind works with the system ld.so in the test
> environment. Then run the actual test inside the test environment,
> using the just build ld.so and new libraries.
>
> Co-authored-by: Mark Wielaard <mark@klomp.org>
> diff --git a/elf/Makefile b/elf/Makefile
> + valgrind-test \
New test. Ok.
> @@ -256,6 +257,12 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
> +tests-special += $(objpfx)tst-valgrind-smoke.out
> +$(objpfx)tst-valgrind-smoke.out: tst-valgrind-smoke.sh $(objpfx)ld.so $(objpfx)valgrind-test
> + $(SHELL) $< $(objpfx)ld.so $(rtlddir)/$(rtld-installed-name) '$(test-wrapper-env)' '$(run-program-env)' '$(rpath-link)' $(objpfx)valgrind-test > $@; \
> + $(evaluate-test)
We're passing everything to the script, which is $<, so "see below".
Long lines should be wrapped, but Ok with that change.
> diff --git a/elf/tst-valgrind-smoke.sh b/elf/tst-valgrind-smoke.sh
> +#!/bin/sh
> +# Valgrind smoke test.
> +# Copyright (C) 2021 Free Software Foundation, Inc.
> +# This file is part of the GNU C Library.
> +
> +# The GNU C Library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2.1 of the License, or (at your option) any later version.
> +
> +# The GNU C Library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +# Lesser General Public License for more details.
> +
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with the GNU C Library; if not, see
> +# <https://www.gnu.org/licenses/>.
Ok.
> +set -e
> +
> +rtld="$1"
> +system_rtld="$2"
> +test_wrapper_env="$3"
> +run_program_env="$4"
> +library_path="$5"
> +test_prog="$6"
rtld is $(objpfx)ld.so
system_rtld is $(rtlddir)/$(rtld-installed-name)
test_wrapper_env is '$(test-wrapper-env)' (either just "env" or script
to ssh, plus "env")
run_program_env is '$(run-program-env)' (just env vars)
library_path is '$(rpath-link)'
test_prog is $(objpfx)valgrind-test
ok
> +# Test whether valgrind is available in the test
> +# environment. If not, skip the test.
> +${test_wrapper_env} ${run_program_env} \
> + /bin/sh -c "command -v valgrind" || exit 77
Run valgrind with no options; we should only exit if valgrind is
missing. Ok.
> +# Test valgrind works with the system ld.so in the test environment
> +/bin/sh -c \
> + "${test_wrapper_env} ${run_program_env} \
> + valgrind -q --error-exitcode=1 \
> + ${system_rtld} ${test_prog} ${system_rtld}" || exit 77
runs on target, native valgrind with in-build test with native rtld...
Note that the test_prog built against the new glibc probably won't run
against the old installed glibc, esp if there's a version bump. You
might need to use something other than the test_prog, like /bin/echo,
for this test.
Running the build/test on an older Fedora version should have picked
this up.
> +# Finally the actual test inside the test environment,
> +# using the just build ld.so and new libraries to run
> +# the smoke test under valgrind.
> +/bin/sh -c \
> + "${test_wrapper_env} ${run_program_env} \
> + valgrind -q --error-exitcode=1 \
> + ${rtld} --library-path ${library_path} ${test_prog} ${rtld}"
runs on target, native valgrind, runs target program with new rtld and
libraries, ok.
> diff --git a/elf/valgrind-test.c b/elf/valgrind-test.c
> +/* This is the simple test intended to be called by
> + tst-valgrind-smoke to perform vagrind smoke test.
> + Copyright (C) 2021 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#include <libintl.h>
> +#include <locale.h>
> +#include <limits.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +int
> +main (int argc, char **argv)
> +{
> + // Do some non-trivial stuff that has been known to trigger
> + // issues under valgrind in the past.
> + // Setting up the locale textdomain makes sure to test some
> + // string/path comparisons, file search and library loading.
> + setlocale (LC_ALL, "");
> + bindtextdomain ("translit", "");
> + textdomain ("translit");
> +
> + // Show what we are executing and how...
> + char *me = realpath (argv[0], NULL);
> + printf ("bin: %s\n", me);
> + printf ("ld.so: %s\n", argv[1]);
> + free (me);
> +
> + return 0;
> +}
Ok.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH] Add valgrind smoke test
2021-12-17 21:07 ` DJ Delorie
@ 2021-12-20 11:31 ` Alexandra Petlanova Hajkova
0 siblings, 0 replies; 28+ messages in thread
From: Alexandra Petlanova Hajkova @ 2021-12-20 11:31 UTC (permalink / raw)
To: DJ Delorie; +Cc: Alexandra Hájková, libc-alpha, Mark Wielaard
> > +# Test valgrind works with the system ld.so in the test environment
> > +/bin/sh -c \
> > + "${test_wrapper_env} ${run_program_env} \
> > + valgrind -q --error-exitcode=1 \
> > + ${system_rtld} ${test_prog} ${system_rtld}" || exit 77
>
> runs on target, native valgrind with in-build test with native rtld...
>
> Note that the test_prog built against the new glibc probably won't run
> against the old installed glibc, esp if there's a version bump. You
> might need to use something other than the test_prog, like /bin/echo,
> for this test.
>
> Running the build/test on an older Fedora version should have picked
> this up.
>
I tried on Fedora 32 and the test ended here with UNSUPPORTED indeed,
we initially tested it on Fedora Rawhide. Using /bin/echo for the next
version.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH] Add valgrind smoke test
2021-12-06 14:40 [PATCH] Add valgrind smoke test Alexandra Hájková
` (2 preceding siblings ...)
2021-12-17 18:26 ` Alexandra Hájková
@ 2021-12-20 11:37 ` Alexandra Hájková
2022-01-10 12:13 ` Mark Wielaard
2022-01-10 12:38 ` Adhemerval Zanella
2022-01-12 17:15 ` Alexandra Hájková
4 siblings, 2 replies; 28+ messages in thread
From: Alexandra Hájková @ 2021-12-20 11:37 UTC (permalink / raw)
To: libc-alpha; +Cc: Alexandra Hájková, Mark Wielaard
From: Alexandra Hájková <ahajkova@redhat.com>
Check if whether valgrind is available in the test environment.
If not, skip the test. Run smoke tests with valgrind to verify dynamic loader.
First, check if algrind works with the system ld.so in the test
environment. Then run the actual test inside the test environment,
using the just build ld.so and new libraries.
Co-authored-by: Mark Wielaard <mark@klomp.org>
---
elf/Makefile | 7 ++++++
elf/tst-valgrind-smoke.sh | 46 +++++++++++++++++++++++++++++++++++++++
elf/valgrind-test.c | 44 +++++++++++++++++++++++++++++++++++++
3 files changed, 97 insertions(+)
create mode 100644 elf/tst-valgrind-smoke.sh
create mode 100644 elf/valgrind-test.c
diff --git a/elf/Makefile b/elf/Makefile
index fe42caeb0e..c2da351c9e 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -230,6 +230,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
tst-glibc-hwcaps tst-glibc-hwcaps-prepend tst-glibc-hwcaps-mask \
tst-tls20 tst-tls21 tst-dlmopen-dlerror tst-dlmopen-gethostbyname \
tst-dl-is_dso tst-ro-dynamic \
+ valgrind-test \
tst-audit18 \
tst-rtld-run-static \
# reldep9
@@ -256,6 +257,12 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
endif
endif
endif
+
+tests-special += $(objpfx)tst-valgrind-smoke.out
+$(objpfx)tst-valgrind-smoke.out: tst-valgrind-smoke.sh $(objpfx)ld.so $(objpfx)valgrind-test
+ $(SHELL) $< $(objpfx)ld.so $(rtlddir)/$(rtld-installed-name) '$(test-wrapper-env)' \
+ '$(run-program-env)' '$(rpath-link)' $(objpfx)valgrind-test > $@; $(evaluate-test)
+
tests += $(tests-execstack-$(have-z-execstack))
ifeq ($(run-built-tests),yes)
tests-special += $(objpfx)tst-leaks1-mem.out \
diff --git a/elf/tst-valgrind-smoke.sh b/elf/tst-valgrind-smoke.sh
new file mode 100644
index 0000000000..cc32136268
--- /dev/null
+++ b/elf/tst-valgrind-smoke.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+# Valgrind smoke test.
+# Copyright (C) 2021 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+
+# The GNU C Library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <https://www.gnu.org/licenses/>.
+
+set -e
+
+rtld="$1"
+system_rtld="$2"
+test_wrapper_env="$3"
+run_program_env="$4"
+library_path="$5"
+test_prog="$6"
+
+# Test whether valgrind is available in the test
+# environment. If not, skip the test.
+${test_wrapper_env} ${run_program_env} \
+ /bin/sh -c "command -v valgrind" || exit 77
+
+# Test valgrind works with the system ld.so in the test environment
+/bin/sh -c \
+ "${test_wrapper_env} ${run_program_env} \
+ valgrind -q --error-exitcode=1 \
+ ${system_rtld} /bin/echo ${system_rtld}" || exit 77
+
+# Finally the actual test inside the test environment,
+# using the just build ld.so and new libraries to run
+# the smoke test under valgrind.
+/bin/sh -c \
+ "${test_wrapper_env} ${run_program_env} \
+ valgrind -q --error-exitcode=1 \
+ ${rtld} --library-path ${library_path} ${test_prog} ${rtld}"
diff --git a/elf/valgrind-test.c b/elf/valgrind-test.c
new file mode 100644
index 0000000000..00c8ec1ad3
--- /dev/null
+++ b/elf/valgrind-test.c
@@ -0,0 +1,44 @@
+/* This is the simple test intended to be called by
+ tst-valgrind-smoke to perform vagrind smoke test.
+ Copyright (C) 2021 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <libintl.h>
+#include <locale.h>
+#include <limits.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+int
+main (int argc, char **argv)
+{
+ // Do some non-trivial stuff that has been known to trigger
+ // issues under valgrind in the past.
+ // Setting up the locale textdomain makes sure to test some
+ // string/path comparisons, file search and library loading.
+ setlocale (LC_ALL, "");
+ bindtextdomain ("translit", "");
+ textdomain ("translit");
+
+ // Show what we are executing and how...
+ char *me = realpath (argv[0], NULL);
+ printf ("bin: %s\n", me);
+ printf ("ld.so: %s\n", argv[1]);
+ free (me);
+
+ return 0;
+}
--
2.26.3
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH] Add valgrind smoke test
2021-12-20 11:37 ` Alexandra Hájková
@ 2022-01-10 12:13 ` Mark Wielaard
2022-01-10 12:38 ` Adhemerval Zanella
1 sibling, 0 replies; 28+ messages in thread
From: Mark Wielaard @ 2022-01-10 12:13 UTC (permalink / raw)
To: Alexandra Hájková, libc-alpha; +Cc: Alexandra Hájková
HI,
On Mon, 2021-12-20 at 12:37 +0100, Alexandra Hájková wrote:
> Check if whether valgrind is available in the test environment.
> If not, skip the test. Run smoke tests with valgrind to verify
> dynamic loader.
> First, check if algrind works with the system ld.so in the test
> environment. Then run the actual test inside the test environment,
> using the just build ld.so and new libraries.
> [...]
> +# Test valgrind works with the system ld.so in the test environment
> +/bin/sh -c \
> + "${test_wrapper_env} ${run_program_env} \
> + valgrind -q --error-exitcode=1 \
> + ${system_rtld} /bin/echo ${system_rtld}" || exit 77
So now you use /bin/echo instead of the newly build testcase.
Looks ok. The main thing is that it tests running the system ld.so
under valgrind works. If it does, then the new ld.so should too.
I think that is correct given DJ's feedback. Although I admit am a
little surprised to be honest. I didn't think the test used any new
symbol (versions), so would also work against the system libc. But
since it was build against the fresh glibc, it does make sense it could
pull in newer symbol versions than are available on the system.
This version looks good to me to get integrated.
Thanks,
Mark
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH] Add valgrind smoke test
2021-12-20 11:37 ` Alexandra Hájková
2022-01-10 12:13 ` Mark Wielaard
@ 2022-01-10 12:38 ` Adhemerval Zanella
1 sibling, 0 replies; 28+ messages in thread
From: Adhemerval Zanella @ 2022-01-10 12:38 UTC (permalink / raw)
To: libc-alpha
On 20/12/2021 08:37, Alexandra Hájková via Libc-alpha wrote:
> From: Alexandra Hájková <ahajkova@redhat.com>
>
> Check if whether valgrind is available in the test environment.
> If not, skip the test. Run smoke tests with valgrind to verify dynamic loader.
> First, check if algrind works with the system ld.so in the test
> environment. Then run the actual test inside the test environment,
> using the just build ld.so and new libraries.
>
> Co-authored-by: Mark Wielaard <mark@klomp.org>
> ---
> elf/Makefile | 7 ++++++
> elf/tst-valgrind-smoke.sh | 46 +++++++++++++++++++++++++++++++++++++++
> elf/valgrind-test.c | 44 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 97 insertions(+)
> create mode 100644 elf/tst-valgrind-smoke.sh
> create mode 100644 elf/valgrind-test.c
>
> diff --git a/elf/Makefile b/elf/Makefile
> index fe42caeb0e..c2da351c9e 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -230,6 +230,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
> tst-glibc-hwcaps tst-glibc-hwcaps-prepend tst-glibc-hwcaps-mask \
> tst-tls20 tst-tls21 tst-dlmopen-dlerror tst-dlmopen-gethostbyname \
> tst-dl-is_dso tst-ro-dynamic \
> + valgrind-test \
> tst-audit18 \
> tst-rtld-run-static \
> # reldep9
> @@ -256,6 +257,12 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
> endif
> endif
> endif
> +
> +tests-special += $(objpfx)tst-valgrind-smoke.out
> +$(objpfx)tst-valgrind-smoke.out: tst-valgrind-smoke.sh $(objpfx)ld.so $(objpfx)valgrind-test
> + $(SHELL) $< $(objpfx)ld.so $(rtlddir)/$(rtld-installed-name) '$(test-wrapper-env)' \
> + '$(run-program-env)' '$(rpath-link)' $(objpfx)valgrind-test > $@; $(evaluate-test)
> +
> tests += $(tests-execstack-$(have-z-execstack))
> ifeq ($(run-built-tests),yes)
> tests-special += $(objpfx)tst-leaks1-mem.out \
> diff --git a/elf/tst-valgrind-smoke.sh b/elf/tst-valgrind-smoke.sh
> new file mode 100644
> index 0000000000..cc32136268
> --- /dev/null
> +++ b/elf/tst-valgrind-smoke.sh
> @@ -0,0 +1,46 @@
> +#!/bin/sh
> +# Valgrind smoke test.
> +# Copyright (C) 2021 Free Software Foundation, Inc.
Update the Copyright date.
> +# This file is part of the GNU C Library.
> +
> +# The GNU C Library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2.1 of the License, or (at your option) any later version.
> +
> +# The GNU C Library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +# Lesser General Public License for more details.
> +
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with the GNU C Library; if not, see
> +# <https://www.gnu.org/licenses/>.
> +
> +set -e
> +
> +rtld="$1"
> +system_rtld="$2"
> +test_wrapper_env="$3"
> +run_program_env="$4"
> +library_path="$5"
> +test_prog="$6"
> +
> +# Test whether valgrind is available in the test
> +# environment. If not, skip the test.
I think double space after period should be used on shell comments as well.
> +${test_wrapper_env} ${run_program_env} \
> + /bin/sh -c "command -v valgrind" || exit 77
> +
> +# Test valgrind works with the system ld.so in the test environment
> +/bin/sh -c \
> + "${test_wrapper_env} ${run_program_env} \
> + valgrind -q --error-exitcode=1 \
> + ${system_rtld} /bin/echo ${system_rtld}" || exit 77
> +
> +# Finally the actual test inside the test environment,
> +# using the just build ld.so and new libraries to run
> +# the smoke test under valgrind.
> +/bin/sh -c \
> + "${test_wrapper_env} ${run_program_env} \
> + valgrind -q --error-exitcode=1 \
> + ${rtld} --library-path ${library_path} ${test_prog} ${rtld}"
> diff --git a/elf/valgrind-test.c b/elf/valgrind-test.c
> new file mode 100644
> index 0000000000..00c8ec1ad3
> --- /dev/null
> +++ b/elf/valgrind-test.c
> @@ -0,0 +1,44 @@
> +/* This is the simple test intended to be called by
> + tst-valgrind-smoke to perform vagrind smoke test.
> + Copyright (C) 2021 Free Software Foundation, Inc.
Ditto.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#include <libintl.h>
> +#include <locale.h>
> +#include <limits.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +int
> +main (int argc, char **argv)
> +{
> + // Do some non-trivial stuff that has been known to trigger
> + // issues under valgrind in the past.
> + // Setting up the locale textdomain makes sure to test some
> + // string/path comparisons, file search and library loading.
Please use '/* ... */' comments.
> + setlocale (LC_ALL, "");
Use xsetlocale.
> + bindtextdomain ("translit", "");
> + textdomain ("translit");
Also check the return here.
> +
> + // Show what we are executing and how...
> + char *me = realpath (argv[0], NULL);
> + printf ("bin: %s\n", me);
> + printf ("ld.so: %s\n", argv[1]);
> + free (me);
> +
> + return 0;
> +}
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH] Add valgrind smoke test
2021-12-06 14:40 [PATCH] Add valgrind smoke test Alexandra Hájková
` (3 preceding siblings ...)
2021-12-20 11:37 ` Alexandra Hájková
@ 2022-01-12 17:15 ` Alexandra Hájková
2022-01-20 19:35 ` Alexandra Hájková
2022-01-20 21:29 ` DJ Delorie
4 siblings, 2 replies; 28+ messages in thread
From: Alexandra Hájková @ 2022-01-12 17:15 UTC (permalink / raw)
To: libc-alpha; +Cc: Alexandra Hájková, Mark Wielaard
From: Alexandra Hájková <ahajkova@redhat.com>
Check if whether valgrind is available in the test environment.
If not, skip the test. Run smoke tests with valgrind to verify dynamic loader.
First, check if algrind works with the system ld.so in the test
environment. Then run the actual test inside the test environment,
using the just build ld.so and new libraries.
Co-authored-by: Mark Wielaard <mark@klomp.org>
---
elf/Makefile | 7 ++++++
elf/tst-valgrind-smoke.sh | 46 ++++++++++++++++++++++++++++++++++++
elf/valgrind-test.c | 49 +++++++++++++++++++++++++++++++++++++++
3 files changed, 102 insertions(+)
create mode 100644 elf/tst-valgrind-smoke.sh
create mode 100644 elf/valgrind-test.c
diff --git a/elf/Makefile b/elf/Makefile
index b86d116be9..77829cf967 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -327,6 +327,7 @@ tests += \
unload6 \
unload7 \
unload8 \
+ valgrind-test \
# reldep9
tests-cxx = \
tst-dlopen-nodelete-reloc \
@@ -361,6 +362,12 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
endif
endif
endif
+
+tests-special += $(objpfx)tst-valgrind-smoke.out
+$(objpfx)tst-valgrind-smoke.out: tst-valgrind-smoke.sh $(objpfx)ld.so $(objpfx)valgrind-test
+ $(SHELL) $< $(objpfx)ld.so $(rtlddir)/$(rtld-installed-name) '$(test-wrapper-env)' \
+ '$(run-program-env)' '$(rpath-link)' $(objpfx)valgrind-test > $@; $(evaluate-test)
+
tests += $(tests-execstack-$(have-z-execstack))
ifeq ($(run-built-tests),yes)
tests-special += $(objpfx)tst-leaks1-mem.out \
diff --git a/elf/tst-valgrind-smoke.sh b/elf/tst-valgrind-smoke.sh
new file mode 100644
index 0000000000..7ce4af6897
--- /dev/null
+++ b/elf/tst-valgrind-smoke.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+# Valgrind smoke test.
+# Copyright (C) 2022 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+
+# The GNU C Library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <https://www.gnu.org/licenses/>.
+
+set -e
+
+rtld="$1"
+system_rtld="$2"
+test_wrapper_env="$3"
+run_program_env="$4"
+library_path="$5"
+test_prog="$6"
+
+# Test whether valgrind is available in the test
+# environment. If not, skip the test.
+${test_wrapper_env} ${run_program_env} \
+ /bin/sh -c "command -v valgrind" || exit 77
+
+# Test valgrind works with the system ld.so in the test environment
+/bin/sh -c \
+ "${test_wrapper_env} ${run_program_env} \
+ valgrind -q --error-exitcode=1 \
+ ${system_rtld} /bin/echo ${system_rtld}" || exit 77
+
+# Finally the actual test inside the test environment,
+# using the just build ld.so and new libraries to run
+# the smoke test under valgrind.
+/bin/sh -c \
+ "${test_wrapper_env} ${run_program_env} \
+ valgrind -q --error-exitcode=1 \
+ ${rtld} --library-path ${library_path} ${test_prog} ${rtld}"
diff --git a/elf/valgrind-test.c b/elf/valgrind-test.c
new file mode 100644
index 0000000000..71b8a09b35
--- /dev/null
+++ b/elf/valgrind-test.c
@@ -0,0 +1,49 @@
+/* This is the simple test intended to be called by
+ tst-valgrind-smoke to perform vagrind smoke test.
+ Copyright (C) 2022 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <errno.h>
+#include <libintl.h>
+#include <locale.h>
+#include <limits.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+#include <support/support.h>
+
+int
+main (int argc, char **argv)
+{
+ /* Do some non-trivial stuff that has been known to trigger
+ issues under valgrind in the past.
+ Setting up the locale textdomain makes sure to test some
+ string/path comparisons, file search and library loading. */
+ xsetlocale (LC_ALL, "");
+ if (bindtextdomain ("translit", "") == NULL)
+ return errno;
+ if (textdomain ("translit") == NULL)
+ return errno;
+
+ /* Show what we are executing and how... */
+ char *me = realpath (argv[0], NULL);
+ printf ("bin: %s\n", me);
+ printf ("ld.so: %s\n", argv[1]);
+ free (me);
+
+ return 0;
+}
--
2.26.3
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH] Add valgrind smoke test
2022-01-12 17:15 ` Alexandra Hájková
@ 2022-01-20 19:35 ` Alexandra Hájková
2022-01-24 18:34 ` Joseph Myers
2022-01-20 21:29 ` DJ Delorie
1 sibling, 1 reply; 28+ messages in thread
From: Alexandra Hájková @ 2022-01-20 19:35 UTC (permalink / raw)
To: libc-alpha
Cc: Alexandra Hájková,
Mark Wielaard, dj, Adhemerval Zanella, Joseph Myers
On Wed, Jan 12, 2022 at 6:15 PM Alexandra Hájková
<alexandra.khirnova@gmail.com> wrote:
>
> From: Alexandra Hájková <ahajkova@redhat.com>
>
> Check if whether valgrind is available in the test environment.
> If not, skip the test. Run smoke tests with valgrind to verify dynamic loader.
> First, check if algrind works with the system ld.so in the test
> environment. Then run the actual test inside the test environment,
> using the just build ld.so and new libraries.
>
> Co-authored-by: Mark Wielaard <mark@klomp.org>
Are there any objections to the new version?
Thank you,
Alexandra
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] Add valgrind smoke test
2022-01-20 19:35 ` Alexandra Hájková
@ 2022-01-24 18:34 ` Joseph Myers
2022-01-26 17:46 ` Joseph Myers
2022-01-26 17:59 ` Mark Wielaard
0 siblings, 2 replies; 28+ messages in thread
From: Joseph Myers @ 2022-01-24 18:34 UTC (permalink / raw)
To: alexandra.khirnova; +Cc: libc-alpha, Mark Wielaard, Alexandra Hájková
I'm seeing
FAIL: elf/tst-valgrind-smoke
for the x86_64 configurations when running build-many-glibcs.py (and
UNSUPPORTED for other architectures). No execution tests of the newly
built glibc should be run at all unless
ifeq ($(run-built-tests),yes)
but I don't see any such conditional around the
tests-special += $(objpfx)tst-valgrind-smoke.out
that causes this test to be run.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] Add valgrind smoke test
2022-01-24 18:34 ` Joseph Myers
@ 2022-01-26 17:46 ` Joseph Myers
2022-01-26 17:59 ` Mark Wielaard
1 sibling, 0 replies; 28+ messages in thread
From: Joseph Myers @ 2022-01-26 17:46 UTC (permalink / raw)
To: alexandra.khirnova; +Cc: Mark Wielaard, libc-alpha, Alexandra Hájková
On Mon, 24 Jan 2022, Joseph Myers wrote:
> I'm seeing
>
> FAIL: elf/tst-valgrind-smoke
>
> for the x86_64 configurations when running build-many-glibcs.py (and
> UNSUPPORTED for other architectures). No execution tests of the newly
> built glibc should be run at all unless
>
> ifeq ($(run-built-tests),yes)
>
> but I don't see any such conditional around the
>
> tests-special += $(objpfx)tst-valgrind-smoke.out
>
> that causes this test to be run.
I haven't seen any further comments on or fixes for this. We're very
close to a release, so please revert the patch introducing problems if
it's not going to be fixed in the next day or two.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] Add valgrind smoke test
2022-01-24 18:34 ` Joseph Myers
2022-01-26 17:46 ` Joseph Myers
@ 2022-01-26 17:59 ` Mark Wielaard
2022-01-26 18:40 ` Joseph Myers
1 sibling, 1 reply; 28+ messages in thread
From: Mark Wielaard @ 2022-01-26 17:59 UTC (permalink / raw)
To: Joseph Myers, alexandra.khirnova; +Cc: libc-alpha, Alexandra Hájková
[-- Attachment #1: Type: text/plain, Size: 628 bytes --]
Hi Joseph,
On Mon, 2022-01-24 at 18:34 +0000, Joseph Myers wrote:
> I'm seeing
>
> FAIL: elf/tst-valgrind-smoke
>
> for the x86_64 configurations when running build-many-glibcs.py (and
> UNSUPPORTED for other architectures). No execution tests of the newly
> built glibc should be run at all unless
>
> ifeq ($(run-built-tests),yes)
>
> but I don't see any such conditional around the
>
> tests-special += $(objpfx)tst-valgrind-smoke.out
>
> that causes this test to be run.
If your analysis is correct then the attached patch should solve the
issue. Could you try that out?
Thanks,
Mark
[-- Attachment #2: 0001-Guard-tst-valgrind-smoke.out-with-run-built-tests.patch --]
[-- Type: text/x-patch, Size: 871 bytes --]
From cc4798c2bf6acfb9e580fab6e4bad469f600e212 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Wed, 26 Jan 2022 18:57:29 +0100
Subject: [PATCH] Guard tst-valgrind-smoke.out with run-built-tests
Prevent tst-valgrind-smoke from running when run-built-tests is not yes.
---
elf/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/elf/Makefile b/elf/Makefile
index daafb5cf12..775c755291 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -530,7 +530,9 @@ endif
endif
endif
+ifeq ($(run-built-tests),yes)
tests-special += $(objpfx)tst-valgrind-smoke.out
+endif
$(objpfx)tst-valgrind-smoke.out: tst-valgrind-smoke.sh $(objpfx)ld.so $(objpfx)valgrind-test
$(SHELL) $< $(objpfx)ld.so $(rtlddir)/$(rtld-installed-name) '$(test-wrapper-env)' \
'$(run-program-env)' '$(rpath-link)' $(objpfx)valgrind-test > $@; $(evaluate-test)
--
2.34.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] Add valgrind smoke test
2022-01-26 17:59 ` Mark Wielaard
@ 2022-01-26 18:40 ` Joseph Myers
2022-01-26 19:23 ` Mark Wielaard
0 siblings, 1 reply; 28+ messages in thread
From: Joseph Myers @ 2022-01-26 18:40 UTC (permalink / raw)
To: Mark Wielaard; +Cc: alexandra.khirnova, libc-alpha, Alexandra Hájková
On Wed, 26 Jan 2022, Mark Wielaard wrote:
> If your analysis is correct then the attached patch should solve the
> issue. Could you try that out?
I confirm that this fixes the problem.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] Add valgrind smoke test
2022-01-26 18:40 ` Joseph Myers
@ 2022-01-26 19:23 ` Mark Wielaard
0 siblings, 0 replies; 28+ messages in thread
From: Mark Wielaard @ 2022-01-26 19:23 UTC (permalink / raw)
To: Joseph Myers; +Cc: alexandra.khirnova, libc-alpha, Alexandra Hájková
On Wed, Jan 26, 2022 at 06:40:10PM +0000, Joseph Myers wrote:
> On Wed, 26 Jan 2022, Mark Wielaard wrote:
>
> > If your analysis is correct then the attached patch should solve the
> > issue. Could you try that out?
>
> I confirm that this fixes the problem.
Great. I did a local make && make check and tst-valgrind-smoke.out is
still generated and the testcase still PASSes locally.
Can I push the patch as is to master, or are we that close to a
release that a release manager needs to ack the patch first?
Thanks,
Mark
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] Add valgrind smoke test
2022-01-12 17:15 ` Alexandra Hájková
2022-01-20 19:35 ` Alexandra Hájková
@ 2022-01-20 21:29 ` DJ Delorie
1 sibling, 0 replies; 28+ messages in thread
From: DJ Delorie @ 2022-01-20 21:29 UTC (permalink / raw)
To: Alexandra Hájková; +Cc: libc-alpha, mark, ahajkova
I think this addresses all my issues. If anything else pops up we can
fix it once we have a known problematic environment.
Reviewed-by: DJ Delorie <dj@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread