diff mbox series

Add valgrind smoke test

Message ID 20220112171539.3205267-1-ahajkova@redhat.com
State Superseded
Headers show
Series Add valgrind smoke test | expand

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Alexandra Hájková Jan. 12, 2022, 5:15 p.m. UTC
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

Comments

Alexandra Hájková Jan. 20, 2022, 7:35 p.m. UTC | #1
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
Joseph Myers Jan. 24, 2022, 6:34 p.m. UTC | #2
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 Myers Jan. 26, 2022, 5:46 p.m. UTC | #3
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.
Mark Wielaard Jan. 26, 2022, 5:59 p.m. UTC | #4
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
Joseph Myers Jan. 26, 2022, 6:40 p.m. UTC | #5
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.
Mark Wielaard Jan. 26, 2022, 7:23 p.m. UTC | #6
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
diff mbox series

Patch

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;
+}