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

[mtrent]

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.

[MaskRay]

llvm/tools/llvm-objdump/llvm-objdump.cpp

1833

Move the variable inside if.

Can you lift MachO below outside the loop?

1833

Please ignore Move the variable inside if.

[mtrent]

llvm/tools/llvm-objdump/llvm-objdump.cpp

1833

Ignoring the variable, is it still desirable to make the MachO available in to the entire function?

[mtrent]

llvm/tools/llvm-objdump/llvm-objdump.cpp

1833

Oh I see. Ill have a look.

[mtrent]

Move MachO casts out of work loops.

[MaskRay]

llvm/tools/llvm-objdump/llvm-objdump.cpp

1152

I think that this comment could be clearer.

Does this comment suggest that the reader should skip this piece of code if they don't know what a STAB section is? (I don't know what a STAB section is.)

[mtrent]

llvm/tools/llvm-objdump/llvm-objdump.cpp

1152

“Don’t ask a Mach-O STAB symbol for its section unless you know the STAB symbol section field refers to a section index. Otherwise SymbolRef may error trying to load a section that does not exist.” ?

[mtrent]

Attempt to clarify STAB symbol restriction comment.

[thegameg]

Thanks for the clarification in the comment. This LGTM!

llvm/test/tools/llvm-objdump/macho-stabs.test

+# RUN: llvm-objdump --syms %p/Inputs/macho-stabs-x86_64
+# RUN: llvm-objdump -D %p/Inputs/macho-stabs-x86_64

llvm/tools/llvm-objdump/llvm-objdump.cpp

if (InlineRelocs)
     RelocMap = getRelocsMap(*Obj);
   bool Is64Bits = Obj->getBytesInAddress() > 4;
 
   // Create a mapping from virtual address to symbol name.  This is used to
   // pretty print the symbols while disassembling.
   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);
 
     StringRef Name = unwrapOrError(Symbol.getName(), FileName);
     if (Name.empty())
       continue;
 
     uint8_t SymbolType = ELF::STT_NOTYPE;
     if (Obj->isELF()) {
       SymbolType = getElfSymbolType(Obj, Symbol);
       if (SymbolType == ELF::STT_SECTION)
         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);
     else
       AbsoluteSymbols.emplace_back(Address, Name, SymbolType);
   }
   if (AllSymbols.empty() && Obj->isELF())
     addDynamicElfSymbols(Obj, AllSymbols);
if (Obj->isELF() && Obj->getArch() == Triple::amdgcn) {
         std::unique_ptr<MCSymbolizer> Symbolizer(
           TheTarget->createMCSymbolizer(
             TripleName, nullptr, nullptr, &Symbols, &Ctx, std::move(RelInfo)));
         DisAsm->setSymbolizer(std::move(Symbolizer));
       }
     }
 
     StringRef SegmentName = "";
-    if (const MachOObjectFile *MachO = dyn_cast<const MachOObjectFile>(Obj)) {
+    if (MachO) {
       DataRefImpl DR = Section.getRawDataRefImpl();
       SegmentName = MachO->getSectionFinalSegmentName(DR);
     }
 
     StringRef SectionName = unwrapOrError(Section.getName(), Obj->getFileName());
     // If the section has no symbol at the start, just insert a dummy one.
     if (Symbols.empty() || std::get<0>(Symbols[0]) != 0) {
       Symbols.insert(
void printSymbolTable(const ObjectFile *O, StringRef ArchiveName,
   outs() << "SYMBOL TABLE:\n";
 
   if (const COFFObjectFile *Coff = dyn_cast<const COFFObjectFile>(O)) {
     printCOFFSymbolTable(Coff);
     return;
   }
 
   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,
                                      ArchitectureName);
     if ((Address < StartAddress) || (Address > StopAddress))
       continue;
     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())
         Name = *NameOrErr;
       else
         consumeError(NameOrErr.takeError());
 
     } else {