[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 Oct 29 17:11:14 PDT 2015


On 28 October 2015 at 15:21, Kevin Enderby <enderby at apple.com> wrote:
> enderby added a comment.
>
> I agree with Lang and like the original patch where were were handling failure was done inside the iterator.  It does allow use of ranged-based for loops and appears to me much cleaner in the code especially in the lld changes needed in http://reviews.llvm.org/D13990 .

I think that the most important consideration is making the error hard
to ignore. That means using ErrorOr.

The sad reality is that the archive format can cause us to find an
error when going from one member to the next. That, with the above
requirement means that it doesn't fit on range loop.

> Rafael, I did "finish it up" based on your t.diff patch and that is below and includes the re-formatting with clang-format-diff, removal of the malformed-archives directory putting the .a files in Inputs and removal of the "nice but independent" changes.
>
> F1026846: llvm_updated_t.diff <http://reviews.llvm.org/F1026846>


Thanks!

You don't need to change ErrorOr.h.

The change to detect the end of the archive seems more complicated
than necessary (see the attached patch for a suggestion).

Cases like the Increment in BinaryHolder expose a naked EC, which is
what using ErrorOr is trying to avoid. Just inlining it solves the
problem (see patch).

Please convert the remaining user of Increment to return void when possible.

Cheers,
Rafael
-------------- next part --------------
diff --git a/include/llvm/Object/Archive.h b/include/llvm/Object/Archive.h
index 32c72a0..1ec0976 100644
--- a/include/llvm/Object/Archive.h
+++ b/include/llvm/Object/Archive.h
@@ -65,7 +65,7 @@ public:
     bool isThinMember() const;
 
   public:
-    Child(const Archive *Parent, const char *Start);
+    Child(const Archive *Parent, const char *Start, std::error_code *EC);
 
     bool operator ==(const Child &other) const {
       assert(Parent == other.Parent);
@@ -77,7 +77,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 +93,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;
@@ -110,7 +110,7 @@ public:
     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; }
@@ -127,9 +127,11 @@ public:
       return child < other.child;
     }
 
-    child_iterator &operator++() { // Preincrement
-      child = child.getNext();
-      return *this;
+    ErrorOr<child_iterator> getNext() {
+      ErrorOr<Child> ChildOrError = child.getNext();
+      if (std::error_code EC = ChildOrError.getError())
+        return EC;
+      return child_iterator(*ChildOrError);
     }
   };
 
@@ -186,12 +188,8 @@ public:
   Kind kind() const { return (Kind)Format; }
   bool isThin() const { return IsThin; }
 
-  child_iterator child_begin(bool SkipInternal = true) const;
+  ErrorOr<child_iterator> child_begin(bool SkipInternal = true) const;
   child_iterator child_end() const;
-  iterator_range<child_iterator> children(bool SkipInternal = true) const {
-    return iterator_range<child_iterator>(child_begin(SkipInternal),
-                                          child_end());
-  }
 
   symbol_iterator symbol_begin() const;
   symbol_iterator symbol_end() const;
diff --git a/lib/Object/Archive.cpp b/lib/Object/Archive.cpp
index 667732b..2465688 100644
--- a/lib/Object/Archive.cpp
+++ b/lib/Object/Archive.cpp
@@ -82,7 +82,8 @@ unsigned ArchiveMemberHeader::getGID() const {
   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,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);
   }
 
@@ -106,20 +110,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();
 }
 
@@ -129,8 +133,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;
@@ -144,7 +152,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)
@@ -152,11 +160,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 {
@@ -183,12 +199,14 @@ ErrorOr<StringRef> Archive::Child::getName() const {
                        + sizeof(ArchiveMemberHeader)
                        + offset;
     // Verify it.
-    if (Parent->StringTable == Parent->child_end()
-        || addr < (Parent->StringTable->Data.begin()
-                   + sizeof(ArchiveMemberHeader))
-        || addr > (Parent->StringTable->Data.begin()
-                   + sizeof(ArchiveMemberHeader)
-                   + Parent->StringTable->getSize()))
+    ErrorOr<uint32_t> Size = Parent->StringTable->getSize();
+    if (std::error_code EC = Size.getError())
+      return EC;
+    if (Parent->StringTable == Parent->child_end() ||
+        addr <
+            (Parent->StringTable->Data.begin() + sizeof(ArchiveMemberHeader)) ||
+        addr > (Parent->StringTable->Data.begin() +
+                sizeof(ArchiveMemberHeader) + Size.get()))
       return object_error::parse_failed;
 
     // GNU long file names end with a "/\n".
@@ -253,7 +271,10 @@ Archive::Archive(MemoryBufferRef Source, std::error_code &ec)
   }
 
   // Get the special members.
-  child_iterator i = child_begin(false);
+  ErrorOr<child_iterator> IOrErr = child_begin(false);
+  if ((ec = IOrErr.getError()))
+    return;
+  child_iterator i = *IOrErr;
   child_iterator e = child_end();
 
   if (i == e) {
@@ -282,10 +303,19 @@ Archive::Archive(MemoryBufferRef Source, std::error_code &ec)
   //  seem to create the third member if there's no member whose filename
   //  exceeds 15 characters. So the third member is optional.
 
+  auto Increment = [&]() {
+    ErrorOr<child_iterator> Next = i.getNext();
+    if ((ec = Next.getError()))
+      return true;
+    i = *Next;
+    return false;
+  };
+
   if (Name == "__.SYMDEF") {
     Format = K_BSD;
     SymbolTable = i;
-    ++i;
+    if (Increment())
+      return;
     FirstRegular = i;
     ec = std::error_code();
     return;
@@ -301,7 +331,8 @@ Archive::Archive(MemoryBufferRef Source, std::error_code &ec)
     Name = NameOrErr.get();
     if (Name == "__.SYMDEF SORTED" || Name == "__.SYMDEF") {
       SymbolTable = i;
-      ++i;
+      if (Increment())
+        return;
     }
     FirstRegular = i;
     return;
@@ -318,7 +349,8 @@ Archive::Archive(MemoryBufferRef Source, std::error_code &ec)
     if (Name == "/SYM64/")
       has64SymTable = true;
 
-    ++i;
+    if (Increment())
+      return;
     if (i == e) {
       ec = std::error_code();
       return;
@@ -329,7 +361,8 @@ Archive::Archive(MemoryBufferRef Source, std::error_code &ec)
   if (Name == "//") {
     Format = has64SymTable ? K_MIPS64 : K_GNU;
     StringTable = i;
-    ++i;
+    if (Increment())
+      return;
     FirstRegular = i;
     ec = std::error_code();
     return;
@@ -350,7 +383,8 @@ Archive::Archive(MemoryBufferRef Source, std::error_code &ec)
   Format = K_COFF;
   SymbolTable = i;
 
-  ++i;
+  if (Increment())
+    return;
   if (i == e) {
     FirstRegular = i;
     ec = std::error_code();
@@ -361,14 +395,15 @@ Archive::Archive(MemoryBufferRef Source, std::error_code &ec)
 
   if (Name == "//") {
     StringTable = i;
-    ++i;
+    if (Increment())
+      return;
   }
 
   FirstRegular = i;
   ec = std::error_code();
 }
 
-Archive::child_iterator Archive::child_begin(bool SkipInternal) const {
+ErrorOr<Archive::child_iterator> Archive::child_begin(bool SkipInternal) const {
   if (Data.getBufferSize() == 8) // empty archive.
     return child_end();
 
@@ -376,12 +411,16 @@ Archive::child_iterator Archive::child_begin(bool SkipInternal) const {
     return FirstRegular;
 
   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 EC;
+  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.
+  return Child(this, nullptr, nullptr);
 }
 
 StringRef Archive::Symbol::getName() const {
@@ -433,8 +472,11 @@ ErrorOr<Archive::child_iterator> Archive::Symbol::getMember() const {
   }
 
   const char *Loc = Parent->getData().begin() + Offset;
-  child_iterator Iter(Child(Parent, Loc));
-  return Iter;
+  std::error_code EC;
+  Child c(Parent, Loc, &EC);
+  if (EC)
+    return EC;
+  return child_iterator(c);
 }
 
 Archive::Symbol Archive::Symbol::getNext() const {
diff --git a/lib/Object/ArchiveWriter.cpp b/lib/Object/ArchiveWriter.cpp
index dae0188..db97291 100644
--- a/lib/Object/ArchiveWriter.cpp
+++ b/lib/Object/ArchiveWriter.cpp
@@ -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);
     }
 
     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..b8ba312
--- /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: llvm-objdump -macho -archive-headers \
+# RUN:   %p/Inputs/libbogus2.a \
+# RUN:   2>&1 | FileCheck -check-prefix=bogus2 %s 
+
+# bogus2: hello.c
+
+# RUN: llvm-objdump -macho -archive-headers \
+# RUN:   %p/Inputs/libbogus3.a \
+# RUN:   2>&1 | FileCheck -check-prefix=bogus3 %s 
+
+# bogus3: foo.c
diff --git a/tools/dsymutil/BinaryHolder.cpp b/tools/dsymutil/BinaryHolder.cpp
index 7ff4fd6..5659783 100644
--- a/tools/dsymutil/BinaryHolder.cpp
+++ b/tools/dsymutil/BinaryHolder.cpp
@@ -109,7 +109,20 @@ BinaryHolder::GetArchiveMemberBuffers(StringRef Filename,
   Buffers.reserve(CurrentArchives.size());
 
   for (const auto &CurrentArchive : CurrentArchives) {
-    for (const auto &Child : CurrentArchive->children()) {
+    ErrorOr<object::Archive::child_iterator> IOrErr =
+        CurrentArchive->child_begin();
+    if (auto Err = IOrErr.getError())
+      return Err;
+    for (object::Archive::child_iterator I = *IOrErr,
+                                         E = CurrentArchive->child_end();
+         I != E;) {
+      object::Archive::Child Child = *I;
+
+      ErrorOr<object::Archive::child_iterator> Next = I.getNext();
+      if (std::error_code EC = Next.getError())
+        return EC;
+      I = *Next;
+
       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 ec3cfcb..473ab9b 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<uint32_t> Size = C.getSize();
+    failIfError(Size.getError());
+    outs() << ' ' << format("%6llu", *Size);
     outs() << ' ' << C.getLastModified().str();
     outs() << ' ';
   }
@@ -395,6 +397,12 @@ static bool shouldCreateArchive(ArchiveOperation Op) {
   llvm_unreachable("Missing entry in covered switch.");
 }
 
+static void Increment(object::Archive::child_iterator &I) {
+  ErrorOr<object::Archive::child_iterator> Next = I.getNext();
+  failIfError(Next.getError());
+  I = *Next;
+}
+
 static void performReadOperation(ArchiveOperation Operation,
                                  object::Archive *OldArchive) {
   if (Operation == Extract && OldArchive->isThin()) {
@@ -403,7 +411,12 @@ static void performReadOperation(ArchiveOperation Operation,
   }
 
   bool Filter = !Members.empty();
-  for (const object::Archive::Child &C : OldArchive->children()) {
+  ErrorOr<object::Archive::child_iterator> IOrErr = OldArchive->child_begin();
+  failIfError(IOrErr.getError());
+
+  for (object::Archive::child_iterator I = *IOrErr, E = OldArchive->child_end();
+       I != E; Increment(I)) {
+    const object::Archive::Child &C = *I;
     ErrorOr<StringRef> NameOrErr = C.getName();
     failIfError(NameOrErr.getError());
     StringRef Name = NameOrErr.get();
@@ -523,7 +536,13 @@ computeNewArchiveMembers(ArchiveOperation Operation,
   int InsertPos = -1;
   StringRef PosName = sys::path::filename(RelPos);
   if (OldArchive) {
-    for (auto &Child : OldArchive->children()) {
+    ErrorOr<object::Archive::child_iterator> IOrErr = OldArchive->child_begin();
+    failIfError(IOrErr.getError());
+
+    for (object::Archive::child_iterator I = *IOrErr,
+                                         E = OldArchive->child_end();
+         I != E; Increment(I)) {
+      const object::Archive::Child &Child = *I;
       int Pos = Ret.size();
       ErrorOr<StringRef> NameOrErr = Child.getName();
       failIfError(NameOrErr.getError());
@@ -726,7 +745,12 @@ 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()) {
+      ErrorOr<object::Archive::child_iterator> IOrErr = Lib.child_begin();
+      failIfError(IOrErr.getError());
+
+      for (object::Archive::child_iterator I = *IOrErr, E = Lib.child_end();
+           I != E; Increment(I)) {
+        const object::Archive::Child &Member = *I;
         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..8f77e5c 100644
--- a/tools/llvm-cxxdump/llvm-cxxdump.cpp
+++ b/tools/llvm-cxxdump/llvm-cxxdump.cpp
@@ -482,7 +482,19 @@ static void dumpCXXData(const ObjectFile *Obj) {
 }
 
 static void dumpArchive(const Archive *Arc) {
-  for (const Archive::Child &ArcC : Arc->children()) {
+  ErrorOr<Archive::child_iterator> IOrErr = Arc->child_begin();
+  if (std::error_code EC = IOrErr.getError())
+    reportError(Arc->getFileName(), EC.message());
+  auto Increment = [&](Archive::child_iterator &I) {
+    ErrorOr<Archive::child_iterator> Next = I.getNext();
+    if (std::error_code EC = Next.getError())
+      reportError(Arc->getFileName(), EC.message());
+    I = *Next;
+  };
+
+  for (Archive::child_iterator I = *IOrErr, E = Arc->child_end(); I != E;
+       Increment(I)) {
+    const Archive::Child &ArcC = *I;
     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 a0b5e9b..da60941 100644
--- a/tools/llvm-nm/llvm-nm.cpp
+++ b/tools/llvm-nm/llvm-nm.cpp
@@ -925,6 +925,13 @@ static bool checkMachOAndArchFlags(SymbolicFile *O, std::string &Filename) {
   return true;
 }
 
+static void Increment(Archive::child_iterator &I) {
+  ErrorOr<Archive::child_iterator> Next = I.getNext();
+  if (error(Next.getError()))
+    exit(1);
+  I = *Next;
+}
+
 static void dumpSymbolNamesFromFile(std::string &Filename) {
   ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr =
       MemoryBuffer::getFileOrSTDIN(Filename);
@@ -958,8 +965,11 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
       }
     }
 
-    for (Archive::child_iterator I = A->child_begin(), E = A->child_end();
-         I != E; ++I) {
+    ErrorOr<Archive::child_iterator> IOrErr = A->child_begin();
+    if (error(IOrErr.getError()))
+      return;
+    for (Archive::child_iterator I = *IOrErr, E = A->child_end(); I != E;
+         Increment(I)) {
       ErrorOr<std::unique_ptr<Binary>> ChildOrErr = I->getAsBinary(&Context);
       if (ChildOrErr.getError())
         continue;
@@ -1012,9 +1022,11 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
             } else if (ErrorOr<std::unique_ptr<Archive>> AOrErr =
                            I->getAsArchive()) {
               std::unique_ptr<Archive> &A = *AOrErr;
-              for (Archive::child_iterator AI = A->child_begin(),
-                                           AE = A->child_end();
-                   AI != AE; ++AI) {
+              ErrorOr<Archive::child_iterator> IOrErr = A->child_begin();
+              if (IOrErr.getError())
+                return;
+              for (Archive::child_iterator AI = *IOrErr, AE = A->child_end();
+                   AI != AE; Increment(AI)) {
                 ErrorOr<std::unique_ptr<Binary>> ChildOrErr =
                     AI->getAsBinary(&Context);
                 if (ChildOrErr.getError())
@@ -1066,9 +1078,11 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
           } else if (ErrorOr<std::unique_ptr<Archive>> AOrErr =
                          I->getAsArchive()) {
             std::unique_ptr<Archive> &A = *AOrErr;
-            for (Archive::child_iterator AI = A->child_begin(),
-                                         AE = A->child_end();
-                 AI != AE; ++AI) {
+            ErrorOr<Archive::child_iterator> IOrErr = A->child_begin();
+            if (IOrErr.getError())
+              return;
+            for (Archive::child_iterator AI = *IOrErr, AE = A->child_end();
+                 AI != AE; Increment(AI)) {
               ErrorOr<std::unique_ptr<Binary>> ChildOrErr =
                   AI->getAsBinary(&Context);
               if (ChildOrErr.getError())
@@ -1116,8 +1130,11 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
         dumpSymbolNamesFromObject(Obj, false, ArchiveName, ArchitectureName);
       } else if (ErrorOr<std::unique_ptr<Archive>> AOrErr = I->getAsArchive()) {
         std::unique_ptr<Archive> &A = *AOrErr;
-        for (Archive::child_iterator AI = A->child_begin(), AE = A->child_end();
-             AI != AE; ++AI) {
+        ErrorOr<Archive::child_iterator> IOrErr = A->child_begin();
+        if (IOrErr.getError())
+          return;
+        for (Archive::child_iterator AI = *IOrErr, AE = A->child_end();
+             AI != AE; Increment(AI)) {
           ErrorOr<std::unique_ptr<Binary>> ChildOrErr =
               AI->getAsBinary(&Context);
           if (ChildOrErr.getError())
diff --git a/tools/llvm-objdump/MachODump.cpp b/tools/llvm-objdump/MachODump.cpp
index 3ea6fe4..57e1ccb 100644
--- a/tools/llvm-objdump/MachODump.cpp
+++ b/tools/llvm-objdump/MachODump.cpp
@@ -1417,8 +1417,10 @@ static void printArchiveChild(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) {
@@ -1451,14 +1453,24 @@ static void printArchiveChild(Archive::Child &C, bool verbose,
   }
 }
 
+static void Increment(Archive::child_iterator &I) {
+  ErrorOr<Archive::child_iterator> Next = I.getNext();
+  if (std::error_code EC = Next.getError())
+    report_fatal_error(EC.message());
+  I = *Next;
+}
+
 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);
   }
-  for (Archive::child_iterator I = A->child_begin(), E = A->child_end(); I != E;
-       ++I) {
+  ErrorOr<Archive::child_iterator> IOrErr = A->child_begin();
+  if (IOrErr.getError())
+    return;
+  for (Archive::child_iterator I = *IOrErr, E = A->child_end(); I != E;
+       Increment(I)) {
     Archive::Child C = *I;
     printArchiveChild(C, verbose, print_offset);
   }
@@ -1494,8 +1506,11 @@ void llvm::ParseInputMachO(StringRef Filename) {
     outs() << "Archive : " << Filename << "\n";
     if (ArchiveHeaders)
       printArchiveHeaders(A, !NonVerbose, ArchiveMemberOffsets);
-    for (Archive::child_iterator I = A->child_begin(), E = A->child_end();
-         I != E; ++I) {
+    ErrorOr<Archive::child_iterator> IOrErr = A->child_begin();
+    if (IOrErr.getError())
+      return;
+    for (Archive::child_iterator I = *IOrErr, E = A->child_end(); I != E;
+         Increment(I)) {
       ErrorOr<std::unique_ptr<Binary>> ChildOrErr = I->getAsBinary();
       if (ChildOrErr.getError())
         continue;
@@ -1541,9 +1556,11 @@ void llvm::ParseInputMachO(StringRef Filename) {
               outs() << "\n";
               if (ArchiveHeaders)
                 printArchiveHeaders(A.get(), !NonVerbose, ArchiveMemberOffsets);
-              for (Archive::child_iterator AI = A->child_begin(),
-                                           AE = A->child_end();
-                   AI != AE; ++AI) {
+              ErrorOr<Archive::child_iterator> IOrErr = A->child_begin();
+              if (IOrErr.getError())
+                return;
+              for (Archive::child_iterator AI = *IOrErr, AE = A->child_end();
+                   AI != AE; Increment(AI)) {
                 ErrorOr<std::unique_ptr<Binary>> ChildOrErr = AI->getAsBinary();
                 if (ChildOrErr.getError())
                   continue;
@@ -1583,9 +1600,11 @@ void llvm::ParseInputMachO(StringRef Filename) {
             outs() << "Archive : " << Filename << "\n";
             if (ArchiveHeaders)
               printArchiveHeaders(A.get(), !NonVerbose, ArchiveMemberOffsets);
-            for (Archive::child_iterator AI = A->child_begin(),
-                                         AE = A->child_end();
-                 AI != AE; ++AI) {
+            ErrorOr<Archive::child_iterator> IOrErr = A->child_begin();
+            if (IOrErr.getError())
+              return;
+            for (Archive::child_iterator AI = *IOrErr, AE = A->child_end();
+                 AI != AE; Increment(AI)) {
               ErrorOr<std::unique_ptr<Binary>> ChildOrErr = AI->getAsBinary();
               if (ChildOrErr.getError())
                 continue;
@@ -1620,8 +1639,11 @@ void llvm::ParseInputMachO(StringRef Filename) {
         outs() << "\n";
         if (ArchiveHeaders)
           printArchiveHeaders(A.get(), !NonVerbose, ArchiveMemberOffsets);
-        for (Archive::child_iterator AI = A->child_begin(), AE = A->child_end();
-             AI != AE; ++AI) {
+        ErrorOr<Archive::child_iterator> IOrErr = A->child_begin();
+        if (IOrErr.getError())
+          return;
+        for (Archive::child_iterator AI = *IOrErr, AE = A->child_end();
+             AI != AE; Increment(AI)) {
           ErrorOr<std::unique_ptr<Binary>> ChildOrErr = AI->getAsBinary();
           if (ChildOrErr.getError())
             continue;
diff --git a/tools/llvm-objdump/llvm-objdump.cpp b/tools/llvm-objdump/llvm-objdump.cpp
index 7292841..1c85308 100644
--- a/tools/llvm-objdump/llvm-objdump.cpp
+++ b/tools/llvm-objdump/llvm-objdump.cpp
@@ -1534,9 +1534,27 @@ static void DumpObject(const ObjectFile *o) {
     printFaultMaps(o);
 }
 
+static std::error_code Increment(Archive::child_iterator &I) {
+  ErrorOr<Archive::child_iterator> Next = I.getNext();
+  std::error_code EC;
+  if ((EC = Next.getError()))
+    return EC;
+  I = *Next;
+  return EC;
+}
+
 /// @brief Dump each object file in \a a;
 static void DumpArchive(const Archive *a) {
-  for (const Archive::Child &C : a->children()) {
+  ErrorOr<Archive::child_iterator> IOrErr = a->child_begin();
+  if (IOrErr.getError())
+    return;
+  std::error_code EC;
+  for (Archive::child_iterator AI = *IOrErr, AE = a->child_end(); AI != AE;
+       (EC = Increment(AI))) {
+    if (EC)
+      report_error(a->getFileName(), EC);
+    ;
+    const Archive::Child C = *AI;
     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..159f3cf 100644
--- a/tools/llvm-readobj/llvm-readobj.cpp
+++ b/tools/llvm-readobj/llvm-readobj.cpp
@@ -375,9 +375,26 @@ static void dumpObject(const ObjectFile *Obj) {
     Dumper->printStackMap();
 }
 
+static std::error_code Increment(Archive::child_iterator &I) {
+  ErrorOr<Archive::child_iterator> Next = I.getNext();
+  std::error_code EC;
+  if ((EC = Next.getError()))
+    return EC;
+  I = *Next;
+  return EC;
+}
+
 /// @brief Dumps each object file in \a Arc;
 static void dumpArchive(const Archive *Arc) {
-  for (const auto &Child : Arc->children()) {
+  ErrorOr<Archive::child_iterator> IOrErr = Arc->child_begin();
+  if (IOrErr.getError())
+    return;
+  std::error_code EC;
+  for (Archive::child_iterator I = *IOrErr, E = Arc->child_end(); I != E;
+       (EC = Increment(I))) {
+    if (EC)
+      reportError(Arc->getFileName(), EC.message());
+    const auto &Child = *I;
     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 de2b045..56beb2a 100644
--- a/tools/llvm-size/llvm-size.cpp
+++ b/tools/llvm-size/llvm-size.cpp
@@ -410,6 +410,15 @@ static bool checkMachOAndArchFlags(ObjectFile *o, StringRef file) {
   return true;
 }
 
+static std::error_code Increment(Archive::child_iterator &I) {
+  ErrorOr<Archive::child_iterator> Next = I.getNext();
+  std::error_code EC;
+  if ((EC = Next.getError()))
+    return EC;
+  I = *Next;
+  return EC;
+}
+
 /// @brief Print the section sizes for @p file. If @p file is an archive, print
 ///        the section sizes for each archive member.
 static void PrintFileSectionSizes(StringRef file) {
@@ -424,9 +433,16 @@ static void PrintFileSectionSizes(StringRef file) {
 
   if (Archive *a = dyn_cast<Archive>(&Bin)) {
     // This is an archive. Iterate over each member and display its sizes.
-    for (object::Archive::child_iterator i = a->child_begin(),
-                                         e = a->child_end();
-         i != e; ++i) {
+    ErrorOr<object::Archive::child_iterator> IOrErr = a->child_begin();
+    if (IOrErr.getError())
+      return;
+    std::error_code EC;
+    for (object::Archive::child_iterator i = *IOrErr, e = a->child_end();
+         i != e; (EC = Increment(i))) {
+      if (EC) {
+        errs() << ToolName << ": " << file << ": " << EC.message() << ".\n";
+        return;
+      }
       ErrorOr<std::unique_ptr<Binary>> ChildOrErr = i->getAsBinary();
       if (std::error_code EC = ChildOrErr.getError()) {
         errs() << ToolName << ": " << file << ": " << EC.message() << ".\n";
@@ -486,9 +502,19 @@ static void PrintFileSectionSizes(StringRef file) {
               std::unique_ptr<Archive> &UA = *AOrErr;
               // This is an archive. Iterate over each member and display its
               // sizes.
-              for (object::Archive::child_iterator i = UA->child_begin(),
+              ErrorOr<object::Archive::child_iterator> IOrErr =
+                  UA->child_begin();
+              if (IOrErr.getError())
+                return;
+              std::error_code EC;
+              for (object::Archive::child_iterator i = *IOrErr,
                                                    e = UA->child_end();
-                   i != e; ++i) {
+                   i != e; (EC = Increment(i))) {
+                if (EC) {
+                  errs() << ToolName << ": " << file << ": " << EC.message()
+                         << ".\n";
+                  return;
+                }
                 ErrorOr<std::unique_ptr<Binary>> ChildOrErr = i->getAsBinary();
                 if (std::error_code EC = ChildOrErr.getError()) {
                   errs() << ToolName << ": " << file << ": " << EC.message()
@@ -563,9 +589,18 @@ static void PrintFileSectionSizes(StringRef file) {
             std::unique_ptr<Archive> &UA = *AOrErr;
             // This is an archive. Iterate over each member and display its
             // sizes.
-            for (object::Archive::child_iterator i = UA->child_begin(),
+            ErrorOr<object::Archive::child_iterator> IOrErr = UA->child_begin();
+            if (IOrErr.getError())
+              return;
+            std::error_code EC;
+            for (object::Archive::child_iterator i = *IOrErr,
                                                  e = UA->child_end();
-                 i != e; ++i) {
+                 i != e; (EC = Increment(i))) {
+              if (EC) {
+                errs() << ToolName << ": " << file << ": " << EC.message()
+                       << ".\n";
+                return;
+              }
               ErrorOr<std::unique_ptr<Binary>> ChildOrErr = i->getAsBinary();
               if (std::error_code EC = ChildOrErr.getError()) {
                 errs() << ToolName << ": " << file << ": " << EC.message()
@@ -627,9 +662,16 @@ static void PrintFileSectionSizes(StringRef file) {
                          I->getAsArchive()) {
         std::unique_ptr<Archive> &UA = *AOrErr;
         // This is an archive. Iterate over each member and display its sizes.
-        for (object::Archive::child_iterator i = UA->child_begin(),
-                                             e = UA->child_end();
-             i != e; ++i) {
+        ErrorOr<object::Archive::child_iterator> IOrErr = UA->child_begin();
+        if (IOrErr.getError())
+          return;
+        std::error_code EC;
+        for (object::Archive::child_iterator i = *IOrErr, e = UA->child_end();
+             i != e; (EC = Increment(i))) {
+          if (EC) {
+            errs() << ToolName << ": " << file << ": " << EC.message() << ".\n";
+            return;
+          }
           ErrorOr<std::unique_ptr<Binary>> ChildOrErr = i->getAsBinary();
           if (std::error_code EC = ChildOrErr.getError()) {
             errs() << ToolName << ": " << file << ": " << EC.message() << ".\n";


More information about the llvm-commits mailing list