[PATCH] D87062: [DebugInfo] Add size to class declarations in debug info.

[Amy Huang via Phabricator via cfe-commits]

akhuang created this revision.
akhuang added reviewers: aprantl, dblaikie.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.
akhuang requested review of this revision.
Herald added a subscriber: ormris.

This adds the size to forward declared class DITypes, if the size is known.

Fixes an issue where we determine whether to emit fragments based on the
type size, so fragments would sometimes be incorrectly emitted if there
was no size.

Also make sure the sizes for forward declared classes aren't emitted in
DWARF.

Bug: https://bugs.llvm.org/show_bug.cgi?id=47338


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87062

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-class.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/Transforms/Scalar/SROA.cpp
  llvm/test/DebugInfo/X86/struct-fwd-decl.ll


Index: llvm/test/DebugInfo/X86/struct-fwd-decl.ll
===================================================================
--- /dev/null
+++ llvm/test/DebugInfo/X86/struct-fwd-decl.ll
@@ -0,0 +1,21 @@
+; RUN: llc -O0 -mtriple=x86_64-unknown-linux %s -o %t -filetype=obj
+; RUN: llvm-dwarfdump -debug-info %t | FileCheck %s
+; Test that size is not emitted for class declarations in DWARF, even if it exists.
+
+ at s = global i16 0, align 2, !dbg !0
+
+!llvm.dbg.cu = !{!4}
+!llvm.module.flags = !{!7}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = !DIGlobalVariable(name: "s", scope: null, file: !2, line: 2, type: !3, isLocal: false, isDefinition: true)
+!2 = !DIFile(filename: "foo.cpp", directory: "/tmp")
+!3 = !DICompositeType(tag: DW_TAG_structure_type, name: "S", file: !2, line: 1, size: 16, align: 16, flags: DIFlagFwdDecl)
+!4 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !2, isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !5, retainedTypes: !5, globals: !6, imports: !5)
+!5 = !{}
+; CHECK: DW_TAG_structure_type
+; CHECK-NEXT: DW_AT_name
+; CHECK-NOT: DW_AT_byte_size
+; CHECK-NEXT: DW_AT_declaration
+!6 = !{!0}
+!7 = !{i32 1, !"Debug Info Version", i32 3}
Index: llvm/lib/Transforms/Scalar/SROA.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/SROA.cpp
+++ llvm/lib/Transforms/Scalar/SROA.cpp
@@ -4534,15 +4534,15 @@
 
         // The alloca may be larger than the variable.
         auto VarSize = DbgDeclare->getVariable()->getSizeInBits();
-        if (VarSize) {
-          if (Size > *VarSize)
-            Size = *VarSize;
-          if (Size == 0 || Start + Size > *VarSize)
-            continue;
-        }
+        assert(VarSize &&
+               "Any variable with a location must have a type with a size");
+        if (Size > *VarSize)
+          Size = *VarSize;
+        if (Size == 0 || Start + Size > *VarSize)
+          continue;
 
         // Avoid creating a fragment expression that covers the entire variable.
-        if (!VarSize || *VarSize != Size) {
+        if (*VarSize != Size) {
           if (auto E =
                   DIExpression::createFragmentExpression(Expr, Start, Size))
             FragmentExpr = *E;
Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===================================================================
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1013,8 +1013,10 @@
       Tag == dwarf::DW_TAG_class_type || Tag == dwarf::DW_TAG_structure_type ||
       Tag == dwarf::DW_TAG_union_type) {
     // Add size if non-zero (derived types might be zero-sized.)
+    // Ignore the size if it's a non-enum forward decl.
     // TODO: Do we care about size for enum forward declarations?
-    if (Size)
+    if (Size &&
+        (!CTy->isForwardDecl() || Tag == dwarf::DW_TAG_enumeration_type))
       addUInt(Buffer, dwarf::DW_AT_byte_size, None, Size);
     else if (!CTy->isForwardDecl())
       // Add zero size if it is not a forward declaration.
Index: clang/test/CodeGenCXX/debug-info-class.cpp
===================================================================
--- clang/test/CodeGenCXX/debug-info-class.cpp
+++ clang/test/CodeGenCXX/debug-info-class.cpp
@@ -136,7 +136,7 @@
 // CHECK: [[C_DTOR]] = !DISubprogram(name: "~C"
 
 // CHECK: [[D:![0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: "D"
-// CHECK-NOT:              size:
+// CHECK-SAME:             size:
 // CHECK-SAME:             DIFlagFwdDecl
 // CHECK-NOT:              identifier:
 // CHECK-SAME:             ){{$}}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===================================================================
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1031,6 +1031,10 @@
   uint64_t Size = 0;
   uint32_t Align = 0;
 
+  const RecordDecl *D = RD->getDefinition();
+  if (D && D->isCompleteDefinition())
+    Size = CGM.getContext().getTypeSize(Ty);
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagFwdDecl;
 
   // Add flag to nontrivial forward declarations. To be consistent with MSVC,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87062.289596.patch
Type: text/x-patch
Size: 4165 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200902/80e0f64b/attachment.bin>