[lld-macho] Unreferenced weak dylib symbols shouldn't fetch archive symbols

[int3]

Summary

We were fetching archive symbols too eagerly, bloating binary size as well as
just screwing up binaries that expected to look up certain symbols only at
runtime.

[int3]

tweak

[oontvoo]

lld/MachO/SymbolTable.cpp

193

I wonder if this should be == RefState::Strong.

can't find docs anywhere for MachO, but in ELF, it seems weak-ref doesn't trigger a fetch from an archive ...

[int3]

lld/MachO/SymbolTable.cpp

193

good question. Turns out that for Mach-O, weak refs do trigger fetches. But I should have tested it :) adding one now

[int3]

extend test

[oontvoo]

LG Thanks!

[MaskRay]

lld/MachO/SymbolTable.cpp

194

file->fetch(sym); is not covered by tests. D116916 will add the coverage.

[MaskRay]

Question: do all these tests match ld64 behavior? I think the weak-dylib condition is quite nice now because its rules are commutative.

## (Weak) archive symbols have the same precedence as dylib symbols.

Non-weak-dylib looks a bit odd as it is not commutative.

[int3]

Non-weak-dylib looks a bit odd as it is not commutative.

Non-weak-dylib has the same precedence as archive symbols, so yeah, it's not commutative. This is what ld64 does too.

It took me a while to find the test that covered this case, so I put up D120938: [lld-macho][nfc] Rename some tests for consistency

lld/MachO/SymbolTable.cpp

 }
 
 Symbol *SymbolTable::addLazy(StringRef name, ArchiveFile *file,
                              const object::Archive::Symbol &sym) {
   Symbol *s;
   bool wasInserted;
   std::tie(s, wasInserted) = insert(name, file);
 
-  if (wasInserted)
+  if (wasInserted) {
     replaceSymbol<LazySymbol>(s, file, sym);
-  else if (isa<Undefined>(s) || (isa<DylibSymbol>(s) && s->isWeakDef()))
+  } else if (isa<Undefined>(s)) {
+    file->fetch(sym);
+  } else if (auto *dysym = dyn_cast<DylibSymbol>(s)) {
+    if (dysym->isWeakDef()) {
+      if (dysym->getRefState() != RefState::Unreferenced)
         file->fetch(sym);
+      else
+        replaceSymbol<LazySymbol>(s, file, sym);
+    }
+  }
   return s;
 }
 
 Defined *SymbolTable::addSynthetic(StringRef name, InputSection *isec,
                                    uint64_t value, bool isPrivateExtern,
                                    bool includeInSymtab,
                                    bool referencedDynamically) {
   Defined *s =

lld/test/MachO/weak-definition-direct-fetch.s

 # REQUIRES: x86
 # RUN: rm -rf %t; split-file %s %t
 
 ## This test exercises the various possible combinations of weak and non-weak
 ## symbols that get referenced directly by a relocation in an object file.
 
-# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/refs-foo.s -o %t/refs-foo.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/weak-refs-foo.s -o %t/weak-refs-foo.o
 # 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/weakfoo.s -o %t/weakfoo.o
 
 # RUN: %lld -dylib -install_name \
 # RUN:   @executable_path/libfoo.dylib %t/foo.o -o %t/libfoo.dylib
 # RUN: %lld -dylib -install_name \
 # RUN:   @executable_path/libweakfoo.dylib %t/weakfoo.o -o %t/libweakfoo.dylib
 

 # RUN: llvm-ar --format=darwin rcs %t/weakfoo.a %t/weakfoo.o
 
 ## End of input file setup. The following lines check which symbol "wins" when
 ## there are multiple definitions.
 
 # PREFER-NONWEAK-DYLIB:  __DATA __la_symbol_ptr 0x{{[0-9a-f]+}} libfoo _foo
 # PREFER-WEAK-OBJECT:    O __TEXT,weak _foo
 # PREFER-NONWEAK-OBJECT: O __TEXT,nonweak _foo
+# NO-SYM-NOT: _foo
 
 ## First, we test the cases where the symbols are of the same type (both from a
 ## dylib, or both from an archive, etc.)
 ##
 ## For dylibs and object files, the non-weak symbol always wins. But the weak
 ## flag has no effect when we are dealing with two archive symbols.
 
-# RUN: %lld -lSystem -o %t/weak-nonweak-dylibs -L%t -lweakfoo -lfoo %t/test.o
+# RUN: %lld -lSystem -o %t/weak-nonweak-dylibs -L%t -lweakfoo -lfoo %t/refs-foo.o
 # RUN: llvm-objdump --macho --lazy-bind --syms %t/weak-nonweak-dylibs | FileCheck %s --check-prefix=PREFER-NONWEAK-DYLIB
-# RUN: %lld -lSystem -o %t/nonweak-weak-dylibs -L%t -lfoo -lweakfoo %t/test.o
+# RUN: %lld -lSystem -o %t/nonweak-weak-dylibs -L%t -lfoo -lweakfoo %t/refs-foo.o
 # RUN: llvm-objdump --macho --lazy-bind --syms %t/nonweak-weak-dylibs | FileCheck %s --check-prefix=PREFER-NONWEAK-DYLIB
 
-# RUN: %lld -lSystem -o %t/weak-nonweak-objs %t/weakfoo.o %t/foo.o %t/test.o
+# RUN: %lld -lSystem -o %t/weak-nonweak-objs %t/weakfoo.o %t/foo.o %t/refs-foo.o
 # RUN: llvm-objdump --macho --lazy-bind --syms %t/weak-nonweak-objs | FileCheck %s --check-prefix=PREFER-NONWEAK-OBJECT
-# RUN: %lld -lSystem -o %t/nonweak-weak-objs %t/foo.o %t/weakfoo.o %t/test.o
+# RUN: %lld -lSystem -o %t/nonweak-weak-objs %t/foo.o %t/weakfoo.o %t/refs-foo.o
 # RUN: llvm-objdump --macho --lazy-bind --syms %t/nonweak-weak-objs | FileCheck %s --check-prefix=PREFER-NONWEAK-OBJECT
 
-# RUN: %lld -lSystem -o %t/weak-nonweak-archives %t/weakfoo.a %t/foo.a %t/test.o
+# RUN: %lld -lSystem -o %t/weak-nonweak-archives %t/weakfoo.a %t/foo.a %t/refs-foo.o
 # RUN: llvm-objdump --macho --lazy-bind --syms %t/weak-nonweak-archives | FileCheck %s --check-prefix=PREFER-WEAK-OBJECT
-# RUN: %lld -lSystem -o %t/nonweak-weak-archives %t/foo.a %t/weakfoo.a %t/test.o
+# RUN: %lld -lSystem -o %t/nonweak-weak-archives %t/foo.a %t/weakfoo.a %t/refs-foo.o
 # RUN: llvm-objdump --macho --lazy-bind --syms %t/nonweak-weak-archives | FileCheck %s --check-prefix=PREFER-NONWEAK-OBJECT
 
-## The remaining lines test symbol pairs of different types.
+## The next 5 chunks refs-foo.symbol pairs of different types.
 
 ## (Weak) archive symbols take precedence over weak dylib symbols.
-# RUN: %lld -lSystem -o %t/weak-dylib-weak-ar -L%t -lweakfoo %t/weakfoo.a %t/test.o
+# RUN: %lld -lSystem -o %t/weak-dylib-weak-ar -L%t -lweakfoo %t/weakfoo.a %t/refs-foo.o
 # RUN: llvm-objdump --macho --lazy-bind --syms %t/weak-dylib-weak-ar | FileCheck %s --check-prefix=PREFER-WEAK-OBJECT
-# RUN: %lld -lSystem -o %t/weak-ar-weak-dylib -L%t %t/weakfoo.a -lweakfoo %t/test.o
+# RUN: %lld -lSystem -o %t/weak-ar-weak-dylib -L%t %t/weakfoo.a -lweakfoo %t/refs-foo.o
 # RUN: llvm-objdump --macho --lazy-bind --syms %t/weak-ar-weak-dylib | FileCheck %s --check-prefix=PREFER-WEAK-OBJECT
 
 ## (Weak) archive symbols have the same precedence as dylib symbols.
-# RUN: %lld -lSystem -o %t/weak-ar-nonweak-dylib -L%t %t/weakfoo.a -lfoo %t/test.o
+# RUN: %lld -lSystem -o %t/weak-ar-nonweak-dylib -L%t %t/weakfoo.a -lfoo %t/refs-foo.o
 # RUN: llvm-objdump --macho --lazy-bind --syms %t/weak-ar-nonweak-dylib | FileCheck %s --check-prefix=PREFER-WEAK-OBJECT
-# RUN: %lld -lSystem -o %t/nonweak-dylib-weak-ar -L%t -lfoo %t/weakfoo.a %t/test.o
+# RUN: %lld -lSystem -o %t/nonweak-dylib-weak-ar -L%t -lfoo %t/weakfoo.a %t/refs-foo.o
 # RUN: llvm-objdump --macho --lazy-bind --syms %t/nonweak-dylib-weak-ar | FileCheck %s --check-prefix=PREFER-NONWEAK-DYLIB
 
 ## Weak defined symbols take precedence over weak dylib symbols.
-# RUN: %lld -lSystem -o %t/weak-dylib-weak-obj -L%t -lweakfoo %t/weakfoo.o %t/test.o
+# RUN: %lld -lSystem -o %t/weak-dylib-weak-obj -L%t -lweakfoo %t/weakfoo.o %t/refs-foo.o
 # RUN: llvm-objdump --macho --lazy-bind --syms %t/weak-dylib-weak-obj | FileCheck %s --check-prefix=PREFER-WEAK-OBJECT
-# RUN: %lld -lSystem -o %t/weak-obj-weak-dylib -L%t %t/weakfoo.o -lweakfoo %t/test.o
+# RUN: %lld -lSystem -o %t/weak-obj-weak-dylib -L%t %t/weakfoo.o -lweakfoo %t/refs-foo.o
 # RUN: llvm-objdump --macho --lazy-bind --syms %t/weak-obj-weak-dylib | FileCheck %s --check-prefix=PREFER-WEAK-OBJECT
 
 ## Weak defined symbols take precedence over dylib symbols.
-# RUN: %lld -lSystem -o %t/weak-obj-nonweak-dylib -L%t %t/weakfoo.o -lfoo %t/test.o
+# RUN: %lld -lSystem -o %t/weak-obj-nonweak-dylib -L%t %t/weakfoo.o -lfoo %t/refs-foo.o
 # RUN: llvm-objdump --macho --lazy-bind --syms %t/weak-obj-nonweak-dylib | FileCheck %s --check-prefix=PREFER-WEAK-OBJECT
-# RUN: %lld -lSystem -o %t/nonweak-dylib-weak-obj -L%t -lfoo %t/weakfoo.o %t/test.o
+# RUN: %lld -lSystem -o %t/nonweak-dylib-weak-obj -L%t -lfoo %t/weakfoo.o %t/refs-foo.o
 # RUN: llvm-objdump --macho --lazy-bind --syms %t/nonweak-dylib-weak-obj | FileCheck %s --check-prefix=PREFER-WEAK-OBJECT
 
 ## Weak defined symbols take precedence over archive symbols.
-# RUN: %lld -lSystem -o %t/weak-obj-nonweak-ar %t/weakfoo.o %t/foo.a %t/test.o
+# RUN: %lld -lSystem -o %t/weak-obj-nonweak-ar %t/weakfoo.o %t/foo.a %t/refs-foo.o
 # RUN: llvm-objdump --macho --lazy-bind --syms %t/weak-obj-nonweak-ar | FileCheck %s --check-prefix=PREFER-WEAK-OBJECT
-# RUN: %lld -lSystem -o %t/nonweak-ar-weak-obj %t/foo.a %t/weakfoo.o %t/test.o
+# RUN: %lld -lSystem -o %t/nonweak-ar-weak-obj %t/foo.a %t/weakfoo.o %t/refs-foo.o
 # RUN: llvm-objdump --macho --lazy-bind --syms %t/nonweak-ar-weak-obj | FileCheck %s --check-prefix=PREFER-WEAK-OBJECT
 
+## Regression test: A weak dylib symbol that isn't referenced by an undefined
+## symbol should not cause an archive symbol to get loaded.
+# RUN: %lld -dylib -lSystem -o %t/weak-ar-weak-unref-dylib -L%t %t/weakfoo.a -lweakfoo
+# RUN: llvm-objdump --macho --lazy-bind --syms %t/weak-ar-weak-unref-dylib | FileCheck %s --check-prefix=NO-SYM
+# RUN: %lld -dylib -lSystem -o %t/weak-unref-dylib-weak-ar -L%t -lweakfoo %t/weakfoo.a
+# RUN: llvm-objdump --macho --lazy-bind --syms %t/weak-unref-dylib-weak-ar | FileCheck %s --check-prefix=NO-SYM
+
+## However, weak references are sufficient to cause the archive to be loaded.
+# RUN: %lld -dylib -lSystem -o %t/weak-ar-weak-ref-weak-dylib -L%t %t/weakfoo.a -lweakfoo %t/weak-refs-foo.o
+# RUN: llvm-objdump --macho --lazy-bind --syms %t/weak-ar-weak-ref-weak-dylib | FileCheck %s --check-prefix=PREFER-WEAK-OBJECT
+# RUN: %lld -dylib -lSystem -o %t/weak-ref-weak-dylib-weak-ar -L%t -lweakfoo %t/weakfoo.a %t/weak-refs-foo.o
+# RUN: llvm-objdump --macho --lazy-bind --syms %t/weak-ref-weak-dylib-weak-ar | FileCheck %s --check-prefix=PREFER-WEAK-OBJECT
+
 #--- foo.s
 .globl _foo
 .section __TEXT,nonweak
 _foo:
 
 #--- weakfoo.s
 .globl _foo
 .weak_definition _foo
 .section __TEXT,weak
 _foo:
 
-#--- test.s
+#--- refs-foo.s
+.globl _main
+_main:
+  callq _foo
+  ret
+
+#--- weak-refs-foo.s
 .globl _main
+.weak_reference _foo
 _main:
   callq _foo
   ret