[llvm] Reland [llvm] Errorize DebuginfodFetcher for inspection at call-sites (PR #194872)
Stefan Gränitz via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 1 07:51:39 PDT 2026
https://github.com/weliveindetail updated https://github.com/llvm/llvm-project/pull/194872
>From 500cafe0c68190035f902e700db7b588f6e68ad0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Tue, 21 Apr 2026 12:24:53 +0200
Subject: [PATCH 1/4] [llvm] Errorize DebuginfodFetcher for inspection at
call-sites
Failure to fetch debuginfod is rarely an error, but there are cases where
we want to distinguish error reasons down the line, for example in order
to test connection timeouts.
---
llvm/include/llvm/Debuginfod/BuildIDFetcher.h | 3 +--
llvm/include/llvm/Object/BuildID.h | 3 ++-
llvm/lib/DebugInfo/Symbolize/Symbolize.cpp | 7 +++++--
llvm/lib/Debuginfod/BuildIDFetcher.cpp | 20 ++++++++++---------
llvm/lib/Object/BuildID.cpp | 8 ++++++--
.../ProfileData/Coverage/CoverageMapping.cpp | 20 +++++++++----------
llvm/lib/ProfileData/InstrProfCorrelator.cpp | 11 +++++++---
.../llvm-debuginfod-find.cpp | 10 ++++++----
llvm/tools/llvm-objdump/llvm-objdump.cpp | 12 ++++++++---
9 files changed, 57 insertions(+), 37 deletions(-)
diff --git a/llvm/include/llvm/Debuginfod/BuildIDFetcher.h b/llvm/include/llvm/Debuginfod/BuildIDFetcher.h
index 8f9c2aa8722ad..66b6720a01226 100644
--- a/llvm/include/llvm/Debuginfod/BuildIDFetcher.h
+++ b/llvm/include/llvm/Debuginfod/BuildIDFetcher.h
@@ -16,7 +16,6 @@
#define LLVM_DEBUGINFOD_DIFETCHER_H
#include "llvm/Object/BuildID.h"
-#include <optional>
namespace llvm {
@@ -28,7 +27,7 @@ class DebuginfodFetcher : public object::BuildIDFetcher {
/// Fetches the given Build ID using debuginfod and returns a local path to
/// the resulting file.
- std::optional<std::string> fetch(object::BuildIDRef BuildID) const override;
+ Expected<std::string> fetch(object::BuildIDRef BuildID) const override;
};
} // namespace llvm
diff --git a/llvm/include/llvm/Object/BuildID.h b/llvm/include/llvm/Object/BuildID.h
index 65ba00f75dd6f..73b1a0784da13 100644
--- a/llvm/include/llvm/Object/BuildID.h
+++ b/llvm/include/llvm/Object/BuildID.h
@@ -18,6 +18,7 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Compiler.h"
+#include "llvm/Support/Error.h"
namespace llvm {
namespace object {
@@ -44,7 +45,7 @@ class LLVM_ABI BuildIDFetcher {
virtual ~BuildIDFetcher() = default;
/// Returns the path to the debug file with the given build ID.
- virtual std::optional<std::string> fetch(BuildIDRef BuildID) const;
+ virtual Expected<std::string> fetch(BuildIDRef BuildID) const;
private:
const std::vector<std::string> DebugFileDirectories;
diff --git a/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp b/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
index 3821f53d26b98..cc818552c8e9c 100644
--- a/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
+++ b/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
@@ -490,14 +490,17 @@ bool LLVMSymbolizer::getOrFindDebugBinary(const ArrayRef<uint8_t> BuildID,
}
if (!BIDFetcher)
return false;
- if (std::optional<std::string> Path = BIDFetcher->fetch(BuildID)) {
+ Expected<std::string> Path = BIDFetcher->fetch(BuildID);
+ if (Path) {
Result = *Path;
auto InsertResult = BuildIDPaths.insert({BuildIDStr, Result});
assert(InsertResult.second);
(void)InsertResult;
return true;
}
-
+ // Failure to fetch debuginfod is rarely an error and most users will not care
+ // why this failed.
+ consumeError(Path.takeError());
return false;
}
diff --git a/llvm/lib/Debuginfod/BuildIDFetcher.cpp b/llvm/lib/Debuginfod/BuildIDFetcher.cpp
index a7f13104abee1..550ee4995008b 100644
--- a/llvm/lib/Debuginfod/BuildIDFetcher.cpp
+++ b/llvm/lib/Debuginfod/BuildIDFetcher.cpp
@@ -15,17 +15,19 @@
#include "llvm/Debuginfod/BuildIDFetcher.h"
#include "llvm/Debuginfod/Debuginfod.h"
+#include "llvm/Support/Error.h"
using namespace llvm;
-std::optional<std::string>
+Expected<std::string>
DebuginfodFetcher::fetch(ArrayRef<uint8_t> BuildID) const {
- if (std::optional<std::string> Path = BuildIDFetcher::fetch(BuildID))
- return std::move(*Path);
-
- Expected<std::string> PathOrErr = getCachedOrDownloadDebuginfo(BuildID);
- if (PathOrErr)
- return *PathOrErr;
- consumeError(PathOrErr.takeError());
- return std::nullopt;
+ Expected<std::string> Path = BuildIDFetcher::fetch(BuildID);
+ if (Path)
+ return Path;
+ // Most users will not care why this failed.
+ assert(errorToErrorCode(Path.takeError()) ==
+ std::errc::no_such_file_or_directory &&
+ "BuildIDFetcher::fetch() failed in an unexpected way");
+ consumeError(Path.takeError());
+ return getCachedOrDownloadDebuginfo(BuildID);
}
diff --git a/llvm/lib/Object/BuildID.cpp b/llvm/lib/Object/BuildID.cpp
index d1ee597a11327..591c3a41e1df7 100644
--- a/llvm/lib/Object/BuildID.cpp
+++ b/llvm/lib/Object/BuildID.cpp
@@ -15,6 +15,7 @@
#include "llvm/Object/BuildID.h"
#include "llvm/Object/ELFObjectFile.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
@@ -79,7 +80,7 @@ BuildIDRef llvm::object::getBuildID(const ObjectFile *Obj) {
return {};
}
-std::optional<std::string> BuildIDFetcher::fetch(BuildIDRef BuildID) const {
+Expected<std::string> BuildIDFetcher::fetch(BuildIDRef BuildID) const {
auto GetDebugPath = [&](StringRef Directory) {
SmallString<128> Path{Directory};
sys::path::append(Path, ".build-id",
@@ -108,5 +109,8 @@ std::optional<std::string> BuildIDFetcher::fetch(BuildIDRef BuildID) const {
return std::string(Path);
}
}
- return std::nullopt;
+ return createStringError(
+ make_error_code(std::errc::no_such_file_or_directory),
+ "could not find debug file for build ID '" +
+ llvm::toHex(BuildID, /*LowerCase=*/true) + "'");
}
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 4ce78fe6e3182..258db23030bb9 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -1092,20 +1092,18 @@ Expected<std::unique_ptr<CoverageMapping>> CoverageMapping::load(
}
for (object::BuildIDRef BinaryID : BinaryIDsToFetch) {
- std::optional<std::string> PathOpt = BIDFetcher->fetch(BinaryID);
- if (PathOpt) {
- std::string Path = std::move(*PathOpt);
+ Expected<std::string> Path = BIDFetcher->fetch(BinaryID);
+ if (Path) {
StringRef Arch = Arches.size() == 1 ? Arches.front() : StringRef();
- if (Error E = loadFromFile(Path, Arch, CompilationDir, ProfileReaderRef,
- *Coverage, DataFound))
+ if (Error E = loadFromFile(*Path, Arch, CompilationDir,
+ ProfileReaderRef, *Coverage, DataFound))
return std::move(E);
- } else if (CheckBinaryIDs) {
- return createFileError(
- ProfileFilename.value(),
- createStringError(errc::no_such_file_or_directory,
- "Missing binary ID: " +
- llvm::toHex(BinaryID, /*LowerCase=*/true)));
}
+ if (CheckBinaryIDs) {
+ return createFileError(ProfileFilename.value(), Path.takeError());
+ }
+ // Ignore error and continue.
+ consumeError(Path.takeError());
}
}
diff --git a/llvm/lib/ProfileData/InstrProfCorrelator.cpp b/llvm/lib/ProfileData/InstrProfCorrelator.cpp
index b38189de31606..faf98a3a7e28e 100644
--- a/llvm/lib/ProfileData/InstrProfCorrelator.cpp
+++ b/llvm/lib/ProfileData/InstrProfCorrelator.cpp
@@ -114,7 +114,6 @@ llvm::Expected<std::unique_ptr<InstrProfCorrelator>>
InstrProfCorrelator::get(StringRef Filename, ProfCorrelatorKind FileKind,
const object::BuildIDFetcher *BIDFetcher,
const ArrayRef<object::BuildID> BIs) {
- std::optional<std::string> Path;
if (BIDFetcher) {
if (BIs.empty())
return make_error<InstrProfError>(
@@ -127,12 +126,18 @@ InstrProfCorrelator::get(StringRef Filename, ProfCorrelatorKind FileKind,
"unsupported profile binary correlation when there are multiple "
"build IDs in a profile");
- Path = BIDFetcher->fetch(BIs.front());
- if (!Path)
+ Expected<std::string> Path = BIDFetcher->fetch(BIs.front());
+ if (!Path) {
+ // Propagate as InstrProf specific error type.
+ assert(errorToErrorCode(Path.takeError()) ==
+ std::errc::no_such_file_or_directory &&
+ "BuildIDFetcher::fetch() failed in an unexpected way");
+ consumeError(Path.takeError());
return make_error<InstrProfError>(
instrprof_error::unable_to_correlate_profile,
"Missing build ID: " + llvm::toHex(BIs.front(),
/*LowerCase=*/true));
+ }
Filename = *Path;
}
diff --git a/llvm/tools/llvm-debuginfod-find/llvm-debuginfod-find.cpp b/llvm/tools/llvm-debuginfod-find/llvm-debuginfod-find.cpp
index 8f1b9d0f4659d..a2658fdd9f74e 100644
--- a/llvm/tools/llvm-debuginfod-find/llvm-debuginfod-find.cpp
+++ b/llvm/tools/llvm-debuginfod-find/llvm-debuginfod-find.cpp
@@ -152,10 +152,12 @@ int llvm_debuginfod_find_main(int argc, char **argv,
// Find a debug file in local build ID directories and via debuginfod.
std::string fetchDebugInfo(object::BuildIDRef BuildID) {
- if (std::optional<std::string> Path =
- DebuginfodFetcher(DebugFileDirectory).fetch(BuildID))
- return *Path;
- errs() << "Build ID " << llvm::toHex(BuildID, /*Lowercase=*/true)
+ Expected<std::string> PathOrErr =
+ DebuginfodFetcher(DebugFileDirectory).fetch(BuildID);
+ if (PathOrErr)
+ return *PathOrErr;
+ errs() << "Build ID " << llvm::toHex(BuildID, /*Lowercase=*/true) << ": "
<< " could not be found.\n";
+ consumeError(PathOrErr.takeError());
exit(1);
}
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
index 3814104153cb7..0347c7ceac835 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -1744,9 +1744,13 @@ fetchBinaryByBuildID(const ObjectFile &Obj) {
object::BuildIDRef BuildID = getBuildID(&Obj);
if (BuildID.empty())
return std::nullopt;
- std::optional<std::string> Path = BIDFetcher->fetch(BuildID);
- if (!Path)
+ Expected<std::string> Path = BIDFetcher->fetch(BuildID);
+ if (!Path) {
+ // Failure to fetch debuginfod is rarely an error and most users will not
+ // care why this failed.
+ consumeError(Path.takeError());
return std::nullopt;
+ }
Expected<OwningBinary<Binary>> DebugBinary = createBinary(*Path);
if (!DebugBinary) {
reportWarning(toString(DebugBinary.takeError()), *Path);
@@ -3842,8 +3846,10 @@ static void parseObjdumpOptions(const llvm::opt::InputArgList &InputArgs) {
// Look up any provided build IDs, then append them to the input filenames.
for (const opt::Arg *A : InputArgs.filtered(OBJDUMP_build_id)) {
object::BuildID BuildID = parseBuildIDArg(A);
- std::optional<std::string> Path = BIDFetcher->fetch(BuildID);
+ Expected<std::string> Path = BIDFetcher->fetch(BuildID);
if (!Path) {
+ // Most users will not care why this failed.
+ consumeError(Path.takeError());
reportCmdLineError(A->getSpelling() + ": could not find build ID '" +
A->getValue() + "'");
}
>From 5ac8bf845fb123765bc256bd7e6c1639948f73cd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Tue, 21 Apr 2026 14:21:13 +0200
Subject: [PATCH 2/4] [llvm-cov] Fix error propagation in
CoverageMapping::load()
Fix a subtle issue on the error path: if loadFromFile() fails there is no error to consume.
---
.../ProfileData/Coverage/CoverageMapping.cpp | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 258db23030bb9..9dd2e7bf7fd7e 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -1092,18 +1092,22 @@ Expected<std::unique_ptr<CoverageMapping>> CoverageMapping::load(
}
for (object::BuildIDRef BinaryID : BinaryIDsToFetch) {
- Expected<std::string> Path = BIDFetcher->fetch(BinaryID);
- if (Path) {
+ if (Expected<std::string> Path = BIDFetcher->fetch(BinaryID)) {
StringRef Arch = Arches.size() == 1 ? Arches.front() : StringRef();
if (Error E = loadFromFile(*Path, Arch, CompilationDir,
ProfileReaderRef, *Coverage, DataFound))
return std::move(E);
+ } else {
+ // Conditionally propagate as new error.
+ consumeError(Path.takeError());
+ if (CheckBinaryIDs) {
+ return createFileError(
+ ProfileFilename.value(),
+ createStringError(errc::no_such_file_or_directory,
+ "Missing binary ID: " +
+ llvm::toHex(BinaryID, /*LowerCase=*/true)));
+ }
}
- if (CheckBinaryIDs) {
- return createFileError(ProfileFilename.value(), Path.takeError());
- }
- // Ignore error and continue.
- consumeError(Path.takeError());
}
}
>From 5f8569c069ea5e34039df1ad9be9957c4f0e8260 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Wed, 29 Apr 2026 16:32:34 +0200
Subject: [PATCH 3/4] Drop assertion in InstrProfCorrelator::get()
---
llvm/lib/ProfileData/InstrProfCorrelator.cpp | 3 ---
1 file changed, 3 deletions(-)
diff --git a/llvm/lib/ProfileData/InstrProfCorrelator.cpp b/llvm/lib/ProfileData/InstrProfCorrelator.cpp
index faf98a3a7e28e..d7ad8762f79fc 100644
--- a/llvm/lib/ProfileData/InstrProfCorrelator.cpp
+++ b/llvm/lib/ProfileData/InstrProfCorrelator.cpp
@@ -129,9 +129,6 @@ InstrProfCorrelator::get(StringRef Filename, ProfCorrelatorKind FileKind,
Expected<std::string> Path = BIDFetcher->fetch(BIs.front());
if (!Path) {
// Propagate as InstrProf specific error type.
- assert(errorToErrorCode(Path.takeError()) ==
- std::errc::no_such_file_or_directory &&
- "BuildIDFetcher::fetch() failed in an unexpected way");
consumeError(Path.takeError());
return make_error<InstrProfError>(
instrprof_error::unable_to_correlate_profile,
>From 2961e58f88a41893887e9fc01576048b04f204ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Wed, 1 Jul 2026 16:49:32 +0200
Subject: [PATCH 4/4] [llvm-profdata] Fix invalid assignment from std::string
to StringRef
---
llvm/lib/ProfileData/InstrProfCorrelator.cpp | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/ProfileData/InstrProfCorrelator.cpp b/llvm/lib/ProfileData/InstrProfCorrelator.cpp
index d7ad8762f79fc..62a758956d365 100644
--- a/llvm/lib/ProfileData/InstrProfCorrelator.cpp
+++ b/llvm/lib/ProfileData/InstrProfCorrelator.cpp
@@ -114,6 +114,8 @@ llvm::Expected<std::unique_ptr<InstrProfCorrelator>>
InstrProfCorrelator::get(StringRef Filename, ProfCorrelatorKind FileKind,
const object::BuildIDFetcher *BIDFetcher,
const ArrayRef<object::BuildID> BIs) {
+ // Might be overwritten from BuildIDFetcher.
+ std::string EffectiveFilename = Filename.str();
if (BIDFetcher) {
if (BIs.empty())
return make_error<InstrProfError>(
@@ -135,12 +137,12 @@ InstrProfCorrelator::get(StringRef Filename, ProfCorrelatorKind FileKind,
"Missing build ID: " + llvm::toHex(BIs.front(),
/*LowerCase=*/true));
}
- Filename = *Path;
+ EffectiveFilename = *Path;
}
if (FileKind == DEBUG_INFO) {
auto DsymObjectsOrErr =
- object::MachOObjectFile::findDsymObjectMembers(Filename);
+ object::MachOObjectFile::findDsymObjectMembers(EffectiveFilename);
if (auto Err = DsymObjectsOrErr.takeError())
return std::move(Err);
if (!DsymObjectsOrErr->empty()) {
@@ -150,16 +152,18 @@ InstrProfCorrelator::get(StringRef Filename, ProfCorrelatorKind FileKind,
return make_error<InstrProfError>(
instrprof_error::unable_to_correlate_profile,
"using multiple objects is not yet supported");
- Filename = *DsymObjectsOrErr->begin();
+ EffectiveFilename = *DsymObjectsOrErr->begin();
}
- auto BufferOrErr = errorOrToExpected(MemoryBuffer::getFile(Filename));
+ auto BufferOrErr =
+ errorOrToExpected(MemoryBuffer::getFile(EffectiveFilename));
if (auto Err = BufferOrErr.takeError())
return std::move(Err);
return get(std::move(*BufferOrErr), FileKind);
}
if (FileKind == BINARY) {
- auto BufferOrErr = errorOrToExpected(MemoryBuffer::getFile(Filename));
+ auto BufferOrErr =
+ errorOrToExpected(MemoryBuffer::getFile(EffectiveFilename));
if (auto Err = BufferOrErr.takeError())
return std::move(Err);
More information about the llvm-commits
mailing list