Skip to content

[lld-macho] Ignore duplicate -rpath entries #99289

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 1 commit into from
Jul 18, 2024

Conversation

BertalanD
Copy link
Member

Starting with Xcode 16 (dyld-1122), Apple's binary utilities, e.g. dyld_info, will refuse to load binaries built against the macOS 15 SDK or newer that contain the same LC_RPATH entry multiple times:

https://github.com/apple-oss-distributions/dyld/blob/rel/dyld-1122/mach_o/Policy.cpp#L246-L249

ld-prime deduplicates entries (regardless of the deployment target), we now do the same. We also match ld-prime's and ld64's behavior by warning on duplicate -rpath arguments. This can be disabled by the LLD-specific --no-warn-duplicate-rpath flag.

Starting with Xcode 16 (dyld-1122), Apple's binary utilities, e.g.
`dyld_info`, will refuse to load binaries built against the macOS 15 SDK
or newer that contain the same `LC_RPATH` entry multiple times:

https://github.com/apple-oss-distributions/dyld/blob/rel/dyld-1122/mach_o/Policy.cpp#L246-L249

`ld-prime` deduplicates entries (regardless of the deployment target),
we now do the same. We also match `ld-prime`'s and `ld64`'s behavior by
warning on duplicate `-rpath` arguments. This can be disabled by the
LLD-specific `--no-warn-duplicate-rpath` flag.
@llvmbot
Copy link
Member

llvmbot commented Jul 17, 2024

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-macho

Author: Daniel Bertalan (BertalanD)

Changes

Starting with Xcode 16 (dyld-1122), Apple's binary utilities, e.g. dyld_info, will refuse to load binaries built against the macOS 15 SDK or newer that contain the same LC_RPATH entry multiple times:

https://github.com/apple-oss-distributions/dyld/blob/rel/dyld-1122/mach_o/Policy.cpp#L246-L249

ld-prime deduplicates entries (regardless of the deployment target), we now do the same. We also match ld-prime's and ld64's behavior by warning on duplicate -rpath arguments. This can be disabled by the LLD-specific --no-warn-duplicate-rpath flag.


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

5 Files Affected:

  • (modified) lld/MachO/Config.h (+1)
  • (modified) lld/MachO/Driver.cpp (+16-1)
  • (modified) lld/MachO/Options.td (+6)
  • (modified) lld/test/MachO/link-search-at-rpath.s (+1)
  • (modified) lld/test/MachO/rpath.s (+15)
diff --git a/lld/MachO/Config.h b/lld/MachO/Config.h
index 4d3f3d05c2338..5c354e0fe8821 100644
--- a/lld/MachO/Config.h
+++ b/lld/MachO/Config.h
@@ -199,6 +199,7 @@ struct Configuration {
   std::vector<llvm::StringRef> systemLibraryRoots;
   std::vector<llvm::StringRef> librarySearchPaths;
   std::vector<llvm::StringRef> frameworkSearchPaths;
+  bool warnDuplicateRpath = true;
   llvm::SmallVector<llvm::StringRef, 0> runtimePaths;
   std::vector<std::string> astPaths;
   std::vector<Symbol *> explicitUndefineds;
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index a370d5734124a..ffb3feae25ca4 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -1402,6 +1402,19 @@ static void eraseInitializerSymbols() {
       sym->used = false;
 }
 
+static SmallVector<StringRef, 0> getRuntimePaths(opt::InputArgList &args) {
+  SmallVector<StringRef, 0> vals;
+  DenseSet<StringRef> seen;
+  for (const Arg *arg : args.filtered(OPT_rpath)) {
+    StringRef val = arg->getValue();
+    if (seen.insert(val).second)
+      vals.push_back(val);
+    else if (config->warnDuplicateRpath)
+      warn("duplicate -rpath '" + val + "' ignored [--warn-duplicate-rpath]");
+  }
+  return vals;
+}
+
 namespace lld {
 namespace macho {
 bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
@@ -1642,7 +1655,9 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
     error("--thinlto-prefix-replace=old_dir;new_dir;obj_dir must be used with "
           "--thinlto-index-only=");
   }
-  config->runtimePaths = args::getStrings(args, OPT_rpath);
+  config->warnDuplicateRpath =
+      args.hasFlag(OPT_warn_duplicate_rpath, OPT_no_warn_duplicate_rpath, true);
+  config->runtimePaths = getRuntimePaths(args);
   config->allLoad = args.hasFlag(OPT_all_load, OPT_noall_load, false);
   config->archMultiple = args.hasArg(OPT_arch_multiple);
   config->applicationExtension = args.hasFlag(
diff --git a/lld/MachO/Options.td b/lld/MachO/Options.td
index aecced9279da4..dc2212399222f 100644
--- a/lld/MachO/Options.td
+++ b/lld/MachO/Options.td
@@ -111,6 +111,12 @@ def no_warn_dylib_install_name: Flag<["--"], "no-warn-dylib-install-name">,
 def warn_dylib_install_name: Flag<["--"], "warn-dylib-install-name">,
     HelpText<"Warn on -install_name if -dylib is not passed">,
     Group<grp_lld>;
+def warn_duplicate_rpath: Flag<["--"], "warn-duplicate-rpath">,
+    HelpText<"Warn if the same -rpath is specified multiple times (default)">,
+    Group<grp_lld>;
+def no_warn_duplicate_rpath: Flag<["--"], "no-warn-duplicate-rpath">,
+    HelpText<"Do not warn if the same -rpath is specified multiple times">,
+    Group<grp_lld>;
 def call_graph_profile_sort: Flag<["--"], "call-graph-profile-sort">,
     HelpText<"Reorder sections with call graph profile (default)">,
     Group<grp_lld>;
diff --git a/lld/test/MachO/link-search-at-rpath.s b/lld/test/MachO/link-search-at-rpath.s
index dbc0d4b3cbf63..71d27fc5033bb 100644
--- a/lld/test/MachO/link-search-at-rpath.s
+++ b/lld/test/MachO/link-search-at-rpath.s
@@ -14,6 +14,7 @@
 # RUN:     -rpath @loader_path/../foo \
 # RUN:     -rpath @loader_path/../subdir \
 # RUN:     -rpath @loader_path/../foo \
+# RUN:     --no-warn-duplicate-rpath \
 # RUN:     %t/bar.o -o %t/subdir2/libbar.dylib
 
 # RUN: %lld -lSystem %t/main.o %t/subdir2/libbar.dylib -o %t/test
diff --git a/lld/test/MachO/rpath.s b/lld/test/MachO/rpath.s
index 5b404a36b26b0..09ae108b34a21 100644
--- a/lld/test/MachO/rpath.s
+++ b/lld/test/MachO/rpath.s
@@ -12,6 +12,21 @@
 # CHECK-NEXT: cmdsize 32
 # CHECK-NEXT: path /another/rpath
 
+## Check that -rpath entries are deduplicated.
+# RUN: not %lld %t.o -o /dev/null -rpath /some/rpath -rpath /other/rpath -rpath /some/rpath 2>&1 | \
+# RUN:     FileCheck --check-prefix=FATAL %s
+# FATAL: error: duplicate -rpath '/some/rpath' ignored [--warn-duplicate-rpath]
+
+# RUN: %lld -o %t-dup %t.o -rpath /some/rpath -rpath /other/rpath -rpath /some/rpath --no-warn-duplicate-rpath
+# RUN: llvm-objdump --macho --all-headers %t-dup | FileCheck %s --check-prefix=DEDUP
+# DEDUP:      LC_RPATH
+# DEDUP-NEXT: cmdsize 24
+# DEDUP-NEXT: path /some/rpath
+# DEDUP:      LC_RPATH
+# DEDUP-NEXT: cmdsize 32
+# DEDUP-NEXT: path /other/rpath
+# DEDUP-NOT:  LC_RPATH
+
 .text
 .global _main
 _main:

Copy link
Contributor

@nico nico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@BertalanD BertalanD merged commit b1864a8 into llvm:main Jul 18, 2024
10 checks passed
@BertalanD BertalanD deleted the duplicate-rpath branch July 18, 2024 08:49
aarongable pushed a commit to chromium/chromium that referenced this pull request Jul 20, 2024
ToT Mac bots fail because of this since
llvm/llvm-project#99289.

Bug: 354162568
Change-Id: I180365db1b422765ac2950263f999c92d938c096
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5725544
Reviewed-by: Amy Huang <[email protected]>
Reviewed-by: Nico Weber <[email protected]>
Commit-Queue: Nico Weber <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1330705}
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
Starting with Xcode 16 (dyld-1122), Apple's binary utilities, e.g.
`dyld_info` (but not dyld itself), will refuse to load binaries built
against the macOS 15 SDK or newer that contain the same `LC_RPATH`
entry multiple times:

https://github.com/apple-oss-distributions/dyld/blob/rel/dyld-1122/mach_o/Policy.cpp#L246-L249

`ld-prime` deduplicates entries (regardless of the deployment target),
we now do the same. We also match `ld-prime`'s and `ld64`'s behavior by
warning on duplicate `-rpath` arguments. This can be disabled by the
LLD-specific `--no-warn-duplicate-rpath` flag.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60250827
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.

3 participants