[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
ZequanWu wrote: > You can repro this by running ./bin/lldb-dotest -p > TestConstStaticIntegralMember.py --dwarf-version 5 > > Could you take a look @ZequanWu ? It should be fixed by the following. We just need to skip the declaration DIEs that are struct/class/union. This failure you see is caused by skipping a declaration `DW_TAG_variable`. ``` diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp index 56717bab1ecd..6330470b970e 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp @@ -87,7 +87,8 @@ bool DebugNamesDWARFIndex::ProcessEntry( return true; // Clang erroneously emits index entries for declaration DIEs in case when the // definition is in a type unit (llvm.org/pr77696). Weed those out. - if (die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0)) + if (die.IsStructUnionOrClass() && + die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0)) return true; return callback(die); } ``` Can you (or anyone with commit access) commit above fix for me? I somehow cannot pull/push from git. https://github.com/llvm/llvm-project/pull/92328 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Fix adding children to clang type that hasn't started definition. (PR #93839)
https://github.com/ZequanWu closed https://github.com/llvm/llvm-project/pull/93839 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Fix adding children to clang type that hasn't started definition. (PR #93839)
https://github.com/ZequanWu edited https://github.com/llvm/llvm-project/pull/93839 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Fix adding children to clang type that hasn't started definition. (PR #93839)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/93839
>From 90cbcf8a97fb2e7c5131ac2cb601b95fe7a331c6 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Thu, 30 May 2024 11:36:10 -0400
Subject: [PATCH 1/4] [lldb][DWARF] Do not differentiate DW_TAG_class_type and
DW_TAG_struct_type in UniqueDWARFASTTypeList.
---
.../Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 5 +
.../Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp | 12 ++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index e0b1b430b266f..78969d4752f80 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2232,6 +2232,11 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE ,
// For objective C we don't start the definition when the class is
// created.
TypeSystemClang::StartTagDeclarationDefinition(clang_type);
+} else if (!clang_type.IsBeingDefined()) {
+ // In case of some weired DWARF causing we don't start definition on this
+ // definition DIE because we failed to find existing clang_type from
+ // UniqueDWARFASTTypeMap due to overstrict checking.
+ TypeSystemClang::StartTagDeclarationDefinition(clang_type);
}
AccessType default_accessibility = eAccessNone;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
index 4762356034cab..3d201e96f92c3 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
@@ -13,12 +13,18 @@
using namespace lldb_private::dwarf;
using namespace lldb_private::plugin::dwarf;
+static bool IsStructOrClassTag(llvm::dwarf::Tag Tag) {
+ return Tag == llvm::dwarf::Tag::DW_TAG_class_type ||
+ Tag == llvm::dwarf::Tag::DW_TAG_structure_type;
+}
+
UniqueDWARFASTType *UniqueDWARFASTTypeList::Find(
const DWARFDIE , const lldb_private::Declaration ,
const int32_t byte_size, bool is_forward_declaration) {
for (UniqueDWARFASTType : m_collection) {
// Make sure the tags match
-if (udt.m_die.Tag() == die.Tag()) {
+if (udt.m_die.Tag() == die.Tag() || (IsStructOrClassTag(udt.m_die.Tag()) &&
+ IsStructOrClassTag(die.Tag( {
// If they are not both definition DIEs or both declaration DIEs, then
// don't check for byte size and declaration location, because
declaration
// DIEs usually don't have those info.
@@ -39,7 +45,9 @@ UniqueDWARFASTType *UniqueDWARFASTTypeList::Find(
while (!done && match && parent_arg_die && parent_pos_die) {
const dw_tag_t parent_arg_tag = parent_arg_die.Tag();
const dw_tag_t parent_pos_tag = parent_pos_die.Tag();
-if (parent_arg_tag == parent_pos_tag) {
+if (parent_arg_tag == parent_pos_tag ||
+(IsStructOrClassTag(parent_arg_tag) &&
+ IsStructOrClassTag(parent_pos_tag))) {
switch (parent_arg_tag) {
case DW_TAG_class_type:
case DW_TAG_structure_type:
>From d78b4d1394643c3cb4a558d91d907bad30e3b4e6 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Thu, 30 May 2024 12:29:03 -0400
Subject: [PATCH 2/4] Just assert if c/c++ clang_type is not being defined in
DWARFASTParserClang::CompleteRecordType.
---
.../Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 78969d4752f80..09325404d6092 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2232,11 +2232,10 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE ,
// For objective C we don't start the definition when the class is
// created.
TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-} else if (!clang_type.IsBeingDefined()) {
- // In case of some weired DWARF causing we don't start definition on this
- // definition DIE because we failed to find existing clang_type from
- // UniqueDWARFASTTypeMap due to overstrict checking.
- TypeSystemClang::StartTagDeclarationDefinition(clang_type);
+} else {
+ assert(
+ clang_type.IsBeingDefined() &&
+ "The clang type should already start its definition at this point.");
}
AccessType default_accessibility = eAccessNone;
>From e7fc16ec5f31693191188b3b95728c4320465923 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Thu, 30 May 2024 12:33:05 -0400
Subject: [PATCH 3/4] Apply suggestions from code review
Update assertion message.
[Lldb-commits] [lldb] [lldb][DWARF] Fix adding children to clang type that hasn't started definition. (PR #93839)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/93839
>From 90cbcf8a97fb2e7c5131ac2cb601b95fe7a331c6 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Thu, 30 May 2024 11:36:10 -0400
Subject: [PATCH 1/3] [lldb][DWARF] Do not differentiate DW_TAG_class_type and
DW_TAG_struct_type in UniqueDWARFASTTypeList.
---
.../Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 5 +
.../Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp | 12 ++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index e0b1b430b266f..78969d4752f80 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2232,6 +2232,11 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE ,
// For objective C we don't start the definition when the class is
// created.
TypeSystemClang::StartTagDeclarationDefinition(clang_type);
+} else if (!clang_type.IsBeingDefined()) {
+ // In case of some weired DWARF causing we don't start definition on this
+ // definition DIE because we failed to find existing clang_type from
+ // UniqueDWARFASTTypeMap due to overstrict checking.
+ TypeSystemClang::StartTagDeclarationDefinition(clang_type);
}
AccessType default_accessibility = eAccessNone;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
index 4762356034cab..3d201e96f92c3 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
@@ -13,12 +13,18 @@
using namespace lldb_private::dwarf;
using namespace lldb_private::plugin::dwarf;
+static bool IsStructOrClassTag(llvm::dwarf::Tag Tag) {
+ return Tag == llvm::dwarf::Tag::DW_TAG_class_type ||
+ Tag == llvm::dwarf::Tag::DW_TAG_structure_type;
+}
+
UniqueDWARFASTType *UniqueDWARFASTTypeList::Find(
const DWARFDIE , const lldb_private::Declaration ,
const int32_t byte_size, bool is_forward_declaration) {
for (UniqueDWARFASTType : m_collection) {
// Make sure the tags match
-if (udt.m_die.Tag() == die.Tag()) {
+if (udt.m_die.Tag() == die.Tag() || (IsStructOrClassTag(udt.m_die.Tag()) &&
+ IsStructOrClassTag(die.Tag( {
// If they are not both definition DIEs or both declaration DIEs, then
// don't check for byte size and declaration location, because
declaration
// DIEs usually don't have those info.
@@ -39,7 +45,9 @@ UniqueDWARFASTType *UniqueDWARFASTTypeList::Find(
while (!done && match && parent_arg_die && parent_pos_die) {
const dw_tag_t parent_arg_tag = parent_arg_die.Tag();
const dw_tag_t parent_pos_tag = parent_pos_die.Tag();
-if (parent_arg_tag == parent_pos_tag) {
+if (parent_arg_tag == parent_pos_tag ||
+(IsStructOrClassTag(parent_arg_tag) &&
+ IsStructOrClassTag(parent_pos_tag))) {
switch (parent_arg_tag) {
case DW_TAG_class_type:
case DW_TAG_structure_type:
>From d78b4d1394643c3cb4a558d91d907bad30e3b4e6 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Thu, 30 May 2024 12:29:03 -0400
Subject: [PATCH 2/3] Just assert if c/c++ clang_type is not being defined in
DWARFASTParserClang::CompleteRecordType.
---
.../Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 78969d4752f80..09325404d6092 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2232,11 +2232,10 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE ,
// For objective C we don't start the definition when the class is
// created.
TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-} else if (!clang_type.IsBeingDefined()) {
- // In case of some weired DWARF causing we don't start definition on this
- // definition DIE because we failed to find existing clang_type from
- // UniqueDWARFASTTypeMap due to overstrict checking.
- TypeSystemClang::StartTagDeclarationDefinition(clang_type);
+} else {
+ assert(
+ clang_type.IsBeingDefined() &&
+ "The clang type should already start its definition at this point.");
}
AccessType default_accessibility = eAccessNone;
>From e7fc16ec5f31693191188b3b95728c4320465923 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Thu, 30 May 2024 12:33:05 -0400
Subject: [PATCH 3/3] Apply suggestions from code review
Update assertion message.
[Lldb-commits] [lldb] [lldb][DWARF] Fix adding children to clang type that hasn't started definition. (PR #93839)
@@ -13,12 +13,18 @@
using namespace lldb_private::dwarf;
using namespace lldb_private::plugin::dwarf;
+static bool IsStructOrClassTag(llvm::dwarf::Tag Tag) {
ZequanWu wrote:
Yeah, we can probably do it in a different change.
https://github.com/llvm/llvm-project/pull/93839
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Fix adding children to clang type that hasn't started definition. (PR #93839)
@@ -2232,6 +2232,11 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE ,
// For objective C we don't start the definition when the class is
// created.
TypeSystemClang::StartTagDeclarationDefinition(clang_type);
+} else if (!clang_type.IsBeingDefined()) {
+ // In case of some weired DWARF causing we don't start definition on this
+ // definition DIE because we failed to find existing clang_type from
+ // UniqueDWARFASTTypeMap due to overstrict checking.
+ TypeSystemClang::StartTagDeclarationDefinition(clang_type);
ZequanWu wrote:
Done.
https://github.com/llvm/llvm-project/pull/93839
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Fix adding children to clang type that hasn't started definition. (PR #93839)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/93839
>From 90cbcf8a97fb2e7c5131ac2cb601b95fe7a331c6 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Thu, 30 May 2024 11:36:10 -0400
Subject: [PATCH 1/2] [lldb][DWARF] Do not differentiate DW_TAG_class_type and
DW_TAG_struct_type in UniqueDWARFASTTypeList.
---
.../Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 5 +
.../Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp | 12 ++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index e0b1b430b266f..78969d4752f80 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2232,6 +2232,11 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE ,
// For objective C we don't start the definition when the class is
// created.
TypeSystemClang::StartTagDeclarationDefinition(clang_type);
+} else if (!clang_type.IsBeingDefined()) {
+ // In case of some weired DWARF causing we don't start definition on this
+ // definition DIE because we failed to find existing clang_type from
+ // UniqueDWARFASTTypeMap due to overstrict checking.
+ TypeSystemClang::StartTagDeclarationDefinition(clang_type);
}
AccessType default_accessibility = eAccessNone;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
index 4762356034cab..3d201e96f92c3 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
@@ -13,12 +13,18 @@
using namespace lldb_private::dwarf;
using namespace lldb_private::plugin::dwarf;
+static bool IsStructOrClassTag(llvm::dwarf::Tag Tag) {
+ return Tag == llvm::dwarf::Tag::DW_TAG_class_type ||
+ Tag == llvm::dwarf::Tag::DW_TAG_structure_type;
+}
+
UniqueDWARFASTType *UniqueDWARFASTTypeList::Find(
const DWARFDIE , const lldb_private::Declaration ,
const int32_t byte_size, bool is_forward_declaration) {
for (UniqueDWARFASTType : m_collection) {
// Make sure the tags match
-if (udt.m_die.Tag() == die.Tag()) {
+if (udt.m_die.Tag() == die.Tag() || (IsStructOrClassTag(udt.m_die.Tag()) &&
+ IsStructOrClassTag(die.Tag( {
// If they are not both definition DIEs or both declaration DIEs, then
// don't check for byte size and declaration location, because
declaration
// DIEs usually don't have those info.
@@ -39,7 +45,9 @@ UniqueDWARFASTType *UniqueDWARFASTTypeList::Find(
while (!done && match && parent_arg_die && parent_pos_die) {
const dw_tag_t parent_arg_tag = parent_arg_die.Tag();
const dw_tag_t parent_pos_tag = parent_pos_die.Tag();
-if (parent_arg_tag == parent_pos_tag) {
+if (parent_arg_tag == parent_pos_tag ||
+(IsStructOrClassTag(parent_arg_tag) &&
+ IsStructOrClassTag(parent_pos_tag))) {
switch (parent_arg_tag) {
case DW_TAG_class_type:
case DW_TAG_structure_type:
>From d78b4d1394643c3cb4a558d91d907bad30e3b4e6 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Thu, 30 May 2024 12:29:03 -0400
Subject: [PATCH 2/2] Just assert if c/c++ clang_type is not being defined in
DWARFASTParserClang::CompleteRecordType.
---
.../Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 78969d4752f80..09325404d6092 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2232,11 +2232,10 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE ,
// For objective C we don't start the definition when the class is
// created.
TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-} else if (!clang_type.IsBeingDefined()) {
- // In case of some weired DWARF causing we don't start definition on this
- // definition DIE because we failed to find existing clang_type from
- // UniqueDWARFASTTypeMap due to overstrict checking.
- TypeSystemClang::StartTagDeclarationDefinition(clang_type);
+} else {
+ assert(
+ clang_type.IsBeingDefined() &&
+ "The clang type should already start its definition at this point.");
}
AccessType default_accessibility = eAccessNone;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Fix adding children to clang type that hasn't started definition. (PR #93839)
@@ -2232,6 +2232,11 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE ,
// For objective C we don't start the definition when the class is
// created.
TypeSystemClang::StartTagDeclarationDefinition(clang_type);
+} else if (!clang_type.IsBeingDefined()) {
+ // In case of some weired DWARF causing we don't start definition on this
+ // definition DIE because we failed to find existing clang_type from
+ // UniqueDWARFASTTypeMap due to overstrict checking.
+ TypeSystemClang::StartTagDeclarationDefinition(clang_type);
ZequanWu wrote:
Yes, the DWARF is reasonable. I added this just in case of
`UniqueDWARFASTTypeMap` failing to find the existing type again for some other
reasons in the future... This checks doesn't get trigger for the test you
reported before. Maybe it's not necessary, because the following loop also just
check for the fully qualified names:
https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp#L39-L71.
https://github.com/llvm/llvm-project/pull/93839
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Fix adding children to clang type that hasn't started definition. (PR #93839)
@@ -13,12 +13,18 @@
using namespace lldb_private::dwarf;
using namespace lldb_private::plugin::dwarf;
+static bool IsStructOrClassTag(llvm::dwarf::Tag Tag) {
ZequanWu wrote:
It's here:
https://github.com/llvm/llvm-project/blob/ed35a92c404650b15a79ff38bcaff41de176cb78/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L136,
maybe move it to namespace `lldb_private::plugin::dwarf`?
https://github.com/llvm/llvm-project/pull/93839
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
ZequanWu wrote: For this specific case, we could fix it by making `DW_TAG_structure_type` equals to `DW_TAG_class_type` in the `UniqueDWARFASTTypeList::Find`. There's few things I noticed with this: 1. If `DW_TAG_structure_type` and `DW_TAG_class_type` are interchangeable, then this comparison on DIE tags also needs to be updated: https://github.com/llvm/llvm-project/blob/a871470a0d0c828718409c7a6dfb067a3231d013/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp#L40-L42, because the one parent DIE could be struct another one could be class and they need to be treated as matched. 2. I wonder why this is not a problem before this change. Before, when `ParseStructureLikeDIE` sees a struct declaration, it searches for definition DIE from type index, which just checks for the fully qualified name of the types. So, it will find the DIE `DW_TAG_class_type` as a definition DIE and create one type from the definition DIE. If `ParseStructureLikeDIE` sees the class definition first. the lldb will be created and `UniqueDWARFASTTypeMap` will have a cache of the type. Later when `ParseStructureLikeDIE` parses the struct declaration, it will still failed to find the cache type in the `UniqueDWARFASTTypeMap` but the call to `FindDefinitionTypeForDWARFDeclContext` will find the definition DIE using fully qualified name which avoid creating the type twice. So, basically this PR relies `UniqueDWARFASTTypeMap` to correctly find the mapping from declaration DIEs to definition DIE and start definition on the clang type (might created from declaration), while it had a backup call to `FindDefinitionTypeForDWARFDeclContext` to find definition DIE with just fully qualified name before this PR. In case of we failed to find existing clang type (created from declaration) in `UniqueDWARFASTTypeMap`, I think it's good to start definition in `CompleteRecordType` if the clang type hasn't started its definition. Sent https://github.com/llvm/llvm-project/pull/93839 to fix it. https://github.com/llvm/llvm-project/pull/92328 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Fix adding children to clang type that hasn't started definition. (PR #93839)
https://github.com/ZequanWu created
https://github.com/llvm/llvm-project/pull/93839
This fixes
https://github.com/llvm/llvm-project/pull/92328#issuecomment-2139339444.
This contains two fixes:
1. Do not differentiate `DW_TAG_class_type` and `DW_TAG_structure_type` in
`UniqueDWARFASTTypeList`, because it's possible that DIE for a type is
`DW_TAG_class_type` in one CU but is `DW_TAG_structure_type` in a different CU.
2. In case of we failed to find existing clang type (created from declaration)
in `UniqueDWARFASTTypeMap` for some other reasons, start clang type definition
in `DWARFASTParserClang::CompleteRecordType` to ensure we always started its
definition before adding children. This should work because we know the DIE
passing in `DWARFASTParserClang::CompleteRecordType` is always a definition
DIE.
>From 90cbcf8a97fb2e7c5131ac2cb601b95fe7a331c6 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Thu, 30 May 2024 11:36:10 -0400
Subject: [PATCH] [lldb][DWARF] Do not differentiate DW_TAG_class_type and
DW_TAG_struct_type in UniqueDWARFASTTypeList.
---
.../Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 5 +
.../Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp | 12 ++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index e0b1b430b266f..78969d4752f80 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2232,6 +2232,11 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE ,
// For objective C we don't start the definition when the class is
// created.
TypeSystemClang::StartTagDeclarationDefinition(clang_type);
+} else if (!clang_type.IsBeingDefined()) {
+ // In case of some weired DWARF causing we don't start definition on this
+ // definition DIE because we failed to find existing clang_type from
+ // UniqueDWARFASTTypeMap due to overstrict checking.
+ TypeSystemClang::StartTagDeclarationDefinition(clang_type);
}
AccessType default_accessibility = eAccessNone;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
index 4762356034cab..3d201e96f92c3 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
@@ -13,12 +13,18 @@
using namespace lldb_private::dwarf;
using namespace lldb_private::plugin::dwarf;
+static bool IsStructOrClassTag(llvm::dwarf::Tag Tag) {
+ return Tag == llvm::dwarf::Tag::DW_TAG_class_type ||
+ Tag == llvm::dwarf::Tag::DW_TAG_structure_type;
+}
+
UniqueDWARFASTType *UniqueDWARFASTTypeList::Find(
const DWARFDIE , const lldb_private::Declaration ,
const int32_t byte_size, bool is_forward_declaration) {
for (UniqueDWARFASTType : m_collection) {
// Make sure the tags match
-if (udt.m_die.Tag() == die.Tag()) {
+if (udt.m_die.Tag() == die.Tag() || (IsStructOrClassTag(udt.m_die.Tag()) &&
+ IsStructOrClassTag(die.Tag( {
// If they are not both definition DIEs or both declaration DIEs, then
// don't check for byte size and declaration location, because
declaration
// DIEs usually don't have those info.
@@ -39,7 +45,9 @@ UniqueDWARFASTType *UniqueDWARFASTTypeList::Find(
while (!done && match && parent_arg_die && parent_pos_die) {
const dw_tag_t parent_arg_tag = parent_arg_die.Tag();
const dw_tag_t parent_pos_tag = parent_pos_die.Tag();
-if (parent_arg_tag == parent_pos_tag) {
+if (parent_arg_tag == parent_pos_tag ||
+(IsStructOrClassTag(parent_arg_tag) &&
+ IsStructOrClassTag(parent_pos_tag))) {
switch (parent_arg_tag) {
case DW_TAG_class_type:
case DW_TAG_structure_type:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
https://github.com/ZequanWu closed https://github.com/llvm/llvm-project/pull/92328 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
https://github.com/ZequanWu edited https://github.com/llvm/llvm-project/pull/92328 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
ZequanWu wrote: Discussed with Pavel, I applied this change to https://github.com/llvm/llvm-project/pull/92328/ so we can ensure the DIEs from the index is always definition DIEs and avoid duplicate/expensive checks later. https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/92328
>From d6de8d9a8bc709b5c4761e9a05f9befede938734 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Wed, 15 May 2024 13:58:42 -0400
Subject: [PATCH 1/4] Reapply [lldb][DWARF] Delay struct/class/union definition
DIE searching when parsing declaration DIEs.
This reapplies 9a7262c2601874e5aa64c5db19746770212d4b44 and
a7eff59f78f08f8ef0487dfe2a136fb311af4fd0 with a fix.
It was causing tests on macos to fail because
`SymbolFileDWARF::GetForwardDeclCompilerTypeToDIE` returned the map owned by
this symol file. When there were two symbol files, two different maps were
created for caching from compiler type to DIE even if they are for the same
module. The solution is to do the same as
`SymbolFileDWARF::GetUniqueDWARFASTTypeMap`: inquery SymbolFileDWARFDebugMap
first to get the shared underlying SymbolFile so the map is shared among
multiple SymbolFileDWARF.
---
.../Plugins/SymbolFile/DWARF/DWARFASTParser.h | 2 +
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 402 ++
.../SymbolFile/DWARF/DWARFASTParserClang.h| 197 -
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 44 +-
.../SymbolFile/DWARF/SymbolFileDWARF.h| 7 +-
.../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 107 ++---
.../SymbolFile/DWARF/UniqueDWARFASTType.h | 36 +-
.../delayed-definition-die-searching.test | 36 ++
8 files changed, 447 insertions(+), 384 deletions(-)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
index 66db396279e06..e144cf0f9bd94 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
@@ -60,6 +60,8 @@ class DWARFASTParser {
virtual ConstString GetDIEClassTemplateParams(const DWARFDIE ) = 0;
+ virtual lldb_private::Type *FindDefinitionTypeForDIE(const DWARFDIE ) =
0;
+
static std::optional
ParseChildArrayInfo(const DWARFDIE _die,
const ExecutionContext *exe_ctx = nullptr);
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index f8101aba5c627..2a46be9216121 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -154,6 +154,26 @@ static bool TagIsRecordType(dw_tag_t tag) {
}
}
+static bool IsForwardDeclaration(const DWARFDIE ,
+ const ParsedDWARFTypeAttributes ,
+ LanguageType cu_language) {
+ if (attrs.is_forward_declaration)
+return true;
+
+ // Work around an issue with clang at the moment where forward
+ // declarations for objective C classes are emitted as:
+ // DW_TAG_structure_type [2]
+ // DW_AT_name( "ForwardObjcClass" )
+ // DW_AT_byte_size( 0x00 )
+ // DW_AT_decl_file( "..." )
+ // DW_AT_decl_line( 1 )
+ //
+ // Note that there is no DW_AT_declaration and there are no children,
+ // and the byte size is zero.
+ return attrs.byte_size && *attrs.byte_size == 0 && attrs.name &&
+ !die.HasChildren() && cu_language == eLanguageTypeObjC;
+}
+
TypeSP DWARFASTParserClang::ParseTypeFromClangModule(const SymbolContext ,
const DWARFDIE ,
Log *log) {
@@ -249,11 +269,9 @@ static void ForcefullyCompleteType(CompilerType type) {
/// This function serves a similar purpose as RequireCompleteType above, but it
/// avoids completing the type if it is not immediately necessary. It only
/// ensures we _can_ complete the type later.
-static void PrepareContextToReceiveMembers(TypeSystemClang ,
- ClangASTImporter _importer,
- clang::DeclContext *decl_ctx,
- DWARFDIE die,
- const char *type_name_cstr) {
+void DWARFASTParserClang::PrepareContextToReceiveMembers(
+clang::DeclContext *decl_ctx, const DWARFDIE _ctx_die,
+const DWARFDIE , const char *type_name_cstr) {
auto *tag_decl_ctx = clang::dyn_cast(decl_ctx);
if (!tag_decl_ctx)
return; // Non-tag context are always ready.
@@ -268,7 +286,8 @@ static void PrepareContextToReceiveMembers(TypeSystemClang
,
// gmodules case), we can complete the type by doing a full import.
// If this type was not imported from an external AST, there's nothing to do.
- CompilerType type = ast.GetTypeForDecl(tag_decl_ctx);
+ CompilerType type = m_ast.GetTypeForDecl(tag_decl_ctx);
+ ClangASTImporter _importer = GetClangASTImporter();
if (type && ast_importer.CanImport(type)) {
auto qual_type = ClangUtil::GetQualType(type);
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
ZequanWu wrote: I removed the checking for DW_AT_declaration attributes when completing type from a DIE and applied https://github.com/llvm/llvm-project/pull/91808 to here to ensure we always have a definition DIE at that point. https://github.com/llvm/llvm-project/pull/92328 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/92328
>From d6de8d9a8bc709b5c4761e9a05f9befede938734 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Wed, 15 May 2024 13:58:42 -0400
Subject: [PATCH 1/3] Reapply [lldb][DWARF] Delay struct/class/union definition
DIE searching when parsing declaration DIEs.
This reapplies 9a7262c2601874e5aa64c5db19746770212d4b44 and
a7eff59f78f08f8ef0487dfe2a136fb311af4fd0 with a fix.
It was causing tests on macos to fail because
`SymbolFileDWARF::GetForwardDeclCompilerTypeToDIE` returned the map owned by
this symol file. When there were two symbol files, two different maps were
created for caching from compiler type to DIE even if they are for the same
module. The solution is to do the same as
`SymbolFileDWARF::GetUniqueDWARFASTTypeMap`: inquery SymbolFileDWARFDebugMap
first to get the shared underlying SymbolFile so the map is shared among
multiple SymbolFileDWARF.
---
.../Plugins/SymbolFile/DWARF/DWARFASTParser.h | 2 +
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 402 ++
.../SymbolFile/DWARF/DWARFASTParserClang.h| 197 -
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 44 +-
.../SymbolFile/DWARF/SymbolFileDWARF.h| 7 +-
.../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 107 ++---
.../SymbolFile/DWARF/UniqueDWARFASTType.h | 36 +-
.../delayed-definition-die-searching.test | 36 ++
8 files changed, 447 insertions(+), 384 deletions(-)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
index 66db396279e06..e144cf0f9bd94 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
@@ -60,6 +60,8 @@ class DWARFASTParser {
virtual ConstString GetDIEClassTemplateParams(const DWARFDIE ) = 0;
+ virtual lldb_private::Type *FindDefinitionTypeForDIE(const DWARFDIE ) =
0;
+
static std::optional
ParseChildArrayInfo(const DWARFDIE _die,
const ExecutionContext *exe_ctx = nullptr);
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index f8101aba5c627..2a46be9216121 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -154,6 +154,26 @@ static bool TagIsRecordType(dw_tag_t tag) {
}
}
+static bool IsForwardDeclaration(const DWARFDIE ,
+ const ParsedDWARFTypeAttributes ,
+ LanguageType cu_language) {
+ if (attrs.is_forward_declaration)
+return true;
+
+ // Work around an issue with clang at the moment where forward
+ // declarations for objective C classes are emitted as:
+ // DW_TAG_structure_type [2]
+ // DW_AT_name( "ForwardObjcClass" )
+ // DW_AT_byte_size( 0x00 )
+ // DW_AT_decl_file( "..." )
+ // DW_AT_decl_line( 1 )
+ //
+ // Note that there is no DW_AT_declaration and there are no children,
+ // and the byte size is zero.
+ return attrs.byte_size && *attrs.byte_size == 0 && attrs.name &&
+ !die.HasChildren() && cu_language == eLanguageTypeObjC;
+}
+
TypeSP DWARFASTParserClang::ParseTypeFromClangModule(const SymbolContext ,
const DWARFDIE ,
Log *log) {
@@ -249,11 +269,9 @@ static void ForcefullyCompleteType(CompilerType type) {
/// This function serves a similar purpose as RequireCompleteType above, but it
/// avoids completing the type if it is not immediately necessary. It only
/// ensures we _can_ complete the type later.
-static void PrepareContextToReceiveMembers(TypeSystemClang ,
- ClangASTImporter _importer,
- clang::DeclContext *decl_ctx,
- DWARFDIE die,
- const char *type_name_cstr) {
+void DWARFASTParserClang::PrepareContextToReceiveMembers(
+clang::DeclContext *decl_ctx, const DWARFDIE _ctx_die,
+const DWARFDIE , const char *type_name_cstr) {
auto *tag_decl_ctx = clang::dyn_cast(decl_ctx);
if (!tag_decl_ctx)
return; // Non-tag context are always ready.
@@ -268,7 +286,8 @@ static void PrepareContextToReceiveMembers(TypeSystemClang
,
// gmodules case), we can complete the type by doing a full import.
// If this type was not imported from an external AST, there's nothing to do.
- CompilerType type = ast.GetTypeForDecl(tag_decl_ctx);
+ CompilerType type = m_ast.GetTypeForDecl(tag_decl_ctx);
+ ClangASTImporter _importer = GetClangASTImporter();
if (type && ast_importer.CanImport(type)) {
auto qual_type = ClangUtil::GetQualType(type);
[Lldb-commits] [lldb] cde1ae4 - [lldb][NativePDB] Fix uninitialized values found by msan.
Author: Zequan Wu
Date: 2024-05-28T11:12:21-04:00
New Revision: cde1ae4c14eecd47215f04d4387845231021d939
URL:
https://github.com/llvm/llvm-project/commit/cde1ae4c14eecd47215f04d4387845231021d939
DIFF:
https://github.com/llvm/llvm-project/commit/cde1ae4c14eecd47215f04d4387845231021d939.diff
LOG: [lldb][NativePDB] Fix uninitialized values found by msan.
Added:
Modified:
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
index fab3ca989c0ec..17c5f6118603f 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -47,15 +47,18 @@ UdtRecordCompleter::UdtRecordCompleter(
CVType cvt = m_index.tpi().getType(m_id.index);
switch (cvt.kind()) {
case LF_ENUM:
+m_cvr.er.Options = ClassOptions::None;
llvm::cantFail(TypeDeserializer::deserializeAs(cvt, m_cvr.er));
break;
case LF_UNION:
+m_cvr.ur.Options = ClassOptions::None;
llvm::cantFail(TypeDeserializer::deserializeAs(cvt,
m_cvr.ur));
m_layout.bit_size = m_cvr.ur.getSize() * 8;
m_record.record.kind = Member::Union;
break;
case LF_CLASS:
case LF_STRUCTURE:
+m_cvr.cr.Options = ClassOptions::None;
llvm::cantFail(TypeDeserializer::deserializeAs(cvt,
m_cvr.cr));
m_layout.bit_size = m_cvr.cr.getSize() * 8;
m_record.record.kind = Member::Struct;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
@@ -2306,6 +2345,11 @@ bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE , if (!die) return false; + ParsedDWARFTypeAttributes attrs(die); ZequanWu wrote: > The reason why this crash does not happen when attempting to complete a type > with no definition is because this function is the actual implementation of > "completing a type from a defintion". I.e., it expects to be called with a > definition DIE, and will never be called if there is no definition (we will > bail out, and possibly "forcefully" complete the type somewhere higher up the > stack). Yes, it should only be called with definition DIE. This extra check just a double-check for it with a bit performance cost. If there's no definition DIE, `SymbolFileDWARF::CompleteType` does an early return. This behaviour is unchanged. Forceful completion behaviour is also remained unchanged, happens when failed to find definition DIE of a containing record type. > If that is true, then I think Greg's patch is a better fix for this problem, > and it also sidesteps all of the performance/memory tradeoff discussions. I agree. If we can ensure the DIE is always a definition DIE for both manual index and debug_names index at a earlier time, there's no need for this check. I'll remove this check when Greg's change is in. https://github.com/llvm/llvm-project/pull/92328 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
@@ -2306,6 +2345,11 @@ bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE , if (!die) return false; + ParsedDWARFTypeAttributes attrs(die); ZequanWu wrote: > Why don't those cases lead to a crash? For example, what would happen if the > name was simply not in the index? That is specific to debug_names because the index gives us a declaration DIE when we are searching for definition DIE in 'FindDefinitionTypeForDWARFDeclContext'. Before, we didn't have the extra check, so we tries to complete the type from declaration DIE, which triggers an assertion in clang. However, it doesn't happen in manual index because we already explicitly checked `DW_AT_declaration` attributes when creating the manual index. It's guaranteed to find a definition DIE from `FindDefinitionTypeForDWARFDeclContext` or nothing (early bailout, won't try to complete it). > So it seems perfectly reasonable to have this check somewhere. I just want to > check whether this is the right place. I assume Greg's change at https://github.com/llvm/llvm-project/pull/91808 will also fix this problem by skipping forward declaration DIE when processing it, which is earlier check than this extra check added here. https://github.com/llvm/llvm-project/pull/92328 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
@@ -2306,6 +2345,11 @@ bool DWARFASTParserClang::CompleteTypeFromDWARF(const
DWARFDIE ,
if (!die)
return false;
+ ParsedDWARFTypeAttributes attrs(die);
ZequanWu wrote:
> How exactly do we get here in that case?
>From https://github.com/llvm/llvm-project/pull/90663#issuecomment-2105194128,
>.debug_names somehow contains entries that are declarations. This causes
>`SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext` to return a type
>created from declaration when searching for definition.
A simple idea I have in mind is to make the `GetForwardDeclCompilerTypeToDIE`'s
value type to a pair `{DIERef, bool}`, and the bool indicate if this is a
definition or not. So we know that info without extra attribute parsing. How do
you think?
https://github.com/llvm/llvm-project/pull/92328
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
https://github.com/ZequanWu edited https://github.com/llvm/llvm-project/pull/92328 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
@@ -2306,6 +2345,11 @@ bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE , if (!die) return false; + ParsedDWARFTypeAttributes attrs(die); ZequanWu wrote: The parsing happens every time when constructing this object, which makes it a bit expensive, should we add a new field `DWARFAttributes m_attributes` in `DWARFBaseDIE`, so that we only parse it once? From a glance at calls to `DWARFBaseDIE::GetAttributes`, there are more than 10 calls to it. The attribute parsing is repetitive. https://github.com/llvm/llvm-project/pull/92328 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
@@ -321,6 +326,10 @@ class SymbolFileDWARFDebugMap : public SymbolFileCommon {
std::vector m_func_indexes; // Sorted by address
std::vector m_glob_indexes;
std::map>, OSOInfoSP>
m_oso_map;
+ // A map from CompilerType to the struct/class/union/enum DIE (might be a
+ // declaration or a definition) that is used to construct it.
+ llvm::DenseMap
+ m_forward_decl_compiler_type_to_die;
ZequanWu wrote:
TL;DR: This is because this change let `UniqueDWARFASTTypeMap` to cache created
Type from declaration DIE. Before this, it was only used for caching Type
created from definition DIE. And `UniqueDWARFASTTypeMap` is shared among all
`SymbolFileDWARF`s belongs to one `SymbolFileDWARFDebugMap`, so should
`m_forward_decl_compiler_type_to_die` which interacts with it.
Here's an example with debug map used:
The declaration DIE for `bar` is in foo.o and the definition DIE is in main.o.
`ParseStructureLikeDIE` was firstly asked to parse the declaration DIE.
Before, it will always find the definition DIE in main.o and insert the
CompilerType to definition DIE to `m_forward_decl_compiler_type_to_die` which
belongs to SymbolFileDWARF(main.o). When `TypeSystemClang::CompleteTagDecl`
wants to complete `bar`, it asks `SymbolFileDWARFDebugMap::CompleteType` to
complete, which iterates all its SymbolFileDWARF(main.o, foo.o) and check if
the any of them has the compiler type in their maps:
https://github.com/llvm/llvm-project/blob/9144553207052a868efc5a8ce61a0afbb0eaf236/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp#L808-L812.
If exists, then it assumes that symbol file should have the definition DIE and
able to complete it. Since `bar`'s compiler type exists in symbol
file(main.o)'s map which also has the definition DIE as value, the type
completion will success.
If I don't add the fix, we have [bar's compiler type -> bar's declaration DIE]
in foo.o's map. When searching for definition DIE, we found that in main.o.
Because we have already created its Type from declaration, the
`UniqueDWARFASTTypeMap` will find the entry. Then it updates the entry to
points to the definition DIE. It updates main.o's
`m_forward_decl_compiler_type_to_die` to pointing to the definition DIE, which
is **wrong**, since there's no such entry in main.o's map. It should update
foo.o's map. The result is that `SymbolFileDWARFDebugMap::CompleteType` find
bar's compiler type exists in foo.o and ask foo.o's symbol file to complete it,
but it only has declaration DIE.
The fix is to share one `m_forward_decl_compiler_type_to_die` among one
`SymbolFileDWARFDebugMap`. With this, when creating compiler type to
declaration DIE in the map or updating an entry to point to a definition DIE,
we are operating in the same map.
https://github.com/llvm/llvm-project/pull/92328
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
@@ -2306,6 +2345,11 @@ bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE , if (!die) return false; + ParsedDWARFTypeAttributes attrs(die); ZequanWu wrote: This extra check was added in https://github.com/llvm/llvm-project/pull/91799 to ensure we don't accidentally parse declaration DIE, which was reported at https://github.com/llvm/llvm-project/pull/90663#issuecomment-2105164917. By checking `ParsedDWARFTypeAttributes`'s constructor, looks like it just parses the DIE's attributes, iterates through them, and updates its fields accordingly. https://github.com/llvm/llvm-project/pull/92328 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
https://github.com/ZequanWu edited https://github.com/llvm/llvm-project/pull/92328 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
ZequanWu wrote: The second commit is the fix. https://github.com/llvm/llvm-project/pull/92328 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
https://github.com/ZequanWu created
https://github.com/llvm/llvm-project/pull/92328
This reapplies
https://github.com/llvm/llvm-project/commit/9a7262c2601874e5aa64c5db19746770212d4b44
and
https://github.com/llvm/llvm-project/commit/a7eff59f78f08f8ef0487dfe2a136fb311af4fd0
with a fix.
It was causing tests on macos to fail because
`SymbolFileDWARF::GetForwardDeclCompilerTypeToDIE` returned the map owned by
this symol file. When there were two symbol files, two different maps were
created for caching from compiler type to DIE even if they are for the same
module. The solution is to do the same as
`SymbolFileDWARF::GetUniqueDWARFASTTypeMap`: inquery SymbolFileDWARFDebugMap
first to get the shared underlying SymbolFile so the map is shared among
multiple SymbolFileDWARF.
>From d6de8d9a8bc709b5c4761e9a05f9befede938734 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Wed, 15 May 2024 13:58:42 -0400
Subject: [PATCH 1/2] Reapply [lldb][DWARF] Delay struct/class/union definition
DIE searching when parsing declaration DIEs.
This reapplies 9a7262c2601874e5aa64c5db19746770212d4b44 and
a7eff59f78f08f8ef0487dfe2a136fb311af4fd0 with a fix.
It was causing tests on macos to fail because
`SymbolFileDWARF::GetForwardDeclCompilerTypeToDIE` returned the map owned by
this symol file. When there were two symbol files, two different maps were
created for caching from compiler type to DIE even if they are for the same
module. The solution is to do the same as
`SymbolFileDWARF::GetUniqueDWARFASTTypeMap`: inquery SymbolFileDWARFDebugMap
first to get the shared underlying SymbolFile so the map is shared among
multiple SymbolFileDWARF.
---
.../Plugins/SymbolFile/DWARF/DWARFASTParser.h | 2 +
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 402 ++
.../SymbolFile/DWARF/DWARFASTParserClang.h| 197 -
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 44 +-
.../SymbolFile/DWARF/SymbolFileDWARF.h| 7 +-
.../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 107 ++---
.../SymbolFile/DWARF/UniqueDWARFASTType.h | 36 +-
.../delayed-definition-die-searching.test | 36 ++
8 files changed, 447 insertions(+), 384 deletions(-)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
index 66db396279e06..e144cf0f9bd94 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
@@ -60,6 +60,8 @@ class DWARFASTParser {
virtual ConstString GetDIEClassTemplateParams(const DWARFDIE ) = 0;
+ virtual lldb_private::Type *FindDefinitionTypeForDIE(const DWARFDIE ) =
0;
+
static std::optional
ParseChildArrayInfo(const DWARFDIE _die,
const ExecutionContext *exe_ctx = nullptr);
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index f8101aba5c627..2a46be9216121 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -154,6 +154,26 @@ static bool TagIsRecordType(dw_tag_t tag) {
}
}
+static bool IsForwardDeclaration(const DWARFDIE ,
+ const ParsedDWARFTypeAttributes ,
+ LanguageType cu_language) {
+ if (attrs.is_forward_declaration)
+return true;
+
+ // Work around an issue with clang at the moment where forward
+ // declarations for objective C classes are emitted as:
+ // DW_TAG_structure_type [2]
+ // DW_AT_name( "ForwardObjcClass" )
+ // DW_AT_byte_size( 0x00 )
+ // DW_AT_decl_file( "..." )
+ // DW_AT_decl_line( 1 )
+ //
+ // Note that there is no DW_AT_declaration and there are no children,
+ // and the byte size is zero.
+ return attrs.byte_size && *attrs.byte_size == 0 && attrs.name &&
+ !die.HasChildren() && cu_language == eLanguageTypeObjC;
+}
+
TypeSP DWARFASTParserClang::ParseTypeFromClangModule(const SymbolContext ,
const DWARFDIE ,
Log *log) {
@@ -249,11 +269,9 @@ static void ForcefullyCompleteType(CompilerType type) {
/// This function serves a similar purpose as RequireCompleteType above, but it
/// avoids completing the type if it is not immediately necessary. It only
/// ensures we _can_ complete the type later.
-static void PrepareContextToReceiveMembers(TypeSystemClang ,
- ClangASTImporter _importer,
- clang::DeclContext *decl_ctx,
- DWARFDIE die,
- const char *type_name_cstr) {
+void DWARFASTParserClang::PrepareContextToReceiveMembers(
+clang::DeclContext *decl_ctx,
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: It actually still crashes at the same place even without this PR using the following command, you can try it on trunk: ``` $ rm -rf lldb/test/API/commands/expression/import-std-module/lldb-api/* $ out/cmake/bin/lldb-dotest --lldb-module-cache-dir=out/cmake/lldb-test-build.noindex/module-cache-lldb/lldb-api lldb/test/API/commands/expression/import-std-module/ /Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.9/bin/python3.9 /Users/zequanwu/work/llvm/lldb/test/API/dotest.py --arch arm64 --build-dir /Users/zequanwu/work/llvm/out/cmake/lldb-test-build.noindex --executable /Users/zequanwu/work/llvm/out/cmake/./bin/lldb --compiler /Users/zequanwu/work/llvm/out/cmake/./bin/clang --dsymutil /Users/zequanwu/work/llvm/out/cmake/./bin/dsymutil --lldb-libs-dir /Users/zequanwu/work/llvm/out/cmake/./lib --llvm-tools-dir /Users/zequanwu/work/llvm/out/cmake/./bin --libcxx-include-dir /Users/zequanwu/work/llvm/out/cmake/include/c++/v1 --libcxx-library-dir /Users/zequanwu/work/llvm/out/cmake/lib --lldb-obj-root /Users/zequanwu/work/llvm/out/cmake/tools/lldb --lldb-module-cache-dir=/Users/zequanwu/work/llvm/out/cmake/lldb-test-build.noindex/module-cache-lldb/lldb-api lldb/test/API/commands/expression/import-std-module/ lldb version 19.0.0git (g...@github.com:ZequanWu/llvm-project.git revision 71fbbb69d63c461f391cbabf1e32cd9977c4ce68) clang revision 71fbbb69d63c461f391cbabf1e32cd9977c4ce68 llvm revision 71fbbb69d63c461f391cbabf1e32cd9977c4ce68 Skipping the following test categories: ['libstdcxx', 'dwo', 'llgs', 'fork'] PASS: LLDB (/Users/zequanwu/work/llvm/out/cmake/bin/clang-arm64) :: test_dsym (TestDbgInfoContentForwardListFromStdModule.TestDbgInfoContentForwardList) PASS: LLDB (/Users/zequanwu/work/llvm/out/cmake/bin/clang-arm64) :: test_dwarf (TestDbgInfoContentForwardListFromStdModule.TestDbgInfoContentForwardList) UNSUPPORTED: LLDB (/Users/zequanwu/work/llvm/out/cmake/bin/clang-arm64) :: test_dwo (TestDbgInfoContentForwardListFromStdModule.TestDbgInfoContentForwardList) (test case does not fall in any category of interest for this run) PASS: LLDB (/Users/zequanwu/work/llvm/out/cmake/bin/clang-arm64) :: test_dsym (TestForwardListFromStdModule.TestBasicForwardList) PASS: LLDB (/Users/zequanwu/work/llvm/out/cmake/bin/clang-arm64) :: test_dwarf (TestForwardListFromStdModule.TestBasicForwardList) UNSUPPORTED: LLDB (/Users/zequanwu/work/llvm/out/cmake/bin/clang-arm64) :: test_dwo (TestForwardListFromStdModule.TestBasicForwardList) (test case does not fall in any category of interest for this run) PASS: LLDB (/Users/zequanwu/work/llvm/out/cmake/bin/clang-arm64) :: test_dsym (TestRetryWithStdModule.TestCase) PASS: LLDB (/Users/zequanwu/work/llvm/out/cmake/bin/clang-arm64) :: test_dwarf (TestRetryWithStdModule.TestCase) UNSUPPORTED: LLDB (/Users/zequanwu/work/llvm/out/cmake/bin/clang-arm64) :: test_dwo (TestRetryWithStdModule.TestCase) (test case does not fall in any category of interest for this run) PASS: LLDB (/Users/zequanwu/work/llvm/out/cmake/bin/clang-arm64) :: test_dsym (TestSharedPtrFromStdModule.TestSharedPtr) PASS: LLDB (/Users/zequanwu/work/llvm/out/cmake/bin/clang-arm64) :: test_dwarf (TestSharedPtrFromStdModule.TestSharedPtr) UNSUPPORTED: LLDB (/Users/zequanwu/work/llvm/out/cmake/bin/clang-arm64) :: test_dwo (TestSharedPtrFromStdModule.TestSharedPtr) (test case does not fall in any category of interest for this run) PASS: LLDB (/Users/zequanwu/work/llvm/out/cmake/bin/clang-arm64) :: test_dsym (TestEmptyStdModule.ImportStdModule) PASS: LLDB (/Users/zequanwu/work/llvm/out/cmake/bin/clang-arm64) :: test_dwarf (TestEmptyStdModule.ImportStdModule) UNSUPPORTED: LLDB (/Users/zequanwu/work/llvm/out/cmake/bin/clang-arm64) :: test_dwo (TestEmptyStdModule.ImportStdModule) (test case does not fall in any category of interest for this run) Assertion failed: (0 && "Invalid SLocOffset or bad function choice"), function getFileIDLoaded, file SourceManager.cpp, line 865. PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace. ``` But I don't know why this change will make this crash explicitly shows up when running check-lldb. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote:
I had a fix to this: Let `SymbolFileDWARF::GetForwardDeclCompilerTypeToDIE` do
the same as `SymbolFileDWARF::GetUniqueDWARFASTTypeMap`: inquery
SymbolFileDWARFDebugMap first to get the shared underlying SymbolFile so the
map is shared among multiple SymbolFileDWARF. It's here:
https://github.com/ZequanWu/llvm-project/commit/2172c660821e273205e7ad3a64eb7f3623b21606
It fixes those failed tests shown on the macos bot. However, I noticed that
lldb crashes on three tests related to clang module (they also crashes when the
fix is not given, but not crash after reverting this PR):
```
Unresolved Tests (3):
lldb-api ::
commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
lldb-api ::
commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
lldb-api ::
commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
```
I found it's the following commands causing crash.
```
$ out/cmake/bin/lldb
out/cmake/lldb-test-build.noindex/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.test_dwarf/a.out
-o "settings set symbols.clang-modules-cache-path
/Users/zequanwu/work/llvm/out/cmake/lldb-test-build.noindex/module-cache-lldb/lldb-api"
-o "settings set target.import-std-module true" -o "b 9" -o "r" -o "expr a"
(lldb) target create
"../cmake/lldb-test-build.noindex/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.test_dwarf/a.out"
Current executable set to
'/Users/zequanwu/work/llvm/out/cmake/lldb-test-build.noindex/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.test_dwarf/a.out'
(arm64).
(lldb) settings set symbols.clang-modules-cache-path
/Users/zequanwu/work/llvm/out/cmake/lldb-test-build.noindex/module-cache-lldb/lldb-api
(lldb) settings set target.import-std-module true
(lldb) b 9
Breakpoint 1: where = a.out`main + 104 at main.cpp:9:3, address =
0x00012508
(lldb) r
Process 12273 launched:
'/Users/zequanwu/work/llvm/out/cmake/lldb-test-build.noindex/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.test_dwarf/a.out'
(arm64)
Process 12273 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
frame #0: 0x00012508 a.out`main(argc=1, argv=0x00016fdff428) at
main.cpp:9:3
6
7int main(int argc, char **argv) {
8 std::vector a = {{3}, {1}, {2}};
-> 9 return 0; // Set break point at this line.
10 }
(lldb) expr a
Assertion failed: (0 && "Invalid SLocOffset or bad function choice"), function
getFileIDLoaded, file SourceManager.cpp, line 865.
LLDB diagnostics will be written to
/var/folders/jf/zylbx28s05n0d_xwqdf5jnrc00lnhs/T/diagnostics-512963
Please include the directory content when filing a bug report
[1]12267 abort bin/lldb -o -o "settings set target.import-std-module
true" -o "b 9" -o "r"
```
The clang module in
`out/cmake/lldb-test-build.noindex/module-cache-lldb/lldb-api` was created when
running `check-lldb`. If I delete the clang modules in that directory and run
the above command again, it no longer crashes and able to give correct result
(after the first run, a new clang module is created in the directory. Following
runs of the above commands no longer crashes). So, it looks like related to
stale clang module. If I use debug built lldb, it no longer crashes. Do you
have any idea how to debug this crash? I'm not familiar with how clang module
interact with type completion etc.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: > Reverting those two commits seems to have caused this build failure on Ubuntu: You forgot the delete the newly added test SymbolFile/DWARF/delayed-definition-die-searching.test in the reverting: https://github.com/llvm/llvm-project/commit/37b8e5feb1d065a7c474e6595bac6d2f65faeb51 https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: > your commit deleted that file I think, I added it back when I did the revert > (possibly a mistake)... It passes on my macOS system but is failing on > Ubuntu after the revert. I think I'll just disable it for now. This change adds the new test, so deleting it as part of the reverting is expected. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: > I was able to reproduce the failure of these three: > > lldb-api :: lang/c/forward/TestForwardDeclaration.py > lldb-api :: lang/cpp/unique-types3/TestUniqueTypes3.py > lldb-api :: types/TestRecursiveTypes.py > > locally. Reverting this patch and > https://github.com/llvm/llvm-project/commit/a7eff59f78f08f8ef0487dfe2a136fb311af4fd0 > which depended on it made the failure go away. > > I reverted the patches for now to get the bots clean. Thanks. Can you provide instructions to repro the failure locally? > @ZequanWu in the future, if one of your commits break a bot, make sure to > revert it immediately, you can always re-land it later with a fix or an > explanation why it wasn't your commit that broke the bots. Reverting a commit > is cheap, red bots are expensive :-) Will do. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: > > Could this commit have broken the bots? > > https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake/1897/ > > Looks like so, but I cannot repro the test failure locally. The error message is different in current latest build (https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake/1907/testReport/junit/lldb-api/lang_c_forward/TestForwardDeclaration_py/) which makes me thinking it's related to clang module being out of date? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: > Could this commit have broken the bots? > https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake/1897/ Looks like so, but I cannot repro the test failure locally. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Do not complete type from declaration die. (PR #91799)
https://github.com/ZequanWu closed https://github.com/llvm/llvm-project/pull/91799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Do not complete type from declaration die. (PR #91799)
https://github.com/ZequanWu updated https://github.com/llvm/llvm-project/pull/91799 >From 1f6bf890dbfce07b6ab531597e876ab83cfd1298 Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Fri, 10 May 2024 16:00:22 -0400 Subject: [PATCH 1/2] [lldb][DWARF] Do not complete type from declaration die. --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index e0b1b430b266f..315ba4cc0ccdf 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2343,7 +2343,10 @@ bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE , // clang::ExternalASTSource queries for this type. m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), false); - if (!die) + ParsedDWARFTypeAttributes attrs(die); + bool is_forward_declaration = IsForwardDeclaration( + die, attrs, SymbolFileDWARF::GetLanguage(*die.GetCU())); + if (!die || is_forward_declaration) return false; const dw_tag_t tag = die.Tag(); >From 3e2791d99be0a21351ee4bf57beaf2c30ba5256e Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Fri, 10 May 2024 16:38:54 -0400 Subject: [PATCH 2/2] address comment --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 315ba4cc0ccdf..2a46be9216121 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2343,10 +2343,12 @@ bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE , // clang::ExternalASTSource queries for this type. m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), false); + if (!die) +return false; ParsedDWARFTypeAttributes attrs(die); bool is_forward_declaration = IsForwardDeclaration( die, attrs, SymbolFileDWARF::GetLanguage(*die.GetCU())); - if (!die || is_forward_declaration) + if (is_forward_declaration) return false; const dw_tag_t tag = die.Tag(); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Do not complete type from declaration die. (PR #91799)
ZequanWu wrote: > To the above function to ensure we don't waste any time trying to parse any > DIE information for forward declaration when using .debug_names. This will > cause a TON of churn on our DWARF parser and cause us to pull in and parse > way too much. That sounds like a better fix. https://github.com/llvm/llvm-project/pull/91799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: I sent an alternative fix at https://github.com/llvm/llvm-project/pull/91799. > The .debug_names spec states that only entries with definitions should be in > the .debug_names table... Do it mean the .debug_names is implemented incorrectly? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Do not complete type from declaration die. (PR #91799)
https://github.com/ZequanWu created https://github.com/llvm/llvm-project/pull/91799 Fix the problem: https://github.com/llvm/llvm-project/pull/90663#issuecomment-2105164917 by enhancing a double-check for #90663 >From 1f6bf890dbfce07b6ab531597e876ab83cfd1298 Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Fri, 10 May 2024 16:00:22 -0400 Subject: [PATCH] [lldb][DWARF] Do not complete type from declaration die. --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index e0b1b430b266f..315ba4cc0ccdf 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2343,7 +2343,10 @@ bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE , // clang::ExternalASTSource queries for this type. m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), false); - if (!die) + ParsedDWARFTypeAttributes attrs(die); + bool is_forward_declaration = IsForwardDeclaration( + die, attrs, SymbolFileDWARF::GetLanguage(*die.GetCU())); + if (!die || is_forward_declaration) return false; const dw_tag_t tag = die.Tag(); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: > The issue might arise from having a .debug_names table that has DW_IDX_parent > entries that means that there might be forward declarations included in the > DWARF index. Do you mean that the searching in the type index returns a declaration DIE (but I expected it to always return a definition DIE if exists: https://github.com/llvm/llvm-project/blob/91feb130d5cd3cafce94bbaf7ad67d1542623a75/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L3135)? If that's the case, we get a type created from declaration DIE `SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext`, which makes it falls through the check. A quick fix I have in mind is to do a double-check if the dwarf_die is a declaration or not before: https://github.com/llvm/llvm-project/blob/d8e73752a5f4f79ef4293d8f446c03062010233d/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L1659-L1661, so we won't complete it. But if there truly exists a definition DIE, it will never be completed because it relies on index to find definition DIE. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: > So this DIE is just a declaration. Shouldn't this code have tried to find a > non declaration DIE for "std::ios_base"? Yes, I suppose `SymbolFileDWARF::CompleteType` will try to find the definition DIE for it before calling `DWARFASTParserClang::CompleteTypeFromDWARF`. If the definition DIE is not found, it's expected to do an early exit because Type is nullptr: https://github.com/llvm/llvm-project/blob/d8e73752a5f4f79ef4293d8f446c03062010233d/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L1643-L1644. If found, we should have the definition DIE in the GetForwardDeclCompilerTypeToDIE map: https://github.com/llvm/llvm-project/blob/d8e73752a5f4f79ef4293d8f446c03062010233d/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L1646-L1651 Can you confirm that there exists a definition DIE for it? How does it look like? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu closed https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Sort ranges list in dwarf 5. (PR #91343)
https://github.com/ZequanWu closed https://github.com/llvm/llvm-project/pull/91343 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -24,13 +24,16 @@ class UniqueDWARFASTType {
UniqueDWARFASTType() : m_type_sp(), m_die(), m_declaration() {}
UniqueDWARFASTType(lldb::TypeSP _sp, const DWARFDIE ,
- const Declaration , int32_t byte_size)
+ const Declaration , int32_t byte_size,
+ bool is_forward_declaration)
: m_type_sp(type_sp), m_die(die), m_declaration(decl),
-m_byte_size(byte_size) {}
+m_byte_size(byte_size),
+m_is_forward_declaration(is_forward_declaration) {}
ZequanWu wrote:
I just removed this dead ctor.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -0,0 +1,40 @@
+# Test definition DIE searching is delayed until complete type is required.
+
+# RUN: split-file %s %t
+# RUN: %clangxx_host %t/main.cpp %t/t1_def.cpp -g -gsimple-template-names -o
%t.out
+# RUN: %lldb -b %t.out -s %t/lldb.cmd | FileCheck %s
+
+# CHECK: (lldb) p v1
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type
(DW_TAG_structure_type) name = 't2'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type
(DW_TAG_structure_type) name = 't1'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type
(DW_TAG_base_type) name = 'int'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't2' resolving forward
declaration...
+# CHECK: (t2 >) {}
+# CHECK: (lldb) p v2
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type
(DW_TAG_structure_type) name = 't1'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type
(DW_TAG_base_type) name = 'int'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't1' resolving forward
declaration...
+# CHECK: (t1) {}
+
+#--- lldb.cmd
ZequanWu wrote:
Yeah, that would work. I have an example at
https://discourse.llvm.org/t/rfc-delay-definition-die-searching-when-parse-a-declaration-die-for-record-type/78526/15.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Sort ranges list in dwarf 5. (PR #91343)
https://github.com/ZequanWu edited https://github.com/llvm/llvm-project/pull/91343 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -0,0 +1,40 @@
+# Test definition DIE searching is delayed until complete type is required.
+
+# RUN: split-file %s %t
+# RUN: %clangxx_host %t/main.cpp %t/t1_def.cpp -g -gsimple-template-names -o
%t.out
+# RUN: %lldb -b %t.out -s %t/lldb.cmd | FileCheck %s
+
+# CHECK: (lldb) p v1
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type
(DW_TAG_structure_type) name = 't2'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type
(DW_TAG_structure_type) name = 't1'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type
(DW_TAG_base_type) name = 'int'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't2' resolving forward
declaration...
+# CHECK: (t2 >) {}
+# CHECK: (lldb) p v2
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type
(DW_TAG_structure_type) name = 't1'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type
(DW_TAG_base_type) name = 'int'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't1' resolving forward
declaration...
+# CHECK: (t1) {}
+
+#--- lldb.cmd
ZequanWu wrote:
I tried something like this:
```
struct Bar {
int x;
Bar(int x): x(x) {}
};
struct Foo {
Bar* bar;
Foo(int x) {
bar = new Bar(x);
}
};
```
In another CU, print variable with type `Foo*`, but it still tries to complete
both `Foo` and `Bar` because the looking-up child count through a pointer
issue.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -24,13 +24,16 @@ class UniqueDWARFASTType {
UniqueDWARFASTType() : m_type_sp(), m_die(), m_declaration() {}
UniqueDWARFASTType(lldb::TypeSP _sp, const DWARFDIE ,
- const Declaration , int32_t byte_size)
+ const Declaration , int32_t byte_size,
+ bool is_forward_declaration)
: m_type_sp(type_sp), m_die(die), m_declaration(decl),
-m_byte_size(byte_size) {}
+m_byte_size(byte_size),
+m_is_forward_declaration(is_forward_declaration) {}
ZequanWu wrote:
It's never used. It first gets assigned at
https://github.com/llvm/llvm-project/pull/90663/files#diff-5dd6736e4d6c38623630df16c4494c1a8b099716ee0f05c9af54b4bafb1d864eR1929
then updated to `false` at
https://github.com/llvm/llvm-project/pull/90663/files#diff-5dd6736e4d6c38623630df16c4494c1a8b099716ee0f05c9af54b4bafb1d864eR1808
when we find definition DIE.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/90663
>From 4e83099b593e66f12dc21be5fbac5279e03e Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Tue, 30 Apr 2024 16:23:11 -0400
Subject: [PATCH 1/8] [lldb][DWARF] Delay struct/class/union definition DIE
searching when parsing declaration DIEs.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 270 +++---
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 105 ++-
.../SymbolFile/DWARF/SymbolFileDWARF.h| 14 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 5 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.h | 2 +
.../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 95 +++---
.../SymbolFile/DWARF/UniqueDWARFASTType.h | 21 +-
7 files changed, 296 insertions(+), 216 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index bea11e0e3840a..7ad661c9a9d49 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -252,7 +252,8 @@ static void ForcefullyCompleteType(CompilerType type) {
static void PrepareContextToReceiveMembers(TypeSystemClang ,
ClangASTImporter _importer,
clang::DeclContext *decl_ctx,
- DWARFDIE die,
+ const DWARFDIE _ctx_die,
+ const DWARFDIE ,
const char *type_name_cstr) {
auto *tag_decl_ctx = clang::dyn_cast(decl_ctx);
if (!tag_decl_ctx)
@@ -279,6 +280,13 @@ static void PrepareContextToReceiveMembers(TypeSystemClang
,
type_name_cstr ? type_name_cstr : "", die.GetOffset());
}
+ // By searching for the definition DIE of the decl_ctx type, we will either:
+ // 1. Found the the definition DIE and start its definition with
+ // TypeSystemClang::StartTagDeclarationDefinition.
+ // 2. Unable to find it, then need to forcefully complete it.
+ die.GetDWARF()->FindDefinitionDIE(decl_ctx_die);
+ if (tag_decl_ctx->isCompleteDefinition() || tag_decl_ctx->isBeingDefined())
+return;
// We don't have a type definition and/or the import failed. We must
// forcefully complete the type to avoid crashes.
ForcefullyCompleteType(type);
@@ -620,10 +628,11 @@ DWARFASTParserClang::ParseTypeModifier(const
SymbolContext ,
if (tag == DW_TAG_typedef) {
// DeclContext will be populated when the clang type is materialized in
// Type::ResolveCompilerType.
-PrepareContextToReceiveMembers(
-m_ast, GetClangASTImporter(),
-GetClangDeclContextContainingDIE(die, nullptr), die,
-attrs.name.GetCString());
+DWARFDIE decl_ctx_die;
+clang::DeclContext *decl_ctx =
+GetClangDeclContextContainingDIE(die, _ctx_die);
+PrepareContextToReceiveMembers(m_ast, GetClangASTImporter(), decl_ctx,
+ decl_ctx_die, die, attrs.name.GetCString());
if (attrs.type.IsValid()) {
// Try to parse a typedef from the (DWARF embedded in the) Clang
@@ -1100,32 +1109,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
// struct and see if this is actually a C++ method
Type *class_type = dwarf->ResolveType(decl_ctx_die);
if (class_type) {
- if (class_type->GetID() != decl_ctx_die.GetID() ||
- IsClangModuleFwdDecl(decl_ctx_die)) {
-
-// We uniqued the parent class of this function to another
-// class so we now need to associate all dies under
-// "decl_ctx_die" to DIEs in the DIE for "class_type"...
-DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID());
-
-if (class_type_die) {
- std::vector failures;
-
- CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die,
- class_type, failures);
-
- // FIXME do something with these failures that's
- // smarter than just dropping them on the ground.
- // Unfortunately classes don't like having stuff added
- // to them after their definitions are complete...
-
- Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
- if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
-return type_ptr->shared_from_this();
- }
-}
- }
-
if (attrs.specification.IsValid()) {
// We have a specification which we are going to base our
// function prototype off of, so we need this type to be
@@ -1260,6 +1243,39 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
}
}
}
+ // By here, we should have already completed the c++ class_type
+ //
[Lldb-commits] [lldb] [lldb][DWARF] Sort ranges list in dwarf 5. (PR #91343)
ZequanWu wrote:
> > For example:
> > https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L922-L927.
> > When parsing function info, it validates low and hi address of the
> > function: `GetMinRangeBase` returns the first range entry base and
> > `GetMaxRangeEnd` returns the last range end. If low >= hi, it stops parsing
> > this function.
> > This causes missing inline stack frames for us when debugging a core dump.
>
> Gotcha, makes sense. Since you know how this fails, could you add a test to
> make sure that scenario continues working?
Yeah, I updated the test
`lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s` so that two ranges in
`.debug_rnglists` are out of order and `image lookup -v -s lookup_rnglists` is
still able to produce sorted ranges for the inner block:
Without this change:
```
Blocks: id = {0x0030}, range = [0x-0x0004)
Symbol: id = {0x0003}, range =
[0x0001-0x0004), name="lookup_rnglists"
```
With this change:
```
Blocks: id = {0x0030}, range = [0x-0x0004)
id = {0x0046}, ranges =
[0x0001-0x0002)[0x0003-0x0004)
Symbol: id = {0x0003}, range =
[0x0001-0x0004), name="lookup_rnglists"
```
https://github.com/llvm/llvm-project/pull/91343
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Sort ranges list in dwarf 5. (PR #91343)
ZequanWu wrote: > > Some places assume the ranges are already sorted but it's not. This fixes > > it. > > What places assumed it was sorted? I assume this means there are some bugs > that this fixes? Can you give some more details about how you came to write > this patch? For example: https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L922-L927. When parsing function info, it validates low and hi address of the function: `GetMinRangeBase` returns the first range entry base and `GetMaxRangeEnd` returns the last range end. If low >= hi, it stops parsing this function. This causes missing inline stack frames for us when debugging a core dump. https://github.com/llvm/llvm-project/pull/91343 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: Will leave it open for few days in case anyone has more comments. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu edited https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -0,0 +1,40 @@
+# Test definition DIE searching is delayed until complete type is required.
+
+# RUN: split-file %s %t
+# RUN: %clangxx_host %t/main.cpp %t/t1_def.cpp -g -gsimple-template-names -o
%t.out
+# RUN: %lldb -b %t.out -s %t/lldb.cmd | FileCheck %s
+
+# CHECK: (lldb) p v1
+# CHECK-NEXT:
DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type
(DW_TAG_structure_type) name = 't2'
ZequanWu wrote:
Updated to use `CHECK`. Unfortunately, it doesn't work with `image dump ast`
because this change doesn't affect when type completion happens. So, after `p
v1`, w/wo this change, lldb just creates `ClassTemplateSpecializationDecl` for
`t1` with no definition. When doing `p v2', lldb tries to complete the
`t1` decl. This change only moves the definition searching to the time
when completing a type.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/90663
>From 4e83099b593e66f12dc21be5fbac5279e03e Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Tue, 30 Apr 2024 16:23:11 -0400
Subject: [PATCH 1/7] [lldb][DWARF] Delay struct/class/union definition DIE
searching when parsing declaration DIEs.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 270 +++---
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 105 ++-
.../SymbolFile/DWARF/SymbolFileDWARF.h| 14 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 5 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.h | 2 +
.../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 95 +++---
.../SymbolFile/DWARF/UniqueDWARFASTType.h | 21 +-
7 files changed, 296 insertions(+), 216 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index bea11e0e3840a..7ad661c9a9d49 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -252,7 +252,8 @@ static void ForcefullyCompleteType(CompilerType type) {
static void PrepareContextToReceiveMembers(TypeSystemClang ,
ClangASTImporter _importer,
clang::DeclContext *decl_ctx,
- DWARFDIE die,
+ const DWARFDIE _ctx_die,
+ const DWARFDIE ,
const char *type_name_cstr) {
auto *tag_decl_ctx = clang::dyn_cast(decl_ctx);
if (!tag_decl_ctx)
@@ -279,6 +280,13 @@ static void PrepareContextToReceiveMembers(TypeSystemClang
,
type_name_cstr ? type_name_cstr : "", die.GetOffset());
}
+ // By searching for the definition DIE of the decl_ctx type, we will either:
+ // 1. Found the the definition DIE and start its definition with
+ // TypeSystemClang::StartTagDeclarationDefinition.
+ // 2. Unable to find it, then need to forcefully complete it.
+ die.GetDWARF()->FindDefinitionDIE(decl_ctx_die);
+ if (tag_decl_ctx->isCompleteDefinition() || tag_decl_ctx->isBeingDefined())
+return;
// We don't have a type definition and/or the import failed. We must
// forcefully complete the type to avoid crashes.
ForcefullyCompleteType(type);
@@ -620,10 +628,11 @@ DWARFASTParserClang::ParseTypeModifier(const
SymbolContext ,
if (tag == DW_TAG_typedef) {
// DeclContext will be populated when the clang type is materialized in
// Type::ResolveCompilerType.
-PrepareContextToReceiveMembers(
-m_ast, GetClangASTImporter(),
-GetClangDeclContextContainingDIE(die, nullptr), die,
-attrs.name.GetCString());
+DWARFDIE decl_ctx_die;
+clang::DeclContext *decl_ctx =
+GetClangDeclContextContainingDIE(die, _ctx_die);
+PrepareContextToReceiveMembers(m_ast, GetClangASTImporter(), decl_ctx,
+ decl_ctx_die, die, attrs.name.GetCString());
if (attrs.type.IsValid()) {
// Try to parse a typedef from the (DWARF embedded in the) Clang
@@ -1100,32 +1109,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
// struct and see if this is actually a C++ method
Type *class_type = dwarf->ResolveType(decl_ctx_die);
if (class_type) {
- if (class_type->GetID() != decl_ctx_die.GetID() ||
- IsClangModuleFwdDecl(decl_ctx_die)) {
-
-// We uniqued the parent class of this function to another
-// class so we now need to associate all dies under
-// "decl_ctx_die" to DIEs in the DIE for "class_type"...
-DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID());
-
-if (class_type_die) {
- std::vector failures;
-
- CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die,
- class_type, failures);
-
- // FIXME do something with these failures that's
- // smarter than just dropping them on the ground.
- // Unfortunately classes don't like having stuff added
- // to them after their definitions are complete...
-
- Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
- if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
-return type_ptr->shared_from_this();
- }
-}
- }
-
if (attrs.specification.IsValid()) {
// We have a specification which we are going to base our
// function prototype off of, so we need this type to be
@@ -1260,6 +1243,39 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
}
}
}
+ // By here, we should have already completed the c++ class_type
+ //
[Lldb-commits] [lldb] [lldb][DWARF] Sort ranges list in dwarf 5. (PR #91343)
https://github.com/ZequanWu created
https://github.com/llvm/llvm-project/pull/91343
Dwarf 5 says "There is no requirement that the entries be ordered in any
particular way" in 2.17.3 Non-Contiguous Address Ranges for rnglist. Some
places assume the ranges are already sorted but it's not. This fixes it.
>From eb1b2faa457522e7c11fbd69acea7f7d736c7c74 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Tue, 7 May 2024 10:52:59 -0400
Subject: [PATCH] [lldb][DWARF] Sort ranges list in dwarf 5.
---
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp| 1 +
lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s | 6 +++---
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index dabc595427df..3a57ec970b07 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -1062,6 +1062,7 @@ DWARFUnit::FindRnglistFromOffset(dw_offset_t offset) {
ranges.Append(DWARFRangeList::Entry(llvm_range.LowPC,
llvm_range.HighPC - llvm_range.LowPC));
}
+ ranges.Sort();
return ranges;
}
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s
b/lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s
index 89b5d94c68c3..af8a1796f3ab 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s
@@ -124,12 +124,12 @@ lookup_rnglists2:
.Lrnglists_table_base0:
.long .Ldebug_ranges0-.Lrnglists_table_base0
.Ldebug_ranges0:
-.byte 4 # DW_RLE_offset_pair
-.uleb128 .Lblock1_begin-rnglists # starting offset
-.uleb128 .Lblock1_end-rnglists# ending offset
.byte 4 # DW_RLE_offset_pair
.uleb128 .Lblock2_begin-rnglists # starting offset
.uleb128 .Lblock2_end-rnglists# ending offset
+.byte 4 # DW_RLE_offset_pair
+.uleb128 .Lblock1_begin-rnglists # starting offset
+.uleb128 .Lblock1_end-rnglists# ending offset
.byte 0 # DW_RLE_end_of_list
.Ldebug_rnglist_table_end0:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: > You could enable logging and check for specific logging after steps. In the > test I described above if you just print the "Foo *foo" variable, it won't > need to complete the definition, you could check for logging, and then if you > print "*foo", then it should complete the definition and you should see > logging. Not sure if that is what you meant Added a test with simple template names. The example you gave doesn't delay definition DIE searching because even when `p foo`, lldb calls `ValueObject::GetNumChildren` which requires complete type, which I think is also a point of unnecessary operation. The `ValueObjectPrinter::PrintChildrenIfNeeded` below ultimately calls `GetNumChildren`: https://github.com/llvm/llvm-project/blob/1241e7692a466ceb420be2780f1c3e8bbab7d469/lldb/source/DataFormatters/ValueObjectPrinter.cpp#L90-L94. I don't see a setting to disable this. If we could avoid this, this patch could further reduce unnecessary definition DIE searching when user just want to print the value of a pointer (declaration DIE should be enough). https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1667,13 +1791,40 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext ,
}
if (dwarf->GetUniqueDWARFASTTypeMap().Find(
-unique_typename, die, unique_decl, attrs.byte_size.value_or(-1),
-*unique_ast_entry_up)) {
+unique_typename, die, unique_decl, byte_size,
+attrs.is_forward_declaration, *unique_ast_entry_up)) {
type_sp = unique_ast_entry_up->m_type_sp;
if (type_sp) {
dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
LinkDeclContextToDIE(
GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die), die);
+if (!attrs.is_forward_declaration) {
+ // If the DIE being parsed in this function is a definition and the
+ // entry in the map is a declaration, then we need to update the
entry
+ // to point to the definition DIE.
+ if (unique_ast_entry_up->m_is_forward_declaration) {
+unique_ast_entry_up->m_die = die;
+unique_ast_entry_up->m_byte_size = byte_size;
+unique_ast_entry_up->m_declaration = unique_decl;
+unique_ast_entry_up->m_is_forward_declaration = false;
+// Need to update Type ID to refer to the definition DIE. because
+// it's used in ParseSubroutine to determine if we need to copy cxx
+// method types from a declaration DIE to this definition DIE.
+type_sp->SetID(die.GetID());
+clang_type = type_sp->GetForwardCompilerType();
+if (attrs.class_language != eLanguageTypeObjC &&
+attrs.class_language != eLanguageTypeObjC_plus_plus)
+ TypeSystemClang::StartTagDeclarationDefinition(clang_type);
+
+CompilerType compiler_type_no_qualifiers =
+ClangUtil::RemoveFastQualifiers(clang_type);
+auto result = dwarf->GetForwardDeclCompilerTypeToDIE().try_emplace(
ZequanWu wrote:
It doesn't compile, because the value of the map is a DIERef, which doesn't
have zero-argument constructor (can only constructed with 1 or 3 arguments).
DIERef is a reference to a parsed DIE, so adding a zero-argument constructor
for it doesn't quite make sense.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/90663
>From 4e83099b593e66f12dc21be5fbac5279e03e Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Tue, 30 Apr 2024 16:23:11 -0400
Subject: [PATCH 1/6] [lldb][DWARF] Delay struct/class/union definition DIE
searching when parsing declaration DIEs.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 270 +++---
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 105 ++-
.../SymbolFile/DWARF/SymbolFileDWARF.h| 14 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 5 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.h | 2 +
.../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 95 +++---
.../SymbolFile/DWARF/UniqueDWARFASTType.h | 21 +-
7 files changed, 296 insertions(+), 216 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index bea11e0e3840af..7ad661c9a9d49b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -252,7 +252,8 @@ static void ForcefullyCompleteType(CompilerType type) {
static void PrepareContextToReceiveMembers(TypeSystemClang ,
ClangASTImporter _importer,
clang::DeclContext *decl_ctx,
- DWARFDIE die,
+ const DWARFDIE _ctx_die,
+ const DWARFDIE ,
const char *type_name_cstr) {
auto *tag_decl_ctx = clang::dyn_cast(decl_ctx);
if (!tag_decl_ctx)
@@ -279,6 +280,13 @@ static void PrepareContextToReceiveMembers(TypeSystemClang
,
type_name_cstr ? type_name_cstr : "", die.GetOffset());
}
+ // By searching for the definition DIE of the decl_ctx type, we will either:
+ // 1. Found the the definition DIE and start its definition with
+ // TypeSystemClang::StartTagDeclarationDefinition.
+ // 2. Unable to find it, then need to forcefully complete it.
+ die.GetDWARF()->FindDefinitionDIE(decl_ctx_die);
+ if (tag_decl_ctx->isCompleteDefinition() || tag_decl_ctx->isBeingDefined())
+return;
// We don't have a type definition and/or the import failed. We must
// forcefully complete the type to avoid crashes.
ForcefullyCompleteType(type);
@@ -620,10 +628,11 @@ DWARFASTParserClang::ParseTypeModifier(const
SymbolContext ,
if (tag == DW_TAG_typedef) {
// DeclContext will be populated when the clang type is materialized in
// Type::ResolveCompilerType.
-PrepareContextToReceiveMembers(
-m_ast, GetClangASTImporter(),
-GetClangDeclContextContainingDIE(die, nullptr), die,
-attrs.name.GetCString());
+DWARFDIE decl_ctx_die;
+clang::DeclContext *decl_ctx =
+GetClangDeclContextContainingDIE(die, _ctx_die);
+PrepareContextToReceiveMembers(m_ast, GetClangASTImporter(), decl_ctx,
+ decl_ctx_die, die, attrs.name.GetCString());
if (attrs.type.IsValid()) {
// Try to parse a typedef from the (DWARF embedded in the) Clang
@@ -1100,32 +1109,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
// struct and see if this is actually a C++ method
Type *class_type = dwarf->ResolveType(decl_ctx_die);
if (class_type) {
- if (class_type->GetID() != decl_ctx_die.GetID() ||
- IsClangModuleFwdDecl(decl_ctx_die)) {
-
-// We uniqued the parent class of this function to another
-// class so we now need to associate all dies under
-// "decl_ctx_die" to DIEs in the DIE for "class_type"...
-DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID());
-
-if (class_type_die) {
- std::vector failures;
-
- CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die,
- class_type, failures);
-
- // FIXME do something with these failures that's
- // smarter than just dropping them on the ground.
- // Unfortunately classes don't like having stuff added
- // to them after their definitions are complete...
-
- Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
- if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
-return type_ptr->shared_from_this();
- }
-}
- }
-
if (attrs.specification.IsValid()) {
// We have a specification which we are going to base our
// function prototype off of, so we need this type to be
@@ -1260,6 +1243,39 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
}
}
}
+ // By here, we should have already completed the c++ class_type
+ //
[Lldb-commits] [lldb] [lldb-dap] Don't fail when SBProcess::GetMemoryRegionInfo returns error. (PR #87649)
https://github.com/ZequanWu closed https://github.com/llvm/llvm-project/pull/87649 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/90663
>From 4e83099b593e66f12dc21be5fbac5279e03e Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Tue, 30 Apr 2024 16:23:11 -0400
Subject: [PATCH 1/5] [lldb][DWARF] Delay struct/class/union definition DIE
searching when parsing declaration DIEs.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 270 +++---
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 105 ++-
.../SymbolFile/DWARF/SymbolFileDWARF.h| 14 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 5 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.h | 2 +
.../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 95 +++---
.../SymbolFile/DWARF/UniqueDWARFASTType.h | 21 +-
7 files changed, 296 insertions(+), 216 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index bea11e0e3840af..7ad661c9a9d49b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -252,7 +252,8 @@ static void ForcefullyCompleteType(CompilerType type) {
static void PrepareContextToReceiveMembers(TypeSystemClang ,
ClangASTImporter _importer,
clang::DeclContext *decl_ctx,
- DWARFDIE die,
+ const DWARFDIE _ctx_die,
+ const DWARFDIE ,
const char *type_name_cstr) {
auto *tag_decl_ctx = clang::dyn_cast(decl_ctx);
if (!tag_decl_ctx)
@@ -279,6 +280,13 @@ static void PrepareContextToReceiveMembers(TypeSystemClang
,
type_name_cstr ? type_name_cstr : "", die.GetOffset());
}
+ // By searching for the definition DIE of the decl_ctx type, we will either:
+ // 1. Found the the definition DIE and start its definition with
+ // TypeSystemClang::StartTagDeclarationDefinition.
+ // 2. Unable to find it, then need to forcefully complete it.
+ die.GetDWARF()->FindDefinitionDIE(decl_ctx_die);
+ if (tag_decl_ctx->isCompleteDefinition() || tag_decl_ctx->isBeingDefined())
+return;
// We don't have a type definition and/or the import failed. We must
// forcefully complete the type to avoid crashes.
ForcefullyCompleteType(type);
@@ -620,10 +628,11 @@ DWARFASTParserClang::ParseTypeModifier(const
SymbolContext ,
if (tag == DW_TAG_typedef) {
// DeclContext will be populated when the clang type is materialized in
// Type::ResolveCompilerType.
-PrepareContextToReceiveMembers(
-m_ast, GetClangASTImporter(),
-GetClangDeclContextContainingDIE(die, nullptr), die,
-attrs.name.GetCString());
+DWARFDIE decl_ctx_die;
+clang::DeclContext *decl_ctx =
+GetClangDeclContextContainingDIE(die, _ctx_die);
+PrepareContextToReceiveMembers(m_ast, GetClangASTImporter(), decl_ctx,
+ decl_ctx_die, die, attrs.name.GetCString());
if (attrs.type.IsValid()) {
// Try to parse a typedef from the (DWARF embedded in the) Clang
@@ -1100,32 +1109,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
// struct and see if this is actually a C++ method
Type *class_type = dwarf->ResolveType(decl_ctx_die);
if (class_type) {
- if (class_type->GetID() != decl_ctx_die.GetID() ||
- IsClangModuleFwdDecl(decl_ctx_die)) {
-
-// We uniqued the parent class of this function to another
-// class so we now need to associate all dies under
-// "decl_ctx_die" to DIEs in the DIE for "class_type"...
-DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID());
-
-if (class_type_die) {
- std::vector failures;
-
- CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die,
- class_type, failures);
-
- // FIXME do something with these failures that's
- // smarter than just dropping them on the ground.
- // Unfortunately classes don't like having stuff added
- // to them after their definitions are complete...
-
- Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
- if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
-return type_ptr->shared_from_this();
- }
-}
- }
-
if (attrs.specification.IsValid()) {
// We have a specification which we are going to base our
// function prototype off of, so we need this type to be
@@ -1260,6 +1243,39 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
}
}
}
+ // By here, we should have already completed the c++ class_type
+ //
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote:
> > > > > Is any of it testable?
> > > >
> > > >
> > > > Good question. Though this is mostly meant to be "NFC" (with very large
> > > > quotes), I can imagine us doing something like forcing the parsing of a
> > > > specific type (`type lookup ` ?), and then checking that the
> > > > module ast (`image dump ast`) does _not_ contain specific types -- as
> > > > that's basically what we're trying to achieve.
> > >
> > >
> > > Yea that could work. But if it's going to be very painful or fragile to
> > > test then don't let that hold back the PR
> >
> >
> > In terms of testing, since this only delays definition DIE searching not
> > type completion, we need to construct a test so that lldb finds the
> > declaration DIE first without trigger a type completion on it and somehow
> > test the incomplete type. The first part is tricky. I'm not sure how to
> > achieve it.
>
> You should be able to make a test case with two files. One file contains the
> class definition and the other uses only a forward declaration. You could
> test by running the binary stopping only in the one with the forward
> declaration. So file 1 would be something like `foo.cpp` containing
>
> ```
> struct Foo {
> int value;
> Foo(int v): value(v) {}
> };
>
> Foo *CreateFoo() {
> return new Foo(1);
> }
> ```
>
> Then having `main.cpp` contain:
>
> ```
> struct Foo; // Forward declare Foo
> // Prototypes that don't need Foo definition
> Foo *CreateFoo();
>
> int main() {
> Foo *foo = CreateFoo();
> return 0; // Breakpoint 1 here
> }
> ```
>
> Then run to "Breakpoint 1 here" and then run `frame variable foo`. Foo won't
> need to be completed for this. But if you then run `frame variable *foo`, it
> should cause the type to be completed and show the instance variable
> correctly.
The tests your described testing this change doesn't break things by delaying
definition DIE searching, which I think is already covered by existing tests
(created for other purposes, but also covers this case). I was thinking about
testing the definition DIE searching is actually delayed. Maybe there isn't a
way to do it.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: > > > Is any of it testable? > > > > > > Good question. Though this is mostly meant to be "NFC" (with very large > > quotes), I can imagine us doing something like forcing the parsing of a > > specific type (`type lookup ` ?), and then checking that the > > module ast (`image dump ast`) does _not_ contain specific types -- as > > that's basically what we're trying to achieve. > > Yea that could work. But if it's going to be very painful or fragile to test > then don't let that hold back the PR In terms of testing, since this only delays definition DIE searching not type completion, we need to construct a test so that lldb finds the declaration DIE first without trigger a type completion on it and somehow test the incomplete type. The first part is tricky. I'm not sure how to achieve it. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu edited https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu edited https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1664,13 +1793,40 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext ,
}
if (dwarf->GetUniqueDWARFASTTypeMap().Find(
-unique_typename, die, unique_decl, attrs.byte_size.value_or(-1),
-*unique_ast_entry_up)) {
+unique_typename, die, unique_decl, byte_size,
+attrs.is_forward_declaration, *unique_ast_entry_up)) {
type_sp = unique_ast_entry_up->m_type_sp;
if (type_sp) {
dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
LinkDeclContextToDIE(
GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die), die);
+if (!attrs.is_forward_declaration) {
+ // If the parameter DIE is definition and the entry in the map is
ZequanWu wrote:
Done.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1921,38 +1970,33 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext ,
GetClangASTImporter().SetRecordLayout(record_decl, layout);
}
}
-} else if (clang_type_was_created) {
- // Start the definition if the class is not objective C since the
- // underlying decls respond to isCompleteDefinition(). Objective
- // C decls don't respond to isCompleteDefinition() so we can't
- // start the declaration definition right away. For C++
- // class/union/structs we want to start the definition in case the
- // class is needed as the declaration context for a contained class
- // or type without the need to complete that type..
-
- if (attrs.class_language != eLanguageTypeObjC &&
- attrs.class_language != eLanguageTypeObjC_plus_plus)
-TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-
- // Leave this as a forward declaration until we need to know the
- // details of the type. lldb_private::Type will automatically call
- // the SymbolFile virtual function
- // "SymbolFileDWARF::CompleteType(Type *)" When the definition
- // needs to be defined.
- assert(!dwarf->GetForwardDeclCompilerTypeToDIE().count(
- ClangUtil::RemoveFastQualifiers(clang_type)
- .GetOpaqueQualType()) &&
- "Type already in the forward declaration map!");
- // Can't assume m_ast.GetSymbolFile() is actually a
- // SymbolFileDWARF, it can be a SymbolFileDWARFDebugMap for Apple
- // binaries.
- dwarf->GetForwardDeclCompilerTypeToDIE().try_emplace(
- ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType(),
- *die.GetDIERef());
- m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true);
}
+// Start the definition if the class is not objective C since the
+// underlying decls respond to isCompleteDefinition(). Objective
+// C decls don't respond to isCompleteDefinition() so we can't
+// start the declaration definition right away. For C++
+// class/union/structs we want to start the definition in case the
+// class is needed as the declaration context for a contained class
+// or type without the need to complete that type..
+
+if (attrs.class_language != eLanguageTypeObjC &&
+attrs.class_language != eLanguageTypeObjC_plus_plus)
+ TypeSystemClang::StartTagDeclarationDefinition(clang_type);
}
+ // Leave this as a forward declaration until we need to know the
ZequanWu wrote:
I didn't modify the original comments. It doesn't make sense to me as well. I
append "If this is a declaration DIE" in front of this comment.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu commented: > Though this is mostly meant to be "NFC" (with very large quotes) Yeah, this is mostly "NFC". A noticeable difference is we now set the type created from declaration with `TypeSystemClang::SetHasExternalStorage` without knowing if there's a definition or not. Based on my knowledge, it simply tells clang that the type can be completed by external AST source and ultimately calls `SymbolFileDWARF::CompleteType` to complete it. If there isn't one, we just fail complete it. Maybe we should also force completion of the type in this case? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -108,6 +108,9 @@ class DWARFASTParserClang : public
lldb_private::plugin::dwarf::DWARFASTParser {
lldb_private::ConstString GetDIEClassTemplateParams(
const lldb_private::plugin::dwarf::DWARFDIE ) override;
ZequanWu wrote:
Done.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1921,38 +1970,33 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext ,
GetClangASTImporter().SetRecordLayout(record_decl, layout);
}
}
-} else if (clang_type_was_created) {
- // Start the definition if the class is not objective C since the
- // underlying decls respond to isCompleteDefinition(). Objective
- // C decls don't respond to isCompleteDefinition() so we can't
- // start the declaration definition right away. For C++
- // class/union/structs we want to start the definition in case the
- // class is needed as the declaration context for a contained class
- // or type without the need to complete that type..
-
- if (attrs.class_language != eLanguageTypeObjC &&
- attrs.class_language != eLanguageTypeObjC_plus_plus)
-TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-
- // Leave this as a forward declaration until we need to know the
- // details of the type. lldb_private::Type will automatically call
- // the SymbolFile virtual function
- // "SymbolFileDWARF::CompleteType(Type *)" When the definition
- // needs to be defined.
- assert(!dwarf->GetForwardDeclCompilerTypeToDIE().count(
- ClangUtil::RemoveFastQualifiers(clang_type)
- .GetOpaqueQualType()) &&
- "Type already in the forward declaration map!");
- // Can't assume m_ast.GetSymbolFile() is actually a
- // SymbolFileDWARF, it can be a SymbolFileDWARFDebugMap for Apple
- // binaries.
- dwarf->GetForwardDeclCompilerTypeToDIE().try_emplace(
- ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType(),
- *die.GetDIERef());
- m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true);
}
+// Start the definition if the class is not objective C since the
+// underlying decls respond to isCompleteDefinition(). Objective
+// C decls don't respond to isCompleteDefinition() so we can't
+// start the declaration definition right away. For C++
+// class/union/structs we want to start the definition in case the
+// class is needed as the declaration context for a contained class
+// or type without the need to complete that type..
+
+if (attrs.class_language != eLanguageTypeObjC &&
+attrs.class_language != eLanguageTypeObjC_plus_plus)
+ TypeSystemClang::StartTagDeclarationDefinition(clang_type);
ZequanWu wrote:
Oh, I didn't notice it. Moved this into an else branch.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -60,6 +60,12 @@ class DWARFASTParser {
virtual ConstString GetDIEClassTemplateParams(const DWARFDIE ) = 0;
+ // Return true if we found the definition DIE for it. is_forward_declaration
+ // is set to true if the parameter die is a declaration.
+ virtual bool
+ FindDefinitionDIE(const lldb_private::plugin::dwarf::DWARFDIE ,
ZequanWu wrote:
Done.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1632,6 +1669,96 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const
DWARFDIE ) {
return qualified_name;
}
ZequanWu wrote:
Yes, it does more than finding the DIE. Updated to return `Type*` and renamed
to `FindDefinitionTypeForDIE`.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -16,60 +16,65 @@ using namespace lldb_private::plugin::dwarf;
bool UniqueDWARFASTTypeList::Find(const DWARFDIE ,
const lldb_private::Declaration ,
const int32_t byte_size,
+ bool is_forward_declaration,
UniqueDWARFASTType ) const {
for (const UniqueDWARFASTType : m_collection) {
// Make sure the tags match
if (udt.m_die.Tag() == die.Tag()) {
- // Validate byte sizes of both types only if both are valid.
- if (udt.m_byte_size < 0 || byte_size < 0 ||
- udt.m_byte_size == byte_size) {
-// Make sure the file and line match
-if (udt.m_declaration == decl) {
- // The type has the same name, and was defined on the same file and
- // line. Now verify all of the parent DIEs match.
- DWARFDIE parent_arg_die = die.GetParent();
- DWARFDIE parent_pos_die = udt.m_die.GetParent();
- bool match = true;
- bool done = false;
- while (!done && match && parent_arg_die && parent_pos_die) {
-const dw_tag_t parent_arg_tag = parent_arg_die.Tag();
-const dw_tag_t parent_pos_tag = parent_pos_die.Tag();
-if (parent_arg_tag == parent_pos_tag) {
- switch (parent_arg_tag) {
- case DW_TAG_class_type:
- case DW_TAG_structure_type:
- case DW_TAG_union_type:
- case DW_TAG_namespace: {
-const char *parent_arg_die_name = parent_arg_die.GetName();
-if (parent_arg_die_name ==
-nullptr) // Anonymous (i.e. no-name) struct
-{
+ // If they are not both definition DIEs or both declaration DIEs, then
+ // don't check for byte size and declaration location, because
declaration
+ // DIEs usually don't have those info.
+ bool matching_size_declaration =
+ udt.m_is_forward_declaration != is_forward_declaration
+ ? true
+ : (udt.m_byte_size < 0 || byte_size < 0 ||
+ udt.m_byte_size == byte_size) &&
+udt.m_declaration == decl;
+ if (matching_size_declaration) {
ZequanWu wrote:
Done.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1632,6 +1669,96 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const
DWARFDIE ) {
return qualified_name;
}
+bool DWARFASTParserClang::FindDefinitionDIE(const DWARFDIE ,
+bool _forward_declaration) {
ZequanWu wrote:
I was about to use it in SymbolFileDWARF::CompleteType:
https://github.com/llvm/llvm-project/pull/90663/files#diff-edef3a65d5d569bbb75a4158d35b827aa5d42ee03ccd3b0c1d4f354afa12210cR1645.
But with some refactor change, it's no longer useful anymore.
Removed.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -249,11 +270,10 @@ static void ForcefullyCompleteType(CompilerType type) {
/// This function serves a similar purpose as RequireCompleteType above, but it
/// avoids completing the type if it is not immediately necessary. It only
/// ensures we _can_ complete the type later.
-static void PrepareContextToReceiveMembers(TypeSystemClang ,
- ClangASTImporter _importer,
- clang::DeclContext *decl_ctx,
- DWARFDIE die,
- const char *type_name_cstr) {
+void DWARFASTParserClang::PrepareContextToReceiveMembers(
+TypeSystemClang , clang::DeclContext *decl_ctx,
ZequanWu wrote:
Done.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu edited https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/90663
>From 4e83099b593e66f12dc21be5fbac5279e03e Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Tue, 30 Apr 2024 16:23:11 -0400
Subject: [PATCH 1/4] [lldb][DWARF] Delay struct/class/union definition DIE
searching when parsing declaration DIEs.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 270 +++---
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 105 ++-
.../SymbolFile/DWARF/SymbolFileDWARF.h| 14 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 5 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.h | 2 +
.../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 95 +++---
.../SymbolFile/DWARF/UniqueDWARFASTType.h | 21 +-
7 files changed, 296 insertions(+), 216 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index bea11e0e3840af..7ad661c9a9d49b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -252,7 +252,8 @@ static void ForcefullyCompleteType(CompilerType type) {
static void PrepareContextToReceiveMembers(TypeSystemClang ,
ClangASTImporter _importer,
clang::DeclContext *decl_ctx,
- DWARFDIE die,
+ const DWARFDIE _ctx_die,
+ const DWARFDIE ,
const char *type_name_cstr) {
auto *tag_decl_ctx = clang::dyn_cast(decl_ctx);
if (!tag_decl_ctx)
@@ -279,6 +280,13 @@ static void PrepareContextToReceiveMembers(TypeSystemClang
,
type_name_cstr ? type_name_cstr : "", die.GetOffset());
}
+ // By searching for the definition DIE of the decl_ctx type, we will either:
+ // 1. Found the the definition DIE and start its definition with
+ // TypeSystemClang::StartTagDeclarationDefinition.
+ // 2. Unable to find it, then need to forcefully complete it.
+ die.GetDWARF()->FindDefinitionDIE(decl_ctx_die);
+ if (tag_decl_ctx->isCompleteDefinition() || tag_decl_ctx->isBeingDefined())
+return;
// We don't have a type definition and/or the import failed. We must
// forcefully complete the type to avoid crashes.
ForcefullyCompleteType(type);
@@ -620,10 +628,11 @@ DWARFASTParserClang::ParseTypeModifier(const
SymbolContext ,
if (tag == DW_TAG_typedef) {
// DeclContext will be populated when the clang type is materialized in
// Type::ResolveCompilerType.
-PrepareContextToReceiveMembers(
-m_ast, GetClangASTImporter(),
-GetClangDeclContextContainingDIE(die, nullptr), die,
-attrs.name.GetCString());
+DWARFDIE decl_ctx_die;
+clang::DeclContext *decl_ctx =
+GetClangDeclContextContainingDIE(die, _ctx_die);
+PrepareContextToReceiveMembers(m_ast, GetClangASTImporter(), decl_ctx,
+ decl_ctx_die, die, attrs.name.GetCString());
if (attrs.type.IsValid()) {
// Try to parse a typedef from the (DWARF embedded in the) Clang
@@ -1100,32 +1109,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
// struct and see if this is actually a C++ method
Type *class_type = dwarf->ResolveType(decl_ctx_die);
if (class_type) {
- if (class_type->GetID() != decl_ctx_die.GetID() ||
- IsClangModuleFwdDecl(decl_ctx_die)) {
-
-// We uniqued the parent class of this function to another
-// class so we now need to associate all dies under
-// "decl_ctx_die" to DIEs in the DIE for "class_type"...
-DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID());
-
-if (class_type_die) {
- std::vector failures;
-
- CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die,
- class_type, failures);
-
- // FIXME do something with these failures that's
- // smarter than just dropping them on the ground.
- // Unfortunately classes don't like having stuff added
- // to them after their definitions are complete...
-
- Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
- if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
-return type_ptr->shared_from_this();
- }
-}
- }
-
if (attrs.specification.IsValid()) {
// We have a specification which we are going to base our
// function prototype off of, so we need this type to be
@@ -1260,6 +1243,39 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
}
}
}
+ // By here, we should have already completed the c++ class_type
+ //
[Lldb-commits] [lldb] ee63f28 - [lldb-dap] Minor cleanup.
Author: Zequan Wu
Date: 2024-05-02T14:22:03-04:00
New Revision: ee63f287e013ab3424372034d4d5a95512ab5b6b
URL:
https://github.com/llvm/llvm-project/commit/ee63f287e013ab3424372034d4d5a95512ab5b6b
DIFF:
https://github.com/llvm/llvm-project/commit/ee63f287e013ab3424372034d4d5a95512ab5b6b.diff
LOG: [lldb-dap] Minor cleanup.
Fix #85974.
Added:
Modified:
lldb/tools/lldb-dap/JSONUtils.cpp
Removed:
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp
b/lldb/tools/lldb-dap/JSONUtils.cpp
index b4a2718bbb096e..788c1bc843db33 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -137,8 +137,7 @@ std::vector GetStrings(const
llvm::json::Object *obj,
static bool IsClassStructOrUnionType(lldb::SBType t) {
return (t.GetTypeClass() & (lldb::eTypeClassUnion | lldb::eTypeClassStruct |
- lldb::eTypeClassUnion | lldb::eTypeClassArray))
!=
- 0;
+ lldb::eTypeClassArray)) != 0;
}
/// Create a short summary for a container that contains the summary of its
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1631,13 +1631,19 @@ bool SymbolFileDWARF::CompleteType(CompilerType
_type) {
return true;
}
- DWARFDIE dwarf_die = GetDIE(die_it->getSecond());
+ DWARFDIE dwarf_die = FindDefinitionDIE(GetDIE(die_it->getSecond()));
if (dwarf_die) {
// Once we start resolving this type, remove it from the forward
// declaration map in case anyone child members or other types require this
// type to get resolved. The type will get resolved when all of the calls
// to SymbolFileDWARF::ResolveClangOpaqueTypeDefinition are done.
-GetForwardDeclCompilerTypeToDIE().erase(die_it);
+// Need to get a new iterator because FindDefinitionDIE might add new
ZequanWu wrote:
Now, we change `m_forward_decl_compiler_type_to_die` to be updated with
definition DIE. So, we may need to erase twice, because the first erase is
always necessary if we failed to find definition DIE for it. The second erase
is because calling FindDefinitionDIE might add new entry to the definition DIE
because the first one removed it.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1654,6 +1660,99 @@ bool SymbolFileDWARF::CompleteType(CompilerType
_type) {
return false;
}
+DWARFDIE SymbolFileDWARF::FindDefinitionDIE(const DWARFDIE ) {
+ auto def_die_it = GetDeclarationDIEToDefinitionDIE().find(die.GetDIE());
+ if (def_die_it != GetDeclarationDIEToDefinitionDIE().end())
+return GetDIE(def_die_it->getSecond());
+
+ ParsedDWARFTypeAttributes attrs(die);
+ const dw_tag_t tag = die.Tag();
+ TypeSP type_sp;
+ Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
+ if (log) {
+GetObjectFile()->GetModule()->LogMessage(
+log,
+"SymbolFileDWARF({0:p}) - {1:x16}: {2} type \"{3}\" is a "
+"forward declaration DIE, trying to find definition DIE",
+static_cast(this), die.GetOffset(), DW_TAG_value_to_name(tag),
+attrs.name.GetCString());
+ }
+ // We haven't parse definition die for this type, starting to search for it.
+ // After we found the definition die, the GetDeclarationDIEToDefinitionDIE()
+ // map will have the new mapping from this declaration die to definition die.
+ if (attrs.class_language == eLanguageTypeObjC ||
ZequanWu wrote:
Done.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -533,9 +540,16 @@ class SymbolFileDWARF : public SymbolFileCommon {
NameToOffsetMap m_function_scope_qualified_name_map;
std::unique_ptr m_ranges;
UniqueDWARFASTTypeMap m_unique_ast_type_map;
+ // A map from DIE to lldb_private::Type. For record type, the key might be
+ // either declaration DIE or definition DIE.
DIEToTypePtr m_die_to_type;
DIEToVariableSP m_die_to_variable_sp;
+ // A map from CompilerType to the struct/class/union/enum DIE (might be a
+ // declaration or a definition) that is used to construct it.
CompilerTypeToDIE m_forward_decl_compiler_type_to_die;
+ // A map from a struct/class/union/enum DIE (might be a declaration or a
+ // definition) to its definition DIE.
+ DIEToDIE m_die_to_def_die;
ZequanWu wrote:
Done.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/90663
>From 4e83099b593e66f12dc21be5fbac5279e03e Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Tue, 30 Apr 2024 16:23:11 -0400
Subject: [PATCH 1/3] [lldb][DWARF] Delay struct/class/union definition DIE
searching when parsing declaration DIEs.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 270 +++---
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 105 ++-
.../SymbolFile/DWARF/SymbolFileDWARF.h| 14 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 5 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.h | 2 +
.../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 95 +++---
.../SymbolFile/DWARF/UniqueDWARFASTType.h | 21 +-
7 files changed, 296 insertions(+), 216 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index bea11e0e3840af..7ad661c9a9d49b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -252,7 +252,8 @@ static void ForcefullyCompleteType(CompilerType type) {
static void PrepareContextToReceiveMembers(TypeSystemClang ,
ClangASTImporter _importer,
clang::DeclContext *decl_ctx,
- DWARFDIE die,
+ const DWARFDIE _ctx_die,
+ const DWARFDIE ,
const char *type_name_cstr) {
auto *tag_decl_ctx = clang::dyn_cast(decl_ctx);
if (!tag_decl_ctx)
@@ -279,6 +280,13 @@ static void PrepareContextToReceiveMembers(TypeSystemClang
,
type_name_cstr ? type_name_cstr : "", die.GetOffset());
}
+ // By searching for the definition DIE of the decl_ctx type, we will either:
+ // 1. Found the the definition DIE and start its definition with
+ // TypeSystemClang::StartTagDeclarationDefinition.
+ // 2. Unable to find it, then need to forcefully complete it.
+ die.GetDWARF()->FindDefinitionDIE(decl_ctx_die);
+ if (tag_decl_ctx->isCompleteDefinition() || tag_decl_ctx->isBeingDefined())
+return;
// We don't have a type definition and/or the import failed. We must
// forcefully complete the type to avoid crashes.
ForcefullyCompleteType(type);
@@ -620,10 +628,11 @@ DWARFASTParserClang::ParseTypeModifier(const
SymbolContext ,
if (tag == DW_TAG_typedef) {
// DeclContext will be populated when the clang type is materialized in
// Type::ResolveCompilerType.
-PrepareContextToReceiveMembers(
-m_ast, GetClangASTImporter(),
-GetClangDeclContextContainingDIE(die, nullptr), die,
-attrs.name.GetCString());
+DWARFDIE decl_ctx_die;
+clang::DeclContext *decl_ctx =
+GetClangDeclContextContainingDIE(die, _ctx_die);
+PrepareContextToReceiveMembers(m_ast, GetClangASTImporter(), decl_ctx,
+ decl_ctx_die, die, attrs.name.GetCString());
if (attrs.type.IsValid()) {
// Try to parse a typedef from the (DWARF embedded in the) Clang
@@ -1100,32 +1109,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
// struct and see if this is actually a C++ method
Type *class_type = dwarf->ResolveType(decl_ctx_die);
if (class_type) {
- if (class_type->GetID() != decl_ctx_die.GetID() ||
- IsClangModuleFwdDecl(decl_ctx_die)) {
-
-// We uniqued the parent class of this function to another
-// class so we now need to associate all dies under
-// "decl_ctx_die" to DIEs in the DIE for "class_type"...
-DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID());
-
-if (class_type_die) {
- std::vector failures;
-
- CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die,
- class_type, failures);
-
- // FIXME do something with these failures that's
- // smarter than just dropping them on the ground.
- // Unfortunately classes don't like having stuff added
- // to them after their definitions are complete...
-
- Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
- if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
-return type_ptr->shared_from_this();
- }
-}
- }
-
if (attrs.specification.IsValid()) {
// We have a specification which we are going to base our
// function prototype off of, so we need this type to be
@@ -1260,6 +1243,39 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
}
}
}
+ // By here, we should have already completed the c++ class_type
+ //
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1654,6 +1660,99 @@ bool SymbolFileDWARF::CompleteType(CompilerType
_type) {
return false;
}
+DWARFDIE SymbolFileDWARF::FindDefinitionDIE(const DWARFDIE ) {
+ auto def_die_it = GetDeclarationDIEToDefinitionDIE().find(die.GetDIE());
+ if (def_die_it != GetDeclarationDIEToDefinitionDIE().end())
+return GetDIE(def_die_it->getSecond());
+
+ ParsedDWARFTypeAttributes attrs(die);
+ const dw_tag_t tag = die.Tag();
+ TypeSP type_sp;
+ Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
+ if (log) {
+GetObjectFile()->GetModule()->LogMessage(
+log,
+"SymbolFileDWARF({0:p}) - {1:x16}: {2} type \"{3}\" is a "
+"forward declaration DIE, trying to find definition DIE",
+static_cast(this), die.GetOffset(), DW_TAG_value_to_name(tag),
+attrs.name.GetCString());
+ }
+ // We haven't parse definition die for this type, starting to search for it.
+ // After we found the definition die, the GetDeclarationDIEToDefinitionDIE()
+ // map will have the new mapping from this declaration die to definition die.
+ if (attrs.class_language == eLanguageTypeObjC ||
ZequanWu wrote:
Which part of this function is clang-specific code?
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: > Hmm - but the byte size wouldn't be known from only a declaration, right? so > how'd that key work between a declaration and a definition? For declaration and definition DIEs, we just look at the name. Byte size and declaration locations are only used as extra info to differentiate two definition DIEs. See the comment in: https://github.com/llvm/llvm-project/pull/90663/files#diff-82e596d532f38e5212da4007f8ffda731ac8c948ab98eaac21f30fc96ca62d30R24-R32 https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: > does this cause multiple (an open ended amount?) of declarations for a type > to be created if the type declarations from multiple CUs are encountered > separately? Or still only one due to the extra map? This only creates one even if type declarations are from different CUs. The definition DIE lookup mechanism is the same as before, via manual index, which is able to search cross CUs. We check if a type is created before using the `UniqueDWARFASTTypeMap` as before: For C++, unique type is identified by full qualified name + byte size. For other languages, it's base name + byte size + declaration location. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/90663
>From 4e83099b593e66f12dc21be5fbac5279e03e Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Tue, 30 Apr 2024 16:23:11 -0400
Subject: [PATCH 1/2] [lldb][DWARF] Delay struct/class/union definition DIE
searching when parsing declaration DIEs.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 270 +++---
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 105 ++-
.../SymbolFile/DWARF/SymbolFileDWARF.h| 14 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 5 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.h | 2 +
.../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 95 +++---
.../SymbolFile/DWARF/UniqueDWARFASTType.h | 21 +-
7 files changed, 296 insertions(+), 216 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index bea11e0e3840af..7ad661c9a9d49b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -252,7 +252,8 @@ static void ForcefullyCompleteType(CompilerType type) {
static void PrepareContextToReceiveMembers(TypeSystemClang ,
ClangASTImporter _importer,
clang::DeclContext *decl_ctx,
- DWARFDIE die,
+ const DWARFDIE _ctx_die,
+ const DWARFDIE ,
const char *type_name_cstr) {
auto *tag_decl_ctx = clang::dyn_cast(decl_ctx);
if (!tag_decl_ctx)
@@ -279,6 +280,13 @@ static void PrepareContextToReceiveMembers(TypeSystemClang
,
type_name_cstr ? type_name_cstr : "", die.GetOffset());
}
+ // By searching for the definition DIE of the decl_ctx type, we will either:
+ // 1. Found the the definition DIE and start its definition with
+ // TypeSystemClang::StartTagDeclarationDefinition.
+ // 2. Unable to find it, then need to forcefully complete it.
+ die.GetDWARF()->FindDefinitionDIE(decl_ctx_die);
+ if (tag_decl_ctx->isCompleteDefinition() || tag_decl_ctx->isBeingDefined())
+return;
// We don't have a type definition and/or the import failed. We must
// forcefully complete the type to avoid crashes.
ForcefullyCompleteType(type);
@@ -620,10 +628,11 @@ DWARFASTParserClang::ParseTypeModifier(const
SymbolContext ,
if (tag == DW_TAG_typedef) {
// DeclContext will be populated when the clang type is materialized in
// Type::ResolveCompilerType.
-PrepareContextToReceiveMembers(
-m_ast, GetClangASTImporter(),
-GetClangDeclContextContainingDIE(die, nullptr), die,
-attrs.name.GetCString());
+DWARFDIE decl_ctx_die;
+clang::DeclContext *decl_ctx =
+GetClangDeclContextContainingDIE(die, _ctx_die);
+PrepareContextToReceiveMembers(m_ast, GetClangASTImporter(), decl_ctx,
+ decl_ctx_die, die, attrs.name.GetCString());
if (attrs.type.IsValid()) {
// Try to parse a typedef from the (DWARF embedded in the) Clang
@@ -1100,32 +1109,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
// struct and see if this is actually a C++ method
Type *class_type = dwarf->ResolveType(decl_ctx_die);
if (class_type) {
- if (class_type->GetID() != decl_ctx_die.GetID() ||
- IsClangModuleFwdDecl(decl_ctx_die)) {
-
-// We uniqued the parent class of this function to another
-// class so we now need to associate all dies under
-// "decl_ctx_die" to DIEs in the DIE for "class_type"...
-DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID());
-
-if (class_type_die) {
- std::vector failures;
-
- CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die,
- class_type, failures);
-
- // FIXME do something with these failures that's
- // smarter than just dropping them on the ground.
- // Unfortunately classes don't like having stuff added
- // to them after their definitions are complete...
-
- Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()];
- if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) {
-return type_ptr->shared_from_this();
- }
-}
- }
-
if (attrs.specification.IsValid()) {
// We have a specification which we are going to base our
// function prototype off of, so we need this type to be
@@ -1260,6 +1243,39 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE ,
}
}
}
+ // By here, we should have already completed the c++ class_type
+ //
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu ready_for_review https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu created
https://github.com/llvm/llvm-project/pull/90663
This is the implementation for
https://discourse.llvm.org/t/rfc-delay-definition-die-searching-when-parse-a-declaration-die-for-record-type/78526.
Motivation
Currently, lldb eagerly searches for definition DIE when parsing a declaration
DIE for struct/class/union definition DIE. It will search for all definition
DIEs with the same unqualified name (just `DW_AT_name` ) and then find out
those DIEs with same fully qualified name. Then lldb will try to resolve those
DIEs to create the Types from definition DIEs. It works fine most time.
However, when built with `-gsimple-template-names`, the search graph expands
very quickly, because for the specialized-template classes, they don’t have
template parameter names encoded inside `DW_AT_name`. They have
`DW_TAG_template_type_parameter` to reference the types used as template
parameters. In order to identify if a definition DIE matches a declaration DIE,
lldb needs to resolve all template parameter types first and those template
parameter types might be template classes as well, and so on… So, the search
graph explodes, causing a lot unnecessary searching/type-resolving to just get
the fully qualified names for a specialized-template class. This causes lldb
stack overflow for us internally on template-heavy libraries.
Implementation
Instead of searching for definition DIEs when parsing declaration DIEs, we
always construct the record type from the DIE regardless if it's definition or
declaration. At the same time, use a map `GetDeclarationDIEToDefinitionDIE()`
to track the mapping from declarations/definition DIEs to the unique definition
DIE. The process of searching for definition DIE is refactored to
`SymbolFileDWARF::FindDefinitionDIE` which is invoked when 1) completing the
type on `SymbolFileDWARF::CompleteType`. 2) the record type needs to start its
definition as a containing type so that nested classes can be added into it in
`PrepareContextToReceiveMembers`.
The key difference is `SymbolFileDWARF::ResolveType` return a `Type*` that
might be created from declaration DIE, which means it hasn't starts its
definition yet. We also need to change according in places where we want the
type to start definition, like `PrepareContextToReceiveMembers` (I'm not aware
of any other places, but this should be a simple call to
`SymbolFileDWARF::FindDefinitionDIE`)
Result
It fixes the stack overflow of lldb for the internal binary built with simple
template name. When constructing the fully qualified name built with
`-gsimple-template-names`, it gets the name of the type parameter by resolving
the referenced DIE, which might be a declaration (we won't try to search for
the definition DIE to just get the name).
>From 4e83099b593e66f12dc21be5fbac5279e03e Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Tue, 30 Apr 2024 16:23:11 -0400
Subject: [PATCH] [lldb][DWARF] Delay struct/class/union definition DIE
searching when parsing declaration DIEs.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 270 +++---
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 105 ++-
.../SymbolFile/DWARF/SymbolFileDWARF.h| 14 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 5 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.h | 2 +
.../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 95 +++---
.../SymbolFile/DWARF/UniqueDWARFASTType.h | 21 +-
7 files changed, 296 insertions(+), 216 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index bea11e0e3840af..7ad661c9a9d49b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -252,7 +252,8 @@ static void ForcefullyCompleteType(CompilerType type) {
static void PrepareContextToReceiveMembers(TypeSystemClang ,
ClangASTImporter _importer,
clang::DeclContext *decl_ctx,
- DWARFDIE die,
+ const DWARFDIE _ctx_die,
+ const DWARFDIE ,
const char *type_name_cstr) {
auto *tag_decl_ctx = clang::dyn_cast(decl_ctx);
if (!tag_decl_ctx)
@@ -279,6 +280,13 @@ static void PrepareContextToReceiveMembers(TypeSystemClang
,
type_name_cstr ? type_name_cstr : "", die.GetOffset());
}
+ // By searching for the definition DIE of the decl_ctx type, we will either:
+ // 1. Found the the definition DIE and start its definition with
+ // TypeSystemClang::StartTagDeclarationDefinition.
+ // 2. Unable to find it, then need to forcefully complete it.
+ die.GetDWARF()->FindDefinitionDIE(decl_ctx_die);
+ if
[Lldb-commits] [lldb] [lldb][DWARF] Remove m_forward_decl_die_to_compiler_type as it never actually being used. (PR #89427)
https://github.com/ZequanWu closed https://github.com/llvm/llvm-project/pull/89427 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Remove m_forward_decl_die_to_compiler_type as it never actually being used. (PR #89427)
https://github.com/ZequanWu edited https://github.com/llvm/llvm-project/pull/89427 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Remove m_forward_decl_die_to_compiler_type as it never actually being used. (PR #89427)
https://github.com/ZequanWu created
https://github.com/llvm/llvm-project/pull/89427
This removes `m_forward_decl_die_to_compiler_type` which is a map from `const
DWARFDebugInfoEntry *` to `lldb::opaque_compiler_type_t`. This map is currently
used in `DWARFASTParserClang::ParseEnum` and
`DWARFASTParserClang::ParseStructureLikeDIE` to avoid creating duplicate
CompilerType for the specific DIE. But before entering these two functions in
`DWARFASTParserClang::ParseTypeFromDWARF`, we already checked with
`SymbolFileDWARF::GetDIEToType()` if we have a Type created from this DIE to
avoid creating two CompilerTypes for the same DIE. So, this map is unnecessary
and unseful.
>From 200e0caa806ca7d84e8722d6408c8c37ffb9f598 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Fri, 19 Apr 2024 13:57:06 -0400
Subject: [PATCH] [lldb][DWARF] Remove m_forward_decl_die_to_compiler_type as
it never actually being used.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 148 --
.../SymbolFile/DWARF/SymbolFileDWARF.h| 9 --
.../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 5 -
.../SymbolFile/DWARF/SymbolFileDWARFDwo.h | 2 -
4 files changed, 65 insertions(+), 99 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 54d06b1115a229..41d81fbcf1b087 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -854,36 +854,26 @@ TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext
,
DW_TAG_value_to_name(tag), type_name_cstr);
CompilerType enumerator_clang_type;
- CompilerType clang_type;
- clang_type = CompilerType(
- m_ast.weak_from_this(),
- dwarf->GetForwardDeclDIEToCompilerType().lookup(die.GetDIE()));
- if (!clang_type) {
-if (attrs.type.IsValid()) {
- Type *enumerator_type =
- dwarf->ResolveTypeUID(attrs.type.Reference(), true);
- if (enumerator_type)
-enumerator_clang_type = enumerator_type->GetFullCompilerType();
-}
+ if (attrs.type.IsValid()) {
+Type *enumerator_type = dwarf->ResolveTypeUID(attrs.type.Reference(),
true);
+if (enumerator_type)
+ enumerator_clang_type = enumerator_type->GetFullCompilerType();
+ }
-if (!enumerator_clang_type) {
- if (attrs.byte_size) {
-enumerator_clang_type = m_ast.GetBuiltinTypeForDWARFEncodingAndBitSize(
-"", DW_ATE_signed, *attrs.byte_size * 8);
- } else {
-enumerator_clang_type = m_ast.GetBasicType(eBasicTypeInt);
- }
+ if (!enumerator_clang_type) {
+if (attrs.byte_size) {
+ enumerator_clang_type = m_ast.GetBuiltinTypeForDWARFEncodingAndBitSize(
+ "", DW_ATE_signed, *attrs.byte_size * 8);
+} else {
+ enumerator_clang_type = m_ast.GetBasicType(eBasicTypeInt);
}
-
-clang_type = m_ast.CreateEnumerationType(
-attrs.name.GetStringRef(),
-GetClangDeclContextContainingDIE(die, nullptr),
-GetOwningClangModule(die), attrs.decl, enumerator_clang_type,
-attrs.is_scoped_enum);
- } else {
-enumerator_clang_type = m_ast.GetEnumerationIntegerType(clang_type);
}
+ CompilerType clang_type = m_ast.CreateEnumerationType(
+ attrs.name.GetStringRef(), GetClangDeclContextContainingDIE(die,
nullptr),
+ GetOwningClangModule(die), attrs.decl, enumerator_clang_type,
+ attrs.is_scoped_enum);
+
LinkDeclContextToDIE(TypeSystemClang::GetDeclContextForType(clang_type),
die);
type_sp =
@@ -1781,65 +1771,59 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext ,
assert(tag_decl_kind != -1);
UNUSED_IF_ASSERT_DISABLED(tag_decl_kind);
bool clang_type_was_created = false;
- clang_type = CompilerType(
- m_ast.weak_from_this(),
- dwarf->GetForwardDeclDIEToCompilerType().lookup(die.GetDIE()));
- if (!clang_type) {
-clang::DeclContext *decl_ctx =
-GetClangDeclContextContainingDIE(die, nullptr);
-
-PrepareContextToReceiveMembers(m_ast, GetClangASTImporter(), decl_ctx, die,
- attrs.name.GetCString());
-
-if (attrs.accessibility == eAccessNone && decl_ctx) {
- // Check the decl context that contains this class/struct/union. If
- // it is a class we must give it an accessibility.
- const clang::Decl::Kind containing_decl_kind = decl_ctx->getDeclKind();
- if (DeclKindIsCXXClass(containing_decl_kind))
-attrs.accessibility = default_accessibility;
-}
-
-ClangASTMetadata metadata;
-metadata.SetUserID(die.GetID());
-metadata.SetIsDynamicCXXType(dwarf->ClassOrStructIsVirtual(die));
-
-TypeSystemClang::TemplateParameterInfos template_param_infos;
-if (ParseTemplateParameterInfos(die, template_param_infos)) {
- clang::ClassTemplateDecl *class_template_decl =
- m_ast.ParseClassTemplateDecl(
- decl_ctx,
[Lldb-commits] [lldb] d3993ac - [lldb][test] Correctly fix break at _dl_debug_state test on arm.
Author: Zequan Wu
Date: 2024-04-18T11:13:17-04:00
New Revision: d3993ac1890731d2b24543646961c95680788207
URL:
https://github.com/llvm/llvm-project/commit/d3993ac1890731d2b24543646961c95680788207
DIFF:
https://github.com/llvm/llvm-project/commit/d3993ac1890731d2b24543646961c95680788207.diff
LOG: [lldb][test] Correctly fix break at _dl_debug_state test on arm.
If lldb finds the dynamic linker in the search path or if the binary is linked
staticlly, it will fail at `lldbutil.run_break_set_by_symbol` because the
breakpoint is resolved. Otherwise, it's not resolved at this point. But we
don't care if it's resolved or not. This test cares about if the breakpoint is
hit or not after launching.
This changes the num_expected_locations to -2, which means don't assert on if
this breakpoint resolved or not.
Added:
Modified:
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
Removed:
diff --git
a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
index b6bdd2d6041935..c219a4ee5bd9c6 100644
---
a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++
b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -672,7 +672,7 @@ def test_breakpoint_statistics_hitcount(self):
for breakpoint_stats in breakpoints_stats:
self.assertIn("hitCount", breakpoint_stats)
-@skipIf(oslist=no_match(["linux"]), archs=["arm", "aarch64"])
+@skipIf(oslist=no_match(["linux"]))
def test_break_at__dl_debug_state(self):
"""
Test lldb is able to stop at _dl_debug_state if it is set before the
@@ -682,7 +682,7 @@ def test_break_at__dl_debug_state(self):
exe = self.getBuildArtifact("a.out")
self.runCmd("target create %s" % exe)
bpid = lldbutil.run_break_set_by_symbol(
-self, "_dl_debug_state", num_expected_locations=0
+self, "_dl_debug_state", num_expected_locations=-2
)
self.runCmd("run")
self.assertIsNotNone(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] a1e7c83 - [lldb] Disable break at _dl_debug_state test on arm
Author: Zequan Wu
Date: 2024-04-18T10:40:35-04:00
New Revision: a1e7c83af11ee111994ec19029494e6e9ea97dbd
URL:
https://github.com/llvm/llvm-project/commit/a1e7c83af11ee111994ec19029494e6e9ea97dbd
DIFF:
https://github.com/llvm/llvm-project/commit/a1e7c83af11ee111994ec19029494e6e9ea97dbd.diff
LOG: [lldb] Disable break at _dl_debug_state test on arm
Added:
Modified:
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
Removed:
diff --git
a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
index 850235fdcefa70..b6bdd2d6041935 100644
---
a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++
b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -672,7 +672,7 @@ def test_breakpoint_statistics_hitcount(self):
for breakpoint_stats in breakpoints_stats:
self.assertIn("hitCount", breakpoint_stats)
-@skipIf(oslist=no_match(["linux"]))
+@skipIf(oslist=no_match(["linux"]), archs=["arm", "aarch64"])
def test_break_at__dl_debug_state(self):
"""
Test lldb is able to stop at _dl_debug_state if it is set before the
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Don't fail when SBProcess::GetMemoryRegionInfo returns error. (PR #87649)
ZequanWu wrote: Ping. https://github.com/llvm/llvm-project/pull/87649 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)
https://github.com/ZequanWu closed https://github.com/llvm/llvm-project/pull/88792 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)
@@ -572,9 +572,14 @@ ModuleSP DynamicLoaderPOSIXDYLD::LoadInterpreterModule() {
ModuleSpec module_spec(file, target.GetArchitecture());
if (ModuleSP module_sp = target.GetOrCreateModule(module_spec,
-true /* notify */)) {
+false /* notify */)) {
ZequanWu wrote:
Updated coding style and added a comment here.
https://github.com/llvm/llvm-project/pull/88792
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/88792
>From 26528cd679478448edf446e0e82e5f207ffd6113 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Mon, 15 Apr 2024 16:30:38 -0400
Subject: [PATCH 1/5] [lldb][DynamicLoader] Fix lldb unable to stop at
_dl_debug_state if user set it before the process launched.
---
lldb/include/lldb/Target/Target.h | 3 +++
.../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 5 +
lldb/source/Target/Target.cpp | 17 ++---
.../breakpoint_command/TestBreakpointCommand.py | 17 +
4 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/lldb/include/lldb/Target/Target.h
b/lldb/include/lldb/Target/Target.h
index 2c2e6b2831ccee..3554ef0ea5a140 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -878,6 +878,9 @@ class Target : public std::enable_shared_from_this,
// the address of its previous instruction and return that address.
lldb::addr_t GetBreakableLoadAddress(lldb::addr_t addr);
+ void UpdateBreakpoints(ModuleList _list, bool added,
+ bool delete_locations);
+
void ModulesDidLoad(ModuleList _list);
void ModulesDidUnload(ModuleList _list, bool delete_locations);
diff --git
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 9baf86da4dc799..901fa53682da8e 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -576,6 +576,11 @@ ModuleSP DynamicLoaderPOSIXDYLD::LoadInterpreterModule() {
UpdateLoadedSections(module_sp, LLDB_INVALID_ADDRESS, m_interpreter_base,
false);
m_interpreter_module = module_sp;
+// Manually update breakpoints after updating loaded sections, because they
+// cannot be resolve at the time when creating this module.
+ModuleList module_list;
+module_list.Append(module_sp);
+m_process->GetTarget().UpdateBreakpoints(module_list, true, false);
return module_sp;
}
return nullptr;
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 09b0ac42631d1a..ec2dea5cc238d3 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1687,6 +1687,13 @@ void
Target::NotifyModulesRemoved(lldb_private::ModuleList _list) {
ModulesDidUnload(module_list, false);
}
+void Target::UpdateBreakpoints(ModuleList _list, bool added,
+ bool delete_locations) {
+ m_breakpoint_list.UpdateBreakpoints(module_list, added, delete_locations);
+ m_internal_breakpoint_list.UpdateBreakpoints(module_list, added,
+ delete_locations);
+}
+
void Target::ModulesDidLoad(ModuleList _list) {
const size_t num_images = module_list.GetSize();
if (m_valid && num_images) {
@@ -1694,8 +1701,7 @@ void Target::ModulesDidLoad(ModuleList _list) {
ModuleSP module_sp(module_list.GetModuleAtIndex(idx));
LoadScriptingResourceForModule(module_sp, this);
}
-m_breakpoint_list.UpdateBreakpoints(module_list, true, false);
-m_internal_breakpoint_list.UpdateBreakpoints(module_list, true, false);
+UpdateBreakpoints(module_list, true, false);
if (m_process_sp) {
m_process_sp->ModulesDidLoad(module_list);
}
@@ -1713,8 +1719,7 @@ void Target::SymbolsDidLoad(ModuleList _list) {
}
}
-m_breakpoint_list.UpdateBreakpoints(module_list, true, false);
-m_internal_breakpoint_list.UpdateBreakpoints(module_list, true, false);
+UpdateBreakpoints(module_list, true, false);
auto data_sp =
std::make_shared(shared_from_this(), module_list);
BroadcastEvent(eBroadcastBitSymbolsLoaded, data_sp);
@@ -1727,9 +1732,7 @@ void Target::ModulesDidUnload(ModuleList _list,
bool delete_locations) {
auto data_sp =
std::make_shared(shared_from_this(), module_list);
BroadcastEvent(eBroadcastBitModulesUnloaded, data_sp);
-m_breakpoint_list.UpdateBreakpoints(module_list, false, delete_locations);
-m_internal_breakpoint_list.UpdateBreakpoints(module_list, false,
- delete_locations);
+UpdateBreakpoints(module_list, false, delete_locations);
// If a module was torn down it will have torn down the 'TypeSystemClang's
// that we used as source 'ASTContext's for the persistent variables in
diff --git
a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
index ea242952e409ec..a7e23fae14a21f 100644
---
a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++
[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)
ZequanWu wrote: > So, could the fix be as simple as passing notify=false in the first call ? Yeah, absolutely. Updated. https://github.com/llvm/llvm-project/pull/88792 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
