-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[LLD] Extend support for RISCV vendor-specific relocations to include dynamic relocations as well. #169274
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.
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
base: main
Are you sure you want to change the base?
Conversation
… dynamic relocations as well. This potentially requires materializing the vendor namespace symbol at link-time, which introduces some complexity. Additionally we have to ensure that R_RISCV_VENDOR relocations do not interfere with the normal dynamic relocation sorting.
|
This change depends on #169273 |
|
@llvm/pr-subscribers-lld-elf @llvm/pr-subscribers-lld Author: Owen Anderson (resistor) ChangesThis potentially requires materializing the vendor namespace symbol at link-time, which introduces Full diff: https://github.com/llvm/llvm-project/pull/169274.diff 3 Files Affected:
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index d21376fd3ee47..75d50de95394f 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -1644,8 +1644,11 @@ void elf::postScanRelocations(Ctx &ctx) {
}
assert(ctx.symAux.size() == 1);
- for (Symbol *sym : ctx.symtab->getSymbols())
- fn(*sym);
+ // When RISCV vendor-specific relocations are used in the GOT, a new marker
+ // symbol may be introduced during this iteration, so we have to use an
+ // invalidation-safe loop.
+ for (size_t i = 0; i < ctx.symtab->getSymbols().size(); ++i)
+ fn(*ctx.symtab->getSymbols()[i]);
// Local symbols may need the aforementioned non-preemptible ifunc and GOT
// handling. They don't need regular PLT.
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index 19b08152ae081..2e2f0d16c16e6 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -14,6 +14,7 @@
//===----------------------------------------------------------------------===//
#include "SyntheticSections.h"
+#include "Arch/RISCVInternalRelocations.h"
#include "Config.h"
#include "DWARF.h"
#include "EhFrame.h"
@@ -1703,6 +1704,18 @@ void DynamicReloc::finalize(Ctx &ctx, SymbolTableBaseSection *symt) {
isFinal = true; // Catch errors
}
+size_t RelocationBaseSection::getSize() const {
+ size_t size = relocs.size() * entsize;
+ if (ctx.arg.emachine == EM_RISCV) {
+ for (const auto &reloc : relocs) {
+ if (reloc.type.v & INTERNAL_RISCV_VENDOR_MASK) {
+ size += entsize;
+ }
+ }
+ }
+ return size;
+}
+
void RelocationBaseSection::computeRels() {
SymbolTableBaseSection *symTab = getPartition(ctx).dynSymTab.get();
parallelForEach(relocs, [&ctx = ctx, symTab](DynamicReloc &rel) {
@@ -1725,6 +1738,47 @@ void RelocationBaseSection::computeRels() {
return std::tie(a.r_sym, a.r_offset) < std::tie(b.r_sym, b.r_offset);
});
}
+ // Insert R_RISCV_VENDOR relocations very late, so that it doesn't interfere
+ // with relocation sorting above.
+ if (ctx.arg.emachine == EM_RISCV) {
+ SmallVector<DynamicReloc, 0> processedRelocs;
+ processedRelocs.reserve(relocs.size());
+ for (auto reloc : relocs) {
+ auto vendorString = getRISCVVendorString(reloc.type);
+ if (vendorString) {
+ // Symbol *vendorSym = ctx.symtab->find(*vendorString);
+ auto *vendorSym = ctx.symtab->find(*vendorString);
+ if (!vendorSym || !vendorSym->isDefined()) {
+ vendorSym = ctx.symtab->addSymbol(Defined{ctx, nullptr, *vendorString,
+ STB_GLOBAL, STV_HIDDEN,
+ STT_NOTYPE, 0, 0, nullptr});
+ symTab->addSymbol(vendorSym);
+ }
+ vendorSym->isUsedInRegularObj = true;
+ vendorSym->isExported = true;
+ processedRelocs.push_back({llvm::ELF::R_RISCV_VENDOR, reloc.inputSec,
+ reloc.offsetInSec, true, *vendorSym, 0,
+ R_ABS});
+ processedRelocs.back().finalize(ctx, symTab);
+ }
+
+ reloc.type.v &= ~INTERNAL_RISCV_VENDOR_MASK;
+ processedRelocs.push_back(reloc);
+ }
+
+ relocs = std::move(processedRelocs);
+ }
+}
+
+void RelocationBaseSection::maybeAddRISCVendorRelocation(
+ const DynamicReloc &reloc, SmallVector<DynamicReloc, 0> &outRelocs) {
+ auto riscvVendorString = getRISCVVendorString(reloc.type);
+ if (ctx.arg.emachine == llvm::ELF::EM_RISCV && riscvVendorString) {
+ Symbol &vendorSym = *ctx.symtab->addSymbol(Defined{
+ ctx, ctx.internalFile, *riscvVendorString, llvm::ELF::STB_GLOBAL,
+ llvm::ELF::STV_HIDDEN, llvm::ELF::STT_NOTYPE, 0, 0, nullptr});
+ vendorSym.isUsedInRegularObj = true;
+ }
}
template <class ELFT>
diff --git a/lld/ELF/SyntheticSections.h b/lld/ELF/SyntheticSections.h
index 66c866d7e8cde..d5365242b85ce 100644
--- a/lld/ELF/SyntheticSections.h
+++ b/lld/ELF/SyntheticSections.h
@@ -478,6 +478,7 @@ class RelocationBaseSection : public SyntheticSection {
/// This overload can be used if the addends are written directly instead of
/// using relocations on the input section (e.g. MipsGotSection::writeTo()).
template <bool shard = false> void addReloc(const DynamicReloc &reloc) {
+ maybeAddRISCVendorRelocation(reloc, relocs);
relocs.push_back(reloc);
}
/// Add a dynamic relocation against \p sym with an optional addend.
@@ -518,7 +519,7 @@ class RelocationBaseSection : public SyntheticSection {
return !relocs.empty() ||
llvm::any_of(relocsVec, [](auto &v) { return !v.empty(); });
}
- size_t getSize() const override { return relocs.size() * this->entsize; }
+ size_t getSize() const override;
size_t getRelativeRelocCount() const { return numRelativeRelocs; }
void mergeRels();
void partitionRels();
@@ -529,6 +530,8 @@ class RelocationBaseSection : public SyntheticSection {
protected:
void computeRels();
+ void maybeAddRISCVendorRelocation(const DynamicReloc &reloc,
+ SmallVector<DynamicReloc, 0> &outRelocs);
// Used when parallel relocation scanning adds relocations. The elements
// will be moved into relocs by mergeRel().
SmallVector<SmallVector<DynamicReloc, 0>, 0> relocsVec;
@@ -538,6 +541,8 @@ class RelocationBaseSection : public SyntheticSection {
template <>
inline void RelocationBaseSection::addReloc<true>(const DynamicReloc &reloc) {
+ maybeAddRISCVendorRelocation(reloc,
+ relocsVec[llvm::parallel::getThreadIndex()]);
relocsVec[llvm::parallel::getThreadIndex()].push_back(reloc);
}
|
🐧 Linux x64 Test ResultsThe build failed before running any tests. Click on a failure below to see the details. tools/lld/ELF/CMakeFiles/lldELF.dir/SyntheticSections.cpp.oIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
lenary
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems reasonable and maintainable.
I understand there is controversy over whether we should have vendor dynamic relocations, but I don't know how else we support the things that CherIoT needs.
| auto riscvVendorString = getRISCVVendorString(reloc.type); | ||
| if (ctx.arg.emachine == llvm::ELF::EM_RISCV && riscvVendorString) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be clearer to push the riscvVendorString declaration inside the if - so that there's no overhead for other architectures.
This potentially requires materializing the vendor namespace symbol at link-time, which introduces
some complexity. Additionally we have to ensure that R_RISCV_VENDOR relocations do not interfere
with the normal dynamic relocation sorting.