Revert "Reland "clang][DebugInfo] Emit global variable definitions for static data members with constant initializers (#70639)""

This casued asserts:

  llvm/lib/IR/Metadata.cpp:689:
  void llvm::MDNode::resolve(): Assertion `isUniqued() && "Expected this to be uniqued"' failed.

See comments on the PR.

This also reverts the dependent follow-up commits, see below.

> When an LLDB user asks for the value of a static data member, LLDB
> starts by searching the Names accelerator table for the corresponding
> variable definition DIE. For static data members with out-of-class
> definitions that works fine, because those get represented as global
> variables with a location and making them eligible to be added to the
> Names table. However, in-class definitions won<E2><80><99>t get indexed because
> we usually don't emit global variables for them. So in DWARF we end
> up with a single `DW_TAG_member` that usually holds the constant
> initializer.  But we don't get a corresponding CU-level
> `DW_TAG_variable` like we do for out-of-class definitions.
>
> To make it more convenient for debuggers to get to the value of
> inline static data members, this patch makes sure we emit definitions
> for static variables with constant initializers the same way we do
> for other static variables. This also aligns Clang closer to GCC,
> which produces CU-level definitions for inline statics and also
> emits these into `.debug_pubnames`.
>
> The implementation keeps track of newly created static data members.
> Then in `CGDebugInfo::finalize`, we emit a global `DW_TAG_variable`
> with a `DW_AT_const_value` for any of those declarations that didn't
> end up with a definition in the `DeclCache`.
>
> The newly emitted `DW_TAG_variable` will look as follows:
> ```
> 0x0000007b:   DW_TAG_structure_type
>                 DW_AT_calling_convention        (DW_CC_pass_by_value)
>                 DW_AT_name      ("Foo")
>                 ...
>
> 0x0000008d:     DW_TAG_member
>                   DW_AT_name    ("i")
>                   DW_AT_type    (0x00000062 "const int")
>                   DW_AT_external        (true)
>                   DW_AT_declaration     (true)
>                   DW_AT_const_value     (4)
>
> Newly added
> vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
>
> 0x0000009a:   DW_TAG_variable
>                 DW_AT_specification     (0x0000008d "i")
>                 DW_AT_const_value       (4)
>                 DW_AT_linkage_name      ("_ZN2t2IiE1iIfEE")
> ```
>
> This patch also drops the `DW_AT_const_value` off of the declaration
> since we now always have it on the definition. This ensures that the
> `DWARFParallelLinker` can type-merge class with static members where
> we couldn't attach the constant on the declaration in some CUs.

This reverts commit 7c3707a.
This reverts commit cab0a19.
This reverts commit 317481b.
This reverts commit 15fc809.
This reverts commit 470de2b.

Author: zmodem

clang/lib/CodeGen/CGDebugInfo.cpp

@@ -1677,13 +1677,22 @@ CGDebugInfo::CreateRecordStaticField(const VarDecl *Var, llvm::DIType *RecordTy,
 
   unsigned LineNumber = getLineNumber(Var->getLocation());
   StringRef VName = Var->getName();
+  llvm::Constant *C = nullptr;
+  if (Var->getInit()) {
+    const APValue *Value = Var->evaluateValue();
+    if (Value) {
+      if (Value->isInt())
+        C = llvm::ConstantInt::get(CGM.getLLVMContext(), Value->getInt());
+      if (Value->isFloat())
+        C = llvm::ConstantFP::get(CGM.getLLVMContext(), Value->getFloat());
+    }
+  }
 
   llvm::DINode::DIFlags Flags = getAccessFlag(Var->getAccess(), RD);
   auto Align = getDeclAlignIfRequired(Var, CGM.getContext());
   llvm::DIDerivedType *GV = DBuilder.createStaticMemberType(
-      RecordTy, VName, VUnit, LineNumber, VTy, Flags, /* Val */ nullptr, Align);
+      RecordTy, VName, VUnit, LineNumber, VTy, Flags, C, Align);
   StaticDataMemberCache[Var->getCanonicalDecl()].reset(GV);
-  StaticDataMemberDefinitionsToEmit.push_back(Var->getCanonicalDecl());
   return GV;
 }
 
@@ -5587,39 +5596,6 @@ void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD, const APValue &Init) {
       TemplateParameters, Align));
 }
 
-void CGDebugInfo::EmitGlobalVariable(const VarDecl *VD) {
-  assert(VD->hasInit());
-  assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
-  if (VD->hasAttr<NoDebugAttr>())
-    return;
-
-  auto &GV = DeclCache[VD];
-  if (GV)
-    return;
-
-  auto const *InitVal = VD->evaluateValue();
-  if (!InitVal)
-    return;
-
-  llvm::DIFile *Unit = nullptr;
-  llvm::DIScope *DContext = nullptr;
-  unsigned LineNo;
-  StringRef DeclName, LinkageName;
-  QualType T;
-  llvm::MDTuple *TemplateParameters = nullptr;
-  collectVarDeclProps(VD, Unit, LineNo, T, DeclName, LinkageName,
-                      TemplateParameters, DContext);
-
-  auto Align = getDeclAlignIfRequired(VD, CGM.getContext());
-  llvm::DINodeArray Annotations = CollectBTFDeclTagAnnotations(VD);
-  llvm::DIExpression *InitExpr = createConstantValueExpression(VD, *InitVal);
-
-  GV.reset(DBuilder.createGlobalVariableExpression(
-      TheCU, DeclName, LinkageName, Unit, LineNo, getOrCreateType(T, Unit),
-      true, true, InitExpr, getOrCreateStaticDataMemberDeclarationOrNull(VD),
-      TemplateParameters, Align, Annotations));
-}
-
 void CGDebugInfo::EmitExternalVariable(llvm::GlobalVariable *Var,
                                        const VarDecl *D) {
   assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
@@ -5890,18 +5866,6 @@ void CGDebugInfo::finalize() {
     DBuilder.replaceTemporary(std::move(FwdDecl), cast<llvm::MDNode>(Repl));
   }
 
-  for (auto const *VD : StaticDataMemberDefinitionsToEmit) {
-    assert(VD->isStaticDataMember());
-
-    if (DeclCache.contains(VD))
-      continue;
-
-    if (!VD->hasInit())
-      continue;
-
-    EmitGlobalVariable(VD);
-  }
-
   // We keep our own list of retained types, because we need to look
   // up the final type in the type cache.
   for (auto &RT : RetainedTypes)

clang/lib/CodeGen/CGDebugInfo.h

@@ -161,9 +161,6 @@ class CGDebugInfo {
   llvm::DenseMap<const Decl *, llvm::TypedTrackingMDRef<llvm::DIDerivedType>>
       StaticDataMemberCache;
 
-  /// Keeps track of static data members for which we should emit a definition.
-  std::vector<const VarDecl *> StaticDataMemberDefinitionsToEmit;
-
   using ParamDecl2StmtTy = llvm::DenseMap<const ParmVarDecl *, const Stmt *>;
   using Param2DILocTy =
       llvm::DenseMap<const ParmVarDecl *, llvm::DILocalVariable *>;
@@ -529,9 +526,6 @@ class CGDebugInfo {
   /// Emit a constant global variable's debug info.
   void EmitGlobalVariable(const ValueDecl *VD, const APValue &Init);
 
-  /// Emit debug-info for a variable with a constant initializer.
-  void EmitGlobalVariable(const VarDecl *VD);
-
   /// Emit information about an external variable.
   void EmitExternalVariable(llvm::GlobalVariable *GV, const VarDecl *Decl);
 

clang/test/CodeGenCXX/debug-info-class.cpp

@@ -116,19 +116,11 @@ int main(int argc, char **argv) {
 // CHECK-SAME:                             DIFlagFwdDecl
 // CHECK-NOT:                             identifier:
 // CHECK-SAME:                            ){{$}}
-
-// CHECK:      !DIGlobalVariableExpression(var: ![[HDR_VAR:[0-9]+]], expr: !DIExpression(DW_OP_constu, 52, DW_OP_stack_value))
-// CHECK:      ![[HDR_VAR]] = distinct !DIGlobalVariable(name: "HdrSize",
-// CHECK-SAME:                isLocal: true, isDefinition: true, declaration: ![[HDR_VAR_DECL:[0-9]+]])
-// CHECK:      ![[INT:[0-9]+]] = !DIBasicType(name: "int"
-// CHECK:      ![[HDR_VAR_DECL]] = !DIDerivedType(tag: DW_TAG_member, name: "HdrSize"
-
-// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "A"
-
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "I"
 // CHECK-NOT:              DIFlagFwdDecl
 // CHECK-SAME:             ){{$}}
 
+// CHECK: ![[INT:[0-9]+]] = !DIBasicType(name: "int"
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
 // CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "bar"
 // CHECK: !DICompositeType(tag: DW_TAG_union_type, name: "baz"
@@ -194,5 +186,8 @@ int main(int argc, char **argv) {
 // CHECK: [[G_INNER_I]] = !DIDerivedType(tag: DW_TAG_member, name: "j"
 // CHECK-SAME:                           baseType: ![[INT]]
 
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "A"
+// CHECK: !DIDerivedType(tag: DW_TAG_member, name: "HdrSize"
+//
 // CHECK: ![[EXCEPTLOC]] = !DILocation(line: 100,
 // CHECK: ![[RETLOC]] = !DILocation(line: 99,

clang/test/CodeGenCXX/debug-info-static-inline-member.cpp

@@ -63,32 +63,32 @@ int C::a = 4;
 // CHECK-NOT:                                 offset:
 // CHECK-SAME:                                flags: DIFlagStaticMember)
 //
-// CHECK:    ![[CONST_A_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "const_a"
-// CHECK-NOT:                           size:
-// CHECK-NOT:                           align:
-// CHECK-NOT:                           offset:
-// CHECK-SAME:                          flags: DIFlagStaticMember
-// CHECK-NOT:                           extraData:
-
-// CHECK:     ![[CONST_B_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "const_b"
-// CHECK-NOT:                            size:
-// CHECK-NOT:                            align:
-// CHECK-NOT:                            offset:
-// CHECK-SAME:                           flags: DIFlagProtected | DIFlagStaticMember
-// CHECK-NOT:                            extraData:
+// CHECK: !DIDerivedType(tag: DW_TAG_member, name: "const_a"
+// CHECK-NOT:            size:
+// CHECK-NOT:            align:
+// CHECK-NOT:            offset:
+// CHECK-SAME:           flags: DIFlagStaticMember,
+// CHECK-SAME:           extraData: i1 true)
+
+// CHECK: !DIDerivedType(tag: DW_TAG_member, name: "const_b"
+// CHECK-NOT:            size:
+// CHECK-NOT:            align:
+// CHECK-NOT:            offset:
+// CHECK-SAME:           flags: DIFlagProtected | DIFlagStaticMember,
+// CHECK-SAME:           extraData: float 0x{{.*}})
 
 // CHECK: ![[DECL_C:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "c"
 // CHECK-NOT:                                 size:
 // CHECK-NOT:                                 align:
 // CHECK-NOT:                                 offset:
 // CHECK-SAME:                                flags: DIFlagPublic | DIFlagStaticMember)
 //
-// CHECK:     ![[CONST_C_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "const_c"
-// CHECK-NOT:                            size:
-// CHECK-NOT:                            align:
-// CHECK-NOT:                            offset:
-// CHECK-SAME:                           flags: DIFlagPublic | DIFlagStaticMember
-// CHECK-NOT:                            extraData:
+// CHECK: !DIDerivedType(tag: DW_TAG_member, name: "const_c"
+// CHECK-NOT:            size:
+// CHECK-NOT:            align:
+// CHECK-NOT:            offset:
+// CHECK-SAME:           flags: DIFlagPublic | DIFlagStaticMember,
+// CHECK-SAME:           extraData: i32 18)
 //
 // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "x_a"
 // CHECK-SAME:           flags: DIFlagPublic | DIFlagStaticMember)
@@ -144,7 +144,7 @@ struct V {
 // const_va is not emitted for MS targets.
 // NOT-MS: !DIDerivedType(tag: DW_TAG_member, name: "const_va",
 // NOT-MS-SAME:           line: [[@LINE-5]]
-// NOT-MS-NOT:            extraData:
+// NOT-MS-SAME:           extraData: i32 42
 const int V::const_va;
 
 namespace x {
@@ -156,15 +156,3 @@ struct y {
 };
 int y::z;
 }
-
-// CHECK:      !DIGlobalVariableExpression(var: ![[CONST_A_VAR:[0-9]+]], expr: !DIExpression(DW_OP_constu, 1, DW_OP_stack_value))
-// CHECK:      ![[CONST_A_VAR]] = distinct !DIGlobalVariable(name: "const_a"
-// CHECK-SAME:                    isLocal: true, isDefinition: true, declaration: ![[CONST_A_DECL]])
-
-// CHECK:      !DIGlobalVariableExpression(var: ![[CONST_B_VAR:[0-9]+]], expr: !DIExpression(DW_OP_constu, {{.*}}, DW_OP_stack_value))
-// CHECK:      ![[CONST_B_VAR]] = distinct !DIGlobalVariable(name: "const_b"
-// CHECK-SAME:                    isLocal: true, isDefinition: true, declaration: ![[CONST_B_DECL]])
-
-// CHECK:      !DIGlobalVariableExpression(var: ![[CONST_C_VAR:[0-9]+]], expr: !DIExpression(DW_OP_constu, 18, DW_OP_stack_value))
-// CHECK:      ![[CONST_C_VAR]] = distinct !DIGlobalVariable(name: "const_c"
-// CHECK-SAME:                    isLocal: true, isDefinition: true, declaration: ![[CONST_C_DECL]])

clang/test/CodeGenCXX/debug-info-static-member.cpp

@@ -31,7 +31,6 @@
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/TypeList.h"
 #include "lldb/Symbol/TypeMap.h"
-#include "lldb/Symbol/VariableList.h"
 #include "lldb/Target/Language.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
@@ -134,54 +133,6 @@ static lldb::ModuleSP GetContainingClangModule(const DWARFDIE &die) {
   return lldb::ModuleSP();
 }
 
-std::optional<DWARFFormValue>
-DWARFASTParserClang::FindConstantOnVariableDefinition(DWARFDIE die) {
-  assert(die.Tag() == llvm::dwarf::DW_TAG_member);
-
-  auto *dwarf = die.GetDWARF();
-  if (!dwarf)
-    return {};
-
-  ConstString name{die.GetName()};
-  if (!name)
-    return {};
-
-  auto *CU = die.GetCU();
-  if (!CU)
-    return {};
-
-  DWARFASTParser *dwarf_ast = dwarf->GetDWARFParser(*CU);
-  auto parent_decl_ctx = dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
-
-  // Make sure we populate the GetDieToVariable cache.
-  VariableList variables;
-  dwarf->FindGlobalVariables(name, parent_decl_ctx, UINT_MAX, variables);
-
-  // The cache contains the variable definition whose DW_AT_specification
-  // points to our declaration DIE. Look up that definition using our
-  // declaration.
-  auto const &die_to_var = dwarf->GetDIEToVariable();
-  auto it = die_to_var.find(die.GetDIE());
-  if (it == die_to_var.end())
-    return {};
-
-  auto var_sp = it->getSecond();
-  assert(var_sp != nullptr);
-
-  if (!var_sp->GetLocationIsConstantValueData())
-    return {};
-
-  auto def = dwarf->GetDIE(var_sp->GetID());
-  auto def_attrs = def.GetAttributes();
-  DWARFFormValue form_value;
-  if (!def_attrs.ExtractFormValueAtIndex(
-          def_attrs.FindAttributeIndex(llvm::dwarf::DW_AT_const_value),
-          form_value))
-    return {};
-
-  return form_value;
-}
-
 TypeSP DWARFASTParserClang::ParseTypeFromClangModule(const SymbolContext &sc,
                                                      const DWARFDIE &die,
                                                      Log *log) {
@@ -2955,21 +2906,9 @@ void DWARFASTParserClang::ParseSingleMember(
 
       bool unused;
       // TODO: Support float/double static members as well.
-      if (!ct.IsIntegerOrEnumerationType(unused))
+      if (!attrs.const_value_form || !ct.IsIntegerOrEnumerationType(unused))
         return;
 
-      // Newer versions of Clang don't emit the DW_AT_const_value
-      // on the declaration of an inline static data member. Instead
-      // it's attached to the definition DIE. If that's the case,
-      // try and fetch it.
-      if (!attrs.const_value_form) {
-        auto maybe_form_value = FindConstantOnVariableDefinition(die);
-        if (!maybe_form_value)
-          return;
-
-        attrs.const_value_form = *maybe_form_value;
-      }
-
       llvm::Expected<llvm::APInt> const_value_or_err =
           ExtractIntFromFormValue(ct, *attrs.const_value_form);
       if (!const_value_or_err) {