From 4031d9f80e67c4e36c233e1c390dabed994e9316 Mon Sep 17 00:00:00 2001 From: Vedant Kumar Date: Wed, 3 Aug 2016 19:02:50 +0000 Subject: [PATCH] Reapply "More fixes to get good error messages for bad archives." This reverts commit the revert commit r277627. The build errors mentioned in r277627 were likely caused by an unclean build directory. Sorry for the noise. llvm-svn: 277630 --- llvm/include/llvm/Object/Archive.h | 16 +++--- llvm/lib/Object/Archive.cpp | 62 +++++++++++++++++----- llvm/lib/Object/ArchiveWriter.cpp | 20 +++++-- llvm/test/tools/llvm-objdump/Inputs/libbogus11.a | 10 ++++ llvm/test/tools/llvm-objdump/Inputs/libbogus12.a | 10 ++++ llvm/test/tools/llvm-objdump/Inputs/libbogus13.a | 10 ++++ llvm/test/tools/llvm-objdump/Inputs/libbogus14.a | 10 ++++ .../tools/llvm-objdump/malformed-archives.test | 28 ++++++++++ llvm/tools/dsymutil/BinaryHolder.cpp | 5 +- llvm/tools/llvm-ar/llvm-ar.cpp | 31 ++++++++--- llvm/tools/llvm-objdump/MachODump.cpp | 18 +++++-- 11 files changed, 183 insertions(+), 37 deletions(-) create mode 100644 llvm/test/tools/llvm-objdump/Inputs/libbogus11.a create mode 100644 llvm/test/tools/llvm-objdump/Inputs/libbogus12.a create mode 100644 llvm/test/tools/llvm-objdump/Inputs/libbogus13.a create mode 100644 llvm/test/tools/llvm-objdump/Inputs/libbogus14.a diff --git a/llvm/include/llvm/Object/Archive.h b/llvm/include/llvm/Object/Archive.h index 4869145..d15b930 100644 --- a/llvm/include/llvm/Object/Archive.h +++ b/llvm/include/llvm/Object/Archive.h @@ -44,14 +44,14 @@ public: /// Members are not larger than 4GB. Expected getSize() const; - sys::fs::perms getAccessMode() const; - sys::TimeValue getLastModified() const; + Expected getAccessMode() const; + Expected getLastModified() const; llvm::StringRef getRawLastModified() const { return StringRef(ArMemHdr->LastModified, sizeof(ArMemHdr->LastModified)).rtrim(' '); } - unsigned getUID() const; - unsigned getGID() const; + Expected getUID() const; + Expected getGID() const; // This returns the size of the private struct ArMemHdrType uint64_t getSizeOf() const { @@ -102,15 +102,15 @@ public: Expected getName() const; ErrorOr getFullName() const; Expected getRawName() const { return Header.getRawName(); } - sys::TimeValue getLastModified() const { + Expected getLastModified() const { return Header.getLastModified(); } StringRef getRawLastModified() const { return Header.getRawLastModified(); } - unsigned getUID() const { return Header.getUID(); } - unsigned getGID() const { return Header.getGID(); } - sys::fs::perms getAccessMode() const { + Expected getUID() const { return Header.getUID(); } + Expected getGID() const { return Header.getGID(); } + Expected getAccessMode() const { return Header.getAccessMode(); } /// \return the size of the archive member without the header or padding. diff --git a/llvm/lib/Object/Archive.cpp b/llvm/lib/Object/Archive.cpp index 586f9c1..827affe 100644 --- a/llvm/lib/Object/Archive.cpp +++ b/llvm/lib/Object/Archive.cpp @@ -221,43 +221,81 @@ Expected ArchiveMemberHeader::getSize() const { return Ret; } -sys::fs::perms ArchiveMemberHeader::getAccessMode() const { +Expected ArchiveMemberHeader::getAccessMode() const { unsigned Ret; if (StringRef(ArMemHdr->AccessMode, - sizeof(ArMemHdr->AccessMode)).rtrim(' ').getAsInteger(8, Ret)) - llvm_unreachable("Access mode is not an octal number."); + sizeof(ArMemHdr->AccessMode)).rtrim(' ').getAsInteger(8, Ret)) { + std::string Buf; + raw_string_ostream OS(Buf); + OS.write_escaped(llvm::StringRef(ArMemHdr->AccessMode, + sizeof(ArMemHdr->AccessMode)).rtrim(" ")); + OS.flush(); + uint64_t Offset = reinterpret_cast(ArMemHdr) - + Parent->getData().data(); + return malformedError("characters in AccessMode field in archive header " + "are not all decimal numbers: '" + Buf + "' for the " + "archive member header at offset " + Twine(Offset)); + } return static_cast(Ret); } -sys::TimeValue ArchiveMemberHeader::getLastModified() const { +Expected ArchiveMemberHeader::getLastModified() const { unsigned Seconds; if (StringRef(ArMemHdr->LastModified, sizeof(ArMemHdr->LastModified)).rtrim(' ') - .getAsInteger(10, Seconds)) - llvm_unreachable("Last modified time not a decimal number."); + .getAsInteger(10, Seconds)) { + std::string Buf; + raw_string_ostream OS(Buf); + OS.write_escaped(llvm::StringRef(ArMemHdr->LastModified, + sizeof(ArMemHdr->LastModified)).rtrim(" ")); + OS.flush(); + uint64_t Offset = reinterpret_cast(ArMemHdr) - + Parent->getData().data(); + return malformedError("characters in LastModified field in archive header " + "are not all decimal numbers: '" + Buf + "' for the " + "archive member header at offset " + Twine(Offset)); + } sys::TimeValue Ret; Ret.fromEpochTime(Seconds); return Ret; } -unsigned ArchiveMemberHeader::getUID() const { +Expected ArchiveMemberHeader::getUID() const { unsigned Ret; StringRef User = StringRef(ArMemHdr->UID, sizeof(ArMemHdr->UID)).rtrim(' '); if (User.empty()) return 0; - if (User.getAsInteger(10, Ret)) - llvm_unreachable("UID time not a decimal number."); + if (User.getAsInteger(10, Ret)) { + std::string Buf; + raw_string_ostream OS(Buf); + OS.write_escaped(User); + OS.flush(); + uint64_t Offset = reinterpret_cast(ArMemHdr) - + Parent->getData().data(); + return malformedError("characters in UID field in archive header " + "are not all decimal numbers: '" + Buf + "' for the " + "archive member header at offset " + Twine(Offset)); + } return Ret; } -unsigned ArchiveMemberHeader::getGID() const { +Expected ArchiveMemberHeader::getGID() const { unsigned Ret; StringRef Group = StringRef(ArMemHdr->GID, sizeof(ArMemHdr->GID)).rtrim(' '); if (Group.empty()) return 0; - if (Group.getAsInteger(10, Ret)) - llvm_unreachable("GID time not a decimal number."); + if (Group.getAsInteger(10, Ret)) { + std::string Buf; + raw_string_ostream OS(Buf); + OS.write_escaped(Group); + OS.flush(); + uint64_t Offset = reinterpret_cast(ArMemHdr) - + Parent->getData().data(); + return malformedError("characters in GID field in archive header " + "are not all decimal numbers: '" + Buf + "' for the " + "archive member header at offset " + Twine(Offset)); + } return Ret; } diff --git a/llvm/lib/Object/ArchiveWriter.cpp b/llvm/lib/Object/ArchiveWriter.cpp index 922d1b7..4ede536 100644 --- a/llvm/lib/Object/ArchiveWriter.cpp +++ b/llvm/lib/Object/ArchiveWriter.cpp @@ -47,10 +47,22 @@ NewArchiveMember::getOldMember(const object::Archive::Child &OldMember, NewArchiveMember M; M.Buf = MemoryBuffer::getMemBuffer(*BufOrErr, false); if (!Deterministic) { - M.ModTime = OldMember.getLastModified(); - M.UID = OldMember.getUID(); - M.GID = OldMember.getGID(); - M.Perms = OldMember.getAccessMode(); + Expected ModTimeOrErr = OldMember.getLastModified(); + if (!ModTimeOrErr) + return ModTimeOrErr.takeError(); + M.ModTime = ModTimeOrErr.get(); + Expected UIDOrErr = OldMember.getUID(); + if (!UIDOrErr) + return UIDOrErr.takeError(); + M.UID = UIDOrErr.get(); + Expected GIDOrErr = OldMember.getGID(); + if (!GIDOrErr) + return GIDOrErr.takeError(); + M.GID = GIDOrErr.get(); + Expected AccessModeOrErr = OldMember.getAccessMode(); + if (!AccessModeOrErr) + return AccessModeOrErr.takeError(); + M.Perms = AccessModeOrErr.get(); } return std::move(M); } diff --git a/llvm/test/tools/llvm-objdump/Inputs/libbogus11.a b/llvm/test/tools/llvm-objdump/Inputs/libbogus11.a new file mode 100644 index 0000000..99a709d --- /dev/null +++ b/llvm/test/tools/llvm-objdump/Inputs/libbogus11.a @@ -0,0 +1,10 @@ +! +hello.c 1444941273 ~97& 0 100644 102 ` +#include +#include +int +main() +{ + printf("Hello World\n"); + return EXIT_SUCCESS; +} diff --git a/llvm/test/tools/llvm-objdump/Inputs/libbogus12.a b/llvm/test/tools/llvm-objdump/Inputs/libbogus12.a new file mode 100644 index 0000000..fab3cfc --- /dev/null +++ b/llvm/test/tools/llvm-objdump/Inputs/libbogus12.a @@ -0,0 +1,10 @@ +! +hello.c 1444941273 124 #55! 100644 102 ` +#include +#include +int +main() +{ + printf("Hello World\n"); + return EXIT_SUCCESS; +} diff --git a/llvm/test/tools/llvm-objdump/Inputs/libbogus13.a b/llvm/test/tools/llvm-objdump/Inputs/libbogus13.a new file mode 100644 index 0000000..f6f80829 --- /dev/null +++ b/llvm/test/tools/llvm-objdump/Inputs/libbogus13.a @@ -0,0 +1,10 @@ +! +hello.c 1444941273 124 0 Feed 102 ` +#include +#include +int +main() +{ + printf("Hello World\n"); + return EXIT_SUCCESS; +} diff --git a/llvm/test/tools/llvm-objdump/Inputs/libbogus14.a b/llvm/test/tools/llvm-objdump/Inputs/libbogus14.a new file mode 100644 index 0000000..003cc98 --- /dev/null +++ b/llvm/test/tools/llvm-objdump/Inputs/libbogus14.a @@ -0,0 +1,10 @@ +! +hello.c 1foobar273 124 0 100644 102 ` +#include +#include +int +main() +{ + printf("Hello World\n"); + return EXIT_SUCCESS; +} diff --git a/llvm/test/tools/llvm-objdump/malformed-archives.test b/llvm/test/tools/llvm-objdump/malformed-archives.test index a9733d5..b8ba48d 100644 --- a/llvm/test/tools/llvm-objdump/malformed-archives.test +++ b/llvm/test/tools/llvm-objdump/malformed-archives.test @@ -58,3 +58,31 @@ # 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) + +# RUN: not llvm-objdump -macho -archive-headers \ +# RUN: %p/Inputs/libbogus11.a \ +# RUN: 2>&1 | FileCheck -check-prefix=bogus11 %s + +# bogus11: libbogus11.a(hello.c) truncated or malformed archive (characters in UID field in archive header are not all decimal numbers: '~97&' for the archive member header at offset 8) + +# RUN: not llvm-objdump -macho -archive-headers \ +# RUN: %p/Inputs/libbogus12.a \ +# RUN: 2>&1 | FileCheck -check-prefix=bogus12 %s + +# bogus12: libbogus12.a(hello.c) truncated or malformed archive (characters in GID field in archive header are not all decimal numbers: '#55!' for the archive member header at offset 8) + +# RUN: not llvm-objdump -macho -archive-headers \ +# RUN: %p/Inputs/libbogus13.a \ +# RUN: 2>&1 | FileCheck -check-prefix=bogus13 %s + +# bogus13: libbogus13.a(hello.c) truncated or malformed archive (characters in AccessMode field in archive header are not all decimal numbers: 'Feed' for the archive member header at offset 8) + +# RUN: llvm-objdump -macho -archive-headers %p/Inputs/libbogus14.a \ +# RUN: 2>&1 | FileCheck -check-prefix=bogus14 %s + +# bogus14: -rw-r--r--124/0 102 (date: "1foobar273" contains non-decimal chars) hello.c + +# RUN: not llvm-ar tv %p/Inputs/libbogus14.a \ +# RUN: 2>&1 | FileCheck -check-prefix=bogus14a %s + +# bogus14a: truncated or malformed archive (characters in LastModified field in archive header are not all decimal numbers: '1foobar273' for the archive member header at offset 8) diff --git a/llvm/tools/dsymutil/BinaryHolder.cpp b/llvm/tools/dsymutil/BinaryHolder.cpp index abb4ea0..579ffc2 100644 --- a/llvm/tools/dsymutil/BinaryHolder.cpp +++ b/llvm/tools/dsymutil/BinaryHolder.cpp @@ -106,8 +106,11 @@ BinaryHolder::GetArchiveMemberBuffers(StringRef Filename, for (auto Child : CurrentArchive->children(Err)) { if (auto NameOrErr = Child.getName()) { if (*NameOrErr == Filename) { + Expected ModTimeOrErr = Child.getLastModified(); + if (!ModTimeOrErr) + return errorToErrorCode(ModTimeOrErr.takeError()); if (Timestamp != sys::TimeValue::PosixZeroTime() && - Timestamp != Child.getLastModified()) { + Timestamp != ModTimeOrErr.get()) { if (Verbose) outs() << "\tmember had timestamp mismatch.\n"; continue; diff --git a/llvm/tools/llvm-ar/llvm-ar.cpp b/llvm/tools/llvm-ar/llvm-ar.cpp index f52f9c3..cf2f98f 100644 --- a/llvm/tools/llvm-ar/llvm-ar.cpp +++ b/llvm/tools/llvm-ar/llvm-ar.cpp @@ -338,16 +338,24 @@ static void printMode(unsigned mode) { // modification time are also printed. static void doDisplayTable(StringRef Name, const object::Archive::Child &C) { if (Verbose) { - sys::fs::perms Mode = C.getAccessMode(); + Expected ModeOrErr = C.getAccessMode(); + failIfError(ModeOrErr.takeError()); + sys::fs::perms Mode = ModeOrErr.get(); printMode((Mode >> 6) & 007); printMode((Mode >> 3) & 007); printMode(Mode & 007); - outs() << ' ' << C.getUID(); - outs() << '/' << C.getGID(); + Expected UIDOrErr = C.getUID(); + failIfError(UIDOrErr.takeError()); + outs() << ' ' << UIDOrErr.get(); + Expected GIDOrErr = C.getGID(); + failIfError(GIDOrErr.takeError()); + outs() << '/' << GIDOrErr.get(); Expected Size = C.getSize(); failIfError(Size.takeError()); outs() << ' ' << format("%6llu", Size.get()); - outs() << ' ' << C.getLastModified().str(); + Expected ModTimeOrErr = C.getLastModified(); + failIfError(ModTimeOrErr.takeError()); + outs() << ' ' << ModTimeOrErr.get().str(); outs() << ' '; } outs() << Name << "\n"; @@ -357,7 +365,9 @@ static void doDisplayTable(StringRef Name, const object::Archive::Child &C) { // system. static void doExtract(StringRef Name, const object::Archive::Child &C) { // Retain the original mode. - sys::fs::perms Mode = C.getAccessMode(); + Expected ModeOrErr = C.getAccessMode(); + failIfError(ModeOrErr.takeError()); + sys::fs::perms Mode = ModeOrErr.get(); int FD; failIfError(sys::fs::openFileForWrite(Name, FD, sys::fs::F_None, Mode), Name); @@ -374,9 +384,12 @@ static void doExtract(StringRef Name, const object::Archive::Child &C) { // If we're supposed to retain the original modification times, etc. do so // now. - if (OriginalDates) + if (OriginalDates) { + Expected ModTimeOrErr = C.getLastModified(); + failIfError(ModTimeOrErr.takeError()); failIfError( - sys::fs::setLastModificationAndAccessTime(FD, C.getLastModified())); + sys::fs::setLastModificationAndAccessTime(FD, ModTimeOrErr.get())); + } if (close(FD)) fail("Could not close the file"); @@ -511,7 +524,9 @@ static InsertAction computeInsertAction(ArchiveOperation Operation, // operation. sys::fs::file_status Status; failIfError(sys::fs::status(*MI, Status), *MI); - if (Status.getLastModificationTime() < Member.getLastModified()) { + Expected ModTimeOrErr = Member.getLastModified(); + failIfError(ModTimeOrErr.takeError()); + if (Status.getLastModificationTime() < ModTimeOrErr.get()) { if (PosName.empty()) return IA_AddOldMember; return IA_MoveOldMember; diff --git a/llvm/tools/llvm-objdump/MachODump.cpp b/llvm/tools/llvm-objdump/MachODump.cpp index 8d924e5..b5e7a06 100644 --- a/llvm/tools/llvm-objdump/MachODump.cpp +++ b/llvm/tools/llvm-objdump/MachODump.cpp @@ -1477,7 +1477,10 @@ static void printArchiveChild(StringRef Filename, const Archive::Child &C, StringRef ArchitectureName = StringRef()) { if (print_offset) outs() << C.getChildOffset() << "\t"; - sys::fs::perms Mode = C.getAccessMode(); + Expected ModeOrErr = C.getAccessMode(); + if (!ModeOrErr) + report_error(Filename, C, ModeOrErr.takeError(), ArchitectureName); + sys::fs::perms Mode = ModeOrErr.get(); 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. @@ -1495,9 +1498,15 @@ static void printArchiveChild(StringRef Filename, const Archive::Child &C, outs() << format("0%o ", Mode); } - unsigned UID = C.getUID(); + Expected UIDOrErr = C.getUID(); + if (!UIDOrErr) + report_error(Filename, C, UIDOrErr.takeError(), ArchitectureName); + unsigned UID = UIDOrErr.get(); outs() << format("%3d/", UID); - unsigned GID = C.getGID(); + Expected GIDOrErr = C.getGID(); + if (!GIDOrErr) + report_error(Filename, C, GIDOrErr.takeError(), ArchitectureName); + unsigned GID = GIDOrErr.get(); outs() << format("%-3d ", GID); Expected Size = C.getRawSize(); if (!Size) @@ -1508,7 +1517,8 @@ static void printArchiveChild(StringRef Filename, const Archive::Child &C, if (verbose) { unsigned Seconds; if (RawLastModified.getAsInteger(10, Seconds)) - outs() << "(date: \"%s\" contains non-decimal chars) " << RawLastModified; + outs() << "(date: \"" << RawLastModified + << "\" contains non-decimal chars) "; else { // Since cime(3) returns a 26 character string of the form: // "Sun Sep 16 01:03:52 1973\n\0" -- 2.7.4