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
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 @@
/// Members are not larger than 4GB.
Expected<uint32_t> getSize() const;
- sys::fs::perms getAccessMode() const;
- sys::TimeValue getLastModified() const;
+ Expected<sys::fs::perms> getAccessMode() const;
+ Expected<sys::TimeValue> getLastModified() const;
llvm::StringRef getRawLastModified() const {
return StringRef(ArMemHdr->LastModified,
sizeof(ArMemHdr->LastModified)).rtrim(' ');
}
- unsigned getUID() const;
- unsigned getGID() const;
+ Expected<unsigned> getUID() const;
+ Expected<unsigned> getGID() const;
// This returns the size of the private struct ArMemHdrType
uint64_t getSizeOf() const {
@@ -102,15 +102,15 @@
Expected<StringRef> getName() const;
ErrorOr<std::string> getFullName() const;
Expected<StringRef> getRawName() const { return Header.getRawName(); }
- sys::TimeValue getLastModified() const {
+ Expected<sys::TimeValue> 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<unsigned> getUID() const { return Header.getUID(); }
+ Expected<unsigned> getGID() const { return Header.getGID(); }
+ Expected<sys::fs::perms> 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 @@
return Ret;
}
-sys::fs::perms ArchiveMemberHeader::getAccessMode() const {
+Expected<sys::fs::perms> 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<const char *>(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<sys::fs::perms>(Ret);
}
-sys::TimeValue ArchiveMemberHeader::getLastModified() const {
+Expected<sys::TimeValue> 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<const char *>(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<unsigned> 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<const char *>(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<unsigned> 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<const char *>(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 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<sys::TimeValue> ModTimeOrErr = OldMember.getLastModified();
+ if (!ModTimeOrErr)
+ return ModTimeOrErr.takeError();
+ M.ModTime = ModTimeOrErr.get();
+ Expected<unsigned> UIDOrErr = OldMember.getUID();
+ if (!UIDOrErr)
+ return UIDOrErr.takeError();
+ M.UID = UIDOrErr.get();
+ Expected<unsigned> GIDOrErr = OldMember.getGID();
+ if (!GIDOrErr)
+ return GIDOrErr.takeError();
+ M.GID = GIDOrErr.get();
+ Expected<sys::fs::perms> 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 @@
+!<arch>
+hello.c 1444941273 ~97& 0 100644 102 `
+#include <stdio.h>
+#include <stdlib.h>
+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 @@
+!<arch>
+hello.c 1444941273 124 #55! 100644 102 `
+#include <stdio.h>
+#include <stdlib.h>
+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..f6f8082
--- /dev/null
+++ b/llvm/test/tools/llvm-objdump/Inputs/libbogus13.a
@@ -0,0 +1,10 @@
+!<arch>
+hello.c 1444941273 124 0 Feed 102 `
+#include <stdio.h>
+#include <stdlib.h>
+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 @@
+!<arch>
+hello.c 1foobar273 124 0 100644 102 `
+#include <stdio.h>
+#include <stdlib.h>
+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 @@
for (auto Child : CurrentArchive->children(Err)) {
if (auto NameOrErr = Child.getName()) {
if (*NameOrErr == Filename) {
+ Expected<sys::TimeValue> 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 @@
// modification time are also printed.
static void doDisplayTable(StringRef Name, const object::Archive::Child &C) {
if (Verbose) {
- sys::fs::perms Mode = C.getAccessMode();
+ Expected<sys::fs::perms> 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<unsigned> UIDOrErr = C.getUID();
+ failIfError(UIDOrErr.takeError());
+ outs() << ' ' << UIDOrErr.get();
+ Expected<unsigned> GIDOrErr = C.getGID();
+ failIfError(GIDOrErr.takeError());
+ outs() << '/' << GIDOrErr.get();
Expected<uint64_t> Size = C.getSize();
failIfError(Size.takeError());
outs() << ' ' << format("%6llu", Size.get());
- outs() << ' ' << C.getLastModified().str();
+ Expected<sys::TimeValue> ModTimeOrErr = C.getLastModified();
+ failIfError(ModTimeOrErr.takeError());
+ outs() << ' ' << ModTimeOrErr.get().str();
outs() << ' ';
}
outs() << Name << "\n";
@@ -357,7 +365,9 @@
// system.
static void doExtract(StringRef Name, const object::Archive::Child &C) {
// Retain the original mode.
- sys::fs::perms Mode = C.getAccessMode();
+ Expected<sys::fs::perms> 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 @@
// If we're supposed to retain the original modification times, etc. do so
// now.
- if (OriginalDates)
+ if (OriginalDates) {
+ Expected<sys::TimeValue> 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 @@
// operation.
sys::fs::file_status Status;
failIfError(sys::fs::status(*MI, Status), *MI);
- if (Status.getLastModificationTime() < Member.getLastModified()) {
+ Expected<sys::TimeValue> 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 @@
StringRef ArchitectureName = StringRef()) {
if (print_offset)
outs() << C.getChildOffset() << "\t";
- sys::fs::perms Mode = C.getAccessMode();
+ Expected<sys::fs::perms> 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 @@
outs() << format("0%o ", Mode);
}
- unsigned UID = C.getUID();
+ Expected<unsigned> 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<unsigned> GIDOrErr = C.getGID();
+ if (!GIDOrErr)
+ report_error(Filename, C, GIDOrErr.takeError(), ArchitectureName);
+ unsigned GID = GIDOrErr.get();
outs() << format("%-3d ", GID);
Expected<uint64_t> Size = C.getRawSize();
if (!Size)
@@ -1508,7 +1517,8 @@
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"