[llvm] r250906 - This removes the eating of the error in Archive::Child::getSize() when the characters

Kevin Enderby via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 21 09:59:25 PDT 2015


Author: enderby
Date: Wed Oct 21 11:59:24 2015
New Revision: 250906

URL: http://llvm.org/viewvc/llvm-project?rev=250906&view=rev
Log:
This removes the eating of the error in Archive::Child::getSize() when the characters
in the size field in the archive header for the member is not a number.  To do this we
have all of the needed methods return ErrorOr to push them up until we get out of lib.
Then the tools and can handle the error in whatever way is appropriate for that tool.

So the solution is to plumb all the ErrorOr stuff through everything that touches archives.
This include its iterators as one can create an Archive object but the first or any other
Child object may fail to be created due to a bad size field in its header.

Thanks to Lang Hames on the changes making child_iterator contain an
ErrorOr<Child> instead of a Child and the needed changes to ErrorOr.h to add
operator overloading for * and -> .

We don’t want to use llvm_unreachable() as it calls abort() and is produces a “crash”
and using report_fatal_error() to move the error checking will cause the program to
stop, neither of which are really correct in library code. There are still some uses of
these that should be cleaned up in this library code for other than the size field.

Also corrected the code where the size gets us to the “at the end of the archive”
which is OK but past the end of the archive will return object_error::parse_failed now.

The test cases use archives with text files so one can see the non-digit character,
in this case a ‘%’, in the size field.

Added:
    llvm/trunk/test/tools/llvm-objdump/Inputs/malformed-archives/
    llvm/trunk/test/tools/llvm-objdump/Inputs/malformed-archives/libbogus1.a
    llvm/trunk/test/tools/llvm-objdump/Inputs/malformed-archives/libbogus2.a
    llvm/trunk/test/tools/llvm-objdump/Inputs/malformed-archives/libbogus3.a
    llvm/trunk/test/tools/llvm-objdump/malformed-archives.test
Modified:
    llvm/trunk/include/llvm/Object/Archive.h
    llvm/trunk/include/llvm/Support/ErrorOr.h
    llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp
    llvm/trunk/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h
    llvm/trunk/lib/Object/Archive.cpp
    llvm/trunk/lib/Object/ArchiveWriter.cpp
    llvm/trunk/tools/dsymutil/BinaryHolder.cpp
    llvm/trunk/tools/llvm-ar/llvm-ar.cpp
    llvm/trunk/tools/llvm-cxxdump/llvm-cxxdump.cpp
    llvm/trunk/tools/llvm-nm/llvm-nm.cpp
    llvm/trunk/tools/llvm-objdump/MachODump.cpp
    llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp
    llvm/trunk/tools/llvm-readobj/llvm-readobj.cpp
    llvm/trunk/tools/llvm-size/llvm-size.cpp

Modified: llvm/trunk/include/llvm/Object/Archive.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/Archive.h?rev=250906&r1=250905&r2=250906&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Object/Archive.h (original)
+++ llvm/trunk/include/llvm/Object/Archive.h Wed Oct 21 11:59:24 2015
@@ -65,7 +65,10 @@ public:
     bool isThinMember() const;
 
   public:
-    Child(const Archive *Parent, const char *Start);
+    Child(const Archive *Parent, const char *Start,
+          std::error_code *EC);
+    static ErrorOr<std::unique_ptr<Child>> create(const Archive *Parent,
+                                                  const char *Start);
 
     bool operator ==(const Child &other) const {
       assert(Parent == other.Parent);
@@ -77,7 +80,7 @@ public:
     }
 
     const Archive *getParent() const { return Parent; }
-    Child getNext() const;
+    ErrorOr<Child> getNext() const;
 
     ErrorOr<StringRef> getName() const;
     StringRef getRawName() const { return getHeader()->getName(); }
@@ -93,9 +96,9 @@ public:
       return getHeader()->getAccessMode();
     }
     /// \return the size of the archive member without the header or padding.
-    uint64_t getSize() const;
+    ErrorOr<uint64_t> getSize() const;
     /// \return the size in the archive header for this member.
-    uint64_t getRawSize() const;
+    ErrorOr<uint64_t> getRawSize() const;
 
     ErrorOr<StringRef> getBuffer() const;
     uint64_t getChildOffset() const;
@@ -107,28 +110,35 @@ public:
   };
 
   class child_iterator {
-    Child child;
+    ErrorOr<Child> child;
 
   public:
-    child_iterator() : child(Child(nullptr, nullptr)) {}
+    child_iterator() : child(Child(nullptr, nullptr, nullptr)) {}
     child_iterator(const Child &c) : child(c) {}
-    const Child *operator->() const { return &child; }
-    const Child &operator*() const { return child; }
+    child_iterator(std::error_code EC) : child(EC) {}
+    const ErrorOr<Child> *operator->() const { return &child; }
+    const ErrorOr<Child> &operator*() const { return child; }
 
     bool operator==(const child_iterator &other) const {
-      return child == other.child;
+      if ((*this)->getError())
+        return false;
+      if (other->getError())
+        return false;
+      return (*this)->get() == other->get();
     }
 
     bool operator!=(const child_iterator &other) const {
       return !(*this == other);
     }
 
-    bool operator<(const child_iterator &other) const {
-      return child < other.child;
-    }
+    // No operator< as we can't do less than compares with iterators that
+    // contain errors.
 
+    // Code in loops with child_iterators must check for errors on each loop
+    // iteration.  And if there is an error break out of the loop.
     child_iterator &operator++() { // Preincrement
-      child = child.getNext();
+      assert(child && "Can't increment iterator with error");
+      child = child->getNext();
       return *this;
     }
   };
@@ -212,7 +222,7 @@ public:
   StringRef getSymbolTable() const {
     // We know that the symbol table is not an external file,
     // so we just assert there is no error.
-    return *SymbolTable->getBuffer();
+    return *(*SymbolTable)->getBuffer();
   }
   uint32_t getNumberOfSymbols() const;
 

Modified: llvm/trunk/include/llvm/Support/ErrorOr.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/ErrorOr.h?rev=250906&r1=250905&r2=250906&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/ErrorOr.h (original)
+++ llvm/trunk/include/llvm/Support/ErrorOr.h Wed Oct 21 11:59:24 2015
@@ -91,6 +91,7 @@ private:
   typedef typename std::remove_reference<T>::type &reference;
   typedef const typename std::remove_reference<T>::type &const_reference;
   typedef typename std::remove_reference<T>::type *pointer;
+  typedef const typename std::remove_reference<T>::type *const_pointer;
 
 public:
   template <class E>
@@ -183,10 +184,18 @@ public:
     return toPointer(getStorage());
   }
 
+  const_pointer operator ->() const {
+    return toPointer(getStorage());
+  }
+
   reference operator *() {
     return *getStorage();
   }
 
+  const_reference operator *() const {
+    return *getStorage();
+  }
+
 private:
   template <class OtherT>
   void copyConstruct(const ErrorOr<OtherT> &Other) {
@@ -246,10 +255,19 @@ private:
     return Val;
   }
 
+  const_pointer toPointer(const_pointer Val) const {
+    return Val;
+  }
+
   pointer toPointer(wrap *Val) {
     return &Val->get();
   }
 
+  const_pointer toPointer(const wrap *Val) const {
+    return &Val->get();
+  }
+
+
   storage_type *getStorage() {
     assert(!HasError && "Cannot get value when an error exists!");
     return reinterpret_cast<storage_type*>(TStorage.buffer);

Modified: llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp?rev=250906&r1=250905&r2=250906&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp Wed Oct 21 11:59:24 2015
@@ -318,10 +318,10 @@ RuntimeDyld::SymbolInfo MCJIT::findSymbo
     object::Archive *A = OB.getBinary();
     // Look for our symbols in each Archive
     object::Archive::child_iterator ChildIt = A->findSym(Name);
-    if (ChildIt != A->child_end()) {
+    if (*ChildIt && ChildIt != A->child_end()) {
       // FIXME: Support nested archives?
       ErrorOr<std::unique_ptr<object::Binary>> ChildBinOrErr =
-          ChildIt->getAsBinary();
+        (*ChildIt)->getAsBinary();
       if (ChildBinOrErr.getError())
         continue;
       std::unique_ptr<object::Binary> &ChildBin = ChildBinOrErr.get();

Modified: llvm/trunk/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h?rev=250906&r1=250905&r2=250906&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h Wed Oct 21 11:59:24 2015
@@ -253,10 +253,10 @@ private:
       object::Archive *A = OB.getBinary();
       // Look for our symbols in each Archive
       object::Archive::child_iterator ChildIt = A->findSym(Name);
-      if (ChildIt != A->child_end()) {
+      if (*ChildIt && ChildIt != A->child_end()) {
         // FIXME: Support nested archives?
         ErrorOr<std::unique_ptr<object::Binary>> ChildBinOrErr =
-            ChildIt->getAsBinary();
+          (*ChildIt)->getAsBinary();
         if (ChildBinOrErr.getError())
           continue;
         std::unique_ptr<object::Binary> &ChildBin = ChildBinOrErr.get();

Modified: llvm/trunk/lib/Object/Archive.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/Archive.cpp?rev=250906&r1=250905&r2=250906&view=diff
==============================================================================
--- llvm/trunk/lib/Object/Archive.cpp (original)
+++ llvm/trunk/lib/Object/Archive.cpp Wed Oct 21 11:59:24 2015
@@ -46,7 +46,7 @@ StringRef ArchiveMemberHeader::getName()
 ErrorOr<uint32_t> ArchiveMemberHeader::getSize() const {
   uint32_t Ret;
   if (llvm::StringRef(Size, sizeof(Size)).rtrim(" ").getAsInteger(10, Ret))
-    return object_error::parse_failed;
+    return object_error::parse_failed; // Size is not a decimal number.
   return Ret;
 }
 
@@ -82,7 +82,8 @@ unsigned ArchiveMemberHeader::getGID() c
   return Ret;
 }
 
-Archive::Child::Child(const Archive *Parent, const char *Start)
+Archive::Child::Child(const Archive *Parent, const char *Start,
+                      std::error_code *EC)
     : Parent(Parent) {
   if (!Start)
     return;
@@ -90,7 +91,13 @@ Archive::Child::Child(const Archive *Par
   uint64_t Size = sizeof(ArchiveMemberHeader);
   Data = StringRef(Start, Size);
   if (!isThinMember()) {
-    Size += getRawSize();
+    ErrorOr<uint64_t> MemberSize = getRawSize();
+    if (MemberSize.getError()) {
+      assert (EC && "Error must be caught");
+      *EC = MemberSize.getError();
+      return;
+    }
+    Size += MemberSize.get();
     Data = StringRef(Start, Size);
   }
 
@@ -100,26 +107,38 @@ Archive::Child::Child(const Archive *Par
   StringRef Name = getRawName();
   if (Name.startswith("#1/")) {
     uint64_t NameSize;
-    if (Name.substr(3).rtrim(" ").getAsInteger(10, NameSize))
-      llvm_unreachable("Long name length is not an integer");
+    if (Name.substr(3).rtrim(" ").getAsInteger(10, NameSize)) {
+      if (EC)
+        *EC = object_error::parse_failed; // Long name offset is not an integer.
+      return;
+    }
     StartOfFile += NameSize;
   }
 }
 
-uint64_t Archive::Child::getSize() const {
+ErrorOr<std::unique_ptr<Archive::Child>> Archive::Child::create(
+  const Archive *Parent, const char *Start) {
+  std::error_code EC;
+  std::unique_ptr<Archive::Child> Ret(new Archive::Child(Parent, Start, &EC));
+  if (EC)
+    return EC;
+  return std::move(Ret);
+}
+
+ErrorOr<uint64_t> Archive::Child::getSize() const {
   if (Parent->IsThin) {
     ErrorOr<uint32_t> Size = getHeader()->getSize();
-    if (Size.getError())
-      return 0;
+    if (std::error_code EC = Size.getError())
+      return EC;
     return Size.get();
   }
   return Data.size() - StartOfFile;
 }
 
-uint64_t Archive::Child::getRawSize() const {
+ErrorOr<uint64_t> Archive::Child::getRawSize() const {
   ErrorOr<uint32_t> Size = getHeader()->getSize();
-  if (Size.getError())
-    return 0;
+  if (std::error_code EC = Size.getError())
+    return EC;
   return Size.get();
 }
 
@@ -129,8 +148,12 @@ bool Archive::Child::isThinMember() cons
 }
 
 ErrorOr<StringRef> Archive::Child::getBuffer() const {
-  if (!isThinMember())
-    return StringRef(Data.data() + StartOfFile, getSize());
+  if (!isThinMember()) {
+    ErrorOr<uint32_t> Size = getSize();
+    if (std::error_code EC = Size.getError())
+      return EC;
+    return StringRef(Data.data() + StartOfFile, Size.get());
+  }
   ErrorOr<StringRef> Name = getName();
   if (std::error_code EC = Name.getError())
     return EC;
@@ -144,19 +167,28 @@ ErrorOr<StringRef> Archive::Child::getBu
   return Parent->ThinBuffers.back()->getBuffer();
 }
 
-Archive::Child Archive::Child::getNext() const {
+ErrorOr<Archive::Child> Archive::Child::getNext() const {
   size_t SpaceToSkip = Data.size();
   // If it's odd, add 1 to make it even.
+  size_t Pad = 0;
   if (SpaceToSkip & 1)
-    ++SpaceToSkip;
+    Pad++;
+
+  const char *NextLoc = Data.data() + SpaceToSkip + Pad;
 
-  const char *NextLoc = Data.data() + SpaceToSkip;
+  // Check to see if this is at the end of the archive.
+  if (NextLoc == Parent->Data.getBufferEnd() ||
+      NextLoc == Parent->Data.getBufferEnd() - Pad )
+    return Child(Parent, nullptr, nullptr);
 
   // Check to see if this is past the end of the archive.
-  if (NextLoc >= Parent->Data.getBufferEnd())
-    return Child(Parent, nullptr);
+  if (NextLoc > Parent->Data.getBufferEnd())
+    return object_error::parse_failed;
 
-  return Child(Parent, NextLoc);
+  auto ChildOrErr = Child::create(Parent, NextLoc);
+  if (std::error_code EC = ChildOrErr.getError())
+    return EC;
+  return std::move(*ChildOrErr.get());
 }
 
 uint64_t Archive::Child::getChildOffset() const {
@@ -178,17 +210,23 @@ ErrorOr<StringRef> Archive::Child::getNa
     // Get the offset.
     std::size_t offset;
     if (name.substr(1).rtrim(" ").getAsInteger(10, offset))
-      llvm_unreachable("Long name offset is not an integer");
-    const char *addr = Parent->StringTable->Data.begin()
+      return object_error::parse_failed; // Long name offset is not an integer.
+    // Check for bad stringtable iterator.
+    if (std::error_code EC = Parent->StringTable->getError())
+      return EC;
+    const char *addr = (*Parent->StringTable)->Data.begin()
                        + sizeof(ArchiveMemberHeader)
                        + offset;
     // Verify it.
+    auto Size = (*Parent->StringTable)->getSize();
+    if (std::error_code EC = Size.getError())
+      return EC;
     if (Parent->StringTable == Parent->child_end()
-        || addr < (Parent->StringTable->Data.begin()
+        || addr < ((*Parent->StringTable)->Data.begin()
                    + sizeof(ArchiveMemberHeader))
-        || addr > (Parent->StringTable->Data.begin()
+        || addr > ((*Parent->StringTable)->Data.begin()
                    + sizeof(ArchiveMemberHeader)
-                   + Parent->StringTable->getSize()))
+                   + Size.get()))
       return object_error::parse_failed;
 
     // GNU long file names end with a "/\n".
@@ -200,7 +238,7 @@ ErrorOr<StringRef> Archive::Child::getNa
   } else if (name.startswith("#1/")) {
     uint64_t name_size;
     if (name.substr(3).rtrim(" ").getAsInteger(10, name_size))
-      llvm_unreachable("Long name length is not an ingeter");
+      return object_error::parse_failed; // Long name offset is not an integer.
     return Data.substr(sizeof(ArchiveMemberHeader), name_size)
         .rtrim(StringRef("\0", 1));
   }
@@ -256,12 +294,12 @@ Archive::Archive(MemoryBufferRef Source,
   child_iterator i = child_begin(false);
   child_iterator e = child_end();
 
-  if (i == e) {
-    ec = std::error_code();
+  if (!*i || i == e) {
+    ec = i->getError();
     return;
   }
 
-  StringRef Name = i->getRawName();
+  StringRef Name = (*i)->getRawName();
 
   // Below is the pattern that is used to figure out the archive format
   // GNU archive format
@@ -286,6 +324,11 @@ Archive::Archive(MemoryBufferRef Source,
     Format = K_BSD;
     SymbolTable = i;
     ++i;
+    if (!*i) {
+      ec = i->getError();
+      return;
+    }
+
     FirstRegular = i;
     ec = std::error_code();
     return;
@@ -294,7 +337,7 @@ Archive::Archive(MemoryBufferRef Source,
   if (Name.startswith("#1/")) {
     Format = K_BSD;
     // We know this is BSD, so getName will work since there is no string table.
-    ErrorOr<StringRef> NameOrErr = i->getName();
+    ErrorOr<StringRef> NameOrErr = (*i)->getName();
     ec = NameOrErr.getError();
     if (ec)
       return;
@@ -302,6 +345,10 @@ Archive::Archive(MemoryBufferRef Source,
     if (Name == "__.SYMDEF SORTED" || Name == "__.SYMDEF") {
       SymbolTable = i;
       ++i;
+      if (!*i) {
+        ec = i->getError();
+        return;
+      }
     }
     FirstRegular = i;
     return;
@@ -319,17 +366,21 @@ Archive::Archive(MemoryBufferRef Source,
       has64SymTable = true;
 
     ++i;
-    if (i == e) {
-      ec = std::error_code();
+    if (!*i || i == e) {
+      ec = i->getError();
       return;
     }
-    Name = i->getRawName();
+    Name = (*i)->getRawName();
   }
 
   if (Name == "//") {
     Format = has64SymTable ? K_MIPS64 : K_GNU;
     StringTable = i;
     ++i;
+    if (!*i) {
+      ec = i->getError();
+      return;
+    }
     FirstRegular = i;
     ec = std::error_code();
     return;
@@ -351,17 +402,25 @@ Archive::Archive(MemoryBufferRef Source,
   SymbolTable = i;
 
   ++i;
+  if (!*i) {
+    ec = i->getError();
+    return;
+  }
   if (i == e) {
     FirstRegular = i;
     ec = std::error_code();
     return;
   }
 
-  Name = i->getRawName();
+  Name = (*i)->getRawName();
 
   if (Name == "//") {
     StringTable = i;
     ++i;
+    if (!*i) {
+      ec = i->getError();
+      return;
+    }
   }
 
   FirstRegular = i;
@@ -376,12 +435,20 @@ Archive::child_iterator Archive::child_b
     return FirstRegular;
 
   const char *Loc = Data.getBufferStart() + strlen(Magic);
-  Child c(this, Loc);
-  return c;
+  auto ChildOrErr = Child::create(this, Loc);
+  if (std::error_code EC = ChildOrErr.getError())
+    return child_iterator(EC);
+  Child c = *(ChildOrErr.get());
+  return child_iterator(c);
 }
 
 Archive::child_iterator Archive::child_end() const {
-  return Child(this, nullptr);
+  // This with a second argument of nullptr can't return an Error.
+  auto ChildOrErr = Child::create(this, nullptr);
+  if (ChildOrErr.getError())
+    llvm_unreachable("Can't create Archive::child_end().");
+  Child c = *(ChildOrErr.get());
+  return child_iterator(c);
 }
 
 StringRef Archive::Symbol::getName() const {
@@ -433,7 +500,10 @@ ErrorOr<Archive::child_iterator> Archive
   }
 
   const char *Loc = Parent->getData().begin() + Offset;
-  child_iterator Iter(Child(Parent, Loc));
+  auto ChildOrErr = Child::create(Parent, Loc);
+  if (std::error_code EC = ChildOrErr.getError())
+    return EC;
+  child_iterator Iter(std::move(*ChildOrErr.get()));
   return Iter;
 }
 

Modified: llvm/trunk/lib/Object/ArchiveWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/ArchiveWriter.cpp?rev=250906&r1=250905&r2=250906&view=diff
==============================================================================
--- llvm/trunk/lib/Object/ArchiveWriter.cpp (original)
+++ llvm/trunk/lib/Object/ArchiveWriter.cpp Wed Oct 21 11:59:24 2015
@@ -347,10 +347,10 @@ llvm::writeArchive(StringRef ArcName,
       MemberRef = Buffers.back()->getMemBufferRef();
     } else {
       object::Archive::child_iterator OldMember = Member.getOld();
-      assert((!Thin || OldMember->getParent()->isThin()) &&
+      assert((!Thin || (*OldMember && (*OldMember)->getParent()->isThin())) &&
              "Thin archives cannot refers to member of other archives");
       ErrorOr<MemoryBufferRef> MemberBufferOrErr =
-          OldMember->getMemoryBufferRef();
+          (*OldMember)->getMemoryBufferRef();
       if (auto EC = MemberBufferOrErr.getError())
         return std::make_pair("", EC);
       MemberRef = MemberBufferOrErr.get();
@@ -398,10 +398,10 @@ llvm::writeArchive(StringRef ArcName,
       Perms = Status.permissions();
     } else {
       object::Archive::child_iterator OldMember = I.getOld();
-      ModTime = OldMember->getLastModified();
-      UID = OldMember->getUID();
-      GID = OldMember->getGID();
-      Perms = OldMember->getAccessMode();
+      ModTime = (*OldMember)->getLastModified();
+      UID = (*OldMember)->getUID();
+      GID = (*OldMember)->getGID();
+      Perms = (*OldMember)->getAccessMode();
     }
 
     if (I.isNewMember()) {
@@ -412,8 +412,11 @@ llvm::writeArchive(StringRef ArcName,
                         Status.getSize());
     } else {
       object::Archive::child_iterator OldMember = I.getOld();
+      ErrorOr<uint32_t> Size = (*OldMember)->getSize();
+      if (std::error_code EC = Size.getError())
+        return std::make_pair("", EC);
       printMemberHeader(Out, Kind, Thin, I.getName(), StringMapIndexIter,
-                        ModTime, UID, GID, Perms, OldMember->getSize());
+                        ModTime, UID, GID, Perms, Size.get());
     }
 
     if (!Thin)

Added: llvm/trunk/test/tools/llvm-objdump/Inputs/malformed-archives/libbogus1.a
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/malformed-archives/libbogus1.a?rev=250906&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-objdump/Inputs/malformed-archives/libbogus1.a (added)
+++ llvm/trunk/test/tools/llvm-objdump/Inputs/malformed-archives/libbogus1.a Wed Oct 21 11:59:24 2015
@@ -0,0 +1,13 @@
+!<arch>
+hello.c         1444941273  124   0     100644  10%       `
+#include <stdio.h>
+#include <stdlib.h>
+int
+main()
+{
+	printf("Hello World\n");
+	return EXIT_SUCCESS;
+}
+foo.c           1444941645  124   0     100644  1%        `
+void foo(void){}
+

Added: llvm/trunk/test/tools/llvm-objdump/Inputs/malformed-archives/libbogus2.a
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/malformed-archives/libbogus2.a?rev=250906&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-objdump/Inputs/malformed-archives/libbogus2.a (added)
+++ llvm/trunk/test/tools/llvm-objdump/Inputs/malformed-archives/libbogus2.a Wed Oct 21 11:59:24 2015
@@ -0,0 +1,13 @@
+!<arch>
+hello.c         1444941273  124   0     100644  102       `
+#include <stdio.h>
+#include <stdlib.h>
+int
+main()
+{
+	printf("Hello World\n");
+	return EXIT_SUCCESS;
+}
+foo.c           1444941645  124   0     100644  1%        `
+void foo(void){}
+

Added: llvm/trunk/test/tools/llvm-objdump/Inputs/malformed-archives/libbogus3.a
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/malformed-archives/libbogus3.a?rev=250906&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-objdump/Inputs/malformed-archives/libbogus3.a (added)
+++ llvm/trunk/test/tools/llvm-objdump/Inputs/malformed-archives/libbogus3.a Wed Oct 21 11:59:24 2015
@@ -0,0 +1,16 @@
+!<arch>
+hello.c         1444941273  124   0     100644  102       `
+#include <stdio.h>
+#include <stdlib.h>
+int
+main()
+{
+	printf("Hello World\n");
+	return EXIT_SUCCESS;
+}
+foo.c           1444941645  124   0     100644  171       `
+void foo(void){}
+
+bar.c           1445026190  124   0     100644  17        `
+void foo(void){}
+

Added: 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=250906&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-objdump/malformed-archives.test (added)
+++ llvm/trunk/test/tools/llvm-objdump/malformed-archives.test Wed Oct 21 11:59:24 2015
@@ -0,0 +1,20 @@
+// These test checks that llvm-objdump will not crash with malformed Archive
+// files.  So the check line is not all that important but the bug fixes to
+// make sure llvm-objdump is robust is what matters.
+# RUN: llvm-objdump -macho -archive-headers \
+# RUN:   %p/Inputs/malformed-archives/libbogus1.a \
+# RUN:   2>&1 | FileCheck -check-prefix=bogus1 %s 
+
+# bogus1: Invalid data was encountered while parsing the file
+
+# RUN: llvm-objdump -macho -archive-headers \
+# RUN:   %p/Inputs/malformed-archives/libbogus2.a \
+# RUN:   2>&1 | FileCheck -check-prefix=bogus2 %s 
+
+# bogus2: hello.c
+
+# RUN: llvm-objdump -macho -archive-headers \
+# RUN:   %p/Inputs/malformed-archives/libbogus3.a \
+# RUN:   2>&1 | FileCheck -check-prefix=bogus3 %s 
+
+# bogus3: foo.c

Modified: llvm/trunk/tools/dsymutil/BinaryHolder.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/BinaryHolder.cpp?rev=250906&r1=250905&r2=250906&view=diff
==============================================================================
--- llvm/trunk/tools/dsymutil/BinaryHolder.cpp (original)
+++ llvm/trunk/tools/dsymutil/BinaryHolder.cpp Wed Oct 21 11:59:24 2015
@@ -109,7 +109,10 @@ BinaryHolder::GetArchiveMemberBuffers(St
   Buffers.reserve(CurrentArchives.size());
 
   for (const auto &CurrentArchive : CurrentArchives) {
-    for (const auto &Child : CurrentArchive->children()) {
+    for (auto ChildOrErr : CurrentArchive->children()) {
+      if (auto Err = ChildOrErr.getError())
+        return Err;
+      const auto &Child = *ChildOrErr;
       if (auto NameOrErr = Child.getName()) {
         if (*NameOrErr == Filename) {
           if (Timestamp != sys::TimeValue::PosixZeroTime() &&

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=250906&r1=250905&r2=250906&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-ar/llvm-ar.cpp (original)
+++ llvm/trunk/tools/llvm-ar/llvm-ar.cpp Wed Oct 21 11:59:24 2015
@@ -338,7 +338,11 @@ static void doDisplayTable(StringRef Nam
     printMode(Mode & 007);
     outs() << ' ' << C.getUID();
     outs() << '/' << C.getGID();
-    outs() << ' ' << format("%6llu", C.getSize());
+    ErrorOr<uint32_t> Size = C.getSize();
+    if (Size.getError())
+      outs() << ' ' << "bad size";
+    else
+      outs() << ' ' << format("%6llu", Size.get());
     outs() << ' ' << C.getLastModified().str();
     outs() << ' ';
   }
@@ -403,7 +407,14 @@ static void performReadOperation(Archive
   }
 
   bool Filter = !Members.empty();
-  for (const object::Archive::Child &C : OldArchive->children()) {
+  for (auto &ChildOrErr : OldArchive->children()) {
+    if (ChildOrErr.getError()) {
+      errs() << ToolName << ": error reading '" << ArchiveName
+             << "': " << ChildOrErr.getError().message() << "!\n";
+      return;
+    }
+    const object::Archive::Child &C = *ChildOrErr;
+
     ErrorOr<StringRef> NameOrErr = C.getName();
     failIfError(NameOrErr.getError());
     StringRef Name = NameOrErr.get();
@@ -448,7 +459,9 @@ void addMember(std::vector<NewArchiveIte
 void addMember(std::vector<NewArchiveIterator> &Members,
                object::Archive::child_iterator I, StringRef Name,
                int Pos = -1) {
-  if (Thin && !I->getParent()->isThin())
+  if (I->getError())
+    fail("New member is not valid: " + I->getError().message());
+  if (Thin && !(*I)->getParent()->isThin())
     fail("Cannot convert a regular archive to a thin one");
   NewArchiveIterator NI(I, Name);
   if (Pos == -1)
@@ -469,6 +482,9 @@ static InsertAction computeInsertAction(
                                         object::Archive::child_iterator I,
                                         StringRef Name,
                                         std::vector<StringRef>::iterator &Pos) {
+  if (I->getError())
+    fail("Invalid member: " + I->getError().message());
+
   if (Operation == QuickAppend || Members.empty())
     return IA_AddOldMember;
 
@@ -500,7 +516,7 @@ static InsertAction computeInsertAction(
     // operation.
     sys::fs::file_status Status;
     failIfError(sys::fs::status(*MI, Status), *MI);
-    if (Status.getLastModificationTime() < I->getLastModified()) {
+    if (Status.getLastModificationTime() < (*I)->getLastModified()) {
       if (PosName.empty())
         return IA_AddOldMember;
       return IA_MoveOldMember;
@@ -523,7 +539,9 @@ computeNewArchiveMembers(ArchiveOperatio
   int InsertPos = -1;
   StringRef PosName = sys::path::filename(RelPos);
   if (OldArchive) {
-    for (auto &Child : OldArchive->children()) {
+    for (auto &ChildOrErr : OldArchive->children()) {
+      failIfError(ChildOrErr.getError());
+      auto &Child = ChildOrErr.get();
       int Pos = Ret.size();
       ErrorOr<StringRef> NameOrErr = Child.getName();
       failIfError(NameOrErr.getError());
@@ -726,7 +744,9 @@ static void runMRIScript() {
       failIfError(LibOrErr.getError(), "Could not parse library");
       Archives.push_back(std::move(*LibOrErr));
       object::Archive &Lib = *Archives.back();
-      for (auto &Member : Lib.children()) {
+      for (auto &MemberOrErr : Lib.children()) {
+        failIfError(MemberOrErr.getError());
+        auto &Member = MemberOrErr.get();
         ErrorOr<StringRef> NameOrErr = Member.getName();
         failIfError(NameOrErr.getError());
         addMember(NewMembers, Member, *NameOrErr);

Modified: llvm/trunk/tools/llvm-cxxdump/llvm-cxxdump.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-cxxdump/llvm-cxxdump.cpp?rev=250906&r1=250905&r2=250906&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-cxxdump/llvm-cxxdump.cpp (original)
+++ llvm/trunk/tools/llvm-cxxdump/llvm-cxxdump.cpp Wed Oct 21 11:59:24 2015
@@ -482,7 +482,12 @@ static void dumpCXXData(const ObjectFile
 }
 
 static void dumpArchive(const Archive *Arc) {
-  for (const Archive::Child &ArcC : Arc->children()) {
+  for (auto &ErrorOrChild : Arc->children()) {
+    if (std::error_code EC = ErrorOrChild.getError()) {
+      reportError(Arc->getFileName(), EC.message());
+      break;
+    }
+    const Archive::Child &ArcC = *ErrorOrChild;
     ErrorOr<std::unique_ptr<Binary>> ChildOrErr = ArcC.getAsBinary();
     if (std::error_code EC = ChildOrErr.getError()) {
       // Ignore non-object files.

Modified: llvm/trunk/tools/llvm-nm/llvm-nm.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-nm/llvm-nm.cpp?rev=250906&r1=250905&r2=250906&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-nm/llvm-nm.cpp (original)
+++ llvm/trunk/tools/llvm-nm/llvm-nm.cpp Wed Oct 21 11:59:24 2015
@@ -945,10 +945,11 @@ static void dumpSymbolNamesFromFile(std:
       if (I != E) {
         outs() << "Archive map\n";
         for (; I != E; ++I) {
-          ErrorOr<Archive::child_iterator> C = I->getMember();
-          if (error(C.getError()))
+          ErrorOr<Archive::child_iterator> ErrorOrChild = I->getMember();
+          if (error(ErrorOrChild.getError()))
             return;
-          ErrorOr<StringRef> FileNameOrErr = C.get()->getName();
+          auto &C = *(ErrorOrChild.get());
+          ErrorOr<StringRef> FileNameOrErr = C.get().getName();
           if (error(FileNameOrErr.getError()))
             return;
           StringRef SymName = I->getName();
@@ -960,7 +961,10 @@ static void dumpSymbolNamesFromFile(std:
 
     for (Archive::child_iterator I = A->child_begin(), E = A->child_end();
          I != E; ++I) {
-      ErrorOr<std::unique_ptr<Binary>> ChildOrErr = I->getAsBinary(&Context);
+      if (I->getError())
+        break;
+      auto &C = I->get();
+      ErrorOr<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary(&Context);
       if (ChildOrErr.getError())
         continue;
       if (SymbolicFile *O = dyn_cast<SymbolicFile>(&*ChildOrErr.get())) {
@@ -1015,8 +1019,11 @@ static void dumpSymbolNamesFromFile(std:
               for (Archive::child_iterator AI = A->child_begin(),
                                            AE = A->child_end();
                    AI != AE; ++AI) {
+                if(AI->getError())
+                  break;
+                auto &C = AI->get();
                 ErrorOr<std::unique_ptr<Binary>> ChildOrErr =
-                    AI->getAsBinary(&Context);
+                    C.getAsBinary(&Context);
                 if (ChildOrErr.getError())
                   continue;
                 if (SymbolicFile *O =
@@ -1069,8 +1076,11 @@ static void dumpSymbolNamesFromFile(std:
             for (Archive::child_iterator AI = A->child_begin(),
                                          AE = A->child_end();
                  AI != AE; ++AI) {
+              if(AI->getError())
+                break;
+              auto &C = AI->get();
               ErrorOr<std::unique_ptr<Binary>> ChildOrErr =
-                  AI->getAsBinary(&Context);
+                  C.getAsBinary(&Context);
               if (ChildOrErr.getError())
                 continue;
               if (SymbolicFile *O =
@@ -1118,8 +1128,11 @@ static void dumpSymbolNamesFromFile(std:
         std::unique_ptr<Archive> &A = *AOrErr;
         for (Archive::child_iterator AI = A->child_begin(), AE = A->child_end();
              AI != AE; ++AI) {
+          if(AI->getError())
+            continue;
+          auto &C = AI->get();
           ErrorOr<std::unique_ptr<Binary>> ChildOrErr =
-              AI->getAsBinary(&Context);
+              C.getAsBinary(&Context);
           if (ChildOrErr.getError())
             continue;
           if (SymbolicFile *O = dyn_cast<SymbolicFile>(&*ChildOrErr.get())) {

Modified: llvm/trunk/tools/llvm-objdump/MachODump.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/MachODump.cpp?rev=250906&r1=250905&r2=250906&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objdump/MachODump.cpp (original)
+++ llvm/trunk/tools/llvm-objdump/MachODump.cpp Wed Oct 21 11:59:24 2015
@@ -1417,8 +1417,11 @@ static void printArchiveChild(Archive::C
   outs() << format("%3d/", UID);
   unsigned GID = C.getGID();
   outs() << format("%-3d ", GID);
-  uint64_t Size = C.getRawSize();
-  outs() << format("%5" PRId64, Size) << " ";
+  ErrorOr<uint64_t> Size = C.getRawSize();
+  if (Size.getError())
+    outs() << "bad size" << " ";
+  else
+    outs() << format("%5" PRId64, Size.get()) << " ";
 
   StringRef RawLastModified = C.getRawLastModified();
   if (verbose) {
@@ -1454,12 +1457,16 @@ static void printArchiveChild(Archive::C
 static void printArchiveHeaders(Archive *A, bool verbose, bool print_offset) {
   if (A->hasSymbolTable()) {
     Archive::child_iterator S = A->getSymbolTableChild();
-    Archive::Child C = *S;
-    printArchiveChild(C, verbose, print_offset);
+    if (!S->getError()) {
+      Archive::Child C = S->get();
+      printArchiveChild(C, verbose, print_offset);
+    }
   }
   for (Archive::child_iterator I = A->child_begin(), E = A->child_end(); I != E;
        ++I) {
-    Archive::Child C = *I;
+    if(I->getError())
+      break;
+    Archive::Child C = I->get();
     printArchiveChild(C, verbose, print_offset);
   }
 }
@@ -1496,7 +1503,10 @@ void llvm::ParseInputMachO(StringRef Fil
       printArchiveHeaders(A, !NonVerbose, ArchiveMemberOffsets);
     for (Archive::child_iterator I = A->child_begin(), E = A->child_end();
          I != E; ++I) {
-      ErrorOr<std::unique_ptr<Binary>> ChildOrErr = I->getAsBinary();
+      if (I->getError())
+        break;
+      auto &C = I->get();
+      ErrorOr<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
       if (ChildOrErr.getError())
         continue;
       if (MachOObjectFile *O = dyn_cast<MachOObjectFile>(&*ChildOrErr.get())) {
@@ -1544,7 +1554,10 @@ void llvm::ParseInputMachO(StringRef Fil
               for (Archive::child_iterator AI = A->child_begin(),
                                            AE = A->child_end();
                    AI != AE; ++AI) {
-                ErrorOr<std::unique_ptr<Binary>> ChildOrErr = AI->getAsBinary();
+                if (AI->getError())
+                  break;
+                auto &C = AI->get();
+                ErrorOr<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
                 if (ChildOrErr.getError())
                   continue;
                 if (MachOObjectFile *O =
@@ -1586,7 +1599,10 @@ void llvm::ParseInputMachO(StringRef Fil
             for (Archive::child_iterator AI = A->child_begin(),
                                          AE = A->child_end();
                  AI != AE; ++AI) {
-              ErrorOr<std::unique_ptr<Binary>> ChildOrErr = AI->getAsBinary();
+              if (AI->getError())
+                break;
+              auto &C = AI->get();
+              ErrorOr<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
               if (ChildOrErr.getError())
                 continue;
               if (MachOObjectFile *O =
@@ -1622,7 +1638,10 @@ void llvm::ParseInputMachO(StringRef Fil
           printArchiveHeaders(A.get(), !NonVerbose, ArchiveMemberOffsets);
         for (Archive::child_iterator AI = A->child_begin(), AE = A->child_end();
              AI != AE; ++AI) {
-          ErrorOr<std::unique_ptr<Binary>> ChildOrErr = AI->getAsBinary();
+          if (AI->getError())
+            break;
+          auto &C = AI->get();
+          ErrorOr<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
           if (ChildOrErr.getError())
             continue;
           if (MachOObjectFile *O =

Modified: llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp?rev=250906&r1=250905&r2=250906&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp (original)
+++ llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp Wed Oct 21 11:59:24 2015
@@ -1536,7 +1536,12 @@ static void DumpObject(const ObjectFile
 
 /// @brief Dump each object file in \a a;
 static void DumpArchive(const Archive *a) {
-  for (const Archive::Child &C : a->children()) {
+  for (auto &ErrorOrChild : a->children()) {
+    if (std::error_code EC = ErrorOrChild.getError()) {
+      report_error(a->getFileName(), EC);
+      break;
+    }
+    const Archive::Child &C = *ErrorOrChild;
     ErrorOr<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
     if (std::error_code EC = ChildOrErr.getError())
       if (EC != object_error::invalid_file_type)

Modified: llvm/trunk/tools/llvm-readobj/llvm-readobj.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-readobj/llvm-readobj.cpp?rev=250906&r1=250905&r2=250906&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-readobj/llvm-readobj.cpp (original)
+++ llvm/trunk/tools/llvm-readobj/llvm-readobj.cpp Wed Oct 21 11:59:24 2015
@@ -377,7 +377,12 @@ static void dumpObject(const ObjectFile
 
 /// @brief Dumps each object file in \a Arc;
 static void dumpArchive(const Archive *Arc) {
-  for (const auto &Child : Arc->children()) {
+  for (auto &ErrorOrChild : Arc->children()) {
+    if (std::error_code EC = ErrorOrChild.getError()) {
+      reportError(Arc->getFileName(), EC.message());
+      break;
+    }
+    const auto &Child = *ErrorOrChild;
     ErrorOr<std::unique_ptr<Binary>> ChildOrErr = Child.getAsBinary();
     if (std::error_code EC = ChildOrErr.getError()) {
       // Ignore non-object files.

Modified: llvm/trunk/tools/llvm-size/llvm-size.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-size/llvm-size.cpp?rev=250906&r1=250905&r2=250906&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-size/llvm-size.cpp (original)
+++ llvm/trunk/tools/llvm-size/llvm-size.cpp Wed Oct 21 11:59:24 2015
@@ -427,7 +427,13 @@ static void PrintFileSectionSizes(String
     for (object::Archive::child_iterator i = a->child_begin(),
                                          e = a->child_end();
          i != e; ++i) {
-      ErrorOr<std::unique_ptr<Binary>> ChildOrErr = i->getAsBinary();
+      if (i->getError()) {
+        errs() << ToolName << ": " << file << ": " << i->getError().message()
+               << ".\n";
+        break;
+      }
+      auto &c = i->get();
+      ErrorOr<std::unique_ptr<Binary>> ChildOrErr = c.getAsBinary();
       if (std::error_code EC = ChildOrErr.getError()) {
         errs() << ToolName << ": " << file << ": " << EC.message() << ".\n";
         continue;
@@ -489,7 +495,13 @@ static void PrintFileSectionSizes(String
               for (object::Archive::child_iterator i = UA->child_begin(),
                                                    e = UA->child_end();
                    i != e; ++i) {
-                ErrorOr<std::unique_ptr<Binary>> ChildOrErr = i->getAsBinary();
+                if (std::error_code EC = i->getError()) {
+                  errs() << ToolName << ": " << file << ": " << EC.message()
+                         << ".\n";
+                  break;
+                }
+                auto &c = i->get();
+                ErrorOr<std::unique_ptr<Binary>> ChildOrErr = c.getAsBinary();
                 if (std::error_code EC = ChildOrErr.getError()) {
                   errs() << ToolName << ": " << file << ": " << EC.message()
                          << ".\n";
@@ -566,7 +578,13 @@ static void PrintFileSectionSizes(String
             for (object::Archive::child_iterator i = UA->child_begin(),
                                                  e = UA->child_end();
                  i != e; ++i) {
-              ErrorOr<std::unique_ptr<Binary>> ChildOrErr = i->getAsBinary();
+              if (std::error_code EC = i->getError()) {
+                errs() << ToolName << ": " << file << ": " << EC.message()
+                       << ".\n";
+                break;
+              }
+              auto &c = i->get();
+              ErrorOr<std::unique_ptr<Binary>> ChildOrErr = c.getAsBinary();
               if (std::error_code EC = ChildOrErr.getError()) {
                 errs() << ToolName << ": " << file << ": " << EC.message()
                        << ".\n";
@@ -630,7 +648,13 @@ static void PrintFileSectionSizes(String
         for (object::Archive::child_iterator i = UA->child_begin(),
                                              e = UA->child_end();
              i != e; ++i) {
-          ErrorOr<std::unique_ptr<Binary>> ChildOrErr = i->getAsBinary();
+          if (std::error_code EC = i->getError()) {
+            errs() << ToolName << ": " << file << ": " << EC.message()
+                   << ".\n";
+            break;
+          }
+          auto &c = i->get();
+          ErrorOr<std::unique_ptr<Binary>> ChildOrErr = c.getAsBinary();
           if (std::error_code EC = ChildOrErr.getError()) {
             errs() << ToolName << ": " << file << ": " << EC.message() << ".\n";
             continue;




More information about the llvm-commits mailing list