[lld-macho][NFC] Refactor ObjCSelRefsSection out of ObjCStubsSection #83878
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
|
@llvm/pr-subscribers-lld-macho @llvm/pr-subscribers-lld Author: None (alx32) ChangesCurrently ObjCStubsSection is handling both the logic for the "__objc_stubs" section, as well as the logic for the "__objc_selrefs" section. Full diff: https://github.com/llvm/llvm-project/pull/83878.diff 3 Files Affected:
diff --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp
index a5d66bb4ea0801..c8a0b29581eed3 100644
--- a/lld/MachO/SyntheticSections.cpp
+++ b/lld/MachO/SyntheticSections.cpp
@@ -806,26 +806,13 @@ void StubHelperSection::setUp() {
dyldPrivate->used = true;
}
-ObjCStubsSection::ObjCStubsSection()
- : SyntheticSection(segment_names::text, section_names::objcStubs) {
- flags = S_ATTR_SOME_INSTRUCTIONS | S_ATTR_PURE_INSTRUCTIONS;
- align = config->objcStubsMode == ObjCStubsMode::fast
- ? target->objcStubsFastAlignment
- : target->objcStubsSmallAlignment;
-}
-
-bool ObjCStubsSection::isObjCStubSymbol(Symbol *sym) {
- return sym->getName().starts_with(symbolPrefix);
-}
-
-StringRef ObjCStubsSection::getMethname(Symbol *sym) {
- assert(isObjCStubSymbol(sym) && "not an objc stub");
- auto name = sym->getName();
- StringRef methname = name.drop_front(symbolPrefix.size());
- return methname;
+ObjCSelRefsSection::ObjCSelRefsSection()
+ : SyntheticSection(segment_names::data, section_names::objcSelrefs) {
+ flags = S_ATTR_NO_DEAD_STRIP;
+ align = target->wordSize;
}
-void ObjCStubsSection::initialize() {
+void ObjCSelRefsSection::initialize() {
// Do not fold selrefs without ICF.
if (config->icfLevel == ICFLevel::none)
return;
@@ -852,32 +839,63 @@ void ObjCStubsSection::initialize() {
}
}
+ConcatInputSection *ObjCSelRefsSection::makeSelRef(StringRef methName) {
+ auto methnameOffset =
+ in.objcMethnameSection->getStringOffset(methName).outSecOff;
+
+ size_t wordSize = target->wordSize;
+ uint8_t *selrefData = bAlloc().Allocate<uint8_t>(wordSize);
+ write64le(selrefData, methnameOffset);
+ ConcatInputSection *objcSelref =
+ makeSyntheticInputSection(segment_names::data, section_names::objcSelrefs,
+ S_LITERAL_POINTERS | S_ATTR_NO_DEAD_STRIP,
+ ArrayRef<uint8_t>{selrefData, wordSize},
+ /*align=*/wordSize);
+ objcSelref->live = true;
+ objcSelref->relocs.push_back({/*type=*/target->unsignedRelocType,
+ /*pcrel=*/false, /*length=*/3,
+ /*offset=*/0,
+ /*addend=*/static_cast<int64_t>(methnameOffset),
+ /*referent=*/in.objcMethnameSection->isec});
+ objcSelref->parent = ConcatOutputSection::getOrCreateForInput(objcSelref);
+ inputSections.push_back(objcSelref);
+ objcSelref->isFinal = true;
+ methnameToSelref[CachedHashStringRef(methName)] = objcSelref;
+ return objcSelref;
+}
+
+ConcatInputSection *
+ObjCSelRefsSection::getSelRefForMethName(StringRef methName) {
+ auto it = methnameToSelref.find(CachedHashStringRef(methName));
+ if (it == methnameToSelref.end())
+ return nullptr;
+ return it->second;
+}
+
+ObjCStubsSection::ObjCStubsSection()
+ : SyntheticSection(segment_names::text, section_names::objcStubs) {
+ flags = S_ATTR_SOME_INSTRUCTIONS | S_ATTR_PURE_INSTRUCTIONS;
+ align = config->objcStubsMode == ObjCStubsMode::fast
+ ? target->objcStubsFastAlignment
+ : target->objcStubsSmallAlignment;
+}
+
+bool ObjCStubsSection::isObjCStubSymbol(Symbol *sym) {
+ return sym->getName().starts_with(symbolPrefix);
+}
+
+StringRef ObjCStubsSection::getMethname(Symbol *sym) {
+ assert(isObjCStubSymbol(sym) && "not an objc stub");
+ auto name = sym->getName();
+ StringRef methname = name.drop_front(symbolPrefix.size());
+ return methname;
+}
+
void ObjCStubsSection::addEntry(Symbol *sym) {
StringRef methname = getMethname(sym);
// We create a selref entry for each unique methname.
- if (!methnameToSelref.count(CachedHashStringRef(methname))) {
- auto methnameOffset =
- in.objcMethnameSection->getStringOffset(methname).outSecOff;
-
- size_t wordSize = target->wordSize;
- uint8_t *selrefData = bAlloc().Allocate<uint8_t>(wordSize);
- write64le(selrefData, methnameOffset);
- auto *objcSelref = makeSyntheticInputSection(
- segment_names::data, section_names::objcSelrefs,
- S_LITERAL_POINTERS | S_ATTR_NO_DEAD_STRIP,
- ArrayRef<uint8_t>{selrefData, wordSize},
- /*align=*/wordSize);
- objcSelref->live = true;
- objcSelref->relocs.push_back(
- {/*type=*/target->unsignedRelocType,
- /*pcrel=*/false, /*length=*/3,
- /*offset=*/0,
- /*addend=*/static_cast<int64_t>(methnameOffset),
- /*referent=*/in.objcMethnameSection->isec});
- objcSelref->parent = ConcatOutputSection::getOrCreateForInput(objcSelref);
- inputSections.push_back(objcSelref);
- objcSelref->isFinal = true;
- methnameToSelref[CachedHashStringRef(methname)] = objcSelref;
+ if (!in.objcSelRefs->getSelRefForMethName(methname)) {
+ in.objcSelRefs->makeSelRef(methname);
}
auto stubSize = config->objcStubsMode == ObjCStubsMode::fast
@@ -927,9 +945,9 @@ void ObjCStubsSection::writeTo(uint8_t *buf) const {
Defined *sym = symbols[i];
auto methname = getMethname(sym);
- auto j = methnameToSelref.find(CachedHashStringRef(methname));
- assert(j != methnameToSelref.end());
- auto selrefAddr = j->second->getVA(0);
+ InputSection *selRef = in.objcSelRefs->getSelRefForMethName(methname);
+ assert(selRef != nullptr && "no selref for methname");
+ auto selrefAddr = selRef->getVA(0);
target->writeObjCMsgSendStub(buf + stubOffset, sym, in.objcStubs->addr,
stubOffset, selrefAddr, objcMsgSend);
}
diff --git a/lld/MachO/SyntheticSections.h b/lld/MachO/SyntheticSections.h
index 8d54cacc8d756a..794ed332f109f1 100644
--- a/lld/MachO/SyntheticSections.h
+++ b/lld/MachO/SyntheticSections.h
@@ -315,6 +315,25 @@ class StubHelperSection final : public SyntheticSection {
Defined *dyldPrivate = nullptr;
};
+class ObjCSelRefsSection final : public SyntheticSection {
+public:
+ ObjCSelRefsSection();
+ void initialize();
+
+ // This SyntheticSection does not do its own writing, but instead uses the
+ // creates SyntheticInputSection's and inserts them into inputSections
+ uint64_t getSize() const override { return 0; }
+ bool isNeeded() const override { return false; }
+ void writeTo(uint8_t *buf) const override {}
+
+ ConcatInputSection *getSelRefForMethName(StringRef methName);
+ ConcatInputSection *makeSelRef(StringRef methName);
+
+private:
+ llvm::DenseMap<llvm::CachedHashStringRef, ConcatInputSection *>
+ methnameToSelref;
+};
+
// Objective-C stubs are hoisted objc_msgSend calls per selector called in the
// program. Apple Clang produces undefined symbols to each stub, such as
// '_objc_msgSend$foo', which are then synthesized by the linker. The stubs
@@ -324,7 +343,6 @@ class StubHelperSection final : public SyntheticSection {
class ObjCStubsSection final : public SyntheticSection {
public:
ObjCStubsSection();
- void initialize();
void addEntry(Symbol *sym);
uint64_t getSize() const override;
bool isNeeded() const override { return !symbols.empty(); }
@@ -338,7 +356,6 @@ class ObjCStubsSection final : public SyntheticSection {
private:
std::vector<Defined *> symbols;
- llvm::DenseMap<llvm::CachedHashStringRef, InputSection *> methnameToSelref;
Symbol *objcMsgSend = nullptr;
};
@@ -794,6 +811,7 @@ struct InStruct {
LazyPointerSection *lazyPointers = nullptr;
StubsSection *stubs = nullptr;
StubHelperSection *stubHelper = nullptr;
+ ObjCSelRefsSection *objcSelRefs = nullptr;
ObjCStubsSection *objcStubs = nullptr;
UnwindInfoSection *unwindInfo = nullptr;
ObjCImageInfoSection *objCImageInfo = nullptr;
diff --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index 65b598d1d7c422..d66c5091143209 100644
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -720,7 +720,7 @@ static void addNonWeakDefinition(const Defined *defined) {
void Writer::scanSymbols() {
TimeTraceScope timeScope("Scan symbols");
- in.objcStubs->initialize();
+ in.objcSelRefs->initialize();
for (Symbol *sym : symtab->getSymbols()) {
if (auto *defined = dyn_cast<Defined>(sym)) {
if (!defined->isLive())
@@ -1359,6 +1359,7 @@ void macho::createSyntheticSections() {
in.got = make<GotSection>();
in.tlvPointers = make<TlvPointerSection>();
in.stubs = make<StubsSection>();
+ in.objcSelRefs = make<ObjCSelRefsSection>();
in.objcStubs = make<ObjCStubsSection>();
in.unwindInfo = makeUnwindInfoSection();
in.objCImageInfo = make<ObjCImageInfoSection>();
|
kyulee-com
reviewed
Mar 5, 2024
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
alx32
force-pushed
the
05_selref_refactor
branch
from
cdbc35c to
e0c056b
Compare
|
LGTM. Suggest waiting for others' reviews or comments. |
kyulee-com
approved these changes
Mar 6, 2024
kyulee-com
reviewed
Mar 9, 2024
alx32
force-pushed
the
05_selref_refactor
branch
from
e0c056b to
e692db0
Compare
alx32
added a commit
that referenced
this pull request
Mar 25, 2024
…6456) In a previous PR: #83878, the intent was to make no functional changes, just refactor out the code for reuse. However, by creating `ObjCSelRefsSection` as a `SyntheticSection` - this slightly changed the functionality of the application as the `SyntheticSection` constructor registers the `SyntheticSection` as a functional one - with an associated `SyntheticInputSection`. With this change we remove this unintended consequence by making the code not use a `SyntheticSection` as base, but just by having it be a static helper.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Currently ObjCStubsSection is handling both the logic for the "__objc_stubs" section, as well as the logic for the "__objc_selrefs" section.
While this is OK for now, it will be an issue for other features that want to interact with the "__objc_selrefs" section, such as upcoming relative method lists feature - which will also want to create / reference entries in the "__objc_selrefs" section.
In this PR we split the logic relating to handling the "__objc_selrefs" section into a new SyntheticSection (ObjCSelRefsSection). Non-functional change - neither the behavior nor implementation changes, the interface is just made more friendly to not have "__objc_selrefs" so bound to "__objc_stubs".