[llvm-objdump] Keep the memory buffer from the dSYM alive when using -g -dsym

[thegameg]

Summary

When using -g and -dsym, llvm-objdump opens the dSYM file and keeps the MachOObjectFile alive, while the memory buffer that the MachOObjectFile was based on gets destroyed.

This segfaults with a pretty large file. I am open to suggestions on how to test this even if it doesn't really segfault, since it seems that there are no -g nor -dsym tests for llvm-objdump.

[JDevlieghere]

LGTM. Thanks for fixing this!

Would we catch this with a small file (we already have binaries checked-in for dsymutil) and the sanitizers enabled?

[thegameg]

Added a test. Let me know if that's what you had in mind.

[JDevlieghere]

Looks good!

test/tools/llvm-objdump/X86/macho-disassembly-g-dsym.test

1 ↗

(On Diff #163381)

nit: I'd probably name this %t.dSYM, makes it a little easier to figure out the path if the test ever fails.

tools/llvm-objdump/MachODump.cpp

 #ifndef NDEBUG
   raw_ostream &DebugOut = DebugFlag ? dbgs() : nulls();
 #else
   raw_ostream &DebugOut = nulls();
 #endif
 
   std::unique_ptr<DIContext> diContext;
   ObjectFile *DbgObj = MachOOF;
+  std::unique_ptr<MemoryBuffer> DSYMBuf;
   // Try to find debug info and set up the DIContext for it.
   if (UseDbg) {
     // A 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()) {
       ErrorOr<std::unique_ptr<MemoryBuffer>> BufOrErr =
           MemoryBuffer::getFileOrSTDIN(DSYMFile);
       if (std::error_code EC = BufOrErr.getError()) {
         errs() << "llvm-objdump: " << Filename << ": " << EC.message() << '\n';
         return;
       }
       Expected<std::unique_ptr<MachOObjectFile>> DbgObjCheck =
           ObjectFile::createMachOObjectFile(BufOrErr.get()->getMemBufferRef());
 
       if (DbgObjCheck.takeError())
         report_error(MachOOF->getFileName(), DbgObjCheck.takeError());
       DbgObj = DbgObjCheck.get().release();
+      // We need to keep the file alive, because we're replacing DbgObj with it.
+      DSYMBuf = std::move(BufOrErr.get());
     }
 
     // Setup the DIContext
     diContext = DWARFContext::create(*DbgObj);
   }
 
   if (FilterSections.size() == 0)
     outs() << "(" << DisSegName << "," << DisSectName << ") section\n";