Skip to content

[lld-macho] Process OSO prefix only textually in both input and output #152063

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[lld-macho] Process OSO prefix only textually in both input and output #152063

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a60fdf7
[lld-macho] Process OSO prefix only textually in both input and output
drodriguez Aug 5, 2025
2c7e2af
Fix logic in rare case current_path fails to expand
drodriguez Aug 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 10 additions & 16 deletions lld/MachO/Driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1635,27 +1635,21 @@ 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 == "." && !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;
Copy link
Member

Choose a reason for hiding this comment

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

Can you clarify why you're dropping the check for other special characters in the path? (ie., ~)

Copy link
Contributor Author

@drodriguez drodriguez Aug 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The main reason is because ld64 doesn't do it, as you pointed in the comment.

The secondary reason is that

llvm-project/lld/MachO/SyntheticSections.cpp

Lines 1177 to 1196 in d478502

void SymtabSection::emitObjectFileStab(ObjFile *file) {
StabsEntry stab(N_OSO);
stab.sect = target->cpuSubtype;
SmallString<261> path(!file->archiveName.empty() ? file->archiveName
: file->getName());
std::error_code ec = sys::fs::make_absolute(path);
if (ec)
fatal("failed to get absolute path for " + path);
if (!file->archiveName.empty())
path.append({"(", file->getName(), ")"});
StringRef adjustedPath = saver().save(path.str());
adjustedPath.consume_front(config->osoPrefix);
stab.strx = stringTableSection.addString(adjustedPath);
stab.desc = 1;
stab.value = file->modTime;
stabs.emplace_back(std::move(stab));
}

uses make_absolute, which does not do the expansion of ., ~ or the contraction of .., but tries to remove the post-processed value of -oso_prefix as a string, and that causes divergences when the value of real_path(".") and make_absolute(".") are not the same (like it happens when the current working directory is part of a symlinked folder). Either both sides use real_path or both sides use make_absolute, but one cannot use different functions in each side.

I preferred to use current_path here (which is what make_absolute uses underneath) to match both sides, which seems to be more similar to what ld64 does under the hood, and avoids the file system access that real_path will need for each path component.

}
config->osoPrefix = saver().save(expanded.str());
}

bool pie = args.hasFlag(OPT_pie, OPT_no_pie, true);
Expand Down
16 changes: 13 additions & 3 deletions lld/test/MachO/stabs.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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{{$}}
Expand Down