[lld-macho] Have -ObjC load archive members before symbol resolution

This is what ld64 does. Deviating in behavior here can result in some subtle duplicate symbol errors, as detailed in the objc.s test. Differential Revision: https://reviews.llvm.org/D108781
llvm · Aug 26, 2021 · 9b5148d · 9b5148d
1 parent 9065fe5
commit 9b5148d
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 17 deletions.
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
@@ -269,8 +269,7 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
     } else if (config->forceLoadObjC) {
       for (const object::Archive::Symbol &sym : file->getArchive().symbols())
         if (sym.getName().startswith(objc::klass))
-          symtab->addUndefined(sym.getName(), /*file=*/nullptr,
-                               /*isWeakRef=*/false);
+          file->fetch(sym);
 
       // TODO: no need to look for ObjC sections for a given archive member if
       // we already found that it contains an ObjC symbol.

diff --git a/lld/test/MachO/lto-archive.ll b/lld/test/MachO/lto-archive.ll
@@ -17,8 +17,8 @@
 
 ; RUN: %lld -lSystem -ObjC -framework CoreFoundation %t/objc.a %t/main.o \
 ; RUN:   -o /dev/null -why_load | FileCheck %s --check-prefix=OBJC
-; OBJC: -ObjC forced load of has-objc-category.o
 ; OBJC: _OBJC_CLASS_$_Foo forced load of has-objc-symbol.o
+; OBJC: -ObjC forced load of has-objc-category.o
 
 ; RUN: %lld -lSystem %t/foo.a %t/main.o -o %t/no-force-load -why_load | count 0
 

diff --git a/lld/test/MachO/objc.s b/lld/test/MachO/objc.s
@@ -3,21 +3,28 @@
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/has-objc-symbol.s -o %t/has-objc-symbol.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/has-objc-category.s -o %t/has-objc-category.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/has-objc-symbol-and-category.s -o %t/has-objc-symbol-and-category.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/has-swift.s -o %t/has-swift.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/no-objc.s -o %t/no-objc.o
 ## Make sure we don't mis-parse a 32-bit file as 64-bit
 # RUN: llvm-mc -filetype=obj -triple=armv7-apple-watchos %t/no-objc.s -o %t/wrong-arch.o
-# RUN: llvm-ar rcs %t/libHasSomeObjC.a %t/has-objc-symbol.o %t/has-objc-category.o %t/has-swift.o %t/no-objc.o %t/wrong-arch.o
+# RUN: llvm-ar rcs %t/libHasSomeObjC.a %t/no-objc.o %t/has-objc-symbol.o %t/has-objc-category.o %t/has-swift.o %t/wrong-arch.o
+# RUN: llvm-ar rcs %t/libHasSomeObjC2.a %t/no-objc.o %t/has-objc-symbol-and-category.o %t/has-swift.o %t/wrong-arch.o
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o
+
 # RUN: %lld -lSystem %t/test.o -o %t/test -L%t -lHasSomeObjC -ObjC
 # RUN: llvm-objdump --section-headers --syms %t/test | FileCheck %s --check-prefix=OBJC
 
+# RUN: %lld -lSystem %t/test.o -o %t/test -L%t -lHasSomeObjC2 -ObjC
+# RUN: llvm-objdump --section-headers --syms %t/test | FileCheck %s --check-prefix=OBJC
+
 # OBJC:       Sections:
-# OBJC-NEXT:  Idx Name           Size   VMA  Type
-# OBJC-NEXT:    0 __text         {{.*}}      TEXT
-# OBJC-NEXT:    1 __swift        {{.*}}      DATA
-# OBJC-NEXT:    2 __objc_catlist {{.*}}      DATA
+# OBJC-NEXT:  Idx Name            Size   VMA  Type
+# OBJC-NEXT:    0 __text          {{.*}}      TEXT
+# OBJC-NEXT:    1 __swift         {{.*}}      DATA
+# OBJC-NEXT:    2 __objc_catlist  {{.*}}      DATA
+# OBJC-NEXT:    3 has_objc_symbol {{.*}}      DATA
 # OBJC-EMPTY:
 # OBJC-NEXT:  SYMBOL TABLE:
 # OBJC-NEXT:  g     F __TEXT,__text _main
@@ -36,31 +43,66 @@
 # NO-OBJC-NEXT:          *UND* dyld_stub_binder
 # NO-OBJC-EMPTY:
 
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/refs-dup.s -o %t/refs-dup.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/refs-objc.s -o %t/refs-objc.o
+
+## Check that -ObjC causes has-objc-symbol.o to be loaded first, prior to symbol
+## resolution. This matches ld64's behavior.
+# RUN: %lld -dylib %t/refs-dup.o %t/refs-objc.o -o %t/refs-dup -L%t -lHasSomeObjC -ObjC
+# RUN: llvm-objdump --macho --syms %t/refs-dup | FileCheck %s --check-prefix=DUP-FROM-OBJC
+# DUP-FROM-OBJC: g     O __DATA,has_objc_symbol _has_dup
+
+## Without -ObjC, no-objc.o gets loaded first during symbol resolution, causing
+## a duplicate symbol error.
+# RUN: not %lld -dylib %t/refs-dup.o %t/refs-objc.o -o %t/refs-dup -L%t \
+# RUN:   -lHasSomeObjC 2>&1 | FileCheck %s --check-prefix=DUP-ERROR
+# DUP-ERROR: error: duplicate symbol: _has_dup
+
 #--- has-objc-symbol.s
-.globl _OBJC_CLASS_$_MyObject
+.globl _OBJC_CLASS_$_MyObject, _has_dup
 _OBJC_CLASS_$_MyObject:
 
+.section __DATA,has_objc_symbol
+_has_dup:
+
 #--- has-objc-category.s
 .section __DATA,__objc_catlist
 .quad 0x1234
 
+#--- has-objc-symbol-and-category.s
+## Make sure we load this archive member exactly once (i.e. no duplicate symbol
+## error).
+.globl _OBJC_CLASS_$_MyObject, _has_dup
+_OBJC_CLASS_$_MyObject:
+
+.section __DATA,has_objc_symbol
+_has_dup:
+
+.section __DATA,__objc_catlist
+.quad 0x1234
+
 #--- has-swift.s
 .section __TEXT,__swift
 .quad 0x1234
 
 #--- no-objc.s
-## This archive member should not be pulled in since it does not contain any
-## ObjC-related data.
-.globl _foo
+## This archive member should not be pulled in by -ObjC since it does not
+## contain any ObjC-related data.
+.globl _has_dup
 .section __DATA,foo
-
-foo:
-  .quad 0x1234
-
 .section __DATA,bar
-.section __DATA,baz
+.section __DATA,no_objc
+_has_dup:
 
 #--- test.s
 .globl _main
 _main:
   ret
+
+#--- refs-dup.s
+.data
+.quad _has_dup
+
+#--- refs-objc.s
+.data
+.quad _OBJC_CLASS_$_MyObject