[llvm-branch-commits] [llvm] [Object][ELF] Pass Error to WarningHandler (PR #191707)

via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Sun Apr 12 06:05:12 PDT 2026


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-binary-utilities

Author: aokblast

<details>
<summary>Changes</summary>

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/191707.diff


7 Files Affected:

- (modified) llvm/include/llvm/Object/ELF.h (+22-23) 
- (modified) llvm/lib/Object/ELF.cpp (+2-2) 
- (modified) llvm/tools/llvm-objdump/llvm-objdump.cpp (+5-4) 
- (modified) llvm/tools/llvm-objdump/llvm-objdump.h (+1-1) 
- (modified) llvm/tools/llvm-readobj/ObjDumper.cpp (+3-2) 
- (modified) llvm/tools/llvm-readobj/ObjDumper.h (+1-1) 
- (modified) llvm/unittests/Object/ELFObjectFileTest.cpp (+2-2) 


``````````diff
diff --git a/llvm/include/llvm/Object/ELF.h b/llvm/include/llvm/Object/ELF.h
index e052519e27e8f..1151294b13f34 100644
--- a/llvm/include/llvm/Object/ELF.h
+++ b/llvm/include/llvm/Object/ELF.h
@@ -164,9 +164,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 +266,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 +621,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(toString(TableOrErr.takeError()),
+                       "unable to read an extended symbol table at index " +
+                           Twine(SymIndex));
   return *TableOrErr;
 }
 
@@ -1153,8 +1151,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(toString(ContentsOrErr.takeError()),
+                       "cannot read content of " + describe(*this, Sec));
 
   const uint8_t *Start = ContentsOrErr->data();
   const uint8_t *End = Start + ContentsOrErr->size();
@@ -1238,7 +1236,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 +1244,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(toString(ContentsOrErr.takeError()),
+                       "cannot read content of " + describe(*this, Sec));
 
   const uint8_t *Start = ContentsOrErr->data();
   const uint8_t *End = Start + ContentsOrErr->size();
@@ -1333,11 +1331,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 +1421,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(toString(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(toString(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/ObjDumper.cpp b/llvm/tools/llvm-readobj/ObjDumper.cpp
index 1d193573b4776..67630b4c41793 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();
   };
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();
         });
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/191707


More information about the llvm-branch-commits mailing list