[Debug-Info][NFC] add -gstrict-dwarf support in backend

[shchenz]

Summary

we add -gstrict-dwarf support in FE in D100809, this patch is to add the same support in BE, it contains:
1: supporting passing the strict dwarf flags to TargetOptions;
2: adding a debug option -strict-dwarf to backend components, like llc.

[probinson]

You have a clang-format warning, aside from that LGTM.

[dblaikie]

Do you know if you're going to be using LTO with DBX? If so, to respect this flag, it would need to be added to LLVM IR module metadata (like the Dwarf Version and Debug Info Version fields) rather than as an MCOption. (we're pretty wishy-washy about what things we decide have to be carried through LTO (& thus go in the IR metadata) and what things we're OK saying "eh, this'll be set on MCOptions or in a -mllvm flag and you'll have to pass the flag to your link step if you want this functionality in an LTO build") - so there's no hard line here)

If you stick with the MCOption, it'll need to be plumbed into llc as a flag there, and should be tested (might be that the whole functionality of the llc flag, MCOption to lower it to, and its use in LLVM's debug info emission should go together in one patch - I'm not sure)

A general architectural issue too: I think the other patches that are using this functionality for specific features should be abandoned if the following alternative is possible: Could the code for adding attributes be modified to check the version of the attribute and omit it if the in-use version doesn't support the attribute (& strict DWARF is enabled)? (is it only attributes you're dealing with, or also tags? I would guess for tags the architectural changes might be a bit more difficult, but I think would still be worth considering such a general solution)

[shchenz]

In D100826#2702135, @dblaikie wrote:

Do you know if you're going to be using LTO with DBX? If so, to respect this flag, it would need to be added to LLVM IR module metadata (like the Dwarf Version and Debug Info Version fields) rather than as an MCOption. (we're pretty wishy-washy about what things we decide have to be carried through LTO (& thus go in the IR metadata) and what things we're OK saying "eh, this'll be set on MCOptions or in a -mllvm flag and you'll have to pass the flag to your link step if you want this functionality in an LTO build") - so there's no hard line here)

If you stick with the MCOption, it'll need to be plumbed into llc as a flag there, and should be tested (might be that the whole functionality of the llc flag, MCOption to lower it to, and its use in LLVM's debug info emission should go together in one patch - I'm not sure)

A general architectural issue too: I think the other patches that are using this functionality for specific features should be abandoned if the following alternative is possible: Could the code for adding attributes be modified to check the version of the attribute and omit it if the in-use version doesn't support the attribute (& strict DWARF is enabled)? (is it only attributes you're dealing with, or also tags? I would guess for tags the architectural changes might be a bit more difficult, but I think would still be worth considering such a general solution)

Hi David @dblaikie , thanks very much for the kind reminder about the LTO part. Yes, we need to consider the LTO part on AIX for debugging. But we can not fix this issue upstream as we did much downstream change for LTO on AIX, so I have to fix this downstream. And I will pass flag(-strict-dwarf=true) to llc, so we don't need to do any other changes in llc after this patch is committed.

And for the general architectural issue: yes, agreed. For now, most issues related to the strict dwarf are for attributes, this should be easy to control under a centralized interface, for example in addValue function of DIEValueList class.
But we also found 1 issue related to tags, as you saw in D100630, unlike attributes, we should normally not ignore the TAGs, especially when there is a replacement tag in the lower version DWARF.
We also found 1 issue related to location expression, (DW_OP_stack_value is generated at -gdwarf-3), I am still working on how to fix this.
I am thinking just adding/modifying a centralized interface for attributes for now, then patches D100440 D100629 D100628 should be abandoned. And let the tags related patch D100630 be unchanged. What do you think? @dblaikie @probinson

[dblaikie]

In D100826#2707145, @shchenz wrote:

In D100826#2702135, @dblaikie wrote:

Do you know if you're going to be using LTO with DBX? If so, to respect this flag, it would need to be added to LLVM IR module metadata (like the Dwarf Version and Debug Info Version fields) rather than as an MCOption. (we're pretty wishy-washy about what things we decide have to be carried through LTO (& thus go in the IR metadata) and what things we're OK saying "eh, this'll be set on MCOptions or in a -mllvm flag and you'll have to pass the flag to your link step if you want this functionality in an LTO build") - so there's no hard line here)

If you stick with the MCOption, it'll need to be plumbed into llc as a flag there, and should be tested (might be that the whole functionality of the llc flag, MCOption to lower it to, and its use in LLVM's debug info emission should go together in one patch - I'm not sure)

A general architectural issue too: I think the other patches that are using this functionality for specific features should be abandoned if the following alternative is possible: Could the code for adding attributes be modified to check the version of the attribute and omit it if the in-use version doesn't support the attribute (& strict DWARF is enabled)? (is it only attributes you're dealing with, or also tags? I would guess for tags the architectural changes might be a bit more difficult, but I think would still be worth considering such a general solution)

Hi David @dblaikie , thanks very much for the kind reminder about the LTO part. Yes, we need to consider the LTO part on AIX for debugging. But we can not fix this issue upstream as we did much downstream change for LTO on AIX, so I have to fix this downstream. And I will pass flag(-strict-dwarf=true) to llc, so we don't need to do any other changes in llc after this patch is committed.

Fair enough - please add a test for the -strict-dwarf flag. (If you want to split this patch from the one that adds any strict-dwarf functionality, then maybe the llc -strict-dwarf test should include something that's expected to have different behavior with CHECKs showing the incorrect behavior and a FIXME explaining what should happen - then in a follow-up patch that behavior would be implemented, the CHECKs updated and the FIXME removed)

And for the general architectural issue: yes, agreed. For now, most issues related to the strict dwarf are for attributes, this should be easy to control under a centralized interface, for example in addValue function of DIEValueList class.
But we also found 1 issue related to tags, as you saw in D100630, unlike attributes, we should normally not ignore the TAGs, especially when there is a replacement tag in the lower version DWARF.
We also found 1 issue related to location expression, (DW_OP_stack_value is generated at -gdwarf-3), I am still working on how to fix this.
I am thinking just adding/modifying a centralized interface for attributes for now, then patches D100440 D100629 D100628 should be abandoned. And let the tags related patch D100630 be unchanged. What do you think? @dblaikie @probinson

Sounds alright.

[shchenz]

1: add NFC testcases to use -gstrict-dwarf option

[dblaikie]

Sounds good - probably only need one test file, rather than two - can put a bunch of strict-dwarf related testing in that one file (& honestly, maybe don't even need to test every DWARF attribute gets the right handling if the handling is generic/applies across all the attributes)

[shchenz]

1: put test cases in single file

clang/lib/CodeGen/BackendUtil.cpp

for (const auto &Entry : HSOpts.UserEntries)
     if (!Entry.IsFramework &&
         (Entry.Group == frontend::IncludeDirGroup::Quoted ||
          Entry.Group == frontend::IncludeDirGroup::Angled ||
          Entry.Group == frontend::IncludeDirGroup::System))
       Options.MCOptions.IASSearchPaths.push_back(
           Entry.IgnoreSysRoot ? Entry.Path : HSOpts.Sysroot + Entry.Path);
   Options.MCOptions.Argv0 = CodeGenOpts.Argv0;
   Options.MCOptions.CommandLineArgs = CodeGenOpts.CommandLineArgs;
+  Options.DebugStrictDwarf = CodeGenOpts.DebugStrictDwarf;
 
   return true;
 }
 
 static Optional<GCOVOptions> getGCOVOptions(const CodeGenOptions &CodeGenOpts,
                                             const LangOptions &LangOpts) {
   if (!CodeGenOpts.EmitGcovArcs && !CodeGenOpts.EmitGcovNotes)
     return None;

llvm/include/llvm/CodeGen/CommandFlags.h

 bool getPseudoProbeForProfiling();
 
 bool getValueTrackingVariableLocations();
 
 bool getForceDwarfFrameSection();
 
 bool getXRayOmitFunctionIndex();
 
+bool getDebugStrictDwarf();
+
 /// Create this object with static storage to register codegen-related command
 /// line options.
 struct RegisterCodeGenFlags {
   RegisterCodeGenFlags();
 };
 
 llvm::BasicBlockSection getBBSectionsMode(llvm::TargetOptions &Options);
 

llvm/include/llvm/Target/TargetOptions.h

TargetOptions()
           TrapUnreachable(false), NoTrapAfterNoreturn(false), TLSSize(0),
           EmulatedTLS(false), ExplicitEmulatedTLS(false), EnableIPRA(false),
           EmitStackSizeSection(false), EnableMachineOutliner(false),
           EnableMachineFunctionSplitter(false), SupportsDefaultOutlining(false),
           EmitAddrsig(false), EmitCallSiteInfo(false),
           SupportsDebugEntryValues(false), EnableDebugEntryValues(false),
           PseudoProbeForProfiling(false), ValueTrackingVariableLocations(false),
           ForceDwarfFrameSection(false), XRayOmitFunctionIndex(false),
+          DebugStrictDwarf(false),
           FPDenormalMode(DenormalMode::IEEE, DenormalMode::IEEE) {}
 
     /// DisableFramePointerElim - This returns true if frame pointer elimination
     /// optimization should be disabled for the given machine function.
     bool DisableFramePointerElim(const MachineFunction &MF) const;
 
     /// If greater than 0, override the default value of
     /// MCAsmInfo::BinutilsVersion.
public:
     unsigned ValueTrackingVariableLocations : 1;
 
     /// Emit DWARF debug frame section.
     unsigned ForceDwarfFrameSection : 1;
 
     /// Emit XRay Function Index section
     unsigned XRayOmitFunctionIndex : 1;
 
+    /// When set to true, don't use DWARF extensions in later DWARF versions.
+    /// By default, it is set to false.
+    unsigned DebugStrictDwarf : 1;
+
     /// Stack protector guard offset to use.
     unsigned StackProtectorGuardOffset = -1U;
 
     /// Stack protector guard mode to use, e.g. tls, global.
     StackProtectorGuards StackProtectorGuard =
                                          StackProtectorGuards::None;
 
     /// Stack protector guard reg to use, e.g. usually fs or gs in X86.

llvm/lib/CodeGen/CommandFlags.cpp

 CGOPT(bool, EnableAddrsig)
 CGOPT(bool, EmitCallSiteInfo)
 CGOPT(bool, EnableMachineFunctionSplitter)
 CGOPT(bool, EnableDebugEntryValues)
 CGOPT(bool, PseudoProbeForProfiling)
 CGOPT(bool, ValueTrackingVariableLocations)
 CGOPT(bool, ForceDwarfFrameSection)
 CGOPT(bool, XRayOmitFunctionIndex)
+CGOPT(bool, DebugStrictDwarf)
 
 codegen::RegisterCodeGenFlags::RegisterCodeGenFlags() {
 #define CGBINDOPT(NAME)                                                        \
   do {                                                                         \
     NAME##View = std::addressof(NAME);                                         \
   } while (0)
 
   static cl::opt<std::string> MArch(
static cl::opt<bool> ForceDwarfFrameSection(
       cl::desc("Always emit a debug frame section."), cl::init(false));
   CGBINDOPT(ForceDwarfFrameSection);
 
   static cl::opt<bool> XRayOmitFunctionIndex(
       "no-xray-index", cl::desc("Don't emit xray_fn_idx section"),
       cl::init(false));
   CGBINDOPT(XRayOmitFunctionIndex);
 
+  static cl::opt<bool> DebugStrictDwarf(
+      "strict-dwarf", cl::desc("use strict dwarf"), cl::init(false));
+  CGBINDOPT(DebugStrictDwarf);
+
 #undef CGBINDOPT
 
   mc::RegisterMCTargetOptionsFlags();
 }
 
 llvm::BasicBlockSection
 codegen::getBBSectionsMode(llvm::TargetOptions &Options) {
   if (getBBSections() == "all")
codegen::InitTargetOptionsFromCodeGenFlags(const Triple &TheTriple) {
   Options.EnableMachineFunctionSplitter = getEnableMachineFunctionSplitter();
   Options.EmitAddrsig = getEnableAddrsig();
   Options.EmitCallSiteInfo = getEmitCallSiteInfo();
   Options.EnableDebugEntryValues = getEnableDebugEntryValues();
   Options.PseudoProbeForProfiling = getPseudoProbeForProfiling();
   Options.ValueTrackingVariableLocations = getValueTrackingVariableLocations();
   Options.ForceDwarfFrameSection = getForceDwarfFrameSection();
   Options.XRayOmitFunctionIndex = getXRayOmitFunctionIndex();
+  Options.DebugStrictDwarf = getDebugStrictDwarf();
 
   Options.MCOptions = mc::InitMCTargetOptionsFromFlags();
 
   Options.ThreadModel = getThreadModel();
   Options.EABIVersion = getEABIVersion();
   Options.DebuggerTuning = getDebuggerTuningOpt();
 
   return Options;

llvm/test/DebugInfo/PowerPC/strict-dwarf.ll

+; RUN: llc -filetype=obj -mtriple=powerpc64le-unknown-linux-gnu < %s | \
+; RUN:   llvm-dwarfdump -debug-info - | FileCheck %s
+; RUN: llc -filetype=obj -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -strict-dwarf=true < %s | llvm-dwarfdump -debug-info - | \
+; RUN:   FileCheck %s
+
+; FIXME: when -strict-dwarf=true is specified, we should check "STRICT" to tell
+; that with DWARF 4, we should not generate DWARF 5 attribute DW_AT_noreturn and
+; DW_AT_alignment.
+
+; CHECK: DW_AT_alignment
+; CHECK: DW_AT_noreturn
+; STRICT-NOT: DW_AT_noreturn
+; STRICT-NOT: DW_AT_alignment
+
+@_ZL3var = internal global i32 0, align 16, !dbg !0
+
+; Function Attrs: noinline noreturn optnone uwtable mustprogress
+define dso_local void @_Z1fv() #0 !dbg !12 {
+entry:
+  call void @_Z4exitv(), !dbg !15
+  ret void, !dbg !16
+}
+
+declare void @_Z4exitv()
+
+; Function Attrs: noinline nounwind optnone uwtable mustprogress
+define dso_local signext i32 @_Z3foov() !dbg !17 {
+entry:
+  %0 = load i32, i32* @_ZL3var, align 16, !dbg !21
+  ret i32 %0, !dbg !22
+}
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!7, !8, !9, !10}
+!llvm.ident = !{!11}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "var", linkageName: "_ZL3var", scope: !2, file: !3, line: 2, type: !6, isLocal: true, isDefinition: true, align: 128)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "clang version 13.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "t.cpp", directory: "./")
+!4 = !{}
+!5 = !{!0}
+!6 = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
+!7 = !{i32 7, !"Dwarf Version", i32 4}
+!8 = !{i32 2, !"Debug Info Version", i32 3}
+!9 = !{i32 1, !"wchar_size", i32 4}
+!10 = !{i32 7, !"uwtable", i32 1}
+!11 = !{!"clang version 13.0.0"}
+!12 = distinct !DISubprogram(name: "f", linkageName: "_Z1fv", scope: !3, file: !3, line: 4, type: !13, scopeLine: 4, flags: DIFlagPrototyped | DIFlagNoReturn, spFlags: DISPFlagDefinition, unit: !2, retainedNodes: !4)
+!13 = !DISubroutineType(types: !14)
+!14 = !{null}
+!15 = !DILocation(line: 5, column: 1, scope: !12)
+!16 = !DILocation(line: 7, column: 1, scope: !12)
+!17 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !3, file: !3, line: 10, type: !18, scopeLine: 11, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !2, retainedNodes: !4)
+!18 = !DISubroutineType(types: !19)
+!19 = !{!20}
+!20 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!21 = !DILocation(line: 12, column: 18, scope: !17)
+!22 = !DILocation(line: 12, column: 11, scope: !17)