[Bolt] Fix issue 110407 (Support CREL) #119150
Conversation
This commit fixes issue llvm#110407 in Bolt subproject: BOLT programs not support CREL-type relocations.
0xfk0
requested review from
aaupov,
ayermolo,
dcci,
maksfb and
rafaelauler
as code owners
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-bolt Author: 2:5030/1559 (0xfk0) ChangesThis commit fixes issue #110407 in Bolt subproject: BOLT programs not support CREL-type relocations. Full diff: https://github.com/llvm/llvm-project/pull/119150.diff 1 Files Affected:
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 76e1f0156f828d..f1ef97888a26ce 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -1938,8 +1938,11 @@ Error RewriteInstance::readSpecialSections() {
"Use -update-debug-sections to keep it.\n";
}
- HasTextRelocations = (bool)BC->getUniqueSectionByName(
- ".rela" + std::string(BC->getMainCodeSectionName()));
+ const char *code_sec = BC->getMainCodeSectionName();
+ HasTextRelocations = !!BC->getUniqueSectionByName(std::string{".rela"} + code_sec);
+ if (!HasTextRelocations)
+ HasTextRelocations = !!BC->getUniqueSectionByName(std::string{".crel"} + code_sec);
+
HasSymbolTable = (bool)BC->getUniqueSectionByName(".symtab");
EHFrameSection = BC->getUniqueSectionByName(".eh_frame");
@@ -2127,6 +2130,11 @@ int64_t getRelocationAddend(const ELFObjectFile<ELFT> *Obj,
llvm_unreachable("unexpected relocation section type");
case ELF::SHT_REL:
break;
+ case ELF::SHT_CREL: {
+ auto ERela = Obj->getCrel(Rel);
+ Addend = ERela.r_addend;
+ break;
+ }
case ELF::SHT_RELA: {
const Elf_Rela *RelA = Obj->getRela(Rel);
Addend = RelA->r_addend;
|
|
Please add a test. Also for background crel is a new compact relocation that was proposed: https://discourse.llvm.org/t/rfc-crel-a-compact-relocation-format-for-elf/77600 |
|
I think not only reading, but patching them is needed after binary processing |
If I understand correctly, there is no need to update relocations metadata unless a second round of instrumentation/optimization is needed for the instrumented/optimized binary? I can use this PR to optimize a clang built with CREL. |
It seems I misunderstood and CREL is currently used for static relocations only. Then yes, the patch looks good to me, except for test absence :) |
You can test this locally with the following command:git-clang-format --diff eff0d8103c5e0db938550dd6e18230ea8ed9ff4b e76d7498b2d85eb555a09e6b08b1b48c865df1ce --extensions cpp -- bolt/lib/Rewrite/RewriteInstance.cppView the diff from clang-format here.diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index f1ef97888a..10dc1c66fc 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -1939,9 +1939,11 @@ Error RewriteInstance::readSpecialSections() {
}
const char *code_sec = BC->getMainCodeSectionName();
- HasTextRelocations = !!BC->getUniqueSectionByName(std::string{".rela"} + code_sec);
+ HasTextRelocations =
+ !!BC->getUniqueSectionByName(std::string{".rela"} + code_sec);
if (!HasTextRelocations)
- HasTextRelocations = !!BC->getUniqueSectionByName(std::string{".crel"} + code_sec);
+ HasTextRelocations =
+ !!BC->getUniqueSectionByName(std::string{".crel"} + code_sec);
HasSymbolTable = (bool)BC->getUniqueSectionByName(".symtab");
EHFrameSection = BC->getUniqueSectionByName(".eh_frame");
@@ -2133,7 +2135,7 @@ int64_t getRelocationAddend(const ELFObjectFile<ELFT> *Obj,
case ELF::SHT_CREL: {
auto ERela = Obj->getCrel(Rel);
Addend = ERela.r_addend;
- break;
+ break;
}
case ELF::SHT_RELA: {
const Elf_Rela *RelA = Obj->getRela(Rel);
|
5 participants
This commit fixes issue #110407 in Bolt subproject: BOLT programs not support CREL-type relocations.