[lld] [lld-macho] Fix bug in makeSyntheticInputSection when -dead_strip flag is specified (PR #86878)

via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 27 16:17:13 PDT 2024


https://github.com/alx32 updated https://github.com/llvm/llvm-project/pull/86878

>From fe5c9d0532e92ee5948407622a2f218be72a9cf3 Mon Sep 17 00:00:00 2001
From: Alex B <alexborcan at meta.com>
Date: Wed, 27 Mar 2024 14:51:19 -0700
Subject: [PATCH 1/3] [lld-macho] Fix Issue with makeSyntheticInputSection +
 dead_strip

---
 lld/MachO/InputSection.cpp                         | 1 +
 lld/test/MachO/objc-relative-method-lists-simple.s | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/lld/MachO/InputSection.cpp b/lld/MachO/InputSection.cpp
index e3d7a400e4ce92..ec0440ce7a9632 100644
--- a/lld/MachO/InputSection.cpp
+++ b/lld/MachO/InputSection.cpp
@@ -281,6 +281,7 @@ ConcatInputSection *macho::makeSyntheticInputSection(StringRef segName,
   Section &section =
       *make<Section>(/*file=*/nullptr, segName, sectName, flags, /*addr=*/0);
   auto isec = make<ConcatInputSection>(section, data, align);
+  isec->live = true;
   section.subsections.push_back({0, isec});
   return isec;
 }
diff --git a/lld/test/MachO/objc-relative-method-lists-simple.s b/lld/test/MachO/objc-relative-method-lists-simple.s
index 1ffec3c6241cf4..5a77085c7d93d8 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

>From 7e09f7508d275b18adeb062479422a8dd9f99e8f Mon Sep 17 00:00:00 2001
From: Alex B <alexborcan at meta.com>
Date: Wed, 27 Mar 2024 16:11:03 -0700
Subject: [PATCH 2/3] Address Feedback #1

---
 lld/MachO/ConcatOutputSection.cpp | 6 +-----
 lld/MachO/InputSection.cpp        | 2 ++
 lld/MachO/InputSection.h          | 1 +
 lld/MachO/SymbolTable.cpp         | 2 +-
 lld/MachO/SyntheticSections.cpp   | 2 +-
 lld/MachO/Writer.cpp              | 4 +---
 6 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/lld/MachO/ConcatOutputSection.cpp b/lld/MachO/ConcatOutputSection.cpp
index c5c0c8a89e2879..c5f974f9074840 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 == true);
 
       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 ec0440ce7a9632..5c1e07cd21b1fb 100644
--- a/lld/MachO/InputSection.cpp
+++ b/lld/MachO/InputSection.cpp
@@ -281,6 +281,8 @@ ConcatInputSection *macho::makeSyntheticInputSection(StringRef segName,
   Section &section =
       *make<Section>(/*file=*/nullptr, segName, sectName, flags, /*addr=*/0);
   auto isec = make<ConcatInputSection>(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 a0e6afef92cb82..0f389e50425a32 100644
--- a/lld/MachO/InputSection.h
+++ b/lld/MachO/InputSection.h
@@ -149,6 +149,7 @@ class ConcatInputSection final : public InputSection {
 };
 
 // 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 825242f2cc72ff..940054aa074476 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 == true);
 
     // 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 808ea8eac6eb59..16b869da2f1619 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<uint8_t>{selrefData, wordSize},
                                 /*align=*/wordSize);
-  objcSelref->live = true;
+  assert(objcSelref->live == true);
   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 fe989de648d789..faffdc62c5b07d 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<uint8_t>{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 == true);
 }
 
 OutputSection *macho::firstTLVDataSection = nullptr;

>From 53710c2e967fabc3132c6dbe0f04412783d953d8 Mon Sep 17 00:00:00 2001
From: Alex B <alexborcan at meta.com>
Date: Wed, 27 Mar 2024 16:16:53 -0700
Subject: [PATCH 3/3] Address Feedback #2

---
 lld/MachO/ConcatOutputSection.cpp | 2 +-
 lld/MachO/SymbolTable.cpp         | 2 +-
 lld/MachO/SyntheticSections.cpp   | 2 +-
 lld/MachO/Writer.cpp              | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lld/MachO/ConcatOutputSection.cpp b/lld/MachO/ConcatOutputSection.cpp
index c5f974f9074840..279423720be9d5 100644
--- a/lld/MachO/ConcatOutputSection.cpp
+++ b/lld/MachO/ConcatOutputSection.cpp
@@ -323,7 +323,7 @@ void TextOutputSection::finalize() {
       thunkInfo.isec =
           makeSyntheticInputSection(isec->getSegName(), isec->getName());
       thunkInfo.isec->parent = this;
-      assert(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/SymbolTable.cpp b/lld/MachO/SymbolTable.cpp
index 940054aa074476..755ff270e2f7a9 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());
-    assert(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 16b869da2f1619..6f6b66118b7a94 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<uint8_t>{selrefData, wordSize},
                                 /*align=*/wordSize);
-  assert(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 faffdc62c5b07d..1c054912551e3e 100644
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -1375,7 +1375,7 @@ void macho::createSyntheticSections() {
       segment_names::data, section_names::data, S_REGULAR,
       ArrayRef<uint8_t>{arr, target->wordSize},
       /*align=*/target->wordSize);
-  assert(in.imageLoaderCache->live == true);
+  assert(in.imageLoaderCache->live);
 }
 
 OutputSection *macho::firstTLVDataSection = nullptr;



More information about the llvm-commits mailing list