Skip to content

valgrind: fix building with llvm #329995

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

valgrind: fix building with llvm #329995

wants to merge 1 commit into from

Conversation

RossComputerGuy
Copy link
Member

Description of changes

Discovered in https://github.com/RossComputerGuy/nixpkgs-llvm-ws/runs/27931320260

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@RossComputerGuy
Copy link
Member Author

New issue found with x86_64, works on aarch64 though:

valgrind-x86_64-unknown-linux-gnu-3.23.0> x86_64-unknown-linux-gnu-ld: error: relocation R_X86_64_64 cannot be used against local symbol; recompile with -fPIC
valgrind-x86_64-unknown-linux-gnu-3.23.0> >>> defined in int3-amd64.o
valgrind-x86_64-unknown-linux-gnu-3.23.0> >>> referenced by int3-amd64.c:34 (/build/valgrind-3.23.0/memcheck/tests/amd64-linux/int3-amd64.c:34)
valgrind-x86_64-unknown-linux-gnu-3.23.0> >>>               int3-amd64.o:(main)
valgrind-x86_64-unknown-linux-gnu-3.23.0> clang: error: linker command failed with exit code 1 (use -v to see invocation

@RossComputerGuy RossComputerGuy force-pushed the fix/pkgsllvm/valgrind branch from 5979f31 to 414a45d Compare July 25, 2024 20:53
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Jul 25, 2024
@RossComputerGuy RossComputerGuy force-pushed the fix/pkgsllvm/valgrind branch 2 times, most recently from 64c07c2 to 69e7ad1 Compare July 31, 2024 05:57
@RossComputerGuy
Copy link
Member Author

Now valgrind is failing because of this:

done
aarch64-unknown-linux-gnu-clang -m64 -O2 -g -Wall -Wmissing-prototypes -Wshadow -Wpointer-arith -Wstrict-prototypes -Wmissing-declarations -Wcast-align -Wcast-qual -Wwrite-strings -Wempty-body -Wformat -Wformat-security -Wignored-qualifiers -Wenum-conversion -finline-functions -fno-stack-protector -fno-strict-aliasing -fno-builtin -Wno-cast-align -Wno-self-assign -Wno-tautological-compare  -O -g -fno-omit-frame-pointer -fno-strict-aliasing -fpic -fno-builtin  -O2  -nodefaultlibs -shared -Wl,-z,interpose,-z,initfirst  -m64 -Wl,--whole-archive ../coregrind/libreplacemalloc_toolpreload-arm64-linux.a -Wl,--no-whole-archive  -o vgpreload_memcheck-arm64-linux.so vgpreload_memcheck_arm64_linux_so-mc_replace_strmem.o  
../coregrind/link_tool_exe_linux 0x58000000 aarch64-unknown-linux-gnu-clang     -o memcheck-arm64-linux  -m64 -O2 -g -Wall -Wmissing-prototypes -Wshadow -Wpointer-arith -Wstrict-prototypes -Wmissing-declarations -Wcast-align -Wcast-qual -Wwrite-strings -Wempty-body -Wformat -Wformat-security -Wignored-qualifiers -Wenum-conversion -finline-functions -fno-stack-protector -fno-strict-aliasing -fno-builtin -Wno-cast-align -Wno-self-assign -Wno-tautological-compare  -O2 -static -nodefaultlibs -nostartfiles -u _start  -m64 memcheck_arm64_linux-mc_leakcheck.o memcheck_arm64_linux-mc_malloc_wrappers.o memcheck_arm64_linux-mc_main.o memcheck_arm64_linux-mc_main_asm.o memcheck_arm64_linux-mc_translate.o memcheck_arm64_linux-mc_machine.o memcheck_arm64_linux-mc_errors.o ../coregrind/libcoregrind-arm64-linux.a ../VEX/libvex-arm64-linux.a -lgcc  ../coregrind/libgcc-sup-arm64-linux.a 
aarch64-unknown-linux-gnu-ld: error: undefined symbol: getauxval
>>> referenced by aarch64.c.o:(init_have_lse_atomics) in archive /nix/store/lfypx41m7m76fxmcazy4fvh9d6pqja48-compiler-rt-libc-aarch64-unknown-linux-gnu-18.1.8/lib/libgcc.a
>>> referenced by aarch64.c.o:(__init_cpu_features) in archive /nix/store/lfypx41m7m76fxmcazy4fvh9d6pqja48-compiler-rt-libc-aarch64-unknown-linux-gnu-18.1.8/lib/libgcc.a
>>> referenced by aarch64.c.o:(__init_cpu_features) in archive /nix/store/lfypx41m7m76fxmcazy4fvh9d6pqja48-compiler-rt-libc-aarch64-unknown-linux-gnu-18.1.8/lib/libgcc.a
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[3]: *** [Makefile:1155: memcheck-arm64-linux] Error 1

@RossComputerGuy RossComputerGuy mentioned this pull request Aug 2, 2024
Merged
13 tasks
@RossComputerGuy
Copy link
Member Author

This PR now builds again.

@Ericson2314
Copy link
Member

Sorry about the conflict with the other PR @RossComputerGuy.

do you think we could instead add a withSantizers ? haveLibc configuration flag to compiler-rt? I think that would be cleaner.

@RossComputerGuy
Copy link
Member Author

do you think we could instead add a withSantizers ? haveLibc configuration flag to compiler-rt? I think that would be cleaner.

Yeah, having the ability to turn off sanitizers or having a llvmPackages.compiler-rt-no-libc-no-sanitizers drv might help here.

@Ericson2314
Copy link
Member

@RossComputerGuy Yeah I would prefer to always building the sanitizers in a separate derivation. Ideally that would not be another compiler-rt but just be the santizers, linking compiler-rt/libc/etc. as needed, but that may require some LLVM-side CMake work to do well.

@RossComputerGuy
Copy link
Member Author

Ideally that would not be another compiler-rt but just be the santizers, linking compiler-rt/libc/etc. as needed,

How exactly would that look like? Would we have a $sanitized output which has lib with several libs that are just the sanitizer built versions of what's in the main output?

@Ericson2314
Copy link
Member

@RossComputerGuy See https://github.com/llvm/llvm-project/blob/0edafc461f5f98b2ed5d2d621e1d9de70ccbd4e5/compiler-rt/CMakeLists.txt#L42-L64 We can enable/disable things fairly independently. Perhaps easier than I thought.

(What I would like to advocate they do however is break the stuff into proper separate packages that can be built together with the stuff that libc++ libc++abi and libunwind use, rather than one mega-package that we can split up with enable/disable flags.)

@RossComputerGuy
Copy link
Member Author

@Ericson2314 So based on your previous comments, is there anything blocking this PR or what should we do exactly? I would like to see this PR come in so Mesa can build under LLVM.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 27, 2024
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 12, 2024
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 2, 2025
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/5395

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 18, 2025
@RossComputerGuy RossComputerGuy mentioned this pull request Apr 18, 2025
Merged
13 tasks
@RossComputerGuy RossComputerGuy force-pushed the fix/pkgsllvm/valgrind branch from 52ee4b6 to 748fab9 Compare May 13, 2025 03:49
@RossComputerGuy
Copy link
Member Author

Updating this PR, for whatever reason resetting back to 52ee4b6 doesn't work.

@RossComputerGuy
Copy link
Member Author

Fixed now

@RossComputerGuy RossComputerGuy mentioned this pull request Sep 5, 2025
Open
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@RossComputerGuy @Ericson2314 @nixos-discourse @wegank