[lld] [lld-macho] Process OSO prefix only textually in both input and output (PR #152063)
Daniel RodrÃguez Troitiño via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 5 14:33:41 PDT 2025
https://github.com/drodriguez updated https://github.com/llvm/llvm-project/pull/152063
>From a60fdf74231e947dcbca1ead957f007a9adc5a68 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Rodr=C3=ADguez?= <danielrodriguez at meta.com>
Date: Mon, 4 Aug 2025 17:42:47 -0700
Subject: [PATCH 1/2] [lld-macho] Process OSO prefix only textually in both
input and output
The processing of `-oso_prefix` uses `llvm::sys::fs::real_path` from the
user value, but it is later tried to be matched with the result of
`make_absolute`. While `real_path` resolves special symbols like `.`,
`..` and `~`, and resolves symlinks along the path, `make_absolute` does
neither, causing an incompatibility in some situations.
In macOS, temporary directories would normally be reported as
`/var/folders/<random>`, but `/var` is in fact a symlink to
`private/var`. If own is working on a temporary directory and uses
`-oso_prefix .`, it will be expanded to `/private/var/folder/<random>`,
while `make_absolute` will expand to `/var/folder/<random>` instead, and
`-oso_prefix` will fail to remove the prefix from the `N_OSO` entries,
leaving absolute paths to the temporary directory in the resulting file.
This would happen in any situation in which the working directory
includes a symlink, not only in temporary directories.
One can change the usage of `make_absolute` to use `real_path` as well,
but `real_path` will mean checking the file system for each `N_OSO`
entry. The other solution is stop using `real_path` when processing
`-oso_prefix` and manually expand an input of `.` like `make_absolute`
will do.
This second option is the one implemented here, since it is the closest
to the visible behaviour of ld64 (like the removed comment notes), so it
is the better one for compatibility. This means that a test that checked
the usage of the tilde as `-oso_prefix` needs to be removed (since it
was done by using `real_path`), and two new tests are provided checking
that symlinks do not affect the result. The second test checks a change
in behaviour, in which if one provides the input files with a prefix of
`./`, even when using `-oso_prefix .` because the matching is textual,
the `./` prefix will stay in the `N_OSO` entries. This matches the
observed behaviour of ld64.
---
lld/MachO/Driver.cpp | 28 ++++++++++++----------------
lld/test/MachO/stabs.s | 16 +++++++++++++---
2 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 9eb391c4ee1b9..0f3db4ffa2797 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -1635,27 +1635,23 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
config->osoPrefix = args.getLastArgValue(OPT_oso_prefix);
if (!config->osoPrefix.empty()) {
- // Expand special characters, such as ".", "..", or "~", if present.
- // Note: LD64 only expands "." and not other special characters.
- // That seems silly to imitate so we will not try to follow it, but rather
- // just use real_path() to do it.
-
// The max path length is 4096, in theory. However that seems quite long
// and seems unlikely that any one would want to strip everything from the
// path. Hence we've picked a reasonably large number here.
SmallString<1024> expanded;
- if (!fs::real_path(config->osoPrefix, expanded,
- /*expand_tilde=*/true)) {
- // Note: LD64 expands "." to be `<current_dir>/`
- // (ie., it has a slash suffix) whereas real_path() doesn't.
- // So we have to append '/' to be consistent.
- StringRef sep = sys::path::get_separator();
- // real_path removes trailing slashes as part of the normalization, but
- // these are meaningful for our text based stripping
- if (config->osoPrefix == "." || config->osoPrefix.ends_with(sep))
- expanded += sep;
- config->osoPrefix = saver().save(expanded.str());
+ // Expand "." into the current working directory.
+ if (config->osoPrefix == ".") {
+ if (!fs::current_path(expanded)) {
+ // Note: LD64 expands "." to be `<current_dir>/
+ // (ie., it has a slash suffix) whereas current_path() doesn't.
+ // So we have to append '/' to be consistent because this is
+ // meaningful for our text based stripping.
+ expanded += sys::path::get_separator();
+ }
+ } else {
+ expanded = config->osoPrefix;
}
+ config->osoPrefix = saver().save(expanded.str());
}
bool pie = args.hasFlag(OPT_pie, OPT_no_pie, true);
diff --git a/lld/test/MachO/stabs.s b/lld/test/MachO/stabs.s
index 968656dba5cba..e32b9fc5b50d6 100644
--- a/lld/test/MachO/stabs.s
+++ b/lld/test/MachO/stabs.s
@@ -63,9 +63,18 @@
# RUN: dsymutil -s %t/test-rel | grep 'N_OSO' | FileCheck %s -D#TEST_TIME=0x10 -D#FOO_TIME=0x20 --check-prefix=REL-PATH-NO-SLASH
# RUN: cd %t && ZERO_AR_DATE=0 %lld -lSystem test.o foo.o no-debug.o -oso_prefix "." -o %t/test-rel-dot
# RUN: dsymutil -s %t/test-rel-dot | grep 'N_OSO' | FileCheck %s -D#TEST_TIME=0x10 -D#FOO_TIME=0x20 --check-prefix=REL-DOT
-## Set HOME to %t (for ~ to expand to)
-# RUN: cd %t && env HOME=%t ZERO_AR_DATE=0 %lld -lSystem test.o foo.o no-debug.o -oso_prefix "~" -o %t/test-rel-tilde
-# RUN: dsymutil -s %t/test-rel-tilde | grep 'N_OSO' | FileCheck %s -D#TEST_TIME=0x10 -D#FOO_TIME=0x20 --check-prefix=REL-PATH
+# RUN: cd %t && ZERO_AR_DATE=0 %lld -lSystem ./test.o ./foo.o ./no-debug.o -oso_prefix "." -o %t/test-rel-dot
+# RUN: dsymutil -s %t/test-rel-dot | grep 'N_OSO' | FileCheck %s -D#TEST_TIME=0x10 -D#FOO_TIME=0x20 --check-prefix=REL-DOT-EXPLICIT
+
+## Check that symlinks are not expanded when -oso_prefix . is used.
+# RUN: mkdir -p %t/private/var/folders/tmp && ln -s private/var %t/var
+# RUN: cp %t/test.o %t/foo.o %t/no-debug.o %t/private/var/folders/tmp
+# RUN: env TZ=GMT touch -t "197001010000.16" %t/private/var/folders/tmp/test.o
+# RUN: env TZ=GMT touch -t "197001010000.32" %t/private/var/folders/tmp/foo.o
+# RUN: cd %t/var/folders/tmp && ZERO_AR_DATE=0 %lld -lSystem test.o foo.o no-debug.o -oso_prefix "." -o test-rel-symlink
+# RUN: dsymutil -s %t/private/var/folders/tmp/test-rel-symlink | grep 'N_OSO' | FileCheck %s -D#TEST_TIME=0x10 -D#FOO_TIME=0x20 --check-prefix=REL-DOT
+# RUN: cd %t/var/folders/tmp && ZERO_AR_DATE=0 %lld -lSystem ./test.o ./foo.o ./no-debug.o -oso_prefix "." -o test-rel-symlink
+# RUN: dsymutil -s %t/private/var/folders/tmp/test-rel-symlink | grep 'N_OSO' | FileCheck %s -D#TEST_TIME=0x10 -D#FOO_TIME=0x20 --check-prefix=REL-DOT-EXPLICIT
## Check that we don't emit DWARF or stabs when -S is used
# RUN: %lld -lSystem test.o foo.o no-debug.o -S -o %t/test-no-debug
@@ -91,6 +100,7 @@
# REL-PATH: (N_OSO ) 03 0001 [[#%.16x,TEST_TIME]] '/test.o'
# REL-PATH-NO-SLASH: (N_OSO ) 03 0001 [[#%.16x,TEST_TIME]] 'test.o'
# REL-DOT: (N_OSO ) 03 0001 [[#%.16x,TEST_TIME]] 'test.o'
+# REL-DOT-EXPLICIT: (N_OSO ) 03 0001 [[#%.16x,TEST_TIME]] './test.o'
# CHECK-NEXT: (N_STSYM ) [[#%.2d,MORE_DATA_ID + 1]] 0000 [[#%.16x,STATIC:]] '_static_var'
# CHECK-NEXT: (N_FUN ) [[#%.2d,TEXT_ID + 1]] 0000 [[#%.16x,MAIN:]] '_main'
# CHECK-NEXT: (N_FUN ) 00 0000 0000000000000006{{$}}
>From 2c7e2afc1dc480bdd233a3882a55d9ddf60cc530 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Rodr=C3=ADguez?= <danielrodriguez at meta.com>
Date: Tue, 5 Aug 2025 14:33:24 -0700
Subject: [PATCH 2/2] Fix logic in rare case current_path fails to expand
---
lld/MachO/Driver.cpp | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 0f3db4ffa2797..f11c65a5583ad 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -1640,14 +1640,12 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
// path. Hence we've picked a reasonably large number here.
SmallString<1024> expanded;
// Expand "." into the current working directory.
- if (config->osoPrefix == ".") {
- if (!fs::current_path(expanded)) {
- // Note: LD64 expands "." to be `<current_dir>/
- // (ie., it has a slash suffix) whereas current_path() doesn't.
- // So we have to append '/' to be consistent because this is
- // meaningful for our text based stripping.
- expanded += sys::path::get_separator();
- }
+ if (config->osoPrefix == "." && !fs::current_path(expanded)) {
+ // Note: LD64 expands "." to be `<current_dir>/
+ // (ie., it has a slash suffix) whereas current_path() doesn't.
+ // So we have to append '/' to be consistent because this is
+ // meaningful for our text based stripping.
+ expanded += sys::path::get_separator();
} else {
expanded = config->osoPrefix;
}
More information about the llvm-commits
mailing list