[lld] 20f51ff - [lld-macho] Have --reproduce account for path rerooting
Jez Ng via llvm-commits
llvm-commits at lists.llvm.org
Wed May 5 11:42:13 PDT 2021
Author: Jez Ng
Date: 2021-05-05T14:41:01-04:00
New Revision: 20f51ffe67d12ab72c917dc4b371b55c80321393
URL: https://github.com/llvm/llvm-project/commit/20f51ffe67d12ab72c917dc4b371b55c80321393
DIFF: https://github.com/llvm/llvm-project/commit/20f51ffe67d12ab72c917dc4b371b55c80321393.diff
LOG: [lld-macho] Have --reproduce account for path rerooting
We need to account for path rerooting when generating the response
file. We could either reroot the paths before generating the file, or pass
through the original filenames and change just the syslibroot. I've opted for
the latter, in order that the reproduction run more closely mirrors the
original.
We must also be careful *not* to make an absolute path relative if it is
shadowed by a rerooted path. See repro6.tar in reroot-path.s for
details.
I've moved the call to `createResponseFile()` after the initialization of
`config->systemLibraryRoots`, since it now needs to know what those roots are.
Reviewed By: #lld-macho, oontvoo
Differential Revision: https://reviews.llvm.org/D101224
Added:
Modified:
lld/MachO/Driver.cpp
lld/MachO/Driver.h
lld/MachO/DriverUtils.cpp
lld/test/MachO/reroot-path.s
Removed:
################################################################################
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 133d3ba6e8e4c..e1efac0b7ec1d 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -77,26 +77,6 @@ static HeaderFileType getOutputType(const InputArgList &args) {
}
}
-// Search for all possible combinations of `{root}/{name}.{extension}`.
-// If \p extensions are not specified, then just search for `{root}/{name}`.
-static Optional<StringRef>
-findPathCombination(const Twine &name, const std::vector<StringRef> &roots,
- ArrayRef<StringRef> extensions = {""}) {
- SmallString<261> base;
- for (StringRef dir : roots) {
- base = dir;
- path::append(base, name);
- for (StringRef ext : extensions) {
- Twine location = base + ext;
- if (fs::exists(location))
- return saver.save(location.str());
- else
- depTracker->logFileNotFound(location);
- }
- }
- return {};
-}
-
static Optional<StringRef> findLibrary(StringRef name) {
if (config->searchDylibsFirst) {
if (Optional<StringRef> path = findPathCombination(
@@ -109,19 +89,6 @@ static Optional<StringRef> findLibrary(StringRef name) {
{".tbd", ".dylib", ".a"});
}
-// If -syslibroot is specified, absolute paths to non-object files may be
-// rerooted.
-static StringRef rerootPath(StringRef path) {
- if (!path::is_absolute(path, path::Style::posix) || path.endswith(".o"))
- return path;
-
- if (Optional<StringRef> rerootedPath =
- findPathCombination(path, config->systemLibraryRoots))
- return *rerootedPath;
-
- return path;
-}
-
static Optional<std::string> findFramework(StringRef name) {
SmallString<260> symlink;
StringRef suffix;
@@ -244,7 +211,7 @@ static std::vector<ArchiveMember> getArchiveMembers(MemoryBufferRef mb) {
std::vector<ArchiveMember> v;
Error err = Error::success();
- // Thin archives refer to .o files, so --reproduces needs the .o files too.
+ // Thin archives refer to .o files, so --reproduce needs the .o files too.
bool addToTar = archive->isThin() && tar;
for (const Archive::Child &c : archive->children(err)) {
@@ -925,6 +892,13 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
return true;
}
+ config = make<Configuration>();
+ symtab = make<SymbolTable>();
+ target = createTargetInfo(args);
+ depTracker =
+ make<DependencyTracker>(args.getLastArgValue(OPT_dependency_info));
+
+ config->systemLibraryRoots = getSystemLibraryRoots(args);
if (const char *path = getReproduceOption(args)) {
// Note that --reproduce is a debug option so you can ignore it
// if you are trying to understand the whole picture of the code.
@@ -939,13 +913,6 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
}
}
- config = make<Configuration>();
- symtab = make<SymbolTable>();
- target = createTargetInfo(args);
-
- depTracker =
- make<DependencyTracker>(args.getLastArgValue(OPT_dependency_info, ""));
-
if (auto *arg = args.getLastArg(OPT_threads_eq)) {
StringRef v(arg->getValue());
unsigned threads = 0;
@@ -1035,7 +1002,6 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
config->undefinedSymbolTreatment = getUndefinedSymbolTreatment(args);
- config->systemLibraryRoots = getSystemLibraryRoots(args);
config->librarySearchPaths =
getLibrarySearchPaths(args, config->systemLibraryRoots);
config->frameworkSearchPaths =
diff --git a/lld/MachO/Driver.h b/lld/MachO/Driver.h
index c1b677a8892e2..72f00cd1237ab 100644
--- a/lld/MachO/Driver.h
+++ b/lld/MachO/Driver.h
@@ -57,6 +57,17 @@ llvm::Optional<DylibFile *> loadDylib(llvm::MemoryBufferRef mbref,
DylibFile *umbrella = nullptr,
bool isBundleLoader = false);
+// Search for all possible combinations of `{root}/{name}.{extension}`.
+// If \p extensions are not specified, then just search for `{root}/{name}`.
+llvm::Optional<llvm::StringRef>
+findPathCombination(const llvm::Twine &name,
+ const std::vector<llvm::StringRef> &roots,
+ ArrayRef<llvm::StringRef> extensions = {""});
+
+// If -syslibroot is specified, absolute paths to non-object files may be
+// rerooted.
+llvm::StringRef rerootPath(llvm::StringRef path);
+
llvm::Optional<InputFile *> loadArchiveMember(MemoryBufferRef, uint32_t modTime,
StringRef archiveName,
bool objCOnly);
diff --git a/lld/MachO/DriverUtils.cpp b/lld/MachO/DriverUtils.cpp
index e0f6a353c03ec..d897eefea751a 100644
--- a/lld/MachO/DriverUtils.cpp
+++ b/lld/MachO/DriverUtils.cpp
@@ -118,6 +118,14 @@ static std::string rewritePath(StringRef s) {
return std::string(s);
}
+static std::string rewriteInputPath(StringRef s) {
+ // Don't bother rewriting "absolute" paths that are actually under the
+ // syslibroot; simply rewriting the syslibroot is sufficient.
+ if (rerootPath(s) == s && fs::exists(s))
+ return relativeToRoot(s);
+ return std::string(s);
+}
+
// Reconstructs command line arguments so that so that you can re-run
// the same command with the same inputs. This is for --reproduce.
std::string macho::createResponseFile(const InputArgList &args) {
@@ -130,7 +138,7 @@ std::string macho::createResponseFile(const InputArgList &args) {
case OPT_reproduce:
break;
case OPT_INPUT:
- os << quote(rewritePath(arg->getValue())) << "\n";
+ os << quote(rewriteInputPath(arg->getValue())) << "\n";
break;
case OPT_o:
os << "-o " << quote(path::filename(arg->getValue())) << "\n";
@@ -138,13 +146,17 @@ std::string macho::createResponseFile(const InputArgList &args) {
case OPT_filelist:
if (Optional<MemoryBufferRef> buffer = readFile(arg->getValue()))
for (StringRef path : args::getLines(*buffer))
- os << quote(rewritePath(path)) << "\n";
+ os << quote(rewriteInputPath(path)) << "\n";
+ break;
+ case OPT_force_load:
+ case OPT_weak_library:
+ os << arg->getSpelling() << " "
+ << quote(rewriteInputPath(arg->getValue())) << "\n";
break;
case OPT_F:
case OPT_L:
case OPT_bundle_loader:
case OPT_exported_symbols_list:
- case OPT_force_load:
case OPT_order_file:
case OPT_rpath:
case OPT_syslibroot:
@@ -217,6 +229,36 @@ Optional<DylibFile *> macho::loadDylib(MemoryBufferRef mbref,
return file;
}
+Optional<StringRef>
+macho::findPathCombination(const Twine &name,
+ const std::vector<StringRef> &roots,
+ ArrayRef<StringRef> extensions) {
+ SmallString<261> base;
+ for (StringRef dir : roots) {
+ base = dir;
+ path::append(base, name);
+ for (StringRef ext : extensions) {
+ Twine location = base + ext;
+ if (fs::exists(location))
+ return saver.save(location.str());
+ else
+ depTracker->logFileNotFound(location);
+ }
+ }
+ return {};
+}
+
+StringRef macho::rerootPath(StringRef path) {
+ if (!path::is_absolute(path, path::Style::posix) || path.endswith(".o"))
+ return path;
+
+ if (Optional<StringRef> rerootedPath =
+ findPathCombination(path, config->systemLibraryRoots))
+ return *rerootedPath;
+
+ return path;
+}
+
Optional<InputFile *> macho::loadArchiveMember(MemoryBufferRef mb,
uint32_t modTime,
StringRef archiveName,
diff --git a/lld/test/MachO/reroot-path.s b/lld/test/MachO/reroot-path.s
index b70c72e14984c..ab3a018f70729 100644
--- a/lld/test/MachO/reroot-path.s
+++ b/lld/test/MachO/reroot-path.s
@@ -23,14 +23,30 @@
# RUN: %lld -dylib %t/bar.o -o %t/%:t/libbar.dylib
## Test our various file-loading flags to make sure all bases are covered.
-# RUN: %lld -lSystem -syslibroot %t %t/foo.a %t/bar.a %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
-# RUN: %lld -lSystem -syslibroot %t -force_load %t/foo.a -force_load %t/bar.a %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
-# RUN: %lld -lSystem -syslibroot %t %t/libfoo.dylib %t/libbar.dylib %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
-# RUN: %lld -lSystem -syslibroot %t -weak_library %t/libfoo.dylib -weak_library %t/libbar.dylib %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
+# RUN: %lld --reproduce %t/repro1.tar -lSystem -syslibroot %t %t/foo.a %t/bar.a %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
+# RUN: tar xf %t/repro1.tar -C %t
+# RUN: cd %t/repro1; ld64.lld @response.txt | FileCheck %s -DDIR="%:t/%:t"
+
+# RUN: %lld --reproduce %t/repro2.tar -lSystem -syslibroot %t -force_load %t/foo.a -force_load %t/bar.a %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
+# RUN: tar xf %t/repro2.tar -C %t
+# RUN: cd %t/repro2; ld64.lld @response.txt | FileCheck %s -DDIR="%:t/%:t"
+
+# RUN: %lld --reproduce %t/repro3.tar -lSystem -syslibroot %t %t/libfoo.dylib %t/libbar.dylib %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
+# RUN: tar xf %t/repro3.tar -C %t
+# RUN: cd %t/repro3; ld64.lld @response.txt | FileCheck %s -DDIR="%:t/%:t"
+
+# RUN: %lld --reproduce %t/repro4.tar -lSystem -syslibroot %t -weak_library %t/libfoo.dylib -weak_library %t/libbar.dylib %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
+# RUN: tar xf %t/repro4.tar -C %t
+# RUN: cd %t/repro4; ld64.lld @response.txt | FileCheck %s -DDIR="%:t/%:t"
+
# RUN: echo "%t/libfoo.dylib" > %t/filelist
# RUN: echo "%t/libbar.dylib" >> %t/filelist
-# RUN: %lld -lSystem -syslibroot %t -filelist %t/filelist %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
-# CHECK: [[DIR]]/{{(lib)?}}bar
+# RUN: %lld --reproduce %t/repro5.tar -lSystem -syslibroot %t -filelist %t/filelist %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
+# RUN: tar xf %t/repro5.tar -C %t
+# RUN: cd %t/repro5; ld64.lld @response.txt | FileCheck %s -DDIR="%:t/%:t"
+
+## The {{^}} ensures that we only match relative paths if DIR is relative.
+# CHECK: {{^}}[[DIR]]/{{(lib)?}}bar
## Paths to object files don't get rerooted.
# RUN: mv %t/bar.o %t/%:t/bar.o
@@ -42,8 +58,15 @@
## rerooted path takes precedence over the original path. We will get an
## undefined symbol error since we aren't loading %t/libfoo.dylib.
# RUN: cp %t/%:t/libbar.dylib %t/%:t/libfoo.dylib
-# RUN: not %lld -lSystem -syslibroot %t %t/libfoo.dylib %t/libbar.dylib %t/test.o \
-# RUN: -o /dev/null 2>&1 | FileCheck %s --check-prefix=UNDEF
+# RUN: not %lld --reproduce %t/repro6.tar -lSystem -syslibroot %t %t/libfoo.dylib %t/libbar.dylib %t/test.o \
+# RUN: -o /dev/null -t 2> %t/error | FileCheck %s -DDIR="%t/%:t" --check-prefix=FOO
+# RUN: FileCheck %s --check-prefix=UNDEF < %t/error
+
+# RUN: tar xf %t/repro6.tar -C %t
+# RUN: cd %t/repro6; not ld64.lld @response.txt 2> %t/error | FileCheck %s -DDIR="%:t/%:t" --check-prefix=FOO
+# RUN: FileCheck %s --check-prefix=UNDEF < %t/error
+
+# FOO: [[DIR]]/libfoo.dylib
# UNDEF: error: undefined symbol: _foo
#--- foo.s
More information about the llvm-commits
mailing list