[objdump] Support finding --source via --dsym files

[radford]

Summary

Add support for auto-detecting or specifying dSYM files/directories to allow interleaving source with disassembly.

[MaskRay]

llvm/tools/llvm-objdump/MachODump.cpp

7395

unneeded blank line

7398

End full sentences with .

7400

https://llvm.org/docs/ProgrammersManual.html#the-twine-class

Avoid using Twine as a local variable.

[MaskRay]

llvm/test/tools/llvm-objdump/MachO/disassemble-source-dsym.test

1 ↗

(On Diff #464851)

Optional: newer test/tools tests use ## for non-RUN non-CHECK comments. That makes comments stand out and helps possibly future FileCheck's more rigid rule catching unused prefixes.

3 ↗

(On Diff #464851)

Add a comment if this intentionally tests reading from stdin?

9 ↗

(On Diff #464851)

The line isn't long enough that needs wrapping...

llvm/tools/llvm-objdump/MachODump.cpp

7457

7484

https://llvm.org/docs/CodingStandards.html#error-and-warning-messages

The convention is omit the trailing period.

[radford]

• Used ## to start comments in the .test
• Addressed line length / grammar conventions
• Removed superfluous leading namespace (llvm::)
• Only stat(2) once when auto-detecting .dSYM files

[radford]

llvm/test/tools/llvm-objdump/MachO/disassemble-source-dsym.test

3 ↗

(On Diff #464851)

I hope the explicit - is enough to make it clear this is intentional.

llvm/tools/llvm-objdump/MachODump.cpp

7400

I don't see any reference escaping here, AFAICT, but I'm new to llvm's string types. Could you offer a suggestion?

7484

These messages are referenced in unrelated tests whose changes would only add noise here. Also, changing just these would make them inconsistent with the rest of the file. :(

[radford]

Merge with another similar patch to detect an alternate location (.build-id) for a debug object:

d033ece0c985d3f89c261d030ff2ff1d9c58bbc6 [llvm-objdump] Find debug information with Build ID/debuginfod

[MaskRay]

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

1294

Perhaps move DbgObj next to Obj? They are both related to ObjectFile.

[radford]

Update argument order to put DbgObj directly after Obj since they are related and usually equal.

[thegameg]

Thanks Jim!

[radford]

Put back accidentally dropped tests.

llvm/tools/llvm-objdump/MachODump.h

 
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/CommandLine.h"
 
 namespace llvm {
 
 class Error;
 class StringRef;
+class MemoryBuffer;
 
 namespace object {
 class MachOObjectFile;
 class MachOUniversalBinary;
 class ObjectFile;
 class RelocationRef;
+class Binary;
 } // namespace object
 
 namespace opt {
 class InputArgList;
 } // namespace opt
 
 namespace objdump {
 

 extern bool UniversalHeaders;
 extern bool Verbose;
 extern bool WeakBind;
 
 Error getMachORelocationValueString(const object::MachOObjectFile *Obj,
                                     const object::RelocationRef &RelRef,
                                     llvm::SmallVectorImpl<char> &Result);
 
+const object::MachOObjectFile *
+getMachODSymObject(const object::MachOObjectFile *O, StringRef Filename,
+                   std::unique_ptr<object::Binary> &DSYMBinary,
+                   std::unique_ptr<MemoryBuffer> &DSYMBuf);
+
 void parseInputMachO(StringRef Filename);
 void parseInputMachO(object::MachOUniversalBinary *UB);
 
 void printMachOUnwindInfo(const object::MachOObjectFile *O);
 void printMachOFileHeader(const object::ObjectFile *O);
 void printMachOLoadCommands(const object::ObjectFile *O);
 
 void printExportsTrie(const object::ObjectFile *O);

llvm/tools/llvm-objdump/MachODump.cpp

while (!Comments.empty()) {
     Comments = Comments.substr(Position + 1);
   }
   FormattedOS.flush();
 
   // Tell the comment stream that the vector changed underneath it.
   CommentsToEmit.clear();
 }
 
+const MachOObjectFile *
+objdump::getMachODSymObject(const MachOObjectFile *MachOOF, StringRef Filename,
+                            std::unique_ptr<Binary> &DSYMBinary,
+                            std::unique_ptr<MemoryBuffer> &DSYMBuf) {
+  const MachOObjectFile *DbgObj = MachOOF;
+  std::string DSYMPath;
+
+  // Auto-detect w/o --dsym.
+  if (DSYMFile.empty()) {
+    sys::fs::file_status DSYMStatus;
+    Twine FilenameDSYM = Filename + ".dSYM";
+    if (!status(FilenameDSYM, DSYMStatus)) {
+      if (sys::fs::is_directory(DSYMStatus)) {
+        SmallString<1024> Path;
+        FilenameDSYM.toVector(Path);
+        sys::path::append(Path, "Contents", "Resources", "DWARF",
+                          sys::path::filename(Filename));
+        DSYMPath = std::string(Path);
+      } else if (sys::fs::is_regular_file(DSYMStatus)) {
+        DSYMPath = FilenameDSYM.str();
+      }
+    }
+  }
+
+  if (DSYMPath.empty() && !DSYMFile.empty()) {
+    // If DSYMPath is a .dSYM directory, append the Mach-O file.
+    if (sys::fs::is_directory(DSYMFile) &&
+        sys::path::extension(DSYMFile) == ".dSYM") {
+      SmallString<128> ShortName(sys::path::filename(DSYMFile));
+      sys::path::replace_extension(ShortName, "");
+      SmallString<1024> FullPath(DSYMFile);
+      sys::path::append(FullPath, "Contents", "Resources", "DWARF", ShortName);
+      DSYMPath = FullPath.str();
+    } else {
+      DSYMPath = DSYMFile;
+    }
+  }
+
+  if (!DSYMPath.empty()) {
+    // Load the file.
+    ErrorOr<std::unique_ptr<MemoryBuffer>> BufOrErr =
+        MemoryBuffer::getFileOrSTDIN(DSYMPath);
+    if (std::error_code EC = BufOrErr.getError()) {
+      reportError(errorCodeToError(EC), DSYMPath);
+      return nullptr;
+    }
+
+    // We need to keep the file alive, because we're replacing DbgObj with it.
+    DSYMBuf = std::move(BufOrErr.get());
+
+    Expected<std::unique_ptr<Binary>> BinaryOrErr =
+        createBinary(DSYMBuf.get()->getMemBufferRef());
+    if (!BinaryOrErr) {
+      reportError(BinaryOrErr.takeError(), DSYMPath);
+      return nullptr;
+    }
+
+    // We need to keep the Binary alive with the buffer
+    DSYMBinary = std::move(BinaryOrErr.get());
+    if (ObjectFile *O = dyn_cast<ObjectFile>(DSYMBinary.get())) {
+      // this is a Mach-O object file, use it
+      if (MachOObjectFile *MachDSYM = dyn_cast<MachOObjectFile>(&*O)) {
+        DbgObj = MachDSYM;
+      } else {
+        WithColor::error(errs(), "llvm-objdump")
+            << DSYMPath << " is not a Mach-O file type.\n";
+        return nullptr;
+      }
+    } else if (auto *UB = dyn_cast<MachOUniversalBinary>(DSYMBinary.get())) {
+      // this is a Universal Binary, find a Mach-O for this architecture
+      uint32_t CPUType, CPUSubType;
+      const char *ArchFlag;
+      if (MachOOF->is64Bit()) {
+        const MachO::mach_header_64 H_64 = MachOOF->getHeader64();
+        CPUType = H_64.cputype;
+        CPUSubType = H_64.cpusubtype;
+      } else {
+        const MachO::mach_header H = MachOOF->getHeader();
+        CPUType = H.cputype;
+        CPUSubType = H.cpusubtype;
+      }
+      Triple T = MachOObjectFile::getArchTriple(CPUType, CPUSubType, nullptr,
+                                                &ArchFlag);
+      Expected<std::unique_ptr<MachOObjectFile>> MachDSYM =
+          UB->getMachOObjectForArch(ArchFlag);
+      if (!MachDSYM) {
+        reportError(MachDSYM.takeError(), DSYMPath);
+        return nullptr;
+      }
+
+      // We need to keep the Binary alive with the buffer
+      DbgObj = &*MachDSYM.get();
+      DSYMBinary = std::move(*MachDSYM);
+    } else {
+      WithColor::error(errs(), "llvm-objdump")
+          << DSYMPath << " is not a Mach-O or Universal file type.\n";
+      return nullptr;
+    }
+  }
+  return DbgObj;
+}
+
 static void DisassembleMachO(StringRef Filename, MachOObjectFile *MachOOF,
                              StringRef DisSegName, StringRef DisSectName) {
   const char *McpuDefault = nullptr;
   const Target *ThumbTarget = nullptr;
   const Target *TheTarget = GetTarget(MachOOF, &McpuDefault, &ThumbTarget);
   if (!TheTarget) {
     // GetTarget prints out stuff.
     return;
#undef CHECK_THUMB_TARGET_INFO_CREATION
   }
   array_pod_sort(Dices.begin(), Dices.end());
 
   // Try to find debug info and set up the DIContext for it.
   std::unique_ptr<DIContext> diContext;
   std::unique_ptr<Binary> DSYMBinary;
   std::unique_ptr<MemoryBuffer> DSYMBuf;
   if (UseDbg) {
-    ObjectFile *DbgObj = MachOOF;
-
-    // A separate DSym file path was specified, parse it as a macho file,
+    // If separate DSym file path was specified, parse it as a macho file,
     // get the sections and supply it to the section name parsing machinery.
-    if (!DSYMFile.empty()) {
-      std::string DSYMPath(DSYMFile);
-
-      // If DSYMPath is a .dSYM directory, append the Mach-O file.
-      if (llvm::sys::fs::is_directory(DSYMPath) &&
-          llvm::sys::path::extension(DSYMPath) == ".dSYM") {
-        SmallString<128> ShortName(llvm::sys::path::filename(DSYMPath));
-        llvm::sys::path::replace_extension(ShortName, "");
-        SmallString<1024> FullPath(DSYMPath);
-        llvm::sys::path::append(FullPath, "Contents", "Resources", "DWARF",
-                                ShortName);
-        DSYMPath = std::string(FullPath.str());
-      }
-
-      // Load the file.
-      ErrorOr<std::unique_ptr<MemoryBuffer>> BufOrErr =
-          MemoryBuffer::getFileOrSTDIN(DSYMPath);
-      if (std::error_code EC = BufOrErr.getError()) {
-        reportError(errorCodeToError(EC), DSYMPath);
-        return;
-      }
-
-      // We need to keep the file alive, because we're replacing DbgObj with it.
-      DSYMBuf = std::move(BufOrErr.get());
-
-      Expected<std::unique_ptr<Binary>> BinaryOrErr =
-      createBinary(DSYMBuf.get()->getMemBufferRef());
-      if (!BinaryOrErr) {
-        reportError(BinaryOrErr.takeError(), DSYMPath);
-        return;
-      }
-
-      // We need to keep the Binary alive with the buffer
-      DSYMBinary = std::move(BinaryOrErr.get());
-      if (ObjectFile *O = dyn_cast<ObjectFile>(DSYMBinary.get())) {
-        // this is a Mach-O object file, use it
-        if (MachOObjectFile *MachDSYM = dyn_cast<MachOObjectFile>(&*O)) {
-          DbgObj = MachDSYM;
-        }
-        else {
-          WithColor::error(errs(), "llvm-objdump")
-            << DSYMPath << " is not a Mach-O file type.\n";
-          return;
-        }
-      }
-      else if (auto UB = dyn_cast<MachOUniversalBinary>(DSYMBinary.get())){
-        // this is a Universal Binary, find a Mach-O for this architecture
-        uint32_t CPUType, CPUSubType;
-        const char *ArchFlag;
-        if (MachOOF->is64Bit()) {
-          const MachO::mach_header_64 H_64 = MachOOF->getHeader64();
-          CPUType = H_64.cputype;
-          CPUSubType = H_64.cpusubtype;
-        } else {
-          const MachO::mach_header H = MachOOF->getHeader();
-          CPUType = H.cputype;
-          CPUSubType = H.cpusubtype;
-        }
-        Triple T = MachOObjectFile::getArchTriple(CPUType, CPUSubType, nullptr,
-                                                  &ArchFlag);
-        Expected<std::unique_ptr<MachOObjectFile>> MachDSYM =
-            UB->getMachOObjectForArch(ArchFlag);
-        if (!MachDSYM) {
-          reportError(MachDSYM.takeError(), DSYMPath);
-          return;
-        }
-
-        // We need to keep the Binary alive with the buffer
-        DbgObj = &*MachDSYM.get();
-        DSYMBinary = std::move(*MachDSYM);
-      }
-      else {
-        WithColor::error(errs(), "llvm-objdump")
-          << DSYMPath << " is not a Mach-O or Universal file type.\n";
-        return;
-      }
-    }
-
-    // Setup the DIContext
-    diContext = DWARFContext::create(*DbgObj);
+    if (const ObjectFile *DbgObj =
+            getMachODSymObject(MachOOF, Filename, DSYMBinary, DSYMBuf)) {
+      // Setup the DIContext
+      diContext = DWARFContext::create(*DbgObj);
   }
 
   if (FilterSections.empty())
     outs() << "(" << DisSegName << "," << DisSectName << ") section\n";
 
   for (unsigned SectIdx = 0; SectIdx != Sections.size(); SectIdx++) {
     Expected<StringRef> SecNameOrErr = Sections[SectIdx].getName();
     if (!SecNameOrErr) {

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

fetchBinaryByBuildID(const ObjectFile &Obj) {
   if (!DebugBinary) {
     reportWarning(toString(DebugBinary.takeError()), *Path);
     return None;
   }
   return std::move(*DebugBinary);
 }
 
 static void disassembleObject(const Target *TheTarget, ObjectFile &Obj,
-                              MCContext &Ctx, MCDisassembler *PrimaryDisAsm,
+                              const ObjectFile &DbgObj, MCContext &Ctx,
+                              MCDisassembler *PrimaryDisAsm,
                               MCDisassembler *SecondaryDisAsm,
                               const MCInstrAnalysis *MIA, MCInstPrinter *IP,
                               const MCSubtargetInfo *PrimarySTI,
                               const MCSubtargetInfo *SecondarySTI,
                               PrettyPrinter &PIP, SourcePrinter &SP,
                               bool InlineRelocs) {
   const MCSubtargetInfo *STI = PrimarySTI;
   MCDisassembler *DisAsm = PrimaryDisAsm;
   bool PrimaryIsThumb = false;
   if (isArmElf(Obj))
     PrimaryIsThumb = STI->checkFeatures("+thumb-mode");
 
   std::map<SectionRef, std::vector<RelocationRef>> RelocMap;
   if (InlineRelocs)
static void disassembleObject(const Target *TheTarget, ObjectFile &Obj,
   for (std::pair<const SectionRef, SectionSymbolsTy> &SecSyms : AllSymbols)
     llvm::stable_sort(SecSyms.second);
   llvm::stable_sort(AbsoluteSymbols);
 
   std::unique_ptr<DWARFContext> DICtx;
   LiveVariablePrinter LVP(*Ctx.getRegisterInfo(), *STI);
 
   if (DbgVariables != DVDisabled) {
-    DICtx = DWARFContext::create(Obj);
+    DICtx = DWARFContext::create(DbgObj);
     for (const std::unique_ptr<DWARFUnit> &CU : DICtx->compile_units())
       LVP.addCompileUnit(CU->getUnitDIE(false));
   }
 
   LLVM_DEBUG(LVP.dump());
 
   std::unordered_map<uint64_t, BBAddrMap> AddrToBBAddrMap;
   auto ReadBBAddrMap = [&](Optional<unsigned> SectionIndex = None) {
if (!IP)
     reportError(Obj->getFileName(),
                 "no instruction printer for target " + TripleName);
   IP->setPrintImmHex(PrintImmHex);
   IP->setPrintBranchImmAsAddress(true);
   IP->setSymbolizeOperands(SymbolizeOperands);
   IP->setMCInstrAnalysis(MIA.get());
 
   PrettyPrinter &PIP = selectPrettyPrinter(Triple(TripleName));
-  ObjectFile *DbgObj = Obj;
+
+  const ObjectFile *DbgObj = Obj;
   OwningBinary<Binary> DebugBinary;
-  if (!Obj->hasDebugInfo()) {
+  if (!DbgObj->hasDebugInfo()) {
     if (Optional<OwningBinary<Binary>> DebugBinaryOpt =
             fetchBinaryByBuildID(*Obj)) {
-      if (ObjectFile *FetchedObj =
-              dyn_cast<ObjectFile>(DebugBinaryOpt->getBinary())) {
+      if (auto *FetchedObj =
+              dyn_cast<const ObjectFile>(DebugBinaryOpt->getBinary())) {
         if (FetchedObj->hasDebugInfo()) {
           DebugBinary = std::move(*DebugBinaryOpt);
           DbgObj = FetchedObj;
         }
       }
     }
   }
+
+  std::unique_ptr<object::Binary> DSYMBinary;
+  std::unique_ptr<MemoryBuffer> DSYMBuf;
+  if (!DbgObj->hasDebugInfo()) {
+    if (const MachOObjectFile *MachOOF = dyn_cast<MachOObjectFile>(&*Obj)) {
+      DbgObj = objdump::getMachODSymObject(MachOOF, Obj->getFileName(),
+                                           DSYMBinary, DSYMBuf);
+      if (!DbgObj)
+        return;
+    }
+  }
+
   SourcePrinter SP(DbgObj, TheTarget->getName());
 
   for (StringRef Opt : DisassemblerOptions)
     if (!IP->applyTargetSpecificCLOption(Opt))
       reportError(Obj->getFileName(),
                   "Unrecognized disassembler option: " + Opt);
 
-  disassembleObject(TheTarget, *Obj, Ctx, DisAsm.get(), SecondaryDisAsm.get(),
-                    MIA.get(), IP.get(), STI.get(), SecondarySTI.get(), PIP, SP,
-                    InlineRelocs);
+  disassembleObject(TheTarget, *Obj, *DbgObj, Ctx, DisAsm.get(),
+                    SecondaryDisAsm.get(), MIA.get(), IP.get(), STI.get(),
+                    SecondarySTI.get(), PIP, SP, InlineRelocs);
 }
 
 void objdump::printRelocations(const ObjectFile *Obj) {
   StringRef Fmt = Obj->getBytesInAddress() > 4 ? "%016" PRIx64 :
                                                  "%08" PRIx64;
 
   // Build a mapping from relocation target to a vector of relocation
   // sections. Usually, there is an only one relocation section for