[Debug-Info][DBX] DW_AT_alignment should not be generated when dwarf version is not 5

[shchenz]

Summary

As discussed in D99250 , we added a new tuning debugger DBX(D99400) to add some debug info customizations for DBX on AIX.

This is part of the customizations, not generating DWARF infos not matching the DWARF version.

This is for DW_AT_alignment attribute.

Now for DBX, we only generate this attribute when DWARF version is 5 or higher.

[dblaikie]

As @probinson mentioned in https://reviews.llvm.org/D99250#2651953

Our practice has been for a "tuning" option to, effectively, expand into other options to control various bits of behavior. Basically, there should never be behavior that can be controlled _only_ via the tuning option. This is a design decision that was debated at length when we introduced tuning.

(@probinson - do we have that written down somewhere?)

So this would have to be changed to add an LLVM flag (only has to be added in DwarfDebug.cpp, most likely - it doesn't have to be a published flag for Clang, and probably shouldn't be)

But this probably gets into Paul's suggestion that it might be easier to add one strict mode flag ( https://reviews.llvm.org/D100630#2694681 ) to group all this (I wouldn't mind adding that as a single flag in DwarfDebug.cpp for now - whether or not that gets plumbed up through into a CLang flag should probably be handled separately, but would be easy enough to do) under.

[shchenz]

In D100629#2696041, @dblaikie wrote:

As @probinson mentioned in https://reviews.llvm.org/D99250#2651953

Our practice has been for a "tuning" option to, effectively, expand into other options to control various bits of behavior. Basically, there should never be behavior that can be controlled _only_ via the tuning option. This is a design decision that was debated at length when we introduced tuning.

(@probinson - do we have that written down somewhere?)

So this would have to be changed to add an LLVM flag (only has to be added in DwarfDebug.cpp, most likely - it doesn't have to be a published flag for Clang, and probably shouldn't be)

But this probably gets into Paul's suggestion that it might be easier to add one strict mode flag ( https://reviews.llvm.org/D100630#2694681 ) to group all this (I wouldn't mind adding that as a single flag in DwarfDebug.cpp for now - whether or not that gets plumbed up through into a CLang flag should probably be handled separately, but would be easy enough to do) under.

Thanks @dblaikie . Yes, I will change these patches according to Paul's suggestion. But seems we have to add this flag as a clang flag. For some cases, the mismatched DWARF infos are generated in clang FE. See D100630. BTW, do you know why there are two options -gstrict-dwarf, -gno-strict-dwarf but with no users in clang options list? Are these options used to check the DWARF infos are generated in the right DWARF version? Thanks.

[shchenz]

1: use strict dwarf flag instead of DBX debugger

[probinson]

In D100629#2696041, @dblaikie wrote:

As @probinson mentioned in https://reviews.llvm.org/D99250#2651953

Our practice has been for a "tuning" option to, effectively, expand into other options to control various bits of behavior. Basically, there should never be behavior that can be controlled _only_ via the tuning option. This is a design decision that was debated at length when we introduced tuning.

(@probinson - do we have that written down somewhere?)

Yes, although it took me a while to find, which means it's probably not in the best place. Look for the definition of DebuggerKind enum in include/llvm/Target/TargetOptions.h. Or in the corresponding doxygen-produced docs.

[shchenz]

we use a new way in D101024, so this is not needed

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

if (auto *SDMDecl = GV->getStaticDataMemberDeclaration()) {
     addSourceLine(*VariableDIE, GV);
   }
 
   if (!GV->isDefinition())
     addFlag(*VariableDIE, dwarf::DW_AT_declaration);
   else
     addGlobalName(GV->getName(), *VariableDIE, DeclContext);
 
+  // For DBX, we generate DWARF 5 attribute at DWARF version no less than 5.
+  if (!DD->tuneForDBX() || DD->getDwarfVersion() >= 5)
     if (uint32_t AlignInBytes = GV->getAlignInBytes())
       addUInt(*VariableDIE, dwarf::DW_AT_alignment, dwarf::DW_FORM_udata,
               AlignInBytes);
 
   if (MDTuple *TP = GV->getTemplateParams())
     addTemplateParams(*VariableDIE, DINodeArray(TP));
 
   // Add location.
   addLocationAttribute(VariableDIE, GV, GlobalExprs);
 
   return VariableDIE;

 }
 
 void DwarfCompileUnit::applyVariableAttributes(const DbgVariable &Var,
                                                DIE &VariableDie) {
   StringRef Name = Var.getName();
   if (!Name.empty())
     addString(VariableDie, dwarf::DW_AT_name, Name);
   const auto *DIVar = Var.getVariable();
-  if (DIVar)
+  // For DBX, we generate DWARF 5 attribute at DWARF version no less than 5.
+  if (DIVar && (!DD->tuneForDBX() || DD->getDwarfVersion() >= 5))
     if (uint32_t AlignInBytes = DIVar->getAlignInBytes())
       addUInt(VariableDie, dwarf::DW_AT_alignment, dwarf::DW_FORM_udata,
               AlignInBytes);
 
   addSourceLine(VariableDie, DIVar);
   addType(VariableDie, Var.getType());
   if (Var.isArtificial())
     addFlag(VariableDie, dwarf::DW_AT_artificial);

llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

if (Tag == dwarf::DW_TAG_enumeration_type ||
 
     // No harm in adding the runtime language to the declaration.
     unsigned RLang = CTy->getRuntimeLang();
     if (RLang)
       addUInt(Buffer, dwarf::DW_AT_APPLE_runtime_class, dwarf::DW_FORM_data1,
               RLang);
 
     // Add align info if available.
+    // For DBX, we generate DWARF 5 attribute at DWARF version no less than 5.
+    if (!DD->tuneForDBX() || DD->getDwarfVersion() >= 5)
       if (uint32_t AlignInBytes = CTy->getAlignInBytes())
         addUInt(Buffer, dwarf::DW_AT_alignment, dwarf::DW_FORM_udata,
                 AlignInBytes);
   }
 }
 
 void DwarfUnit::constructTemplateTypeParameterDIE(
     DIE &Buffer, const DITemplateTypeParameter *TP) {
   DIE &ParamDIE =
       createAndAddDIE(dwarf::DW_TAG_template_type_parameter, Buffer);
   // Add the type if it exists, it could be void and therefore no type.
if (IsBitfield) {
         addUInt(MemberDie, dwarf::DW_AT_bit_offset, None, Offset);
         OffsetInBytes = FieldOffset >> 3;
       } else {
         addUInt(MemberDie, dwarf::DW_AT_data_bit_offset, None, Offset);
       }
     } else {
       // This is not a bitfield.
       OffsetInBytes = DT->getOffsetInBits() / 8;
-      if (AlignInBytes)
+      // For DBX, we generate DWARF 5 attribute at DWARF version no less than 5.
+      if (AlignInBytes && (!DD->tuneForDBX() || DD->getDwarfVersion() >= 5))
         addUInt(MemberDie, dwarf::DW_AT_alignment, dwarf::DW_FORM_udata,
                 AlignInBytes);
     }
 
     if (DD->getDwarfVersion() <= 2) {
       DIELoc *MemLocationDie = new (DIEValueAllocator) DIELoc;
       addUInt(*MemLocationDie, dwarf::DW_FORM_data1, dwarf::DW_OP_plus_uconst);
       addUInt(*MemLocationDie, dwarf::DW_FORM_udata, OffsetInBytes);
else if (DT->isPublic())
     addUInt(StaticMemberDIE, dwarf::DW_AT_accessibility, dwarf::DW_FORM_data1,
             dwarf::DW_ACCESS_public);
 
   if (const ConstantInt *CI = dyn_cast_or_null<ConstantInt>(DT->getConstant()))
     addConstantValue(StaticMemberDIE, CI, Ty);
   if (const ConstantFP *CFP = dyn_cast_or_null<ConstantFP>(DT->getConstant()))
     addConstantFPValue(StaticMemberDIE, CFP);
 
+  // For DBX, we generate DWARF 5 attribute at DWARF version no less than 5.
+  if (!DD->tuneForDBX() || DD->getDwarfVersion() >= 5)
     if (uint32_t AlignInBytes = DT->getAlignInBytes())
       addUInt(StaticMemberDIE, dwarf::DW_AT_alignment, dwarf::DW_FORM_udata,
               AlignInBytes);
 
   return &StaticMemberDIE;
 }
 
 void DwarfUnit::emitCommonHeader(bool UseOffsets, dwarf::UnitType UT) {
   // Emit size of content not including length itself
   if (!DD->useSectionsAsReferences())
     EndLabel = Asm->emitDwarfUnitLength(

llvm/test/DebugInfo/PowerPC/align.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:   -debugger-tune=dbx < %s | llvm-dwarfdump -debug-info - | \
+; RUN:   FileCheck %s --check-prefix=DBX
+
+; CHECK: DW_AT_alignment
+; DBX-NOT: DW_AT_alignment
+
+@_ZL3var = internal global i32 0, align 16, !dbg !0
+
+define dso_local signext i32 @_Z3foov() #0 !dbg !11 {
+  %1 = load i32, i32* @_ZL3var, align 16, !dbg !15
+  ret i32 %1, !dbg !16
+}
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!7, !8, !9}
+!llvm.ident = !{!10}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "var", linkageName: "_ZL3var", scope: !2, file: !3, line: 1, 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: "align.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 = !{!"clang version 13.0.0"}
+!11 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !3, file: !3, line: 3, type: !12, scopeLine: 4, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !2, retainedNodes: !4)
+!12 = !DISubroutineType(types: !13)
+!13 = !{!14}
+!14 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!15 = !DILocation(line: 5, column: 18, scope: !11)
+!16 = !DILocation(line: 5, column: 11, scope: !11)