[dsymutil] Copy eh_frame content into the dSYM companion file.

[JDevlieghere]

Summary

Copy over the __eh_frame from the binary into the dSYM. This helps kernel developers that are working with only dSYMs (i.e. no binaries) when debugging a core file. This only kicks in when the __eh_frame exists in the linked binary. Most of the time ld64 will remove the section in favor of compact unwind info. When it is emitted, it's generally small enough and should not bloat the dSYM.

rdar://69774935

[aprantl]

I think this looks reasonable!

llvm/test/tools/dsymutil/X86/eh_frame.test

3

This test has an odd combination of syntax :-)
Why C-style comments here ...

20

... and not here?

[JDevlieghere]

llvm/test/tools/dsymutil/X86/eh_frame.test

3

I had already pasted the source code when I added this line, I think it tricked my brain into writing comment markers subconsciously...

[friss]

Couple comments/questions, but this generally looks good

llvm/test/tools/dsymutil/X86/eh_frame.test

25

Can we add a check that the generated DWARF segment is well-formed?

llvm/tools/dsymutil/MachOUtils.cpp

292–298

Is reloff set to anything in the original file? I think it's supposed to be the offset to the list of relocations for the section, but eh_frame shouldn't have any relocations at this point. Anyway, I think it's better to force it to 0 as well as nreloc.

[JDevlieghere]

Address code review feedback

llvm/tools/dsymutil/MachOUtils.cpp

292–298

Yeah that was a typo (copy paste-o?)

[jasonmolenda]

llvm/test/tools/dsymutil/X86/eh_frame.test

26

These eh_frame bytes are architecture-specific (and likely codegen-specific if the compiler ever changes how many bytes are emitted). Is it possible to use dwarfdump on the .dSYM binary and search for the function name in the eh_frame dump, and assume that the instructions that it has are correct?

[jasonmolenda]

llvm/test/tools/dsymutil/X86/eh_frame.test

26

Or confirm that the eh_frame bytes from the executable are the same as the dSYM's.

[JDevlieghere]

llvm/test/tools/dsymutil/X86/eh_frame.test

26

The dsymutil tests use checked-in binaries so the test is resilient in that regard. The RUN line on 21 checks the input binary as a sanity check (in case someone decides to regenerate the binary).

[friss]

LGTM, thanks!

llvm/test/tools/dsymutil/X86/eh_frame.test

+FIXME: Replace otool with llvm-objcopy --dump-section=__TEXT,__eh_frame.
+REQUIRES : system-darwin
+
+$ cat eh_frame.cpp
+int f1()
+{
+  volatile int i;
+  return i;
+}
+
+int main(int argc, char** argv)
+{
+  return f1();
+}
+
+$ clang eh_frame.cpp -g -c -o eh_frame.o
+$ ld -no_compact_unwind eh_frame.o -o eh_frame.out
+
+RUN: dsymutil -oso-prepend-path %p/../Inputs %p/../Inputs/private/tmp/eh_frame/eh_frame.out -o %t.dSYM
+RUN: dwarfdump --verify %t.dSYM
+RUN: otool -s __TEXT __eh_frame %p/../Inputs/private/tmp/eh_frame/eh_frame.out | FileCheck %s
+RUN: otool -s __TEXT __eh_frame %t.dSYM/Contents/Resources/DWARF/eh_frame.out | FileCheck %s
+
+CHECK: 14 00 00 00 00 00 00 00 01 7a 52 00 01 78 10 01
+CHECK: 10 0c 07 08 90 01 00 00

llvm/tools/dsymutil/MachOUtils.cpp

getSection(const object::MachOObjectFile &Obj,
            const MachO::segment_command_64 &Seg,
            const object::MachOObjectFile::LoadCommandInfo &LCI, unsigned Idx) {
   return Obj.getSection64(LCI, Idx);
 }
 
 // Transfer \a Segment from \a Obj to the output file. This calls into \a Writer
 // to write these load commands directly in the output file at the current
 // position.
+//
 // The function also tries to find a hole in the address map to fit the __DWARF
 // segment of \a DwarfSegmentSize size. \a EndAddress is updated to point at the
 // highest segment address.
+//
 // When the __LINKEDIT segment is transferred, its offset and size are set resp.
 // to \a LinkeditOffset and \a LinkeditSize.
+//
+// When the eh_frame section is transferred, its offset and size are set resp.
+// to \a EHFrameOffset and \a EHFrameSize.
 template <typename SegmentTy>
 static void transferSegmentAndSections(
     const object::MachOObjectFile::LoadCommandInfo &LCI, SegmentTy Segment,
     const object::MachOObjectFile &Obj, MachObjectWriter &Writer,
-    uint64_t LinkeditOffset, uint64_t LinkeditSize, uint64_t DwarfSegmentSize,
-    uint64_t &GapForDwarf, uint64_t &EndAddress) {
+    uint64_t LinkeditOffset, uint64_t LinkeditSize, uint64_t EHFrameOffset,
+    uint64_t EHFrameSize, uint64_t DwarfSegmentSize, uint64_t &GapForDwarf,
+    uint64_t &EndAddress) {
   if (StringRef("__DWARF") == Segment.segname)
     return;
 
-  Segment.fileoff = Segment.filesize = 0;
-
-  if (StringRef("__LINKEDIT") == Segment.segname) {
+  if (StringRef("__TEXT") == Segment.segname && EHFrameSize > 0) {
+    Segment.fileoff = EHFrameOffset;
+    Segment.filesize = EHFrameSize;
+  } else if (StringRef("__LINKEDIT") == Segment.segname) {
     Segment.fileoff = LinkeditOffset;
     Segment.filesize = LinkeditSize;
     // Resize vmsize by rounding to the page size.
     Segment.vmsize = alignTo(LinkeditSize, 0x1000);
+  } else {
+    Segment.fileoff = Segment.filesize = 0;
   }
 
   // Check if the end address of the last segment and our current
   // start address leave a sufficient gap to store the __DWARF
   // segment.
   uint64_t PrevEndAddress = EndAddress;
   EndAddress = alignTo(EndAddress, 0x1000);
   if (GapForDwarf == UINT64_MAX && Segment.vmaddr > EndAddress &&
       Segment.vmaddr - EndAddress >= DwarfSegmentSize)
     GapForDwarf = EndAddress;
 
   // The segments are not necessarily sorted by their vmaddr.
   EndAddress =
       std::max<uint64_t>(PrevEndAddress, Segment.vmaddr + Segment.vmsize);
   unsigned nsects = Segment.nsects;
   if (Obj.isLittleEndian() != sys::IsLittleEndianHost)
     MachO::swapStruct(Segment);
   Writer.W.OS.write(reinterpret_cast<char *>(&Segment), sizeof(Segment));
   for (unsigned i = 0; i < nsects; ++i) {
     auto Sect = getSection(Obj, Segment, LCI, i);
+    if (StringRef("__eh_frame") == Sect.sectname) {
+      Sect.offset = EHFrameOffset;
+      Sect.reloff = Sect.nreloc = 0;
+    } else {
       Sect.offset = Sect.reloff = Sect.nreloc = 0;
+    }
     if (Obj.isLittleEndian() != sys::IsLittleEndianHost)
       MachO::swapStruct(Sect);
     Writer.W.OS.write(reinterpret_cast<char *>(&Sect), sizeof(Sect));
   }
 }
 
 // Write the __DWARF segment load command to the output file.
 static void createDwarfSegment(uint64_t VMAddr, uint64_t FileOffset,
                                uint64_t FileSize, unsigned NumSections,
bool generateDsymCompanion(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
   // If we have a valid symtab to copy, do it.
   bool ShouldEmitSymtab =
       isExecutable(InputBinary) && hasLinkEditSegment(InputBinary);
   if (ShouldEmitSymtab) {
     LoadCommandSize += sizeof(MachO::symtab_command);
     ++NumLoadCommands;
   }
 
+  // If we have a valid eh_frame to copy, do it.
+  uint64_t EHFrameSize = 0;
+  StringRef EHFrameData;
+  for (const object::SectionRef &Section : InputBinary.sections()) {
+    Expected<StringRef> NameOrErr = Section.getName();
+    if (!NameOrErr) {
+      consumeError(NameOrErr.takeError());
+      continue;
+    }
+    StringRef SectionName = *NameOrErr;
+    SectionName = SectionName.substr(SectionName.find_first_not_of("._"));
+    if (SectionName == "eh_frame") {
+      if (Expected<StringRef> ContentsOrErr = Section.getContents()) {
+        EHFrameData = *ContentsOrErr;
+        EHFrameSize = Section.getSize();
+      } else {
+        consumeError(ContentsOrErr.takeError());
+      }
+    }
+  }
+
   unsigned HeaderSize =
       Is64Bit ? sizeof(MachO::mach_header_64) : sizeof(MachO::mach_header);
   // We will copy every segment that isn't __DWARF.
   iterateOnSegments(InputBinary, [&](const MachO::segment_command_64 &Segment) {
     if (StringRef("__DWARF") == Segment.segname)
       return;
 
     ++NumLoadCommands;
bool generateDsymCompanion(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
   }
 
   assert(SymtabCmd.cmd && "No symbol table.");
   uint64_t StringStart = SymtabStart + NumSyms * NListSize;
   if (ShouldEmitSymtab)
     Writer.writeSymtabLoadCommand(SymtabStart, NumSyms, StringStart,
                                   NewStringsSize);
 
-  uint64_t DwarfSegmentStart = StringStart + NewStringsSize;
+  uint64_t EHFrameStart = StringStart + NewStringsSize;
+  EHFrameStart = alignTo(EHFrameStart, 0x1000);
+
+  uint64_t DwarfSegmentStart = EHFrameStart + EHFrameSize;
   DwarfSegmentStart = alignTo(DwarfSegmentStart, 0x1000);
 
   // Write the load commands for the segments and sections we 'import' from
   // the original binary.
   uint64_t EndAddress = 0;
   uint64_t GapForDwarf = UINT64_MAX;
   for (auto &LCI : InputBinary.load_commands()) {
     if (LCI.C.cmd == MachO::LC_SEGMENT)
-      transferSegmentAndSections(LCI, InputBinary.getSegmentLoadCommand(LCI),
-                                 InputBinary, Writer, SymtabStart,
-                                 StringStart + NewStringsSize - SymtabStart,
-                                 DwarfSegmentSize, GapForDwarf, EndAddress);
+      transferSegmentAndSections(
+          LCI, InputBinary.getSegmentLoadCommand(LCI), InputBinary, Writer,
+          SymtabStart, StringStart + NewStringsSize - SymtabStart, EHFrameStart,
+          EHFrameSize, DwarfSegmentSize, GapForDwarf, EndAddress);
     else if (LCI.C.cmd == MachO::LC_SEGMENT_64)
-      transferSegmentAndSections(LCI, InputBinary.getSegment64LoadCommand(LCI),
-                                 InputBinary, Writer, SymtabStart,
-                                 StringStart + NewStringsSize - SymtabStart,
-                                 DwarfSegmentSize, GapForDwarf, EndAddress);
+      transferSegmentAndSections(
+          LCI, InputBinary.getSegment64LoadCommand(LCI), InputBinary, Writer,
+          SymtabStart, StringStart + NewStringsSize - SymtabStart, EHFrameStart,
+          EHFrameSize, DwarfSegmentSize, GapForDwarf, EndAddress);
   }
 
   uint64_t DwarfVMAddr = alignTo(EndAddress, 0x1000);
   uint64_t DwarfVMMax = Is64Bit ? UINT64_MAX : UINT32_MAX;
   if (DwarfVMAddr + DwarfSegmentSize > DwarfVMMax ||
       DwarfVMAddr + DwarfSegmentSize < DwarfVMAddr /* Overflow */) {
     // There is no room for the __DWARF segment at the end of the
     // address space. Look through segments to find a gap.
if (ShouldEmitSymtab) {
     OutFile << '\0';
     std::vector<DwarfStringPoolEntryRef> Strings =
         NewStrings.getEntriesForEmission();
     for (auto EntryRef : Strings) {
       OutFile.write(EntryRef.getString().data(),
                     EntryRef.getString().size() + 1);
     }
   }
-
   assert(OutFile.tell() == StringStart + NewStringsSize);
 
+  // Pad till the EH frame start.
+  OutFile.write_zeros(EHFrameStart - (StringStart + NewStringsSize));
+  assert(OutFile.tell() == EHFrameStart);
+
+  // Transfer eh_frame.
+  if (EHFrameSize > 0)
+    OutFile << EHFrameData;
+  assert(OutFile.tell() == EHFrameStart + EHFrameSize);
+
   // Pad till the Dwarf segment start.
-  OutFile.write_zeros(DwarfSegmentStart - (StringStart + NewStringsSize));
+  OutFile.write_zeros(DwarfSegmentStart - (EHFrameStart + EHFrameSize));
   assert(OutFile.tell() == DwarfSegmentStart);
 
   // Emit the Dwarf sections contents.
   for (const MCSection &Sec : MCAsm) {
     if (Sec.begin() == Sec.end())
       continue;
 
     uint64_t Pos = OutFile.tell();