[lld] 187ce07 - [lld-macho] Fix duplicate symbols with relocatable objects

[Keith Smiley via llvm-commits]

Author: Keith Smiley
Date: 2022-02-02T14:54:10-08:00
New Revision: 187ce07a06f58ad9e751b32faec5c3f85e5597d6

URL: https://github.com/llvm/llvm-project/commit/187ce07a06f58ad9e751b32faec5c3f85e5597d6
DIFF: https://github.com/llvm/llvm-project/commit/187ce07a06f58ad9e751b32faec5c3f85e5597d6.diff

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

In the case your framework bundles contain relocatable objects, and your
objects include LC_LINKER_OPTIONs for the framework, previously they
would not be deduplicated like they would have if they were static
archives. This was also the case if you passed `-framework` for the
framework as well.

Reviewed By: #lld-macho, thakis, oontvoo

Differential Revision: https://reviews.llvm.org/D114841

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index f0cddab94f557..c88f2482855fd 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -388,12 +388,17 @@ static void addLibrary(StringRef name, bool isNeeded, bool isWeak,
   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 @@ static void addFramework(StringRef name, bool isNeeded, bool isWeak,
         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 @@ bool macho::link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
     inputFiles.clear();
     inputSections.clear();
     loadedArchives.clear();
+    loadedObjectFrameworks.clear();
     syntheticSections.clear();
     thunkMap.clear();
 

diff  --git a/lld/test/MachO/lc-linker-option.ll b/lld/test/MachO/lc-linker-option.ll
index ea56226fb6be4..1bee13781176a 100644
--- a/lld/test/MachO/lc-linker-option.ll
+++ b/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 @@ target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16
 !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"