[llvm-branch-commits] [llvm] [Object][ELF] Pass Error to WarningHandler (PR #191707)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Apr 16 06:59:17 PDT 2026
https://github.com/aokblast updated https://github.com/llvm/llvm-project/pull/191707
>From 33b0281bc9e8cb7f0f3b037917e62924f0f77af2 Mon Sep 17 00:00:00 2001
From: ShengYi Hung <aokblast at FreeBSD.org>
Date: Sun, 12 Apr 2026 20:13:20 +0800
Subject: [PATCH 1/2] [Support] Add WrappedError class
The error consumer filters duplicate errors based on a portion of the
error message. Introduce a new Error kind that carries a prefix string
to support this use case.
---
llvm/include/llvm/Support/Error.h | 47 ++++++++++++++++++++++++++++
llvm/lib/Support/Error.cpp | 6 ++++
llvm/unittests/Support/ErrorTest.cpp | 29 +++++++++++++++++
3 files changed, 82 insertions(+)
diff --git a/llvm/include/llvm/Support/Error.h b/llvm/include/llvm/Support/Error.h
index c9fd16fdb7c2b..8e414428c8286 100644
--- a/llvm/include/llvm/Support/Error.h
+++ b/llvm/include/llvm/Support/Error.h
@@ -1497,6 +1497,53 @@ inline Error unwrap(LLVMErrorRef ErrRef) {
reinterpret_cast<ErrorInfoBase *>(ErrRef)));
}
+class LLVM_ABI ContextualizedError : public ErrorInfo<ContextualizedError> {
+public:
+ ContextualizedError(std::unique_ptr<ErrorInfoBase> E,
+ const Twine &Prefix = "")
+ : Err(std::move(E)), Context(Prefix.str()) {}
+
+ void log(raw_ostream &OS) const override {
+ assert(Err && "Trying to log after takeError().");
+ if (Context.size())
+ OS << Context << ": ";
+ Err->log(OS);
+ }
+
+ Error takeError() { return Error(std::move(Err)); }
+
+ const std::string &getContext() const { return Context; }
+
+ std::string getMessageWithoutContext() const {
+ std::string Msg;
+ raw_string_ostream OS(Msg);
+ Err->log(OS);
+ return Msg;
+ }
+
+ std::error_code convertToErrorCode() const override;
+
+ static Error build(Error E, const Twine &Prefix) {
+ std::unique_ptr<ErrorInfoBase> Payload;
+ handleAllErrors(std::move(E),
+ [&](std::unique_ptr<ErrorInfoBase> EIB) -> Error {
+ Payload = std::move(EIB);
+ return Error::success();
+ });
+ return Error(std::unique_ptr<ContextualizedError>(
+ new ContextualizedError(std::move(Payload), Prefix)));
+ }
+
+ static char ID;
+
+private:
+ std::unique_ptr<ErrorInfoBase> Err;
+ std::string Context;
+};
+
+inline Error createContextualizedError(Error E, const Twine &Prefix = "") {
+ return ContextualizedError::build(std::move(E), Prefix);
+}
} // end namespace llvm
#endif // LLVM_SUPPORT_ERROR_H
diff --git a/llvm/lib/Support/Error.cpp b/llvm/lib/Support/Error.cpp
index c6743155ced85..aabb701b9b999 100644
--- a/llvm/lib/Support/Error.cpp
+++ b/llvm/lib/Support/Error.cpp
@@ -205,3 +205,9 @@ LLVMErrorTypeId LLVMGetStringErrorTypeId() {
LLVMErrorRef LLVMCreateStringError(const char *ErrMsg) {
return wrap(make_error<StringError>(ErrMsg, inconvertibleErrorCode()));
}
+
+char ContextualizedError::ID = 0;
+
+std::error_code ContextualizedError::convertToErrorCode() const {
+ return Err->convertToErrorCode();
+}
diff --git a/llvm/unittests/Support/ErrorTest.cpp b/llvm/unittests/Support/ErrorTest.cpp
index 45c0a4f450b51..4ce269d6b71b0 100644
--- a/llvm/unittests/Support/ErrorTest.cpp
+++ b/llvm/unittests/Support/ErrorTest.cpp
@@ -18,6 +18,8 @@
#include "gtest/gtest-spi.h"
#include "gtest/gtest.h"
#include <memory>
+#include <string>
+#include <unordered_set>
using namespace llvm;
@@ -1219,4 +1221,31 @@ TEST(Error, ForwardToExpected) {
EXPECT_THAT_ERROR(ExpectedReturningFct(false).moveInto(MaybeV), Succeeded());
EXPECT_EQ(*MaybeV, 42);
}
+
+TEST(Error, DeduplicateByContextualized) {
+ std::unordered_set<std::string> Visit;
+
+ auto InsertError = [&](Error E) {
+ std::string InnerErrorStr;
+ EXPECT_THAT_ERROR(handleErrors(std::move(E),
+ [&](const ContextualizedError &E) {
+ InnerErrorStr =
+ E.getMessageWithoutContext();
+ }),
+ Succeeded());
+ return Visit.insert(InnerErrorStr).second;
+ };
+ EXPECT_EQ(
+ InsertError(createContextualizedError(
+ createStringError("failed to execute operation A"), "Context A")),
+ true);
+ EXPECT_EQ(
+ InsertError(createContextualizedError(
+ createStringError("failed to execute operation B"), "Context A")),
+ true);
+ EXPECT_EQ(
+ InsertError(createContextualizedError(
+ createStringError("failed to execute operation A"), "Context B")),
+ false);
+}
} // namespace
>From 83ce7d9b2ad832489cd1bccc58df73986f82b80d Mon Sep 17 00:00:00 2001
From: ShengYi Hung <aokblast at FreeBSD.org>
Date: Sun, 12 Apr 2026 20:23:18 +0800
Subject: [PATCH 2/2] [Object][ELF] Pass Error to WarningHandler
Warning consumers may need to handle errors based on their type. Pass
the Error object instead of a string representation to enable this. This
also brings WarningHandler in line with Support/WithColor.h.
---
llvm/include/llvm/Object/ELF.h | 54 +++++++++++----------
llvm/lib/Object/ELF.cpp | 4 +-
llvm/tools/llvm-objdump/llvm-objdump.cpp | 9 ++--
llvm/tools/llvm-objdump/llvm-objdump.h | 2 +-
llvm/tools/llvm-readobj/ELFDumper.cpp | 4 +-
llvm/tools/llvm-readobj/ObjDumper.cpp | 7 +--
llvm/tools/llvm-readobj/ObjDumper.h | 2 +-
llvm/unittests/Object/ELFObjectFileTest.cpp | 4 +-
8 files changed, 46 insertions(+), 40 deletions(-)
diff --git a/llvm/include/llvm/Object/ELF.h b/llvm/include/llvm/Object/ELF.h
index e052519e27e8f..8f1d23b54622e 100644
--- a/llvm/include/llvm/Object/ELF.h
+++ b/llvm/include/llvm/Object/ELF.h
@@ -18,6 +18,7 @@
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
#include "llvm/BinaryFormat/ELF.h"
#include "llvm/Object/ELFTypes.h"
#include "llvm/Object/Error.h"
@@ -77,6 +78,10 @@ LLVM_ABI StringRef getRISCVVendorRelocationTypeName(uint32_t Type,
LLVM_ABI uint32_t getELFRelativeRelocationType(uint32_t Machine);
LLVM_ABI StringRef getELFSectionTypeName(uint32_t Machine, uint32_t Type);
+static inline Error createError(Error E, const Twine &Context) {
+ return createContextualizedError(std::move(E), Context);
+}
+
// Subclasses of ELFFile may need this for template instantiation
inline std::pair<unsigned char, unsigned char>
getElfArchType(StringRef Object) {
@@ -164,9 +169,7 @@ static std::string getPhdrIndexForError(const ELFFile<ELFT> &Obj,
return "[unknown index]";
}
-static inline Error defaultWarningHandler(const Twine &Msg) {
- return createError(Msg);
-}
+static inline Error defaultWarningHandler(Error Err) { return Err; }
template <class ELFT>
static bool checkSectionOffsets(const typename ELFT::Phdr &Phdr,
@@ -268,9 +271,9 @@ class ELFFile {
// This is a callback that can be passed to a number of functions.
// It can be used to ignore non-critical errors (warnings), which is
// useful for dumpers, like llvm-readobj.
- // It accepts a warning message string and returns a success
+ // It accepts an error and returns a success
// when the warning should be ignored or an error otherwise.
- using WarningHandler = llvm::function_ref<Error(const Twine &Msg)>;
+ using WarningHandler = llvm::function_ref<Error(Error)>;
const uint8_t *base() const { return Buf.bytes_begin(); }
const uint8_t *end() const { return base() + getBufSize(); }
@@ -623,9 +626,9 @@ getExtendedSymbolTableIndex(const typename ELFT::Sym &Sym, unsigned SymIndex,
Expected<typename ELFT::Word> TableOrErr = ShndxTable[SymIndex];
if (!TableOrErr)
- return createError("unable to read an extended symbol table at index " +
- Twine(SymIndex) + ": " +
- toString(TableOrErr.takeError()));
+ return createError(TableOrErr.takeError(),
+ "unable to read an extended symbol table at index " +
+ Twine(SymIndex));
return *TableOrErr;
}
@@ -842,8 +845,8 @@ ELFFile<ELFT>::getSectionStringTable(Elf_Shdr_Range Sections,
Expected<uint32_t> ShStrNdxOrErr = getShStrNdx();
if (!ShStrNdxOrErr)
return createError(
- "e_shstrndx == SHN_XINDEX, but cannot read section header 0: " +
- toString(ShStrNdxOrErr.takeError()));
+ ShStrNdxOrErr.takeError(),
+ "e_shstrndx == SHN_XINDEX, but cannot read section header 0");
uint32_t Index = *ShStrNdxOrErr;
// There is no section name string table. Return FakeSectionStrings which
@@ -1153,8 +1156,8 @@ ELFFile<ELFT>::getVersionDefinitions(const Elf_Shdr &Sec) const {
Expected<ArrayRef<uint8_t>> ContentsOrErr = getSectionContents(Sec);
if (!ContentsOrErr)
- return createError("cannot read content of " + describe(*this, Sec) + ": " +
- toString(ContentsOrErr.takeError()));
+ return createError(ContentsOrErr.takeError(),
+ "cannot read content of " + describe(*this, Sec));
const uint8_t *Start = ContentsOrErr->data();
const uint8_t *End = Start + ContentsOrErr->size();
@@ -1238,7 +1241,7 @@ ELFFile<ELFT>::getVersionDependencies(const Elf_Shdr &Sec,
StringRef StrTab;
Expected<StringRef> StrTabOrErr = getLinkAsStrtab(Sec);
if (!StrTabOrErr) {
- if (Error E = WarnHandler(toString(StrTabOrErr.takeError())))
+ if (Error E = WarnHandler(StrTabOrErr.takeError()))
return std::move(E);
} else {
StrTab = *StrTabOrErr;
@@ -1246,8 +1249,8 @@ ELFFile<ELFT>::getVersionDependencies(const Elf_Shdr &Sec,
Expected<ArrayRef<uint8_t>> ContentsOrErr = getSectionContents(Sec);
if (!ContentsOrErr)
- return createError("cannot read content of " + describe(*this, Sec) + ": " +
- toString(ContentsOrErr.takeError()));
+ return createError(ContentsOrErr.takeError(),
+ "cannot read content of " + describe(*this, Sec));
const uint8_t *Start = ContentsOrErr->data();
const uint8_t *End = Start + ContentsOrErr->size();
@@ -1333,11 +1336,12 @@ Expected<StringRef>
ELFFile<ELFT>::getStringTable(const Elf_Shdr &Section,
WarningHandler WarnHandler) const {
if (Section.sh_type != ELF::SHT_STRTAB)
- if (Error E = WarnHandler("invalid sh_type for string table section " +
- getSecIndexForError(*this, Section) +
- ": expected SHT_STRTAB, but got " +
- object::getELFSectionTypeName(
- getHeader().e_machine, Section.sh_type)))
+ if (Error E = WarnHandler(
+ createError("invalid sh_type for string table section " +
+ getSecIndexForError(*this, Section) +
+ ": expected SHT_STRTAB, but got " +
+ object::getELFSectionTypeName(getHeader().e_machine,
+ Section.sh_type))))
return std::move(E);
auto V = getSectionContentsAsArray<char>(Section);
@@ -1422,14 +1426,14 @@ ELFFile<ELFT>::getLinkAsStrtab(const typename ELFT::Shdr &Sec) const {
Expected<const typename ELFT::Shdr *> StrTabSecOrErr =
getSection(Sec.sh_link);
if (!StrTabSecOrErr)
- return createError("invalid section linked to " + describe(*this, Sec) +
- ": " + toString(StrTabSecOrErr.takeError()));
+ return createError(StrTabSecOrErr.takeError(),
+ "invalid section linked to " + describe(*this, Sec));
Expected<StringRef> StrTabOrErr = getStringTable(**StrTabSecOrErr);
if (!StrTabOrErr)
- return createError("invalid string table linked to " +
- describe(*this, Sec) + ": " +
- toString(StrTabOrErr.takeError()));
+ return createError(StrTabOrErr.takeError(),
+ "invalid string table linked to " +
+ describe(*this, Sec));
return *StrTabOrErr;
}
diff --git a/llvm/lib/Object/ELF.cpp b/llvm/lib/Object/ELF.cpp
index 660331d5da96d..3a73cd405aabe 100644
--- a/llvm/lib/Object/ELF.cpp
+++ b/llvm/lib/Object/ELF.cpp
@@ -690,8 +690,8 @@ ELFFile<ELFT>::toMappedAddr(uint64_t VAddr, WarningHandler WarnHandler) const {
return A->p_vaddr < B->p_vaddr;
};
if (!llvm::is_sorted(LoadSegments, SortPred)) {
- if (Error E =
- WarnHandler("loadable segments are unsorted by virtual address"))
+ if (Error E = WarnHandler(
+ createError("loadable segments are unsorted by virtual address")))
return std::move(E);
llvm::stable_sort(LoadSegments, SortPred);
}
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
index 776e9c6e2f89f..410e9c26bd243 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -355,19 +355,20 @@ static StringRef ToolName;
std::unique_ptr<BuildIDFetcher> BIDFetcher;
Dumper::Dumper(const object::ObjectFile &O) : O(O), OS(outs()) {
- WarningHandler = [this](const Twine &Msg) {
- if (Warnings.insert(Msg.str()).second)
+ WarningHandler = [this](Error Err) {
+ std::string Msg = toString(std::move(Err));
+ if (Warnings.insert(Msg).second)
reportWarning(Msg, this->O.getFileName());
return Error::success();
};
}
void Dumper::reportUniqueWarning(Error Err) {
- reportUniqueWarning(toString(std::move(Err)));
+ cantFail(WarningHandler(std::move(Err)));
}
void Dumper::reportUniqueWarning(const Twine &Msg) {
- cantFail(WarningHandler(Msg));
+ cantFail(WarningHandler(createError(Msg)));
}
static Expected<std::unique_ptr<Dumper>> createDumper(const ObjectFile &Obj) {
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.h b/llvm/tools/llvm-objdump/llvm-objdump.h
index 185908b6856c4..33c6f06b048a1 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.h
+++ b/llvm/tools/llvm-objdump/llvm-objdump.h
@@ -88,7 +88,7 @@ class Dumper {
protected:
llvm::raw_ostream &OS;
- std::function<Error(const Twine &Msg)> WarningHandler;
+ std::function<Error(Error E)> WarningHandler;
public:
Dumper(const object::ObjectFile &O);
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index bcb580119fb85..d722102a114e6 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -2028,8 +2028,8 @@ ELFDumper<ELFT>::ELFDumper(const object::ELFObjectFile<ELFT> &O,
template <typename ELFT> void ELFDumper<ELFT>::parseDynamicTable() {
auto toMappedAddr = [&](uint64_t Tag, uint64_t VAddr) -> const uint8_t * {
- auto MappedAddrOrError = Obj.toMappedAddr(VAddr, [&](const Twine &Msg) {
- this->reportUniqueWarning(Msg);
+ auto MappedAddrOrError = Obj.toMappedAddr(VAddr, [&](Error E) {
+ this->reportUniqueWarning(std::move(E));
return Error::success();
});
if (!MappedAddrOrError) {
diff --git a/llvm/tools/llvm-readobj/ObjDumper.cpp b/llvm/tools/llvm-readobj/ObjDumper.cpp
index 1d193573b4776..4b178b47e25bb 100644
--- a/llvm/tools/llvm-readobj/ObjDumper.cpp
+++ b/llvm/tools/llvm-readobj/ObjDumper.cpp
@@ -33,8 +33,9 @@ static inline Error createError(const Twine &Msg) {
ObjDumper::ObjDumper(ScopedPrinter &Writer, StringRef ObjName) : W(Writer) {
// Dumper reports all non-critical errors as warnings.
// It does not print the same warning more than once.
- WarningHandler = [=](const Twine &Msg) {
- if (Warnings.insert(Msg.str()).second)
+ WarningHandler = [=](Error E) {
+ std::string Msg = toString(std::move(E));
+ if (Warnings.insert(Msg).second)
reportWarning(createError(Msg), ObjName);
return Error::success();
};
@@ -47,7 +48,7 @@ void ObjDumper::reportUniqueWarning(Error Err) const {
}
void ObjDumper::reportUniqueWarning(const Twine &Msg) const {
- cantFail(WarningHandler(Msg),
+ cantFail(WarningHandler(createError(Msg)),
"WarningHandler should always return ErrorSuccess");
}
diff --git a/llvm/tools/llvm-readobj/ObjDumper.h b/llvm/tools/llvm-readobj/ObjDumper.h
index 0dba8252fd466..a99e3c5bf307a 100644
--- a/llvm/tools/llvm-readobj/ObjDumper.h
+++ b/llvm/tools/llvm-readobj/ObjDumper.h
@@ -187,7 +187,7 @@ class ObjDumper {
void printSectionsAsHex(const object::ObjectFile &Obj,
ArrayRef<std::string> Sections, bool Decompress);
- std::function<Error(const Twine &Msg)> WarningHandler;
+ std::function<Error(Error)> WarningHandler;
void reportUniqueWarning(Error Err) const;
void reportUniqueWarning(const Twine &Msg) const;
void printOffloading(const object::ObjectFile &Obj);
diff --git a/llvm/unittests/Object/ELFObjectFileTest.cpp b/llvm/unittests/Object/ELFObjectFileTest.cpp
index 1e2955ae40a66..1407493c9145f 100644
--- a/llvm/unittests/Object/ELFObjectFileTest.cpp
+++ b/llvm/unittests/Object/ELFObjectFileTest.cpp
@@ -389,9 +389,9 @@ TEST(ELFObjectFileTest, InvalidLoadSegmentsOrderTest) {
std::string WarnString;
auto ToMappedAddr = [&](uint64_t Addr) -> const uint8_t * {
Expected<const uint8_t *> DataOrErr =
- ExpectedFile->getELFFile().toMappedAddr(Addr, [&](const Twine &Msg) {
+ ExpectedFile->getELFFile().toMappedAddr(Addr, [&](Error E) {
EXPECT_TRUE(WarnString.empty());
- WarnString = Msg.str();
+ WarnString = toString(std::move(E));
return Error::success();
});
More information about the llvm-branch-commits
mailing list