Created attachment 168460 [details] valgrind-amd64-redir-strcmp.patch This was originally observed as a glibc test case failure: [PATCH] x86_64: Suppress false positive valgrind error <https://inbox.sourceware.org/libc-alpha/20240226000326.3844791-1-hjl.tools@gmail.com/> However, the analysis was incorrect, and the test failure actually shows that the newly built glibc is incompatible with valgrind. I think it's necessary to add a redirector for strcmp. We see this in Fedora with valgrind-3.22.0-7.fc40.x86_64, which already has one AVX2 inline strcmp fix applied.
The proposed patch looks reasonable. Did you test it? Is there a (fedora?) glibc build that triggers this issue?
(In reply to Mark Wielaard from comment #1) > The proposed patch looks reasonable. Did you test it? Sort of, the valgrind build passed, and the run-time failures are gone. Earlier versions of the patch with an incorrect implementation failed really spectacularly. But I do not write x86-64 assembly for a living (well, not really), so take it with a grain of salt. > Is there a (fedora?) glibc build that triggers this issue? We had a glibc build bug/RPM spec file issue that resulted in rawhide binaries not using x86-64-v3 string functions in ld.so, and I missed the incorrect glibc upstream testsuite change at the time. We have this as a redundant test in glibc.spec that fails the Fedora build, that's how I noticed. We are getting better at not producing a glibc that is totally incompatible with valgrind. But all this means that there is no Fedora build. 8-) I can prepare a rawhide scratch build for you if this helps, though.
H.J. suggests to use the implementation from <https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/i386/i686/strcmp.S;h=e71d97f2a63a2d3c31e4c1be395c1a1f5e0200ff;hb=HEAD> (adapted for x86-64). I can submit that if there are no licensing concerns.
Based on a further suggestion from H.J., I put the strcmp implementation posted here into glibc and ran its strcmp test suite against it. It passes. For such a simple implementation it should have really good coverage. So I think the posted implementation should be okay.
(In reply to Florian Weimer from comment #4) > Based on a further suggestion from H.J., I put the strcmp implementation > posted here into glibc and ran its strcmp test suite against it. It passes. > For such a simple implementation it should have really good coverage. So I > think the posted implementation should be okay. Yes. The license would be fine. But simpler is actually better for valgrind. The code will run on the virtual machine and will be instrumented by tools. I'll add it to the next fedora rawhide valgrind build so we can test it a bit more.
(In reply to Mark Wielaard from comment #5) > I'll add it to the next fedora rawhide valgrind build so we can test it a > bit more. There is now valgrind-3.22.0-8.fc41 with this and some other (x86-64-v3) fixes to try out. https://bodhi.fedoraproject.org/updates/FEDORA-2024-7579427715
There is now also an fedora eln build: https://koji.fedoraproject.org/koji/buildinfo?buildID=2437715 Since eln is build with -march=x86-64-v3 this might be a good testing environment. Please let me know if you are able to build/test a glibc against one of the above packages with this patch applied.
The ELN glibc scratch build succeeded: https://src.fedoraproject.org/rpms/glibc/pull-request/92#comment-194579 There's a TMT failure, but it's unrelated (potential kernel robust mutex issue). This test does not run under valgrind anyway. (That POWER10 work fixed some glibc.spec issues, which exposed the pre-existing x86-64-v3 bug.)
commit dad279b80c1da4fb2c842cca9783ad0e49339ddc Author: Florian Weimer <fweimer@redhat.com> Date: Sun Apr 14 21:56:03 2024 +0200 add_hardwired_spec for ld-linux-x86-64.so.2 strcmp glibc built with -march=x86-64-v3 does not work due to ld.so strcmp https://bugs.kde.org/show_bug.cgi?id=485487