Actually add error handling to unpacking the dyld compact bind and

other tables.  Providing a helpful error message to what the error is and
where the error occurred based on which opcode it was associated with.

There have been handful of bug fixes dealing with bad bind info in
object files, r294021 and r249845, which only put a band aid on the
problem after a bad bind table was created after unpacking from
its compact info.  In these cases a bind table should have never been
created and an error should have simply been generated.

This change puts in place the plumbing to allow checking and returning
of an error when the compact info is unpacked.  This follows the model
of iterators that can fail that Lang Hanes designed when fixing the problem
for bad archives r275316 (or r275361).

This change uses one of the existing test cases that now causes an
error instead of printing <<bad library ordinal>> after a bad bind table
is created.  The error uses the offset into the opcode table as shown with
the macOS dyldinfo(1) tool to indicate where the error is and which
opcode and which parameter is in error.

For example the exiting test case has this lazy binding opcode table:

% dyldinfo -opcodes test/tools/llvm-objdump/Inputs/bad-ordinal.macho-x86_64 
…
lazy binding opcodes:
0x0000 BIND_OPCODE_SET_SEGMENT_AND_OFFSET_ULEB(0x02, 0x00000010)
0x0002 BIND_OPCODE_SET_DYLIB_ORDINAL_IMM(2)

In the test case the binary only has one library so setting the library 
ordinal to the value of 2 in the BIND_OPCODE_SET_DYLIB_ORDINAL_IMM
opcode at 0x0002 above is an error.  This now produces this error message:

% llvm-objdump -lazy-bind bad-ordinal.macho-x86_64 
…
llvm-objdump: 'bad-ordinal.macho-x86_64': truncated or malformed object (for BIND_OPCODE_SET_DYLIB_ORDINAL_ULEB bad library ordinal: 2 (max 1) for opcode at: 0x2)

This change provides the plumbing for the error handling and one example
of an error message.  Other error checks and test cases will be added in follow
on commits.



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

include/llvm/Object/MachO.h

@@ -139,13 +139,16 @@ typedef content_iterator<MachORebaseEntry> rebase_iterator;
 /// MachOBindEntry encapsulates the current state in the decompression of
 /// binding opcodes. This allows you to iterate through the compressed table of
 /// bindings using:
-///    for (const llvm::object::MachOBindEntry &Entry : Obj->bindTable()) {
+///    Error Err;
+///    for (const llvm::object::MachOBindEntry &Entry : Obj->bindTable(&Err)) {
 ///    }
+///    if (Err) { report error ...
 class MachOBindEntry {
 public:
   enum class Kind { Regular, Lazy, Weak };
 
-  MachOBindEntry(ArrayRef<uint8_t> Opcodes, bool is64Bit, MachOBindEntry::Kind);
+  MachOBindEntry(Error *Err, const MachOObjectFile *O,
+                 ArrayRef<uint8_t> Opcodes, bool is64Bit, MachOBindEntry::Kind);
 
   uint32_t segmentIndex() const;
   uint64_t segmentOffset() const;
@@ -166,6 +169,8 @@ class MachOBindEntry {
   uint64_t readULEB128();
   int64_t readSLEB128();
 
+  Error *E;
+  const MachOObjectFile *O;
   ArrayRef<uint8_t> Opcodes;
   const uint8_t *Ptr;
   uint64_t SegmentOffset;
@@ -245,6 +250,7 @@ class MachOObjectFile : public ObjectFile {
 
   // MachO specific.
   std::error_code getLibraryShortNameByIndex(unsigned Index, StringRef &) const;
+  uint32_t getLibraryCount() const;
 
   section_iterator getRelocationRelocatedSection(relocation_iterator Rel) const;
 
@@ -292,16 +298,18 @@ class MachOObjectFile : public ObjectFile {
                                                      bool is64);
 
   /// For use iterating over all bind table entries.
-  iterator_range<bind_iterator> bindTable() const;
+  iterator_range<bind_iterator> bindTable(Error &Err) const;
 
   /// For use iterating over all lazy bind table entries.
-  iterator_range<bind_iterator> lazyBindTable() const;
+  iterator_range<bind_iterator> lazyBindTable(Error &Err) const;
 
-  /// For use iterating over all lazy bind table entries.
-  iterator_range<bind_iterator> weakBindTable() const;
+  /// For use iterating over all weak bind table entries.
+  iterator_range<bind_iterator> weakBindTable(Error &Err) const;
 
   /// For use examining bind opcodes not in a MachOObjectFile.
-  static iterator_range<bind_iterator> bindTable(ArrayRef<uint8_t> Opcodes,
+  static iterator_range<bind_iterator> bindTable(Error &Err,
+                                                 const MachOObjectFile *O,
+                                                 ArrayRef<uint8_t> Opcodes,
                                                  bool is64,
                                                  MachOBindEntry::Kind);
 

lib/Object/MachOObjectFile.cpp

@@ -2272,6 +2272,10 @@ std::error_code MachOObjectFile::getLibraryShortNameByIndex(unsigned Index,
   return std::error_code();
 }
 
+uint32_t MachOObjectFile::getLibraryCount() const {
+  return Libraries.size();
+}
+
 section_iterator
 MachOObjectFile::getRelocationRelocatedSection(relocation_iterator Rel) const {
   DataRefImpl Sec;
@@ -2925,8 +2929,9 @@ iterator_range<rebase_iterator> MachOObjectFile::rebaseTable() const {
   return rebaseTable(getDyldInfoRebaseOpcodes(), is64Bit());
 }
 
-MachOBindEntry::MachOBindEntry(ArrayRef<uint8_t> Bytes, bool is64Bit, Kind BK)
-    : Opcodes(Bytes), Ptr(Bytes.begin()), SegmentOffset(0), SegmentIndex(0),
+MachOBindEntry::MachOBindEntry(Error *E, const MachOObjectFile *O,
+                               ArrayRef<uint8_t> Bytes, bool is64Bit, Kind BK)
+    : E(E), O(O), Opcodes(Bytes), Ptr(Bytes.begin()), SegmentOffset(0), SegmentIndex(0),
       Ordinal(0), Flags(0), Addend(0), RemainingLoopCount(0), AdvanceAmount(0),
       BindType(0), PointerSize(is64Bit ? 8 : 4),
       TableKind(BK), Malformed(false), Done(false) {}
@@ -2943,6 +2948,7 @@ void MachOBindEntry::moveToEnd() {
 }
 
 void MachOBindEntry::moveNext() {
+  ErrorAsOutParameter ErrAsOutParam(E);
   // If in the middle of some loop, move to next binding in loop.
   SegmentOffset += AdvanceAmount;
   if (RemainingLoopCount) {
@@ -2956,6 +2962,7 @@ void MachOBindEntry::moveNext() {
   bool More = true;
   while (More && !Malformed) {
     // Parse next opcode and set up next loop.
+    const uint8_t *OpcodeStart = Ptr;
     uint8_t Byte = *Ptr++;
     uint8_t ImmValue = Byte & MachO::BIND_IMMEDIATE_MASK;
     uint8_t Opcode = Byte & MachO::BIND_OPCODE_MASK;
@@ -2982,6 +2989,14 @@ void MachOBindEntry::moveNext() {
       break;
     case MachO::BIND_OPCODE_SET_DYLIB_ORDINAL_IMM:
       Ordinal = ImmValue;
+      if (ImmValue > O->getLibraryCount()) {
+        *E = malformedError("for BIND_OPCODE_SET_DYLIB_ORDINAL_ULEB bad "
+             "library ordinal: " + Twine((int)ImmValue) + " (max " +
+             Twine((int)O->getLibraryCount()) + ") for opcode at: 0x" +
+             utohexstr(OpcodeStart - Opcodes.begin()));
+        moveToEnd();
+        return;
+      }
       DEBUG_WITH_TYPE(
           "mach-o-bind",
           llvm::dbgs() << "BIND_OPCODE_SET_DYLIB_ORDINAL_IMM: "
@@ -3165,29 +3180,30 @@ bool MachOBindEntry::operator==(const MachOBindEntry &Other) const {
 }
 
 iterator_range<bind_iterator>
-MachOObjectFile::bindTable(ArrayRef<uint8_t> Opcodes, bool is64,
+MachOObjectFile::bindTable(Error &Err, const MachOObjectFile *O,
+                           ArrayRef<uint8_t> Opcodes, bool is64,
                            MachOBindEntry::Kind BKind) {
-  MachOBindEntry Start(Opcodes, is64, BKind);
+  MachOBindEntry Start(&Err, O, Opcodes, is64, BKind);
   Start.moveToFirst();
 
-  MachOBindEntry Finish(Opcodes, is64, BKind);
+  MachOBindEntry Finish(&Err, O, Opcodes, is64, BKind);
   Finish.moveToEnd();
 
   return make_range(bind_iterator(Start), bind_iterator(Finish));
 }
 
-iterator_range<bind_iterator> MachOObjectFile::bindTable() const {
-  return bindTable(getDyldInfoBindOpcodes(), is64Bit(),
+iterator_range<bind_iterator> MachOObjectFile::bindTable(Error &Err) const {
+  return bindTable(Err, this, getDyldInfoBindOpcodes(), is64Bit(),
                    MachOBindEntry::Kind::Regular);
 }
 
-iterator_range<bind_iterator> MachOObjectFile::lazyBindTable() const {
-  return bindTable(getDyldInfoLazyBindOpcodes(), is64Bit(),
+iterator_range<bind_iterator> MachOObjectFile::lazyBindTable(Error &Err) const {
+  return bindTable(Err, this, getDyldInfoLazyBindOpcodes(), is64Bit(),
                    MachOBindEntry::Kind::Lazy);
 }
 
-iterator_range<bind_iterator> MachOObjectFile::weakBindTable() const {
-  return bindTable(getDyldInfoWeakBindOpcodes(), is64Bit(),
+iterator_range<bind_iterator> MachOObjectFile::weakBindTable(Error &Err) const {
+  return bindTable(Err, this, getDyldInfoWeakBindOpcodes(), is64Bit(),
                    MachOBindEntry::Kind::Weak);
 }
 

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

@@ -1,6 +1,4 @@
-# RUN: llvm-objdump -macho -bind -lazy-bind %p/Inputs/bad-ordinal.macho-x86_64 \
-# RUN:   | FileCheck %s 
+# RUN: not llvm-objdump -macho -lazy-bind %p/Inputs/bad-ordinal.macho-x86_64 \
+# RUN: 2>&1 | FileCheck %s 
 
-
-# CHECK: __DATA   __nl_symbol_ptr    0x100001000 pointer         0 <<bad library ordinal>> dyld_stub_binder
-# CHECK: __DATA   __la_symbol_ptr    0x100001010 <<bad library ordinal>> _printf
+# CHECK: bad-ordinal.macho-x86_64': truncated or malformed object (for BIND_OPCODE_SET_DYLIB_ORDINAL_ULEB bad library ordinal: 2 (max 1) for opcode at: 0x2)

tools/llvm-objdump/MachODump.cpp

@@ -9535,7 +9535,8 @@ void llvm::printMachOBindTable(const object::MachOObjectFile *Obj) {
 
   outs() << "segment  section            address    type       "
             "addend dylib            symbol\n";
-  for (const llvm::object::MachOBindEntry &Entry : Obj->bindTable()) {
+  Error Err = Error::success();
+  for (const llvm::object::MachOBindEntry &Entry : Obj->bindTable(Err)) {
     uint32_t SegIndex = Entry.segmentIndex();
     uint64_t OffsetInSeg = Entry.segmentOffset();
     StringRef SegmentName = sectionTable.segmentName(SegIndex);
@@ -9555,6 +9556,8 @@ void llvm::printMachOBindTable(const object::MachOObjectFile *Obj) {
            << left_justify(ordinalName(Obj, Entry.ordinal()), 16) << " "
            << Entry.symbolName() << Attr << "\n";
   }
+  if (Err)
+    report_error(Obj->getFileName(), std::move(Err));
 }
 
 //===----------------------------------------------------------------------===//
@@ -9567,7 +9570,8 @@ void llvm::printMachOLazyBindTable(const object::MachOObjectFile *Obj) {
 
   outs() << "segment  section            address     "
             "dylib            symbol\n";
-  for (const llvm::object::MachOBindEntry &Entry : Obj->lazyBindTable()) {
+  Error Err = Error::success();
+  for (const llvm::object::MachOBindEntry &Entry : Obj->lazyBindTable(Err)) {
     uint32_t SegIndex = Entry.segmentIndex();
     uint64_t OffsetInSeg = Entry.segmentOffset();
     StringRef SegmentName = sectionTable.segmentName(SegIndex);
@@ -9582,6 +9586,8 @@ void llvm::printMachOLazyBindTable(const object::MachOObjectFile *Obj) {
            << left_justify(ordinalName(Obj, Entry.ordinal()), 16) << " "
            << Entry.symbolName() << "\n";
   }
+  if (Err)
+    report_error(Obj->getFileName(), std::move(Err));
 }
 
 //===----------------------------------------------------------------------===//
@@ -9594,7 +9600,8 @@ void llvm::printMachOWeakBindTable(const object::MachOObjectFile *Obj) {
 
   outs() << "segment  section            address     "
             "type       addend   symbol\n";
-  for (const llvm::object::MachOBindEntry &Entry : Obj->weakBindTable()) {
+  Error Err = Error::success();
+  for (const llvm::object::MachOBindEntry &Entry : Obj->weakBindTable(Err)) {
     // Strong symbols don't have a location to update.
     if (Entry.flags() & MachO::BIND_SYMBOL_FLAGS_NON_WEAK_DEFINITION) {
       outs() << "                                        strong              "
@@ -9616,6 +9623,8 @@ void llvm::printMachOWeakBindTable(const object::MachOObjectFile *Obj) {
            << format_decimal(Entry.addend(), 8) << "   " << Entry.symbolName()
            << "\n";
   }
+  if (Err)
+    report_error(Obj->getFileName(), std::move(Err));
 }
 
 // get_dyld_bind_info_symbolname() is used for disassembly and passed an
@@ -9627,16 +9636,22 @@ static const char *get_dyld_bind_info_symbolname(uint64_t ReferenceValue,
   if (info->bindtable == nullptr) {
     info->bindtable = llvm::make_unique<SymbolAddressMap>();
     SegInfo sectionTable(info->O);
-    for (const llvm::object::MachOBindEntry &Entry : info->O->bindTable()) {
+    Error Err = Error::success();
+    for (const llvm::object::MachOBindEntry &Entry : info->O->bindTable(Err)) {
       uint32_t SegIndex = Entry.segmentIndex();
       uint64_t OffsetInSeg = Entry.segmentOffset();
-      if (!sectionTable.isValidSegIndexAndOffset(SegIndex, OffsetInSeg))
+      if (!sectionTable.isValidSegIndexAndOffset(SegIndex, OffsetInSeg)) {
+        if (Err)
+          report_error(info->O->getFileName(), std::move(Err));
         return nullptr;
+      }
       uint64_t Address = sectionTable.address(SegIndex, OffsetInSeg);
       StringRef name = Entry.symbolName();
       if (!name.empty())
         (*info->bindtable)[Address] = name;
     }
+    if (Err)
+      report_error(info->O->getFileName(), std::move(Err));
   }
   auto name = info->bindtable->lookup(ReferenceValue);
   return !name.empty() ? name.data() : nullptr;