Revert "More fixes to get good error messages for bad archives."

This reverts commit r277540. It breaks the build with:

../lib/Object/Archive.cpp:264:41: error: return type of out-of-line definition of 'llvm::object::ArchiveMemberHeader::getUID' differs from that in the declaration
Expected<unsigned> ArchiveMemberHeader::getUID() const {
~~~~~~~~~~~~~~~~~~                      ^
include/llvm/Object/Archive.h:53:12: note: previous declaration is here
  unsigned getUID() const;
  ~~~~~~~~ ^

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@277627 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/Object/Archive.h

@@ -44,14 +44,14 @@ public:
   /// Members are not larger than 4GB.
   Expected<uint32_t> getSize() const;
 
-  Expected<sys::fs::perms> getAccessMode() const;
-  Expected<sys::TimeValue> getLastModified() const;
+  sys::fs::perms getAccessMode() const;
+  sys::TimeValue getLastModified() const;
   llvm::StringRef getRawLastModified() const {
     return StringRef(ArMemHdr->LastModified,
                      sizeof(ArMemHdr->LastModified)).rtrim(' ');
   }
-  Expected<unsigned> getUID() const;
-  Expected<unsigned> getGID() const;
+  unsigned getUID() const;
+  unsigned getGID() const;
 
   // This returns the size of the private struct ArMemHdrType
   uint64_t getSizeOf() const {
@@ -102,15 +102,15 @@ public:
     Expected<StringRef> getName() const;
     ErrorOr<std::string> getFullName() const;
     Expected<StringRef> getRawName() const { return Header.getRawName(); }
-    Expected<sys::TimeValue> getLastModified() const {
+    sys::TimeValue getLastModified() const {
       return Header.getLastModified();
     }
     StringRef getRawLastModified() const {
       return Header.getRawLastModified();
     }
-    Expected<unsigned> getUID() const { return Header.getUID(); }
-    Expected<unsigned> getGID() const { return Header.getGID(); }
-    Expected<sys::fs::perms> getAccessMode() const {
+    unsigned getUID() const { return Header.getUID(); }
+    unsigned getGID() const { return Header.getGID(); }
+    sys::fs::perms getAccessMode() const {
       return Header.getAccessMode();
     }
     /// \return the size of the archive member without the header or padding.

lib/Object/Archive.cpp

@@ -221,81 +221,43 @@ Expected<uint32_t> ArchiveMemberHeader::getSize() const {
   return Ret;
 }
 
-Expected<sys::fs::perms> ArchiveMemberHeader::getAccessMode() const {
+sys::fs::perms ArchiveMemberHeader::getAccessMode() const {
   unsigned Ret;
   if (StringRef(ArMemHdr->AccessMode,
-                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));
-  }
+                sizeof(ArMemHdr->AccessMode)).rtrim(' ').getAsInteger(8, Ret))
+    llvm_unreachable("Access mode is not an octal number.");
   return static_cast<sys::fs::perms>(Ret);
 }
 
-Expected<sys::TimeValue> ArchiveMemberHeader::getLastModified() const {
+sys::TimeValue ArchiveMemberHeader::getLastModified() const {
   unsigned Seconds;
   if (StringRef(ArMemHdr->LastModified,
                 sizeof(ArMemHdr->LastModified)).rtrim(' ')
-          .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));
-  }
+          .getAsInteger(10, Seconds))
+    llvm_unreachable("Last modified time not a decimal number.");
 
   sys::TimeValue Ret;
   Ret.fromEpochTime(Seconds);
   return Ret;
 }
 
-Expected<unsigned> ArchiveMemberHeader::getUID() const {
+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)) {
-    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));
-  }
+  if (User.getAsInteger(10, Ret))
+    llvm_unreachable("UID time not a decimal number.");
   return Ret;
 }
 
-Expected<unsigned> ArchiveMemberHeader::getGID() const {
+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)) {
-    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));
-  }
+  if (Group.getAsInteger(10, Ret))
+    llvm_unreachable("GID time not a decimal number.");
   return Ret;
 }
 

lib/Object/ArchiveWriter.cpp

@@ -47,22 +47,10 @@ NewArchiveMember::getOldMember(const object::Archive::Child &OldMember,
   NewArchiveMember M;
   M.Buf = MemoryBuffer::getMemBuffer(*BufOrErr, false);
   if (!Deterministic) {
-    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();
+    M.ModTime = OldMember.getLastModified();
+    M.UID = OldMember.getUID();
+    M.GID = OldMember.getGID();
+    M.Perms = OldMember.getAccessMode();
   }
   return std::move(M);
 }

test/tools/llvm-objdump/malformed-archives.test

@@ -58,31 +58,3 @@
 # 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)

tools/dsymutil/BinaryHolder.cpp

@@ -106,11 +106,8 @@ BinaryHolder::GetArchiveMemberBuffers(StringRef Filename,
     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 != ModTimeOrErr.get()) {
+              Timestamp != Child.getLastModified()) {
             if (Verbose)
               outs() << "\tmember had timestamp mismatch.\n";
             continue;

tools/llvm-ar/llvm-ar.cpp

@@ -338,24 +338,16 @@ static void printMode(unsigned mode) {
 // modification time are also printed.
 static void doDisplayTable(StringRef Name, const object::Archive::Child &C) {
   if (Verbose) {
-    Expected<sys::fs::perms> ModeOrErr = C.getAccessMode();
-    failIfError(ModeOrErr.takeError());
-    sys::fs::perms Mode = ModeOrErr.get();
+    sys::fs::perms Mode = C.getAccessMode();
     printMode((Mode >> 6) & 007);
     printMode((Mode >> 3) & 007);
     printMode(Mode & 007);
-    Expected<unsigned> UIDOrErr = C.getUID();
-    failIfError(UIDOrErr.takeError());
-    outs() << ' ' << UIDOrErr.get();
-    Expected<unsigned> GIDOrErr = C.getGID();
-    failIfError(GIDOrErr.takeError());
-    outs() << '/' << GIDOrErr.get();
+    outs() << ' ' << C.getUID();
+    outs() << '/' << C.getGID();
     Expected<uint64_t> Size = C.getSize();
     failIfError(Size.takeError());
     outs() << ' ' << format("%6llu", Size.get());
-    Expected<sys::TimeValue> ModTimeOrErr = C.getLastModified();
-    failIfError(ModTimeOrErr.takeError());
-    outs() << ' ' << ModTimeOrErr.get().str();
+    outs() << ' ' << C.getLastModified().str();
     outs() << ' ';
   }
   outs() << Name << "\n";
@@ -365,9 +357,7 @@ 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.
-  Expected<sys::fs::perms> ModeOrErr = C.getAccessMode();
-  failIfError(ModeOrErr.takeError());
-  sys::fs::perms Mode = ModeOrErr.get();
+  sys::fs::perms Mode = C.getAccessMode();
 
   int FD;
   failIfError(sys::fs::openFileForWrite(Name, FD, sys::fs::F_None, Mode), Name);
@@ -384,12 +374,9 @@ 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) {
-    Expected<sys::TimeValue> ModTimeOrErr = C.getLastModified();
-    failIfError(ModTimeOrErr.takeError());
+  if (OriginalDates)
     failIfError(
-        sys::fs::setLastModificationAndAccessTime(FD, ModTimeOrErr.get()));
-  }
+        sys::fs::setLastModificationAndAccessTime(FD, C.getLastModified()));
 
   if (close(FD))
     fail("Could not close the file");
@@ -524,9 +511,7 @@ static InsertAction computeInsertAction(ArchiveOperation Operation,
     // operation.
     sys::fs::file_status Status;
     failIfError(sys::fs::status(*MI, Status), *MI);
-    Expected<sys::TimeValue> ModTimeOrErr = Member.getLastModified();
-    failIfError(ModTimeOrErr.takeError());
-    if (Status.getLastModificationTime() < ModTimeOrErr.get()) {
+    if (Status.getLastModificationTime() < Member.getLastModified()) {
       if (PosName.empty())
         return IA_AddOldMember;
       return IA_MoveOldMember;

tools/llvm-objdump/MachODump.cpp

@@ -1477,10 +1477,7 @@ static void printArchiveChild(StringRef Filename, const Archive::Child &C,
                               StringRef ArchitectureName = StringRef()) {
   if (print_offset)
     outs() << C.getChildOffset() << "\t";
-  Expected<sys::fs::perms> ModeOrErr = C.getAccessMode();
-  if (!ModeOrErr)
-    report_error(Filename, C, ModeOrErr.takeError(), ArchitectureName);
-  sys::fs::perms Mode = ModeOrErr.get();
+  sys::fs::perms Mode = C.getAccessMode();
   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.
@@ -1498,15 +1495,9 @@ static void printArchiveChild(StringRef Filename, const Archive::Child &C,
     outs() << format("0%o ", Mode);
   }
 
-  Expected<unsigned> UIDOrErr = C.getUID();
-  if (!UIDOrErr)
-    report_error(Filename, C, UIDOrErr.takeError(), ArchitectureName);
-  unsigned UID = UIDOrErr.get();
+  unsigned UID = C.getUID();
   outs() << format("%3d/", UID);
-  Expected<unsigned> GIDOrErr = C.getGID();
-  if (!GIDOrErr)
-    report_error(Filename, C, GIDOrErr.takeError(), ArchitectureName);
-  unsigned GID = GIDOrErr.get();
+  unsigned GID = C.getGID();
   outs() << format("%-3d ", GID);
   Expected<uint64_t> Size = C.getRawSize();
   if (!Size)
@@ -1517,8 +1508,7 @@ static void printArchiveChild(StringRef Filename, const Archive::Child &C,
   if (verbose) {
     unsigned Seconds;
     if (RawLastModified.getAsInteger(10, Seconds))
-      outs() << "(date: \"" << RawLastModified
-             << "\" contains non-decimal chars) ";
+      outs() << "(date: \"%s\" contains non-decimal chars) " << RawLastModified;
     else {
       // Since cime(3) returns a 26 character string of the form:
       // "Sun Sep 16 01:03:52 1973\n\0"