Skip to content

[lld-macho] Don't double emit reexported libraries #132275

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 21, 2025

Conversation

speednoisemovement
Copy link
Contributor

When a library is specified with both -l and -reexport_libraries, lld will emit two load commands for it, in contrast with ld64.
In an upcoming version of macOS, this fails dyld validation; see https://crbug.com/404905688

When a library is specified with both `-l` and
`-reexport_libraries`, lld will emit two load commands
for it, in contrast with ld64. In an upcoming version
of macOS, this fails dyld validation; see
https://crbug.com/404905688

Co-authored-by: Mark Rowe <[email protected]>>
@llvmbot
Copy link
Member

llvmbot commented Mar 20, 2025

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-macho

Author: Leonard Grey (speednoisemovement)

Changes

When a library is specified with both -l and -reexport_libraries, lld will emit two load commands for it, in contrast with ld64.
In an upcoming version of macOS, this fails dyld validation; see https://crbug.com/404905688


Full diff: https://github.com/llvm/llvm-project/pull/132275.diff

2 Files Affected:

  • (modified) lld/MachO/Writer.cpp (+11-9)
  • (modified) lld/test/MachO/sub-library.s (+11)
diff --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index d9856a46e8cb8..7a5fdcd2276a5 100644
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -938,17 +938,19 @@ template <class LP> void Writer::createLoadCommands() {
     }
 
     ordinal = dylibFile->ordinal = dylibOrdinal++;
-    LoadCommandType lcType =
-        dylibFile->forceWeakImport || dylibFile->refState == RefState::Weak
-            ? LC_LOAD_WEAK_DYLIB
-            : LC_LOAD_DYLIB;
+    LoadCommandType lcType = LC_LOAD_DYLIB;
+    if (dylibFile->reexport) {
+      if (dylibFile->forceWeakImport)
+        warn(path::filename(dylibFile->getName()) + " is re-exported so cannot be weak-linked");
+
+      lcType = LC_REEXPORT_DYLIB;
+    } else if (dylibFile->forceWeakImport || dylibFile->refState == RefState::Weak) {
+      lcType = LC_LOAD_WEAK_DYLIB;
+    }
     in.header->addLoadCommand(make<LCDylib>(lcType, dylibFile->installName,
-                                            dylibFile->compatibilityVersion,
-                                            dylibFile->currentVersion));
+                                        dylibFile->compatibilityVersion,
+                                        dylibFile->currentVersion));
 
-    if (dylibFile->reexport)
-      in.header->addLoadCommand(
-          make<LCDylib>(LC_REEXPORT_DYLIB, dylibFile->installName));
   }
 
   for (const auto &dyldEnv : config->dyldEnvs)
diff --git a/lld/test/MachO/sub-library.s b/lld/test/MachO/sub-library.s
index ed0a96aa58f90..d799b8d54c01a 100644
--- a/lld/test/MachO/sub-library.s
+++ b/lld/test/MachO/sub-library.s
@@ -42,6 +42,17 @@
 # REEXPORT-HEADERS-NOT: Load command
 # REEXPORT-HEADERS:     name    [[PATH]]
 
+## Check that specifying a library both with `l` and `reexport_library`
+## doesn't emit two load commands.
+# RUN: %lld -dylib -reexport_library %t/libgoodbye.dylib \
+# RUN:   -L%t -lgoodbye %t/libsuper.o -o %t/libsuper.dylib
+# RUN: llvm-otool -L %t/libsuper.dylib | FileCheck %s \
+# RUN:   --check-prefix=REEXPORT-DOUBLE -DPATH=%t/libgoodbye.dylib
+
+# REEXPORT-DOUBLE: [[PATH]]
+# REEXPORT-DOUBLE-SAME: reexport
+# REEXPORT-DOUBLE-NOT: [[PATH]]
+
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t/sub-library.o
 # RUN: %lld -o %t/sub-library -L%t -lsuper %t/sub-library.o
 

Copy link

github-actions bot commented Mar 20, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@speednoisemovement speednoisemovement merged commit e385ec9 into llvm:main Mar 21, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants