Skip to content

Commit

Permalink
Reland "[clang][DebugInfo] Emit global variable definitions for stati…
Browse files Browse the repository at this point in the history
…c data members with constant initializers" (#71780)

This patch relands #70639

It was reverted because under certain conditions we triggered an
assertion
in `DIBuilder`. Specifically, in the original patch we called
`EmitGlobalVariable`
at the end of `CGDebugInfo::finalize`, after all the temporary `DIType`s
have
been uniqued. With limited debug-info such temporary nodes would be
created
more frequently, leaving us with non-uniqued nodes by the time we got to
`DIBuilder::finalize`; this violated its pre-condition and caused
assertions to trigger.

To fix this, the latest iteration of the patch moves
`EmitGlobalVariable` to the
beginning of `CGDebugInfo::finalize`. Now, when we create a temporary
`DIType` node as a result of emitting a variable definition, it will get
uniqued
in time. A test-case was added for this scenario.

We also now don't emit a linkage name for non-locationed constants since
LLDB doesn't make use of it anyway.

Original commit message:
"""
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’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.
"""

Dependent changes:
* #71800
  • Loading branch information
Michael137 authored Nov 13, 2023
1 parent 9a3d3c7 commit 638a839
Show file tree
Hide file tree
Showing 7 changed files with 219 additions and 42 deletions.
60 changes: 49 additions & 11 deletions clang/lib/CodeGen/CGDebugInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1677,22 +1677,13 @@ 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, C, Align);
RecordTy, VName, VUnit, LineNumber, VTy, Flags, /* Val */ nullptr, Align);
StaticDataMemberCache[Var->getCanonicalDecl()].reset(GV);
StaticDataMemberDefinitionsToEmit.push_back(Var->getCanonicalDecl());
return GV;
}

Expand Down Expand Up @@ -5596,6 +5587,44 @@ void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD, const APValue &Init) {
TemplateParameters, Align));
}

void CGDebugInfo::EmitGlobalVariable(const VarDecl *VD) {
assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
if (VD->hasAttr<NoDebugAttr>())
return;

if (!VD->hasInit())
return;

const auto CacheIt = DeclCache.find(VD);
if (CacheIt != DeclCache.end())
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);

// Omit linkage name for variable definitions that represent constants.
// There hasn't been a need from consumers yet to have it attached.
DeclCache[VD].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());
Expand Down Expand Up @@ -5800,6 +5829,15 @@ void CGDebugInfo::setDwoId(uint64_t Signature) {
}

void CGDebugInfo::finalize() {
for (auto const *VD : StaticDataMemberDefinitionsToEmit) {
assert(VD->isStaticDataMember());

if (DeclCache.contains(VD))
continue;

EmitGlobalVariable(VD);
}

// Creating types might create further types - invalidating the current
// element and the size(), so don't cache/reference them.
for (size_t i = 0; i != ObjCInterfaceCache.size(); ++i) {
Expand Down
6 changes: 6 additions & 0 deletions clang/lib/CodeGen/CGDebugInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,9 @@ 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 *>;
Expand Down Expand Up @@ -526,6 +529,9 @@ 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);

Expand Down
13 changes: 9 additions & 4 deletions clang/test/CodeGenCXX/debug-info-class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,19 @@ 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"
Expand Down Expand Up @@ -186,8 +194,5 @@ 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,
114 changes: 114 additions & 0 deletions clang/test/CodeGenCXX/debug-info-static-inline-member.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
// RUN: %clangxx -target arm64-apple-macosx11.0.0 -g -debug-info-kind=standalone %s -emit-llvm -S -o - | FileCheck --check-prefixes=CHECK %s
// RUN: %clangxx -target arm64-apple-macosx11.0.0 -g -debug-info-kind=limited %s -emit-llvm -S -o - | FileCheck --check-prefixes=CHECK %s

enum class Enum : int {
VAL = -1
};

struct Empty {};
struct Fwd;

constexpr auto func() { return 25; }

struct Foo {
static constexpr int cexpr_int_with_addr = func();
static constexpr int cexpr_int2 = func() + 1;
static constexpr float cexpr_float = 2.0 + 1.0;
static constexpr Enum cexpr_enum = Enum::VAL;
static constexpr Empty cexpr_struct_with_addr{};
static inline Enum inline_enum = Enum::VAL;

template<typename T, unsigned V>
static constexpr auto cexpr_template = V;

static const auto empty_templated = cexpr_template<Fwd, 1>;
};

int main() {
Foo f;
//Bar b;

// Force global variable definitions to be emitted.
(void)&Foo::cexpr_int_with_addr;
(void)&Foo::cexpr_struct_with_addr;

return Foo::cexpr_int_with_addr + Foo::cexpr_float
+ (int)Foo::cexpr_enum + Foo::cexpr_template<short, 5>
+ Foo::empty_templated;
}

// CHECK: @{{.*}}cexpr_int_with_addr{{.*}} =
// CHECK-SAME: !dbg ![[INT_GLOBAL:[0-9]+]]

// CHECK: @{{.*}}cexpr_struct_with_addr{{.*}} =
// CHECK-SAME !dbg ![[EMPTY_GLOBAL:[0-9]+]]

// CHECK: !DIGlobalVariableExpression(var: ![[INT_VAR:[0-9]+]], expr: !DIExpression())
// CHECK: ![[INT_VAR]] = distinct !DIGlobalVariable(name: "cexpr_int_with_addr", linkageName:
// CHECK-SAME: isLocal: false, isDefinition: true, declaration: ![[INT_DECL:[0-9]+]])

// CHECK: ![[INT_DECL]] = !DIDerivedType(tag: DW_TAG_member, name: "cexpr_int_with_addr",
// CHECK-SAME: flags: DIFlagStaticMember
// CHECK-NOT: extraData:

// CHECK: ![[INT_DECL2:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "cexpr_int2",
// CHECK-SAME: flags: DIFlagStaticMember
// CHECK-NOT: extraData:

// CHECK: ![[FLOAT_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "cexpr_float",
// CHECK-SAME: flags: DIFlagStaticMember
// CHECK-NOT: extraData:

// CHECK: ![[ENUM_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "cexpr_enum",
// CHECK-SAME: flags: DIFlagStaticMember
// CHECK-NOT: extraData:

// CHECK: ![[EMPTY_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "cexpr_struct_with_addr",
// CHECK-SAME: flags: DIFlagStaticMember
// CHECK-NOT: extraData:

// CHECK: ![[IENUM_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "inline_enum",
// CHECK-SAME: flags: DIFlagStaticMember
// CHECK-NOT: extraData:

// CHECK: ![[EMPTY_TEMPLATED_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "empty_templated",
// CHECK-SAME: flags: DIFlagStaticMember
// CHECK-NOT: extraData:

// CHECK: ![[TEMPLATE_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "cexpr_template",
// CHECK-SAME: flags: DIFlagStaticMember
// CHECK-NOT: extraData:

// CHECK: !DIGlobalVariableExpression(var: ![[EMPTY_VAR:[0-9]+]], expr: !DIExpression())
// CHECK: ![[EMPTY_VAR]] = distinct !DIGlobalVariable(name: "cexpr_struct_with_addr", linkageName:
// CHECK-SAME: isLocal: false, isDefinition: true, declaration: ![[EMPTY_DECL]])

// CHECK: !DIGlobalVariableExpression(var: ![[INT_VAR2:[0-9]+]], expr: !DIExpression(DW_OP_constu, 26, DW_OP_stack_value))
// CHECK: ![[INT_VAR2]] = distinct !DIGlobalVariable(name: "cexpr_int2"
// CHECK-NOT: linkageName:
// CHECK-SAME: isLocal: true, isDefinition: true, declaration: ![[INT_DECL2]])

// CHECK: !DIGlobalVariableExpression(var: ![[FLOAT_VAR:[0-9]+]], expr: !DIExpression(DW_OP_constu, {{.*}}, DW_OP_stack_value))
// CHECK: ![[FLOAT_VAR]] = distinct !DIGlobalVariable(name: "cexpr_float"
// CHECK-NOT: linkageName:
// CHECK-SAME: isLocal: true, isDefinition: true, declaration: ![[FLOAT_DECL]])

// CHECK: !DIGlobalVariableExpression(var: ![[ENUM_VAR:[0-9]+]], expr: !DIExpression(DW_OP_constu, {{.*}}, DW_OP_stack_value))
// CHECK: ![[ENUM_VAR]] = distinct !DIGlobalVariable(name: "cexpr_enum"
// CHECK-NOT: linkageName:
// CHECK-SAME: isLocal: true, isDefinition: true, declaration: ![[ENUM_DECL]])

// CHECK: !DIGlobalVariableExpression(var: ![[IENUM_VAR:[0-9]+]], expr: !DIExpression(DW_OP_constu, {{.*}}, DW_OP_stack_value))
// CHECK: ![[IENUM_VAR]] = distinct !DIGlobalVariable(name: "inline_enum"
// CHECK-NOT: linkageName:
// CHECK-SAME: isLocal: true, isDefinition: true, declaration: ![[IENUM_DECL]])

// CHECK: !DIGlobalVariableExpression(var: ![[EMPTY_TEMPLATED_VAR:[0-9]+]], expr: !DIExpression(DW_OP_constu, 1, DW_OP_stack_value))
// CHECK: ![[EMPTY_TEMPLATED_VAR]] = distinct !DIGlobalVariable(name: "empty_templated"
// CHECK-NOT: linkageName:
// CHECK-SAME: isLocal: true, isDefinition: true, declaration: ![[EMPTY_TEMPLATED_DECL]])

// CHECK: !DIGlobalVariableExpression(var: ![[TEMPLATE_VAR:[0-9]+]], expr: !DIExpression(DW_OP_constu, 5, DW_OP_stack_value))
// CHECK: ![[TEMPLATE_VAR]] = distinct !DIGlobalVariable(name: "cexpr_template"
// CHECK-NOT: linkageName:
// CHECK-SAME: isLocal: true, isDefinition: true, declaration: ![[TEMPLATE_DECL]], templateParams: ![[TEMPLATE_PARMS_2:[0-9]+]])
52 changes: 32 additions & 20 deletions clang/test/CodeGenCXX/debug-info-static-member.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,32 +63,32 @@ int C::a = 4;
// CHECK-NOT: offset:
// CHECK-SAME: flags: DIFlagStaticMember)
//
// 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: ![[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: ![[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: !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: ![[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: "x_a"
// CHECK-SAME: flags: DIFlagPublic | DIFlagStaticMember)
Expand Down Expand Up @@ -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-SAME: extraData: i32 42
// NOT-MS-NOT: extraData:
const int V::const_va;

namespace x {
Expand All @@ -156,3 +156,15 @@ 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]])
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,10 @@ def test(self):
# it does not crash.
self.expect("image lookup -t A")

# dsymutil strips the debug info for classes that only have const static
# data members without a definition namespace scope.
@expectedFailureAll(debug_info=["dsym"])
# For debug-info produced by older versions of clang, dsymutil strips the
# debug info for classes that only have const static data members without
# definitions.
@expectedFailureAll(compiler=["clang"], compiler_version=["<", "18.0"])
def test_class_with_only_const_static(self):
self.build()
lldbutil.run_to_source_breakpoint(
Expand Down
9 changes: 5 additions & 4 deletions lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,11 @@ def verify_values(self, verify_dict, actual, varref_dict=None, expression=None):
def verify_variables(self, verify_dict, variables, varref_dict=None):
for variable in variables:
name = variable["name"]
self.assertIn(
name, verify_dict, 'variable "%s" in verify dictionary' % (name)
)
self.verify_values(verify_dict[name], variable, varref_dict)
if not name.startswith("std::"):
self.assertIn(
name, verify_dict, 'variable "%s" in verify dictionary' % (name)
)
self.verify_values(verify_dict[name], variable, varref_dict)

def darwin_dwarf_missing_obj(self, initCommands):
self.build(debug_info="dwarf")
Expand Down

0 comments on commit 638a839

Please sign in to comment.