[DebugInfo] Fix a fatal error originating from split-macro support

[SouraVX]

Summary

When generting DWARFv5 split dwarf macro info, debug_line_offset in
macro section header was pointing to debug_line_start symbol in .debug_line
section in primary binary resulting in relocation and subsequent
fatal error:

fatal error: error in backend: A dwo section may not contain relocations

In this case this should point to debug_line_start symbol in debug_line.dwo
section. LLVM does not emit debug_line.dwo section except for type
units.
As of now skip this field emission, since it is optional.

Link to previous discussion:
http://lists.llvm.org/pipermail/llvm-dev/2020-February/138861.html

[SouraVX]

Added corrected test case.

[dblaikie]

I don't think this field (debug_line_offset) is optional if the debug_macro.dwo section contains DW_MACRO_start_file: "If a DW_MACRO_start_file entry is present, the header contains a reference to the .debug_line section of the compilation."

I'm not entirely clear on how that's meant to work for Split DWARF - what does GCC do? Seems like this might have to, unfortunately duplicate more file names (same design limitation as DWARF type units, though) into .debug_line.dwo.

[SouraVX]

I don't think this field (debug_line_offset) is optional if the debug_macro.dwo section contains DW_MACRO_start_file: "If a DW_MACRO_start_file entry is present, the header contains a reference to the .debug_line section of the compilation."

Yes, but spec seems a bit ambiguous in stating this. "If zero, that field is omitted. Pg. 166, line 14-16", Not explicitly stating the fact/case where it should be always present.
And spec also doesn't say anything about explicit presence of debug_line.dwo when debug_macro.dwo is present, although it seems it should be present.

I'm not entirely clear on how that's meant to work for Split DWARF - what does GCC do? Seems like this might have to, unfortunately duplicate more file names (same design limitation as DWARF type units, though) into .debug_line.dwo.

GCC(trunk) in non-split mode uses *_strp and for split case uses some forms(Not available in v5).
objdump(binutils 2.32) reports:

objdump: Error:  Unknown macro opcode 22 seen
objdump: Error:  Unknown macro opcode da seen

Anyways in ASM, it's clear that debug_line_offset referring to debug_line_start in debug_line.dwo. Dispensing relocation with a difference of labels(Start & End labels of debug_line.dwo section)

        .section        .debug_macro.dwo,"e",@progbits
.Ldebug_macro0:
        .value  0x5
        .byte   0x2
        .long   .Lskeleton_debug_line0
        .byte   0x7
...
       .section        .debug_line.dwo,"e",@progbits
.Lskeleton_debug_line0:
        .long   .LELT0-.LSLT0

I think if we want this then debug_line.dwo should be enabled first(Not always, but for type units and for macro info).

[probinson]

The correct fix is to emit the .debug_line.dwo section (which repeats the directory/file tables from the .debug_line section in the .o file) and have .debug_macro.dwo point to it. "If a DW_MACRO_start_file entry is present, the header contains a reference to the .debug_line section of the compilation." (p.169) Yes, it would be clearer to mention both normal and .dwo cases, and I will file a clarification issue about that, but I think the intent is clear.

The description of the flag on p.166 is merely describing the relationship of the flag and the existence of the field. It doesn't mean you can arbitrarily choose to omit the field.

[dblaikie]

In D80945#2068934, @probinson wrote:

The correct fix is to emit the .debug_line.dwo section (which repeats the directory/file tables from the .debug_line section in the .o file) and have .debug_macro.dwo point to it. "If a DW_MACRO_start_file entry is present, the header contains a reference to the .debug_line section of the compilation." (p.169) Yes, it would be clearer to mention both normal and .dwo cases, and I will file a clarification issue about that, but I think the intent is clear.

The description of the flag on p.166 is merely describing the relationship of the flag and the existence of the field. It doesn't mean you can arbitrarily choose to omit the field.

+1 to all that - thanks Paul!

[SouraVX]

I've raised another review D81476 for addressing this. Will revise this afterwards.
Thank You.

[SouraVX]

Fixed By: https://reviews.llvm.org/D84278

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

 
 /// Emit the header of a DWARF 5 macro section.
 static void emitMacroHeader(AsmPrinter *Asm, const DwarfDebug &DD,
                             const DwarfCompileUnit &CU) {
   enum HeaderFlagMask {
 #define HANDLE_MACRO_FLAG(ID, NAME) MACRO_FLAG_##NAME = ID,
 #include "llvm/BinaryFormat/Dwarf.def"
   };
+  // Flags `0` in macro section header implies DWARF32.
   uint8_t Flags = 0;
   Asm->OutStreamer->AddComment("Macro information version");
   Asm->emitInt16(5);
-  // We are setting Offset and line offset flags unconditionally here,
-  // since we're only supporting DWARF32 and line offset should be mostly
-  // present.
+  // We are setting DWARF32 and line offset flags unconditionally here
+  // for non-split case, since we're only supporting DWARF32 and line
+  // offset should be mostly present.
   // FIXME: Add support for DWARF64.
+  if (!DD.useSplitDwarf()) {
     Flags |= MACRO_FLAG_DEBUG_LINE_OFFSET;
     Asm->OutStreamer->AddComment("Flags: 32 bit, debug_line_offset present");
     Asm->emitInt8(Flags);
     Asm->OutStreamer->AddComment("debug_line_offset");
     Asm->OutStreamer->emitSymbolValue(CU.getLineTableStartSym(), /*Size=*/4);
+  } else {
+    // FIXME: Add support for debug_ling_offset in .debug_macro.dwo.
+    Asm->OutStreamer->AddComment("Flags: 32 bit");
+    Asm->emitInt8(Flags);
+  }
 }
 
 void DwarfDebug::handleMacroNodes(DIMacroNodeArray Nodes, DwarfCompileUnit &U) {
   for (auto *MN : Nodes) {
     if (auto *M = dyn_cast<DIMacro>(MN))
       emitMacro(*M);
     else if (auto *F = dyn_cast<DIMacroFile>(MN))
       emitMacroFile(*F, U);