[Lldb-commits] [lldb] [lldb/DWARF] Unique enums parsed from declarations (PR #96751)
@@ -1798,6 +1805,13 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext &sc,
}
if (def_die) {
+if (auto [it, inserted] = dwarf->GetDIEToType().try_emplace(
ZequanWu wrote:
This part seems unnecessary as `UniqueDWARFASTType` map already handles it,
right?
https://github.com/llvm/llvm-project/pull/96751
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Unique enums parsed from declarations (PR #96751)
https://github.com/ZequanWu approved this pull request. https://github.com/llvm/llvm-project/pull/96751 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)
https://github.com/ZequanWu approved this pull request. https://github.com/llvm/llvm-project/pull/96755 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix the flaky test dwp-foreign-type-units.cpp. (PR #98237)
https://github.com/ZequanWu created https://github.com/llvm/llvm-project/pull/98237 Use `--match-full-lines` with FileCheck. Otherwise, the checks for `int` and `unsigned int` will match to the fields inside two `CustomType`s and FileCheck will start scanning from there, causing string not found error. >From f103032c6218dc6cddc3c5d267aed86adc2aac56 Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Tue, 9 Jul 2024 15:47:38 -0700 Subject: [PATCH] [lldb] Fix the flaky test dwp-foreign-type-units.cpp. Use `--match-full-lines` with FileCheck. Otherwise, the checks for `int` and `unsigned int` will match to the fields inside two `CustomType`s and FileCheck will start scanning from there, causing stirng not found error. --- .../Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp index ef15d418b4cfe..415b4850a244c 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp @@ -1,6 +1,4 @@ // REQUIRES: lld -// Is flaky on Windows. -// UNSUPPORTED: system-windows // This test will make a type that will be compiled differently into two // different .dwo files in a type unit with the same type hash, but with @@ -30,7 +28,7 @@ // RUN: -o "type lookup IntegerType" \ // RUN: -o "type lookup FloatType" \ // RUN: -o "type lookup CustomType" \ -// RUN: -b %t | FileCheck %s --check-prefix=NODWP +// RUN: -b %t | FileCheck %s --check-prefix=NODWP --match-full-lines // NODWP: (lldb) type lookup IntegerType // NODWP-DAG: int // NODWP-DAG: unsigned int ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix the test to deal with non-deterministic output. (PR #96800)
ZequanWu wrote: This also failed for me on Linux. I sent a fix at https://github.com/llvm/llvm-project/pull/98237 which also enables it on windows. https://github.com/llvm/llvm-project/pull/96800 ___ 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 #98361)
https://github.com/ZequanWu created
https://github.com/llvm/llvm-project/pull/98361
This is a reapply of https://github.com/llvm/llvm-project/pull/92328 and
https://github.com/llvm/llvm-project/pull/93839.
It now passes the
[test](https://github.com/llvm/llvm-project/commit/de3f1b6d68ab8a0e827db84b328803857a4f60df),
which crashes with the original reverted changes.
>From 37b6878b9125c314c75053f7d5b0ba520111e9a3 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Tue, 9 Jul 2024 15:28:19 -0700
Subject: [PATCH] Reapply [lldb][DWARF] Delay struct/class/union definition DIE
searching when parsing declaration DIEs.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 279 --
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 67 +++--
.../SymbolFile/DWARF/SymbolFileDWARF.h| 15 +-
.../DWARF/SymbolFileDWARFDebugMap.h | 9 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 2 +-
.../SymbolFile/DWARF/SymbolFileDWARFDwo.h | 3 +-
.../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 117
.../SymbolFile/DWARF/UniqueDWARFASTType.h | 36 +--
.../delayed-definition-die-searching.test | 36 +++
.../x86/simple-template-names-context.cpp | 4 +-
10 files changed, 301 insertions(+), 267 deletions(-)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 8e297141f4e13..7b93f6941ddda 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1603,41 +1603,74 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const
DWARFDIE &die) {
TypeSP
DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
- const DWARFDIE &decl_die,
+ const DWARFDIE &die,
ParsedDWARFTypeAttributes &attrs) {
CompilerType clang_type;
- const dw_tag_t tag = decl_die.Tag();
- SymbolFileDWARF *dwarf = decl_die.GetDWARF();
- LanguageType cu_language = SymbolFileDWARF::GetLanguage(*decl_die.GetCU());
+ const dw_tag_t tag = die.Tag();
+ SymbolFileDWARF *dwarf = die.GetDWARF();
+ LanguageType cu_language = SymbolFileDWARF::GetLanguage(*die.GetCU());
Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
- // UniqueDWARFASTType is large, so don't create a local variables on the
- // stack, put it on the heap. This function is often called recursively and
- // clang isn't good at sharing the stack space for variables in different
- // blocks.
- auto unique_ast_entry_up = std::make_unique();
-
ConstString unique_typename(attrs.name);
Declaration unique_decl(attrs.decl);
+ uint64_t byte_size = attrs.byte_size.value_or(0);
+ if (attrs.byte_size && *attrs.byte_size == 0 && attrs.name &&
+ !die.HasChildren() && cu_language == eLanguageTypeObjC) {
+// 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.
+attrs.is_forward_declaration = true;
+ }
if (attrs.name) {
if (Language::LanguageIsCPlusPlus(cu_language)) {
// For C++, we rely solely upon the one definition rule that says
// only one thing can exist at a given decl context. We ignore the
// file and line that things are declared on.
- std::string qualified_name = GetCPlusPlusQualifiedName(decl_die);
+ std::string qualified_name = GetCPlusPlusQualifiedName(die);
if (!qualified_name.empty())
unique_typename = ConstString(qualified_name);
unique_decl.Clear();
}
-if (dwarf->GetUniqueDWARFASTTypeMap().Find(
-unique_typename, decl_die, unique_decl,
-attrs.byte_size.value_or(-1), *unique_ast_entry_up)) {
- if (TypeSP type_sp = unique_ast_entry_up->m_type_sp) {
+if (UniqueDWARFASTType *unique_ast_entry_type =
+dwarf->GetUniqueDWARFASTTypeMap().Find(
+unique_typename, die, unique_decl, byte_size,
+attrs.is_forward_declaration)) {
+ if (TypeSP type_sp = unique_ast_entry_type->m_type_sp) {
+dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
LinkDeclContextToDIE(
-GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die),
-decl_die);
+GetCachedClangDeclContextForDIE(unique_ast_entry_type->m_die),
die);
+// If the DIE being parsed in this function is a definition and the
+// entry in the map is a declaration, then we need to updat
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)
@@ -1631,26 +1638,48 @@ bool SymbolFileDWARF::CompleteType(CompilerType
&compiler_type) {
return true;
}
- DWARFDIE dwarf_die = 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);
+ DWARFDIE decl_die = GetDIE(die_it->getSecond());
+ // Once we start resolving this type, remove it from the forward
+ // declaration map in case anyone's child members or other types require this
+ // type to get resolved.
+ GetForwardDeclCompilerTypeToDIE().erase(die_it);
+ DWARFDIE def_die = FindDefinitionDIE(decl_die);
+ if (!def_die) {
+SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
+if (debug_map_symfile) {
+ // We weren't able to find a full declaration in this DWARF, see
+ // if we have a declaration anywhere else...
+ def_die = debug_map_symfile->FindDefinitionDIE(decl_die);
+}
+ }
+ if (!def_die) {
+// No definition found. Proceed with the declaration die. We can use it to
+// create a forward-declared type.
+def_die = decl_die;
+ }
-Type *type = GetDIEToType().lookup(dwarf_die.GetDIE());
+ Type *type = ResolveType(def_die);
ZequanWu wrote:
`ResolveType(def_die)` just do [this
part](https://github.com/llvm/llvm-project/pull/98361/files/37b6878b9125c314c75053f7d5b0ba520111e9a3#diff-5dd6736e4d6c38623630df16c4494c1a8b099716ee0f05c9af54b4bafb1d864eR1607-R1674)
of work and then return the type, which was created from the declaration DIE.
It doesn't create a loop or recursion. We need it to do the work when finding
existing type in the UniqueDWARFASTTypeMap, which is basically this block:
https://github.com/llvm/llvm-project/pull/98361/files/37b6878b9125c314c75053f7d5b0ba520111e9a3#diff-5dd6736e4d6c38623630df16c4494c1a8b099716ee0f05c9af54b4bafb1d864eR1648-R1674.
Those code are important to keep some maps(`GetDIEToType()`,
`LinkDeclContextToDIE`, `UniqueDWARFASTTypeMap`) updated.
https://github.com/llvm/llvm-project/pull/98361
___
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 #98361)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/98361
>From 37b6878b9125c314c75053f7d5b0ba520111e9a3 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Tue, 9 Jul 2024 15:28:19 -0700
Subject: [PATCH 1/2] Reapply [lldb][DWARF] Delay struct/class/union definition
DIE searching when parsing declaration DIEs.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 279 --
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 67 +++--
.../SymbolFile/DWARF/SymbolFileDWARF.h| 15 +-
.../DWARF/SymbolFileDWARFDebugMap.h | 9 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 2 +-
.../SymbolFile/DWARF/SymbolFileDWARFDwo.h | 3 +-
.../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 117
.../SymbolFile/DWARF/UniqueDWARFASTType.h | 36 +--
.../delayed-definition-die-searching.test | 36 +++
.../x86/simple-template-names-context.cpp | 4 +-
10 files changed, 301 insertions(+), 267 deletions(-)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 8e297141f4e13..7b93f6941ddda 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1603,41 +1603,74 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const
DWARFDIE &die) {
TypeSP
DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
- const DWARFDIE &decl_die,
+ const DWARFDIE &die,
ParsedDWARFTypeAttributes &attrs) {
CompilerType clang_type;
- const dw_tag_t tag = decl_die.Tag();
- SymbolFileDWARF *dwarf = decl_die.GetDWARF();
- LanguageType cu_language = SymbolFileDWARF::GetLanguage(*decl_die.GetCU());
+ const dw_tag_t tag = die.Tag();
+ SymbolFileDWARF *dwarf = die.GetDWARF();
+ LanguageType cu_language = SymbolFileDWARF::GetLanguage(*die.GetCU());
Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
- // UniqueDWARFASTType is large, so don't create a local variables on the
- // stack, put it on the heap. This function is often called recursively and
- // clang isn't good at sharing the stack space for variables in different
- // blocks.
- auto unique_ast_entry_up = std::make_unique();
-
ConstString unique_typename(attrs.name);
Declaration unique_decl(attrs.decl);
+ uint64_t byte_size = attrs.byte_size.value_or(0);
+ if (attrs.byte_size && *attrs.byte_size == 0 && attrs.name &&
+ !die.HasChildren() && cu_language == eLanguageTypeObjC) {
+// 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.
+attrs.is_forward_declaration = true;
+ }
if (attrs.name) {
if (Language::LanguageIsCPlusPlus(cu_language)) {
// For C++, we rely solely upon the one definition rule that says
// only one thing can exist at a given decl context. We ignore the
// file and line that things are declared on.
- std::string qualified_name = GetCPlusPlusQualifiedName(decl_die);
+ std::string qualified_name = GetCPlusPlusQualifiedName(die);
if (!qualified_name.empty())
unique_typename = ConstString(qualified_name);
unique_decl.Clear();
}
-if (dwarf->GetUniqueDWARFASTTypeMap().Find(
-unique_typename, decl_die, unique_decl,
-attrs.byte_size.value_or(-1), *unique_ast_entry_up)) {
- if (TypeSP type_sp = unique_ast_entry_up->m_type_sp) {
+if (UniqueDWARFASTType *unique_ast_entry_type =
+dwarf->GetUniqueDWARFASTTypeMap().Find(
+unique_typename, die, unique_decl, byte_size,
+attrs.is_forward_declaration)) {
+ if (TypeSP type_sp = unique_ast_entry_type->m_type_sp) {
+dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
LinkDeclContextToDIE(
-GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die),
-decl_die);
+GetCachedClangDeclContextForDIE(unique_ast_entry_type->m_die),
die);
+// 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 (!attrs.is_forward_declaration &&
+unique_ast_entry_type->m_is_forward_declaration) {
+ unique_ast_entry_type->m_die = die;
+ unique_ast_entry_type->m_byte_size = byte_size;
+ un
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)
@@ -1631,26 +1638,48 @@ bool SymbolFileDWARF::CompleteType(CompilerType
&compiler_type) {
return true;
}
- DWARFDIE dwarf_die = 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);
+ DWARFDIE decl_die = GetDIE(die_it->getSecond());
+ // Once we start resolving this type, remove it from the forward
+ // declaration map in case anyone's child members or other types require this
+ // type to get resolved.
+ GetForwardDeclCompilerTypeToDIE().erase(die_it);
+ DWARFDIE def_die = FindDefinitionDIE(decl_die);
+ if (!def_die) {
+SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
+if (debug_map_symfile) {
+ // We weren't able to find a full declaration in this DWARF, see
+ // if we have a declaration anywhere else...
+ def_die = debug_map_symfile->FindDefinitionDIE(decl_die);
+}
+ }
+ if (!def_die) {
+// No definition found. Proceed with the declaration die. We can use it to
+// create a forward-declared type.
+def_die = decl_die;
+ }
-Type *type = GetDIEToType().lookup(dwarf_die.GetDIE());
+ Type *type = ResolveType(def_die);
ZequanWu wrote:
That makes sense. I created another function to update those maps and use
decl_die to lookup in UniqueDWARFASTTypeMap.
https://github.com/llvm/llvm-project/pull/98361
___
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 #98361)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/98361
>From 37b6878b9125c314c75053f7d5b0ba520111e9a3 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Tue, 9 Jul 2024 15:28:19 -0700
Subject: [PATCH 1/3] Reapply [lldb][DWARF] Delay struct/class/union definition
DIE searching when parsing declaration DIEs.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 279 --
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 67 +++--
.../SymbolFile/DWARF/SymbolFileDWARF.h| 15 +-
.../DWARF/SymbolFileDWARFDebugMap.h | 9 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 2 +-
.../SymbolFile/DWARF/SymbolFileDWARFDwo.h | 3 +-
.../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 117
.../SymbolFile/DWARF/UniqueDWARFASTType.h | 36 +--
.../delayed-definition-die-searching.test | 36 +++
.../x86/simple-template-names-context.cpp | 4 +-
10 files changed, 301 insertions(+), 267 deletions(-)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 8e297141f4e13..7b93f6941ddda 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1603,41 +1603,74 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const
DWARFDIE &die) {
TypeSP
DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
- const DWARFDIE &decl_die,
+ const DWARFDIE &die,
ParsedDWARFTypeAttributes &attrs) {
CompilerType clang_type;
- const dw_tag_t tag = decl_die.Tag();
- SymbolFileDWARF *dwarf = decl_die.GetDWARF();
- LanguageType cu_language = SymbolFileDWARF::GetLanguage(*decl_die.GetCU());
+ const dw_tag_t tag = die.Tag();
+ SymbolFileDWARF *dwarf = die.GetDWARF();
+ LanguageType cu_language = SymbolFileDWARF::GetLanguage(*die.GetCU());
Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
- // UniqueDWARFASTType is large, so don't create a local variables on the
- // stack, put it on the heap. This function is often called recursively and
- // clang isn't good at sharing the stack space for variables in different
- // blocks.
- auto unique_ast_entry_up = std::make_unique();
-
ConstString unique_typename(attrs.name);
Declaration unique_decl(attrs.decl);
+ uint64_t byte_size = attrs.byte_size.value_or(0);
+ if (attrs.byte_size && *attrs.byte_size == 0 && attrs.name &&
+ !die.HasChildren() && cu_language == eLanguageTypeObjC) {
+// 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.
+attrs.is_forward_declaration = true;
+ }
if (attrs.name) {
if (Language::LanguageIsCPlusPlus(cu_language)) {
// For C++, we rely solely upon the one definition rule that says
// only one thing can exist at a given decl context. We ignore the
// file and line that things are declared on.
- std::string qualified_name = GetCPlusPlusQualifiedName(decl_die);
+ std::string qualified_name = GetCPlusPlusQualifiedName(die);
if (!qualified_name.empty())
unique_typename = ConstString(qualified_name);
unique_decl.Clear();
}
-if (dwarf->GetUniqueDWARFASTTypeMap().Find(
-unique_typename, decl_die, unique_decl,
-attrs.byte_size.value_or(-1), *unique_ast_entry_up)) {
- if (TypeSP type_sp = unique_ast_entry_up->m_type_sp) {
+if (UniqueDWARFASTType *unique_ast_entry_type =
+dwarf->GetUniqueDWARFASTTypeMap().Find(
+unique_typename, die, unique_decl, byte_size,
+attrs.is_forward_declaration)) {
+ if (TypeSP type_sp = unique_ast_entry_type->m_type_sp) {
+dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
LinkDeclContextToDIE(
-GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die),
-decl_die);
+GetCachedClangDeclContextForDIE(unique_ast_entry_type->m_die),
die);
+// 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 (!attrs.is_forward_declaration &&
+unique_ast_entry_type->m_is_forward_declaration) {
+ unique_ast_entry_type->m_die = die;
+ unique_ast_entry_type->m_byte_size = byte_size;
+ un
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)
@@ -824,6 +824,36 @@ DWARFASTParserClang::GetDIEClassTemplateParams(const
DWARFDIE &die) {
return {};
}
+void DWARFASTParserClang::MappingDeclDIEToDefDIE(
+const lldb_private::plugin::dwarf::DWARFDIE &decl_die,
+const lldb_private::plugin::dwarf::DWARFDIE &def_die) {
+ LinkDeclContextToDIE(GetCachedClangDeclContextForDIE(decl_die), def_die);
+ SymbolFileDWARF *dwarf = def_die.GetDWARF();
+ ParsedDWARFTypeAttributes decl_attrs(decl_die);
+ ParsedDWARFTypeAttributes def_attrs(def_die);
+ ConstString unique_typename(decl_attrs.name);
+ Declaration decl_declaration(decl_attrs.decl);
+ if (Language::LanguageIsCPlusPlus(
+ SymbolFileDWARF::GetLanguage(*decl_die.GetCU( {
+std::string qualified_name = GetCPlusPlusQualifiedName(decl_die);
+if (!qualified_name.empty())
+ unique_typename = ConstString(qualified_name);
+decl_declaration.Clear();
+ }
ZequanWu wrote:
I don't see any equivalent code in here and ParseTypeFromDWARF.
https://github.com/llvm/llvm-project/pull/98361
___
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 #98361)
@@ -824,6 +824,36 @@ DWARFASTParserClang::GetDIEClassTemplateParams(const
DWARFDIE &die) {
return {};
}
+void DWARFASTParserClang::MappingDeclDIEToDefDIE(
+const lldb_private::plugin::dwarf::DWARFDIE &decl_die,
+const lldb_private::plugin::dwarf::DWARFDIE &def_die) {
+ LinkDeclContextToDIE(GetCachedClangDeclContextForDIE(decl_die), def_die);
+ SymbolFileDWARF *dwarf = def_die.GetDWARF();
+ ParsedDWARFTypeAttributes decl_attrs(decl_die);
+ ParsedDWARFTypeAttributes def_attrs(def_die);
+ ConstString unique_typename(decl_attrs.name);
+ Declaration decl_declaration(decl_attrs.decl);
+ if (Language::LanguageIsCPlusPlus(
+ SymbolFileDWARF::GetLanguage(*decl_die.GetCU( {
+std::string qualified_name = GetCPlusPlusQualifiedName(decl_die);
+if (!qualified_name.empty())
+ unique_typename = ConstString(qualified_name);
+decl_declaration.Clear();
+ }
+ if (UniqueDWARFASTType *unique_ast_entry_type =
ZequanWu wrote:
Yes, this should always be true. Added a log.
https://github.com/llvm/llvm-project/pull/98361
___
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 #98361)
@@ -4,8 +4,8 @@ // REQUIRES: lld -// RUN: %clang --target=x86_64-pc-linux -c %s -o %t-a.o -g -gsimple-template-names -DFILE_A -// RUN: %clang --target=x86_64-pc-linux -c %s -o %t-b.o -g -gsimple-template-names -DFILE_B ZequanWu wrote: Added another set of RUN for -fdebug-types-section https://github.com/llvm/llvm-project/pull/98361 ___ 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 #98361)
@@ -824,6 +824,36 @@ DWARFASTParserClang::GetDIEClassTemplateParams(const
DWARFDIE &die) {
return {};
}
+void DWARFASTParserClang::MappingDeclDIEToDefDIE(
ZequanWu wrote:
Done.
https://github.com/llvm/llvm-project/pull/98361
___
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 #98361)
@@ -1631,27 +1638,49 @@ bool SymbolFileDWARF::CompleteType(CompilerType
&compiler_type) {
return true;
}
- DWARFDIE dwarf_die = 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);
-
-Type *type = GetDIEToType().lookup(dwarf_die.GetDIE());
+ DWARFDIE decl_die = GetDIE(die_it->getSecond());
+ // Once we start resolving this type, remove it from the forward
+ // declaration map in case anyone's child members or other types require this
+ // type to get resolved.
+ GetForwardDeclCompilerTypeToDIE().erase(die_it);
+ DWARFDIE def_die = FindDefinitionDIE(decl_die);
+ if (!def_die) {
+SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
+if (debug_map_symfile) {
+ // We weren't able to find a full declaration in this DWARF, see
+ // if we have a declaration anywhere else...
+ def_die = debug_map_symfile->FindDefinitionDIE(decl_die);
+}
+ }
+ if (!def_die) {
+// If we don't have definition DIE, CompleteTypeFromDWARF will forcefully
+// complete this type.
+def_die = decl_die;
+ }
-Log *log = GetLog(DWARFLog::DebugInfo | DWARFLog::TypeCompletion);
-if (log)
- GetObjectFile()->GetModule()->LogMessageVerboseBacktrace(
- log, "{0:x8}: {1} ({2}) '{3}' resolving forward declaration...",
- dwarf_die.GetID(), DW_TAG_value_to_name(dwarf_die.Tag()),
- dwarf_die.Tag(), type->GetName().AsCString());
-assert(compiler_type);
-if (DWARFASTParser *dwarf_ast = GetDWARFParser(*dwarf_die.GetCU()))
- return dwarf_ast->CompleteTypeFromDWARF(dwarf_die, type, compiler_type);
+ DWARFASTParser *dwarf_ast = GetDWARFParser(*def_die.GetCU());
+ if (!dwarf_ast)
+return false;
+ Type *type = GetDIEToType().lookup(decl_die.GetDIE());
+ if (decl_die != def_die) {
+GetDIEToType()[def_die.GetDIE()] = type;
+// Need to update Type ID to refer to the definition DIE. because
+// it's used in ParseCXXMethod to determine if we need to copy cxx
+// method types from a declaration DIE to this definition DIE.
+type->SetID(def_die.GetID());
+if (DWARFASTParserClang *ast_parser =
+static_cast(dwarf_ast))
ZequanWu wrote:
Removed.
https://github.com/llvm/llvm-project/pull/98361
___
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 #98361)
@@ -1603,41 +1633,76 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const
DWARFDIE &die) {
TypeSP
DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
- const DWARFDIE &decl_die,
+ const DWARFDIE &die,
ParsedDWARFTypeAttributes &attrs) {
CompilerType clang_type;
- const dw_tag_t tag = decl_die.Tag();
- SymbolFileDWARF *dwarf = decl_die.GetDWARF();
- LanguageType cu_language = SymbolFileDWARF::GetLanguage(*decl_die.GetCU());
+ const dw_tag_t tag = die.Tag();
+ SymbolFileDWARF *dwarf = die.GetDWARF();
+ LanguageType cu_language = SymbolFileDWARF::GetLanguage(*die.GetCU());
Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
- // UniqueDWARFASTType is large, so don't create a local variables on the
- // stack, put it on the heap. This function is often called recursively and
- // clang isn't good at sharing the stack space for variables in different
- // blocks.
- auto unique_ast_entry_up = std::make_unique();
-
ConstString unique_typename(attrs.name);
Declaration unique_decl(attrs.decl);
+ uint64_t byte_size = attrs.byte_size.value_or(0);
+ if (attrs.byte_size && *attrs.byte_size == 0 && attrs.name &&
+ !die.HasChildren() && cu_language == eLanguageTypeObjC) {
+// 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.
+attrs.is_forward_declaration = true;
+ }
if (attrs.name) {
if (Language::LanguageIsCPlusPlus(cu_language)) {
// For C++, we rely solely upon the one definition rule that says
// only one thing can exist at a given decl context. We ignore the
// file and line that things are declared on.
- std::string qualified_name = GetCPlusPlusQualifiedName(decl_die);
+ std::string qualified_name = GetCPlusPlusQualifiedName(die);
if (!qualified_name.empty())
unique_typename = ConstString(qualified_name);
unique_decl.Clear();
}
-if (dwarf->GetUniqueDWARFASTTypeMap().Find(
-unique_typename, decl_die, unique_decl,
-attrs.byte_size.value_or(-1), *unique_ast_entry_up)) {
- if (TypeSP type_sp = unique_ast_entry_up->m_type_sp) {
+if (UniqueDWARFASTType *unique_ast_entry_type =
+dwarf->GetUniqueDWARFASTTypeMap().Find(
+unique_typename, die, unique_decl, byte_size,
+attrs.is_forward_declaration)) {
+ if (TypeSP type_sp = unique_ast_entry_type->m_type_sp) {
+dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
LinkDeclContextToDIE(
-GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die),
-decl_die);
+GetCachedClangDeclContextForDIE(unique_ast_entry_type->m_die),
die);
+// 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 (!attrs.is_forward_declaration &&
+unique_ast_entry_type->m_is_forward_declaration) {
+ unique_ast_entry_type->m_die = die;
+ if (byte_size)
+unique_ast_entry_type->m_byte_size = byte_size;
+ if (unique_decl.IsValid())
+unique_ast_entry_type->m_declaration = unique_decl;
+ unique_ast_entry_type->m_is_forward_declaration = false;
+ // Need to update Type ID to refer to the definition DIE. because
+ // it's used in ParseCXXMethod to determine if we need to copy cxx
+ // method types from a declaration DIE to this definition DIE.
+ type_sp->SetID(die.GetID());
ZequanWu wrote:
Done.
https://github.com/llvm/llvm-project/pull/98361
___
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 #98361)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/98361
>From 37b6878b9125c314c75053f7d5b0ba520111e9a3 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Tue, 9 Jul 2024 15:28:19 -0700
Subject: [PATCH 1/4] Reapply [lldb][DWARF] Delay struct/class/union definition
DIE searching when parsing declaration DIEs.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 279 --
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 67 +++--
.../SymbolFile/DWARF/SymbolFileDWARF.h| 15 +-
.../DWARF/SymbolFileDWARFDebugMap.h | 9 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 2 +-
.../SymbolFile/DWARF/SymbolFileDWARFDwo.h | 3 +-
.../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 117
.../SymbolFile/DWARF/UniqueDWARFASTType.h | 36 +--
.../delayed-definition-die-searching.test | 36 +++
.../x86/simple-template-names-context.cpp | 4 +-
10 files changed, 301 insertions(+), 267 deletions(-)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 8e297141f4e13..7b93f6941ddda 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1603,41 +1603,74 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const
DWARFDIE &die) {
TypeSP
DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
- const DWARFDIE &decl_die,
+ const DWARFDIE &die,
ParsedDWARFTypeAttributes &attrs) {
CompilerType clang_type;
- const dw_tag_t tag = decl_die.Tag();
- SymbolFileDWARF *dwarf = decl_die.GetDWARF();
- LanguageType cu_language = SymbolFileDWARF::GetLanguage(*decl_die.GetCU());
+ const dw_tag_t tag = die.Tag();
+ SymbolFileDWARF *dwarf = die.GetDWARF();
+ LanguageType cu_language = SymbolFileDWARF::GetLanguage(*die.GetCU());
Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
- // UniqueDWARFASTType is large, so don't create a local variables on the
- // stack, put it on the heap. This function is often called recursively and
- // clang isn't good at sharing the stack space for variables in different
- // blocks.
- auto unique_ast_entry_up = std::make_unique();
-
ConstString unique_typename(attrs.name);
Declaration unique_decl(attrs.decl);
+ uint64_t byte_size = attrs.byte_size.value_or(0);
+ if (attrs.byte_size && *attrs.byte_size == 0 && attrs.name &&
+ !die.HasChildren() && cu_language == eLanguageTypeObjC) {
+// 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.
+attrs.is_forward_declaration = true;
+ }
if (attrs.name) {
if (Language::LanguageIsCPlusPlus(cu_language)) {
// For C++, we rely solely upon the one definition rule that says
// only one thing can exist at a given decl context. We ignore the
// file and line that things are declared on.
- std::string qualified_name = GetCPlusPlusQualifiedName(decl_die);
+ std::string qualified_name = GetCPlusPlusQualifiedName(die);
if (!qualified_name.empty())
unique_typename = ConstString(qualified_name);
unique_decl.Clear();
}
-if (dwarf->GetUniqueDWARFASTTypeMap().Find(
-unique_typename, decl_die, unique_decl,
-attrs.byte_size.value_or(-1), *unique_ast_entry_up)) {
- if (TypeSP type_sp = unique_ast_entry_up->m_type_sp) {
+if (UniqueDWARFASTType *unique_ast_entry_type =
+dwarf->GetUniqueDWARFASTTypeMap().Find(
+unique_typename, die, unique_decl, byte_size,
+attrs.is_forward_declaration)) {
+ if (TypeSP type_sp = unique_ast_entry_type->m_type_sp) {
+dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
LinkDeclContextToDIE(
-GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die),
-decl_die);
+GetCachedClangDeclContextForDIE(unique_ast_entry_type->m_die),
die);
+// 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 (!attrs.is_forward_declaration &&
+unique_ast_entry_type->m_is_forward_declaration) {
+ unique_ast_entry_type->m_die = die;
+ unique_ast_entry_type->m_byte_size = byte_size;
+ un
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/98361
>From 37b6878b9125c314c75053f7d5b0ba520111e9a3 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Tue, 9 Jul 2024 15:28:19 -0700
Subject: [PATCH 1/5] Reapply [lldb][DWARF] Delay struct/class/union definition
DIE searching when parsing declaration DIEs.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 279 --
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 67 +++--
.../SymbolFile/DWARF/SymbolFileDWARF.h| 15 +-
.../DWARF/SymbolFileDWARFDebugMap.h | 9 +
.../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 2 +-
.../SymbolFile/DWARF/SymbolFileDWARFDwo.h | 3 +-
.../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 117
.../SymbolFile/DWARF/UniqueDWARFASTType.h | 36 +--
.../delayed-definition-die-searching.test | 36 +++
.../x86/simple-template-names-context.cpp | 4 +-
10 files changed, 301 insertions(+), 267 deletions(-)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 8e297141f4e13..7b93f6941ddda 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1603,41 +1603,74 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const
DWARFDIE &die) {
TypeSP
DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
- const DWARFDIE &decl_die,
+ const DWARFDIE &die,
ParsedDWARFTypeAttributes &attrs) {
CompilerType clang_type;
- const dw_tag_t tag = decl_die.Tag();
- SymbolFileDWARF *dwarf = decl_die.GetDWARF();
- LanguageType cu_language = SymbolFileDWARF::GetLanguage(*decl_die.GetCU());
+ const dw_tag_t tag = die.Tag();
+ SymbolFileDWARF *dwarf = die.GetDWARF();
+ LanguageType cu_language = SymbolFileDWARF::GetLanguage(*die.GetCU());
Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
- // UniqueDWARFASTType is large, so don't create a local variables on the
- // stack, put it on the heap. This function is often called recursively and
- // clang isn't good at sharing the stack space for variables in different
- // blocks.
- auto unique_ast_entry_up = std::make_unique();
-
ConstString unique_typename(attrs.name);
Declaration unique_decl(attrs.decl);
+ uint64_t byte_size = attrs.byte_size.value_or(0);
+ if (attrs.byte_size && *attrs.byte_size == 0 && attrs.name &&
+ !die.HasChildren() && cu_language == eLanguageTypeObjC) {
+// 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.
+attrs.is_forward_declaration = true;
+ }
if (attrs.name) {
if (Language::LanguageIsCPlusPlus(cu_language)) {
// For C++, we rely solely upon the one definition rule that says
// only one thing can exist at a given decl context. We ignore the
// file and line that things are declared on.
- std::string qualified_name = GetCPlusPlusQualifiedName(decl_die);
+ std::string qualified_name = GetCPlusPlusQualifiedName(die);
if (!qualified_name.empty())
unique_typename = ConstString(qualified_name);
unique_decl.Clear();
}
-if (dwarf->GetUniqueDWARFASTTypeMap().Find(
-unique_typename, decl_die, unique_decl,
-attrs.byte_size.value_or(-1), *unique_ast_entry_up)) {
- if (TypeSP type_sp = unique_ast_entry_up->m_type_sp) {
+if (UniqueDWARFASTType *unique_ast_entry_type =
+dwarf->GetUniqueDWARFASTTypeMap().Find(
+unique_typename, die, unique_decl, byte_size,
+attrs.is_forward_declaration)) {
+ if (TypeSP type_sp = unique_ast_entry_type->m_type_sp) {
+dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
LinkDeclContextToDIE(
-GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die),
-decl_die);
+GetCachedClangDeclContextForDIE(unique_ast_entry_type->m_die),
die);
+// 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 (!attrs.is_forward_declaration &&
+unique_ast_entry_type->m_is_forward_declaration) {
+ unique_ast_entry_type->m_die = die;
+ unique_ast_entry_type->m_byte_size = byte_size;
+ un
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)
@@ -1595,49 +1627,67 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const
DWARFDIE &die) {
if (qualified_name.empty())
qualified_name.append("::");
- qualified_name.append(name);
+ qualified_name.append(unique_typename.GetCString());
qualified_name.append(GetDIEClassTemplateParams(die));
- return qualified_name;
+ unique_typename = ConstString(qualified_name);
ZequanWu wrote:
This functions modifies both `unique_typename` and `decl_declaration` if it's
c++, so there're two out values. Returning one out value while modifying
another out value via reference parameter looks strange.
https://github.com/llvm/llvm-project/pull/98361
___
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 #98361)
@@ -1659,128 +1709,56 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext &sc,
default_accessibility = eAccessPrivate;
}
- if (attrs.byte_size && *attrs.byte_size == 0 && attrs.name &&
- !decl_die.HasChildren() && cu_language == eLanguageTypeObjC) {
-// 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.
-attrs.is_forward_declaration = true;
- }
+ if ((attrs.class_language == eLanguageTypeObjC ||
+ attrs.class_language == eLanguageTypeObjC_plus_plus) &&
+ !attrs.is_complete_objc_class &&
+ die.Supports_DW_AT_APPLE_objc_complete_type()) {
+// We have a valid eSymbolTypeObjCClass class symbol whose name
+// matches the current objective C class that we are trying to find
+// and this DIE isn't the complete definition (we checked
+// is_complete_objc_class above and know it is false), so the real
+// definition is in here somewhere
+TypeSP type_sp =
+dwarf->FindCompleteObjCDefinitionTypeForDIE(die, attrs.name, true);
- if (attrs.class_language == eLanguageTypeObjC ||
- attrs.class_language == eLanguageTypeObjC_plus_plus) {
-if (!attrs.is_complete_objc_class &&
-decl_die.Supports_DW_AT_APPLE_objc_complete_type()) {
- // We have a valid eSymbolTypeObjCClass class symbol whose name
- // matches the current objective C class that we are trying to find
- // and this DIE isn't the complete definition (we checked
- // is_complete_objc_class above and know it is false), so the real
- // definition is in here somewhere
- TypeSP type_sp =
- dwarf->FindCompleteObjCDefinitionTypeForDIE(decl_die, attrs.name,
true);
-
- if (!type_sp) {
-SymbolFileDWARFDebugMap *debug_map_symfile =
-dwarf->GetDebugMapSymfile();
-if (debug_map_symfile) {
- // We weren't able to find a full declaration in this DWARF,
- // see if we have a declaration anywhere else...
- type_sp = debug_map_symfile->FindCompleteObjCDefinitionTypeForDIE(
- decl_die, attrs.name, true);
-}
+if (!type_sp) {
+ SymbolFileDWARFDebugMap *debug_map_symfile = dwarf->GetDebugMapSymfile();
+ if (debug_map_symfile) {
+// We weren't able to find a full declaration in this DWARF,
+// see if we have a declaration anywhere else...
+type_sp = debug_map_symfile->FindCompleteObjCDefinitionTypeForDIE(
+die, attrs.name, true);
}
+}
- if (type_sp) {
-if (log) {
- dwarf->GetObjectFile()->GetModule()->LogMessage(
- log,
- "SymbolFileDWARF({0:p}) - {1:x16}: {2} ({3}) type \"{4}\" is an "
- "incomplete objc type, complete type is {5:x8}",
- static_cast(this), decl_die.GetOffset(),
- DW_TAG_value_to_name(tag), tag, attrs.name.GetCString(),
- type_sp->GetID());
-}
-return type_sp;
+if (type_sp) {
+ if (log) {
+dwarf->GetObjectFile()->GetModule()->LogMessage(
+log,
+"SymbolFileDWARF({0:p}) - {1:x16}: {2} ({3}) type \"{4}\" is an "
+"incomplete objc type, complete type is {5:x8}",
+static_cast(this), die.GetID(), DW_TAG_value_to_name(tag),
+tag, attrs.name.GetCString(), type_sp->GetID());
}
+ return type_sp;
}
}
- DWARFDIE def_die;
if (attrs.is_forward_declaration) {
-Progress progress(llvm::formatv(
ZequanWu wrote:
I moved it inside `FindDefinitionDIE` and changed the logging message to
`Searching definition DIE in ...` as that function just do searching not
parsing.
https://github.com/llvm/llvm-project/pull/98361
___
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 #98361)
ZequanWu wrote: This buildkite seems got stuck somehow, no logging at all: https://buildkite.com/llvm-project/github-pull-requests/builds/81790#0190bca9-bde7-4fad-8478-9dffd4f669f7. Will merge without waiting for it to finish. https://github.com/llvm/llvm-project/pull/98361 ___ 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 #98361)
https://github.com/ZequanWu closed https://github.com/llvm/llvm-project/pull/98361 ___ 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: > 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: > 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:
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: 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] 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 &die) = 0;
+ virtual lldb_private::Type *FindDefinitionTypeForDIE(const DWARFDIE &die) =
0;
+
static std::optional
ParseChildArrayInfo(const DWARFDIE &parent_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 &die,
+ const ParsedDWARFTypeAttributes &attrs,
+ 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 &sc,
const DWARFDIE &die,
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 &ast,
- ClangASTImporter &ast_importer,
- clang::DeclContext *decl_ctx,
- DWARFDIE die,
- const char *type_name_cstr) {
+void DWARFASTParserClang::PrepareContextToReceiveMember
[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 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 &die, 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)
@@ -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 &die, 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)
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 &die,
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)
@@ -2306,6 +2345,11 @@ bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE &die, 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 &die, 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] 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)
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 &die) = 0;
+ virtual lldb_private::Type *FindDefinitionTypeForDIE(const DWARFDIE &die) =
0;
+
static std::optional
ParseChildArrayInfo(const DWARFDIE &parent_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 &die,
+ const ParsedDWARFTypeAttributes &attrs,
+ 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 &sc,
const DWARFDIE &die,
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 &ast,
- ClangASTImporter &ast_importer,
- clang::DeclContext *decl_ctx,
- DWARFDIE die,
- const char *type_name_cstr) {
+void DWARFASTParserClang::PrepareContextToReceiveMembers(
+clang::DeclContext *decl_ctx, const DWARFDIE &decl_ctx_die,
+const DWARFDIE &die, 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
&ast,
// 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 &ast_importer = GetClangASTImporter();
if (type && ast_importer.CanImport(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/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 &die) = 0;
+ virtual lldb_private::Type *FindDefinitionTypeForDIE(const DWARFDIE &die) =
0;
+
static std::optional
ParseChildArrayInfo(const DWARFDIE &parent_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 &die,
+ const ParsedDWARFTypeAttributes &attrs,
+ 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 &sc,
const DWARFDIE &die,
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 &ast,
- ClangASTImporter &ast_importer,
- clang::DeclContext *decl_ctx,
- DWARFDIE die,
- const char *type_name_cstr) {
+void DWARFASTParserClang::PrepareContextToReceiveMembers(
+clang::DeclContext *decl_ctx, const DWARFDIE &decl_ctx_die,
+const DWARFDIE &die, 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
&ast,
// 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 &ast_importer = GetClangASTImporter();
if (type && ast_importer.CanImport(type))
[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 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)
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] [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 &die,
// 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 &die, const lldb_private::Declaration &decl,
const int32_t byte_size, bool is_forward_declaration) {
for (UniqueDWARFASTType &udt : 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)
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)
@@ -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] [lldb][DWARF] Fix adding children to clang type that hasn't started definition. (PR #93839)
@@ -2232,6 +2232,11 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE &die,
// 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)
@@ -2232,6 +2232,11 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE &die,
// 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 &die,
// 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 &die, const lldb_private::Declaration &decl,
const int32_t byte_size, bool is_forward_declaration) {
for (UniqueDWARFASTType &udt : 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 &die,
// 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)
@@ -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)
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 &die,
// 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 &die, const lldb_private::Declaration &decl,
const int32_t byte_size, bool is_forward_declaration) {
for (UniqueDWARFASTType &udt : 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 &die,
// 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
[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 &die,
// 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 &die, const lldb_private::Declaration &decl,
const int32_t byte_size, bool is_forward_declaration) {
for (UniqueDWARFASTType &udt : 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 &die,
// 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
[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 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] 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] Remove parsing recursion when searching for definition DIEs (PR #96484)
@@ -897,32 +895,39 @@ TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext &sc, } CompilerType clang_type = m_ast.CreateEnumerationType( - attrs.name.GetStringRef(), GetClangDeclContextContainingDIE(die, nullptr), - GetOwningClangModule(die), attrs.decl, enumerator_clang_type, + attrs.name.GetStringRef(), GetClangDeclContextContainingDIE(def_die, nullptr), + GetOwningClangModule(def_die), attrs.decl, enumerator_clang_type, attrs.is_scoped_enum); - - LinkDeclContextToDIE(TypeSystemClang::GetDeclContextForType(clang_type), die); - - type_sp = - dwarf->MakeType(die.GetID(), attrs.name, attrs.byte_size, nullptr, + TypeSP type_sp = + dwarf->MakeType(def_die.GetID(), attrs.name, attrs.byte_size, nullptr, ZequanWu wrote: If two different enum decl_die (reference the same definition def_die) were called with this function, doesn't it create two `CompilerType` and two `Type` from the same def_die? It's not a problem for `ParseStructureLikeDIE` because it will check if we have already created the type in `UniqueDWARFASTTypeMap`. https://github.com/llvm/llvm-project/pull/96484 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Enabling instruction breakpoint support to lldb-dap. (PR #105278)
@@ -4082,6 +4084,260 @@ void request__testGetTargetBreakpoints(const
llvm::json::Object &request) {
g_dap.SendJSON(llvm::json::Value(std::move(response)));
}
+// "SetInstructionBreakpointsRequest" : {
+// "allOf" : [
+// {"$ref" : "#/definitions/Request"}, {
+// "type" : "object",
+// "description" :
+// "Replaces all existing instruction breakpoints. Typically, "
+// "instruction breakpoints would be set from a disassembly window. "
+// "\nTo clear all instruction breakpoints, specify an empty "
+// "array.\nWhen an instruction breakpoint is hit, a `stopped` event
"
+// "(with reason `instruction breakpoint`) is generated.\nClients "
+// "should only call this request if the corresponding capability "
+// "`supportsInstructionBreakpoints` is true.",
+// "properties" : {
+// "command" : {"type" : "string", "enum" :
+// ["setInstructionBreakpoints"]}, "arguments" :
+// {"$ref" : "#/definitions/SetInstructionBreakpointsArguments"}
+// },
+// "required" : [ "command", "arguments" ]
+// }
+// ]
+// },
+// "SetInstructionBreakpointsArguments"
+// : {
+// "type" : "object",
+// "description" : "Arguments for `setInstructionBreakpoints` request",
+// "properties" : {
+// "breakpoints" : {
+// "type" : "array",
+// "items" : {"$ref" : "#/definitions/InstructionBreakpoint"},
+// "description" : "The instruction references of the breakpoints"
+// }
+// },
+// "required" : ["breakpoints"]
+// },
+// "SetInstructionBreakpointsResponse"
+// : {
+// "allOf" : [
+// {"$ref" : "#/definitions/Response"}, {
+// "type" : "object",
+// "description" : "Response to `setInstructionBreakpoints` request",
+// "properties" : {
+// "body" : {
+// "type" : "object",
+// "properties" : {
+// "breakpoints" : {
+// "type" : "array",
+// "items" : {"$ref" : "#/definitions/Breakpoint"},
+// "description" :
+// "Information about the breakpoints. The array elements
+// " "correspond to the elements of the `breakpoints`
+// array."
+// }
+// },
+// "required" : ["breakpoints"]
+// }
+// },
+// "required" : ["body"]
+// }
+// ]
+// },
+// "InstructionBreakpoint" : {
+// "type" : "object",
+// "description" : "Properties of a breakpoint passed to the "
+// "`setInstructionBreakpoints` request",
+// "properties" : {
+// "instructionReference" : {
+// "type" : "string",
+// "description" :
+// "The instruction reference of the breakpoint.\nThis should be a "
+// "memory or instruction pointer reference from an
+// `EvaluateResponse`, "
+// "`Variable`, `StackFrame`, `GotoTarget`, or `Breakpoint`."
+// },
+// "offset" : {
+// "type" : "integer",
+// "description" : "The offset from the instruction reference in "
+// "bytes.\nThis can be negative."
+// },
+// "condition" : {
+// "type" : "string",
+// "description" : "An expression for conditional breakpoints.\nIt is
only
+// "
+// "honored by a debug adapter if the corresponding "
+// "capability `supportsConditionalBreakpoints` is true."
+// },
+// "hitCondition" : {
+// "type" : "string",
+// "description" : "An expression that controls how many hits of the "
+// "breakpoint are ignored.\nThe debug adapter is
expected
+// " "to interpret the expression as needed.\nThe
+// attribute " "is only honored by a debug adapter if the
+// corresponding " "capability
+// `supportsHitConditionalBreakpoints` is true."
+// },
+// "mode" : {
+// "type" : "string",
+// "description" : "The mode of this breakpoint. If defined, this must be
+// "
+// "one of the `breakpointModes` the debug adapter "
+// "advertised in its `Capabilities`."
+// }
+// },
+// "required" : ["instructionReference"]
+// },
+// "Breakpoint"
+// : {
+// "type" : "object",
+// "description" :
+// "Information about a breakpoint created in `setBreakpoints`, "
+// "`setFunctionBreakpoints`, `setInstructionBreakpoints`, or "
+// "`setDataBreakpoints` requests.",
+// "properties" : {
+// "id" : {
+// "type" : "integer",
+// "description" :
+// "The ident
[Lldb-commits] [lldb] [lldb-dap] Enabling instruction breakpoint support to lldb-dap. (PR #105278)
@@ -766,6 +766,102 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame) {
return llvm::json::Value(std::move(object));
}
+// Response to `setInstructionBreakpoints` request.
+// "Breakpoint": {
+// "type": "object",
+// "description": "Response to `setInstructionBreakpoints` request.",
+// "properties": {
+// "id": {
+// "type": "number",
+// "description": "The identifier for the breakpoint. It is needed if
+// breakpoint events are used to update or remove breakpoints."
+// },
+// "verified": {
+// "type": "boolean",
+// "description": "If true, the breakpoint could be set (but not
+// necessarily at the desired location."
+// },
+// "message": {
+// "type": "string",
+// "description": "A message about the state of the breakpoint.
+// This is shown to the user and can be used to explain why a breakpoint
+// could not be verified."
+// },
+// "source": {
+// "type": "Source",
+// "description": "The source where the breakpoint is located."
+// },
+// "line": {
+// "type": "number",
+// "description": "The start line of the actual range covered by the
+// breakpoint."
+// },
+// "column": {
+// "type": "number",
+// "description": "The start column of the actual range covered by the
+// breakpoint."
+// },
+// "endLine": {
+// "type": "number",
+// "description": "The end line of the actual range covered by the
+// breakpoint."
+// },
+// "endColumn": {
+// "type": "number",
+// "description": "The end column of the actual range covered by the
+// breakpoint. If no end line is given, then the end column is assumed to
+// be in the start line."
+// },
+// "instructionReference": {
+// "type": "string",
+// "description": "A memory reference to where the breakpoint is set."
+// },
+// "offset": {
+// "type": "number",
+// "description": "The offset from the instruction reference.
+// This can be negative."
+// },
+// },
+// "required": [ "id", "verified", "line"]
+// }
+llvm::json::Value CreateInstructionBreakpoint(lldb::SBBreakpoint &bp) {
ZequanWu wrote:
There's a method `Breakpoint::CreateJsonObject` which basically does the same
thing as this function. Can you reuse that one?
https://github.com/llvm/llvm-project/pull/105278
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [mlir] [clang-tools-extra] [clang] [lldb] [compiler-rt] [flang] [Profile] Add binary profile correlation for code coverage. (PR #69493)
ZequanWu wrote: Ping. https://github.com/llvm/llvm-project/pull/69493 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [mlir] [llvm] [lldb] [flang] [clang] [compiler-rt] [clang-tools-extra] [Profile] Add binary profile correlation for code coverage. (PR #69493)
ZequanWu wrote: > Can you break up all the changes to tests that replace > `-debug-info-correlate` with `--profile-correlate=debug-info` into a separate > PR to reduce the size of this PR? Done. https://github.com/llvm/llvm-project/pull/69493 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [flang] [llvm] [clang-tools-extra] [mlir] [lldb] [compiler-rt] [Profile] Add binary profile correlation for code coverage. (PR #69493)
@@ -702,6 +708,8 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure, #define INSTR_PROF_COVMAP_COMMON __llvm_covmap #define INSTR_PROF_COVFUN_COMMON __llvm_covfun #define INSTR_PROF_ORDERFILE_COMMON __llvm_orderfile ZequanWu wrote: Done. https://github.com/llvm/llvm-project/pull/69493 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [compiler-rt] [lldb] [clang] [mlir] [flang] [clang-tools-extra] [Profile] Add binary profile correlation for code coverage. (PR #69493)
@@ -1829,6 +1833,22 @@ void CoverageMappingModuleGen::emit() {
llvm::GlobalValue::InternalLinkage, NamesArrVal,
llvm::getCoverageUnusedNamesVarName());
}
+ const StringRef VarName(INSTR_PROF_QUOTE(INSTR_PROF_RAW_VERSION_VAR));
ZequanWu wrote:
I already reverted changes in Clang because `VARIANT_MASK_BIN_CORRELATE` flag
is no longer necessary:
https://github.com/llvm/llvm-project/pull/69493#issuecomment-1815324995
https://github.com/llvm/llvm-project/pull/69493
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [llvm] [lldb] [mlir] [compiler-rt] [clang-tools-extra] [flang] [Profile] Add binary profile correlation for code coverage. (PR #69493)
@@ -1829,6 +1833,22 @@ void CoverageMappingModuleGen::emit() {
llvm::GlobalValue::InternalLinkage, NamesArrVal,
llvm::getCoverageUnusedNamesVarName());
}
+ const StringRef VarName(INSTR_PROF_QUOTE(INSTR_PROF_RAW_VERSION_VAR));
+ llvm::Type *IntTy64 = llvm::Type::getInt64Ty(Ctx);
+ uint64_t ProfileVersion = INSTR_PROF_RAW_VERSION;
+ if (llvm::ProfileCorrelate == llvm::InstrProfCorrelator::BINARY)
+ProfileVersion |= VARIANT_MASK_BIN_CORRELATE;
ZequanWu wrote:
ditto
https://github.com/llvm/llvm-project/pull/69493
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [compiler-rt] [flang] [llvm] [lldb] [mlir] [clang-tools-extra] [Profile] Add binary profile correlation for code coverage. (PR #69493)
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm
-o - %s | FileCheck %s
+// RUN: %clang_cc1 -mllvm -profile-correlate=binary -fprofile-instrument=clang
-fcoverage-mapping -emit-llvm -o - %s | FileCheck %s
--check-prefix=BIN-CORRELATE
+
+// CHECK: @__llvm_profile_raw_version = {{.*}} i64 9
ZequanWu wrote:
ditto
https://github.com/llvm/llvm-project/pull/69493
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [llvm] [mlir] [lldb] [clang-tools-extra] [compiler-rt] [flang] [Profile] Add binary profile correlation for code coverage. (PR #69493)
https://github.com/ZequanWu closed https://github.com/llvm/llvm-project/pull/69493 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [mlir] [lldb] [clang-tools-extra] [compiler-rt] [clang] [flang] [Profile] Add binary profile correlation for code coverage. (PR #69493)
ZequanWu wrote: > Seems like a mismatch on the diff, so maybe the check is too stringent. > > If this will take a while to fix, would you mind reverting until it can be > addressed? It passed for me locally on x64. Maybe I should use `diff <(llvm-profdata show --all-functions --counts %t.normal.profdata) <(llvm-profdata show --all-functions --counts %t.d4.profdata)` like it does on debug-info correlate test: https://github.com/llvm/llvm-project/blob/29e043cb5c2efaad7fb203fb8240a91b77ca0c5b/compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c#L10 Sent a possible fix at: https://github.com/llvm/llvm-project/commit/f34325307eb36d6032ccea3773e3e0c1746b7f98, hope that fixes it. https://github.com/llvm/llvm-project/pull/69493 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang-tools-extra] [compiler-rt] [clang] [llvm] [flang] [lldb] [mlir] [Profile] Add binary profile correlation for code coverage. (PR #69493)
ZequanWu wrote: > Well, seems like someone broke ToT w/ a compiler error. I'll let you know if > the forward fix fails to address the issue. The latest build passed: https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8761696377585255057/overview. https://github.com/llvm/llvm-project/pull/69493 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 4004f65 - [LLDB][NativePDB] Fix use-after-free error detected by asan.
Author: Zequan Wu
Date: 2024-01-04T12:32:40-05:00
New Revision: 4004f655ceb9623608ba0471aa7037c142956e31
URL:
https://github.com/llvm/llvm-project/commit/4004f655ceb9623608ba0471aa7037c142956e31
DIFF:
https://github.com/llvm/llvm-project/commit/4004f655ceb9623608ba0471aa7037c142956e31.diff
LOG: [LLDB][NativePDB] Fix use-after-free error detected by asan.
Added:
Modified:
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 8375010ae3dedd..9234768323e713 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -2265,7 +2265,8 @@ void SymbolFileNativePDB::BuildParentMap() {
}
for (TypeIndex fwd : fwd_keys) {
TypeIndex full = forward_to_full[fwd];
-m_parent_types[full] = m_parent_types[fwd];
+TypeIndex parent_idx = m_parent_types[fwd];
+m_parent_types[full] = parent_idx;
}
for (TypeIndex full : full_keys) {
TypeIndex fwd = full_to_forward[full];
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Re-land [lldb-dap] Add support for data breakpoint. (PR #81909)
ZequanWu wrote: Ping. https://github.com/llvm/llvm-project/pull/81909 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Re-land [lldb-dap] Add support for data breakpoint. (PR #81909)
https://github.com/ZequanWu closed https://github.com/llvm/llvm-project/pull/81909 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Followup fixs for data breakpoints (PR #81680)
https://github.com/ZequanWu closed https://github.com/llvm/llvm-project/pull/81680 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][dap] Avoid concurrent `HandleCommand` calls (PR #83162)
https://github.com/ZequanWu approved this pull request. https://github.com/llvm/llvm-project/pull/83162 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Deduplicate watchpoints starting at the same address on SetDataBreakpointsRequest. (PR #83192)
https://github.com/ZequanWu created
https://github.com/llvm/llvm-project/pull/83192
If a SetDataBreakpointsRequest contains a list data breakpoints which have
duplicate starting addresses, the current behaviour is returning `{verified:
true}` to both watchpoints with duplicated starting addresses. This confuses
the client and what actually happens in lldb is the second one overwrite the
first one.
This fixes it by letting the last watchpoint at given address have `{verified:
true}` and all previous watchpoints at the same address should have `{verfied:
false}` at response.
>From 563a4e5c9306fb5f06bdcc4a1b71dc92efbc86f8 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Tue, 27 Feb 2024 16:40:38 -0500
Subject: [PATCH] [lldb-dap] Deduplicate watchpoints starting at the same
address on SetDataBreakpointsRequest.
---
.../TestDAP_setDataBreakpoints.py | 45 +++
lldb/tools/lldb-dap/Watchpoint.cpp| 23 +-
lldb/tools/lldb-dap/Watchpoint.h | 5 +++
lldb/tools/lldb-dap/lldb-dap.cpp | 16 ++-
4 files changed, 78 insertions(+), 11 deletions(-)
diff --git
a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
index 17cdad89aa6d10..52c0bbfb33dad8 100644
--- a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
+++ b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
@@ -12,6 +12,51 @@ def setUp(self):
lldbdap_testcase.DAPTestCaseBase.setUp(self)
self.accessTypes = ["read", "write", "readWrite"]
+@skipIfWindows
+@skipIfRemote
+def test_duplicate_start_addresses(self):
+"""Test setDataBreakpoints with multiple watchpoints starting at the
same addresses."""
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = "main.cpp"
+first_loop_break_line = line_number(source, "// first loop breakpoint")
+self.set_source_breakpoints(source, [first_loop_break_line])
+self.continue_to_next_stop()
+self.dap_server.get_stackFrame()
+# Test setting write watchpoint using expressions: &x, arr+2
+response_x = self.dap_server.request_dataBreakpointInfo(0, "&x")
+response_arr_2 = self.dap_server.request_dataBreakpointInfo(0, "arr+2")
+# Test response from dataBreakpointInfo request.
+self.assertEquals(response_x["body"]["dataId"].split("/")[1], "4")
+self.assertEquals(response_x["body"]["accessTypes"], self.accessTypes)
+self.assertEquals(response_arr_2["body"]["dataId"].split("/")[1], "4")
+self.assertEquals(response_arr_2["body"]["accessTypes"],
self.accessTypes)
+# The first one should be overwritten by the third one as they start at
+# the same address. This is indicated by returning {verified: False}
for
+# the first one.
+dataBreakpoints = [
+{"dataId": response_x["body"]["dataId"], "accessType": "read"},
+{"dataId": response_arr_2["body"]["dataId"], "accessType":
"write"},
+{"dataId": response_x["body"]["dataId"], "accessType": "write"},
+]
+set_response =
self.dap_server.request_setDataBreakpoint(dataBreakpoints)
+self.assertEquals(
+set_response["body"]["breakpoints"],
+[{"verified": False}, {"verified": True}, {"verified": True}],
+)
+
+self.continue_to_next_stop()
+x_val = self.dap_server.get_local_variable_value("x")
+i_val = self.dap_server.get_local_variable_value("i")
+self.assertEquals(x_val, "2")
+self.assertEquals(i_val, "1")
+
+self.continue_to_next_stop()
+arr_2 = self.dap_server.get_local_variable_child("arr", "[2]")
+i_val = self.dap_server.get_local_variable_value("i")
+self.assertEquals(arr_2["value"], "42")
+self.assertEquals(i_val, "2")
+
@skipIfWindows
@skipIfRemote
def test_expression(self):
diff --git a/lldb/tools/lldb-dap/Watchpoint.cpp
b/lldb/tools/lldb-dap/Watchpoint.cpp
index 2f176e0da84f15..21765509449140 100644
--- a/lldb/tools/lldb-dap/Watchpoint.cpp
+++ b/lldb/tools/lldb-dap/Watchpoint.cpp
@@ -16,17 +16,11 @@ Watchpoint::Watchpoint(const llvm::json::Object &obj) :
BreakpointBase(obj) {
llvm::StringRef dataId = GetString(obj, "dataId");
std::string accessType = GetString(obj, "accessType").str();
auto [addr_str, size_str] = dataId.split('/');
- lldb::addr_t addr;
- size_t size;
llvm::to_integer(addr_str, addr, 16);
llvm::to_integer(size_str, size);
- lldb::SBWatchpointOptions options;
options.SetWatchpointTypeRead(accessType != "write");
if (accessType != "read")
options.SetWatchpointTypeWrite(lldb::eWatchpointWriteTypeOnModify);
- wp = g_dap.target.WatchpointCreateByAddress(addr, size, options, error);
- SetCondition();
- SetHitConditi
[Lldb-commits] [lldb] [lldb-dap] Deduplicate watchpoints starting at the same address on SetDataBreakpointsRequest. (PR #83192)
https://github.com/ZequanWu closed https://github.com/llvm/llvm-project/pull/83192 ___ 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 dwarf parse time for line table and .debug_abbrev. (PR #86568)
https://github.com/ZequanWu created
https://github.com/llvm/llvm-project/pull/86568
`ParseLineTable` not only parses .debug_line but also constructs `LineTable`.
This moves `m_parse_time` into the the function body of `ParseLLVMLineTable` to
more accurately reflect parsing time on .debug_line. This also add missing
timer when parsing `.debug_abbrev`.
>From 19dd9a13c21d70b42b9d68aed6fb0b5a5e494685 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Mon, 25 Mar 2024 15:49:42 -0400
Subject: [PATCH] [lldb][Dwarf] Fix dwarf parse time for line table and
.debug_abbrev.
---
.../Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 08ce7b82b0c16a..8039a35ed8941c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -145,8 +145,10 @@ static PluginProperties &GetGlobalPluginProperties() {
static const llvm::DWARFDebugLine::LineTable *
ParseLLVMLineTable(DWARFContext &context, llvm::DWARFDebugLine &line,
- dw_offset_t line_offset, dw_offset_t unit_offset) {
+ dw_offset_t line_offset, dw_offset_t unit_offset,
+ StatsDuration &parse_time) {
Log *log = GetLog(DWARFLog::DebugInfo);
+ ElapsedTime elapsed(parse_time);
llvm::DWARFDataExtractor data = context.getOrLoadLineData().GetAsLLVMDWARF();
llvm::DWARFContext &ctx = context.GetAsLLVM();
@@ -693,6 +695,7 @@ llvm::DWARFDebugAbbrev *SymbolFileDWARF::DebugAbbrev() {
if (debug_abbrev_data.GetByteSize() == 0)
return nullptr;
+ ElapsedTime elapsed(m_parse_time);
auto abbr =
std::make_unique(debug_abbrev_data.GetAsLLVM());
llvm::Error error = abbr->parse();
@@ -1228,10 +1231,9 @@ bool SymbolFileDWARF::ParseLineTable(CompileUnit
&comp_unit) {
if (offset == DW_INVALID_OFFSET)
return false;
- ElapsedTime elapsed(m_parse_time);
llvm::DWARFDebugLine line;
- const llvm::DWARFDebugLine::LineTable *line_table =
- ParseLLVMLineTable(m_context, line, offset, dwarf_cu->GetOffset());
+ const llvm::DWARFDebugLine::LineTable *line_table = ParseLLVMLineTable(
+ m_context, line, offset, dwarf_cu->GetOffset(), m_parse_time);
if (!line_table)
return false;
___
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 dwarf parse time for line table and .debug_abbrev. (PR #86568)
@@ -1228,10 +1231,9 @@ bool SymbolFileDWARF::ParseLineTable(CompileUnit
&comp_unit) {
if (offset == DW_INVALID_OFFSET)
return false;
- ElapsedTime elapsed(m_parse_time);
ZequanWu wrote:
My understanding is m_parse_time is the time spent only on parsing the raw data
and not about constructing the data structures which will be used by lldb. In
https://github.com/llvm/llvm-project/blob/b7611370491873722e08e4ce9374312d0c936af1/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp#L223,
m_parse_time is used to only track the time spent on parsing a DIE in
.debug_info, which also doesn't include the time on constructing necessary data
structures. This change is trying to do the same by narrowing the time to just
on parsing with llvm.
https://github.com/llvm/llvm-project/pull/86568
___
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 dwarf parse time for line table and .debug_abbrev. (PR #86568)
https://github.com/ZequanWu updated
https://github.com/llvm/llvm-project/pull/86568
>From 19dd9a13c21d70b42b9d68aed6fb0b5a5e494685 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Mon, 25 Mar 2024 15:49:42 -0400
Subject: [PATCH 1/2] [lldb][Dwarf] Fix dwarf parse time for line table and
.debug_abbrev.
---
.../Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 08ce7b82b0c16a..8039a35ed8941c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -145,8 +145,10 @@ static PluginProperties &GetGlobalPluginProperties() {
static const llvm::DWARFDebugLine::LineTable *
ParseLLVMLineTable(DWARFContext &context, llvm::DWARFDebugLine &line,
- dw_offset_t line_offset, dw_offset_t unit_offset) {
+ dw_offset_t line_offset, dw_offset_t unit_offset,
+ StatsDuration &parse_time) {
Log *log = GetLog(DWARFLog::DebugInfo);
+ ElapsedTime elapsed(parse_time);
llvm::DWARFDataExtractor data = context.getOrLoadLineData().GetAsLLVMDWARF();
llvm::DWARFContext &ctx = context.GetAsLLVM();
@@ -693,6 +695,7 @@ llvm::DWARFDebugAbbrev *SymbolFileDWARF::DebugAbbrev() {
if (debug_abbrev_data.GetByteSize() == 0)
return nullptr;
+ ElapsedTime elapsed(m_parse_time);
auto abbr =
std::make_unique(debug_abbrev_data.GetAsLLVM());
llvm::Error error = abbr->parse();
@@ -1228,10 +1231,9 @@ bool SymbolFileDWARF::ParseLineTable(CompileUnit
&comp_unit) {
if (offset == DW_INVALID_OFFSET)
return false;
- ElapsedTime elapsed(m_parse_time);
llvm::DWARFDebugLine line;
- const llvm::DWARFDebugLine::LineTable *line_table =
- ParseLLVMLineTable(m_context, line, offset, dwarf_cu->GetOffset());
+ const llvm::DWARFDebugLine::LineTable *line_table = ParseLLVMLineTable(
+ m_context, line, offset, dwarf_cu->GetOffset(), m_parse_time);
if (!line_table)
return false;
>From ef97e93b9b26c6a1923ff3b6c3dad76a67f2b64d Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Tue, 26 Mar 2024 10:46:18 -0400
Subject: [PATCH 2/2] revert change in ParseLineTable
---
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 8039a35ed8941c..0f459706c05643 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -145,10 +145,8 @@ static PluginProperties &GetGlobalPluginProperties() {
static const llvm::DWARFDebugLine::LineTable *
ParseLLVMLineTable(DWARFContext &context, llvm::DWARFDebugLine &line,
- dw_offset_t line_offset, dw_offset_t unit_offset,
- StatsDuration &parse_time) {
+ dw_offset_t line_offset, dw_offset_t unit_offset) {
Log *log = GetLog(DWARFLog::DebugInfo);
- ElapsedTime elapsed(parse_time);
llvm::DWARFDataExtractor data = context.getOrLoadLineData().GetAsLLVMDWARF();
llvm::DWARFContext &ctx = context.GetAsLLVM();
@@ -1231,9 +1229,10 @@ bool SymbolFileDWARF::ParseLineTable(CompileUnit
&comp_unit) {
if (offset == DW_INVALID_OFFSET)
return false;
+ ElapsedTime elapsed(m_parse_time);
llvm::DWARFDebugLine line;
- const llvm::DWARFDebugLine::LineTable *line_table = ParseLLVMLineTable(
- m_context, line, offset, dwarf_cu->GetOffset(), m_parse_time);
+ const llvm::DWARFDebugLine::LineTable *line_table =
+ ParseLLVMLineTable(m_context, line, offset, dwarf_cu->GetOffset());
if (!line_table)
return false;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Dwarf] Add missing timer when parsing .debug_abbrev. (PR #86568)
https://github.com/ZequanWu edited https://github.com/llvm/llvm-project/pull/86568 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Dwarf] Add missing timer when parsing .debug_abbrev. (PR #86568)
https://github.com/ZequanWu edited https://github.com/llvm/llvm-project/pull/86568 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Dwarf] Add missing timer when parsing .debug_abbrev. (PR #86568)
https://github.com/ZequanWu closed https://github.com/llvm/llvm-project/pull/86568 ___ 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 created
https://github.com/llvm/llvm-project/pull/88792
If user sets a breakpoint at `_dl_debug_state` before the process launched, the
breakpoint is not resolved yet. When lldb loads dynamic loader module, it's
created with `Target::GetOrCreateModule` which notifies any pending breakpoint
to resolve. However, the module's sections are not loaded at this time. They
are loaded after returned from
[Target::GetOrCreateModule](https://github.com/llvm/llvm-project/blob/0287a5cc4e2a5ded1ae2e4079f91052e6a6b8d9b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L574-L577).
This change fixes it by manually resolving breakpoints after creating dynamic
loader module.
>From 26528cd679478448edf446e0e82e5f207ffd6113 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Mon, 15 Apr 2024 16:30:38 -0400
Subject: [PATCH] [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 &module_list, bool added,
+ bool delete_locations);
+
void ModulesDidLoad(ModuleList &module_list);
void ModulesDidUnload(ModuleList &module_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 &module_list) {
ModulesDidUnload(module_list, false);
}
+void Target::UpdateBreakpoints(ModuleList &module_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 &module_list) {
const size_t num_images = module_list.GetSize();
if (m_valid && num_images) {
@@ -1694,8 +1701,7 @@ void Target::ModulesDidLoad(ModuleList &module_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 &module_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 &module_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,
-
[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: > why not just call ModulesDidLoad and delegate this (and possibly other > binary-just-loaded) behaviors to it? That's what I did first, but it breaks the test `TestModuleLoadedNotifys.ModuleLoadedNotifysTestCase.test_launch_notifications` because of extra broadcaster event. So, I moved out the part of the part just updating breakpoints. 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)
ZequanWu wrote: > I'm missing why the _dl_debug_state breakpoint is special here, such that it > requires a force load of the section info? How does that happen? Jason's comment explains it well. It's because ld.so's loading is special here. Normal binaries are loaded via `DynamicLoaderPOSIXDYLD::RefreshModules` which updates loaded section addresses. 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/2] [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 &module_list, bool added,
+ bool delete_locations);
+
void ModulesDidLoad(ModuleList &module_list);
void ModulesDidUnload(ModuleList &module_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 &module_list) {
ModulesDidUnload(module_list, false);
}
+void Target::UpdateBreakpoints(ModuleList &module_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 &module_list) {
const size_t num_images = module_list.GetSize();
if (m_valid && num_images) {
@@ -1694,8 +1701,7 @@ void Target::ModulesDidLoad(ModuleList &module_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 &module_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 &module_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
+++
b/lldb/test/API/fun
[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/3] [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 &module_list, bool added,
+ bool delete_locations);
+
void ModulesDidLoad(ModuleList &module_list);
void ModulesDidUnload(ModuleList &module_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 &module_list) {
ModulesDidUnload(module_list, false);
}
+void Target::UpdateBreakpoints(ModuleList &module_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 &module_list) {
const size_t num_images = module_list.GetSize();
if (m_valid && num_images) {
@@ -1694,8 +1701,7 @@ void Target::ModulesDidLoad(ModuleList &module_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 &module_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 &module_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
+++
b/lldb/test/API/fun
[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: > > > why not just call ModulesDidLoad and delegate this (and possibly other > > > binary-just-loaded) behaviors to it? > > > > > > That's what I did first, but it breaks the test > > `TestModuleLoadedNotifys.ModuleLoadedNotifysTestCase.test_launch_notifications` > > because of extra broadcaster event. So, I moved out the part just updating > > breakpoints. > > The point of the test is to ensure you don't get a hundred notifications, one > for each module. Having one notification for ld.so, and 99 for the rest of > the modules is ok. It should be fine to just update the test to match new > reality. Updated to use `Target::ModulesDisLoad` and the test. 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)
ZequanWu wrote: > Can you check where does the second event get sent from? Is it by any chance > when we go through > [this](https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L460) > place ? The first event is sent when creating the module for ld.so: https://github.com/llvm/llvm-project/blob/458328ae23d318a5055d5bac66426b8551bce01f/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L574. It gets notified at here: https://github.com/llvm/llvm-project/blob/458328ae23d318a5055d5bac66426b8551bce01f/lldb/source/Target/Target.cpp#L1660. The second event is sent because I explicitly calls `ModulesDidLoad` in this change (the current version) after the creating the ld.so module. 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/4] [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 &module_list, bool added,
+ bool delete_locations);
+
void ModulesDidLoad(ModuleList &module_list);
void ModulesDidUnload(ModuleList &module_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 &module_list) {
ModulesDidUnload(module_list, false);
}
+void Target::UpdateBreakpoints(ModuleList &module_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 &module_list) {
const size_t num_images = module_list.GetSize();
if (m_valid && num_images) {
@@ -1694,8 +1701,7 @@ void Target::ModulesDidLoad(ModuleList &module_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 &module_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 &module_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
+++
b/lldb/test/API/fun
[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
[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 &module_list, bool added,
+ bool delete_locations);
+
void ModulesDidLoad(ModuleList &module_list);
void ModulesDidUnload(ModuleList &module_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 &module_list) {
ModulesDidUnload(module_list, false);
}
+void Target::UpdateBreakpoints(ModuleList &module_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 &module_list) {
const size_t num_images = module_list.GetSize();
if (m_valid && num_images) {
@@ -1694,8 +1701,7 @@ void Target::ModulesDidLoad(ModuleList &module_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 &module_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 &module_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
+++
b/lldb/test/API/fun
[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 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-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] 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] 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] [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
&sc,
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 &sc,
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, G
[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 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] 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 &ast,
ClangASTImporter &ast_importer,
clang::DeclContext *decl_ctx,
- DWARFDIE die,
+ const DWARFDIE &decl_ctx_die,
+ const DWARFDIE &die,
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
&ast,
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->
