From bbfa50696e43f337f55f8bacf739683b181debd5 Mon Sep 17 00:00:00 2001 From: alx32 <103613512+alx32@users.noreply.github.com> Date: Wed, 27 Mar 2024 17:27:51 -0700 Subject: [PATCH] [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 (https://github.com/llvm/llvm-project/pull/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. --- lld/MachO/ConcatOutputSection.cpp | 6 +----- lld/MachO/InputSection.cpp | 3 +++ lld/MachO/InputSection.h | 1 + lld/MachO/SymbolTable.cpp | 2 +- lld/MachO/SyntheticSections.cpp | 2 +- lld/MachO/Writer.cpp | 4 +--- lld/test/MachO/objc-relative-method-lists-simple.s | 4 ++++ 7 files changed, 12 insertions(+), 10 deletions(-) diff --git a/lld/MachO/ConcatOutputSection.cpp b/lld/MachO/ConcatOutputSection.cpp index c5c0c8a89e28..279423720be9 100644 --- a/lld/MachO/ConcatOutputSection.cpp +++ b/lld/MachO/ConcatOutputSection.cpp @@ -323,11 +323,7 @@ void TextOutputSection::finalize() { thunkInfo.isec = makeSyntheticInputSection(isec->getSegName(), isec->getName()); thunkInfo.isec->parent = this; - - // This code runs after dead code removal. Need to set the `live` bit - // on the thunk isec so that asserts that check that only live sections - // get written are happy. - thunkInfo.isec->live = true; + assert(thunkInfo.isec->live); StringRef thunkName = saver().save(funcSym->getName() + ".thunk." + std::to_string(thunkInfo.sequence++)); diff --git a/lld/MachO/InputSection.cpp b/lld/MachO/InputSection.cpp index e3d7a400e4ce..5c1e07cd21b1 100644 --- a/lld/MachO/InputSection.cpp +++ b/lld/MachO/InputSection.cpp @@ -281,6 +281,9 @@ ConcatInputSection *macho::makeSyntheticInputSection(StringRef segName, Section §ion = *make
(/*file=*/nullptr, segName, sectName, flags, /*addr=*/0); auto isec = make(section, data, align); + // Since this is an explicitly created 'fake' input section, + // it should not be dead stripped. + isec->live = true; section.subsections.push_back({0, isec}); return isec; } diff --git a/lld/MachO/InputSection.h b/lld/MachO/InputSection.h index a0e6afef92cb..0f389e50425a 100644 --- a/lld/MachO/InputSection.h +++ b/lld/MachO/InputSection.h @@ -149,6 +149,7 @@ public: }; // Initialize a fake InputSection that does not belong to any InputFile. +// The created ConcatInputSection will always have 'live=true' ConcatInputSection *makeSyntheticInputSection(StringRef segName, StringRef sectName, uint32_t flags = 0, diff --git a/lld/MachO/SymbolTable.cpp b/lld/MachO/SymbolTable.cpp index 825242f2cc72..755ff270e2f7 100644 --- a/lld/MachO/SymbolTable.cpp +++ b/lld/MachO/SymbolTable.cpp @@ -377,7 +377,7 @@ static void handleSectionBoundarySymbol(const Undefined &sym, StringRef segSect, // live. Marking the isec live ensures an OutputSection is created that the // start/end symbol can refer to. assert(sym.isLive()); - isec->live = true; + assert(isec->live); // This runs after gatherInputSections(), so need to explicitly set parent // and add to inputSections. diff --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp index 808ea8eac6eb..6f6b66118b7a 100644 --- a/lld/MachO/SyntheticSections.cpp +++ b/lld/MachO/SyntheticSections.cpp @@ -850,7 +850,7 @@ ConcatInputSection *ObjCSelRefsHelper::makeSelRef(StringRef methname) { S_LITERAL_POINTERS | S_ATTR_NO_DEAD_STRIP, ArrayRef{selrefData, wordSize}, /*align=*/wordSize); - objcSelref->live = true; + assert(objcSelref->live); objcSelref->relocs.push_back({/*type=*/target->unsignedRelocType, /*pcrel=*/false, /*length=*/3, /*offset=*/0, diff --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp index fe989de648d7..1c054912551e 100644 --- a/lld/MachO/Writer.cpp +++ b/lld/MachO/Writer.cpp @@ -1375,9 +1375,7 @@ void macho::createSyntheticSections() { segment_names::data, section_names::data, S_REGULAR, ArrayRef{arr, target->wordSize}, /*align=*/target->wordSize); - // References from dyld are not visible to us, so ensure this section is - // always treated as live. - in.imageLoaderCache->live = true; + assert(in.imageLoaderCache->live); } OutputSection *macho::firstTLVDataSection = nullptr; diff --git a/lld/test/MachO/objc-relative-method-lists-simple.s b/lld/test/MachO/objc-relative-method-lists-simple.s index 1ffec3c6241c..5a77085c7d93 100644 --- a/lld/test/MachO/objc-relative-method-lists-simple.s +++ b/lld/test/MachO/objc-relative-method-lists-simple.s @@ -8,6 +8,10 @@ # 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 # RUN: llvm-objdump --macho --objc-meta-data a64_rel_dylib.dylib | FileCheck %s --check-prefix=CHK_REL +## Test arm64 + relative method lists + dead-strip +# 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 +# RUN: llvm-objdump --macho --objc-meta-data a64_rel_dylib.dylib | FileCheck %s --check-prefix=CHK_REL + ## Test arm64 + traditional method lists (no relative offsets) # 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 # RUN: llvm-objdump --macho --objc-meta-data a64_rel_dylib.dylib | FileCheck %s --check-prefix=CHK_NO_REL