[llvm] r276025 - Next step along the way to getting good error messages for bad archives.
Kevin Enderby via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 19 13:47:12 PDT 2016
Author: enderby
Date: Tue Jul 19 15:47:07 2016
New Revision: 276025
URL: http://llvm.org/viewvc/llvm-project?rev=276025&view=rev
Log:
Next step along the way to getting good error messages for bad archives.
This step builds on Lang Hames work to change Archive::child_iterator
for better interoperation with Error/Expected. Building on that it is now
possible to return an error message when the size field of an archive
contains non-decimal characters.
Modified:
llvm/trunk/include/llvm/Object/Archive.h
llvm/trunk/lib/Object/Archive.cpp
llvm/trunk/test/tools/llvm-objdump/malformed-archives.test
llvm/trunk/tools/llvm-ar/llvm-ar.cpp
llvm/trunk/tools/llvm-objdump/MachODump.cpp
Modified: llvm/trunk/include/llvm/Object/Archive.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/Archive.h?rev=276025&r1=276024&r2=276025&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Object/Archive.h (original)
+++ llvm/trunk/include/llvm/Object/Archive.h Tue Jul 19 15:47:07 2016
@@ -38,7 +38,7 @@ struct ArchiveMemberHeader {
llvm::StringRef getName() const;
/// Members are not larger than 4GB.
- ErrorOr<uint32_t> getSize() const;
+ Expected<uint32_t> getSize() const;
sys::fs::perms getAccessMode() const;
sys::TimeValue getLastModified() const;
@@ -67,7 +67,7 @@ public:
bool isThinMember() const;
public:
- Child(const Archive *Parent, const char *Start, std::error_code *EC);
+ Child(const Archive *Parent, const char *Start, Error *Err);
Child(const Archive *Parent, StringRef Data, uint16_t StartOfFile);
bool operator ==(const Child &other) const {
@@ -76,7 +76,7 @@ public:
}
const Archive *getParent() const { return Parent; }
- ErrorOr<Child> getNext() const;
+ Expected<Child> getNext() const;
ErrorOr<StringRef> getName() const;
ErrorOr<std::string> getFullName() const;
@@ -93,9 +93,9 @@ public:
return getHeader()->getAccessMode();
}
/// \return the size of the archive member without the header or padding.
- ErrorOr<uint64_t> getSize() const;
+ Expected<uint64_t> getSize() const;
/// \return the size in the archive header for this member.
- ErrorOr<uint64_t> getRawSize() const;
+ Expected<uint64_t> getRawSize() const;
ErrorOr<StringRef> getBuffer() const;
uint64_t getChildOffset() const;
@@ -136,7 +136,7 @@ public:
else {
ErrorAsOutParameter ErrAsOutParam(*E);
C = C.getParent()->child_end().C;
- *E = errorCodeToError(ChildOrErr.getError());
+ *E = ChildOrErr.takeError();
E = nullptr;
}
return *this;
Modified: llvm/trunk/lib/Object/Archive.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/Archive.cpp?rev=276025&r1=276024&r2=276025&view=diff
==============================================================================
--- llvm/trunk/lib/Object/Archive.cpp (original)
+++ llvm/trunk/lib/Object/Archive.cpp Tue Jul 19 15:47:07 2016
@@ -27,6 +27,13 @@ static const char *const ThinMagic = "!<
void Archive::anchor() { }
+static Error
+malformedError(Twine Msg) {
+ std::string StringMsg = "truncated or malformed archive (" + Msg.str() + ")";
+ return make_error<GenericBinaryError>(std::move(StringMsg),
+ object_error::parse_failed);
+}
+
StringRef ArchiveMemberHeader::getName() const {
char EndCond;
if (Name[0] == '/' || Name[0] == '#')
@@ -42,10 +49,16 @@ StringRef ArchiveMemberHeader::getName()
return llvm::StringRef(Name, end);
}
-ErrorOr<uint32_t> ArchiveMemberHeader::getSize() const {
+Expected<uint32_t> ArchiveMemberHeader::getSize() const {
uint32_t Ret;
- if (llvm::StringRef(Size, sizeof(Size)).rtrim(" ").getAsInteger(10, Ret))
- return object_error::parse_failed; // Size is not a decimal number.
+ if (llvm::StringRef(Size, sizeof(Size)).rtrim(" ").getAsInteger(10, Ret)) {
+ std::string Buf;
+ raw_string_ostream OS(Buf);
+ OS.write_escaped(llvm::StringRef(Size, sizeof(Size)).rtrim(" "));
+ OS.flush();
+ return malformedError("characters in size field in archive header are not "
+ "all decimal numbers: '" + Buf + "'");
+ }
return Ret;
}
@@ -91,8 +104,7 @@ Archive::Child::Child(const Archive *Par
uint16_t StartOfFile)
: Parent(Parent), Data(Data), StartOfFile(StartOfFile) {}
-Archive::Child::Child(const Archive *Parent, const char *Start,
- std::error_code *EC)
+Archive::Child::Child(const Archive *Parent, const char *Start, Error *Err)
: Parent(Parent) {
if (!Start)
return;
@@ -100,9 +112,14 @@ Archive::Child::Child(const Archive *Par
uint64_t Size = sizeof(ArchiveMemberHeader);
Data = StringRef(Start, Size);
if (!isThinMember()) {
- ErrorOr<uint64_t> MemberSize = getRawSize();
- if ((*EC = MemberSize.getError()))
+ Expected<uint64_t> MemberSize = getRawSize();
+ if (!MemberSize) {
+ if (Err) {
+ ErrorAsOutParameter ErrAsOutParam(*Err);
+ *Err = MemberSize.takeError();
+ }
return;
+ }
Size += MemberSize.get();
Data = StringRef(Start, Size);
}
@@ -119,21 +136,18 @@ Archive::Child::Child(const Archive *Par
}
}
-ErrorOr<uint64_t> Archive::Child::getSize() const {
+Expected<uint64_t> Archive::Child::getSize() const {
if (Parent->IsThin) {
- ErrorOr<uint32_t> Size = getHeader()->getSize();
- if (std::error_code EC = Size.getError())
- return EC;
+ Expected<uint32_t> Size = getHeader()->getSize();
+ if (!Size)
+ return Size.takeError();
return Size.get();
}
return Data.size() - StartOfFile;
}
-ErrorOr<uint64_t> Archive::Child::getRawSize() const {
- ErrorOr<uint32_t> Size = getHeader()->getSize();
- if (std::error_code EC = Size.getError())
- return EC;
- return Size.get();
+Expected<uint64_t> Archive::Child::getRawSize() const {
+ return getHeader()->getSize();
}
bool Archive::Child::isThinMember() const {
@@ -158,9 +172,9 @@ ErrorOr<std::string> Archive::Child::get
ErrorOr<StringRef> Archive::Child::getBuffer() const {
if (!isThinMember()) {
- ErrorOr<uint32_t> Size = getSize();
- if (std::error_code EC = Size.getError())
- return EC;
+ Expected<uint32_t> Size = getSize();
+ if (!Size)
+ return errorToErrorCode(Size.takeError());
return StringRef(Data.data() + StartOfFile, Size.get());
}
ErrorOr<std::string> FullNameOrEr = getFullName();
@@ -174,7 +188,7 @@ ErrorOr<StringRef> Archive::Child::getBu
return Parent->ThinBuffers.back()->getBuffer();
}
-ErrorOr<Archive::Child> Archive::Child::getNext() const {
+Expected<Archive::Child> Archive::Child::getNext() const {
size_t SpaceToSkip = Data.size();
// If it's odd, add 1 to make it even.
if (SpaceToSkip & 1)
@@ -188,12 +202,13 @@ ErrorOr<Archive::Child> Archive::Child::
// Check to see if this is past the end of the archive.
if (NextLoc > Parent->Data.getBufferEnd())
- return object_error::parse_failed;
+ return malformedError("offset to next archive member past the end of the "
+ "archive");
- std::error_code EC;
- Child Ret(Parent, NextLoc, &EC);
- if (EC)
- return EC;
+ Error Err;
+ Child Ret(Parent, NextLoc, &Err);
+ if (Err)
+ return std::move(Err);
return Ret;
}
@@ -472,13 +487,9 @@ Archive::child_iterator Archive::child_b
&Err);
const char *Loc = Data.getBufferStart() + strlen(Magic);
- std::error_code EC;
- Child C(this, Loc, &EC);
- if (EC) {
- ErrorAsOutParameter ErrAsOutParam(Err);
- Err = errorCodeToError(EC);
+ Child C(this, Loc, &Err);
+ if (Err)
return child_end();
- }
return child_iterator(C, &Err);
}
@@ -543,10 +554,10 @@ ErrorOr<Archive::Child> Archive::Symbol:
}
const char *Loc = Parent->getData().begin() + Offset;
- std::error_code EC;
- Child C(Parent, Loc, &EC);
- if (EC)
- return EC;
+ Error Err;
+ Child C(Parent, Loc, &Err);
+ if (Err)
+ return errorToErrorCode(std::move(Err));
return C;
}
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=276025&r1=276024&r2=276025&view=diff
==============================================================================
--- llvm/trunk/test/tools/llvm-objdump/malformed-archives.test (original)
+++ llvm/trunk/test/tools/llvm-objdump/malformed-archives.test Tue Jul 19 15:47:07 2016
@@ -5,16 +5,16 @@
# RUN: %p/Inputs/libbogus1.a \
# RUN: 2>&1 | FileCheck -check-prefix=bogus1 %s
-# bogus1: Invalid data was encountered while parsing the file
+# bogus1: libbogus1.a': truncated or malformed archive (characters in size field in archive header are not all decimal numbers: '10%')
# RUN: not llvm-objdump -macho -archive-headers \
# RUN: %p/Inputs/libbogus2.a \
# RUN: 2>&1 | FileCheck -check-prefix=bogus2 %s
-# bogus2: LLVM ERROR: Invalid data was encountered while parsing the file
+# bogus2: libbogus2.a': truncated or malformed archive (characters in size field in archive header are not all decimal numbers: '1%')
# RUN: not llvm-objdump -macho -archive-headers \
# RUN: %p/Inputs/libbogus3.a \
# RUN: 2>&1 | FileCheck -check-prefix=bogus3 %s
-# bogus3: LLVM ERROR: Invalid data was encountered while parsing the file
+# bogus3: libbogus3.a': truncated or malformed archive (offset to next archive member past the end of the archive)
Modified: llvm/trunk/tools/llvm-ar/llvm-ar.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-ar/llvm-ar.cpp?rev=276025&r1=276024&r2=276025&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-ar/llvm-ar.cpp (original)
+++ llvm/trunk/tools/llvm-ar/llvm-ar.cpp Tue Jul 19 15:47:07 2016
@@ -343,8 +343,8 @@ static void doDisplayTable(StringRef Nam
printMode(Mode & 007);
outs() << ' ' << C.getUID();
outs() << '/' << C.getGID();
- ErrorOr<uint64_t> Size = C.getSize();
- failIfError(Size.getError());
+ Expected<uint64_t> Size = C.getSize();
+ failIfError(Size.takeError());
outs() << ' ' << format("%6llu", Size.get());
outs() << ' ' << C.getLastModified().str();
outs() << ' ';
Modified: llvm/trunk/tools/llvm-objdump/MachODump.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/MachODump.cpp?rev=276025&r1=276024&r2=276025&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objdump/MachODump.cpp (original)
+++ llvm/trunk/tools/llvm-objdump/MachODump.cpp Tue Jul 19 15:47:07 2016
@@ -1472,8 +1472,9 @@ static void printMachOUniversalHeaders(c
}
}
-static void printArchiveChild(const Archive::Child &C, bool verbose,
- bool print_offset) {
+static void printArchiveChild(StringRef Filename, const Archive::Child &C,
+ bool verbose, bool print_offset,
+ StringRef ArchitectureName = StringRef()) {
if (print_offset)
outs() << C.getChildOffset() << "\t";
sys::fs::perms Mode = C.getAccessMode();
@@ -1498,9 +1499,9 @@ static void printArchiveChild(const Arch
outs() << format("%3d/", UID);
unsigned GID = C.getGID();
outs() << format("%-3d ", GID);
- ErrorOr<uint64_t> Size = C.getRawSize();
- if (std::error_code EC = Size.getError())
- report_fatal_error(EC.message());
+ Expected<uint64_t> Size = C.getRawSize();
+ if (!Size)
+ report_error(Filename, C, Size.takeError(), ArchitectureName);
outs() << format("%5" PRId64, Size.get()) << " ";
StringRef RawLastModified = C.getRawLastModified();
@@ -1534,12 +1535,15 @@ static void printArchiveChild(const Arch
}
}
-static void printArchiveHeaders(Archive *A, bool verbose, bool print_offset) {
+static void printArchiveHeaders(StringRef Filename, Archive *A, bool verbose,
+ bool print_offset,
+ StringRef ArchitectureName = StringRef()) {
Error Err;
for (const auto &C : A->children(Err, false))
- printArchiveChild(C, verbose, print_offset);
+ printArchiveChild(Filename, C, verbose, print_offset, ArchitectureName);
+
if (Err)
- report_fatal_error(std::move(Err));
+ report_error(Filename, std::move(Err));
}
// ParseInputMachO() parses the named Mach-O file in Filename and handles the
@@ -1569,7 +1573,8 @@ void llvm::ParseInputMachO(StringRef Fil
if (Archive *A = dyn_cast<Archive>(&Bin)) {
outs() << "Archive : " << Filename << "\n";
if (ArchiveHeaders)
- printArchiveHeaders(A, !NonVerbose, ArchiveMemberOffsets);
+ printArchiveHeaders(Filename, A, !NonVerbose, ArchiveMemberOffsets);
+
Error Err;
for (auto &C : A->children(Err)) {
Expected<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
@@ -1626,7 +1631,8 @@ void llvm::ParseInputMachO(StringRef Fil
outs() << " (architecture " << ArchitectureName << ")";
outs() << "\n";
if (ArchiveHeaders)
- printArchiveHeaders(A.get(), !NonVerbose, ArchiveMemberOffsets);
+ printArchiveHeaders(Filename, A.get(), !NonVerbose,
+ ArchiveMemberOffsets, ArchitectureName);
Error Err;
for (auto &C : A->children(Err)) {
Expected<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
@@ -1681,7 +1687,8 @@ void llvm::ParseInputMachO(StringRef Fil
std::unique_ptr<Archive> &A = *AOrErr;
outs() << "Archive : " << Filename << "\n";
if (ArchiveHeaders)
- printArchiveHeaders(A.get(), !NonVerbose, ArchiveMemberOffsets);
+ printArchiveHeaders(Filename, A.get(), !NonVerbose,
+ ArchiveMemberOffsets);
Error Err;
for (auto &C : A->children(Err)) {
Expected<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
@@ -1732,7 +1739,8 @@ void llvm::ParseInputMachO(StringRef Fil
outs() << " (architecture " << ArchitectureName << ")";
outs() << "\n";
if (ArchiveHeaders)
- printArchiveHeaders(A.get(), !NonVerbose, ArchiveMemberOffsets);
+ printArchiveHeaders(Filename, A.get(), !NonVerbose,
+ ArchiveMemberOffsets, ArchitectureName);
Error Err;
for (auto &C : A->children(Err)) {
Expected<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
More information about the llvm-commits
mailing list