[XCOFF] Improve error message context.

Summary: This patch improves the error message context of the
XCOFF interfaces by providing more details.

Reviewed By: jhenderson

Differential Revision: https://reviews.llvm.org/D110320

llvm/lib/Object/XCOFFObjectFile.cpp

@@ -43,6 +43,10 @@ static uintptr_t getWithOffset(uintptr_t Base, ptrdiff_t Offset) {
                                      Offset);
 }
 
+static Error createError(const Twine &Err) {
+  return make_error<StringError>(Err, object_error::parse_failed);
+}
+
 template <typename T> static const T *viewAs(uintptr_t in) {
   return reinterpret_cast<const T *>(in);
 }
@@ -190,8 +194,9 @@ XCOFFObjectFile::getStringTableEntry(uint32_t Offset) const {
   if (StringTable.Data != nullptr && StringTable.Size > Offset)
     return (StringTable.Data + Offset);
 
-  return make_error<GenericBinaryError>("Bad offset for string table entry",
-                                        object_error::parse_failed);
+  return createError("entry with offset 0x" + Twine::utohexstr(Offset) +
+                     " in a string table with size 0x" +
+                     Twine::utohexstr(StringTable.Size) + " is invalid");
 }
 
 StringRef XCOFFObjectFile::getStringTable() const {
@@ -367,7 +372,10 @@ XCOFFObjectFile::getSectionContents(DataRefImpl Sec) const {
   uint64_t SectionSize = getSectionSize(Sec);
   if (Error E = Binary::checkOffset(
           Data, reinterpret_cast<uintptr_t>(ContentStart), SectionSize))
-    return std::move(E);
+    return createError(
+        toString(std::move(E)) + ": section data with offset 0x" +
+        Twine::utohexstr(OffsetToRaw) + " and size 0x" +
+        Twine::utohexstr(SectionSize) + " goes past the end of the file");
 
   return makeArrayRef(ContentStart,SectionSize);
 }
@@ -406,7 +414,12 @@ Expected<uintptr_t> XCOFFObjectFile::getLoaderSectionAddress() const {
       reinterpret_cast<uintptr_t>(base() + OffsetToLoaderSection);
   if (Error E =
           Binary::checkOffset(Data, LoderSectionStart, SizeOfLoaderSection))
-    return std::move(E);
+    return createError(toString(std::move(E)) +
+                       ": loader section with offset 0x" +
+                       Twine::utohexstr(OffsetToLoaderSection) +
+                       " and size 0x" + Twine::utohexstr(SizeOfLoaderSection) +
+                       " goes past the end of the file");
+
   return LoderSectionStart;
 }
 
@@ -825,7 +838,9 @@ XCOFFObjectFile::getSymbolNameByIndex(uint32_t Index) const {
   const uint32_t NumberOfSymTableEntries = getNumberOfSymbolTableEntries();
 
   if (Index >= NumberOfSymTableEntries)
-    return errorCodeToError(object_error::invalid_symbol_index);
+    return createError("symbol index " + Twine(Index) +
+                       " exceeds symbol count " +
+                       Twine(NumberOfSymTableEntries));
 
   DataRefImpl SymDRI;
   SymDRI.p = getSymbolEntryAddressByIndex(Index);
@@ -904,8 +919,12 @@ Expected<ArrayRef<Reloc>> XCOFFObjectFile::relocations(const Shdr &Sec) const {
   auto RelocationOrErr =
       getObject<Reloc>(Data, reinterpret_cast<void *>(RelocAddr),
                        NumRelocEntries * sizeof(Reloc));
-  if (Error E = RelocationOrErr.takeError())
-    return std::move(E);
+  if (!RelocationOrErr)
+    return createError(
+        toString(RelocationOrErr.takeError()) + ": relocations with offset 0x" +
+        Twine::utohexstr(Sec.FileOffsetToRelocationInfo) + " and size 0x" +
+        Twine::utohexstr(NumRelocEntries * sizeof(Reloc)) +
+        " go past the end of the file");
 
   const Reloc *StartReloc = RelocationOrErr.get();
 
@@ -932,8 +951,12 @@ XCOFFObjectFile::parseStringTable(const XCOFFObjectFile *Obj, uint64_t Offset) {
 
   auto StringTableOrErr =
       getObject<char>(Obj->Data, Obj->base() + Offset, Size);
-  if (Error E = StringTableOrErr.takeError())
-    return std::move(E);
+  if (!StringTableOrErr)
+    return createError(toString(StringTableOrErr.takeError()) +
+                       ": string table with offset 0x" +
+                       Twine::utohexstr(Offset) + " and size 0x" +
+                       Twine::utohexstr(Size) +
+                       " goes past the end of the file");
 
   const char *StringTablePtr = StringTableOrErr.get();
   if (StringTablePtr[Size - 1] != '\0')
@@ -971,14 +994,21 @@ Expected<StringRef> XCOFFObjectFile::getImportFileTable() const {
       Data,
       reinterpret_cast<void *>(LoaderSectionAddr + OffsetToImportFileTable),
       LengthOfImportFileTable);
-  if (Error E = ImportTableOrErr.takeError())
-    return std::move(E);
+  if (!ImportTableOrErr)
+    return createError(
+        toString(ImportTableOrErr.takeError()) +
+        ": import file table with offset 0x" +
+        Twine::utohexstr(LoaderSectionAddr + OffsetToImportFileTable) +
+        " and size 0x" + Twine::utohexstr(LengthOfImportFileTable) +
+        " goes past the end of the file");
 
   const char *ImportTablePtr = ImportTableOrErr.get();
   if (ImportTablePtr[LengthOfImportFileTable - 1] != '\0')
-    return createStringError(
-        object_error::parse_failed,
-        "the import file table must end with a null terminator");
+    return createError(
+        ": import file name table with offset 0x" +
+        Twine::utohexstr(LoaderSectionAddr + OffsetToImportFileTable) +
+        " and size 0x" + Twine::utohexstr(LengthOfImportFileTable) +
+        " must end with a null terminator");
 
   return StringRef(ImportTablePtr, LengthOfImportFileTable);
 }
@@ -1007,11 +1037,17 @@ XCOFFObjectFile::create(unsigned Type, MemoryBufferRef MBR) {
 
   // Parse the section header table if it is present.
   if (Obj->getNumberOfSections()) {
-    auto SecHeadersOrErr = getObject<void>(Data, Base + CurOffset,
-                                           Obj->getNumberOfSections() *
-                                               Obj->getSectionHeaderSize());
-    if (Error E = SecHeadersOrErr.takeError())
-      return std::move(E);
+    uint64_t SectionHeadersSize =
+        Obj->getNumberOfSections() * Obj->getSectionHeaderSize();
+    auto SecHeadersOrErr =
+        getObject<void>(Data, Base + CurOffset, SectionHeadersSize);
+    if (!SecHeadersOrErr)
+      return createError(toString(SecHeadersOrErr.takeError()) +
+                         ": section headers with offset 0x" +
+                         Twine::utohexstr(CurOffset) + " and size 0x" +
+                         Twine::utohexstr(SectionHeadersSize) +
+                         " go past the end of the file");
+
     Obj->SectionHeaderTable = SecHeadersOrErr.get();
   }
 
@@ -1030,8 +1066,12 @@ XCOFFObjectFile::create(unsigned Type, MemoryBufferRef MBR) {
       NumberOfSymbolTableEntries;
   auto SymTableOrErr =
       getObject<void *>(Data, Base + CurOffset, SymbolTableSize);
-  if (Error E = SymTableOrErr.takeError())
-    return std::move(E);
+  if (!SymTableOrErr)
+    return createError(
+        toString(SymTableOrErr.takeError()) + ": symbol table with offset 0x" +
+        Twine::utohexstr(CurOffset) + " and size 0x" +
+        Twine::utohexstr(SymbolTableSize) + " goes past the end of the file");
+
   Obj->SymbolTblPtr = SymTableOrErr.get();
   CurOffset += SymbolTableSize;
 
@@ -1101,10 +1141,10 @@ Expected<XCOFFCsectAuxRef> XCOFFSymbolRef::getXCOFFCsectAuxRef() const {
   if (auto Err = NameOrErr.takeError())
     return std::move(Err);
 
+  uint32_t SymbolIdx = OwningObjectPtr->getSymbolIndex(getEntryAddress());
   if (!NumberOfAuxEntries) {
-    return createStringError(object_error::parse_failed,
-                             "csect symbol \"" + *NameOrErr +
-                                 "\" contains no auxiliary entry");
+    return createError("csect symbol \"" + *NameOrErr + "\" with index " +
+                       Twine(SymbolIdx) + " contains no auxiliary entry");
   }
 
   if (!OwningObjectPtr->is64Bit()) {
@@ -1129,9 +1169,9 @@ Expected<XCOFFCsectAuxRef> XCOFFSymbolRef::getXCOFFCsectAuxRef() const {
     }
   }
 
-  return createStringError(
-      object_error::parse_failed,
-      "a csect auxiliary entry is not found for symbol \"" + *NameOrErr + "\"");
+  return createError(
+      "a csect auxiliary entry has not been found for symbol \"" + *NameOrErr +
+      "\" with index " + Twine(SymbolIdx));
 }
 
 Expected<StringRef> XCOFFSymbolRef::getName() const {

llvm/test/tools/llvm-objdump/XCOFF/section-headers.test

@@ -73,7 +73,7 @@ Sections:
 # RUN: not llvm-objdump --section-headers %t-truncate.o 2>&1 \
 # RUN:  | FileCheck --check-prefix=ERROR %s
 
-# ERROR: The end of the file was unexpectedly encountered
+# ERROR: The end of the file was unexpectedly encountered: section headers with offset 0x14 and size 0x28 go past the end of the file
 
 --- !XCOFF
 FileHeader:

llvm/test/tools/llvm-readobj/XCOFF/relocations-invalid.test

@@ -5,7 +5,7 @@
 # RUN:   FileCheck %s -DFILE=%t1 --check-prefix=INVALID-REL
 
 # INVALID-REL:      Relocations [
-# INVALID-REL-NEXT:   warning: '[[FILE]]': The end of the file was unexpectedly encountered
+# INVALID-REL-NEXT:   warning: '[[FILE]]': The end of the file was unexpectedly encountered: relocations with offset 0x222 and size 0x0 go past the end of the file
 # INVALID-REL-NEXT: ]
 
 --- !XCOFF
@@ -23,7 +23,7 @@ Sections:
 
 # INVALID-SYM:      Relocations [
 # INVALID-SYM-NEXT:   Section (index: 1) .text {
-# INVALID-SYM-NEXT:     warning: '[[FILE]]': Invalid symbol index
+# INVALID-SYM-NEXT:     warning: '[[FILE]]': symbol index 33 exceeds symbol count 0
 # INVALID-SYM-NEXT:   }
 # INVALID-SYM-NEXT: ]
 

llvm/test/tools/obj2yaml/XCOFF/invalid-section.yaml

@@ -5,7 +5,7 @@
 # RUN: yaml2obj %s --docnum=1 -o %t1
 # RUN: not obj2yaml %t1 2>&1 | FileCheck %s -DFILE=%t1 --check-prefix=ERROR1
 
-# ERROR1: The end of the file was unexpectedly encountered
+# ERROR1: The end of the file was unexpectedly encountered: section data with offset 0x70 and size 0x4 goes past the end of the file
 
 --- !XCOFF
 FileHeader:
@@ -18,7 +18,7 @@ Sections:
 # RUN: yaml2obj %s --docnum=2 -o %t2
 # RUN: not obj2yaml %t2 2>&1 | FileCheck %s -DFILE=%t2 --check-prefix=ERROR2
 
-# ERROR2: The end of the file was unexpectedly encountered
+# ERROR2: The end of the file was unexpectedly encountered: relocations with offset 0x3c and size 0x1e go past the end of the file
 
 --- !XCOFF
 FileHeader:

llvm/test/tools/obj2yaml/XCOFF/invalid-symbol.yaml

@@ -3,9 +3,9 @@
 
 ## Error1: failed to get the section name for a symbol.
 # RUN: yaml2obj %s --docnum=1 -o %t1
-# RUN: not obj2yaml %t1 2>&1 | FileCheck %s --check-prefix=ERROR1
+# RUN: not obj2yaml %t1 2>&1 | FileCheck %s -DFILE=%t1 --check-prefix=ERROR1
 
-# ERROR1: Invalid section index
+# ERROR1: Error reading file: [[FILE]]: the section index (2) is invalid
 
 --- !XCOFF
 FileHeader:
@@ -17,9 +17,9 @@ Symbols:
 
 ## Error2: failed to get the symbol name.
 # RUN: yaml2obj %s --docnum=2 -o %t2
-# RUN: not obj2yaml %t2 2>&1 | FileCheck %s --check-prefix=ERROR2
+# RUN: not obj2yaml %t2 2>&1 | FileCheck %s -DFILE=%t2 --check-prefix=ERROR2
 
-# ERROR2: Invalid data was encountered while parsing the file
+# ERROR2: Error reading file: [[FILE]]: entry with offset 0x4 in a string table with size 0x4 is invalid
 
 --- !XCOFF
 FileHeader:

llvm/tools/obj2yaml/obj2yaml.cpp

@@ -23,7 +23,7 @@ static Error dumpObject(const ObjectFile &Obj) {
     return errorCodeToError(coff2yaml(outs(), cast<COFFObjectFile>(Obj)));
 
   if (Obj.isXCOFF())
-    return errorCodeToError(xcoff2yaml(outs(), cast<XCOFFObjectFile>(Obj)));
+    return xcoff2yaml(outs(), cast<XCOFFObjectFile>(Obj));
 
   if (Obj.isELF())
     return elf2yaml(outs(), Obj);

llvm/tools/obj2yaml/obj2yaml.h

@@ -28,8 +28,8 @@ llvm::Error macho2yaml(llvm::raw_ostream &Out,
                            const llvm::object::Binary &Obj);
 llvm::Error minidump2yaml(llvm::raw_ostream &Out,
                           const llvm::object::MinidumpFile &Obj);
-std::error_code xcoff2yaml(llvm::raw_ostream &Out,
-                           const llvm::object::XCOFFObjectFile &Obj);
+llvm::Error xcoff2yaml(llvm::raw_ostream &Out,
+                       const llvm::object::XCOFFObjectFile &Obj);
 std::error_code wasm2yaml(llvm::raw_ostream &Out,
                           const llvm::object::WasmObjectFile &Obj);
 llvm::Error archive2yaml(llvm::raw_ostream &Out, llvm::MemoryBufferRef Source);

llvm/tools/obj2yaml/xcoff2yaml.cpp

@@ -138,15 +138,14 @@ Error XCOFFDumper::dumpSymbols() {
   return Error::success();
 }
 
-std::error_code xcoff2yaml(raw_ostream &Out,
-                           const object::XCOFFObjectFile &Obj) {
+Error xcoff2yaml(raw_ostream &Out, const object::XCOFFObjectFile &Obj) {
   XCOFFDumper Dumper(Obj);
 
   if (Error E = Dumper.dump())
-    return errorToErrorCode(std::move(E));
+    return E;
 
   yaml::Output Yout(Out);
   Yout << Dumper.getYAMLObj();
 
-  return std::error_code();
+  return Error::success();
 }

llvm/unittests/Object/XCOFFObjectFileTest.cpp

@@ -445,7 +445,8 @@ TEST(XCOFFObjectFileTest, XCOFFGetCsectAuxRef32) {
   Expected<XCOFFCsectAuxRef> ExpectErr = SymRef.getXCOFFCsectAuxRef();
   EXPECT_THAT_ERROR(
       ExpectErr.takeError(),
-      FailedWithMessage("csect symbol \".data\" contains no auxiliary entry"));
+      FailedWithMessage(
+          "csect symbol \".data\" with index 2 contains no auxiliary entry"));
 }
 
 TEST(XCOFFObjectFileTest, XCOFFGetCsectAuxRef64) {
@@ -500,15 +501,16 @@ TEST(XCOFFObjectFileTest, XCOFFGetCsectAuxRef64) {
   Expected<XCOFFCsectAuxRef> NotFoundErr = SymRef.getXCOFFCsectAuxRef();
   EXPECT_THAT_ERROR(
       NotFoundErr.takeError(),
-      FailedWithMessage(
-          "a csect auxiliary entry is not found for symbol \".data\""));
+      FailedWithMessage("a csect auxiliary entry has not been found for symbol "
+                        "\".data\" with index 2"));
 
   // Set csect symbol's auxiliary entry count to 0.
   XCOFF64Binary[149] = 0;
   Expected<XCOFFCsectAuxRef> ExpectErr = SymRef.getXCOFFCsectAuxRef();
   EXPECT_THAT_ERROR(
       ExpectErr.takeError(),
-      FailedWithMessage("csect symbol \".data\" contains no auxiliary entry"));
+      FailedWithMessage(
+          "csect symbol \".data\" with index 2 contains no auxiliary entry"));
 }
 
 TEST(XCOFFObjectFileTest, XCOFFTracebackTableErrorAtParameterType) {