Skip to content

Commit bbfa506

Browse files
authored
[lld-macho] Fix bug in makeSyntheticInputSection when -dead_strip flag is specified (#86878)
Previously, `makeSyntheticInputSection` would create a new `ConcatInputSection` without setting `live` explicitly for it. Without `-dead_strip` this would be OK since `live` would default to `true`. However, with `-dead_strip`, `live` would default to false, and it would remain set to `false`. This hasn't resulted in any issues so far since no code paths that exposed this issue were present. However a recent change - ObjC relative method lists (#86231) exposes this issue by creating relocations to the `SyntheticInputSection`. When these relocations are attempted to be written, this ends up with a crash(assert), since the `SyntheticInputSection` they refer to is marked as dead (`live` = `false`). With this change, we set the correct behavior - `live` will always be `true`. We add a test case that before this change would trigger an assert in the linker.
1 parent 0fda758 commit bbfa506

File tree

7 files changed

+12
-10
lines changed

7 files changed

+12
-10
lines changed

lld/MachO/ConcatOutputSection.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -323,11 +323,7 @@ void TextOutputSection::finalize() {
323323
thunkInfo.isec =
324324
makeSyntheticInputSection(isec->getSegName(), isec->getName());
325325
thunkInfo.isec->parent = this;
326-
327-
// This code runs after dead code removal. Need to set the `live` bit
328-
// on the thunk isec so that asserts that check that only live sections
329-
// get written are happy.
330-
thunkInfo.isec->live = true;
326+
assert(thunkInfo.isec->live);
331327

332328
StringRef thunkName = saver().save(funcSym->getName() + ".thunk." +
333329
std::to_string(thunkInfo.sequence++));

lld/MachO/InputSection.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,9 @@ ConcatInputSection *macho::makeSyntheticInputSection(StringRef segName,
281281
Section &section =
282282
*make<Section>(/*file=*/nullptr, segName, sectName, flags, /*addr=*/0);
283283
auto isec = make<ConcatInputSection>(section, data, align);
284+
// Since this is an explicitly created 'fake' input section,
285+
// it should not be dead stripped.
286+
isec->live = true;
284287
section.subsections.push_back({0, isec});
285288
return isec;
286289
}

lld/MachO/InputSection.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ class ConcatInputSection final : public InputSection {
149149
};
150150

151151
// Initialize a fake InputSection that does not belong to any InputFile.
152+
// The created ConcatInputSection will always have 'live=true'
152153
ConcatInputSection *makeSyntheticInputSection(StringRef segName,
153154
StringRef sectName,
154155
uint32_t flags = 0,

lld/MachO/SymbolTable.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ static void handleSectionBoundarySymbol(const Undefined &sym, StringRef segSect,
377377
// live. Marking the isec live ensures an OutputSection is created that the
378378
// start/end symbol can refer to.
379379
assert(sym.isLive());
380-
isec->live = true;
380+
assert(isec->live);
381381

382382
// This runs after gatherInputSections(), so need to explicitly set parent
383383
// and add to inputSections.

lld/MachO/SyntheticSections.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -850,7 +850,7 @@ ConcatInputSection *ObjCSelRefsHelper::makeSelRef(StringRef methname) {
850850
S_LITERAL_POINTERS | S_ATTR_NO_DEAD_STRIP,
851851
ArrayRef<uint8_t>{selrefData, wordSize},
852852
/*align=*/wordSize);
853-
objcSelref->live = true;
853+
assert(objcSelref->live);
854854
objcSelref->relocs.push_back({/*type=*/target->unsignedRelocType,
855855
/*pcrel=*/false, /*length=*/3,
856856
/*offset=*/0,

lld/MachO/Writer.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1375,9 +1375,7 @@ void macho::createSyntheticSections() {
13751375
segment_names::data, section_names::data, S_REGULAR,
13761376
ArrayRef<uint8_t>{arr, target->wordSize},
13771377
/*align=*/target->wordSize);
1378-
// References from dyld are not visible to us, so ensure this section is
1379-
// always treated as live.
1380-
in.imageLoaderCache->live = true;
1378+
assert(in.imageLoaderCache->live);
13811379
}
13821380

13831381
OutputSection *macho::firstTLVDataSection = nullptr;

lld/test/MachO/objc-relative-method-lists-simple.s

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@
88
# RUN: %no-lsystem-lld a64_rel_dylib.o -o a64_rel_dylib.dylib -map a64_rel_dylib.map -dylib -arch arm64 -objc_relative_method_lists
99
# RUN: llvm-objdump --macho --objc-meta-data a64_rel_dylib.dylib | FileCheck %s --check-prefix=CHK_REL
1010

11+
## Test arm64 + relative method lists + dead-strip
12+
# RUN: %no-lsystem-lld a64_rel_dylib.o -o a64_rel_dylib.dylib -map a64_rel_dylib.map -dylib -arch arm64 -objc_relative_method_lists -dead_strip
13+
# RUN: llvm-objdump --macho --objc-meta-data a64_rel_dylib.dylib | FileCheck %s --check-prefix=CHK_REL
14+
1115
## Test arm64 + traditional method lists (no relative offsets)
1216
# RUN: %no-lsystem-lld a64_rel_dylib.o -o a64_rel_dylib.dylib -map a64_rel_dylib.map -dylib -arch arm64 -no_objc_relative_method_lists
1317
# RUN: llvm-objdump --macho --objc-meta-data a64_rel_dylib.dylib | FileCheck %s --check-prefix=CHK_NO_REL

0 commit comments

Comments
 (0)