[llvm] r277630 - Reapply "More fixes to get good error messages for bad archives."

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 3 12:02:51 PDT 2016


Author: vedantk
Date: Wed Aug  3 14:02:50 2016
New Revision: 277630

URL: http://llvm.org/viewvc/llvm-project?rev=277630&view=rev
Log:
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.

Added:
    llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus11.a
    llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus12.a
    llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus13.a
    llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus14.a
Modified:
    llvm/trunk/include/llvm/Object/Archive.h
    llvm/trunk/lib/Object/Archive.cpp
    llvm/trunk/lib/Object/ArchiveWriter.cpp
    llvm/trunk/test/tools/llvm-objdump/malformed-archives.test
    llvm/trunk/tools/dsymutil/BinaryHolder.cpp
    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=277630&r1=277629&r2=277630&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Object/Archive.h (original)
+++ llvm/trunk/include/llvm/Object/Archive.h Wed Aug  3 14:02:50 2016
@@ -44,14 +44,14 @@ public:
   /// 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 @@ public:
     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.

Modified: llvm/trunk/lib/Object/Archive.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/Archive.cpp?rev=277630&r1=277629&r2=277630&view=diff
==============================================================================
--- llvm/trunk/lib/Object/Archive.cpp (original)
+++ llvm/trunk/lib/Object/Archive.cpp Wed Aug  3 14:02:50 2016
@@ -221,43 +221,81 @@ Expected<uint32_t> ArchiveMemberHeader::
   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;
 }
 

Modified: llvm/trunk/lib/Object/ArchiveWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/ArchiveWriter.cpp?rev=277630&r1=277629&r2=277630&view=diff
==============================================================================
--- llvm/trunk/lib/Object/ArchiveWriter.cpp (original)
+++ llvm/trunk/lib/Object/ArchiveWriter.cpp Wed Aug  3 14:02:50 2016
@@ -47,10 +47,22 @@ NewArchiveMember::getOldMember(const obj
   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);
 }

Added: llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus11.a
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus11.a?rev=277630&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus11.a (added)
+++ llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus11.a Wed Aug  3 14:02:50 2016
@@ -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;
+}

Added: llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus12.a
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus12.a?rev=277630&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus12.a (added)
+++ llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus12.a Wed Aug  3 14:02:50 2016
@@ -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;
+}

Added: llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus13.a
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus13.a?rev=277630&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus13.a (added)
+++ llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus13.a Wed Aug  3 14:02:50 2016
@@ -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;
+}

Added: llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus14.a
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus14.a?rev=277630&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus14.a (added)
+++ llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus14.a Wed Aug  3 14:02:50 2016
@@ -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;
+}

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=277630&r1=277629&r2=277630&view=diff
==============================================================================
--- llvm/trunk/test/tools/llvm-objdump/malformed-archives.test (original)
+++ llvm/trunk/test/tools/llvm-objdump/malformed-archives.test Wed Aug  3 14:02:50 2016
@@ -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)

Modified: llvm/trunk/tools/dsymutil/BinaryHolder.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/BinaryHolder.cpp?rev=277630&r1=277629&r2=277630&view=diff
==============================================================================
--- llvm/trunk/tools/dsymutil/BinaryHolder.cpp (original)
+++ llvm/trunk/tools/dsymutil/BinaryHolder.cpp Wed Aug  3 14:02:50 2016
@@ -106,8 +106,11 @@ BinaryHolder::GetArchiveMemberBuffers(St
     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;

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=277630&r1=277629&r2=277630&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-ar/llvm-ar.cpp (original)
+++ llvm/trunk/tools/llvm-ar/llvm-ar.cpp Wed Aug  3 14:02:50 2016
@@ -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<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 @@ static void doDisplayTable(StringRef Nam
 // 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 @@ static void doExtract(StringRef Name, co
 
   // 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 @@ static InsertAction computeInsertAction(
     // 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;

Modified: llvm/trunk/tools/llvm-objdump/MachODump.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/MachODump.cpp?rev=277630&r1=277629&r2=277630&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objdump/MachODump.cpp (original)
+++ llvm/trunk/tools/llvm-objdump/MachODump.cpp Wed Aug  3 14:02:50 2016
@@ -1477,7 +1477,10 @@ static void printArchiveChild(StringRef
                               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 @@ static void printArchiveChild(StringRef
     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 @@ static void printArchiveChild(StringRef
   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"




More information about the llvm-commits mailing list