[lld-macho] Add implicit dylib support for frameworks

[int3]

Summary

D93000: [lld-macho] Implement `-no_implicit_dylibs` applied to frameworks. Partial fix for PR48511.

[int3]

@thakis I'm unable to properly test PR48511 as the CoreGraphics on my local system seems to have a slightly different set of symbols (despite it being version 10.15.7 too). However, the binary is now trying to import the symbol from CoreGraphics rather than ApplicationServices, so I think this diff is on the right track.

[thakis]

Thanks! I tested this and put the details in https://bugs.llvm.org/show_bug.cgi?id=48511#c3 and https://bugs.llvm.org/show_bug.cgi?id=48511#c5 . Summary: This is a good change, but it's not sufficient for that bug. One comment about pedantic correctness below :)

lld/MachO/InputFiles.cpp

514

This matches File<A>::isPublicLocation in ld64 which lists a few special cases here https://github.com/apple-opensource/ld64/blob/fd3feabb0a1eb18ab5d7910f3c3a5eed99cef6ab/src/ld/parsers/generic_dylib_file.hpp#L490 that this code as-is doesn't quite get right yet I think:

		// but only top level framework
		// /System/Library/Frameworks/Foo.framework/Versions/A/Foo                 ==> true
		// /System/Library/Frameworks/Foo.framework/Resources/libBar.dylib         ==> false
		// /System/Library/Frameworks/Foo.framework/Frameworks/Bar.framework/Bar   ==> false
		// /System/Library/Frameworks/Foo.framework/Frameworks/Xfoo.framework/XFoo ==> false

[thakis]

I suppose even with the pedantic correctness thing fixed it's a progression, so I should lg. But probably good to get it fully right immediately so we can consider implicit_dylibs as done and forget about it -- I'd recommend implementing that suggestion before committing :)

[int3]

lld/MachO/InputFiles.cpp

514

I think the code gets this right, but I'll extend the test to verify :)

[int3]

verify that we only re-export top-level frameworks

lld/MachO/InputFiles.cpp

 // If a re-exported dylib is public (lives in /usr/lib or
 // /System/Library/Frameworks), then it is considered implicitly linked: we
 // should bind to its symbols directly instead of via the re-exporting umbrella
 // library.
 static bool isImplicitlyLinked(StringRef path) {
   if (!config->implicitDylibs)
     return false;
 
-  return path::parent_path(path) == "/usr/lib";
-  // TODO: check for public frameworks too. We'll need to implement
-  // -sub_umbrella first to write a test case.
+  if (path::parent_path(path) == "/usr/lib")
+    return true;
+
+  // Match /System/Library/Frameworks/$FOO.framework/**/$FOO
+  if (path.consume_front("/System/Library/Frameworks/")) {
+    StringRef frameworkName = path.take_until([](char c) { return c == '.'; });
+    return path::filename(path) == frameworkName;
+  }
+
+  return false;
 }
 
 void loadReexport(StringRef path, DylibFile *umbrella) {
   Optional<DylibFile *> reexport = loadReexportHelper(path, umbrella);
   if (reexport && isImplicitlyLinked(path))
     inputFiles.insert(*reexport);
 }
 

lld/test/MachO/implicit-dylibs.s

 # REQUIRES: x86
 ## FIXME: We appear to emit LC_LOAD_DYLIBs in a different order on Windows.
 # UNSUPPORTED: system-windows
 # RUN: rm -rf %t; split-file %s %t
 # RUN: mkdir -p %t/usr/lib/system
+# RUN: mkdir -p %t/System/Library/Frameworks/Foo.framework/Versions/A
+# RUN: mkdir -p %t/System/Library/Frameworks/Foo.framework/Frameworks/Bar.framework/Versions/A
+# RUN: mkdir -p %t/Baz.framework/Versions/A
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/libfoo.s -o %t/libfoo.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/libtoplevel.s -o %t/libtoplevel.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/libsublevel.s -o %t/libsublevel.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/framework-foo.s -o %t/framework-foo.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/framework-bar.s -o %t/framework-bar.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/framework-baz.s -o %t/framework-baz.o
 ## libunused will be used to verify that we load implicit dylibs even if we
 ## don't use any symbols they contain.
 # RUN: echo "" | llvm-mc -filetype=obj -triple=x86_64-apple-darwin -o %t/libunused.o
 # RUN: echo "" | llvm-mc -filetype=obj -triple=x86_64-apple-darwin -o %t/reexporter.o
 
 # RUN: %lld -dylib -lSystem %t/libfoo.o -o %t/libfoo.dylib
 # RUN: %lld -dylib -lSystem %t/libtoplevel.o -o %t/usr/lib/libtoplevel.dylib -install_name /usr/lib/libtoplevel.dylib
 # RUN: %lld -dylib -lSystem %t/libsublevel.o -o %t/usr/lib/system/libsublevel.dylib -install_name /usr/lib/system/libsublevel.dylib
 # RUN: %lld -dylib -lSystem %t/libunused.o -o %t/usr/lib/libunused.dylib -install_name /usr/lib/libunused.dylib
-# RUN: %lld -dylib -syslibroot %t \
+
+## Bar.framework is nested within Foo.framework.
+# RUN: %lld -dylib -lSystem %t/framework-baz.o -o %t/System/Library/Frameworks/Foo.framework/Frameworks/Bar.framework/Versions/A/Bar \
+# RUN:   -install_name /System/Library/Frameworks/Foo.framework/Frameworks/Bar.framework/Versions/A/Bar
+# RUN: ln -sf %t/System/Library/Frameworks/Foo.framework/Frameworks/Bar.framework/Versions/A/Bar \
+# RUN:   %t/System/Library/Frameworks/Foo.framework/Frameworks/Bar.framework/Bar
+
+## Have Foo re-export Bar.
+# RUN: %lld -dylib -F %t/System/Library/Frameworks/Foo.framework/Frameworks \
+# RUN:   -framework Bar -sub_umbrella Bar -lSystem %t/framework-foo.o -o %t/System/Library/Frameworks/Foo.framework/Versions/A/Foo \
+# RUN:   -install_name /System/Library/Frameworks/Foo.framework/Versions/A/Foo
+# RUN: ln -sf %t/System/Library/Frameworks/Foo.framework/Versions/A/Foo %t/System/Library/Frameworks/Foo.framework/Foo
+
+# RUN: %lld -dylib -lSystem %t/framework-bar.o -o %t/Baz.framework/Versions/A/Baz \
+# RUN:   -install_name %t/Baz.framework/Versions/A/Baz
+# RUN: ln -sf %t/Baz.framework/Versions/A/Baz %t/Baz.framework/Baz
+
+# RUN: %lld -dylib -syslibroot %t -framework Foo -F %t -framework Baz \
 # RUN:   -lc++ -ltoplevel -lunused %t/usr/lib/system/libsublevel.dylib %t/libfoo.dylib \
 # RUN:   -sub_library libc++ -sub_library libfoo -sub_library libtoplevel \
 # RUN:   -sub_library libsublevel -sub_library libunused \
+# RUN:   -sub_umbrella Baz -sub_umbrella Foo \
 # RUN:   %t/reexporter.o -o %t/libreexporter.dylib
 
 # RUN: llvm-mc -filetype obj -triple x86_64-apple-darwin %t/test.s -o %t/test.o
 # RUN: %lld -syslibroot %t -o %t/test -lSystem -L%t -lreexporter %t/test.o
 # RUN: llvm-objdump --bind --no-show-raw-insn -d %t/test | FileCheck %s
 # CHECK:     Bind table:
 # CHECK-DAG: __DATA __data {{.*}} pointer 0 libreexporter _foo
 # CHECK-DAG: __DATA __data {{.*}} pointer 0 libtoplevel   _toplevel
 # CHECK-DAG: __DATA __data {{.*}} pointer 0 libreexporter _sublevel
+# CHECK-DAG: __DATA __data {{.*}} pointer 0 Foo           _framework_foo
+# CHECK-DAG: __DATA __data {{.*}} pointer 0 libreexporter _framework_bar
+# CHECK-DAG: __DATA __data {{.*}} pointer 0 libreexporter _framework_baz
 # CHECK-DAG: __DATA __data {{.*}} pointer 0 libc++abi     ___gxx_personality_v0
 
 # RUN: llvm-objdump --macho --all-headers %t/test | FileCheck %s \
 # RUN:   --check-prefix=LOAD -DDIR=%t --implicit-check-not LC_LOAD_DYLIB
 ## Check that we don't create duplicate LC_LOAD_DYLIBs.
 # RUN: %lld -syslibroot %t -o %t/test -lSystem -L%t -lreexporter -ltoplevel %t/test.o
 # RUN: llvm-objdump --macho --all-headers %t/test | FileCheck %s \
 # RUN:   --check-prefix=LOAD -DDIR=%t --implicit-check-not LC_LOAD_DYLIB
 
 # LOAD:          cmd LC_LOAD_DYLIB
 # LOAD-NEXT: cmdsize
 # LOAD-NEXT:    name /usr/lib/libSystem.B.dylib
 # LOAD:          cmd LC_LOAD_DYLIB
 # LOAD-NEXT: cmdsize
+# LOAD-NEXT:    name /System/Library/Frameworks/Foo.framework/Versions/A/Foo
+# LOAD:          cmd LC_LOAD_DYLIB
+# LOAD-NEXT: cmdsize
 # LOAD-NEXT:    name /usr/lib/libc++abi.dylib
 # LOAD:          cmd LC_LOAD_DYLIB
 # LOAD-NEXT: cmdsize
 # LOAD-NEXT:    name /usr/lib/libc++.dylib
 # LOAD:          cmd LC_LOAD_DYLIB
 # LOAD-NEXT: cmdsize
 # LOAD-NEXT:    name /usr/lib/libtoplevel.dylib
 # LOAD:          cmd LC_LOAD_DYLIB
 # LOAD-NEXT: cmdsize
 # LOAD-NEXT:    name /usr/lib/libunused.dylib
 # LOAD:          cmd LC_LOAD_DYLIB
 # LOAD-NEXT: cmdsize
 # LOAD-NEXT:    name [[DIR]]/libreexporter.dylib
 
 # RUN: %lld -no_implicit_dylibs -syslibroot %t -o %t/no-implicit -lSystem -L%t -lreexporter %t/test.o
 # RUN: llvm-objdump --bind --no-show-raw-insn -d %t/no-implicit | FileCheck %s --check-prefix=NO-IMPLICIT
 # NO-IMPLICIT:     Bind table:
 # NO-IMPLICIT-DAG: __DATA __data {{.*}} pointer 0 libreexporter _foo
 # NO-IMPLICIT-DAG: __DATA __data {{.*}} pointer 0 libreexporter _toplevel
 # NO-IMPLICIT-DAG: __DATA __data {{.*}} pointer 0 libreexporter _sublevel
+# NO-IMPLICIT-DAG: __DATA __data {{.*}} pointer 0 libreexporter _framework_foo
+# NO-IMPLICIT-DAG: __DATA __data {{.*}} pointer 0 libreexporter _framework_bar
+# NO-IMPLICIT-DAG: __DATA __data {{.*}} pointer 0 libreexporter _framework_baz
 # NO-IMPLICIT-DAG: __DATA __data {{.*}} pointer 0 libreexporter ___gxx_personality_v0
 
 #--- libfoo.s
 .data
 .globl _foo
 _foo:
 
 #--- libtoplevel.s
 .data
 .globl _toplevel
 _toplevel:
 
 #--- libsublevel.s
 .data
 .globl _sublevel
 _sublevel:
 
+#--- framework-foo.s
+.data
+.globl _framework_foo
+_framework_foo:
+
+#--- framework-bar.s
+.data
+.globl _framework_bar
+_framework_bar:
+
+#--- framework-baz.s
+.data
+.globl _framework_baz
+_framework_baz:
+
 #--- test.s
 .text
 .globl _main
 
 _main:
   ret
 
 .data
   .quad _foo
   .quad _toplevel
   .quad _sublevel
+  .quad _framework_foo
+  .quad _framework_bar
+  .quad _framework_baz
   .quad ___gxx_personality_v0