[llvm] a00ff71 - [XCOFF] Improve error message context.
via llvm-commits
llvm-commits at lists.llvm.org
Sun Oct 10 19:55:01 PDT 2021
Author: Esme-Yi
Date: 2021-10-11T02:52:20Z
New Revision: a00ff71668202b2ee3e16a23f9b16dad01001def
URL: https://github.com/llvm/llvm-project/commit/a00ff71668202b2ee3e16a23f9b16dad01001def
DIFF: https://github.com/llvm/llvm-project/commit/a00ff71668202b2ee3e16a23f9b16dad01001def.diff
LOG: [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
Added:
Modified:
llvm/lib/Object/XCOFFObjectFile.cpp
llvm/test/tools/llvm-objdump/XCOFF/section-headers.test
llvm/test/tools/llvm-readobj/XCOFF/relocations-invalid.test
llvm/test/tools/obj2yaml/XCOFF/invalid-section.yaml
llvm/test/tools/obj2yaml/XCOFF/invalid-symbol.yaml
llvm/tools/obj2yaml/obj2yaml.cpp
llvm/tools/obj2yaml/obj2yaml.h
llvm/tools/obj2yaml/xcoff2yaml.cpp
llvm/unittests/Object/XCOFFObjectFileTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Object/XCOFFObjectFile.cpp b/llvm/lib/Object/XCOFFObjectFile.cpp
index e4daa8cceecaa..a0ce8be8761e9 100644
--- a/llvm/lib/Object/XCOFFObjectFile.cpp
+++ b/llvm/lib/Object/XCOFFObjectFile.cpp
@@ -43,6 +43,10 @@ static uintptr_t getWithOffset(uintptr_t Base, ptr
diff _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 {
diff --git a/llvm/test/tools/llvm-objdump/XCOFF/section-headers.test b/llvm/test/tools/llvm-objdump/XCOFF/section-headers.test
index fd7527423eb1f..e80d5f6a711b1 100644
--- a/llvm/test/tools/llvm-objdump/XCOFF/section-headers.test
+++ b/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:
diff --git a/llvm/test/tools/llvm-readobj/XCOFF/relocations-invalid.test b/llvm/test/tools/llvm-readobj/XCOFF/relocations-invalid.test
index 76d75ae7d5dab..d9557dc98d30c 100644
--- a/llvm/test/tools/llvm-readobj/XCOFF/relocations-invalid.test
+++ b/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: ]
diff --git a/llvm/test/tools/obj2yaml/XCOFF/invalid-section.yaml b/llvm/test/tools/obj2yaml/XCOFF/invalid-section.yaml
index 81c02b2208959..1e16c5f66baa3 100644
--- a/llvm/test/tools/obj2yaml/XCOFF/invalid-section.yaml
+++ b/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:
diff --git a/llvm/test/tools/obj2yaml/XCOFF/invalid-symbol.yaml b/llvm/test/tools/obj2yaml/XCOFF/invalid-symbol.yaml
index 6a243797770cb..0f6f6b202669a 100644
--- a/llvm/test/tools/obj2yaml/XCOFF/invalid-symbol.yaml
+++ b/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:
diff --git a/llvm/tools/obj2yaml/obj2yaml.cpp b/llvm/tools/obj2yaml/obj2yaml.cpp
index ff6b470f5244b..e9e47d1a2b186 100644
--- a/llvm/tools/obj2yaml/obj2yaml.cpp
+++ b/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);
diff --git a/llvm/tools/obj2yaml/obj2yaml.h b/llvm/tools/obj2yaml/obj2yaml.h
index e312050271067..fdd9b2a00185c 100644
--- a/llvm/tools/obj2yaml/obj2yaml.h
+++ b/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);
diff --git a/llvm/tools/obj2yaml/xcoff2yaml.cpp b/llvm/tools/obj2yaml/xcoff2yaml.cpp
index 5f3b580c645a5..86bef80d3773e 100644
--- a/llvm/tools/obj2yaml/xcoff2yaml.cpp
+++ b/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();
}
diff --git a/llvm/unittests/Object/XCOFFObjectFileTest.cpp b/llvm/unittests/Object/XCOFFObjectFileTest.cpp
index fc7e48100aa68..0559a1ee3c575 100644
--- a/llvm/unittests/Object/XCOFFObjectFileTest.cpp
+++ b/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) {
More information about the llvm-commits
mailing list