[lld-macho] Don't attempt to emit rebase opcodes for debug sections

[int3]

Summary

This was causing a crash as we were attempting to look up the
nonexistent parent OutputSection of the debug sections. We didn't detect
it earlier because there was no test for PIEs with debug info (PIEs
require us to emit rebases for X86_64_RELOC_UNSIGNED).

This diff filters out the debug sections while loading the ObjFiles. In
addition to fixing the above problem, it also lets us avoid doing
redundant work -- we no longer parse / apply relocations / attempt to
emit dyld opcodes for these sections that we don't emit.

Fixes llvm.org/PR48392.

[thakis]

This looks like a much nicer design, thanks!

lld/MachO/Dwarf.cpp

24

I would find a comment here helpful why these are the isDebugSection sections that get a DwarfObject. It's not obvious to me why __debug_line and __debug_ranges aren't listed here (or why we do any filtering at all). I don't know much debug information and I can believe that it's obvious to domain experts :)

lld/test/MachO/stabs.s

141

Does this have "interesting enough" debug info? The .o file rom comment 8 in http://llvm.org/pr48392 :

$ out/gn/bin/llvm-objdump -h test.o | grep debug
  4 __debug_abbrev   000000ae 0000000000000078 DATA
  5 __debug_info     00000108 0000000000000126 DATA
  6 __debug_ranges   00000040 000000000000022e DATA
  7 __debug_str      0000009c 000000000000026e DATA
 13 __debug_line     00000066 0000000000000540 DATA

This file:

$ out/gn/bin/llvm-objdump -h out/gn/obj/lld/test/MachO/Output/stabs.s.tmp/foo.o | grep debug
  1 __debug_str    00000009 0000000000000001 DATA
  2 __debug_abbrev 0000000c 000000000000000a DATA
  3 __debug_info   00000021 0000000000000016 DATA
  4 __debug_line   00000000 0000000000000037 DATA

(ie no __debug_ranges)

[thakis]

(ps: I think "Fixes PR48392" is slightly more common than "Fixes bug 48392", because with the former you can double-click that, ctrl-c, browser tab, llvm.org/<ctrl-v> to open that bug, while you have to manually type the "PR" bit. Hmm, thinking this through, I suppose "Fixes llvm.org/PR48392" would be even more convenient, but I haven't' seen that being used :) )

[tschuett]

lld/MachO/Dwarf.cpp

24

llvm does not provide a isDwarfSection function?

[tschuett]

I used -glldb. Does -ggdb create different sections?

[int3]

I think "Fixes PR48392" is slightly more common

TIL about PR shortlinks. Thanks!

I used -glldb. Does -ggdb create different sections?

It appears to omit a bunch of __apple_* sections, but __debug_ranges is present. But I as mentioned in the other comments, I don't think that section is critical to reproducing this bug.

lld/MachO/Dwarf.cpp

24

I'm not a domain expert on debug info either, so maybe @clayborg can help fill in some details / correct any misconceptions :)

It's not obvious to me why debug_line and debug_ranges aren't listed here (or why we do any filtering at all)

We filter out the debug sections because the debugger can locate the debug info via the STABS symbols that point to the corresponding source & object files. So all lld needs from the debug info is the source file path. Thus, I initialized the DwarfObject with just the sections that were required to obtain this info. __debug_line and __debug_ranges are presumably used by LLDB.

llvm does not provide a isDwarfSection function?

Nope, or at least I haven't seen any that fits this use case.

lld/test/MachO/stabs.s

141

I don't think the presence of __debug_ranges is important to reproducing this bug. The important thing is the presence of X86_64_RELOC_UNSIGNED relocations that target debug sections, which are indeed generated by this test input.

[int3]

add comment

[thakis]

Great comment, thanks!

lld/MachO/Dwarf.cpp

 
 using namespace lld;
 using namespace lld::macho;
 using namespace llvm;
 
 std::unique_ptr<DwarfObject> DwarfObject::create(ObjFile *obj) {
   auto dObj = std::make_unique<DwarfObject>();
   bool hasDwarfInfo = false;
-  for (SubsectionMap subsecMap : obj->subsections) {
-    for (auto it : subsecMap) {
-      InputSection *isec = it.second;
-      if (!(isDebugSection(isec->flags) &&
-            isec->segname == segment_names::dwarf))
-        continue;
-
-      if (isec->name == "__debug_info") {
-        dObj->infoSection.Data = toStringRef(isec->data);
-        hasDwarfInfo = true;
-        continue;
-      }
-
-      if (StringRef *s = StringSwitch<StringRef *>(isec->name)
-                             .Case("__debug_abbrev", &dObj->abbrevSection)
-                             .Case("__debug_str", &dObj->strSection)
-                             .Default(nullptr)) {
-        *s = toStringRef(isec->data);
-        hasDwarfInfo = true;
-      }
-    }
-  }
+  // LLD only needs to extract the source file path from the debug info, so we
+  // initialize DwarfObject with just the sections necessary to get that path.
+  // The debugger will locate the debug info via the object file paths that we
+  // emit in our STABS symbols, so we don't need to process & emit them
+  // ourselves.
+  for (InputSection *isec : obj->debugSections) {
+                           .Case("__debug_info", &dObj->infoSection.Data)
 
   if (hasDwarfInfo)
     return dObj;
   return nullptr;
 }

lld/MachO/InputFiles.h

 class ObjFile : public InputFile {
 public:
   ObjFile(MemoryBufferRef mb, uint32_t modTime, StringRef archiveName);
   static bool classof(const InputFile *f) { return f->kind() == ObjKind; }
 
   llvm::DWARFUnit *compileUnit = nullptr;
   const uint32_t modTime;
   ArrayRef<llvm::MachO::section_64> sectionHeaders;
+  std::vector<InputSection *> debugSections;
 
 private:
   void parseSections(ArrayRef<llvm::MachO::section_64>);
   void parseSymbols(ArrayRef<lld::structs::nlist_64> nList, const char *strtab,
                     bool subsectionsViaSymbols);
   Symbol *parseNonSectionSymbol(const structs::nlist_64 &sym, StringRef name);
   void parseRelocations(const llvm::MachO::section_64 &, SubsectionMap &);
   void parseDebugInfo();

lld/MachO/InputFiles.cpp

for (const section_64 &sec : sections) {
     isec->data = {isZeroFill(sec.flags) ? nullptr : buf + sec.offset,
                   static_cast<size_t>(sec.size)};
     if (sec.align >= 32)
       error("alignment " + std::to_string(sec.align) + " of section " +
             isec->name + " is too large");
     else
       isec->align = 1 << sec.align;
     isec->flags = sec.flags;
+
+    if (!(isDebugSection(isec->flags) &&
+          isec->segname == segment_names::dwarf)) {
       subsections.push_back({{0, isec}});
+    } else {
+      // Instead of emitting DWARF sections, we emit STABS symbols to the
+      // object files that contain them. We filter them out early to avoid
+      // parsing their relocations unnecessarily. But we must still push an
+      // empty map to ensure the indices line up for the remaining sections.
+      subsections.push_back({});
+      debugSections.push_back(isec);
+    }
   }
 }
 
 // Find the subsection corresponding to the greatest section offset that is <=
 // that of the given offset.
 //
 // offset: an offset relative to the start of the original InputSection (before
 // any subsection splitting has occurred). It will be updated to represent the
for (size_t i = 0, n = nList.size(); i < n; ++i) {
 
     if ((sym.n_type & N_TYPE) != N_SECT) {
       symbols[i] = parseNonSectionSymbol(sym, name);
       continue;
     }
 
     const section_64 &sec = sectionHeaders[sym.n_sect - 1];
     SubsectionMap &subsecMap = subsections[sym.n_sect - 1];
+    assert(!subsecMap.empty());
     uint64_t offset = sym.n_value - sec.addr;
 
     // If the input file does not use subsections-via-symbols, all symbols can
     // use the same subsection. Otherwise, we must split the sections along
     // symbol boundaries.
     if (!subsectionsViaSymbols) {
       symbols[i] = createDefined(sym, name, subsecMap[0], offset);
       continue;
if (const load_command *cmd = findCommand(hdr, LC_SYMTAB)) {
     const char *strtab = reinterpret_cast<const char *>(buf) + c->stroff;
     bool subsectionsViaSymbols = hdr->flags & MH_SUBSECTIONS_VIA_SYMBOLS;
     parseSymbols(nList, strtab, subsectionsViaSymbols);
   }
 
   // The relocations may refer to the symbols, so we parse them after we have
   // parsed all the symbols.
   for (size_t i = 0, n = subsections.size(); i < n; ++i)
+    if (!subsections[i].empty())
       parseRelocations(sectionHeaders[i], subsections[i]);
 
   parseDebugInfo();
 }
 
 void ObjFile::parseDebugInfo() {
   std::unique_ptr<DwarfObject> dObj = DwarfObject::create(this);
   if (!dObj)
     return;

lld/MachO/Writer.cpp

void Writer::createOutputSections() {
   default:
     llvm_unreachable("unhandled output file type");
   }
 
   // Then merge input sections into output sections.
   MapVector<std::pair<StringRef, StringRef>, MergedOutputSection *>
       mergedOutputSections;
   for (InputSection *isec : inputSections) {
-    // Instead of emitting DWARF sections, we emit STABS symbols to the object
-    // files that contain them.
-    if (isDebugSection(isec->flags) && isec->segname == segment_names::dwarf)
-      continue;
     MergedOutputSection *&osec =
         mergedOutputSections[{isec->segname, isec->name}];
     if (osec == nullptr)
       osec = make<MergedOutputSection>(isec->name);
     osec->mergeInput(isec);
   }
 
   for (const auto &it : mergedOutputSections) {

lld/test/MachO/stabs.s

 # CHECK-NEXT:  {{[0-9af]+}}     A _abs
 # CHECK-NEXT:  [[#FUN]]         S _fun
 # CHECK-NEXT:  [[#GLOB]]        D _global_var
 # CHECK-NEXT:  [[#ZERO]]        S _zero
 # CHECK-NEXT:  [[#FOO]]         T _foo
 # CHECK-NEXT:  {{[0-9af]+}}     T _no_debug
 # CHECK-EMPTY:
 
+## Check that we don't attempt to emit rebase opcodes for the debug sections
+## when building a PIE (since we have filtered the sections out).
+# RUN: %lld -lSystem -pie %t/test.o %t/foo.a %t/no-debug.o -o %t/test
+# RUN: llvm-objdump --macho --rebase %t/test | FileCheck %s --check-prefix=PIE
+# PIE:       Rebase table:
+# PIE-NEXT:  segment  section            address     type
+# PIE-EMPTY:
+
 #--- test.s
 
 ## Make sure we don't create STABS entries for absolute symbols.
 .globl _abs
 _abs = 0x123
 
 .section __DATA, __data
 .globl _global_var

 .subsections_via_symbols
 .section  __DWARF,__debug_line,regular,debug
 
 .section OTHER,more_text,regular,pure_instructions
 .globl _fun
 _fun:
   ret
 
 #--- foo.s
 .text
 .globl  _foo
 _foo:
 Lfunc_begin0:
   retq
 Lfunc_end0:
 
 .section  __DWARF,__debug_str,regular,debug