Details
- Reviewers
Bdragon28 • espindola jhenderson grimar sfertile - Group Reviewers
Restricted Project - Commits
- rG425d15aeb13e: [ELF][PowerPC] Support R_PPC_COPY and R_PPC64_COPY
rGf1dab29908d2: [ELF][PowerPC] Support R_PPC_COPY and R_PPC64_COPY
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Unit tests: pass. 62087 tests passed, 0 failed and 784 were skipped.
clang-tidy: pass.
clang-format: pass.
Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml
For anyone who is curious about this, copy relocations are especially needed on powerpc32, where they are used to implement global data (such as sharing a pointer between a .so and the main program) in situations where the main program's copy lives in .bss.
Given that FreeBSD libc's API uses this extensively for things such as providing stderr in stdio, this fix is critical on ppc32, as the lack of it prevents anything more complex than /bin/echo from running without crashing due to dereferencing a null pointer.
I am currently running though a powerpc32 buildworld and testing on a G4 so I can see just how much closer that gets us to switching FreeBSD powerpc32 to lld.
On PPC64, the need for copy relocations is much reduced, as generally things go through the TOC in the first place (when using PIC), so it is sufficient to generate an R_PPC64_ADDR64 relocation against the TOC entry, instead of a copy relocation against an offset into .bss.
Unit tests: fail. 62137 tests passed, 5 failed and 811 were skipped.
failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp
clang-tidy: pass.
clang-format: pass.
Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml
Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.
Aside from a minor nit I have no more comments, looks fine to me.
lld/test/ELF/ppc-reloc-copy.s | ||
---|---|---|
10 | It was a bit strange to see t1 again. |
lld/test/ELF/ppc64-reloc-copy.s | ||
---|---|---|
1 ↗ | (On Diff #239785) | Agree. Placed in ppc-reloc-copy.s |
Unit tests: fail. 62153 tests passed, 5 failed and 811 were skipped.
failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp
clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 0 of them are added as review comments below (why?).
clang-format: pass.
Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml
Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.
LGTM, but as I'm a bit rusty on some parts of LLD, and know very little about PPC, I can't be particularly confident about the LLD-aspects of this change, so it's probably worth somebody else confirming.
While I ran into further problems on ppc32, I think it's just because there are further problems on ppc32 (unrelated to this one). At the very least this gets FreeBSD ppc32/lld10 as far as the rescue shell, which is great progress!
Thanks to everyone for review!
Cherry picked the commit to release/10.x as f1dab29908d25a4044abff6ffc120c48b20f034d
It was a bit strange to see t1 again.
I'd suggest to rename t1.o, t1.so, t.o and t to something else.
Perhaps use ppc/ppc64 suffixes somewhere?