[llvm] r296527 - Actually add error handling to unpacking the dyld compact bind and

Kevin Enderby via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 28 13:47:07 PST 2017


Author: enderby
Date: Tue Feb 28 15:47:07 2017
New Revision: 296527

URL: http://llvm.org/viewvc/llvm-project?rev=296527&view=rev
Log:
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.


Modified:
    llvm/trunk/include/llvm/Object/MachO.h
    llvm/trunk/lib/Object/MachOObjectFile.cpp
    llvm/trunk/test/tools/llvm-objdump/macho-bad-ordinal.test
    llvm/trunk/tools/llvm-objdump/MachODump.cpp

Modified: llvm/trunk/include/llvm/Object/MachO.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/MachO.h?rev=296527&r1=296526&r2=296527&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Object/MachO.h (original)
+++ llvm/trunk/include/llvm/Object/MachO.h Tue Feb 28 15:47:07 2017
@@ -139,13 +139,16 @@ typedef content_iterator<MachORebaseEntr
 /// 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 @@ private:
   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 @@ public:
 
   // 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 @@ public:
                                                      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);
 

Modified: llvm/trunk/lib/Object/MachOObjectFile.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/MachOObjectFile.cpp?rev=296527&r1=296526&r2=296527&view=diff
==============================================================================
--- llvm/trunk/lib/Object/MachOObjectFile.cpp (original)
+++ llvm/trunk/lib/Object/MachOObjectFile.cpp Tue Feb 28 15:47:07 2017
@@ -2272,6 +2272,10 @@ std::error_code MachOObjectFile::getLibr
   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> MachOObj
   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 Ma
 }
 
 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);
 }
 

Modified: llvm/trunk/test/tools/llvm-objdump/macho-bad-ordinal.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/macho-bad-ordinal.test?rev=296527&r1=296526&r2=296527&view=diff
==============================================================================
--- llvm/trunk/test/tools/llvm-objdump/macho-bad-ordinal.test (original)
+++ llvm/trunk/test/tools/llvm-objdump/macho-bad-ordinal.test Tue Feb 28 15:47:07 2017
@@ -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)

Modified: llvm/trunk/tools/llvm-objdump/MachODump.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/MachODump.cpp?rev=296527&r1=296526&r2=296527&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objdump/MachODump.cpp (original)
+++ llvm/trunk/tools/llvm-objdump/MachODump.cpp Tue Feb 28 15:47:07 2017
@@ -9535,7 +9535,8 @@ void llvm::printMachOBindTable(const 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 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
 
   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
            << 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
 
   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
            << 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_sy
   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;




More information about the llvm-commits mailing list