[DebugInfo] Refactored macro related generation, added a test case for macinfo.dwo emission.

[SouraVX]

Summary

This patch refactors some part of D70705, based on review suggestion and also adds a test case for emission of macinfo.dwo section.

[merge_guards_bot]

Build result: pass - 60453 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt, CMakeCache.txt

[dblaikie]

Sorry, I'm a bit confused - this sort of thing should be included in the original review.

But I think this whole direction (supporting macinfo.dwo) should not be done, since macinfo is not in DWARFv5 - so to reach DWARFv5 support, you/we/someone should first fix the existing bug (which is that debug_macro should be emitted in DWARFv5, but currently LLVM produces debug_macinfo instead). and then that correct support can be expanded to support Split DWARFv5.

[SouraVX]

In D71008#1769278, @dblaikie wrote:

Sorry, I'm a bit confused - this sort of thing should be included in the original review.

Ah, that was showing accepted. Even you reopened it[I noticed]. So that's why I created a new one. I think, I mentioned my intentions in previous revision also ??

But I think this whole direction (supporting macinfo.dwo) should not be done, since macinfo is not in DWARFv5 - so to reach DWARFv5 support, you/we/someone should first fix the existing bug (which is that debug_macro should be emitted in DWARFv5, but currently LLVM produces debug_macinfo instead). and then that correct support can be expanded to support Split DWARFv5.

I'm working on DWARFv5 debug_macro also, holding up till this also gets it.
But Main motivation behind this are -- {Some mentioned in previous revision}

• One of the major factor is GDB and how it handles stuff, if you try using clang 9.0 or any older version. behavior of macro is like this -g -gsplit-dwarf -fdebug-macro{-g defaulting to DWARF version 4}. debug_macinfo got populated{even when split-dwarf is specified}. Now load that binary in GDB, try expanding macros, it fails. that renders all the heavy lifting compiler did to produce debug_macinfo section is finally useless. Please note here that this behavior is contrary to GCC, GCC produces debug_macinfo.dwo when -gsplit-dwarf is specified. GDB also seems to be happy, macro expansion works fine. Seems like for split-dwarf mode, GDB looks for macinfo.dwo in DWO file. Just a side note GCC-9.2, I think trunk also, only generate macinfo/macinfo.dwo only when -gstrict-dwarf is also specified along with the usual ones, otherwise GCC by defaults generates macro/macro.dwo.
• For a C++ project, the macro information, can be huge. moving this to separate file{DWO} can be a huge win. I think saves a bit of linking time also ?
• Nevertheless, considering the present situation{assuming you/someone else don't want macinfo.dwo possibly revert it}. For binaries compiled with -g -gsplit-dwarf compiler will still produce debug_macinfo, that GDB cannot use.

I think, these seems points reasonable enough. I also want to here your opinion/motivation as to why should not be doing this.

[dblaikie]

In D71008#1769501, @SouraVX wrote:

In D71008#1769278, @dblaikie wrote:

Sorry, I'm a bit confused - this sort of thing should be included in the original review.

Ah, that was showing accepted. Even you reopened it[I noticed]. So that's why I created a new one. I think, I mentioned my intentions in previous revision also ??

Ah, I saw it had been accepted, but didn't realize it had already been /committed/. Makes sense - sorry for the confusion.

But I think this whole direction (supporting macinfo.dwo) should not be done, since macinfo is not in DWARFv5 - so to reach DWARFv5 support, you/we/someone should first fix the existing bug (which is that debug_macro should be emitted in DWARFv5, but currently LLVM produces debug_macinfo instead). and then that correct support can be expanded to support Split DWARFv5.

I'm working on DWARFv5 debug_macro also, holding up till this also gets it.
But Main motivation behind this are -- {Some mentioned in previous revision}

• One of the major factor is GDB and how it handles stuff, if you try using clang 9.0 or any older version. behavior of macro is like this -g -gsplit-dwarf -fdebug-macro{-g defaulting to DWARF version 4}. debug_macinfo got populated{even when split-dwarf is specified}. Now load that binary in GDB, try expanding macros, it fails. that renders all the heavy lifting compiler did to produce debug_macinfo section is finally useless. Please note here that this behavior is contrary to GCC, GCC produces debug_macinfo.dwo when -gsplit-dwarf is specified. GDB also seems to be happy, macro expansion works fine. Seems like for split-dwarf mode, GDB looks for macinfo.dwo in DWO file. Just a side note GCC-9.2, I think trunk also, only generate macinfo/macinfo.dwo only when -gstrict-dwarf is also specified along with the usual ones, otherwise GCC by defaults generates macro/macro.dwo.
• For a C++ project, the macro information, can be huge. moving this to separate file{DWO} can be a huge win. I think saves a bit of linking time also ?
• Nevertheless, considering the present situation{assuming you/someone else don't want macinfo.dwo possibly revert it}. For binaries compiled with -g -gsplit-dwarf compiler will still produce debug_macinfo, that GDB cannot use.

I think, these seems points reasonable enough. I also want to here your opinion/motivation as to why should not be doing this.

Ah, you're looking to fix the existing DWARFv4 support, then? It might be better if the test case were for DWARFv4, rather than DWARFv5, then? Then it's less a case of "adding buggy functionality/replacing one bug with another" (debug_macinfo.dwo in DWARFv5 being buggy/invalid) and more a case of "fixing a bug" (debug_macinfo.dwo in DWARFv4 non-standard fission extension/pre-standard implementation)

https://gcc.gnu.org/wiki/DebugFission (the proposal/design document for Split DWARF) does specify the behavior of macros under Split DWARF, so it seems fair to me to implement/fix that (though I'm mostly of the opinion that fixing pre-DWARFv5 Split DWARF support is a low priority & we should be planning to remove that support to reduce the support surface) - but since you want to work on it/the patches are written/committed, so that's OK.

So, yeah, in short: Approved, conditional on changing the test case to DWARFv4 rather than DWARFv5. Thanks!

[SouraVX]

In D71008#1772126, @dblaikie wrote:

In D71008#1769501, @SouraVX wrote:

In D71008#1769278, @dblaikie wrote:

Sorry, I'm a bit confused - this sort of thing should be included in the original review.

Ah, that was showing accepted. Even you reopened it[I noticed]. So that's why I created a new one. I think, I mentioned my intentions in previous revision also ??

Ah, I saw it had been accepted, but didn't realize it had already been /committed/. Makes sense - sorry for the confusion.

But I think this whole direction (supporting macinfo.dwo) should not be done, since macinfo is not in DWARFv5 - so to reach DWARFv5 support, you/we/someone should first fix the existing bug (which is that debug_macro should be emitted in DWARFv5, but currently LLVM produces debug_macinfo instead). and then that correct support can be expanded to support Split DWARFv5.

I'm working on DWARFv5 debug_macro also, holding up till this also gets it.
But Main motivation behind this are -- {Some mentioned in previous revision}

• One of the major factor is GDB and how it handles stuff, if you try using clang 9.0 or any older version. behavior of macro is like this -g -gsplit-dwarf -fdebug-macro{-g defaulting to DWARF version 4}. debug_macinfo got populated{even when split-dwarf is specified}. Now load that binary in GDB, try expanding macros, it fails. that renders all the heavy lifting compiler did to produce debug_macinfo section is finally useless. Please note here that this behavior is contrary to GCC, GCC produces debug_macinfo.dwo when -gsplit-dwarf is specified. GDB also seems to be happy, macro expansion works fine. Seems like for split-dwarf mode, GDB looks for macinfo.dwo in DWO file. Just a side note GCC-9.2, I think trunk also, only generate macinfo/macinfo.dwo only when -gstrict-dwarf is also specified along with the usual ones, otherwise GCC by defaults generates macro/macro.dwo.
• For a C++ project, the macro information, can be huge. moving this to separate file{DWO} can be a huge win. I think saves a bit of linking time also ?
• Nevertheless, considering the present situation{assuming you/someone else don't want macinfo.dwo possibly revert it}. For binaries compiled with -g -gsplit-dwarf compiler will still produce debug_macinfo, that GDB cannot use.

I think, these seems points reasonable enough. I also want to here your opinion/motivation as to why should not be doing this.

Ah, you're looking to fix the existing DWARFv4 support, then? It might be better if the test case were for DWARFv4, rather than DWARFv5, then? Then it's less a case of "adding buggy functionality/replacing one bug with another" (debug_macinfo.dwo in DWARFv5 being buggy/invalid) and more a case of "fixing a bug" (debug_macinfo.dwo in DWARFv4 non-standard fission extension/pre-standard implementation)

Sure, will be changing dwarf version in test case before committing, the approach I'm following for preparing patches is that. If it's not DWARFv5 related, I mark that one as [DebugInfo]. and [DWARF5] [DebugInfo] for the one related to DWARFv5.

https://gcc.gnu.org/wiki/DebugFission (the proposal/design document for Split DWARF) does specify the behavior of macros under Split DWARF, so it seems fair to me to implement/fix that (though I'm mostly of the opinion that fixing pre-DWARFv5 Split DWARF support is a low priority & we should be planning to remove that support to reduce the support surface) - but since you want to work on it/the patches are written/committed, so that's OK.

While analyzing/working on macro stuff, I stumbled upon this hole. So thought I should fix this first.

So, yeah, in short: Approved, conditional on changing the test case to DWARFv4 rather than DWARFv5. Thanks!

Thank you for this!

[SouraVX]

llvm/test/DebugInfo/X86/debug-macro-split-dwarf.ll

67 ↗

(On Diff #232095)

@dblaikie are you suggesting to change DWARF version here, or renaming the test case it self to something like debug-macinfo-split-dwarf.ll.
Please note here, that their is a bit of inconsistency, I noticed a non-split DWARFv4 test case named as debug-macro.ll .
If that's what you're suggesting; then maybe we should also rename debug-macro.ll to debug-macinfo.ll for naming consistency and future addition of test cases.

[SouraVX]

Gentle Ping! @dblaikie

[dblaikie]

llvm/test/DebugInfo/X86/debug-macro-split-dwarf.ll

67 ↗

(On Diff #232095)

Yep, this is where I was suggesting changing the version.

Yeah, at the time the debug-macro.ll test was written there was no debug_macro section, so there was no ambiguity (debug-macro obviously tested the debug_macinfo section), but now that there is, it may be somewhat confusing & could be renamed (don't do that in this patch though).

Renaming this test before it's committed seems like a good idea to go with the version change, yes.

[SouraVX]

llvm/test/DebugInfo/X86/debug-macro-split-dwarf.ll

67 ↗

(On Diff #232095)

Thank you for clarifying this.
Sure will do this, before committing,

[thakis]

This breaks tests on Windows: http://45.33.8.238/win/3783/step_11.txt

Please take a look, and if it takes a while to investigate, please revert for now.

[thakis]

Reverted in caa412090666c10f854322cdc701c1cbf8ed726e

[SouraVX]

In D71008#1778586, @thakis wrote:

This breaks tests on Windows: http://45.33.8.238/win/3783/step_11.txt

Please take a look, and if it takes a while to investigate, please revert for now.

Sorry, if that caused trouble. I'll be investigating this. BTW, this seems strange to me Harbour Build B41847: Diff 232095, here everything worked fine without a problem. Does this suggests Harbour master doesn't build and test for Windows ?

[thakis]

Sorry, if that caused trouble. I'll be investigating this. BTW, this seems strange to me Harbour Build B41847: Diff 232095, here everything worked fine without a problem. Does this suggests Harbour master doesn't build and test for Windows ?

As far as I know, that runs Linux only for now. I believe there are plans to add Windows soon-ish.

We saw the same failure on a different bot (https://ci.chromium.org/p/chromium/builders/try/win_upload_clang/777), so it should repro reliably on Windows.

Sorry for the trouble!

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h

class DwarfDebug : public DebugHandlerBase {
   void emitDebugRanges();
   void emitDebugRangesDWO();
   void emitDebugRangesImpl(const DwarfFile &Holder, MCSection *Section);
 
   /// Emit macros into a debug macinfo section.
   void emitDebugMacinfo();
   /// Emit macros into a debug macinfo.dwo section.
   void emitDebugMacinfoDWO();
+  void emitDebugMacinfoImpl(MCSection *Section);
   void emitMacro(DIMacro &M);
   void emitMacroFile(DIMacroFile &F, DwarfCompileUnit &U);
   void handleMacroNodes(DIMacroNodeArray Nodes, DwarfCompileUnit &U);
 
   /// DWARF 5 Experimental Split Dwarf Emitters
 
   /// Initialize common features of skeleton units.
   void initSkeletonUnit(const DwarfUnit &U, DIE &Die,

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

void DwarfDebug::emitMacroFile(DIMacroFile &F, DwarfCompileUnit &U) {
   assert(F.getMacinfoType() == dwarf::DW_MACINFO_start_file);
   Asm->EmitULEB128(dwarf::DW_MACINFO_start_file);
   Asm->EmitULEB128(F.getLine());
   Asm->EmitULEB128(U.getOrCreateSourceID(F.getFile()));
   handleMacroNodes(F.getElements(), U);
   Asm->EmitULEB128(dwarf::DW_MACINFO_end_file);
 }
 
-/// Emit macros into a debug macinfo section.
-void DwarfDebug::emitDebugMacinfo() {
+void DwarfDebug::emitDebugMacinfoImpl(MCSection *Section) {
   for (const auto &P : CUMap) {
     auto &TheCU = *P.second;
     auto *SkCU = TheCU.getSkeleton();
     DwarfCompileUnit &U = SkCU ? *SkCU : TheCU;
     auto *CUNode = cast<DICompileUnit>(P.first);
     DIMacroNodeArray Macros = CUNode->getMacros();
     if (Macros.empty())
       continue;
-    Asm->OutStreamer->SwitchSection(
-        Asm->getObjFileLowering().getDwarfMacinfoSection());
+    Asm->OutStreamer->SwitchSection(Section);
     Asm->OutStreamer->EmitLabel(U.getMacroLabelBegin());
     handleMacroNodes(Macros, U);
     Asm->OutStreamer->AddComment("End Of Macro List Mark");
     Asm->emitInt8(0);
   }
 }
 
-void DwarfDebug::emitDebugMacinfoDWO() {
-  for (const auto &P : CUMap) {
-    auto &TheCU = *P.second;
-    auto *SkCU = TheCU.getSkeleton();
-    DwarfCompileUnit &U = SkCU ? *SkCU : TheCU;
-    auto *CUNode = cast<DICompileUnit>(P.first);
-    DIMacroNodeArray Macros = CUNode->getMacros();
-    if (Macros.empty())
-      continue;
-    Asm->OutStreamer->SwitchSection(
-        Asm->getObjFileLowering().getDwarfMacinfoDWOSection());
-    Asm->OutStreamer->EmitLabel(U.getMacroLabelBegin());
-    handleMacroNodes(Macros, U);
-    Asm->OutStreamer->AddComment("End Of Macro List Mark");
-    Asm->emitInt8(0);
-  }
+/// Emit macros into a debug macinfo section.
+void DwarfDebug::emitDebugMacinfo() {
+  emitDebugMacinfoImpl(Asm->getObjFileLowering().getDwarfMacinfoSection());
+
+void DwarfDebug::emitDebugMacinfoDWO() {
+  emitDebugMacinfoImpl(Asm->getObjFileLowering().getDwarfMacinfoDWOSection());
 }
 
 // DWARF5 Experimental Separate Dwarf emitters.
 
 void DwarfDebug::initSkeletonUnit(const DwarfUnit &U, DIE &Die,
                                   std::unique_ptr<DwarfCompileUnit> NewU) {
 
   if (!CompilationDir.empty())

llvm/test/DebugInfo/X86/debug-macinfo-split-dwarf.ll

+; RUN: %llc_dwarf -O0 -split-dwarf-file=foo.dwo -filetype=obj < %s | llvm-dwarfdump -v - | FileCheck %s
+
+;CHECK-LABEL:.debug_macinfo.dwo contents:
+;CHECK-NEXT:  DW_MACINFO_start_file - lineno: 0 filenum: 1
+;CHECK-NEXT:    DW_MACINFO_start_file - lineno: 1 filenum: 2
+;CHECK-NEXT:      DW_MACINFO_define - lineno: 1 macro: define_1 12
+;CHECK-NEXT:    DW_MACINFO_end_file
+;CHECK-NEXT:    DW_MACINFO_start_file - lineno: 2 filenum: 3
+;CHECK-NEXT:      DW_MACINFO_define - lineno: 1 macro: define_2 14
+;CHECK-NEXT:    DW_MACINFO_end_file
+;CHECK-NEXT:  DW_MACINFO_end_file
+;CHECK-NEXT:  DW_MACINFO_define - lineno: 0 macro: __llvm__ 1
+;CHECK-NEXT:  DW_MACINFO_define - lineno: 0 macro: __clang__ 1
+;CHECK-NEXT:  DW_MACINFO_define - lineno: 0 macro: __clang_major__ 10
+;CHECK-NEXT:  DW_MACINFO_define - lineno: 0 macro: __clang_minor__ 0
+;CHECK-NEXT:  DW_MACINFO_define - lineno: 0 macro: __clang_patchlevel__ 0
+;CHECK-NEXT:  DW_MACINFO_define - lineno: 0 macro: __clang_version__ 10.0.0
+;CHECK-NEXT:  DW_MACINFO_define - lineno: 0 macro: __GNUC__ 4
+
+; ModuleID = 'debug-macro-split-dwarf.c'
+source_filename = "debug-macro-split-dwarf.c"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local void @foo(i32 %a, i32 %b) #0 !dbg !25 {
+entry:
+  %a.addr = alloca i32, align 4
+  %b.addr = alloca i32, align 4
+  store i32 %a, i32* %a.addr, align 4
+  call void @llvm.dbg.declare(metadata i32* %a.addr, metadata !29, metadata !DIExpression()), !dbg !30
+  store i32 %b, i32* %b.addr, align 4
+  call void @llvm.dbg.declare(metadata i32* %b.addr, metadata !31, metadata !DIExpression()), !dbg !32
+  store i32 26, i32* %a.addr, align 4, !dbg !33
+  ret void, !dbg !34
+}
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { noinline nounwind optnone uwtable }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!21, !22, !23}
+!llvm.ident = !{!24}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 10.0.0", isOptimized: false, runtimeVersion: 0, splitDebugFilename: "debug-macro-split-dwarf.dwo", emissionKind: FullDebug, enums: !2, macros: !3, nameTableKind: GNU)
+!1 = !DIFile(filename: "debug-macro-split-dwarf.c", directory: "/", checksumkind: CSK_MD5, checksum: "e74d0fa8f714535c1bac6da2ffbbd898")
+!2 = !{}
+!3 = !{!4, !14, !15, !16, !17, !18, !19, !20}
+!4 = !DIMacroFile(file: !1, nodes: !5)
+!5 = !{!6, !10}
+!6 = !DIMacroFile(line: 1, file: !7, nodes: !8)
+!7 = !DIFile(filename: "./1.h", directory: "/", checksumkind: CSK_MD5, checksum: "6185a3a5ae6eb7d1fd2692718f9d95e5")
+!8 = !{!9}
+!9 = !DIMacro(type: DW_MACINFO_define, line: 1, name: "define_1", value: "12")
+!10 = !DIMacroFile(line: 2, file: !11, nodes: !12)
+!11 = !DIFile(filename: "./2.h", directory: "/", checksumkind: CSK_MD5, checksum: "d48d124c86c1b50a32517884ff962f83")
+!12 = !{!13}
+!13 = !DIMacro(type: DW_MACINFO_define, line: 1, name: "define_2", value: "14")
+!14 = !DIMacro(type: DW_MACINFO_define, name: "__llvm__", value: "1")
+!15 = !DIMacro(type: DW_MACINFO_define, name: "__clang__", value: "1")
+!16 = !DIMacro(type: DW_MACINFO_define, name: "__clang_major__", value: "10")
+!17 = !DIMacro(type: DW_MACINFO_define, name: "__clang_minor__", value: "0")
+!18 = !DIMacro(type: DW_MACINFO_define, name: "__clang_patchlevel__", value: "0")
+!19 = !DIMacro(type: DW_MACINFO_define, name: "__clang_version__", value: "10.0.0")
+!20 = !DIMacro(type: DW_MACINFO_define, name: "__GNUC__", value: "4")
+!21 = !{i32 7, !"Dwarf Version", i32 4}
+!22 = !{i32 2, !"Debug Info Version", i32 3}
+!23 = !{i32 1, !"wchar_size", i32 4}
+!24 = !{!"clang version 10.0.0 "}
+!25 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 4, type: !26, scopeLine: 4, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!26 = !DISubroutineType(types: !27)
+!27 = !{null, !28, !28}
+!28 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!29 = !DILocalVariable(name: "a", arg: 1, scope: !25, file: !1, line: 4, type: !28)
+!30 = !DILocation(line: 4, column: 14, scope: !25)
+!31 = !DILocalVariable(name: "b", arg: 2, scope: !25, file: !1, line: 4, type: !28)
+!32 = !DILocation(line: 4, column: 21, scope: !25)
+!33 = !DILocation(line: 5, column: 4, scope: !25)
+!34 = !DILocation(line: 6, column: 1, scope: !25)