[PATCH] D114841: [lld-macho] Fix duplicate symbols with relocatable objects

[Keith Smiley via Phabricator via llvm-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG187ce07a06f5: [lld-macho] Fix duplicate symbols with relocatable objects (authored by keith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114841

Files:
  lld/MachO/Driver.cpp
  lld/test/MachO/lc-linker-option.ll


Index: lld/test/MachO/lc-linker-option.ll
===================================================================
--- lld/test/MachO/lc-linker-option.ll
+++ lld/test/MachO/lc-linker-option.ll
@@ -77,6 +77,21 @@
 ; SYMS-NEXT:  g     F __TEXT,__text __mh_execute_header
 ; SYMS-EMPTY:
 
+;; Make sure that frameworks containing object files or bitcode instead of
+;; dylibs or archives do not cause duplicate symbol errors
+; RUN: mkdir -p %t/Foo.framework
+; RUN: llc --filetype=obj %t/foo.ll -o %t/Foo.framework/Foo
+; RUN: llc --filetype=obj %t/load-framework-twice.ll -o %t/main
+;; Order of the object with the LC_LINKER_OPTION vs -framework arg is important.
+; RUN: %lld %t/main -F %t -framework Foo -framework Foo -o /dev/null
+; RUN: %lld -F %t -framework Foo -framework Foo %t/main -o /dev/null
+
+; RUN: llvm-as %t/foo.ll -o %t/Foo.framework/Foo
+; RUN: llvm-as %t/load-framework-twice.ll -o %t/main
+;; Order of the object with the LC_LINKER_OPTION vs -framework arg is important.
+; RUN: %lld %t/main -F %t -framework Foo -framework Foo -o /dev/null
+; RUN: %lld -F %t -framework Foo -framework Foo %t/main -o /dev/null
+
 ;--- framework.ll
 target triple = "x86_64-apple-macosx10.15.0"
 target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
@@ -127,6 +142,17 @@
 !0 = !{!"-framework", !"Foo"}
 !llvm.linker.options = !{!0}
 
+;--- load-framework-twice.ll
+target triple = "x86_64-apple-macosx10.15.0"
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+!0 = !{!"-framework", !"Foo"}
+!llvm.linker.options = !{!0, !0}
+
+define void @main() {
+  ret void
+}
+
 ;--- load-library-foo.ll
 target triple = "x86_64-apple-macosx10.15.0"
 target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
Index: lld/MachO/Driver.cpp
===================================================================
--- lld/MachO/Driver.cpp
+++ lld/MachO/Driver.cpp
@@ -388,12 +388,17 @@
   error("library not found for -l" + name);
 }
 
+static DenseSet<StringRef> loadedObjectFrameworks;
 static void addFramework(StringRef name, bool isNeeded, bool isWeak,
                          bool isReexport, bool isExplicit,
                          ForceLoad forceLoadArchive) {
   if (Optional<StringRef> path = findFramework(name)) {
-    if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
-            addFile(*path, forceLoadArchive, /*isLazy=*/false, isExplicit))) {
+    if (loadedObjectFrameworks.contains(*path))
+      return;
+
+    InputFile *file =
+        addFile(*path, forceLoadArchive, /*isLazy=*/false, isExplicit);
+    if (auto *dylibFile = dyn_cast_or_null<DylibFile>(file)) {
       if (isNeeded)
         dylibFile->forceNeeded = true;
       if (isWeak)
@@ -402,6 +407,13 @@
         config->hasReexports = true;
         dylibFile->reexport = true;
       }
+    } else if (isa<ObjFile>(file) || isa<BitcodeFile>(file)) {
+      // Cache frameworks containing object or bitcode files to avoid duplicate
+      // symbols. Frameworks containing static archives are cached separately
+      // in addFile() to share caching with libraries, and frameworks
+      // containing dylibs should allow overwriting of attributes such as
+      // forceNeeded by subsequent loads
+      loadedObjectFrameworks.insert(*path);
     }
     return;
   }
@@ -1069,6 +1081,7 @@
     inputFiles.clear();
     inputSections.clear();
     loadedArchives.clear();
+    loadedObjectFrameworks.clear();
     syntheticSections.clear();
     thunkMap.clear();
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114841.405465.patch
Type: text/x-patch
Size: 3557 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220202/ceac250a/attachment.bin>