[lld] 17c43c4 - [lld/mac] Add reexports after reexporter to inputFiles

Nico Weber via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 7 14:04:10 PDT 2021


Author: Nico Weber
Date: 2021-06-07T17:04:03-04:00
New Revision: 17c43c404535fe427d2cddb665154ae601d505bd

URL: https://github.com/llvm/llvm-project/commit/17c43c404535fe427d2cddb665154ae601d505bd
DIFF: https://github.com/llvm/llvm-project/commit/17c43c404535fe427d2cddb665154ae601d505bd.diff

LOG: [lld/mac] Add reexports after reexporter to inputFiles

When a library "host"'s reexports change their installName with
`$ld$os10.11$install_name$host`, we used to write a load command for "host" but
write the version numbers of the reexport instead of "host". This fixes that.

I first thought that the rule is to take the version numbers from the library
that originally had that install name (implemented in D103819), but that's not
what ld64 seems to be doing: It takes the version number from the first dylib
with that install name it loads, and it loads the reexporting library before
the reexports. We already did most of that, we just added reexports before the
reexporter. After this change, we add the reexporter before the reexports.

Addresses https://bugs.llvm.org/show_bug.cgi?id=49800#c11 part 1.

(ld64 seems to add reexports after processing _all_ files on the command line,
while we add them right after the reexporter. For the common case of reexport +
$ld$ symbol changing back to the exporter name, this doesn't make a difference,
but you can construct a case where it does. I expect this to not make a
difference in practice though.)

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

Added: 
    

Modified: 
    lld/MachO/Driver.cpp
    lld/MachO/InputFiles.cpp
    lld/test/MachO/implicit-dylibs.s
    lld/test/MachO/special-symbol-ld-install-name.s
    lld/test/MachO/tapi-link.s

Removed: 
    


################################################################################
diff  --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 7003533b5fdf..a25033ae8a31 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -316,11 +316,10 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
   default:
     error(path + ": unhandled file type");
   }
-  if (newFile) {
+  if (newFile && !isa<DylibFile>(newFile)) {
     // printArchiveMemberLoad() prints both .a and .o names, so no need to
     // print the .a name here.
-    if (config->printEachFile && magic != file_magic::archive &&
-        !isa<DylibFile>(newFile))
+    if (config->printEachFile && magic != file_magic::archive)
       message(toString(newFile));
     inputFiles.insert(newFile);
   }

diff  --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 9051ae12d023..68208002dbf1 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -826,8 +826,6 @@ static void loadReexport(StringRef path, DylibFile *umbrella,
   DylibFile *reexport = findDylib(path, umbrella, currentTopLevelTapi);
   if (!reexport)
     error("unable to locate re-export with install name " + path);
-  else if (isImplicitlyLinked(path))
-    inputFiles.insert(reexport);
 }
 
 DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella,
@@ -858,6 +856,7 @@ DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella,
 
   if (config->printEachFile)
     message(toString(this));
+  inputFiles.insert(this);
 
   deadStrippable = hdr->flags & MH_DEAD_STRIPPABLE_DYLIB;
 
@@ -944,6 +943,7 @@ DylibFile::DylibFile(const InterfaceFile &interface, DylibFile *umbrella,
 
   if (config->printEachFile)
     message(toString(this));
+  inputFiles.insert(this);
 
   if (!is_contained(skipPlatformChecks, installName) &&
       !is_contained(interface.targets(), config->platformInfo.target)) {

diff  --git a/lld/test/MachO/implicit-dylibs.s b/lld/test/MachO/implicit-dylibs.s
index fbc18f7e55f7..ae96f979cdd1 100644
--- a/lld/test/MachO/implicit-dylibs.s
+++ b/lld/test/MachO/implicit-dylibs.s
@@ -70,6 +70,9 @@
 # LOAD-NEXT:    name /usr/lib/libSystem.dylib
 # LOAD:          cmd LC_LOAD_DYLIB
 # LOAD-NEXT: cmdsize
+# LOAD-NEXT:    name [[DIR]]/libreexporter.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
@@ -77,9 +80,6 @@
 # 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 [[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

diff  --git a/lld/test/MachO/special-symbol-ld-install-name.s b/lld/test/MachO/special-symbol-ld-install-name.s
index ca5163f70da3..ce40b93f7c6c 100644
--- a/lld/test/MachO/special-symbol-ld-install-name.s
+++ b/lld/test/MachO/special-symbol-ld-install-name.s
@@ -8,14 +8,14 @@
 ## since the specified version 11.0.0 matches the target version 11.0.0
 
 # RUN: %lld -o %t/libfoo1.dylib %t/libLDInstallName.tbd %t/foo.o -dylib -platform_version macos 11.0.0 11.0.0
-# RUN: llvm-objdump --macho --dylibs-used %t/libfoo1.dylib | FileCheck --check-prefix=CASE1 %s
+# RUN: llvm-otool -L %t/libfoo1.dylib | FileCheck --check-prefix=CASE1 %s
 # CASE1: /New (compatibility version 1.1.1, current version 5.0.0)
 
 ## Case 2: special symbol $ld$install_name does not affect the install name
 ## since the specified version 11.0.0 does not match the target version 12.0.0
 
 # RUN: %lld -o %t/libfoo2.dylib %t/libLDInstallName.tbd %t/foo.o -dylib -platform_version macos 12.0.0 12.0.0
-# RUN: llvm-objdump --macho --dylibs-used %t/libfoo2.dylib | FileCheck --check-prefix=CASE2 %s
+# RUN: llvm-otool -L %t/libfoo2.dylib | FileCheck --check-prefix=CASE2 %s
 # CASE2: /Old (compatibility version 1.1.1, current version 5.0.0)
 
 ## Check that we emit a warning for an invalid os version.
@@ -25,6 +25,19 @@
 
 # INVALID-VERSION: failed to parse os version, symbol '$ld$install_name$os11.a$/New' ignored
 
+## Case 3: If there's another library that has '/New' as its original
+## install_name, we should take current-version and compatibility-version from
+## the first library we load with that install name. For a reexport,
+## the first loaded library is the library doing the reexport, not the
+## reexported library.
+# RUN: %lld -o %t/libfoo3.dylib %t/libLDInstallNameNew.tbd %t/foo.o -dylib -platform_version macos 11.0.0 11.0.0
+# RUN: llvm-otool -L %t/libfoo3.dylib | FileCheck --check-prefix=CASE3 %s
+# RUN: %lld -o %t/libfoo3.dylib %t/libLDInstallName.tbd %t/libLDInstallNameNew.tbd %t/foo.o -dylib -platform_version macos 11.0.0 11.0.0
+# RUN: llvm-otool -L %t/libfoo3.dylib | FileCheck --check-prefix=CASE1 %s
+# RUN: %lld -o %t/libfoo3.dylib %t/libLDInstallNameNew.tbd %t/libLDInstallName.tbd %t/foo.o -dylib -platform_version macos 11.0.0 11.0.0
+# RUN: llvm-otool -L %t/libfoo3.dylib | FileCheck --check-prefix=CASE3 %s
+# CASE3: /New (compatibility version 4.5.6, current version 9.0.0)
+
 #--- foo.s
 .long	_xxx at GOTPCREL
 
@@ -53,3 +66,18 @@ exports:
   - archs:           [ x86_64 ]
     symbols:         [ '$ld$install_name$os11.a$/New', _xxx ]
 ...
+
+#--- libLDInstallNameNew.tbd
+--- !tapi-tbd
+tbd-version: 4
+targets: [ x86_64-macos ]
+uuids:
+  - target: x86_64-macos
+    value:  2E994C7F-3F03-3A07-879C-55690D22BEDA
+install-name: '/New'
+current-version: 9
+compatibility-version: 4.5.6
+reexported-libraries:
+  - targets:         [ x86_64-macos ]
+    libraries:       [ '@loader_path/libLDInstallName' ]
+...

diff  --git a/lld/test/MachO/tapi-link.s b/lld/test/MachO/tapi-link.s
index c43963c280e6..786aca68c890 100644
--- a/lld/test/MachO/tapi-link.s
+++ b/lld/test/MachO/tapi-link.s
@@ -25,18 +25,10 @@
 # CHECK-DAG: __DATA __data {{.*}} pointer 0 libc++abi      ___gxx_personality_v0
 # CHECK-DAG: __DATA __data {{.*}} pointer 0 libNested3     _deeply_nested
 
-# RUN: llvm-objdump --macho --all-headers %t/test | \
-# RUN:     FileCheck --check-prefix=LOAD %s
+# RUN: llvm-otool -l %t/test | FileCheck --check-prefix=LOAD %s
 
-# RUN: llvm-objdump --macho --all-headers %t/with-reexport | \
-# RUN:     FileCheck --check-prefixes=LOAD,LOAD-REEXPORT %s
-
-# LOAD:          cmd LC_LOAD_DYLIB
-# LOAD-NEXT:               cmdsize
-# LOAD-NEXT:                  name /usr/lib/libSystem.dylib
-# LOAD-NEXT:            time stamp
-# LOAD-NEXT:       current version 1.1.1
-# LOAD-NEXT: compatibility version
+# RUN: llvm-otool -l %t/with-reexport | \
+# RUN:     FileCheck --check-prefixes=LOAD-REEXPORT,LOAD %s
 
 # LOAD-REEXPORT:          cmd LC_LOAD_DYLIB
 # LOAD-REEXPORT-NEXT:               cmdsize
@@ -45,6 +37,13 @@
 # LOAD-REEXPORT-NEXT:       current version 1.0.0
 # LOAD-REEXPORT-NEXT: compatibility version
 
+# LOAD:          cmd LC_LOAD_DYLIB
+# LOAD-NEXT:               cmdsize
+# LOAD-NEXT:                  name /usr/lib/libSystem.dylib
+# LOAD-NEXT:            time stamp
+# LOAD-NEXT:       current version 1.1.1
+# LOAD-NEXT: compatibility version
+
 #--- test.s
 .section __TEXT,__text
 .global _main


        


More information about the llvm-commits mailing list