[llvm] f72d001 - llvm-objdump should ignore Mach-O stab symbols for disassembly.

[Michael Trent via llvm-commits]

Author: Michael Trent
Date: 2019-12-20T15:20:53-08:00
New Revision: f72d001e099871fd5943679a8377f2fcc675f860

URL: https://github.com/llvm/llvm-project/commit/f72d001e099871fd5943679a8377f2fcc675f860
DIFF: https://github.com/llvm/llvm-project/commit/f72d001e099871fd5943679a8377f2fcc675f860.diff

LOG: llvm-objdump should ignore Mach-O stab symbols for disassembly.

Summary:
llvm-objdump will commonly error out when disassembling a Mach-O binary with
stab symbols, or when printing a Mach-O symbol table that includesstab symbols.
That is because the Mach-O N_OSO symbol has been modified to include the
bottom 8-bit value of the Mach-O's cpusubtype value in the section field. In
general, one cannot blindly assume a stab symbol's section field is valid
unless one has actually consulted the specification for the specific stab.

Since objdump mostly just walks the symbol table to get mnemonics for code
disassembly it's best for objdump to just ignore stab symbols. llvm-nm will
do a more complete and correct job of displaying Mach-O symbol table contents.

Reviewers: pete, lhames, ab, thegameg, jhenderson, MaskRay

Reviewed By: thegameg, MaskRay

Subscribers: MaskRay, rupprecht, seiya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D71394

Added: 
    llvm/test/tools/llvm-objdump/Inputs/macho-stabs-x86_64
    llvm/test/tools/llvm-objdump/macho-stabs.test

Modified: 
    llvm/tools/llvm-objdump/llvm-objdump.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/tools/llvm-objdump/Inputs/macho-stabs-x86_64 b/llvm/test/tools/llvm-objdump/Inputs/macho-stabs-x86_64
new file mode 100755
index 000000000000..e95c8204c710
Binary files /dev/null and b/llvm/test/tools/llvm-objdump/Inputs/macho-stabs-x86_64 
diff er

diff  --git a/llvm/test/tools/llvm-objdump/macho-stabs.test b/llvm/test/tools/llvm-objdump/macho-stabs.test
new file mode 100644
index 000000000000..e2cdc6796004
--- /dev/null
+++ b/llvm/test/tools/llvm-objdump/macho-stabs.test
@@ -0,0 +1,2 @@
+# RUN: llvm-objdump --syms %p/Inputs/macho-stabs-x86_64
+# RUN: llvm-objdump -D %p/Inputs/macho-stabs-x86_64

diff  --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
index a9014caf4bda..004edfff0cef 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -1134,6 +1134,7 @@ static void disassembleObject(const Target *TheTarget, const ObjectFile *Obj,
   std::map<SectionRef, SectionSymbolsTy> AllSymbols;
   SectionSymbolsTy AbsoluteSymbols;
   const StringRef FileName = Obj->getFileName();
+  const MachOObjectFile *MachO = dyn_cast<const MachOObjectFile>(Obj);
   for (const SymbolRef &Symbol : Obj->symbols()) {
     uint64_t Address = unwrapOrError(Symbol.getAddress(), FileName);
 
@@ -1148,6 +1149,18 @@ static void disassembleObject(const Target *TheTarget, const ObjectFile *Obj,
         continue;
     }
 
+    // Don't ask a Mach-O STAB symbol for its section unless you know that 
+    // STAB symbol's section field refers to a valid section index. Otherwise
+    // the symbol may error trying to load a section that does not exist.
+    if (MachO) {
+      DataRefImpl SymDRI = Symbol.getRawDataRefImpl();
+      uint8_t NType = (MachO->is64Bit() ?
+                       MachO->getSymbol64TableEntry(SymDRI).n_type:
+                       MachO->getSymbolTableEntry(SymDRI).n_type);
+      if (NType & MachO::N_STAB)
+        continue;
+    }
+
     section_iterator SecI = unwrapOrError(Symbol.getSection(), FileName);
     if (SecI != Obj->section_end())
       AllSymbols[*SecI].emplace_back(Address, Name, SymbolType);
@@ -1244,7 +1257,7 @@ static void disassembleObject(const Target *TheTarget, const ObjectFile *Obj,
     }
 
     StringRef SegmentName = "";
-    if (const MachOObjectFile *MachO = dyn_cast<const MachOObjectFile>(Obj)) {
+    if (MachO) {
       DataRefImpl DR = Section.getRawDataRefImpl();
       SegmentName = MachO->getSectionFinalSegmentName(DR);
     }
@@ -1804,6 +1817,7 @@ void printSymbolTable(const ObjectFile *O, StringRef ArchiveName,
   }
 
   const StringRef FileName = O->getFileName();
+  const MachOObjectFile *MachO = dyn_cast<const MachOObjectFile>(O);
   for (auto I = O->symbol_begin(), E = O->symbol_end(); I != E; ++I) {
     const SymbolRef &Symbol = *I;
     uint64_t Address = unwrapOrError(Symbol.getAddress(), FileName, ArchiveName,
@@ -1813,8 +1827,23 @@ void printSymbolTable(const ObjectFile *O, StringRef ArchiveName,
     SymbolRef::Type Type = unwrapOrError(Symbol.getType(), FileName,
                                          ArchiveName, ArchitectureName);
     uint32_t Flags = Symbol.getFlags();
-    section_iterator Section = unwrapOrError(Symbol.getSection(), FileName,
+
+    // Don't ask a Mach-O STAB symbol for its section unless you know that 
+    // STAB symbol's section field refers to a valid section index. Otherwise
+    // the symbol may error trying to load a section that does not exist.
+    bool isSTAB = false;
+    if (MachO) {
+      DataRefImpl SymDRI = Symbol.getRawDataRefImpl();
+      uint8_t NType = (MachO->is64Bit() ?
+                       MachO->getSymbol64TableEntry(SymDRI).n_type:
+                       MachO->getSymbolTableEntry(SymDRI).n_type);
+      if (NType & MachO::N_STAB)
+        isSTAB = true;
+    }
+    section_iterator Section = isSTAB ? O->section_end() :
+                               unwrapOrError(Symbol.getSection(), FileName,
                                              ArchiveName, ArchitectureName);
+
     StringRef Name;
     if (Type == SymbolRef::ST_Debug && Section != O->section_end()) {
       if (Expected<StringRef> NameOrErr = Section->getName())