[MC] Generate a compilation unit in the 64-bit DWARF format [3/7]

[ikudrin]

Summary

The patch enables producing DWARF64 compilation units and fixes generating references to .debug_abbrev and .debug_line sections. A similar change for .debug_ranges/.debug_rnglists will be added in a forthcoming patch.

[jhenderson]

llvm/lib/MC/MCDwarf.cpp

983

Probably should be a full stop instead of a comma on this line, though I'm not 100% sure I entirely follow the comment as a whole.

985–987

There are two modified cases here, but only one test update for .debug_abbrev as far as I can see. Does that indicate some missing coverage? Similar comment re. DW_AT_stmt_list below.

997–998

Again, this comment probably needs a bit of improvement.

llvm/test/MC/ELF/gen-dwarf64.s

36–38

Why has this been added?

[MaskRay]

llvm/test/MC/ELF/gen-dwarf64.s

9

REL-NEXT:

[probinson]

This patch is for generating DWARF-64 format for raw assembler source. Why is that an interesting case? I could see wanting to emit a .debug_info section for higher level languages in DWARF-64 format, but that is not what this patch does.

[dblaikie]

In D81145#2074692, @probinson wrote:

This patch is for generating DWARF-64 format for raw assembler source. Why is that an interesting case? I could see wanting to emit a .debug_info section for higher level languages in DWARF-64 format, but that is not what this patch does.

Might be best to keep the fundamental design direction/question here in one thread ( https://reviews.llvm.org/D81144 ) rather than in all of them. Patches are in a series, so if that one ends up going another direction I don't think there's a risk these later ones will be reviewed/approved/committed in spite of that fundamental question.

[probinson]

In D81145#2074793, @dblaikie wrote:

In D81145#2074692, @probinson wrote:

This patch is for generating DWARF-64 format for raw assembler source. Why is that an interesting case? I could see wanting to emit a .debug_info section for higher level languages in DWARF-64 format, but that is not what this patch does.

Might be best to keep the fundamental design direction/question here in one thread ( https://reviews.llvm.org/D81144 ) rather than in all of them. Patches are in a series, so if that one ends up going another direction I don't think there's a risk these later ones will be reviewed/approved/committed in spite of that fundamental question.

Okay, but is the idea that this patch handles both asm source and the normal case? Because I don't think it does.

[dblaikie]

In D81145#2075135, @probinson wrote:

In D81145#2074793, @dblaikie wrote:

In D81145#2074692, @probinson wrote:

This patch is for generating DWARF-64 format for raw assembler source. Why is that an interesting case? I could see wanting to emit a .debug_info section for higher level languages in DWARF-64 format, but that is not what this patch does.

Might be best to keep the fundamental design direction/question here in one thread ( https://reviews.llvm.org/D81144 ) rather than in all of them. Patches are in a series, so if that one ends up going another direction I don't think there's a risk these later ones will be reviewed/approved/committed in spite of that fundamental question.

Okay, but is the idea that this patch handles both asm source and the normal case? Because I don't think it does.

Nope, the LLVM IR support should be in a separate patch - that'll be in llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp - llvm::DwarfUnit::emitCommonHeader, for instance. And other support through lib/CodeGen/AsmPrinter for the form encoding length, etc.

[ikudrin]

• Fixed comments.
• Added a check for a DIE for "foo" in the test to validate that the CU is not skewed.
• Added a test targeting Mach-O, so that all changed code paths are covered.

[ikudrin]

llvm/lib/MC/MCDwarf.cpp

983

I've added a full stop. For my taste, it sounds OK, meaning that we know that we generate only one block of data, which is at the start of the section, so the real offset to that data is zero. Probably, Mach-O does not require a relocation for that case, so the raw value can be used.

985–987

The code paths without symbols are for the Mach-O target. I have added a new test, but have to admit that I do not know much about that platform.

llvm/test/MC/ELF/gen-dwarf64.s

9

Well, the test does not intend to check the absence of other relocations, just that the expected relocations for particular sections are in place. I think I'd better remove the check for Relocations [.

36–38

These lines trigger generating .debug_info and some related debug info sections.

[dblaikie]

llvm/lib/MC/MCDwarf.cpp

983

Yeah, it's still a bit confusing to me too. The ambiguities in the second sentence I think are what I'm tripping over - it is unclear to me what "it" and "this" are in that context.

But maybe out of scope for this patch - if someone comes up with a good phrasing & wants to replace it - just commit that directly/separately.

[jhenderson]

LGTM.

llvm/lib/MC/MCDwarf.cpp

983

I'll leave this up to @ikudrin whether he includes it or not. My repo is a little bit of a mess at the moment, so can't easily add it myself. If someone does want to improve the wording, I'd suggest "Since the abbrevs are at the start of that section, the offset is zero."

997–998

Approximately same suggestion here, if updating the comment again: "The line table is at the start of that section, so the offset is zero."

[ikudrin]

• Rearranged branches in if (AbbrevSectionSymbol) to unify it with other similar cases.
• Updated comments about emitting zero values.

llvm/lib/MC/MCDwarf.cpp

983

Thanks for the suggestion! I've used your wording but changed "that section" to "the section" because it is now a separate comment. How does it look now?

[jhenderson]

llvm/lib/MC/MCDwarf.cpp

983

Works for me, thanks.

llvm/lib/MC/MCDwarf.cpp

static void EmitGenDwarfInfo(MCStreamer *MCOS,
   // Create a symbol at the start and end of this section used in here for the
   // expression to calculate the length in the header.
   MCSymbol *InfoStart = context.createTempSymbol();
   MCOS->emitLabel(InfoStart);
   MCSymbol *InfoEnd = context.createTempSymbol();
 
   // First part: the header.
 
-  // The 4 byte total length of the information for this compilation unit, not
-  // including these 4 bytes.
+  unsigned UnitLengthBytes =
+      dwarf::getUnitLengthFieldByteSize(context.getDwarfFormat());
+  unsigned OffsetSize = dwarf::getDwarfOffsetByteSize(context.getDwarfFormat());
+
+  if (context.getDwarfFormat() == dwarf::DWARF64)
+    // Emit DWARF64 mark.
+    MCOS->emitInt32(dwarf::DW_LENGTH_DWARF64);
+
+  // The 4 (8 for DWARF64) byte total length of the information for this
+  // compilation unit, not including the unit length field itself.
   const MCExpr *Length =
-      MakeStartMinusEndExpr(context, *InfoStart, *InfoEnd, 4);
-  emitAbsValue(*MCOS, Length, 4);
+      MakeStartMinusEndExpr(context, *InfoStart, *InfoEnd, UnitLengthBytes);
+  emitAbsValue(*MCOS, Length, OffsetSize);
 
   // The 2 byte DWARF version.
   MCOS->emitInt16(context.getDwarfVersion());
 
   // The DWARF v5 header has unit type, address size, abbrev offset.
   // Earlier versions have abbrev offset, address size.
   const MCAsmInfo &AsmInfo = *context.getAsmInfo();
   int AddrSize = AsmInfo.getCodePointerSize();
   if (context.getDwarfVersion() >= 5) {
     MCOS->emitInt8(dwarf::DW_UT_compile);
     MCOS->emitInt8(AddrSize);
   }
-  // The 4 byte offset to the debug abbrevs from the start of the .debug_abbrev,
-  // it is at the start of that section so this is zero.
+  // The 4 (8 for DWARF64) byte offset to the debug abbrevs from the start of
+  // the .debug_abbrev, it is at the start of that section so this is zero.
   if (AbbrevSectionSymbol == nullptr)
-    MCOS->emitInt32(0);
+    MCOS->emitIntValue(0, OffsetSize);
   else
-    MCOS->emitSymbolValue(AbbrevSectionSymbol, 4,
+    MCOS->emitSymbolValue(AbbrevSectionSymbol, OffsetSize,
                           AsmInfo.needsDwarfSectionOffsetDirective());
   if (context.getDwarfVersion() <= 4)
     MCOS->emitInt8(AddrSize);
 
   // Second part: the compile_unit DIE.
 
   // The DW_TAG_compile_unit DIE abbrev (1).
   MCOS->emitULEB128IntValue(1);
 
-  // DW_AT_stmt_list, a 4 byte offset from the start of the .debug_line section,
-  // which is at the start of that section so this is zero.
+  // DW_AT_stmt_list, a 4 (8 for DWARF64) byte offset from the start of the
+  // .debug_line section, which is at the start of that section so this is zero.
   if (LineSectionSymbol)
-    MCOS->emitSymbolValue(LineSectionSymbol, 4,
+    MCOS->emitSymbolValue(LineSectionSymbol, OffsetSize,
                           AsmInfo.needsDwarfSectionOffsetDirective());
   else
-    MCOS->emitInt32(0);
+    MCOS->emitIntValue(0, OffsetSize);
 
   if (RangesSymbol) {
     // There are multiple sections containing code, so we must use
     // .debug_ranges/.debug_rnglists. AT_ranges, the 4 byte offset from the
     // start of the .debug_ranges/.debug_rnglists.
     MCOS->emitSymbolValue(RangesSymbol, 4);
   } else {
     // If we only have one non-empty code section, we can use the simpler

llvm/test/MC/ELF/gen-dwarf64.s

 ## This checks that llvm-mc is able to produce 64-bit debug info.
 
 # RUN: llvm-mc -g -dwarf-version 5 -dwarf64 -triple x86_64 %s -filetype=obj -o %t5.o
 # RUN: llvm-readobj -r %t5.o | FileCheck --check-prefixes=REL,REL5 %s
 # RUN: llvm-dwarfdump -v %t5.o | FileCheck --check-prefixes=DUMP,DUMP5 %s
 
 ## The references to other debug info sections are 64-bit, as required for DWARF64.
 # REL:       Relocations [
+# REL:         Section ({{[0-9]+}}) .rela.debug_info {
+# REL-NEXT:      R_X86_64_64 .debug_abbrev 0x0
+# REL-NEXT:      R_X86_64_64 .debug_line 0x0
 # REL5:        Section ({{[0-9]+}}) .rela.debug_line {
 # REL5-NEXT:     R_X86_64_64 .debug_line_str 0x0
 # REL5-NEXT:     R_X86_64_64 .debug_line_str 0x
 
+# DUMP:       .debug_info contents:
+# DUMP-NEXT:  0x00000000: Compile Unit:
+# DUMP-SAME:    format = DWARF64
+# DUMP:       DW_TAG_compile_unit [1] *
+# DUMP5-NEXT:   DW_AT_stmt_list [DW_FORM_sec_offset] (0x0000000000000000)
+
 # DUMP:       .debug_line contents:
 # DUMP-NEXT:  debug_line[0x00000000]
 # DUMP-NEXT:  Line table prologue:
 # DUMP-NEXT:      total_length:
 # DUMP-NEXT:            format: DWARF64
 # DUMP5:      include_directories[  0] = .debug_line_str[0x0000000000000000] = "[[DIR:.+]]"
 # DUMP5-NEXT: file_names[  0]:
 # DUMP5-NEXT:            name: .debug_line_str[0x00000000[[FILEOFF:[[:xdigit:]]{8}]]] = "[[FILE:.+]]"
 # DUMP5-NEXT:       dir_index: 0
 
 # DUMP5:      .debug_line_str contents:
 # DUMP5-NEXT: 0x00000000: "[[DIR]]"
 # DUMP5-NEXT: 0x[[FILEOFF]]: "[[FILE]]"
+
+    .section .foo, "ax", @progbits
+foo:
+    nop