-
Notifications
You must be signed in to change notification settings - Fork 13.4k
[lldb][DWARFASTParserClang] GetClangDeclForDIE: don't create VarDecl for static data members #77155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[lldb][DWARFASTParserClang] GetClangDeclForDIE: don't create VarDecl for static data members #77155
Conversation
…for static data members With DWARFv5, C++ static data members are represented as `DW_TAG_variable`s (see `faa3a5ea9ae481da757dab1c95c589e2d5645982`). In GetClangDeclForDIE, when trying to parse the `DW_AT_specification` that the CU-level DW_TAG_variable points to, we would try to `CreateVariableDeclaration`, because the static member is a `DW_TAG_variable` too. Whereas previously it was a no-op (for `DW_TAG_member`s). Adding `VarDecls` to RecordDecls for static data members should always be done in `CreateStaticMemberVariable`. The test-case is an exapmle where we would crash if we tried to create a `VarDecl` from within `GetClangDeclForDIE` for a static data member. This patch simply checks whether the `DW_TAG_variable` being parsed is a static data member, and if so, trivially return from `GetClangDeclForDIE` (as we previously did for `DW_TAG_member`s).
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesWith DWARFv5, C++ static data members are represented as In GetClangDeclForDIE, when trying to parse the This patch simply checks whether the Full diff: https://github.com/llvm/llvm-project/pull/77155.diff 3 Files Affected:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 3e08f2550081f2..771d33bf8ff7ae 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -142,6 +142,18 @@ static bool ShouldIgnoreArtificialField(llvm::StringRef FieldName) {
|| FieldName.starts_with("_vptr.");
}
+/// Returns true for C++ constructs represented by clang::CXXRecordDecl
+static bool TagIsRecordType(dw_tag_t tag) {
+ switch (tag) {
+ case DW_TAG_class_type:
+ case DW_TAG_structure_type:
+ case DW_TAG_union_type:
+ return true;
+ default:
+ return false;
+ }
+}
+
TypeSP DWARFASTParserClang::ParseTypeFromClangModule(const SymbolContext &sc,
const DWARFDIE &die,
Log *log) {
@@ -3293,12 +3305,19 @@ clang::Decl *DWARFASTParserClang::GetClangDeclForDIE(const DWARFDIE &die) {
return nullptr;
switch (die.Tag()) {
- case DW_TAG_variable:
case DW_TAG_constant:
case DW_TAG_formal_parameter:
case DW_TAG_imported_declaration:
case DW_TAG_imported_module:
break;
+ case DW_TAG_variable:
+ // This means 'die' is a C++ static data member.
+ // We don't want to create decls for such members
+ // here.
+ if (auto parent = die.GetParent();
+ parent.IsValid() && TagIsRecordType(parent.Tag()))
+ return nullptr;
+ break;
default:
return nullptr;
}
diff --git a/lldb/test/Shell/SymbolFile/DWARF/Inputs/dwo-static-data-member.cpp b/lldb/test/Shell/SymbolFile/DWARF/Inputs/dwo-static-data-member.cpp
new file mode 100644
index 00000000000000..fa7c3500df0f9d
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/Inputs/dwo-static-data-member.cpp
@@ -0,0 +1,8 @@
+struct NoCtor {
+ NoCtor();
+ static int i;
+};
+
+int NoCtor::i = 15;
+
+int main() { return NoCtor::i; }
diff --git a/lldb/test/Shell/SymbolFile/DWARF/dwo-static-data-member-access.test b/lldb/test/Shell/SymbolFile/DWARF/dwo-static-data-member-access.test
new file mode 100644
index 00000000000000..da5a862c35568f
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/dwo-static-data-member-access.test
@@ -0,0 +1,23 @@
+# UNSUPPORTED: system-darwin, system-windows
+
+# In DWARFv5, C++ static data members are represented
+# as DW_TAG_variable. We make sure LLDB's expression
+# evaluator doesn't crash when trying to parse such
+# a DW_TAG_variable DIE, whose parent DIE is only
+# a forward declaration.
+
+# RUN: %clangxx_host %S/Inputs/dwo-static-data-member.cpp \
+# RUN: -g -gdwarf-5 -gsplit-dwarf -o %t
+# RUN: %lldb %t -s %s -o exit 2>&1 | FileCheck %s
+
+breakpoint set -n main
+process launch
+
+# CHECK: Process {{.*}} stopped
+
+# There is no definition for NoCtor anywhere
+# in the debug-info, so LLDB can't evaluate
+# this expression.
+expression NoCtor::i
+# CHECK-LABEL: expression NoCtor::i
+# CHECK: use of undeclared identifier 'NoCtor'
|
@@ -0,0 +1,23 @@ | |||
# UNSUPPORTED: system-darwin, system-windows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just out of curiosity — we are evaluating a static constant. Do we actually need to run the binary, or could we run the expression against e.g., an ELF binary on Darwin, too, by just evaluating the expression and not launching at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's actually a great question, but I think for now the answer with lldb is "no, expression evaluation doesn't work without a running process" - but I was just commenting on an internal bug where folks were having trouble investigating core dumps with some of our pretty printers that evaluate expressions that happen to be constants that gdb can evaluate on a core/without a running process, but it seems lldb can't do that...
So while that sounds like a good thing, my understanding is that lldb can't do that at the moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As @dwblaikie points out, we need a process to run the expression. But if we specify -flimit-debug-info
I can repro the crash on Darwin too, so we can remove UNSUPPORTED
here and get the coverage that way.
lldb/test/Shell/SymbolFile/DWARF/dwo-static-data-member-access.test
Outdated
Show resolved
Hide resolved
…for static data members (llvm#77155) With DWARFv5, C++ static data members are represented as `DW_TAG_variable`s (see `faa3a5ea9ae481da757dab1c95c589e2d5645982`). In GetClangDeclForDIE, when trying to parse the `DW_AT_specification` that a static data member's CU-level `DW_TAG_variable` points to, we would try to `CreateVariableDeclaration`. Whereas previously it was a no-op (for `DW_TAG_member`s). However, adding `VarDecls` to RecordDecls for static data members should always be done in `CreateStaticMemberVariable`. The test-case is an exapmle where we would crash if we tried to create a `VarDecl` from within `GetClangDeclForDIE` for a static data member. This patch simply checks whether the `DW_TAG_variable` being parsed is a static data member, and if so, trivially returns from `GetClangDeclForDIE` (as we previously did for `DW_TAG_member`s).
With DWARFv5, C++ static data members are represented as
DW_TAG_variable
s (seefaa3a5ea9ae481da757dab1c95c589e2d5645982
).In GetClangDeclForDIE, when trying to parse the
DW_AT_specification
that a static data member's CU-levelDW_TAG_variable
points to, we would try toCreateVariableDeclaration
. Whereas previously it was a no-op (forDW_TAG_member
s). However, addingVarDecls
to RecordDecls for static data members should always be done inCreateStaticMemberVariable
. The test-case is an exapmle where we would crash if we tried to create aVarDecl
from withinGetClangDeclForDIE
for a static data member.This patch simply checks whether the
DW_TAG_variable
being parsed is a static data member, and if so, trivially returns fromGetClangDeclForDIE
(as we previously did forDW_TAG_member
s).