diff mbox series

Revert "x86_64: Suppress false positive valgrind error"

Message ID 87ttk51e69.fsf@oldenburg.str.redhat.com
State New
Headers show
Series Revert "x86_64: Suppress false positive valgrind error" | expand

Commit Message

Florian Weimer April 13, 2024, 3:21 p.m. UTC 
This reverts commit a1735e0aa858f0c8b15e5ee9975bff4279423680.

The test failure is a real valgrind bug that needs to be fixed before
valgrind is usable with a glibc that has been built with
CC="gcc -march=x86-64-v3".  The proposed valgrind patch teaches
valgrind to replace ld.so strcmp with an unoptimized scalar
implementation, thus avoiding any AVX2-related problems.

Valgrind bug: <https://bugs.kde.org/show_bug.cgi?id=485487>

---
 elf/Makefile                           |  3 +--
 elf/tst-valgrind-smoke.sh              |  5 ++---
 sysdeps/x86_64/Makefile                |  7 -------
 sysdeps/x86_64/tst-valgrind-smoke.supp | 17 -----------------
 4 files changed, 3 insertions(+), 29 deletions(-)


base-commit: ae7468a7b0bcf22e9cd5fcae42bb9e4f65de83ee

Comments

H.J. Lu April 13, 2024, 3:27 p.m. UTC | #1 
On Sat, Apr 13, 2024 at 8:21 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> This reverts commit a1735e0aa858f0c8b15e5ee9975bff4279423680.
>
> The test failure is a real valgrind bug that needs to be fixed before
> valgrind is usable with a glibc that has been built with
> CC="gcc -march=x86-64-v3".  The proposed valgrind patch teaches
> valgrind to replace ld.so strcmp with an unoptimized scalar
> implementation, thus avoiding any AVX2-related problems.
>
> Valgrind bug: <https://bugs.kde.org/show_bug.cgi?id=485487>
>
> ---
>  elf/Makefile                           |  3 +--
>  elf/tst-valgrind-smoke.sh              |  5 ++---
>  sysdeps/x86_64/Makefile                |  7 -------
>  sysdeps/x86_64/tst-valgrind-smoke.supp | 17 -----------------
>  4 files changed, 3 insertions(+), 29 deletions(-)
>
> diff --git a/elf/Makefile b/elf/Makefile
> index 6dad11bcfb..fb3c3c4dff 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -618,8 +618,7 @@ 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 \
> -               '$(valgrind-suppressions-tst-valgrind-smoke)' > $@; $(evaluate-test)
> +               '$(run-program-env)' '$(rpath-link)' $(objpfx)valgrind-test > $@; $(evaluate-test)
>
>  ifeq ($(run-built-tests),yes)
>  tests-special += $(objpfx)tst-rtld-does-not-exist.out
> diff --git a/elf/tst-valgrind-smoke.sh b/elf/tst-valgrind-smoke.sh
> index 7d0ceac6b1..e33b2fa1d7 100644
> --- a/elf/tst-valgrind-smoke.sh
> +++ b/elf/tst-valgrind-smoke.sh
> @@ -25,7 +25,6 @@ test_wrapper_env="$3"
>  run_program_env="$4"
>  library_path="$5"
>  test_prog="$6"
> -valgrind_suppressions="$7"
>
>  # Test whether valgrind is available in the test
>  # environment.  If not, skip the test.
> @@ -35,7 +34,7 @@ ${test_wrapper_env} ${run_program_env} \
>  # 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 ${valgrind_suppressions} \
> +   valgrind -q --error-exitcode=1 \
>       ${system_rtld} /bin/echo ${system_rtld}" || exit 77
>
>  # Finally the actual test inside the test environment,
> @@ -43,5 +42,5 @@ ${test_wrapper_env} ${run_program_env} \
>  # the smoke test under valgrind.
>  /bin/sh -c \
>    "${test_wrapper_env} ${run_program_env} \
> -   valgrind -q --error-exitcode=1 ${valgrind_suppressions} \
> +   valgrind -q --error-exitcode=1 \
>       ${rtld} --library-path ${library_path} ${test_prog} ${rtld}"
> diff --git a/sysdeps/x86_64/Makefile b/sysdeps/x86_64/Makefile
> index 39048b12e0..ce949dba27 100644
> --- a/sysdeps/x86_64/Makefile
> +++ b/sysdeps/x86_64/Makefile
> @@ -211,14 +211,7 @@ tst-plt-rewrite2-ENV = GLIBC_TUNABLES=glibc.cpu.plt_rewrite=2
>  $(objpfx)tst-plt-rewrite2: $(objpfx)tst-plt-rewritemod2.so
>  endif
>
> -# Check if ISA level is 3 or above.
> -ifneq (,$(filter $(have-x86-isa-level),$(x86-isa-level-3-or-above)))
> -valgrind-suppressions-tst-valgrind-smoke = \
> -  --suppressions=$(..)sysdeps/x86_64/tst-valgrind-smoke.supp
> -endif
> -
>  test-internal-extras += tst-gnu2-tls2mod1
> -
>  endif # $(subdir) == elf
>
>  ifeq ($(subdir),csu)
> diff --git a/sysdeps/x86_64/tst-valgrind-smoke.supp b/sysdeps/x86_64/tst-valgrind-smoke.supp
> deleted file mode 100644
> index 533c2deaff..0000000000
> --- a/sysdeps/x86_64/tst-valgrind-smoke.supp
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -{
> -   False positive at strcmp-avx2.S:462
> -   Memcheck:Cond
> -   fun:strcmp
> -   fun:_dl_name_match_p
> -   fun:_dl_map_object
> -   fun:map_doit
> -   fun:_dl_catch_exception
> -   fun:_dl_catch_error
> -   fun:do_preload
> -   fun:handle_preload_list
> -   fun:dl_main
> -   fun:_dl_sysdep_start
> -   fun:_dl_start_final
> -   fun:_dl_start
> -   obj:*/elf/ld.so
> -}
>
> base-commit: ae7468a7b0bcf22e9cd5fcae42bb9e4f65de83ee
>

LGTM.

Reviewed-by: H.J. Lu <hjl.tools@gmail.com>

Thanks.
diff mbox series

Patch

diff --git a/elf/Makefile b/elf/Makefile
index 6dad11bcfb..fb3c3c4dff 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -618,8 +618,7 @@  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 \
-		'$(valgrind-suppressions-tst-valgrind-smoke)' > $@; $(evaluate-test)
+		'$(run-program-env)' '$(rpath-link)' $(objpfx)valgrind-test > $@; $(evaluate-test)
 
 ifeq ($(run-built-tests),yes)
 tests-special += $(objpfx)tst-rtld-does-not-exist.out
diff --git a/elf/tst-valgrind-smoke.sh b/elf/tst-valgrind-smoke.sh
index 7d0ceac6b1..e33b2fa1d7 100644
--- a/elf/tst-valgrind-smoke.sh
+++ b/elf/tst-valgrind-smoke.sh
@@ -25,7 +25,6 @@  test_wrapper_env="$3"
 run_program_env="$4"
 library_path="$5"
 test_prog="$6"
-valgrind_suppressions="$7"
 
 # Test whether valgrind is available in the test
 # environment.  If not, skip the test.
@@ -35,7 +34,7 @@  ${test_wrapper_env} ${run_program_env} \
 # 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 ${valgrind_suppressions} \
+   valgrind -q --error-exitcode=1 \
      ${system_rtld} /bin/echo ${system_rtld}" || exit 77
 
 # Finally the actual test inside the test environment,
@@ -43,5 +42,5 @@  ${test_wrapper_env} ${run_program_env} \
 # the smoke test under valgrind.
 /bin/sh -c \
   "${test_wrapper_env} ${run_program_env} \
-   valgrind -q --error-exitcode=1 ${valgrind_suppressions} \
+   valgrind -q --error-exitcode=1 \
      ${rtld} --library-path ${library_path} ${test_prog} ${rtld}"
diff --git a/sysdeps/x86_64/Makefile b/sysdeps/x86_64/Makefile
index 39048b12e0..ce949dba27 100644
--- a/sysdeps/x86_64/Makefile
+++ b/sysdeps/x86_64/Makefile
@@ -211,14 +211,7 @@  tst-plt-rewrite2-ENV = GLIBC_TUNABLES=glibc.cpu.plt_rewrite=2
 $(objpfx)tst-plt-rewrite2: $(objpfx)tst-plt-rewritemod2.so
 endif
 
-# Check if ISA level is 3 or above.
-ifneq (,$(filter $(have-x86-isa-level),$(x86-isa-level-3-or-above)))
-valgrind-suppressions-tst-valgrind-smoke = \
-  --suppressions=$(..)sysdeps/x86_64/tst-valgrind-smoke.supp
-endif
-
 test-internal-extras += tst-gnu2-tls2mod1
-
 endif # $(subdir) == elf
 
 ifeq ($(subdir),csu)
diff --git a/sysdeps/x86_64/tst-valgrind-smoke.supp b/sysdeps/x86_64/tst-valgrind-smoke.supp
deleted file mode 100644
index 533c2deaff..0000000000
--- a/sysdeps/x86_64/tst-valgrind-smoke.supp
+++ /dev/null
@@ -1,17 +0,0 @@ 
-{
-   False positive at strcmp-avx2.S:462
-   Memcheck:Cond
-   fun:strcmp
-   fun:_dl_name_match_p
-   fun:_dl_map_object
-   fun:map_doit
-   fun:_dl_catch_exception
-   fun:_dl_catch_error
-   fun:do_preload
-   fun:handle_preload_list
-   fun:dl_main
-   fun:_dl_sysdep_start
-   fun:_dl_start_final
-   fun:_dl_start
-   obj:*/elf/ld.so
-}