Add error handling to the dyld compact export entries in libObject.

lld needs a matching change for this will be my next commit.
Expect it to fail build until that matching commit is picked up by the bots.

Like the changes in r296527 for dyld bind entires and the changes in
r298883 for lazy bind, weak bind and rebase entries the export
entries are the last of the dyld compact info to have error handling added.

This follows the model of iterators that can fail that Lang Hanes
designed when fixing the problem for bad archives r275316 (or r275361).

So that iterating through the exports now terminates if there is an error
and returns an llvm::Error with an error message in all cases for malformed
input.

This change provides the plumbing for the error handling, all the needed
testing of error conditions and test cases for all of the unique error messages.


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

include/llvm/Object/MachO.h

@@ -66,11 +66,13 @@ using dice_iterator = content_iterator<DiceRef>;
 /// ExportEntry encapsulates the current-state-of-the-walk used when doing a
 /// non-recursive walk of the trie data structure.  This allows you to iterate
 /// across all exported symbols using:
-///      for (const llvm::object::ExportEntry &AnExport : Obj->exports()) {
+///      Error Err;
+///      for (const llvm::object::ExportEntry &AnExport : Obj->exports(&Err)) {
 ///      }
+///      if (Err) { report error ...
 class ExportEntry {
 public:
-  ExportEntry(ArrayRef<uint8_t> Trie);
+  ExportEntry(Error *Err, const MachOObjectFile *O, ArrayRef<uint8_t> Trie);
 
   StringRef name() const;
   uint64_t flags() const;
@@ -88,7 +90,7 @@ private:
 
   void moveToFirst();
   void moveToEnd();
-  uint64_t readULEB128(const uint8_t *&p);
+  uint64_t readULEB128(const uint8_t *&p, const char **error);
   void pushDownUntilBottom();
   void pushNode(uint64_t Offset);
 
@@ -107,12 +109,19 @@ private:
     unsigned ParentStringLength = 0;
     bool IsExportNode = false;
   };
+  using NodeList = SmallVector<NodeState, 16>;
+  using node_iterator = NodeList::const_iterator;
 
+  Error *E;
+  const MachOObjectFile *O;
   ArrayRef<uint8_t> Trie;
   SmallString<256> CumulativeString;
-  SmallVector<NodeState, 16> Stack;
-  bool Malformed = false;
+  NodeList Stack;
   bool Done = false;
+
+  iterator_range<node_iterator> nodes() const {
+    return make_range(Stack.begin(), Stack.end());
+  }
 };
 using export_iterator = content_iterator<ExportEntry>;
 
@@ -356,10 +365,14 @@ public:
   iterator_range<load_command_iterator> load_commands() const;
 
   /// For use iterating over all exported symbols.
-  iterator_range<export_iterator> exports() const;
+  iterator_range<export_iterator> exports(Error &Err,
+                                          const MachOObjectFile *O) const;
 
   /// For use examining a trie not in a MachOObjectFile.
-  static iterator_range<export_iterator> exports(ArrayRef<uint8_t> Trie);
+  static iterator_range<export_iterator> exports(Error &Err,
+                                                 ArrayRef<uint8_t> Trie,
+                                                 const MachOObjectFile *O =
+                                                                      nullptr);
 
   /// For use iterating over all rebase table entries.
   iterator_range<rebase_iterator> rebaseTable(Error &Err);

lib/Object/MachOObjectFile.cpp

@@ -2607,10 +2607,14 @@ dice_iterator MachOObjectFile::end_dices() const {
   return dice_iterator(DiceRef(DRI, this));
 }
 
-ExportEntry::ExportEntry(ArrayRef<uint8_t> T) : Trie(T) {}
+ExportEntry::ExportEntry(Error *E, const MachOObjectFile *O,
+                         ArrayRef<uint8_t> T) : E(E), O(O), Trie(T) {}
 
 void ExportEntry::moveToFirst() {
+  ErrorAsOutParameter ErrAsOutParam(E);
   pushNode(0);
+  if (*E)
+    return;
   pushDownUntilBottom();
 }
 
@@ -2637,14 +2641,12 @@ bool ExportEntry::operator==(const ExportEntry &Other) const {
   return true;
 }
 
-uint64_t ExportEntry::readULEB128(const uint8_t *&Ptr) {
+uint64_t ExportEntry::readULEB128(const uint8_t *&Ptr, const char **error) {
   unsigned Count;
-  uint64_t Result = decodeULEB128(Ptr, &Count);
+  uint64_t Result = decodeULEB128(Ptr, &Count, Trie.end(), error);
   Ptr += Count;
-  if (Ptr > Trie.end()) {
+  if (Ptr > Trie.end())
     Ptr = Trie.end();
-    Malformed = true;
-  }
   return Result;
 }
 
@@ -2679,22 +2681,119 @@ ExportEntry::NodeState::NodeState(const uint8_t *Ptr)
     : Start(Ptr), Current(Ptr) {}
 
 void ExportEntry::pushNode(uint64_t offset) {
+  ErrorAsOutParameter ErrAsOutParam(E);
   const uint8_t *Ptr = Trie.begin() + offset;
   NodeState State(Ptr);
-  uint64_t ExportInfoSize = readULEB128(State.Current);
+  const char *error;
+  uint64_t ExportInfoSize = readULEB128(State.Current, &error);
+  if (error) {
+    *E = malformedError("export info size " + Twine(error) + " in export trie "
+           "data at node: 0x" + utohexstr(offset));
+    moveToEnd();
+    return;
+  }
   State.IsExportNode = (ExportInfoSize != 0);
   const uint8_t* Children = State.Current + ExportInfoSize;
+  if (Children > Trie.end()) {
+    *E = malformedError("export info size: 0x" + utohexstr(ExportInfoSize) +
+           " in export trie data at node: 0x" + utohexstr(offset) +
+           " too big and extends past end of trie data");
+    moveToEnd();
+    return;
+  }
   if (State.IsExportNode) {
-    State.Flags = readULEB128(State.Current);
+    const uint8_t *ExportStart = State.Current;
+    State.Flags = readULEB128(State.Current, &error);
+    if (error) {
+      *E = malformedError("flags " + Twine(error) + " in export trie data at "
+             "node: 0x" + utohexstr(offset));
+      moveToEnd();
+      return;
+    }
+    uint64_t Kind = State.Flags & MachO::EXPORT_SYMBOL_FLAGS_KIND_MASK;
+    if (State.Flags != 0 &&
+        (Kind != MachO::EXPORT_SYMBOL_FLAGS_KIND_REGULAR &&
+         Kind != MachO::EXPORT_SYMBOL_FLAGS_KIND_ABSOLUTE &&
+         Kind != MachO::EXPORT_SYMBOL_FLAGS_KIND_THREAD_LOCAL)) {
+      *E = malformedError("unsupported exported symbol kind: "
+             + Twine((int)Kind) + " in flags: 0x" + utohexstr(State.Flags) +
+             " in export trie data at node: 0x" + utohexstr(offset));
+      moveToEnd();
+      return;
+    }
     if (State.Flags & MachO::EXPORT_SYMBOL_FLAGS_REEXPORT) {
       State.Address = 0;
-      State.Other = readULEB128(State.Current); // dylib ordinal
+      State.Other = readULEB128(State.Current, &error); // dylib ordinal
+      if (error) {
+        *E = malformedError("dylib ordinal of re-export " + Twine(error) +
+               " in export trie data at node: 0x" + utohexstr(offset));
+        moveToEnd();
+        return;
+      }
+      if (O != nullptr) {
+        if (State.Other > O->getLibraryCount()) {
+          *E = malformedError("bad library ordinal: " + Twine((int)State.Other)
+               + " (max " + Twine((int)O->getLibraryCount()) + ") in export "
+               "trie data at node: 0x" + utohexstr(offset));
+          moveToEnd();
+          return;
+        }
+      }
       State.ImportName = reinterpret_cast<const char*>(State.Current);
+      if (*State.ImportName == '\0') {
+        State.Current++;
+      } else {
+        const uint8_t *End = State.Current + 1;
+        if (End >= Trie.end()) {
+          *E = malformedError("import name of re-export in export trie data at "
+                 "node: 0x" + utohexstr(offset) + " starts past end of trie "
+                 "data");
+          moveToEnd();
+          return;
+        }
+        while(*End != '\0' && End < Trie.end())
+          End++;
+        if (*End != '\0') {
+          *E = malformedError("import name of re-export in export trie data at "
+                 "node: 0x" + utohexstr(offset) + " extends past end of trie "
+                 "data");
+          moveToEnd();
+          return;
+        }
+        State.Current = End + 1;
+      }
     } else {
-      State.Address = readULEB128(State.Current);
-      if (State.Flags & MachO::EXPORT_SYMBOL_FLAGS_STUB_AND_RESOLVER)
-        State.Other = readULEB128(State.Current);
+      State.Address = readULEB128(State.Current, &error);
+      if (error) {
+        *E = malformedError("address " + Twine(error) + " in export trie data "
+               "at node: 0x" + utohexstr(offset));
+        moveToEnd();
+        return;
+      }
+      if (State.Flags & MachO::EXPORT_SYMBOL_FLAGS_STUB_AND_RESOLVER) {
+        State.Other = readULEB128(State.Current, &error);
+        if (error) {
+          *E = malformedError("resolver of stub and resolver " + Twine(error) +
+                 " in export trie data at node: 0x" + utohexstr(offset));
+          moveToEnd();
+          return;
+        }
+      }
     }
+    if(ExportStart + ExportInfoSize != State.Current) {
+      *E = malformedError("inconsistant export info size: 0x" +
+             utohexstr(ExportInfoSize) + " where actual size was: 0x" +
+             utohexstr(State.Current - ExportStart) + " in export trie data "
+             "at node: 0x" + utohexstr(offset));
+      moveToEnd();
+      return;
+    }
+  }
+  if (Children + 1 >= Trie.end()) {
+    *E = malformedError("byte for count of childern in export trie data at "
+           "node: 0x" + utohexstr(offset) + " extends past end of trie data");
+    moveToEnd();
+    return;
   }
   State.ChildCount = *Children;
   State.Current = Children + 1;
@@ -2704,21 +2803,50 @@ void ExportEntry::pushNode(uint64_t offset) {
 }
 
 void ExportEntry::pushDownUntilBottom() {
+  ErrorAsOutParameter ErrAsOutParam(E);
+  const char *error;
   while (Stack.back().NextChildIndex < Stack.back().ChildCount) {
     NodeState &Top = Stack.back();
     CumulativeString.resize(Top.ParentStringLength);
-    for (;*Top.Current != 0; Top.Current++) {
+    for (;*Top.Current != 0 && Top.Current < Trie.end(); Top.Current++) {
       char C = *Top.Current;
       CumulativeString.push_back(C);
     }
+    if (Top.Current >= Trie.end()) {
+      *E = malformedError("edge sub-string in export trie data at node: 0x" +
+             utohexstr(Top.Start - Trie.begin()) + " for child #" + 
+             Twine((int)Top.NextChildIndex) + " extends past end of trie data");
+      moveToEnd();
+      return;
+    }
     Top.Current += 1;
-    uint64_t childNodeIndex = readULEB128(Top.Current);
+    uint64_t childNodeIndex = readULEB128(Top.Current, &error);
+    if (error) {
+      *E = malformedError("child node offset " + Twine(error) +
+             " in export trie data at node: 0x" +
+             utohexstr(Top.Start - Trie.begin()));
+      moveToEnd();
+      return;
+    }
+    for (const NodeState &node : nodes()) {
+      if (node.Start == Trie.begin() + childNodeIndex){
+        *E = malformedError("loop in childern in export trie data at node: 0x" +
+               utohexstr(Top.Start - Trie.begin()) + " back to node: 0x" +
+               utohexstr(childNodeIndex));
+        moveToEnd();
+        return;
+      }
+    }
     Top.NextChildIndex += 1;
     pushNode(childNodeIndex);
+    if (*E)
+      return;
   }
   if (!Stack.back().IsExportNode) {
-    Malformed = true;
+    *E = malformedError("node is not an export node in export trie data at "
+           "node: 0x" + utohexstr(Stack.back().Start - Trie.begin()));
     moveToEnd();
+    return;
   }
 }
 
@@ -2738,8 +2866,10 @@ void ExportEntry::pushDownUntilBottom() {
 // stack ivar.  If there is no more ways down, it pops up one and tries to go
 // down a sibling path until a childless node is reached.
 void ExportEntry::moveNext() {
-  if (Stack.empty() || !Stack.back().IsExportNode) {
-    Malformed = true;
+  assert(!Stack.empty() && "ExportEntry::moveNext() with empty node stack");
+  if (!Stack.back().IsExportNode) {
+    *E = malformedError("node is not an export node in export trie data at "
+           "node: 0x" + utohexstr(Stack.back().Start - Trie.begin()));
     moveToEnd();
     return;
   }
@@ -2764,21 +2894,23 @@ void ExportEntry::moveNext() {
 }
 
 iterator_range<export_iterator>
-MachOObjectFile::exports(ArrayRef<uint8_t> Trie) {
-  ExportEntry Start(Trie);
+MachOObjectFile::exports(Error &E, ArrayRef<uint8_t> Trie,
+                         const MachOObjectFile *O) {
+  ExportEntry Start(&E, O, Trie);
   if (Trie.empty())
     Start.moveToEnd();
   else
     Start.moveToFirst();
 
-  ExportEntry Finish(Trie);
+  ExportEntry Finish(&E, O, Trie);
   Finish.moveToEnd();
 
   return make_range(export_iterator(Start), export_iterator(Finish));
 }
 
-iterator_range<export_iterator> MachOObjectFile::exports() const {
-  return exports(getDyldInfoExportsTrie());
+iterator_range<export_iterator> MachOObjectFile::exports(Error &Err,
+                                  const MachOObjectFile *O) const {
+  return exports(Err, getDyldInfoExportsTrie(), O);
 }
 
 MachORebaseEntry::MachORebaseEntry(Error *E, const MachOObjectFile *O,

test/tools/llvm-objdump/macho-bad-trie.test

@@ -0,0 +1,35 @@
+RUN: not llvm-objdump -macho -exports-trie %p/Inputs/macho-trie-bad-kind 2>&1 | FileCheck -check-prefix BAD_KIND %s 
+BAD_KIND: macho-trie-bad-kind': truncated or malformed object (unsupported exported symbol kind: 3 in flags: 0x13 in export trie data at node: 0x53)
+
+RUN: not llvm-objdump -macho -exports-trie %p/Inputs/macho-trie-bad-export-info-malformed-uleb128 2>&1 | FileCheck -check-prefix MALFORMED_ULEB128 %s 
+MALFORMED_ULEB128: macho-trie-bad-export-info-malformed-uleb128': truncated or malformed object (export info size malformed uleb128, extends past end in export trie data at node: 0x5A)
+
+RUN: not llvm-objdump -macho -exports-trie %p/Inputs/macho-trie-bad-export-info-malformed-uleb128_too_big 2>&1 | FileCheck -check-prefix MALFORMED_ULEB128_TOO_BIG %s 
+MALFORMED_ULEB128_TOO_BIG: macho-trie-bad-export-info-malformed-uleb128_too_big': truncated or malformed object (export info size uleb128 too big for uint64 in export trie data at node: 0x5A)
+
+RUN: not llvm-objdump -macho -exports-trie %p/Inputs/macho-trie-export-info-size-too-big 2>&1 | FileCheck -check-prefix EXPORT_INFO_SIZE_TOO_BIG %s 
+EXPORT_INFO_SIZE_TOO_BIG: macho-trie-export-info-size-too-big': truncated or malformed object (export info size: 0x1234 in export trie data at node: 0x33 too big and extends past end of trie data)
+
+RUN: not llvm-objdump -macho -exports-trie %p/Inputs/macho-trie-children-count-byte 2>&1 | FileCheck -check-prefix CHILDREN_COUNT_BYTE %s 
+CHILDREN_COUNT_BYTE: macho-trie-children-count-byte': truncated or malformed object (byte for count of childern in export trie data at node: 0x5 extends past end of trie data)
+
+RUN: not llvm-objdump -macho -exports-trie %p/Inputs/macho-trie-import-name-start 2>&1 | FileCheck -check-prefix IMPORT_NAME_START %s 
+IMPORT_NAME_START: macho-trie-import-name-start': truncated or malformed object (import name of re-export in export trie data at node: 0x33 starts past end of trie data)
+
+RUN: not llvm-objdump -macho -exports-trie %p/Inputs/macho-trie-import-name-end 2>&1 | FileCheck -check-prefix IMPORT_NAME_END %s 
+IMPORT_NAME_END: macho-trie-import-name-end': truncated or malformed object (import name of re-export in export trie data at node: 0x33 extends past end of trie data)
+
+RUN: not llvm-objdump -macho -exports-trie %p/Inputs/macho-trie-edge-string-end 2>&1 | FileCheck -check-prefix EDGE_STRING_END %s 
+EDGE_STRING_END: macho-trie-edge-string-end': truncated or malformed object (edge sub-string in export trie data at node: 0x42 for child #0 extends past end of trie data)
+
+RUN: not llvm-objdump -macho -exports-trie %p/Inputs/macho-trie-not-export-node 2>&1 | FileCheck -check-prefix NOT_EXPORT_NODE %s 
+NOT_EXPORT_NODE: macho-trie-not-export-node': truncated or malformed object (node is not an export node in export trie data at node: 0x5A)
+
+RUN: not llvm-objdump -macho -exports-trie %p/Inputs/macho-trie-node-loop 2>&1 | FileCheck -check-prefix LOOP_OF_CHILDERN %s 
+LOOP_OF_CHILDERN: macho-trie-node-loop': truncated or malformed object (loop in childern in export trie data at node: 0x42 back to node: 0x5)
+
+RUN: not llvm-objdump -macho -exports-trie %p/Inputs/macho-trie-bad-library-ordinal 2>&1 | FileCheck -check-prefix BAD_LIBRARY_ORDINAL %s 
+BAD_LIBRARY_ORDINAL: macho-trie-bad-library-ordinal': truncated or malformed object (bad library ordinal: 69 (max 3) in export trie data at node: 0x33)
+
+RUN: not llvm-objdump -macho -exports-trie %p/Inputs/macho-inconsistant-export 2>&1 | FileCheck -check-prefix INCONSISTANT_EXPORT_SIZE %s 
+INCONSISTANT_EXPORT_SIZE: macho-inconsistant-export': truncated or malformed object (inconsistant export info size: 0x9 where actual size was: 0x5 in export trie data at node: 0x53)

tools/llvm-nm/llvm-nm.cpp

@@ -1226,7 +1226,9 @@ dumpSymbolNamesFromObject(SymbolicFile &Obj, bool printName,
     if (DyldInfoOnly || AddDyldInfo ||
         HFlags & MachO::MH_NLIST_OUTOFSYNC_WITH_DYLDINFO) {
       unsigned ExportsAdded = 0;
-      for (const llvm::object::ExportEntry &Entry : MachO->exports()) {
+      Error Err = Error::success();
+      for (const llvm::object::ExportEntry &Entry : MachO->exports(Err,
+                                                                   MachO)) {
         bool found = false;
         bool ReExport = false;
         if (!DyldInfoOnly) {
@@ -1362,6 +1364,8 @@ dumpSymbolNamesFromObject(SymbolicFile &Obj, bool printName,
           }
         }
       }
+      if (Err)
+        error(std::move(Err), MachO->getFileName());
       // Set the symbol names and indirect names for the added symbols.
       if (ExportsAdded) {
         EOS.flush();

tools/llvm-objdump/MachODump.cpp

@@ -9402,7 +9402,8 @@ void llvm::printMachOExportsTrie(const object::MachOObjectFile *Obj) {
       }
     }
   }
-  for (const llvm::object::ExportEntry &Entry : Obj->exports()) {
+  Error Err = Error::success();
+  for (const llvm::object::ExportEntry &Entry : Obj->exports(Err, Obj)) {
     uint64_t Flags = Entry.flags();
     bool ReExport = (Flags & MachO::EXPORT_SYMBOL_FLAGS_REEXPORT);
     bool WeakDef = (Flags & MachO::EXPORT_SYMBOL_FLAGS_WEAK_DEFINITION);
@@ -9455,6 +9456,8 @@ void llvm::printMachOExportsTrie(const object::MachOObjectFile *Obj) {
     }
     outs() << "\n";
   }
+  if (Err)
+    report_error(Obj->getFileName(), std::move(Err));
 }
 
 //===----------------------------------------------------------------------===//