[PATCH] D13989: This removes the eating of the error in Archive::Child::getSize() when the charactersin the size field in the archive header for the member is not a number.

Rafael EspĂ­ndola via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 5 06:48:04 PST 2015


There is still cases of "exit with 0 on error":

+      if (std::error_code EC = I->getError()) {
+        errs() << "llvm-objdump: '" << Filename << "': " << EC.message()
+               << ".\n";
+        return;
+      }


+                if (std::error_code EC = AI->getError()) {
+                  errs() << "llvm-objdump: '" << Filename
+                         << "': " << EC.message() << ".\n";
+                  return;
+                }

+              if (std::error_code EC = AI->getError()) {
+                errs() << "llvm-objdump: '" << Filename << "': " <<
EC.message()
+                       << ".\n";
+                return;
+              }


+          if (std::error_code EC = AI->getError()) {
+            errs() << "llvm-objdump: '" << Filename << "': " << EC.message()
+                   << ".\n";
+            return;
+          }

The llvm patch is not git-clang-format clean.

The lld patch is not git-clang-format clean.

Rebased and git-clang-formated patches are attached.

LGTM with the "exit with 0 on error" issues fixed.

Cheers,
Rafael
-------------- next part --------------
diff --git a/include/llvm/Object/Archive.h b/include/llvm/Object/Archive.h
index bd1be99..a68f200 100644
--- a/include/llvm/Object/Archive.h
+++ b/include/llvm/Object/Archive.h
@@ -66,7 +66,7 @@ public:
     bool isThinMember() const;
 
   public:
-    Child(const Archive *Parent, const char *Start);
+    Child(const Archive *Parent, const char *Start, std::error_code *EC);
     Child(const Archive *Parent, StringRef Data, uint16_t StartOfFile);
 
     bool operator ==(const Child &other) const {
@@ -75,7 +75,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(); }
@@ -91,9 +91,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;
@@ -105,24 +105,32 @@ 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;
+      // We ignore error states so that comparisions with end() work, which
+      // allows range loops.
+      if (child.getError() || other.child.getError())
+        return false;
+      return *child == *other.child;
     }
 
     bool operator!=(const child_iterator &other) const {
       return !(*this == other);
     }
 
+    // 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;
     }
   };
diff --git a/include/llvm/Support/ErrorOr.h b/include/llvm/Support/ErrorOr.h
index 46b6192..ca6ede7 100644
--- a/include/llvm/Support/ErrorOr.h
+++ b/include/llvm/Support/ErrorOr.h
@@ -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,14 @@ 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 +251,14 @@ 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);
diff --git a/lib/ExecutionEngine/MCJIT/MCJIT.cpp b/lib/ExecutionEngine/MCJIT/MCJIT.cpp
index bbe4432..6cbebe9 100644
--- a/lib/ExecutionEngine/MCJIT/MCJIT.cpp
+++ b/lib/ExecutionEngine/MCJIT/MCJIT.cpp
@@ -318,10 +318,12 @@ RuntimeDyld::SymbolInfo MCJIT::findSymbol(const std::string &Name,
     object::Archive *A = OB.getBinary();
     // Look for our symbols in each Archive
     object::Archive::child_iterator ChildIt = A->findSym(Name);
+    if (std::error_code EC = ChildIt->getError())
+      report_fatal_error(EC.message());
     if (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();
diff --git a/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h b/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h
index d1aff5a..38a27cf 100644
--- a/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h
+++ b/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h
@@ -252,10 +252,12 @@ private:
       object::Archive *A = OB.getBinary();
       // Look for our symbols in each Archive
       object::Archive::child_iterator ChildIt = A->findSym(Name);
+      if (std::error_code EC = ChildIt->getError())
+        report_fatal_error(EC.message());
       if (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();
diff --git a/lib/Object/Archive.cpp b/lib/Object/Archive.cpp
index 5ac91f2..99b0650 100644
--- a/lib/Object/Archive.cpp
+++ b/lib/Object/Archive.cpp
@@ -46,7 +46,7 @@ StringRef ArchiveMemberHeader::getName() const {
 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;
 }
 
@@ -86,7 +86,8 @@ Archive::Child::Child(const Archive *Parent, StringRef Data,
                       uint16_t StartOfFile)
     : Parent(Parent), Data(Data), StartOfFile(StartOfFile) {}
 
-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;
@@ -94,7 +95,10 @@ Archive::Child::Child(const Archive *Parent, const char *Start)
   uint64_t Size = sizeof(ArchiveMemberHeader);
   Data = StringRef(Start, Size);
   if (!isThinMember()) {
-    Size += getRawSize();
+    ErrorOr<uint64_t> MemberSize = getRawSize();
+    if ((*EC = MemberSize.getError()))
+      return;
+    Size += MemberSize.get();
     Data = StringRef(Start, Size);
   }
 
@@ -110,20 +114,20 @@ Archive::Child::Child(const Archive *Parent, const char *Start)
   }
 }
 
-uint64_t Archive::Child::getSize() const {
+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();
 }
 
@@ -133,8 +137,12 @@ bool Archive::Child::isThinMember() const {
 }
 
 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;
@@ -148,7 +156,7 @@ ErrorOr<StringRef> Archive::Child::getBuffer() const {
   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.
   if (SpaceToSkip & 1)
@@ -156,11 +164,19 @@ Archive::Child Archive::Child::getNext() const {
 
   const char *NextLoc = Data.data() + SpaceToSkip;
 
+  // Check to see if this is at the end of the archive.
+  if (NextLoc == Parent->Data.getBufferEnd())
+    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);
+  std::error_code EC;
+  Child Ret(Parent, NextLoc, &EC);
+  if (EC)
+    return EC;
+  return Ret;
 }
 
 uint64_t Archive::Child::getChildOffset() const {
@@ -255,15 +271,26 @@ Archive::Archive(MemoryBufferRef Source, std::error_code &ec)
   }
 
   // Get the special members.
-  child_iterator i = child_begin(false);
-  child_iterator e = child_end();
+  child_iterator I = child_begin(false);
+  if ((ec = I->getError()))
+    return;
+  child_iterator E = child_end();
 
-  if (i == e) {
+  if (I == E) {
     ec = std::error_code();
     return;
   }
+  const Child *C = &**I;
 
-  StringRef Name = i->getRawName();
+  auto Increment = [&]() {
+    ++I;
+    if ((ec = I->getError()))
+      return true;
+    C = &**I;
+    return false;
+  };
+
+  StringRef Name = C->getRawName();
 
   // Below is the pattern that is used to figure out the archive format
   // GNU archive format
@@ -288,9 +315,11 @@ Archive::Archive(MemoryBufferRef Source, std::error_code &ec)
     Format = K_BSD;
     // We know that the symbol table is not an external file, so we just assert
     // there is no error.
-    SymbolTable = *i->getBuffer();
-    ++i;
-    setFirstRegular(*i);
+    SymbolTable = *C->getBuffer();
+    if (Increment())
+      return;
+    setFirstRegular(*C);
+
     ec = std::error_code();
     return;
   }
@@ -298,7 +327,7 @@ Archive::Archive(MemoryBufferRef Source, std::error_code &ec)
   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 = C->getName();
     ec = NameOrErr.getError();
     if (ec)
       return;
@@ -306,10 +335,11 @@ Archive::Archive(MemoryBufferRef Source, std::error_code &ec)
     if (Name == "__.SYMDEF SORTED" || Name == "__.SYMDEF") {
       // We know that the symbol table is not an external file, so we just
       // assert there is no error.
-      SymbolTable = *i->getBuffer();
-      ++i;
+      SymbolTable = *C->getBuffer();
+      if (Increment())
+        return;
     }
-    setFirstRegular(*i);
+    setFirstRegular(*C);
     return;
   }
 
@@ -322,32 +352,34 @@ Archive::Archive(MemoryBufferRef Source, std::error_code &ec)
   if (Name == "/" || Name == "/SYM64/") {
     // We know that the symbol table is not an external file, so we just assert
     // there is no error.
-    SymbolTable = *i->getBuffer();
+    SymbolTable = *C->getBuffer();
     if (Name == "/SYM64/")
       has64SymTable = true;
 
-    ++i;
-    if (i == e) {
+    if (Increment())
+      return;
+    if (I == E) {
       ec = std::error_code();
       return;
     }
-    Name = i->getRawName();
+    Name = C->getRawName();
   }
 
   if (Name == "//") {
     Format = has64SymTable ? K_MIPS64 : K_GNU;
     // The string table is never an external member, so we just assert on the
     // ErrorOr.
-    StringTable = *i->getBuffer();
-    ++i;
-    setFirstRegular(*i);
+    StringTable = *C->getBuffer();
+    if (Increment())
+      return;
+    setFirstRegular(*C);
     ec = std::error_code();
     return;
   }
 
   if (Name[0] != '/') {
     Format = has64SymTable ? K_MIPS64 : K_GNU;
-    setFirstRegular(*i);
+    setFirstRegular(*C);
     ec = std::error_code();
     return;
   }
@@ -360,25 +392,28 @@ Archive::Archive(MemoryBufferRef Source, std::error_code &ec)
   Format = K_COFF;
   // We know that the symbol table is not an external file, so we just assert
   // there is no error.
-  SymbolTable = *i->getBuffer();
+  SymbolTable = *C->getBuffer();
+
+  if (Increment())
+    return;
 
-  ++i;
-  if (i == e) {
-    setFirstRegular(*i);
+  if (I == E) {
+    setFirstRegular(*C);
     ec = std::error_code();
     return;
   }
 
-  Name = i->getRawName();
+  Name = C->getRawName();
 
   if (Name == "//") {
     // The string table is never an external member, so we just assert on the
     // ErrorOr.
-    StringTable = *i->getBuffer();
-    ++i;
+    StringTable = *C->getBuffer();
+    if (Increment())
+      return;
   }
 
-  setFirstRegular(*i);
+  setFirstRegular(*C);
   ec = std::error_code();
 }
 
@@ -390,12 +425,15 @@ Archive::child_iterator Archive::child_begin(bool SkipInternal) const {
     return Child(this, FirstRegularData, FirstRegularStartOfFile);
 
   const char *Loc = Data.getBufferStart() + strlen(Magic);
-  Child c(this, Loc);
-  return c;
+  std::error_code EC;
+  Child c(this, Loc, &EC);
+  if (EC)
+    return child_iterator(EC);
+  return child_iterator(c);
 }
 
 Archive::child_iterator Archive::child_end() const {
-  return Child(this, nullptr);
+  return Child(this, nullptr, nullptr);
 }
 
 StringRef Archive::Symbol::getName() const {
@@ -447,7 +485,11 @@ ErrorOr<Archive::Child> Archive::Symbol::getMember() const {
   }
 
   const char *Loc = Parent->getData().begin() + Offset;
-  return Child(Parent, Loc);
+  std::error_code EC;
+  Child C(Parent, Loc, &EC);
+  if (EC)
+    return EC;
+  return C;
 }
 
 Archive::Symbol Archive::Symbol::getNext() const {
diff --git a/lib/Object/ArchiveWriter.cpp b/lib/Object/ArchiveWriter.cpp
index b68f7ef..f207dfb 100644
--- a/lib/Object/ArchiveWriter.cpp
+++ b/lib/Object/ArchiveWriter.cpp
@@ -39,7 +39,7 @@ NewArchiveIterator::NewArchiveIterator(const object::Archive::Child &OldMember,
     : IsNewMember(false), Name(Name), OldMember(OldMember) {}
 
 NewArchiveIterator::NewArchiveIterator(StringRef FileName)
-    : IsNewMember(true), Name(FileName), OldMember(nullptr, nullptr) {}
+    : IsNewMember(true), Name(FileName), OldMember(nullptr, nullptr, nullptr) {}
 
 StringRef NewArchiveIterator::getName() const { return Name; }
 
@@ -412,8 +412,11 @@ llvm::writeArchive(StringRef ArcName,
                         Status.getSize());
     } else {
       const object::Archive::Child &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)
diff --git a/test/tools/llvm-objdump/Inputs/libbogus1.a b/test/tools/llvm-objdump/Inputs/libbogus1.a
new file mode 100644
index 0000000..510c145
--- /dev/null
+++ b/test/tools/llvm-objdump/Inputs/libbogus1.a
@@ -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){}
+
diff --git a/test/tools/llvm-objdump/Inputs/libbogus2.a b/test/tools/llvm-objdump/Inputs/libbogus2.a
new file mode 100644
index 0000000..2ccb7f3
--- /dev/null
+++ b/test/tools/llvm-objdump/Inputs/libbogus2.a
@@ -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){}
+
diff --git a/test/tools/llvm-objdump/Inputs/libbogus3.a b/test/tools/llvm-objdump/Inputs/libbogus3.a
new file mode 100644
index 0000000..f15a732
--- /dev/null
+++ b/test/tools/llvm-objdump/Inputs/libbogus3.a
@@ -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){}
+
diff --git a/test/tools/llvm-objdump/malformed-archives.test b/test/tools/llvm-objdump/malformed-archives.test
new file mode 100644
index 0000000..e0f165d
--- /dev/null
+++ b/test/tools/llvm-objdump/malformed-archives.test
@@ -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/libbogus1.a \
+# RUN:   2>&1 | FileCheck -check-prefix=bogus1 %s 
+
+# bogus1: Invalid data was encountered while parsing the file
+
+# RUN: not llvm-objdump -macho -archive-headers \
+# RUN:   %p/Inputs/libbogus2.a \
+# RUN:   2>&1 | FileCheck -check-prefix=bogus2 %s 
+
+# bogus2: LLVM ERROR: Invalid data was encountered while parsing the file
+
+# RUN: not llvm-objdump -macho -archive-headers \
+# RUN:   %p/Inputs/libbogus3.a \
+# RUN:   2>&1 | FileCheck -check-prefix=bogus3 %s 
+
+# bogus3: LLVM ERROR: Invalid data was encountered while parsing the file
diff --git a/tools/dsymutil/BinaryHolder.cpp b/tools/dsymutil/BinaryHolder.cpp
index 7ff4fd6..4c2c1d1 100644
--- a/tools/dsymutil/BinaryHolder.cpp
+++ b/tools/dsymutil/BinaryHolder.cpp
@@ -109,7 +109,10 @@ BinaryHolder::GetArchiveMemberBuffers(StringRef Filename,
   Buffers.reserve(CurrentArchives.size());
 
   for (const auto &CurrentArchive : CurrentArchives) {
-    for (const auto &Child : CurrentArchive->children()) {
+    for (auto ChildOrErr : CurrentArchive->children()) {
+      if (std::error_code Err = ChildOrErr.getError())
+        return Err;
+      const auto &Child = *ChildOrErr;
       if (auto NameOrErr = Child.getName()) {
         if (*NameOrErr == Filename) {
           if (Timestamp != sys::TimeValue::PosixZeroTime() &&
diff --git a/tools/llvm-ar/llvm-ar.cpp b/tools/llvm-ar/llvm-ar.cpp
index f0e7284..093f7f9 100644
--- a/tools/llvm-ar/llvm-ar.cpp
+++ b/tools/llvm-ar/llvm-ar.cpp
@@ -338,7 +338,9 @@ static void doDisplayTable(StringRef Name, const object::Archive::Child &C) {
     printMode(Mode & 007);
     outs() << ' ' << C.getUID();
     outs() << '/' << C.getGID();
-    outs() << ' ' << format("%6llu", C.getSize());
+    ErrorOr<uint64_t> Size = C.getSize();
+    failIfError(Size.getError());
+    outs() << ' ' << format("%6llu", Size.get());
     outs() << ' ' << C.getLastModified().str();
     outs() << ' ';
   }
@@ -403,7 +405,10 @@ static void performReadOperation(ArchiveOperation Operation,
   }
 
   bool Filter = !Members.empty();
-  for (const object::Archive::Child &C : OldArchive->children()) {
+  for (auto &ChildOrErr : OldArchive->children()) {
+    failIfError(ChildOrErr.getError());
+    const object::Archive::Child &C = *ChildOrErr;
+
     ErrorOr<StringRef> NameOrErr = C.getName();
     failIfError(NameOrErr.getError());
     StringRef Name = NameOrErr.get();
@@ -523,7 +528,9 @@ computeNewArchiveMembers(ArchiveOperation Operation,
   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 +733,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);
diff --git a/tools/llvm-cxxdump/llvm-cxxdump.cpp b/tools/llvm-cxxdump/llvm-cxxdump.cpp
index d45a28a..3dda692 100644
--- a/tools/llvm-cxxdump/llvm-cxxdump.cpp
+++ b/tools/llvm-cxxdump/llvm-cxxdump.cpp
@@ -482,7 +482,9 @@ static void dumpCXXData(const ObjectFile *Obj) {
 }
 
 static void dumpArchive(const Archive *Arc) {
-  for (const Archive::Child &ArcC : Arc->children()) {
+  for (auto &ErrorOrChild : Arc->children()) {
+    error(ErrorOrChild.getError());
+    const Archive::Child &ArcC = *ErrorOrChild;
     ErrorOr<std::unique_ptr<Binary>> ChildOrErr = ArcC.getAsBinary();
     if (std::error_code EC = ChildOrErr.getError()) {
       // Ignore non-object files.
diff --git a/tools/llvm-nm/llvm-nm.cpp b/tools/llvm-nm/llvm-nm.cpp
index 9486629..770731f 100644
--- a/tools/llvm-nm/llvm-nm.cpp
+++ b/tools/llvm-nm/llvm-nm.cpp
@@ -967,7 +967,10 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
 
     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 (error(I->getError()))
+        return;
+      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())) {
@@ -1022,8 +1025,11 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
               for (Archive::child_iterator AI = A->child_begin(),
                                            AE = A->child_end();
                    AI != AE; ++AI) {
+                if (error(AI->getError()))
+                  return;
+                auto &C = AI->get();
                 ErrorOr<std::unique_ptr<Binary>> ChildOrErr =
-                    AI->getAsBinary(&Context);
+                    C.getAsBinary(&Context);
                 if (ChildOrErr.getError())
                   continue;
                 if (SymbolicFile *O =
@@ -1076,8 +1082,11 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
             for (Archive::child_iterator AI = A->child_begin(),
                                          AE = A->child_end();
                  AI != AE; ++AI) {
+              if (error(AI->getError()))
+                return;
+              auto &C = AI->get();
               ErrorOr<std::unique_ptr<Binary>> ChildOrErr =
-                  AI->getAsBinary(&Context);
+                  C.getAsBinary(&Context);
               if (ChildOrErr.getError())
                 continue;
               if (SymbolicFile *O =
@@ -1125,8 +1134,10 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
         std::unique_ptr<Archive> &A = *AOrErr;
         for (Archive::child_iterator AI = A->child_begin(), AE = A->child_end();
              AI != AE; ++AI) {
-          ErrorOr<std::unique_ptr<Binary>> ChildOrErr =
-              AI->getAsBinary(&Context);
+          if (error(AI->getError()))
+            return;
+          auto &C = AI->get();
+          ErrorOr<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary(&Context);
           if (ChildOrErr.getError())
             continue;
           if (SymbolicFile *O = dyn_cast<SymbolicFile>(&*ChildOrErr.get())) {
diff --git a/tools/llvm-objdump/MachODump.cpp b/tools/llvm-objdump/MachODump.cpp
index b4a05df..8235686 100644
--- a/tools/llvm-objdump/MachODump.cpp
+++ b/tools/llvm-objdump/MachODump.cpp
@@ -1417,8 +1417,10 @@ static void printArchiveChild(const Archive::Child &C, bool verbose,
   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 (std::error_code EC = Size.getError())
+    report_fatal_error(EC.message());
+  outs() << format("%5" PRId64, Size.get()) << " ";
 
   StringRef RawLastModified = C.getRawLastModified();
   if (verbose) {
@@ -1454,7 +1456,9 @@ static void printArchiveChild(const Archive::Child &C, bool verbose,
 static void printArchiveHeaders(Archive *A, bool verbose, bool print_offset) {
   for (Archive::child_iterator I = A->child_begin(false), E = A->child_end();
        I != E; ++I) {
-    Archive::Child C = *I;
+    if (std::error_code EC = I->getError())
+      report_fatal_error(EC.message());
+    const Archive::Child &C = **I;
     printArchiveChild(C, verbose, print_offset);
   }
 }
@@ -1491,7 +1495,13 @@ void llvm::ParseInputMachO(StringRef Filename) {
       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 (std::error_code EC = I->getError()) {
+        errs() << "llvm-objdump: '" << Filename << "': " << EC.message()
+               << ".\n";
+        return;
+      }
+      auto &C = I->get();
+      ErrorOr<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
       if (ChildOrErr.getError())
         continue;
       if (MachOObjectFile *O = dyn_cast<MachOObjectFile>(&*ChildOrErr.get())) {
@@ -1539,7 +1549,13 @@ void llvm::ParseInputMachO(StringRef Filename) {
               for (Archive::child_iterator AI = A->child_begin(),
                                            AE = A->child_end();
                    AI != AE; ++AI) {
-                ErrorOr<std::unique_ptr<Binary>> ChildOrErr = AI->getAsBinary();
+                if (std::error_code EC = AI->getError()) {
+                  errs() << "llvm-objdump: '" << Filename
+                         << "': " << EC.message() << ".\n";
+                  return;
+                }
+                auto &C = AI->get();
+                ErrorOr<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
                 if (ChildOrErr.getError())
                   continue;
                 if (MachOObjectFile *O =
@@ -1581,7 +1597,13 @@ void llvm::ParseInputMachO(StringRef Filename) {
             for (Archive::child_iterator AI = A->child_begin(),
                                          AE = A->child_end();
                  AI != AE; ++AI) {
-              ErrorOr<std::unique_ptr<Binary>> ChildOrErr = AI->getAsBinary();
+              if (std::error_code EC = AI->getError()) {
+                errs() << "llvm-objdump: '" << Filename << "': " << EC.message()
+                       << ".\n";
+                return;
+              }
+              auto &C = AI->get();
+              ErrorOr<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
               if (ChildOrErr.getError())
                 continue;
               if (MachOObjectFile *O =
@@ -1617,7 +1639,13 @@ void llvm::ParseInputMachO(StringRef Filename) {
           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 (std::error_code EC = AI->getError()) {
+            errs() << "llvm-objdump: '" << Filename << "': " << EC.message()
+                   << ".\n";
+            return;
+          }
+          auto &C = AI->get();
+          ErrorOr<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary();
           if (ChildOrErr.getError())
             continue;
           if (MachOObjectFile *O =
diff --git a/tools/llvm-objdump/llvm-objdump.cpp b/tools/llvm-objdump/llvm-objdump.cpp
index 15a6b9a..27a36c0 100644
--- a/tools/llvm-objdump/llvm-objdump.cpp
+++ b/tools/llvm-objdump/llvm-objdump.cpp
@@ -1537,7 +1537,10 @@ static void DumpObject(const ObjectFile *o) {
 
 /// @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);
+    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)
diff --git a/tools/llvm-readobj/llvm-readobj.cpp b/tools/llvm-readobj/llvm-readobj.cpp
index cb0c9c6..63cec03 100644
--- a/tools/llvm-readobj/llvm-readobj.cpp
+++ b/tools/llvm-readobj/llvm-readobj.cpp
@@ -377,7 +377,10 @@ static void dumpObject(const ObjectFile *Obj) {
 
 /// @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());
+    const auto &Child = *ErrorOrChild;
     ErrorOr<std::unique_ptr<Binary>> ChildOrErr = Child.getAsBinary();
     if (std::error_code EC = ChildOrErr.getError()) {
       // Ignore non-object files.
diff --git a/tools/llvm-size/llvm-size.cpp b/tools/llvm-size/llvm-size.cpp
index bb29988..069cc62 100644
--- a/tools/llvm-size/llvm-size.cpp
+++ b/tools/llvm-size/llvm-size.cpp
@@ -428,7 +428,13 @@ static void PrintFileSectionSizes(StringRef file) {
     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";
+        exit(1);
+      }
+      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;
@@ -490,7 +496,13 @@ static void PrintFileSectionSizes(StringRef file) {
               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";
+                  exit(1);
+                }
+                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";
@@ -567,7 +579,13 @@ static void PrintFileSectionSizes(StringRef file) {
             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";
+                exit(1);
+              }
+              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";
@@ -631,7 +649,12 @@ static void PrintFileSectionSizes(StringRef file) {
         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";
+            exit(1);
+          }
+          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;
-------------- next part --------------
diff --git a/COFF/DriverUtils.cpp b/COFF/DriverUtils.cpp
index 375c4cf..9e46e82 100644
--- a/COFF/DriverUtils.cpp
+++ b/COFF/DriverUtils.cpp
@@ -521,7 +521,9 @@ static std::unique_ptr<MemoryBuffer> createEmptyImportLibrary() {
 static std::vector<NewArchiveIterator>
 readMembers(const object::Archive &Archive) {
   std::vector<NewArchiveIterator> V;
-  for (const object::Archive::Child &C : Archive.children()) {
+  for (const auto &ChildOrErr : Archive.children()) {
+    error(ChildOrErr, "Archive::Child::getName failed");
+    const object::Archive::Child C(*ChildOrErr);
     ErrorOr<StringRef> NameOrErr = C.getName();
     error(NameOrErr, "Archive::Child::getName failed");
     V.emplace_back(C, *NameOrErr);
@@ -570,7 +572,7 @@ public:
     P += Sym.size() + 1;
     memcpy(P, DLLName.data(), DLLName.size());
 
-    object::Archive::Child C(Parent, Buf);
+    object::Archive::Child C(Parent, Buf, nullptr);
     return NewArchiveIterator(C, DLLName);
   }
 
diff --git a/COFF/InputFiles.cpp b/COFF/InputFiles.cpp
index bcf9a29..e819c1b 100644
--- a/COFF/InputFiles.cpp
+++ b/COFF/InputFiles.cpp
@@ -67,8 +67,11 @@ void ArchiveFile::parse() {
   // Seen is a map from member files to boolean values. Initially
   // all members are mapped to false, which indicates all these files
   // are not read yet.
-  for (const Archive::Child &Child : File->children())
+  for (auto &ChildOrErr : File->children()) {
+    error(ChildOrErr, "Failed to parse static library");
+    const Archive::Child &Child = *ChildOrErr;
     Seen[Child.getChildOffset()].clear();
+  }
 }
 
 // Returns a buffer pointing to a member file containing a given symbol.
diff --git a/ELF/InputFiles.cpp b/ELF/InputFiles.cpp
index ed6bb83..364b388 100644
--- a/ELF/InputFiles.cpp
+++ b/ELF/InputFiles.cpp
@@ -308,7 +308,10 @@ std::vector<MemoryBufferRef> ArchiveFile::getMembers() {
   File = openArchive(MB);
 
   std::vector<MemoryBufferRef> Result;
-  for (const Archive::Child &Child : File->children()) {
+  for (auto &ChildOrErr : File->children()) {
+    error(ChildOrErr,
+          "Could not get the child of the archive " + File->getFileName());
+    const Archive::Child Child(*ChildOrErr);
     ErrorOr<MemoryBufferRef> MbOrErr = Child.getMemoryBufferRef();
     error(MbOrErr, "Could not get the buffer for a child of the archive " +
                        File->getFileName());
diff --git a/lib/ReaderWriter/FileArchive.cpp b/lib/ReaderWriter/FileArchive.cpp
index 4d0d552..a09923f 100644
--- a/lib/ReaderWriter/FileArchive.cpp
+++ b/lib/ReaderWriter/FileArchive.cpp
@@ -49,9 +49,11 @@ public:
     if (member == _symbolMemberMap.end())
       return nullptr;
     Archive::child_iterator ci = member->second;
+    if (ci->getError())
+      return nullptr;
 
     // Don't return a member already returned
-    ErrorOr<StringRef> buf = ci->getBuffer();
+    ErrorOr<StringRef> buf = (*ci)->getBuffer();
     if (!buf)
       return nullptr;
     const char *memberStart = buf->data();
@@ -90,9 +92,11 @@ public:
     if (member == _symbolMemberMap.end())
       return;
     Archive::child_iterator ci = member->second;
+    if (ci->getError())
+      return;
 
     // Do nothing if a member is already instantiated.
-    ErrorOr<StringRef> buf = ci->getBuffer();
+    ErrorOr<StringRef> buf = (*ci)->getBuffer();
     if (!buf)
       return;
     const char *memberStart = buf->data();
@@ -167,10 +171,12 @@ protected:
   }
 
 private:
-  std::error_code
-  instantiateMember(Archive::child_iterator member,
-                    std::unique_ptr<File> &result) const {
-    ErrorOr<llvm::MemoryBufferRef> mbOrErr = member->getMemoryBufferRef();
+  std::error_code instantiateMember(Archive::child_iterator cOrErr,
+                                    std::unique_ptr<File> &result) const {
+    if (std::error_code ec = cOrErr->getError())
+      return ec;
+    Archive::child_iterator member = cOrErr->get();
+    ErrorOr<llvm::MemoryBufferRef> mbOrErr = (*member)->getMemoryBufferRef();
     if (std::error_code ec = mbOrErr.getError())
       return ec;
     llvm::MemoryBufferRef mb = mbOrErr.get();
@@ -201,8 +207,11 @@ private:
   // Parses the given memory buffer as an object file, and returns true
   // code if the given symbol is a data symbol. If the symbol is not a data
   // symbol or does not exist, returns false.
-  bool isDataSymbol(Archive::child_iterator member, StringRef symbol) const {
-    ErrorOr<llvm::MemoryBufferRef> buf = member->getMemoryBufferRef();
+  bool isDataSymbol(Archive::child_iterator cOrErr, StringRef symbol) const {
+    if (cOrErr->getError())
+      return false;
+    Archive::child_iterator member = cOrErr->get();
+    ErrorOr<llvm::MemoryBufferRef> buf = (*member)->getMemoryBufferRef();
     if (buf.getError())
       return false;
     std::unique_ptr<MemoryBuffer> mb(MemoryBuffer::getMemBuffer(
@@ -242,10 +251,11 @@ private:
       if (std::error_code ec = memberOrErr.getError())
         return ec;
       Archive::child_iterator member = memberOrErr.get();
-      DEBUG_WITH_TYPE(
-          "FileArchive",
-          llvm::dbgs() << llvm::format("0x%08llX ", member->getBuffer()->data())
-                       << "'" << name << "'\n");
+      DEBUG_WITH_TYPE("FileArchive",
+                      llvm::dbgs()
+                          << llvm::format("0x%08llX ",
+                                          (*member)->getBuffer()->data())
+                          << "'" << name << "'\n");
       _symbolMemberMap.insert(std::make_pair(name, member));
     }
     return std::error_code();


More information about the llvm-commits mailing list