[llvm] [dsymutil] Share one BinaryHolder between debug map parsing & linking (PR #113234)
Jonas Devlieghere via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 21 17:01:26 PDT 2024
https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/113234
I (re)discovered that dsymutil was instantiating two BinaryHolders: one for parsing the debug map and one for linking. That really defeats the purpose of the BinaryHolder as it serves as a cache. Fix the issue and remove an old FIXME.
>From 0b225999f6738da7a107c29d3d0a0fa4e33d1b7b Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Mon, 21 Oct 2024 16:59:17 -0700
Subject: [PATCH] [dsymutil] Share one BinaryHolder between debug map parsing &
linking
I (re)discovered that dsymutil was instantiating two BinaryHolders: one
for parsing the debug map and one for linking. That really defeats the
purpose of the BinaryHolder as it serves as a cache. Fix the issue and
remove an old FIXME.
---
llvm/tools/dsymutil/DebugMap.cpp | 14 +++++------
llvm/tools/dsymutil/DebugMap.h | 4 +++-
llvm/tools/dsymutil/MachODebugMapParser.cpp | 23 +++++++++---------
llvm/tools/dsymutil/dsymutil.cpp | 26 ++++++---------------
llvm/tools/dsymutil/dsymutil.h | 9 +++----
5 files changed, 34 insertions(+), 42 deletions(-)
diff --git a/llvm/tools/dsymutil/DebugMap.cpp b/llvm/tools/dsymutil/DebugMap.cpp
index 8724b70422f326..b38d502dda7c97 100644
--- a/llvm/tools/dsymutil/DebugMap.cpp
+++ b/llvm/tools/dsymutil/DebugMap.cpp
@@ -126,6 +126,9 @@ void DebugMap::dump() const { print(errs()); }
namespace {
struct YAMLContext {
+ YAMLContext(BinaryHolder &BinHolder, StringRef PrependPath)
+ : BinHolder(BinHolder), PrependPath(PrependPath) {}
+ BinaryHolder &BinHolder;
StringRef PrependPath;
Triple BinaryTriple;
};
@@ -133,15 +136,13 @@ struct YAMLContext {
} // end anonymous namespace
ErrorOr<std::vector<std::unique_ptr<DebugMap>>>
-DebugMap::parseYAMLDebugMap(StringRef InputFile, StringRef PrependPath,
- bool Verbose) {
+DebugMap::parseYAMLDebugMap(BinaryHolder &BinHolder, StringRef InputFile,
+ StringRef PrependPath, bool Verbose) {
auto ErrOrFile = MemoryBuffer::getFileOrSTDIN(InputFile);
if (auto Err = ErrOrFile.getError())
return Err;
- YAMLContext Ctxt;
-
- Ctxt.PrependPath = PrependPath;
+ YAMLContext Ctxt(BinHolder, PrependPath);
std::unique_ptr<DebugMap> Res;
yaml::Input yin((*ErrOrFile)->getBuffer(), &Ctxt);
@@ -244,14 +245,13 @@ MappingTraits<dsymutil::DebugMapObject>::YamlDMO::YamlDMO(
dsymutil::DebugMapObject
MappingTraits<dsymutil::DebugMapObject>::YamlDMO::denormalize(IO &IO) {
- BinaryHolder BinHolder(vfs::getRealFileSystem(), /* Verbose =*/false);
const auto &Ctxt = *reinterpret_cast<YAMLContext *>(IO.getContext());
SmallString<80> Path(Ctxt.PrependPath);
StringMap<uint64_t> SymbolAddresses;
sys::path::append(Path, Filename);
- auto ObjectEntry = BinHolder.getObjectEntry(Path);
+ auto ObjectEntry = Ctxt.BinHolder.getObjectEntry(Path);
if (!ObjectEntry) {
auto Err = ObjectEntry.takeError();
WithColor::warning() << "Unable to open " << Path << " "
diff --git a/llvm/tools/dsymutil/DebugMap.h b/llvm/tools/dsymutil/DebugMap.h
index 9c3a698fa1191d..8e2a4de94c89ef 100644
--- a/llvm/tools/dsymutil/DebugMap.h
+++ b/llvm/tools/dsymutil/DebugMap.h
@@ -21,6 +21,7 @@
#ifndef LLVM_TOOLS_DSYMUTIL_DEBUGMAP_H
#define LLVM_TOOLS_DSYMUTIL_DEBUGMAP_H
+#include "BinaryHolder.h"
#include "RelocationMap.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/StringMap.h"
@@ -127,7 +128,8 @@ class DebugMap {
/// Read a debug map for \a InputFile.
static ErrorOr<std::vector<std::unique_ptr<DebugMap>>>
- parseYAMLDebugMap(StringRef InputFile, StringRef PrependPath, bool Verbose);
+ parseYAMLDebugMap(BinaryHolder &BinHolder, StringRef InputFile,
+ StringRef PrependPath, bool Verbose);
};
/// The DebugMapObject represents one object file described by the DebugMap. It
diff --git a/llvm/tools/dsymutil/MachODebugMapParser.cpp b/llvm/tools/dsymutil/MachODebugMapParser.cpp
index 4a06731946b97d..a0ba2512e12f20 100644
--- a/llvm/tools/dsymutil/MachODebugMapParser.cpp
+++ b/llvm/tools/dsymutil/MachODebugMapParser.cpp
@@ -27,14 +27,14 @@ using namespace llvm::object;
class MachODebugMapParser {
public:
- MachODebugMapParser(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
- StringRef BinaryPath, ArrayRef<std::string> Archs,
+ MachODebugMapParser(BinaryHolder &BinHolder, StringRef BinaryPath,
+ ArrayRef<std::string> Archs,
ArrayRef<std::string> DSYMSearchPaths,
StringRef PathPrefix = "", StringRef VariantSuffix = "",
bool Verbose = false)
: BinaryPath(std::string(BinaryPath)), Archs(Archs),
DSYMSearchPaths(DSYMSearchPaths), PathPrefix(std::string(PathPrefix)),
- VariantSuffix(std::string(VariantSuffix)), BinHolder(VFS, Verbose),
+ VariantSuffix(std::string(VariantSuffix)), BinHolder(BinHolder),
CurrentDebugMapObject(nullptr), SkipDebugMapObject(false) {}
/// Parses and returns the DebugMaps of the input binary. The binary contains
@@ -56,7 +56,7 @@ class MachODebugMapParser {
std::string VariantSuffix;
/// Owns the MemoryBuffer for the main binary.
- BinaryHolder BinHolder;
+ BinaryHolder &BinHolder;
/// Map of the binary symbol addresses.
StringMap<uint64_t> MainBinarySymbolAddresses;
StringRef MainBinaryStrings;
@@ -854,24 +854,25 @@ void MachODebugMapParser::loadMainBinarySymbols(
namespace llvm {
namespace dsymutil {
llvm::ErrorOr<std::vector<std::unique_ptr<DebugMap>>>
-parseDebugMap(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
- StringRef InputFile, ArrayRef<std::string> Archs,
+parseDebugMap(BinaryHolder &BinHolder, StringRef InputFile,
+ ArrayRef<std::string> Archs,
ArrayRef<std::string> DSYMSearchPaths, StringRef PrependPath,
StringRef VariantSuffix, bool Verbose, bool InputIsYAML) {
if (InputIsYAML)
- return DebugMap::parseYAMLDebugMap(InputFile, PrependPath, Verbose);
+ return DebugMap::parseYAMLDebugMap(BinHolder, InputFile, PrependPath,
+ Verbose);
- MachODebugMapParser Parser(VFS, InputFile, Archs, DSYMSearchPaths,
+ MachODebugMapParser Parser(BinHolder, InputFile, Archs, DSYMSearchPaths,
PrependPath, VariantSuffix, Verbose);
return Parser.parse();
}
-bool dumpStab(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
- StringRef InputFile, ArrayRef<std::string> Archs,
+bool dumpStab(BinaryHolder &BinHolder, StringRef InputFile,
+ ArrayRef<std::string> Archs,
ArrayRef<std::string> DSYMSearchPaths, StringRef PrependPath,
StringRef VariantSuffix) {
- MachODebugMapParser Parser(VFS, InputFile, Archs, DSYMSearchPaths,
+ MachODebugMapParser Parser(BinHolder, InputFile, Archs, DSYMSearchPaths,
PrependPath, VariantSuffix, false);
return Parser.dumpStab();
}
diff --git a/llvm/tools/dsymutil/dsymutil.cpp b/llvm/tools/dsymutil/dsymutil.cpp
index 364a7d63d486e1..f510fa7f75bd87 100644
--- a/llvm/tools/dsymutil/dsymutil.cpp
+++ b/llvm/tools/dsymutil/dsymutil.cpp
@@ -180,17 +180,6 @@ static Error verifyOptions(const DsymutilOptions &Options) {
errc::invalid_argument);
}
- if (Options.LinkOpts.Update && llvm::is_contained(Options.InputFiles, "-")) {
- // FIXME: We cannot use stdin for an update because stdin will be
- // consumed by the BinaryHolder during the debugmap parsing, and
- // then we will want to consume it again in DwarfLinker. If we
- // used a unique BinaryHolder object that could cache multiple
- // binaries this restriction would go away.
- return make_error<StringError>(
- "standard input cannot be used as input for a dSYM update.",
- errc::invalid_argument);
- }
-
if (!Options.Flat && Options.OutputFile == "-")
return make_error<StringError>(
"cannot emit to standard output without --flat.",
@@ -674,9 +663,12 @@ int dsymutil_main(int argc, char **argv, const llvm::ToolContext &) {
}
for (auto &InputFile : Options.InputFiles) {
+ // Shared a single binary holder for all the link steps.
+ BinaryHolder BinHolder(Options.LinkOpts.VFS, Options.LinkOpts.Verbose);
+
// Dump the symbol table for each input file and requested arch
if (Options.DumpStab) {
- if (!dumpStab(Options.LinkOpts.VFS, InputFile, Options.Archs,
+ if (!dumpStab(BinHolder, InputFile, Options.Archs,
Options.LinkOpts.DSYMSearchPaths,
Options.LinkOpts.PrependPath,
Options.LinkOpts.BuildVariantSuffix))
@@ -685,10 +677,9 @@ int dsymutil_main(int argc, char **argv, const llvm::ToolContext &) {
}
auto DebugMapPtrsOrErr = parseDebugMap(
- Options.LinkOpts.VFS, InputFile, Options.Archs,
- Options.LinkOpts.DSYMSearchPaths, Options.LinkOpts.PrependPath,
- Options.LinkOpts.BuildVariantSuffix, Options.LinkOpts.Verbose,
- Options.InputIsYAMLDebugMap);
+ BinHolder, InputFile, Options.Archs, Options.LinkOpts.DSYMSearchPaths,
+ Options.LinkOpts.PrependPath, Options.LinkOpts.BuildVariantSuffix,
+ Options.LinkOpts.Verbose, Options.InputIsYAMLDebugMap);
if (auto EC = DebugMapPtrsOrErr.getError()) {
WithColor::error() << "cannot parse the debug map for '" << InputFile
@@ -714,9 +705,6 @@ int dsymutil_main(int argc, char **argv, const llvm::ToolContext &) {
return EXIT_FAILURE;
}
- // Shared a single binary holder for all the link steps.
- BinaryHolder BinHolder(Options.LinkOpts.VFS);
-
// Compute the output location and update the resource directory.
Expected<OutputLocation> OutputLocationOrErr =
getOutputFileName(InputFile, Options);
diff --git a/llvm/tools/dsymutil/dsymutil.h b/llvm/tools/dsymutil/dsymutil.h
index 5504dd57c7e558..7b97b8bcd3a250 100644
--- a/llvm/tools/dsymutil/dsymutil.h
+++ b/llvm/tools/dsymutil/dsymutil.h
@@ -16,6 +16,7 @@
#ifndef LLVM_TOOLS_DSYMUTIL_DSYMUTIL_H
#define LLVM_TOOLS_DSYMUTIL_DSYMUTIL_H
+#include "BinaryHolder.h"
#include "DebugMap.h"
#include "LinkUtils.h"
#include "llvm/ADT/ArrayRef.h"
@@ -33,14 +34,14 @@ namespace dsymutil {
/// The file has to be a MachO object file. Multiple debug maps can be
/// returned when the file is universal (aka fat) binary.
ErrorOr<std::vector<std::unique_ptr<DebugMap>>>
-parseDebugMap(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
- StringRef InputFile, ArrayRef<std::string> Archs,
+parseDebugMap(BinaryHolder &BinHolder, StringRef InputFile,
+ ArrayRef<std::string> Archs,
ArrayRef<std::string> DSYMSearchPaths, StringRef PrependPath,
StringRef VariantSuffix, bool Verbose, bool InputIsYAML);
/// Dump the symbol table.
-bool dumpStab(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
- StringRef InputFile, ArrayRef<std::string> Archs,
+bool dumpStab(BinaryHolder &BinHolder, StringRef InputFile,
+ ArrayRef<std::string> Archs,
ArrayRef<std::string> DSYMSearchPaths, StringRef PrependPath = "",
StringRef VariantSuffix = "");
More information about the llvm-commits
mailing list