ELF: Store EhInputSection relocations to simplify code. NFC #161041
Merged
+99
−100
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Created using spr 1.3.5-bogner
|
@llvm/pr-subscribers-lld @llvm/pr-subscribers-lld-elf Author: Fangrui Song (MaskRay) ChangesStore relocations directly as Next: Refactor RelocationScanner to utilize EhInputSection::rels Full diff: https://github.com/llvm/llvm-project/pull/161041.diff 5 Files Affected:
diff --git a/lld/ELF/InputSection.cpp b/lld/ELF/InputSection.cpp
index 98267d1e081db..f933a36aa4768 100644
--- a/lld/ELF/InputSection.cpp
+++ b/lld/ELF/InputSection.cpp
@@ -1358,20 +1358,21 @@ SyntheticSection *EhInputSection::getParent() const {
// .eh_frame is a sequence of CIE or FDE records.
// This function splits an input section into records and returns them.
template <class ELFT> void EhInputSection::split() {
- const RelsOrRelas<ELFT> rels = relsOrRelas<ELFT>(/*supportsCrel=*/false);
+ const RelsOrRelas<ELFT> elfRels = relsOrRelas<ELFT>();
+ if (elfRels.areRelocsCrel())
+ preprocessRelocs<ELFT>(elfRels.crels);
+ else if (elfRels.areRelocsRel())
+ preprocessRelocs<ELFT>(elfRels.rels);
+ else
+ preprocessRelocs<ELFT>(elfRels.relas);
+ auto cmp = [](const Relocation &a, const Relocation &b) {
+ return a.offset < b.offset;
+ };
+ if (!llvm::is_sorted(rels, cmp))
+ llvm::stable_sort(rels, cmp);
+
// getReloc expects the relocations to be sorted by r_offset. See the comment
// in scanRelocs.
- if (rels.areRelocsRel()) {
- SmallVector<typename ELFT::Rel, 0> storage;
- split<ELFT>(sortRels(rels.rels, storage));
- } else {
- SmallVector<typename ELFT::Rela, 0> storage;
- split<ELFT>(sortRels(rels.relas, storage));
- }
-}
-
-template <class ELFT, class RelTy>
-void EhInputSection::split(ArrayRef<RelTy> rels) {
ArrayRef<uint8_t> d = content();
const char *msg = nullptr;
unsigned relI = 0;
@@ -1397,10 +1398,10 @@ void EhInputSection::split(ArrayRef<RelTy> rels) {
// Find the first relocation that points to [off,off+size). Relocations
// have been sorted by r_offset.
const uint64_t off = d.data() - content().data();
- while (relI != rels.size() && rels[relI].r_offset < off)
+ while (relI != rels.size() && rels[relI].offset < off)
++relI;
unsigned firstRel = -1;
- if (relI != rels.size() && rels[relI].r_offset < off + size)
+ if (relI != rels.size() && rels[relI].offset < off + size)
firstRel = relI;
(id == 0 ? cies : fdes).emplace_back(off, this, size, firstRel);
d = d.slice(size);
@@ -1410,6 +1411,23 @@ void EhInputSection::split(ArrayRef<RelTy> rels) {
<< getObjMsg(d.data() - content().data());
}
+template <class ELFT, class RelTy>
+void EhInputSection::preprocessRelocs(Relocs<RelTy> elfRels) {
+ Ctx &ctx = file->ctx;
+ rels.reserve(elfRels.size());
+ for (auto rel : elfRels) {
+ uint64_t offset = rel.r_offset;
+ Symbol &sym = file->getSymbol(rel.getSymbol(ctx.arg.isMips64EL));
+ RelType type = rel.getType(ctx.arg.isMips64EL);
+ RelExpr expr = ctx.target->getRelExpr(type, sym, content().data() + offset);
+ int64_t addend =
+ RelTy::HasAddend
+ ? getAddend<ELFT>(rel)
+ : ctx.target->getImplicitAddend(content().data() + offset, type);
+ rels.push_back({expr, type, offset, addend, &sym});
+ }
+}
+
// Return the offset in an output section for a given input offset.
uint64_t EhInputSection::getParentOffset(uint64_t offset) const {
auto it = partition_point(
diff --git a/lld/ELF/InputSection.h b/lld/ELF/InputSection.h
index 8462f03bdb77e..dc29fedbc5c53 100644
--- a/lld/ELF/InputSection.h
+++ b/lld/ELF/InputSection.h
@@ -394,7 +394,7 @@ class EhInputSection : public InputSectionBase {
StringRef name);
static bool classof(const SectionBase *s) { return s->kind() == EHFrame; }
template <class ELFT> void split();
- template <class ELFT, class RelTy> void split(ArrayRef<RelTy> rels);
+ template <class ELFT, class RelTy> void preprocessRelocs(Relocs<RelTy> rels);
// Splittable sections are handled as a sequence of data
// rather than a single large blob of data.
@@ -402,6 +402,10 @@ class EhInputSection : public InputSectionBase {
SyntheticSection *getParent() const;
uint64_t getParentOffset(uint64_t offset) const;
+
+ // Preprocessed relocations in uniform format to avoid REL/RELA/CREL
+ // relocation format handling throughout the codebase.
+ SmallVector<Relocation, 0> rels;
};
// This is a section that is added directly to an output section
diff --git a/lld/ELF/MarkLive.cpp b/lld/ELF/MarkLive.cpp
index 83ae9fb7689e0..975a32e52511e 100644
--- a/lld/ELF/MarkLive.cpp
+++ b/lld/ELF/MarkLive.cpp
@@ -68,10 +68,9 @@ template <class ELFT, bool TrackWhyLive> class MarkLive {
void mark();
template <class RelTy>
- void resolveReloc(InputSectionBase &sec, RelTy &rel, bool fromFDE);
+ void resolveReloc(InputSectionBase &sec, const RelTy &rel, bool fromFDE);
- template <class RelTy>
- void scanEhFrameSection(EhInputSection &eh, ArrayRef<RelTy> rels);
+ void scanEhFrameSection(EhInputSection &eh);
Ctx &ctx;
// The index of the partition that we are currently processing.
@@ -115,23 +114,38 @@ static uint64_t getAddend(Ctx &, InputSectionBase &sec,
template <class ELFT, bool TrackWhyLive>
template <class RelTy>
void MarkLive<ELFT, TrackWhyLive>::resolveReloc(InputSectionBase &sec,
- RelTy &rel, bool fromFDE) {
+ const RelTy &rel,
+ bool fromFDE) {
// If a symbol is referenced in a live section, it is used.
- Symbol &sym = sec.file->getRelocTargetSym(rel);
- sym.used = true;
+ Symbol *sym;
+ if constexpr (std::is_same_v<RelTy, Relocation>) {
+ assert(isa<EhInputSection>(sec));
+ sym = rel.sym;
+ } else {
+ sym = &sec.file->getRelocTargetSym(rel);
+ }
+ sym->used = true;
LiveReason reason;
- if (TrackWhyLive)
- reason = {SecOffset(&sec, rel.r_offset), "referenced by"};
+ if (TrackWhyLive) {
+ if constexpr (std::is_same_v<RelTy, Relocation>)
+ reason = {SecOffset(&sec, rel.offset), "referenced by"};
+ else
+ reason = {SecOffset(&sec, rel.r_offset), "referenced by"};
+ }
- if (auto *d = dyn_cast<Defined>(&sym)) {
+ if (auto *d = dyn_cast<Defined>(sym)) {
auto *relSec = dyn_cast_or_null<InputSectionBase>(d->section);
if (!relSec)
return;
uint64_t offset = d->value;
- if (d->isSection())
- offset += getAddend<ELFT>(ctx, sec, rel);
+ if (d->isSection()) {
+ if constexpr (std::is_same_v<RelTy, Relocation>)
+ offset += rel.addend;
+ else
+ offset += getAddend<ELFT>(ctx, sec, rel);
+ }
// fromFDE being true means this is referenced by a FDE in a .eh_frame
// piece. The relocation points to the described function or to a LSDA. We
@@ -141,8 +155,9 @@ void MarkLive<ELFT, TrackWhyLive>::resolveReloc(InputSectionBase &sec,
// associated text section is live, the LSDA will be retained due to section
// group/SHF_LINK_ORDER rules (b) if the associated text section should be
// discarded, marking the LSDA will unnecessarily retain the text section.
- if (!(fromFDE && ((relSec->flags & (SHF_EXECINSTR | SHF_LINK_ORDER)) ||
- relSec->nextInSectionGroup))) {
+ if (!(std::is_same_v<RelTy, Relocation> && fromFDE &&
+ ((relSec->flags & (SHF_EXECINSTR | SHF_LINK_ORDER)) ||
+ relSec->nextInSectionGroup))) {
Symbol *canonicalSym = d;
if (TrackWhyLive && d->isSection()) {
// This is expensive, so ideally this would be deferred until it's known
@@ -159,15 +174,15 @@ void MarkLive<ELFT, TrackWhyLive>::resolveReloc(InputSectionBase &sec,
return;
}
- if (auto *ss = dyn_cast<SharedSymbol>(&sym)) {
+ if (auto *ss = dyn_cast<SharedSymbol>(sym)) {
if (!ss->isWeak()) {
cast<SharedFile>(ss->file)->isNeeded = true;
if (TrackWhyLive)
- whyLive.try_emplace(&sym, reason);
+ whyLive.try_emplace(sym, reason);
}
}
- for (InputSectionBase *sec : cNamedSections.lookup(sym.getName()))
+ for (InputSectionBase *sec : cNamedSections.lookup(sym->getName()))
enqueue(sec, /*offset=*/0, /*sym=*/nullptr, reason);
}
@@ -186,9 +201,8 @@ void MarkLive<ELFT, TrackWhyLive>::resolveReloc(InputSectionBase &sec,
// the gc pass. With that we would be able to also gc some sections holding
// LSDAs and personality functions if we found that they were unused.
template <class ELFT, bool TrackWhyLive>
-template <class RelTy>
-void MarkLive<ELFT, TrackWhyLive>::scanEhFrameSection(EhInputSection &eh,
- ArrayRef<RelTy> rels) {
+void MarkLive<ELFT, TrackWhyLive>::scanEhFrameSection(EhInputSection &eh) {
+ ArrayRef<Relocation> rels = eh.rels;
for (const EhSectionPiece &cie : eh.cies)
if (cie.firstRelocation != unsigned(-1))
resolveReloc(eh, rels[cie.firstRelocation], false);
@@ -198,7 +212,7 @@ void MarkLive<ELFT, TrackWhyLive>::scanEhFrameSection(EhInputSection &eh,
continue;
uint64_t pieceEnd = fde.inputOff + fde.size;
for (size_t j = firstRelI, end2 = rels.size();
- j < end2 && rels[j].r_offset < pieceEnd; ++j)
+ j < end2 && rels[j].offset < pieceEnd; ++j)
resolveReloc(eh, rels[j], true);
}
}
@@ -360,14 +374,8 @@ void MarkLive<ELFT, TrackWhyLive>::run() {
// that point to .eh_frames. Otherwise, the garbage collector would drop
// all of them. We also want to preserve personality routines and LSDA
// referenced by .eh_frame sections, so we scan them for that here.
- for (EhInputSection *eh : ctx.ehInputSections) {
- const RelsOrRelas<ELFT> rels =
- eh->template relsOrRelas<ELFT>(/*supportsCrel=*/false);
- if (rels.areRelocsRel())
- scanEhFrameSection(*eh, rels.rels);
- else if (rels.relas.size())
- scanEhFrameSection(*eh, rels.relas);
- }
+ for (EhInputSection *eh : ctx.ehInputSections)
+ scanEhFrameSection(*eh);
for (InputSectionBase *sec : ctx.inputSections) {
if (sec->flags & SHF_GNU_RETAIN) {
enqueue(sec, /*offset=*/0, /*sym=*/nullptr, {std::nullopt, "retained"});
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index 457a794a8c3a8..bbf4b29a9fda5 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -403,12 +403,12 @@ EhFrameSection::EhFrameSection(Ctx &ctx)
// Search for an existing CIE record or create a new one.
// CIE records from input object files are uniquified by their contents
// and where their relocations point to.
-template <class RelTy>
-CieRecord *EhFrameSection::addCie(EhSectionPiece &cie, ArrayRef<RelTy> rels) {
+CieRecord *EhFrameSection::addCie(EhSectionPiece &cie,
+ ArrayRef<Relocation> rels) {
Symbol *personality = nullptr;
unsigned firstRelI = cie.firstRelocation;
if (firstRelI != (unsigned)-1)
- personality = &cie.sec->file->getRelocTargetSym(rels[firstRelI]);
+ personality = rels[firstRelI].sym;
// Search for an existing CIE by CIE contents/relocation target pair.
CieRecord *&rec = cieMap[{cie.data(), personality}];
@@ -424,25 +424,20 @@ CieRecord *EhFrameSection::addCie(EhSectionPiece &cie, ArrayRef<RelTy> rels) {
// There is one FDE per function. Returns a non-null pointer to the function
// symbol if the given FDE points to a live function.
-template <class RelTy>
-Defined *EhFrameSection::isFdeLive(EhSectionPiece &fde, ArrayRef<RelTy> rels) {
- auto *sec = cast<EhInputSection>(fde.sec);
- unsigned firstRelI = fde.firstRelocation;
-
+Defined *EhFrameSection::isFdeLive(EhSectionPiece &fde,
+ ArrayRef<Relocation> rels) {
// An FDE should point to some function because FDEs are to describe
// functions. That's however not always the case due to an issue of
// ld.gold with -r. ld.gold may discard only functions and leave their
// corresponding FDEs, which results in creating bad .eh_frame sections.
// To deal with that, we ignore such FDEs.
+ unsigned firstRelI = fde.firstRelocation;
if (firstRelI == (unsigned)-1)
return nullptr;
- const RelTy &rel = rels[firstRelI];
- Symbol &b = sec->file->getRelocTargetSym(rel);
-
// FDEs for garbage-collected or merged-by-ICF sections, or sections in
// another partition, are dead.
- if (auto *d = dyn_cast<Defined>(&b))
+ if (auto *d = dyn_cast<Defined>(rels[firstRelI].sym))
if (!d->folded && d->section && d->section->partition == partition)
return d;
return nullptr;
@@ -452,13 +447,13 @@ Defined *EhFrameSection::isFdeLive(EhSectionPiece &fde, ArrayRef<RelTy> rels) {
// is one CIE record per input object file which is followed by
// a list of FDEs. This function searches an existing CIE or create a new
// one and associates FDEs to the CIE.
-template <class ELFT, class RelTy>
-void EhFrameSection::addRecords(EhInputSection *sec, ArrayRef<RelTy> rels) {
+template <endianness e> void EhFrameSection::addRecords(EhInputSection *sec) {
+ auto rels = sec->rels;
offsetToCie.clear();
for (EhSectionPiece &cie : sec->cies)
- offsetToCie[cie.inputOff] = addCie<RelTy>(cie, rels);
+ offsetToCie[cie.inputOff] = addCie(cie, rels);
for (EhSectionPiece &fde : sec->fdes) {
- uint32_t id = endian::read32<ELFT::Endianness>(fde.data().data() + 4);
+ uint32_t id = endian::read32<e>(fde.data().data() + 4);
CieRecord *rec = offsetToCie[fde.inputOff + 4 - id];
if (!rec)
Fatal(ctx) << sec << ": invalid CIE reference";
@@ -470,23 +465,11 @@ void EhFrameSection::addRecords(EhInputSection *sec, ArrayRef<RelTy> rels) {
}
}
-template <class ELFT>
-void EhFrameSection::addSectionAux(EhInputSection *sec) {
- if (!sec->isLive())
- return;
- const RelsOrRelas<ELFT> rels =
- sec->template relsOrRelas<ELFT>(/*supportsCrel=*/false);
- if (rels.areRelocsRel())
- addRecords<ELFT>(sec, rels.rels);
- else
- addRecords<ELFT>(sec, rels.relas);
-}
-
// Used by ICF<ELFT>::handleLSDA(). This function is very similar to
// EhFrameSection::addRecords().
-template <class ELFT, class RelTy>
+template <class ELFT>
void EhFrameSection::iterateFDEWithLSDAAux(
- EhInputSection &sec, ArrayRef<RelTy> rels, DenseSet<size_t> &ciesWithLSDA,
+ EhInputSection &sec, DenseSet<size_t> &ciesWithLSDA,
llvm::function_ref<void(InputSection &)> fn) {
for (EhSectionPiece &cie : sec.cies)
if (hasLSDA(cie))
@@ -497,7 +480,7 @@ void EhFrameSection::iterateFDEWithLSDAAux(
continue;
// The CIE has a LSDA argument. Call fn with d's section.
- if (Defined *d = isFdeLive(fde, rels))
+ if (Defined *d = isFdeLive(fde, sec.rels))
if (auto *s = dyn_cast_or_null<InputSection>(d->section))
fn(*s);
}
@@ -509,12 +492,7 @@ void EhFrameSection::iterateFDEWithLSDA(
DenseSet<size_t> ciesWithLSDA;
for (EhInputSection *sec : sections) {
ciesWithLSDA.clear();
- const RelsOrRelas<ELFT> rels =
- sec->template relsOrRelas<ELFT>(/*supportsCrel=*/false);
- if (rels.areRelocsRel())
- iterateFDEWithLSDAAux<ELFT>(*sec, rels.rels, ciesWithLSDA, fn);
- else
- iterateFDEWithLSDAAux<ELFT>(*sec, rels.relas, ciesWithLSDA, fn);
+ iterateFDEWithLSDAAux<ELFT>(*sec, ciesWithLSDA, fn);
}
}
@@ -531,20 +509,16 @@ void EhFrameSection::finalizeContents() {
case ELFNoneKind:
llvm_unreachable("invalid ekind");
case ELF32LEKind:
- for (EhInputSection *sec : sections)
- addSectionAux<ELF32LE>(sec);
- break;
- case ELF32BEKind:
- for (EhInputSection *sec : sections)
- addSectionAux<ELF32BE>(sec);
- break;
case ELF64LEKind:
for (EhInputSection *sec : sections)
- addSectionAux<ELF64LE>(sec);
+ if (sec->isLive())
+ addRecords<endianness::little>(sec);
break;
+ case ELF32BEKind:
case ELF64BEKind:
for (EhInputSection *sec : sections)
- addSectionAux<ELF64BE>(sec);
+ if (sec->isLive())
+ addRecords<endianness::big>(sec);
break;
}
diff --git a/lld/ELF/SyntheticSections.h b/lld/ELF/SyntheticSections.h
index 55a10716c054b..ac3ec63f0a7a5 100644
--- a/lld/ELF/SyntheticSections.h
+++ b/lld/ELF/SyntheticSections.h
@@ -80,19 +80,14 @@ class EhFrameSection final : public SyntheticSection {
uint64_t size = 0;
- template <class ELFT, class RelTy>
- void addRecords(EhInputSection *s, llvm::ArrayRef<RelTy> rels);
- template <class ELFT> void addSectionAux(EhInputSection *s);
- template <class ELFT, class RelTy>
- void iterateFDEWithLSDAAux(EhInputSection &sec, ArrayRef<RelTy> rels,
+ template <llvm::endianness E> void addRecords(EhInputSection *s);
+ template <class ELFT>
+ void iterateFDEWithLSDAAux(EhInputSection &sec,
llvm::DenseSet<size_t> &ciesWithLSDA,
llvm::function_ref<void(InputSection &)> fn);
- template <class RelTy>
- CieRecord *addCie(EhSectionPiece &piece, ArrayRef<RelTy> rels);
-
- template <class RelTy>
- Defined *isFdeLive(EhSectionPiece &piece, ArrayRef<RelTy> rels);
+ CieRecord *addCie(EhSectionPiece &piece, ArrayRef<Relocation> rels);
+ Defined *isFdeLive(EhSectionPiece &piece, ArrayRef<Relocation> rels);
uint64_t getFdePc(uint8_t *buf, size_t off, uint8_t enc) const;
|
smithp35
approved these changes
Sep 29, 2025
There was a problem hiding this comment.
LGTM. It is a pity that this makes some of MarkLive more complicated, but all I can think of an obvious way of mitigating that. Possibly a helper routine to extract the field, but I don't think that would be worth doing unless it could be used in other places too.
A couple of small suggestions.
lld/ELF/MarkLive.cpp
Outdated
| // discarded, marking the LSDA will unnecessarily retain the text section. | ||
| if (!(fromFDE && ((relSec->flags & (SHF_EXECINSTR | SHF_LINK_ORDER)) || | ||
| relSec->nextInSectionGroup))) { | ||
| if (!(std::is_same_v<RelTy, Relocation> && fromFDE && |
There was a problem hiding this comment.
Could we put fromFDE before the std::is_same_v as it matches the comment above?
| }; | ||
| if (!llvm::is_sorted(rels, cmp)) | ||
| llvm::stable_sort(rels, cmp); | ||
|
|
There was a problem hiding this comment.
The comment below looks to be stale. Should it be moved above the stable_sort, possibly updated to mention offset rather than r_offset.
Created using spr 1.3.5-bogner
MaskRay
deleted the
users/MaskRay/spr/elf-store-ehinputsection-relocations-to-simplify-code-nfc
branch
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Sep 29, 2025
Store relocations directly as `SmallVector<Relocation, 0>` within EhInputSection to avoid processing different relocation formats (REL/RELA/CREL) throughout the codebase. Next: Refactor RelocationScanner to utilize EhInputSection::rels Pull Request: llvm/llvm-project#161041
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Oct 3, 2025
Store relocations directly as `SmallVector<Relocation, 0>` within EhInputSection to avoid processing different relocation formats (REL/RELA/CREL) throughout the codebase. Next: Refactor RelocationScanner to utilize EhInputSection::rels Pull Request: llvm#161041
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Store relocations directly as
SmallVector<Relocation, 0>withinEhInputSection to avoid processing different relocation formats
(REL/RELA/CREL) throughout the codebase.
Next: Refactor RelocationScanner to utilize EhInputSection::rels