[PATCH] D108016: [lld-macho] Ignore LLVM segments to prevent duplicate syms

[Vincent Lee via Phabricator via llvm-commits]

thevinster updated this revision to Diff 366663.
thevinster added a comment.

Address feedback


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108016/new/

https://reviews.llvm.org/D108016

Files:
  lld/MachO/InputFiles.cpp
  lld/test/MachO/discard-llvm-sections.s


Index: lld/test/MachO/discard-llvm-sections.s
===================================================================
--- /dev/null
+++ lld/test/MachO/discard-llvm-sections.s
@@ -0,0 +1,46 @@
+# REQUIRES: x86
+# RUN: rm -rf %t; split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/foo.s -o %t/foo.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/bar.s -o %t/bar.o
+
+## _llvm. symbols are not special. LLD would produce duplicate symbol errors
+## if they were not within the LLVM segment.
+
+## 1/ Test that LLD does not produce duplicate symbols errors when linking global symbols
+##    with the same name under the LLVM segment.
+# RUN: %lld -dylib %t/foo.o %t/bar.o -o %t/libDuplicate.dylib
+
+## 2/ Test that all sections within an LLVM segment are dropped.
+# RUN: llvm-objdump --section-headers %t/libDuplicate.dylib | FileCheck %s
+
+# CHECK-LABEL: Sections:
+# CHECK-NEXT:  Idx  Name    Size      VMA               Type
+# CHECK-NEXT:  0    __text  00000000 00000000000002a8   TEXT
+
+## 3/ Test that linking global symbol that is not under the LLVM segment produces duplicate
+##    symbols
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin --defsym TEXT=0 %t/foo.s -o %t/foo.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin --defsym TEXT=0 %t/bar.s -o %t/bar.o
+# RUN: not %lld -dylib %t/foo.o %t/bar.o -o %t/libDuplicate.dylib 2>&1 | FileCheck %s --check-prefix=DUP
+
+# DUP: ld64.lld: error: duplicate symbol: _llvm.foo
+
+#--- foo.s
+.globl _llvm.foo
+.ifdef TEXT
+  .section __TEXT,__cstring
+.else
+  .section __LLVM,__bitcode
+.endif
+  _llvm.foo:
+    .asciz "test"
+
+#--- bar.s
+.globl _llvm.foo
+.ifdef TEXT
+  .section __TEXT,__cstring
+.else
+  .section __LLVM,__bitcode
+.endif
+  _llvm.foo:
+    .asciz "test"
Index: lld/MachO/InputFiles.cpp
===================================================================
--- lld/MachO/InputFiles.cpp
+++ lld/MachO/InputFiles.cpp
@@ -282,16 +282,24 @@
     } else {
       auto *isec =
           make<ConcatInputSection>(segname, name, this, data, align, flags);
-      if (!(isDebugSection(isec->getFlags()) &&
-            isec->getSegName() == segment_names::dwarf)) {
-        subsections.push_back({{0, isec}});
-      } else {
+      if (isDebugSection(isec->getFlags()) &&
+          isec->getSegName() == segment_names::dwarf) {
         // Instead of emitting DWARF sections, we emit STABS symbols to the
         // object files that contain them. We filter them out early to avoid
         // parsing their relocations unnecessarily. But we must still push an
         // empty map to ensure the indices line up for the remaining sections.
         subsections.push_back({});
         debugSections.push_back(isec);
+      } else if (isec->getSegName() == segment_names::llvm) {
+        // ld64 does not appear to emit contents from sections within the __LLVM
+        // segment. Symbols within those sections point to bitcode metadata
+        // instead of actual symbols. Global symbols within those sections could
+        // have the same name without causing duplicate symbol errors. Push an
+        // empty map to ensure indices line up for the remaining sections.
+        // TODO: Evaluate whether the bitcode metadata is needed.
+        subsections.push_back({});
+      } else {
+        subsections.push_back({{0, isec}});
       }
     }
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D108016.366663.patch
Type: text/x-patch
Size: 3391 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210816/2a1df504/attachment.bin>