Skip to content

Commit

Permalink
[clang][DebugInfo] Revert "emit definitions for constant-initialized …
Browse files Browse the repository at this point in the history
…static data-members" (#74580)

This commit reverts the changes in
#71780 and all of its follow-up
patches.

We got reports of the `.debug_names/.debug_gnu_pubnames/gdb_index/etc.`
sections growing by a non-trivial amount for some large projects. While
GCC emits definitions for static data member constants into the Names
index, they do so *only* for explicitly `constexpr` members. We were
indexing *all* constant-initialized const-static members, which is
likely where the significant size difference comes from. However, only
emitting explicitly `constexpr` variables into the index doesn't seem
like a good way forward, since from clang's perspective `const`-static
integrals are `constexpr` too, and that shouldn't be any different in
the debug-info component. Also, as new code moves to `constexpr` instead
of `const` static for constants, such solution would just delay the
growth of the Names index.

To prevent the size regression we revert to not emitting definitions for
static data-members that have no location.

To support access to such constants from LLDB we'll most likely have to
have to make LLDB find the constants by looking at the containing class
first.
  • Loading branch information
Michael137 authored Dec 6, 2023
1 parent c5a1732 commit 4db54e6
Show file tree
Hide file tree
Showing 8 changed files with 17 additions and 293 deletions.
73 changes: 0 additions & 73 deletions clang/lib/CodeGen/CGDebugInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,29 +69,6 @@ static uint32_t getDeclAlignIfRequired(const Decl *D, const ASTContext &Ctx) {
return D->hasAttr<AlignedAttr>() ? D->getMaxAlignment() : 0;
}

/// Given a VarDecl corresponding to either the definition or
/// declaration of a C++ static data member, if it has a constant
/// initializer and is evaluatable, return the evaluated value.
/// Returns std::nullopt otherwise.
static std::optional<APValue>
evaluateConstantInitializer(const clang::VarDecl *VD,
const clang::ASTContext &Ctx) {
assert(VD != nullptr);

if (!VD->isStaticDataMember())
return std::nullopt;

if (!VD->isUsableInConstantExpressions(Ctx))
return std::nullopt;

auto const *InitExpr = VD->getAnyInitializer();
Expr::EvalResult Result;
if (!InitExpr->EvaluateAsConstantExpr(Result, Ctx))
return std::nullopt;

return Result.Val;
}

CGDebugInfo::CGDebugInfo(CodeGenModule &CGM)
: CGM(CGM), DebugKind(CGM.getCodeGenOpts().getDebugInfo()),
DebugTypeExtRefs(CGM.getCodeGenOpts().DebugTypeExtRefs),
Expand Down Expand Up @@ -1724,7 +1701,6 @@ CGDebugInfo::CreateRecordStaticField(const VarDecl *Var, llvm::DIType *RecordTy,
llvm::DIDerivedType *GV = DBuilder.createStaticMemberType(
RecordTy, VName, VUnit, LineNumber, VTy, Flags, C, Tag, Align);
StaticDataMemberCache[Var->getCanonicalDecl()].reset(GV);
StaticDataMemberDefinitionsToEmit.push_back(Var->getCanonicalDecl());
return GV;
}

Expand Down Expand Up @@ -5628,41 +5604,6 @@ 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;

const auto CacheIt = DeclCache.find(VD);
if (CacheIt != DeclCache.end())
return;

const auto InitVal = evaluateConstantInitializer(VD, CGM.getContext());
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 @@ -5867,20 +5808,6 @@ void CGDebugInfo::setDwoId(uint64_t Signature) {
}

void CGDebugInfo::finalize() {
// We can't use a for-each here because `EmitGlobalVariable`
// may push new decls into `StaticDataMemberDefinitionsToEmit`,
// which would invalidate any iterator.
for (size_t i = 0; i < StaticDataMemberDefinitionsToEmit.size(); ++i) {
auto const *VD = StaticDataMemberDefinitionsToEmit[i];

assert(VD && 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: 0 additions & 6 deletions clang/lib/CodeGen/CGDebugInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 *>;
Expand Down Expand Up @@ -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);

Expand Down
12 changes: 4 additions & 8 deletions clang/test/CodeGenCXX/debug-info-class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,18 +117,11 @@ int main(int argc, char **argv) {
// 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 @@ -194,5 +187,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,
104 changes: 0 additions & 104 deletions clang/test/CodeGenCXX/debug-info-static-inline-member.cpp

This file was deleted.

20 changes: 4 additions & 16 deletions clang/test/CodeGenCXX/debug-info-static-member.cpp
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// RUN: %clangxx -target x86_64-unknown-unknown -g -gdwarf-4 %s -emit-llvm -S -o - | FileCheck --check-prefixes=CHECK,DWARF4,CPP11,NOT-MS %s
// RUN: %clangxx -target x86_64-unknown-unknown -g -gdwarf-4 %s -emit-llvm -S -o - | FileCheck --check-prefixes=CHECK,DWARF4,NOT-MS %s
// RUN: %clangxx -target x86_64-unknown-unknown -g -gdwarf-4 -std=c++98 %s -emit-llvm -S -o - | FileCheck --check-prefixes=CHECK,DWARF4,NOT-MS %s
// RUN: %clangxx -target x86_64-unknown-unknown -g -gdwarf-4 -std=c++11 %s -emit-llvm -S -o - | FileCheck --check-prefixes=CHECK,DWARF4,CPP11,NOT-MS %s
// RUN: %clangxx -target x86_64-unknown-unknown -g -gdwarf-5 -std=c++11 %s -emit-llvm -S -o - | FileCheck --check-prefixes=CHECK,DWARF5,CPP11 %s
// RUN: %clangxx -target x86_64-windows-msvc -g -gdwarf-4 %s -emit-llvm -S -o - | FileCheck --check-prefixes=CHECK,DWARF4,CPP11 %s
// RUN: %clangxx -target x86_64-unknown-unknown -g -gdwarf-4 -std=c++11 %s -emit-llvm -S -o - | FileCheck --check-prefixes=CHECK,DWARF4,NOT-MS %s
// RUN: %clangxx -target x86_64-unknown-unknown -g -gdwarf-5 -std=c++11 %s -emit-llvm -S -o - | FileCheck --check-prefixes=CHECK,DWARF5 %s
// RUN: %clangxx -target x86_64-windows-msvc -g -gdwarf-4 %s -emit-llvm -S -o - | FileCheck --check-prefixes=CHECK,DWARF4 %s
// PR14471

// CHECK: @{{.*}}a{{.*}} = dso_local global i32 4, align 4, !dbg [[A:![0-9]+]]
Expand Down Expand Up @@ -166,15 +166,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]])

// CPP11: !DIGlobalVariableExpression(var: ![[CONST_B_VAR:[0-9]+]], expr: !DIExpression(DW_OP_constu, {{.*}}, DW_OP_stack_value))
// CPP11: ![[CONST_B_VAR]] = distinct !DIGlobalVariable(name: "const_b"
// CPP11-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]])
64 changes: 2 additions & 62 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,54 +142,6 @@ static bool ShouldIgnoreArtificialField(llvm::StringRef FieldName) {
|| FieldName.starts_with("_vptr.");
}

std::optional<DWARFFormValue>
DWARFASTParserClang::FindConstantOnVariableDefinition(DWARFDIE die) {
assert(die.Tag() == DW_TAG_member || die.Tag() == DW_TAG_variable);

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) {
Expand Down Expand Up @@ -2916,23 +2868,11 @@ void DWARFASTParserClang::CreateStaticMemberVariable(

bool unused;
// TODO: Support float/double static members as well.
if (!ct.IsIntegerOrEnumerationType(unused))
if (!ct.IsIntegerOrEnumerationType(unused) || !attrs.const_value_form)
return;

auto maybe_const_form_value = attrs.const_value_form;

// 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 (!maybe_const_form_value) {
maybe_const_form_value = FindConstantOnVariableDefinition(die);
if (!maybe_const_form_value)
return;
}

llvm::Expected<llvm::APInt> const_value_or_err =
ExtractIntFromFormValue(ct, *maybe_const_form_value);
ExtractIntFromFormValue(ct, *attrs.const_value_form);
if (!const_value_or_err) {
LLDB_LOG_ERROR(log, const_value_or_err.takeError(),
"Failed to add const value to variable {1}: {0}",
Expand Down
11 changes: 0 additions & 11 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
Original file line number Diff line number Diff line change
Expand Up @@ -413,17 +413,6 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
lldb_private::CompilerType &class_clang_type,
const lldb::AccessType default_accesibility,
lldb_private::ClangASTImporter::LayoutInfo &layout_info);

/// Tries to find the definition DW_TAG_variable DIE of the the specified
/// DW_TAG_member 'die'. If such definition exists, returns the
/// DW_AT_const_value of that definition if available. Returns std::nullopt
/// otherwise.
///
/// In newer versions of clang, DW_AT_const_value attributes are not attached
/// to the declaration of a inline static data-member anymore, but rather on
/// its definition. This function is used to locate said constant.
std::optional<lldb_private::plugin::dwarf::DWARFFormValue>
FindConstantOnVariableDefinition(lldb_private::plugin::dwarf::DWARFDIE die);
};

/// Parsed form of all attributes that are relevant for type reconstruction.
Expand Down
Loading

0 comments on commit 4db54e6

Please sign in to comment.