Add valgrind smoke test

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

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á Dec. 20, 2021, 11:37 a.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       | 44 +++++++++++++++++++++++++++++++++++++
 3 files changed, 97 insertions(+)
 create mode 100644 elf/tst-valgrind-smoke.sh
 create mode 100644 elf/valgrind-test.c
  

Comments

Mark Wielaard Jan. 10, 2022, 12:13 p.m. UTC | #1
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
  
Adhemerval Zanella Netto Jan. 10, 2022, 12:38 p.m. UTC | #2
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;
> +}
  

Patch

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