[llvm] r369382 - [llvm-objdump] - Remove one of `report_error` functions and improve the error reporting.
George Rimar via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 20 06:19:16 PDT 2019
Author: grimar
Date: Tue Aug 20 06:19:16 2019
New Revision: 369382
URL: http://llvm.org/viewvc/llvm-project?rev=369382&view=rev
Log:
[llvm-objdump] - Remove one of `report_error` functions and improve the error reporting.
One of the report_error functions was taking object::Archive::Child as an
argument. It feels excessive, this patch removes it and introduce a helper
function instead. Also I fixed a "TODO" in this patch what improved the message printed.
Differential revision: https://reviews.llvm.org/D66468
Modified:
llvm/trunk/test/tools/llvm-objdump/malformed-archives.test
llvm/trunk/tools/llvm-objdump/MachODump.cpp
llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp
llvm/trunk/tools/llvm-objdump/llvm-objdump.h
Modified: llvm/trunk/test/tools/llvm-objdump/malformed-archives.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/malformed-archives.test?rev=369382&r1=369381&r2=369382&view=diff
==============================================================================
--- llvm/trunk/test/tools/llvm-objdump/malformed-archives.test (original)
+++ llvm/trunk/test/tools/llvm-objdump/malformed-archives.test Tue Aug 20 06:19:16 2019
@@ -47,17 +47,17 @@
# RUN: %p/Inputs/libbogus8.a \
# RUN: 2>&1 | FileCheck -check-prefix=bogus8 %s
-# bogus8: libbogus8.a(???): truncated or malformed archive (long name length: 1234 extends past the end of the member or archive for archive member header at offset 86)
+# bogus8: libbogus8.a(<file index: 1>): truncated or malformed archive (long name length: 1234 extends past the end of the member or archive for archive member header at offset 86)
# RUN: not llvm-objdump -s %p/Inputs/libbogus9.a \
# RUN: 2>&1 | FileCheck -check-prefix=bogus9 %s
-# bogus9: libbogus9.a(???): truncated or malformed archive (long name offset characters after the '/' are not all decimal numbers: '&a25*' for archive member header at offset 94)
+# bogus9: libbogus9.a(<file index: 0>): truncated or malformed archive (long name offset characters after the '/' are not all decimal numbers: '&a25*' for archive member header at offset 94)
# RUN: not llvm-objdump -s %p/Inputs/libbogus10.a \
# RUN: 2>&1 | FileCheck -check-prefix=bogus10 %s
-# bogus10: libbogus10.a(???): truncated or malformed archive (long name offset 507 past the end of the string table for archive member header at offset 94)
+# bogus10: libbogus10.a(<file index: 0>): truncated or malformed archive (long name offset 507 past the end of the string table for archive member header at offset 94)
# RUN: not llvm-objdump -macho -archive-headers \
# RUN: %p/Inputs/libbogus11.a \
Modified: llvm/trunk/tools/llvm-objdump/MachODump.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/MachODump.cpp?rev=369382&r1=369381&r2=369382&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objdump/MachODump.cpp (original)
+++ llvm/trunk/tools/llvm-objdump/MachODump.cpp Tue Aug 20 06:19:16 2019
@@ -2213,12 +2213,15 @@ static void printMachOUniversalHeaders(c
}
static void printArchiveChild(StringRef Filename, const Archive::Child &C,
- bool verbose, bool print_offset,
+ size_t ChildIndex, bool verbose,
+ bool print_offset,
StringRef ArchitectureName = StringRef()) {
if (print_offset)
outs() << C.getChildOffset() << "\t";
sys::fs::perms Mode =
- unwrapOrError(C.getAccessMode(), Filename, C, ArchitectureName);
+ unwrapOrError(C.getAccessMode(), Filename,
+ getFileNameForError(C, ChildIndex),
+ ArchitectureName);
if (verbose) {
// FIXME: this first dash, "-", is for (Mode & S_IFMT) == S_IFREG.
// But there is nothing in sys::fs::perms for S_IFMT or S_IFREG.
@@ -2238,9 +2241,12 @@ static void printArchiveChild(StringRef
outs() << format(
"%3d/%-3d %5" PRId64 " ",
- unwrapOrError(C.getUID(), Filename, C, ArchitectureName),
- unwrapOrError(C.getGID(), Filename, C, ArchitectureName),
- unwrapOrError(C.getRawSize(), Filename, C, ArchitectureName));
+ unwrapOrError(C.getUID(), Filename, getFileNameForError(C, ChildIndex),
+ ArchitectureName),
+ unwrapOrError(C.getGID(), Filename, getFileNameForError(C, ChildIndex),
+ ArchitectureName),
+ unwrapOrError(C.getRawSize(), Filename,
+ getFileNameForError(C, ChildIndex), ArchitectureName));
StringRef RawLastModified = C.getRawLastModified();
if (verbose) {
@@ -2263,14 +2269,18 @@ static void printArchiveChild(StringRef
Expected<StringRef> NameOrErr = C.getName();
if (!NameOrErr) {
consumeError(NameOrErr.takeError());
- outs() << unwrapOrError(C.getRawName(), Filename, C, ArchitectureName)
+ outs() << unwrapOrError(C.getRawName(), Filename,
+ getFileNameForError(C, ChildIndex),
+ ArchitectureName)
<< "\n";
} else {
StringRef Name = NameOrErr.get();
outs() << Name << "\n";
}
} else {
- outs() << unwrapOrError(C.getRawName(), Filename, C, ArchitectureName)
+ outs() << unwrapOrError(C.getRawName(), Filename,
+ getFileNameForError(C, ChildIndex),
+ ArchitectureName)
<< "\n";
}
}
@@ -2279,8 +2289,10 @@ static void printArchiveHeaders(StringRe
bool print_offset,
StringRef ArchitectureName = StringRef()) {
Error Err = Error::success();
+ size_t I = 0;
for (const auto &C : A->children(Err, false))
- printArchiveChild(Filename, C, verbose, print_offset, ArchitectureName);
+ printArchiveChild(Filename, C, I++, verbose, print_offset,
+ ArchitectureName);
if (Err)
report_error(std::move(Err), StringRef(), Filename, ArchitectureName);
@@ -2328,11 +2340,13 @@ void parseInputMachO(StringRef Filename)
printArchiveHeaders(Filename, A, !NonVerbose, ArchiveMemberOffsets);
Error Err = Error::success();
+ unsigned I = -1;
for (auto &C : A->children(Err)) {
+ ++I;
Expected<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
if (!ChildOrErr) {
if (Error E = isNotObjectErrorInvalidFileType(ChildOrErr.takeError()))
- report_error(std::move(E), Filename, C);
+ report_error(std::move(E), Filename, getFileNameForError(C, I));
continue;
}
if (MachOObjectFile *O = dyn_cast<MachOObjectFile>(&*ChildOrErr.get())) {
@@ -2407,11 +2421,15 @@ void parseInputMachO(MachOUniversalBinar
printArchiveHeaders(Filename, A.get(), !NonVerbose,
ArchiveMemberOffsets, ArchitectureName);
Error Err = Error::success();
+ unsigned I = -1;
for (auto &C : A->children(Err)) {
+ ++I;
Expected<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
if (!ChildOrErr) {
- if (Error E = isNotObjectErrorInvalidFileType(ChildOrErr.takeError()))
- report_error(std::move(E), Filename, C, ArchitectureName);
+ if (Error E =
+ isNotObjectErrorInvalidFileType(ChildOrErr.takeError()))
+ report_error(std::move(E), Filename, getFileNameForError(C, I),
+ ArchitectureName);
continue;
}
if (MachOObjectFile *O =
@@ -2463,12 +2481,14 @@ void parseInputMachO(MachOUniversalBinar
printArchiveHeaders(Filename, A.get(), !NonVerbose,
ArchiveMemberOffsets);
Error Err = Error::success();
+ unsigned I = -1;
for (auto &C : A->children(Err)) {
+ ++I;
Expected<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
if (!ChildOrErr) {
if (Error E =
isNotObjectErrorInvalidFileType(ChildOrErr.takeError()))
- report_error(std::move(E), Filename, C);
+ report_error(std::move(E), Filename, getFileNameForError(C, I));
continue;
}
if (MachOObjectFile *O =
@@ -2514,11 +2534,14 @@ void parseInputMachO(MachOUniversalBinar
printArchiveHeaders(Filename, A.get(), !NonVerbose,
ArchiveMemberOffsets, ArchitectureName);
Error Err = Error::success();
+ unsigned I = -1;
for (auto &C : A->children(Err)) {
+ ++I;
Expected<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
if (!ChildOrErr) {
if (Error E = isNotObjectErrorInvalidFileType(ChildOrErr.takeError()))
- report_error(std::move(E), Filename, C, ArchitectureName);
+ report_error(std::move(E), Filename, getFileNameForError(C, I),
+ ArchitectureName);
continue;
}
if (MachOObjectFile *O =
Modified: llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp?rev=369382&r1=369381&r2=369382&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp (original)
+++ llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp Tue Aug 20 06:19:16 2019
@@ -364,6 +364,17 @@ SectionFilter ToolSectionFilter(object::
return SectionFilter([](object::SectionRef S) { return shouldKeep(S); }, O);
}
+std::string getFileNameForError(const object::Archive::Child &C,
+ unsigned Index) {
+ Expected<StringRef> NameOrErr = C.getName();
+ if (NameOrErr)
+ return NameOrErr.get();
+ // If we have an error getting the name then we print the index of the archive
+ // member. Since we are already in an error state, we just ignore this error.
+ consumeError(NameOrErr.takeError());
+ return "<file index: " + std::to_string(Index) + ">";
+}
+
void error(std::error_code EC) {
if (!EC)
return;
@@ -429,20 +440,6 @@ LLVM_ATTRIBUTE_NORETURN void report_erro
exit(1);
}
-LLVM_ATTRIBUTE_NORETURN void report_error(Error E, StringRef ArchiveName,
- const object::Archive::Child &C,
- StringRef ArchitectureName) {
- Expected<StringRef> NameOrErr = C.getName();
- // TODO: if we have a error getting the name then it would be nice to print
- // the index of which archive member this is and or its offset in the
- // archive instead of "???" as the name.
- if (!NameOrErr) {
- consumeError(NameOrErr.takeError());
- report_error(std::move(E), ArchiveName, "???", ArchitectureName);
- } else
- report_error(std::move(E), ArchiveName, NameOrErr.get(), ArchitectureName);
-}
-
static void warnOnNoMatchForSections() {
SetVector<StringRef> MissingSections;
for (StringRef S : FilterSections) {
@@ -2160,11 +2157,13 @@ static void dumpObject(const COFFImportF
/// Dump each object file in \a a;
static void dumpArchive(const Archive *A) {
Error Err = Error::success();
+ unsigned I = -1;
for (auto &C : A->children(Err)) {
+ ++I;
Expected<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
if (!ChildOrErr) {
if (auto E = isNotObjectErrorInvalidFileType(ChildOrErr.takeError()))
- report_error(std::move(E), A->getFileName(), C);
+ report_error(std::move(E), A->getFileName(), getFileNameForError(C, I));
continue;
}
if (ObjectFile *O = dyn_cast<ObjectFile>(&*ChildOrErr.get()))
Modified: llvm/trunk/tools/llvm-objdump/llvm-objdump.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/llvm-objdump.h?rev=369382&r1=369381&r2=369382&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objdump/llvm-objdump.h (original)
+++ llvm/trunk/tools/llvm-objdump/llvm-objdump.h Tue Aug 20 06:19:16 2019
@@ -136,9 +136,6 @@ LLVM_ATTRIBUTE_NORETURN void report_erro
LLVM_ATTRIBUTE_NORETURN void
report_error(Error E, StringRef FileName, StringRef ArchiveName,
StringRef ArchitectureName = StringRef());
-LLVM_ATTRIBUTE_NORETURN void
-report_error(Error E, StringRef ArchiveName, const object::Archive::Child &C,
- StringRef ArchitectureName = StringRef());
template <typename T, typename... Ts>
T unwrapOrError(Expected<T> EO, Ts &&... Args) {
@@ -147,6 +144,9 @@ T unwrapOrError(Expected<T> EO, Ts &&...
report_error(EO.takeError(), std::forward<Ts>(Args)...);
}
+std::string getFileNameForError(const object::Archive::Child &C,
+ unsigned Index);
+
} // end namespace llvm
#endif
More information about the llvm-commits
mailing list