From 9b5148d4262dd07019f66d122a960607985fdcd3 Mon Sep 17 00:00:00 2001 From: Jez Ng Date: Thu, 26 Aug 2021 13:51:38 -0400 Subject: [PATCH] [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 --- lld/MachO/Driver.cpp | 3 +- lld/test/MachO/lto-archive.ll | 2 +- lld/test/MachO/objc.s | 70 ++++++++++++++++++++++++++++++++++--------- 3 files changed, 58 insertions(+), 17 deletions(-) diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp index b5458dc..2780d2b 100644 --- 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 index 80b6784..c37fdd0 100644 --- 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 index d604c7d..6d494d4 100644 --- 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 -- 2.7.4