[clang][DebugInfo] DWARFv5: static data members declarations are DW_TAG_variable #72235
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-debuginfo @llvm/pr-subscribers-clang-codegen Author: Michael Buch (Michael137) ChangesThis patch implements the DWARFv5 issue This will simplify LLDB's handling of static data Full diff: https://github.com/llvm/llvm-project/pull/72235.diff 3 Files Affected:
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 04ca02cfe858579..f3de91d4a39ebee 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1679,9 +1679,13 @@ CGDebugInfo::CreateRecordStaticField(const VarDecl *Var, llvm::DIType *RecordTy,
StringRef VName = Var->getName();
llvm::DINode::DIFlags Flags = getAccessFlag(Var->getAccess(), RD);
+ auto Tag = CGM.getCodeGenOpts().DwarfVersion >= 5
+ ? llvm::dwarf::DW_TAG_variable
+ : llvm::dwarf::DW_TAG_member;
auto Align = getDeclAlignIfRequired(Var, CGM.getContext());
- llvm::DIDerivedType *GV = DBuilder.createStaticMemberType(
- RecordTy, VName, VUnit, LineNumber, VTy, Flags, /* Val */ nullptr, Align);
+ llvm::DIDerivedType *GV =
+ DBuilder.createStaticMemberType(RecordTy, VName, VUnit, LineNumber, VTy,
+ Flags, /* Val */ nullptr, Tag, Align);
StaticDataMemberCache[Var->getCanonicalDecl()].reset(GV);
StaticDataMemberDefinitionsToEmit.push_back(Var->getCanonicalDecl());
return GV;
diff --git a/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp b/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp
index bfb7ad6b80fabf3..f2d4d9408a8297a 100644
--- a/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp
+++ b/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp
@@ -1,5 +1,5 @@
-// 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
+// RUN: %clangxx -target arm64-apple-macosx11.0.0 -g -gdwarf-4 -debug-info-kind=standalone %s -emit-llvm -S -o - | FileCheck --check-prefixes=CHECK %s
+// RUN: %clangxx -target arm64-apple-macosx11.0.0 -g -gdwarf-4 -debug-info-kind=limited %s -emit-llvm -S -o - | FileCheck --check-prefixes=CHECK %s
enum class Enum : int {
VAL = -1
diff --git a/clang/test/CodeGenCXX/debug-info-static-member.cpp b/clang/test/CodeGenCXX/debug-info-static-member.cpp
index 578995801c6400b..a2d25e98ed1cb62 100644
--- a/clang/test/CodeGenCXX/debug-info-static-member.cpp
+++ b/clang/test/CodeGenCXX/debug-info-static-member.cpp
@@ -1,7 +1,8 @@
-// RUN: %clangxx -target x86_64-unknown-unknown -g %s -emit-llvm -S -o - | FileCheck --check-prefixes=CHECK,NOT-MS %s
-// RUN: %clangxx -target x86_64-unknown-unknown -g -std=c++98 %s -emit-llvm -S -o - | FileCheck --check-prefixes=CHECK,NOT-MS %s
-// RUN: %clangxx -target x86_64-unknown-unknown -g -std=c++11 %s -emit-llvm -S -o - | FileCheck --check-prefixes=CHECK,NOT-MS %s
-// RUN: %clangxx -target x86_64-windows-msvc -g %s -emit-llvm -S -o - | FileCheck --check-prefixes=CHECK %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,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]+]]
@@ -39,17 +40,20 @@ class C
//
// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "X"{{.*}})
// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "anon_static_decl_struct"
-// CHECK: !DIDerivedType(tag: DW_TAG_member, name: "anon_static_decl_var"
+// DWARF4: !DIDerivedType(tag: DW_TAG_member, name: "anon_static_decl_var"
+// DWARF5: !DIDerivedType(tag: DW_TAG_variable, name: "anon_static_decl_var"
// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "static_decl_templ<int>"
// CHECK-NOT: DIFlagFwdDecl
// CHECK-SAME: ){{$}}
-// CHECK: !DIDerivedType(tag: DW_TAG_member, name: "static_decl_templ_var"
+// DWARF4: !DIDerivedType(tag: DW_TAG_member, name: "static_decl_templ_var"
+// DWARF5: !DIDerivedType(tag: DW_TAG_variable, name: "static_decl_templ_var"
int C::a = 4;
// CHECK: [[B]] = !DIGlobalVariableExpression(var: [[BV:.*]], expr: !DIExpression())
// CHECK: [[BV]] = distinct !DIGlobalVariable(name: "b",
// CHECK-SAME: declaration: ![[DECL_B:[0-9]+]])
-// CHECK: ![[DECL_B]] = !DIDerivedType(tag: DW_TAG_member, name: "b"
+// DWARF4: ![[DECL_B]] = !DIDerivedType(tag: DW_TAG_member, name: "b"
+// DWARF5: ![[DECL_B]] = !DIDerivedType(tag: DW_TAG_variable, name: "b"
// CHECK-NOT: size:
// CHECK-NOT: align:
// CHECK-NOT: offset:
@@ -57,40 +61,46 @@ int C::a = 4;
//
// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "C"{{.*}})
//
-// CHECK: ![[DECL_A]] = !DIDerivedType(tag: DW_TAG_member, name: "a"
+// DWARF4: ![[DECL_A]] = !DIDerivedType(tag: DW_TAG_member, name: "a"
+// DWARF5: ![[DECL_A]] = !DIDerivedType(tag: DW_TAG_variable, name: "a"
// CHECK-NOT: size:
// CHECK-NOT: align:
// CHECK-NOT: offset:
// CHECK-SAME: flags: DIFlagStaticMember)
//
-// CHECK: ![[CONST_A_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "const_a"
+// DWARF4: ![[CONST_A_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "const_a"
+// DWARF5: ![[CONST_A_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_variable, 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"
+// DWARF4: ![[CONST_B_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "const_b"
+// DWARF5: ![[CONST_B_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_variable, 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"
+// DWARF4: ![[DECL_C:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "c"
+// DWARF5: ![[DECL_C:[0-9]+]] = !DIDerivedType(tag: DW_TAG_variable, 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"
+// DWARF4: ![[CONST_C_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "const_c"
+// DWARF5: ![[CONST_C_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_variable, 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"
+// DWARF4: !DIDerivedType(tag: DW_TAG_member, name: "x_a"
+// DWARF5: !DIDerivedType(tag: DW_TAG_variable, name: "x_a"
// CHECK-SAME: flags: DIFlagPublic | DIFlagStaticMember)
int C::b = 2;
|
…AG_variable This patch implements the DWARFv5 issue 161118.1: "DW_TAG for C++ static data members". This will simplify LLDB's handling of static data members greatly in the long term since we no longer need to differentiate non-static from static data member declarations using non-portable heuristics.
Michael137
force-pushed
the
dwarf/tag-variable-static-members/PR/2-clang
branch
from
216100a to
a933f17
Compare
zahiraam
pushed a commit
to zahiraam/llvm-project
that referenced
this pull request
Nov 20, 2023
…AG_variable (llvm#72235) This patch implements the DWARFv5 issue 161118.1: "DW_TAG for C++ static data members". This will simplify LLDB's handling of static data members greatly in the long term since we no longer need to differentiate non-static from static data member declarations using non-portable heuristics. Depends on: * llvm#72234
zahiraam
pushed a commit
to zahiraam/llvm-project
that referenced
this pull request
Nov 20, 2023
…ata members declarations (llvm#72236) The accepted DWARFv5 issue 161118.1: "DW_TAG for C++ static data members" specifies that static data member declaration be described by DW_TAG_variable. Make sure we recognize such members. Depends on: * llvm#72234 * llvm#72235
|
Should've mentioned this earlier, but we've been seeing crashes in lldb due to this change (though possibly related to our use of GALA python/gdb interop https://github.com/sivachandra/gala - we haven't isolated the failures from that yet (though we've been using it for years, so it's not the proximal cause, at least)). Might be worth reverting so other people aren't tripping over it too. but working on some repro/isolated/reduced case/something useful to share |
Something reproducible, or a stack trace would be appreciated! The only way I see this change breaking something is in tools that don't expect |
Oh, a stack is easy to provide, sure: |
|
Perhaps not surprisingly from the backtrace, the DIE that's tripping things up is the DW_TAG_variable here: (so a case where ctor homing/ Might help me reproduce this in a standalone example.. |
|
Yeah, that seems to do it: (Hmm, maybe our google build of lldb enables something to print the stack trace - I can reproduce this with (in this case with a debug+asserts build) an open source build of lldb too, like this:) |
Awesome thanks for the reproducer! Will look into it asap |
|
In our practical example this clearly reproduces in cases where the definition is available in another .dwo within the same executable. So I'm guessing something needs to go looking for a definition (& then if it can't find one, fail gracefully) here. There's probably some abstraction in lldb for doing that "oh, we need a type definition here, try to find one". |
|
Ah looks like in Maybe the solution is to add a check inside In your reproducer it looks like we only ever emit a forward declaration for the containing structure |
|
Yes, in the totally reduced reproducer - another debugger might be able to do this name lookup with only a declaration, but I understand that at least Clang's AST expression evaluator wouldn't be able to handle this case. (though perhaps the lldb-eval work will provide some opportunity to catch this case) But at least in our real-world example I think the definition is available, in another DWO in the same executable. But I don't have a small reproducer for that - small examples "just work" (they do find the type definition, don't crash) - but in the larger one we still see the crash depending on exactly where the expression is evaluated (within a fully statically linked binary - so the definition is available, broadly speaking, no matter the context of evaluation - not sure what makes the difference) If it's helpful to try to identify the more nuanced case where a crash occurs despite the definition being available /somewhere/, I can try to figure that out but it seems difficult to do so. |
I'll submit a PR later today to get us back to previous behaviour. The test will just use your minimal reproducer to make sure we don't crash |
|
Thanks! |
|
Proposed fix: #77155 |
|
@dwblaikie merged the fix, let me know if you're still seeing problems |
|
looking good from what I've tested - thanks again! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch implements the DWARFv5 issue 161118.1: "DW_TAG for C++ static data members".
This will simplify LLDB's handling of static data members greatly in the long term since we no longer need to differentiate non-static from static data member declarations using non-portable heuristics.
Depends on: