[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
dwblaikie wrote: & FWIW, I think it is valid to include these declarations as entries, though not as named/index entries, per the spec: > It is possible that an indexed debugging information entry has a parent that > is not indexed (for example, if its parent does not have a name attribute). > In such a case, a parent attribute may point to a nameless index entry (that > is, one that cannot be reached from any entry in the name table), or it may > point to the nearest ancestor that does have an index entry So I think you could have a parent that's a declaration, represented as a nameless index entry. That'd allow faster comparisons when examining the entry for the named child that had such a parent - because you could potentially check that named entry's fully qualified name directly from the named entry and the parents - without needing to parse all the DIEs in the CU to walk and find the start of the parents. But I don't believe clang is producing such DWARF today. 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] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
dwblaikie wrote: What's an actual test case for this issue? The example given above doesn't look like it'd produce entries for the declaration of ios_base? Like here's something that looks equivalent, but is complete, and doesn't have a DW_IDX_parent on the nested typedef entry in the index: https://godbolt.org/z/efjbze3x1 it'd be good to have a reproducer for this to motivate the discussion... 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] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -24,13 +24,16 @@ class UniqueDWARFASTType {
UniqueDWARFASTType() : m_type_sp(), m_die(), m_declaration() {}
UniqueDWARFASTType(lldb::TypeSP _sp, const DWARFDIE ,
- const Declaration , int32_t byte_size)
+ const Declaration , int32_t byte_size,
+ bool is_forward_declaration)
: m_type_sp(type_sp), m_die(die), m_declaration(decl),
-m_byte_size(byte_size) {}
+m_byte_size(byte_size),
+m_is_forward_declaration(is_forward_declaration) {}
dwblaikie wrote:
Ah, cool - thanks, sorry I missed that.
If you wanted to remove the dead ctor either before or after this commit, that
might be handy/nice.
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -0,0 +1,40 @@
+# Test definition DIE searching is delayed until complete type is required.
+
+# RUN: split-file %s %t
+# RUN: %clangxx_host %t/main.cpp %t/t1_def.cpp -g -gsimple-template-names -o
%t.out
+# RUN: %lldb -b %t.out -s %t/lldb.cmd | FileCheck %s
+
+# CHECK: (lldb) p v1
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type
(DW_TAG_structure_type) name = 't2'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type
(DW_TAG_structure_type) name = 't1'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type
(DW_TAG_base_type) name = 'int'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't2' resolving forward
declaration...
+# CHECK: (t2 >) {}
+# CHECK: (lldb) p v2
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type
(DW_TAG_structure_type) name = 't1'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type
(DW_TAG_base_type) name = 'int'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't1' resolving forward
declaration...
+# CHECK: (t1) {}
+
+#--- lldb.cmd
dwblaikie wrote:
Hmm - what about template parameters, even without simplified template names?
`t1` t2 should be a declaration, without looking up the definition DIE in
this case? (though perhaps clang/LLDB doesn't do that correctly, in which case
maybe `t1` might work)
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -24,13 +24,16 @@ class UniqueDWARFASTType {
UniqueDWARFASTType() : m_type_sp(), m_die(), m_declaration() {}
UniqueDWARFASTType(lldb::TypeSP _sp, const DWARFDIE ,
- const Declaration , int32_t byte_size)
+ const Declaration , int32_t byte_size,
+ bool is_forward_declaration)
: m_type_sp(type_sp), m_die(die), m_declaration(decl),
-m_byte_size(byte_size) {}
+m_byte_size(byte_size),
+m_is_forward_declaration(is_forward_declaration) {}
dwblaikie wrote:
Where is this ctor used? I can't find any calls to it.
And if it is never used, how is `m_is_forward_declaration` ever `false`, if
it's initialized to true and this ctor is never called, I don't see any other
place that assigns to it?
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1632,27 +1632,34 @@ bool SymbolFileDWARF::CompleteType(CompilerType
_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());
+ // 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.
+ DWARFDIE dwarf_die = GetDIE(die_it->second);
+ GetForwardDeclCompilerTypeToDIE().erase(die_it);
+ Type *type = nullptr;
+ if (DWARFASTParser *dwarf_ast = GetDWARFParser(*dwarf_die.GetCU()))
+type = dwarf_ast->FindDefinitionTypeForDIE(dwarf_die);
+ if (!type)
+return false;
-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);
+ die_it = GetForwardDeclCompilerTypeToDIE().find(
+ compiler_type_no_qualifiers.GetOpaqueQualType());
+ if (die_it != GetForwardDeclCompilerTypeToDIE().end()) {
+dwarf_die = GetDIE(die_it->getSecond());
+GetForwardDeclCompilerTypeToDIE().erase(die_it);
}
- return false;
+
+ Log *log = GetLog(DWARFLog::DebugInfo | DWARFLog::TypeCompletion);
+ if (log)
dwblaikie wrote:
```
if (Log *log = GetLog(DWARFLog::DebugInfo | DWARFLog::TypeCompletion))
```
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -0,0 +1,40 @@
+# Test definition DIE searching is delayed until complete type is required.
+
+# RUN: split-file %s %t
+# RUN: %clangxx_host %t/main.cpp %t/t1_def.cpp -g -gsimple-template-names -o
%t.out
+# RUN: %lldb -b %t.out -s %t/lldb.cmd | FileCheck %s
+
+# CHECK: (lldb) p v1
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type
(DW_TAG_structure_type) name = 't2'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type
(DW_TAG_structure_type) name = 't1'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type
(DW_TAG_base_type) name = 'int'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't2' resolving forward
declaration...
+# CHECK: (t2 >) {}
+# CHECK: (lldb) p v2
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type
(DW_TAG_structure_type) name = 't1'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type
(DW_TAG_base_type) name = 'int'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't1' resolving forward
declaration...
+# CHECK: (t1) {}
+
+#--- lldb.cmd
dwblaikie wrote:
Might be better to test this without `-gsimple-template-names`, because it's a
more generally valuable feature than that?
Oh, but that "look for the child count through a pointer" issue comes up and
it's hard to reach this situation without `-gsimple-template-names`? (Maybe
indirectly - if you print a struct with a pointer - perhaps that doesn't do the
child count searching through the pointer?)
https://github.com/llvm/llvm-project/pull/90663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -16,61 +16,66 @@ using namespace lldb_private::plugin::dwarf;
bool UniqueDWARFASTTypeList::Find(const DWARFDIE ,
const lldb_private::Declaration ,
const int32_t byte_size,
+ bool is_forward_declaration,
UniqueDWARFASTType ) const {
for (const UniqueDWARFASTType : m_collection) {
// Make sure the tags match
if (udt.m_die.Tag() == die.Tag()) {
- // Validate byte sizes of both types only if both are valid.
- if (udt.m_byte_size < 0 || byte_size < 0 ||
- udt.m_byte_size == byte_size) {
-// Make sure the file and line match
-if (udt.m_declaration == decl) {
- // The type has the same name, and was defined on the same file and
- // line. Now verify all of the parent DIEs match.
- DWARFDIE parent_arg_die = die.GetParent();
- DWARFDIE parent_pos_die = udt.m_die.GetParent();
- bool match = true;
- bool done = false;
- while (!done && match && parent_arg_die && parent_pos_die) {
-const dw_tag_t parent_arg_tag = parent_arg_die.Tag();
-const dw_tag_t parent_pos_tag = parent_pos_die.Tag();
-if (parent_arg_tag == parent_pos_tag) {
- switch (parent_arg_tag) {
- case DW_TAG_class_type:
- case DW_TAG_structure_type:
- case DW_TAG_union_type:
- case DW_TAG_namespace: {
-const char *parent_arg_die_name = parent_arg_die.GetName();
-if (parent_arg_die_name ==
-nullptr) // Anonymous (i.e. no-name) struct
-{
- match = false;
-} else {
- const char *parent_pos_die_name = parent_pos_die.GetName();
- if (parent_pos_die_name == nullptr ||
- ((parent_arg_die_name != parent_pos_die_name) &&
- strcmp(parent_arg_die_name, parent_pos_die_name)))
-match = false;
-}
- } break;
-
- case DW_TAG_compile_unit:
- case DW_TAG_partial_unit:
-done = true;
-break;
- default:
-break;
- }
+ // If they are not both definition DIEs or both declaration DIEs, then
+ // don't check for byte size and declaration location, because
declaration
+ // DIEs usually don't have those info.
+ bool matching_size_declaration =
+ udt.m_is_forward_declaration != is_forward_declaration
+ ? true
+ : (udt.m_byte_size < 0 || byte_size < 0 ||
+ udt.m_byte_size == byte_size) &&
+udt.m_declaration == decl;
+ if (!matching_size_declaration)
+continue;
+ // The type has the same name, and was defined on the same file and
+ // line. Now verify all of the parent DIEs match.
+ DWARFDIE parent_arg_die = die.GetParent();
+ DWARFDIE parent_pos_die = udt.m_die.GetParent();
+ bool match = true;
+ bool done = false;
+ while (!done && match && parent_arg_die && parent_pos_die) {
+const dw_tag_t parent_arg_tag = parent_arg_die.Tag();
+const dw_tag_t parent_pos_tag = parent_pos_die.Tag();
+if (parent_arg_tag == parent_pos_tag) {
+ switch (parent_arg_tag) {
+ case DW_TAG_class_type:
+ case DW_TAG_structure_type:
+ case DW_TAG_union_type:
+ case DW_TAG_namespace: {
+const char *parent_arg_die_name = parent_arg_die.GetName();
+if (parent_arg_die_name ==
+nullptr) // Anonymous (i.e. no-name) struct
+{
dwblaikie wrote:
The comment between the `)` and `{` creates some weird line wrapping, maybe put
the comment inside the `{` block?
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] 41574f5 - Add new BuiltinType introduced in 7a484d3a1f630ba9ce7b22e744818be974971470
Author: David Blaikie Date: 2024-05-05T17:59:54Z New Revision: 41574f5a6e2d961f398d3c671c34ac3c8e417464 URL: https://github.com/llvm/llvm-project/commit/41574f5a6e2d961f398d3c671c34ac3c8e417464 DIFF: https://github.com/llvm/llvm-project/commit/41574f5a6e2d961f398d3c671c34ac3c8e417464.diff LOG: Add new BuiltinType introduced in 7a484d3a1f630ba9ce7b22e744818be974971470 I don't think this is one lldb would encounter when building ASTs from DWARF. Added: Modified: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp Removed: diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 3bdb288e97dd6a..a771016039e851 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -4996,6 +4996,9 @@ lldb::Encoding TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type, case clang::BuiltinType::IncompleteMatrixIdx: break; + +case clang::BuiltinType::UnresolvedTemplate: + break; } break; // All pointer types are represented as unsigned integer encodings. We may ___ 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)
dwblaikie wrote: Hmm - but the byte size wouldn't be known from only a declaration, right? so how'd that key work between a declaration and a definition? 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)
dwblaikie wrote: does this cause multiple (an open ended amount?) of declarations for a type to be created if the type declarations from multiple CUs are encountered separately? Or still only one due to the extra map? 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] [llvm] lldb simplified template names rebuild without clang ast (PR #90008)
https://github.com/dwblaikie created
https://github.com/llvm/llvm-project/pull/90008
- DO NOT SUBMIT: lldb DWARF-Clang AST parsing tracing
- Fix scope operator ordering
- DO NOT SUBMIT: Really dodgy demonstration of DWARFTypePrinter reuse in lldb
>From 863343317c47602163d75c13b2687601740e8410 Mon Sep 17 00:00:00 2001
From: David Blaikie
Date: Fri, 19 Apr 2024 03:34:27 +
Subject: [PATCH 1/3] DO NOT SUBMIT: lldb DWARF-Clang AST parsing tracing
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp| 13 +
1 file changed, 13 insertions(+)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 54d06b1115a229..17445e0c3ad17d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -7,6 +7,7 @@
//===--===//
#include
+#include
#include "DWARFASTParser.h"
#include "DWARFASTParserClang.h"
@@ -1556,6 +1557,15 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const
DWARFDIE ) {
const char *name = die.GetName();
if (!name)
return "";
+ static int indent = 0;
+ std::cerr << std::string(indent, ' ') << "starting qualified name for: " <<
name << '\n';
+ auto = die.GetCU()->GetSymbolFileDWARF().GetObjectFile()->GetFileSpec();
+ std::string Directory = FS.GetDirectory().AsCString("");
+ std::cerr << std::string(indent, ' ')
+<< Directory.substr(std::min(59ul, Directory.size())) << '/'
+<< FS.GetFilename().AsCString("") << ':' << std::hex
+<< die.GetDIE()->GetOffset() << '\n';
+ ++indent;
std::string qualified_name;
DWARFDIE parent_decl_ctx_die = die.GetParentDeclContextDIE();
// TODO: change this to get the correct decl context parent
@@ -1601,6 +1611,9 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const
DWARFDIE ) {
qualified_name.append(name);
qualified_name.append(GetDIEClassTemplateParams(die).AsCString(""));
+ --indent;
+ std::cerr << std::string(indent, ' ') << "computed qualified name: " <<
qualified_name << '\n';
+
return qualified_name;
}
>From b2926499710b7bf673111aabc5be51597a46493c Mon Sep 17 00:00:00 2001
From: David Blaikie
Date: Thu, 25 Apr 2024 00:28:57 +
Subject: [PATCH 2/3] Fix scope operator ordering
---
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 17445e0c3ad17d..ab181b482e4fbc 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1590,9 +1590,9 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const
DWARFDIE ) {
case DW_TAG_structure_type:
case DW_TAG_union_type: {
if (const char *class_union_struct_name = parent_decl_ctx_die.GetName())
{
+qualified_name.insert(0, "::");
qualified_name.insert(
0, GetDIEClassTemplateParams(parent_decl_ctx_die).AsCString(""));
-qualified_name.insert(0, "::");
qualified_name.insert(0, class_union_struct_name);
}
parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
>From 9a654b056d9c05c0aa4856db161c1f1b08b9dfe9 Mon Sep 17 00:00:00 2001
From: David Blaikie
Date: Thu, 25 Apr 2024 00:46:48 +
Subject: [PATCH 3/3] DO NOT SUBMIT: Really dodgy demonstration of
DWARFTypePrinter reuse in lldb
The hacks necessary to make lldb's DWARFDIE APIs sufficiently compatible
with LLVM's DWARFDie API aren't shippable, but maybe somewhere to start
the conversation.
With all these changes, an internal example that would crash expanding
too many types (computing the fully qualified name for 414671 types before
crashing due to running out of stack) - but with these patches applied,
it comes down to 856 expansions (compared to 848 for non-simplified
template names inputs)
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 11 +
.../Plugins/SymbolFile/DWARF/DWARFBaseDIE.h | 10 +
.../Plugins/SymbolFile/DWARF/DWARFDIE.cpp | 21 +-
.../Plugins/SymbolFile/DWARF/DWARFDIE.h | 13 +
.../Plugins/SymbolFile/DWARF/DWARFFormValue.h | 37 +
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 24 +-
llvm/include/llvm-c/Error.h | 8 +
llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h | 2 +
.../llvm/DebugInfo/DWARF/DWARFFormValue.h | 7 +-
.../llvm/DebugInfo/DWARF/DWARFTypePrinter.h | 734 +-
llvm/lib/DebugInfo/DWARF/DWARFDie.cpp | 4 +-
llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp | 674
12 files changed, 830 insertions(+), 715 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
[Lldb-commits] [lldb] [lldb/test] Add basic ld.lld --debug-names tests (PR #88335)
dwblaikie wrote: looks approximately right to me, but wouldn't' mind a set of eyes more familiar with lldb to take a look https://github.com/llvm/llvm-project/pull/88335 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
@@ -273,6 +301,44 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
if (!isType(entry.tag()))
continue;
+
+DWARFTypeUnit *foreign_tu = GetForeignTypeUnit(entry);
+if (foreign_tu) {
+ // If this entry represents a foreign type unit, we need to verify that
+ // the type unit that ended up in the final .dwp file is the right type
+ // unit. Type units have signatures which are the same across multiple
+ // .dwo files, but only one of those type units will end up in the .dwp
+ // file. The contents of type units for the same type can be different
+ // in different .dwo file, which means the DIE offsets might not be the
+ // same between two different type units. So we need to determine if this
+ // accelerator table matches the type unit in the .dwp file. If it
doesn't
+ // match, then we need to ignore this accelerator table entry as the type
+ // unit that is in the .dwp file will have its own index.
+ const llvm::DWARFDebugNames::NameIndex *name_index =
entry.getNameIndex();
+ if (name_index == nullptr)
+continue;
+ // In order to determine if the type unit that ended up in a .dwp file
+ // is valid, we need to grab the type unit and check the attribute on the
+ // type unit matches the .dwo file. For this to happen we rely on each
+ // .dwo file having its own .debug_names table with a single compile unit
+ // and multiple type units. This is the only way we can tell if a type
+ // unit came from a specific .dwo file.
dwblaikie wrote:
Ah, indeed - no, currently there's no well defined way to do LTO (LTO is
basically the only time you get multiple CUs in a single .o straight out of the
compiler) and Split DWARF (there's that long-standing issue that Cary and I
should discuss how that should work & propose something to the DWARF committee,
but we haven't as-yet).
I guess even if we did make that work, all the skeleton CUs in the dwo file
would have the same DW_AT_dwo_name
https://github.com/llvm/llvm-project/pull/87740
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
@@ -0,0 +1,91 @@
+// REQUIRES: lld
+
+// 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
+// differing contents. I have discovered that the hash for the type unit is
+// simply based off of the typename and doesn't seem to differ when the
contents
+// differ, so that will help us test foreign type units in the .debug_names
+// section of the main executable. When a DWP file is made, only one type unit
+// will be kept and the type unit that is kept has the .dwo file name that it
+// came from. When LLDB loads the foreign type units, it needs to verify that
+// any entries from foreign type units come from the right .dwo file. We test
+// this since the contents of type units are not always the same even though
+// they have the same type hash. We don't want invalid accelerator table
entries
+// to come from one .dwo file and be used on a type unit from another since
this
+// could cause invalid lookups to happen. LLDB knows how to track down which
+// .dwo file a type unit comes from by looking at the DW_AT_dwo_name attribute
+// in the DW_TAG_type_unit.
+
+// Now test with DWARF5
+// RUN: %clang -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf \
+// RUN: -fdebug-types-section -gpubnames -c %s -o %t.main.o
+// RUN: %clang -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -DVARIANT \
+// RUN: -fdebug-types-section -gpubnames -c %s -o %t.foo.o
+// RUN: ld.lld %t.main.o %t.foo.o -o %t
+
+// First we check when we make the .dwp file with %t.main.dwo first so it will
+// pick the type unit from %t.main.dwo. Verify we find only the types from
+// %t.main.dwo's type unit.
+// RUN: llvm-dwp %t.main.dwo %t.foo.dwo -o %t.dwp
+// RUN: %lldb \
+// RUN: -o "type lookup IntegerType" \
+// RUN: -o "type lookup FloatType" \
+// RUN: -o "type lookup IntegerType" \
+// RUN: -b %t | FileCheck %s
+// CHECK: (lldb) type lookup IntegerType
+// CHECK-NEXT: int
+// CHECK-NEXT: (lldb) type lookup FloatType
+// CHECK-NEXT: double
+// CHECK-NEXT: (lldb) type lookup IntegerType
+// CHECK-NEXT: int
+
+// Next we check when we make the .dwp file with %t.foo.dwo first so it will
+// pick the type unit from %t.main.dwo. Verify we find only the types from
+// %t.main.dwo's type unit.
+// RUN: llvm-dwp %t.foo.dwo %t.main.dwo -o %t.dwp
+// RUN: %lldb \
+// RUN: -o "type lookup IntegerType" \
+// RUN: -o "type lookup FloatType" \
+// RUN: -o "type lookup IntegerType" \
+// RUN: -b %t | FileCheck %s --check-prefix=VARIANT
+
+// VARIANT: (lldb) type lookup IntegerType
+// VARIANT-NEXT: unsigned int
+// VARIANT-NEXT: (lldb) type lookup FloatType
+// VARIANT-NEXT: float
+// VARIANT-NEXT: (lldb) type lookup IntegerType
+// VARIANT-NEXT: unsigned int
+
+
+// We need to do this so we end with a type unit in each .dwo file and that has
+// the same signature but different contents. When we make the .dwp file, then
+// one of the type units will end up in the .dwp file and we will have
+// .debug_names accelerator tables for both type units and we need to ignore
+// the type units .debug_names entries that don't match the .dwo file whose
+// copy of the type unit ends up in the final .dwp file. To do this, LLDB will
+// look at the type unit and take the DWO name attribute and make sure it
+// matches, and if it doesn't, it will ignore the accelerator table entry.
+struct CustomType {
+ // We switch the order of "FloatType" and "IntegerType" so that if we do
+ // end up reading the wrong accelerator table entry, that we would end up
+ // getting an invalid offset and not find anything, or the offset would have
+ // matched and we would find the wrong thing.
dwblaikie wrote:
Couldn't we have a difference between these type variants that's simpler - like
one version of the struct with a single `int` member, and one version of the
type with no members? Then do lookup of the `CustomType` and make sure the type
we get back is the one with the `int` member? (or not, depending on the test)
https://github.com/llvm/llvm-project/pull/87740
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
@@ -273,6 +301,44 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
if (!isType(entry.tag()))
continue;
+
+DWARFTypeUnit *foreign_tu = GetForeignTypeUnit(entry);
+if (foreign_tu) {
+ // If this entry represents a foreign type unit, we need to verify that
+ // the type unit that ended up in the final .dwp file is the right type
+ // unit. Type units have signatures which are the same across multiple
+ // .dwo files, but only one of those type units will end up in the .dwp
+ // file. The contents of type units for the same type can be different
+ // in different .dwo file, which means the DIE offsets might not be the
+ // same between two different type units. So we need to determine if this
+ // accelerator table matches the type unit in the .dwp file. If it
doesn't
+ // match, then we need to ignore this accelerator table entry as the type
+ // unit that is in the .dwp file will have its own index.
+ const llvm::DWARFDebugNames::NameIndex *name_index =
entry.getNameIndex();
+ if (name_index == nullptr)
+continue;
+ // In order to determine if the type unit that ended up in a .dwp file
+ // is valid, we need to grab the type unit and check the attribute on the
+ // type unit matches the .dwo file. For this to happen we rely on each
+ // .dwo file having its own .debug_names table with a single compile unit
+ // and multiple type units. This is the only way we can tell if a type
+ // unit came from a specific .dwo file.
dwblaikie wrote:
Sounds like this wouldn't work for a merged `.debug_names` table? Could you
leave a FIXME/do you plan to fix this?
Oh, it also wouldn't work for any kind of LTO which could have multiple CUs in
a single object file/dwo file.
(FYI @cmtice )
https://github.com/llvm/llvm-project/pull/87740
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
@@ -273,6 +301,44 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
if (!isType(entry.tag()))
continue;
+
+DWARFTypeUnit *foreign_tu = GetForeignTypeUnit(entry);
+if (foreign_tu) {
+ // If this entry represents a foreign type unit, we need to verify that
+ // the type unit that ended up in the final .dwp file is the right type
+ // unit. Type units have signatures which are the same across multiple
+ // .dwo files, but only one of those type units will end up in the .dwp
+ // file. The contents of type units for the same type can be different
+ // in different .dwo file, which means the DIE offsets might not be the
+ // same between two different type units. So we need to determine if this
+ // accelerator table matches the type unit in the .dwp file. If it
doesn't
+ // match, then we need to ignore this accelerator table entry as the type
+ // unit that is in the .dwp file will have its own index.
+ const llvm::DWARFDebugNames::NameIndex *name_index =
entry.getNameIndex();
+ if (name_index == nullptr)
+continue;
+ // In order to determine if the type unit that ended up in a .dwp file
+ // is valid, we need to grab the type unit and check the attribute on the
+ // type unit matches the .dwo file. For this to happen we rely on each
+ // .dwo file having its own .debug_names table with a single compile unit
+ // and multiple type units. This is the only way we can tell if a type
+ // unit came from a specific .dwo file.
dwblaikie wrote:
I think our conclusion from previous discussions was to put
`DW_IDX_compile_unit`, in addition to the `DW_IDX_type_unit` on the entries in
the .debug_names table - and add the CU column to the .debug_tu_index in the
dwp file, to match these things up?
https://github.com/llvm/llvm-project/pull/87740
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
@@ -0,0 +1,91 @@ +// REQUIRES: lld + +// 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 +// differing contents. I have discovered that the hash for the type unit is +// simply based off of the typename and doesn't seem to differ when the contents +// differ, so that will help us test foreign type units in the .debug_names dwblaikie wrote: Rather than the personal statement, could maybe just be explicit: "Clang's type unit signature is based only on the mangled name of the type, regardless of the contents of the type" (I can tell you that's how it works - that's how I implemented it - it is a violation of the DWARF spec (the spec says to use a content hash - though that content hash is of the semantic contents, not the syntactic content (eg: if the TU contained a type with a single int member, the spec-compliant hash would be the same whether it was the type DIE followed by the int DIE, or the other way around) - so it would still be the same despite some variations in layout, so the index entries still wouldn't be portable to all matched-hash definitions)) https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
@@ -58,6 +58,8 @@ class DWARFDebugInfo {
const DWARFDebugAranges ();
+ const std::shared_ptr GetDwpSymbolFile();
dwblaikie wrote:
Remove const from by-value return. (it messes with move semantics and some
other things) - or was this meant to return by reference? - yeah, I guess this
latter, the underlying `m_dwarf.GetDwpSymbolFile()` seems to return by const
reference, so this function probably should too?
https://github.com/llvm/llvm-project/pull/87740
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
@@ -1726,44 +1725,59 @@ lldb::ModuleSP
SymbolFileDWARF::GetExternalModule(ConstString name) {
return pos->second;
}
-DWARFDIE
-SymbolFileDWARF::GetDIE(const DIERef _ref) {
- // This method can be called without going through the symbol vendor so we
- // need to lock the module.
- std::lock_guard guard(GetModuleMutex());
-
- SymbolFileDWARF *symbol_file = nullptr;
-
+SymbolFileDWARF *SymbolFileDWARF::GetDIERefSymbolFile(const DIERef _ref) {
// Anytime we get a "lldb::user_id_t" from an lldb_private::SymbolFile API we
// must make sure we use the correct DWARF file when resolving things. On
// MacOSX, when using SymbolFileDWARFDebugMap, we will use multiple
// SymbolFileDWARF classes, one for each .o file. We can often end up with
// references to other DWARF objects and we must be ready to receive a
// "lldb::user_id_t" that specifies a DIE from another SymbolFileDWARF
// instance.
+
std::optional file_index = die_ref.file_index();
+
+ // If the file index matches, then we have the right SymbolFileDWARF already.
+ // This will work for both .dwo file and DWARF in .o files for mac. Also if
+ // both the file indexes are invalid, then we have a match.
+ if (GetFileIndex() == file_index)
+return this;
+
+ // If we are currently in a .dwo file and our file index doesn't match we
need
+ // to let the base symbol file handle this.
+ SymbolFileDWARFDwo *dwo = llvm::dyn_cast_or_null(this);
+ if (dwo)
+return dwo->GetBaseSymbolFile().GetDIERefSymbolFile(die_ref);
+
if (file_index) {
-if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile()) {
- symbol_file = debug_map->GetSymbolFileByOSOIndex(*file_index); // OSO
case
- if (symbol_file)
-return symbol_file->DebugInfo().GetDIE(die_ref);
- return DWARFDIE();
-}
+SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile();
+if (debug_map) {
+// We have a SymbolFileDWARFDebugMap, so let it find the right file
+ return debug_map->GetSymbolFileByOSOIndex(*file_index);
+} else {
+ // Handle the .dwp file case correctly
+ if (*file_index == DIERef::k_file_index_mask)
+return GetDwpSymbolFile().get(); // DWP case
-if (*file_index == DIERef::k_file_index_mask)
- symbol_file = GetDwpSymbolFile().get(); // DWP case
-else
- symbol_file = this->DebugInfo()
-.GetUnitAtIndex(*die_ref.file_index())
+ // Handle the .dwo file case correctly
+ return DebugInfo().GetUnitAtIndex(*die_ref.file_index())
->GetDwoSymbolFile(); // DWO case
- } else if (die_ref.die_offset() == DW_INVALID_OFFSET) {
-return DWARFDIE();
+}
}
+ return this;
+}
- if (symbol_file)
-return symbol_file->GetDIE(die_ref);
+DWARFDIE
+SymbolFileDWARF::GetDIE(const DIERef _ref) {
+ if (die_ref.die_offset() == DW_INVALID_OFFSET)
+return DWARFDIE();
- return DebugInfo().GetDIE(die_ref);
+ // This method can be called without going through the symbol vendor so we
+ // need to lock the module.
+ std::lock_guard guard(GetModuleMutex());
+ SymbolFileDWARF *symbol_file = GetDIERefSymbolFile(die_ref);
+ if (symbol_file)
dwblaikie wrote:
Similar feedback others have given elsewhere, roll the variable declaration
into the `if` condition
https://github.com/llvm/llvm-project/pull/87740
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
@@ -48,15 +60,31 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames
_names) {
return result;
}
+DWARFTypeUnit *
+DebugNamesDWARFIndex::GetForeignTypeUnit(const DebugNames::Entry ) const
{
+ std::optional type_sig = entry.getForeignTUTypeSignature();
+ if (type_sig)
+if (auto dwp_sp = m_debug_info.GetDwpSymbolFile())
dwblaikie wrote:
this `auto` should probably be const ref, to avoid inc/dec on the ref counted
smart pointer's ref count
https://github.com/llvm/llvm-project/pull/87740
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 9df19ce - Add uncovered enums in switches caused by 9434c083475e42f47383f3067fe2a155db5c6a30
Author: David Blaikie
Date: 2024-04-01T23:07:01Z
New Revision: 9df19ce40281551bd348b262a131085cf98dadf5
URL:
https://github.com/llvm/llvm-project/commit/9df19ce40281551bd348b262a131085cf98dadf5
DIFF:
https://github.com/llvm/llvm-project/commit/9df19ce40281551bd348b262a131085cf98dadf5.diff
LOG: Add uncovered enums in switches caused by
9434c083475e42f47383f3067fe2a155db5c6a30
These are probably actually unreachable - perhaps an lldb developer
would be interested in rephrasing this change to move the new cases into
some unreachable/unsupported bucket, rather than my half-hearted guess
at what the desired behavior would be (completely untested, because
they're probably untestable/unreachable - maybe debugging from modules?)
Added:
Modified:
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
Removed:
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index ebcc3bc99a801f..4a1c8d57655215 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4097,6 +4097,8 @@
TypeSystemClang::GetTypeClass(lldb::opaque_compiler_type_t type) {
return lldb::eTypeClassArray;
case clang::Type::DependentSizedArray:
return lldb::eTypeClassArray;
+ case clang::Type::ArrayParameter:
+return lldb::eTypeClassArray;
case clang::Type::DependentSizedExtVector:
return lldb::eTypeClassVector;
case clang::Type::DependentVector:
@@ -4776,6 +4778,7 @@ lldb::Encoding
TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type,
case clang::Type::IncompleteArray:
case clang::Type::VariableArray:
+ case clang::Type::ArrayParameter:
break;
case clang::Type::ConstantArray:
@@ -5109,6 +5112,7 @@ lldb::Format
TypeSystemClang::GetFormat(lldb::opaque_compiler_type_t type) {
case clang::Type::IncompleteArray:
case clang::Type::VariableArray:
+ case clang::Type::ArrayParameter:
break;
case clang::Type::ConstantArray:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lld] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)
dwblaikie wrote: > For those files in the repository that do need CRLF endings, I've adopted a > policy of eol=crlf which means that git will store them in history with LF, > but regardless of user config, they'll be checked out in tree with CRLF. This ^ seems a bit problematic to me, though (where the contents of the repo isn't representative of the bits we want - but perhaps it's schrodinger's line endings & it doesn't matter if it's always checked out as crlf - though if we one day converted to another source control system, would that be a problem? are there some things that examine the underlying/"real" repo contents?) - what would be the cost to using `eol=input` as you've done for the mixed line ending case? below \= > There also appears to be a single test, clang/test/Frontend/rewrite-includes-mixed-eol-crlf.[ch] which needs mixed line endings. This one uses eol=input to preserve it as-is. https://github.com/llvm/llvm-project/pull/86318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Store SupportFile in FileEntry (NFC) (PR #85468)
dwblaikie wrote: (because we don't have a good sense of where feedback should be provided... crosslinking here from some feedback on the commit: https://github.com/llvm/llvm-project/commit/d5a277d309e92b1d3e493da6036cffdf815105b1#commitcomment-139991120 ) https://github.com/llvm/llvm-project/pull/85468 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [lld] [lldb] [llvm] [mlir] Rename llvm::ThreadPool -> llvm::DefaultThreadPool (NFC) (PR #83702)
dwblaikie wrote: Oh, maybe issues related to layered patches - this is intended to be submitted after the introduction of the ThreadPoolInterface? But includes those changes in this review at the moment (I still haven't figured out what we're doing for dependent changes - and I thought the ThreadPoolInterface-introducing patch was already in - so not sure why it appears in this patch too) https://github.com/llvm/llvm-project/pull/83702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [lld] [lldb] [llvm] [mlir] Rename llvm::ThreadPool -> llvm::DefaultThreadPool (NFC) (PR #83702)
dwblaikie wrote: > I did the first part of the renaming @dwblaikie : looks good? Hmm - this patch still seems to have both renamings in it, if I'm reading the PR correctly? I take it from the subject that isn't the intent/the intent is that his patch only does the ThreadPool->DefaultThreadPool change? https://github.com/llvm/llvm-project/pull/83702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lld] [lldb] [llvm] [mlir] Rename ThreadPool->DefaultThreadPool and ThreadPoolInterface->ThreadPool (NFC) (PR #83702)
dwblaikie wrote: > @dwblaikie : how would you split it? I didn't quite get the two renamings you > have in mind? One patch `ThreadPool->DefaultThreadPool` (people get a build error about `ThreadPool` not being the name of anything, find this patch as the root cause, and rename all their ThreadPool->DefaultThreadPool) Follow-up patch, `ThreadPoolInterface->ThreadPool` (similarly clear/separate errors) Changing both in one patch risks folks getting confusing error messages because their existing ThreadPool usage will now instantly start being interpreted as the interface type - resulting in different/confusing errors about inability to instantiate abstract types, etc, presumably. Rather than just that the name is no longer present at all. Ultimately they can root cause and figure out both renamings - probably not a huge deal either way, but my understanding was that separating them might be marginally better for downstrteamers. https://github.com/llvm/llvm-project/pull/83702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lld] [lldb] [llvm] [mlir] Rename ThreadPool->DefaultThreadPool and ThreadPoolInterface->ThreadPool (NFC) (PR #83702)
dwblaikie wrote: I don't have really firm feelings/justification for this, but I'd have guessed that doing this as two separate (separated by a few days, maybe a week) renamings would be better for downstream consumers - they'd get a clear break without any ambiguity/name reuse. No strong feels if other folks reckon doing it in one go is better/OK, though. https://github.com/llvm/llvm-project/pull/83702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [libclc] [libcxx] [lld] [lldb] [llvm] [NFC] Remove trailing whitespace across all non-test related files (PR #82838)
dwblaikie wrote: The dev policy says "Avoid committing formatting- or whitespace-only changes outside of code you plan to make subsequent changes to." - I think it's reasonable to consider changing this, but probably under the "clang-format everything" or a similar discussion (broad discussion before we worry about making pull requests https://github.com/llvm/llvm-project/pull/82838 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add more ways to find the .dwp file. (PR #81067)
dwblaikie wrote: > > > If the client strips the debug info first into "a.out.debug" and then > > > runs llvm-dwp, they will end up with a "a.out.debug.dwp". We have clients > > > that are doing this already and we want to support them. > > > > > > OK, could we fix llvm-dwp to match the behavior, then? If the file has a > > .debug extension, strip that and add the .dwp extension. > > Here people are not using ".debug", but are using ".debuginfo"... They could only use that for symbolizing, yeah? They wouldn't be able to debug their binary, because a debugger wouldn't know that, given the stripped binary, they need to append `.debuginfo` to find the debug info, right? (but I think debuggers do currently have the ability to add ".debug" to find the debug info) - or maybe it doesn't? (I guess reading https://sourceware.org/gdb/current/onlinedocs/gdb.html/Separate-Debug-Files.html - it's never a simple mapping from `binary` to `binary.debug` - either it uses debuglink, in which case the filename is encoded in the debuglink and could be anything, or it's buildID, in which case it is `binary.debug`, but in a buildID-named directory) > Again, nothing is enforced and people are left to use llvm-objcopy + llvm-dwp > how ever they want. Getting a solution that does everything might be nice. > Any thoughts on modifying llvm-dwp to be able to do all of this and provide > some path for people where it can either just create a .dwp file for a given > executable _or_ it can create a `.debug` file, strip the original file, and > create a `.dwp` file? I don't know that llvm-dwp needs to do everything, that's a bit against the grain of *nix tool design. But certainly llvm-objcopy could be more ergonomic (like a one-shot, that both strips the debug info from the binary, and produces the keep-only-debug in `binary.debug` - an even smaller feature would be, like `llvm-dwp` has a default `-o` of `binary.dwp`, `llvm-objcopy` could have a default output file when using `--only-keep-debug` of `binary.debug`, and `llvm-dwp` could grow a special case for `if the file ends in .debug, strip that before adding .dwp` (maybe more generally, strip any `.word` file extension before adding `.dwp` though that might be overly aggressive on *nix systems where users don't expect `.suffix` to be special cased at all) So I guess that'd be the smallest two features I'd suggest starting with 1) llvm-objcopy (& ideally, if someone wanetd to write the patch, binutils objcopy) defaulting to `binary.debug` as the output file when using `--only-keep-debug` 2) llvm-dwp stripping the `.debug` suffix, if it appears, before adding `.dwp` (this one's a bit tricky/I could imagine some disagreement, because someone might`ve just named their real binary "blah.debug" but I'm not sure there's much else to do in that case if we're going to support doing things with debug info with the `--only-keep-debug` file, and with the original unstripped binary, and with the stripped binary+`--only-keep-debug`, etc... have to have some basename to derive everything from) A wrapper script or program (maybe it could be built into llvm-objcopy, but I'm a little hesitant there, but would be open to other folks opinion on it for sure) that ingests the binary and produces the 3 products (stripped binary, binary.debug, binary.dwp) seems easy enough to provide. > > > The compiler and linker drivers are staying out of this and we expect > > > people to do this on their own, so this is what we end up with when there > > > is no enforcement. > > > > > > They aren't doing it on their own though - they're using llvm-dwp and its > > defaults (they're passing it a .debug file and getting a .debug.dwp file - > > it's the defaults you/we are worried about, and how to make other tools > > work well with those defaults). We can change those defaults if they don't > > work well/don't create a consistent environment. > > > I am not sure why this is such a sticking point. Lets make the debugger > > > work for people. > > > > > > As I explained above - my concern is that supporting a wider variety of > > ways these files can be named/arranged means more variants that need to be > > supported across a variety of tooling (symbolizers and debuggers - not just > > LLVM's but binutils, etc too). > > But that's my 2c - if LLDB owners prefer this direction, so be it. Wouldn't > > mind hearing some other people's perspectives on the issues around limiting > > variation here. > > I am happy to hear any other opinions as well. I tend to want to make my life > easier and ease the support burden I run into everyday where people that know > nothing about split DWARF are trying to use it and failing and require tech > support to make it work for them. I am happy to suggest a path to follow, in > fact I am going to write up the best practices on a DWARF group here at work > that I can point poeple to. Yeah, I do want people to not have
[Lldb-commits] [lldb] [lldb] Add more ways to find the .dwp file. (PR #81067)
dwblaikie wrote: > > > I am fine with telling people what to do and giving them a golden path to > > > what is easiest for our debuggers. And I will suggest to everyone that > > > they use `.debug` and `.dwp`, but if we want to only support this, this > > > leaves the downloading of the `.debug` file requiring a rename from > > > `.dwp` to `.debug.dwp` in order for it to work for people. So then > > > everything in this patch should be supported to allow loading the > > > `.debug` file with a `.dwp` like we will encourage people to do. > > > > > > Not sure I follow - one of the scenarios mentioned in this patch is > > "lldb loads which is stripped but has .gnu_debuglink pointing to .debug > > with skeleton DWARF and needs to find .debug.dwp" > > I don't think we should support that, for instance - we should load > > `.dwp` in that case. > > If the client strips the debug info first into "a.out.debug" and then runs > llvm-dwp, they will end up with a "a.out.debug.dwp". We have clients that are > doing this already and we want to support them. OK, could we fix llvm-dwp to match the behavior, then? If the file has a .debug extension, strip that and add the .dwp extension. > The compiler and linker drivers are staying out of this and we expect people > to do this on their own, so this is what we end up with when there is no > enforcement. They aren't doing it on their own though - they're using llvm-dwp and its defaults (they're passing it a .debug file and getting a .debug.dwp file - it's the defaults you/we are worried about, and how to make other tools work well with those defaults). We can change those defaults if they don't work well/don't create a consistent environment. > I am not sure why this is such a sticking point. Lets make the debugger work > for people. As I explained above - my concern is that supporting a wider variety of ways these files can be named/arranged means more variants that need to be supported across a variety of tooling (symbolizers and debuggers - not just LLVM's but binutils, etc too). But that's my 2c - if LLDB owners prefer this direction, so be it. Wouldn't mind hearing some other people's perspectives on the issues around limiting variation here. https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [lldb] [c++20] P1907R1: Support for generalized non-type template arguments of scalar type. (PR #78041)
@@ -5401,6 +5409,8 @@ std::string CGDebugInfo::GetName(const Decl *D, bool
Qualified) const {
// feasible some day.
return TA.getAsIntegral().getBitWidth() <= 64 &&
IsReconstitutableType(TA.getIntegralType());
+ case TemplateArgument::StructuralValue:
+return false;
dwblaikie wrote:
I did finally take a look at this - I think the Clang side of things is fine -
if anything improvements could be made to LLVM to be able to lower constants to
DWARF for a wider variety of values.
eg: For the `float` example, the IR is:
```
!8 = !DITemplateValueParameter(name: "F", type: !9, value: float 1.00e+00)
```
But the DWARF backend can't handle the float constant - the handling is in
`DwarfUnit::constructTemplateValueParameterDIE` (in
llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp) and currently only handles
ConstantInt (for basic stuff - bools, ints, etc) and GlobalValue (for pointer
non-type template parameters).
So for these new floats and such, the DWARF doesn't include the value, only the
type.
GCC uses this encoding, for instance:
```
0x002b: DW_TAG_template_value_parameter [3] (0x001e)
DW_AT_name [DW_FORM_string] ("F")
DW_AT_type [DW_FORM_ref4] (cu + 0x004d => {0x004d}
"float")
DW_AT_const_value [DW_FORM_block1](<0x04> 00 00 80 3f )
```
https://github.com/llvm/llvm-project/pull/78041
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFIndex] Use IDX_parent to implement GetFullyQualifiedType query (PR #79932)
dwblaikie wrote: > > > I will aim to land this sometime tomorrow if there are no further > > > objections > > > > > > Per the documentation ( > > https://llvm.org/docs/CodeReview.html#code-review-workflow ), please don't > > do this. Once it's sent for review, please wait for approval (ping as > > needed, etc) before landing. > > Felipe already had Adrian's approval, and so far as I can see there were no > outstanding comments he hadn't addressed. Ah, sorry - I scanned up and down but missed it (the UI's not quite as explicit as Phab is on whether a patch has been reviewed - but I could've looked in the top right and seen the check next to Adrian's name). Yeah, I don't expect multiple reviewer approval unless folks (either the submitter, or other reviewers) ask for it. If the it's approved with some optional comments and the discussion around those aspects has been resolved, fine by me to commit. https://github.com/llvm/llvm-project/pull/79932 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add more ways to find the .dwp file. (PR #81067)
dwblaikie wrote: > I am fine with telling people what to do and giving them a golden path to > what is easiest for our debuggers. And I will suggest to everyone that they > use `.debug` and `.dwp`, but if we want to only support this, this leaves the > downloading of the `.debug` file requiring a rename from `.dwp` to > `.debug.dwp` in order for it to work for people. So then everything in this > patch should be supported to allow loading the `.debug` file with a `.dwp` > like we will encourage people to do. Not sure I follow - one of the scenarios mentioned in this patch is "lldb loads which is stripped but has .gnu_debuglink pointing to .debug with skeleton DWARF and needs to find .debug.dwp" I don't think we should support that, for instance - we should load `.dwp` in that case. > It would also be nice if we do have a single `.debug` file that has debug > info only, it would be nice to allow it and the `.dwp` file to be combined > into a single file. There is no reason for them to be separate anymore once > we have `a.out` stripped, it would be nice to only require `a.out.debug` > which contains the `.dwp` sections inside of it already instead of requiring > people to have two files needed for debug info. Maybe? I figure once you've got to download one file, two isn't a substantial imposition... - it'd be a bit weird having a DWP file and a .debug file mashed up together, but can't see any reason it wouldn't work - with the logic of "check if this program has a cu_index in it, if so, treat it as a dwp, otherwise look for .dwp, otherwise look for the dwos". https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFIndex] Use IDX_parent to implement GetFullyQualifiedType query (PR #79932)
dwblaikie wrote: > I will aim to land this sometime tomorrow if there are no further objections Per the documentation ( https://llvm.org/docs/CodeReview.html#code-review-workflow ), please don't do this. Once it's sent for review, please wait for approval (ping as needed, etc) before landing. https://github.com/llvm/llvm-project/pull/79932 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add more ways to find the .dwp file. (PR #81067)
dwblaikie wrote: > > FWIW, I think we should be opinionated (& consistent with whatever gdb > > does, if it has some precedent here - if it is less opinionated, then maybe > > we have to be accepting) when it comes to whether x.debug goes with x.dwp > > or x.debug.dwp - we shouldn't support both unless there's some prior > > precedent that's unavoidable to support. It'd be better for everyone if > > there was one option we encourage people to follow. (so I think we > > shouldn't support (2) and (3) if we can help it - we should pick one) > > There currently is no enforcement on any of this from the compiler or linker > driver side and everyone must do the stripping of the executable and creating > the .dwp file manually. Since there is no actual enforcement, it seems like > we should support both IMHO. More on this below... > > > I'm not sure I understand the "lldb loads .debug and needs to find .dwp" > > case. the .debug file usually only has the debug info, not the executable > > code, so you can't load it directly, right? (see the documentation in > > https://sourceware.org/gdb/current/onlinedocs/gdb.html/Separate-Debug-Files.html > > - `objcopy --only-keep-debug foo foo.debug`). > > If we have a build system that creates: > > * `a.out` which is stripped and shipped for distribution > * `a.out.debug` which contains debug info only with the skeleton units, > address table, line tables, accelerator tables > * `a.out.dwp` or `a.out.debug.dwp` which contains the .dwo files for > `a.out.debug` > > We might request to download debug info for this build using a GNU build ID > or some other build number for say a symbolication server which wants to use > LLDB. If we get this debug info, we might only download `a.out.debug` and the > associated `.dwp` file (either `a.out.dwp` or `a.out.debug.dwp`). It would be > a shame to require that we download the `a.out` binary as well just so that > we can load this into a debugger and get the .dwp file to load automatically. Ah, OK, yeah I can see how for symbolizing that could be done with just the .debug and .dwp files, without the original executable. Thanks for explaining. (I wouldn't've expected someone to use a /debugger/ in this case, rather than llvm-symbolizer, addr2line, etc - but don't object to a debugger supporting that functionality) > If we don't fix this, then people will get confused and not know how to fix > things and get full debug info to get loaded. No one besides debugger, > compiler, and linker engineers and very few other people actually know what > split DWARF does and or means and if they can't get things to work, they give > up or waste time figuring out the magic things that need to happen to make us > able to load the right debug info. So I prefer to allow things to work more > seamlessly without trying to enforce something that is left up to users to do > in the first place. I am someone that often responds to peoples posts when I > am on call and I need to help people figure out how to load their debug > symbols correrctl. My concern with supporting a wider variety of naming conventions is the risk of fragmentation and more user confusion - if there's multiple naming conventions, there's a risk that some tools end up supporting some of them but not others, and users are confused why their files work with some tools and not others. I think we'd be doing users a favor, in terms of focussing the ecosystem on fewer permutations, by being opinionated on how these things should be named. We are going to be somewhat opinionated regardless (we are using /some/ naming convention rules to derive .dwp/.debug names from other names - so we're only haggling over how many different naming conventions we should support, not whether or not we should limit users - they'll always be limited to some set of rules we pick). > > With split DWARF it is even worse because if they get the debug info file > with the skeleton compile units but the .dwp file isn't named consistent with > GDB conventions, then they can set file and line breakpoints and hit them, > but as soon as they stop there, they get no variable information because the > .dwo file doesn't get loaded. They don't know what a .dwo or a .dwp file is > or why it is needed. We get a few posts a week with people trying to use > split DWARF symbols and not being able to get things to work. Yeah, it's certainly a cost that comes with the benefits of Split DWARF - they should get suitable warnings/error messages, and perhaps those should link to an article describing what they're talking about if necessary. > So IMHO there is no harm in making all scenarios work without having to > require some naming convention that isn't actually enforced by _any_ compiler > or linker driver, and when things don't work, we put the pressure on the user > to find some wiki somewhere that says "here is how you were supposed to do > it" and figure out how to
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
dwblaikie wrote: > If the DWO ID is just a hash of the file path or something that isn't > guaranteed to be unique with each new build, then we need the UUID in the > .dwp file. Nah, the DWO ID, as per spec, is a semantic hash of the DWARF contents. It should change, generally, if any part of the DWARF changes. https://github.com/llvm/llvm-project/blob/1d4fc381d3da4317cc2cfa59b2d59d53decddf71/llvm/lib/CodeGen/AsmPrinter/DIEHash.cpp#L399 https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
dwblaikie wrote: FWIW, I think we should be opinionated (& consistent with whatever gdb does, if it has some precedent here - if it is less opinionated, then maybe we have to be accepting) when it comes to whether x.debug goes with x.dwp or x.debug.dwp - we shouldn't support both unless there's some prior precedent that's unavoidable to support. It'd be better for everyone if there was one option we encourage people to follow. (so I think we shouldn't support (2) and (3) if we can help it - we should pick one) I'm not sure I understand the "lldb loads .debug and needs to find .dwp" case. the .debug file usually only has the debug info, not the executable code, so you can't load it directly, right? (see the documentation in https://sourceware.org/gdb/current/onlinedocs/gdb.html/Separate-Debug-Files.html - `objcopy --only-keep-debug foo foo.debug`). https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix expressions that involve nested structs/classes/unions. (PR #77029)
dwblaikie wrote: Be great to see D101950 picked up again, so I'm glad to hear that's being looked at. Be nice to get some way to address this regression sooner rather than later... & yeah, I'd argue that classes derived from DWARF are not complete - nested types, member function templates, special member functions, and... static members I think - none of those things are consistently emitted in every copy of a type. So we probably shouldn't turn off the queries back into lldb for a type ever, really. https://github.com/llvm/llvm-project/pull/77029 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] f6b3875 - Reapply "lldb: Cache string hash during ConstString pool queries/insertions"
Author: David Blaikie
Date: 2024-02-02T20:01:51Z
New Revision: f6b387589d648945372528a4ac77c58f310e5165
URL:
https://github.com/llvm/llvm-project/commit/f6b387589d648945372528a4ac77c58f310e5165
DIFF:
https://github.com/llvm/llvm-project/commit/f6b387589d648945372528a4ac77c58f310e5165.diff
LOG: Reapply "lldb: Cache string hash during ConstString pool
queries/insertions"
Reverted due to an internally discovered lld crash due to the underlying
StringMap changes, which turned out to be an existing lld bug that got
tickled by the StringMap changes. That's addressed in
dee8786f70a3d62b639113343fa36ef55bdbad63 so let's have another go with
this change.
Original commit message:
lldb was rehashing the string 3 times (once to determine which StringMap
to use, once to query the StringMap, once to insert) on insertion (twice
on successful lookup).
This patch allows the lldb to benefit from hash improvements in LLVM
(from djbHash to xxh3).
Though further changes would be needed to cache this value to disk - we
shouldn't rely on the StringMap::hash remaining the same in the
future/this value should not be serialized to disk. If we want cache
this value StringMap should take a hashing template parameter to allow
for a fixed hash to be requested.
This reverts commit 5bc1adff69315dcef670e9fcbe04067b5d5963fb.
Effectively reapplying the original 2e197602305be18b963928e6ae024a004a95af6d.
Added:
Modified:
lldb/source/Utility/ConstString.cpp
llvm/lib/Support/StringMap.cpp
Removed:
diff --git a/lldb/source/Utility/ConstString.cpp
b/lldb/source/Utility/ConstString.cpp
index 4535771adfb73..ea897dc611cc9 100644
--- a/lldb/source/Utility/ConstString.cpp
+++ b/lldb/source/Utility/ConstString.cpp
@@ -77,10 +77,10 @@ class Pool {
return 0;
}
- StringPoolValueType GetMangledCounterpart(const char *ccstr) const {
+ StringPoolValueType GetMangledCounterpart(const char *ccstr) {
if (ccstr != nullptr) {
- const uint8_t h = hash(llvm::StringRef(ccstr));
- llvm::sys::SmartScopedReader rlock(m_string_pools[h].m_mutex);
+ const PoolEntry = selectPool(llvm::StringRef(ccstr));
+ llvm::sys::SmartScopedReader rlock(pool.m_mutex);
return GetStringMapEntryFromKeyData(ccstr).getValue();
}
return nullptr;
@@ -100,19 +100,20 @@ class Pool {
const char *GetConstCStringWithStringRef(llvm::StringRef string_ref) {
if (string_ref.data()) {
- const uint8_t h = hash(string_ref);
+ const uint32_t string_hash = StringPool::hash(string_ref);
+ PoolEntry = selectPool(string_hash);
{
-llvm::sys::SmartScopedReader rlock(m_string_pools[h].m_mutex);
-auto it = m_string_pools[h].m_string_map.find(string_ref);
-if (it != m_string_pools[h].m_string_map.end())
+llvm::sys::SmartScopedReader rlock(pool.m_mutex);
+auto it = pool.m_string_map.find(string_ref, string_hash);
+if (it != pool.m_string_map.end())
return it->getKeyData();
}
- llvm::sys::SmartScopedWriter wlock(m_string_pools[h].m_mutex);
+ llvm::sys::SmartScopedWriter wlock(pool.m_mutex);
StringPoolEntryType =
- *m_string_pools[h]
- .m_string_map.insert(std::make_pair(string_ref, nullptr))
+ *pool.m_string_map
+ .insert(std::make_pair(string_ref, nullptr), string_hash)
.first;
return entry.getKeyData();
}
@@ -125,12 +126,14 @@ class Pool {
const char *demangled_ccstr = nullptr;
{
- const uint8_t h = hash(demangled);
- llvm::sys::SmartScopedWriter wlock(m_string_pools[h].m_mutex);
+ const uint32_t demangled_hash = StringPool::hash(demangled);
+ PoolEntry = selectPool(demangled_hash);
+ llvm::sys::SmartScopedWriter wlock(pool.m_mutex);
// Make or update string pool entry with the mangled counterpart
- StringPool = m_string_pools[h].m_string_map;
- StringPoolEntryType = *map.try_emplace(demangled).first;
+ StringPool = pool.m_string_map;
+ StringPoolEntryType =
+ *map.try_emplace_with_hash(demangled, demangled_hash).first;
entry.second = mangled_ccstr;
@@ -141,8 +144,8 @@ class Pool {
{
// Now assign the demangled const string as the counterpart of the
// mangled const string...
- const uint8_t h = hash(llvm::StringRef(mangled_ccstr));
- llvm::sys::SmartScopedWriter wlock(m_string_pools[h].m_mutex);
+ PoolEntry = selectPool(llvm::StringRef(mangled_ccstr));
+ llvm::sys::SmartScopedWriter wlock(pool.m_mutex);
GetStringMapEntryFromKeyData(mangled_ccstr).setValue(demangled_ccstr);
}
@@ -171,17 +174,20 @@ class Pool {
}
protected:
- uint8_t hash(llvm::StringRef s) const {
-uint32_t h = llvm::djbHash(s);
-return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff;
- }
[Lldb-commits] [lldb] [lldb] Fix a crash when using .dwp files and make type lookup reliable with the index cache (PR #79544)
dwblaikie wrote: > > > Even if we want to have the skeleton compile unit be parsed first, we > > > would need to know this from any accelerator table entry, wether it be > > > from .debug_names or from our internal manual index. > > > > > > True enough, but I think letting this become a lazy property > > post-construction is a bit more problematic as it might lead to more > > complicated invariants/situations further downstream. By making it a > > precondition that any split unit, on construction, has the association with > > the skeleton unit - there would be fewer "interesting" situations later > > where the lookup occurs when someone isn't expecting it. The split unit > > isn't useful without the skeleton for addresses, etc, so it's not like > > delaying the skeleton lookup provides better performance, etc. > > It actually does provide better performance as we will often do the type > lookup solely in the .dwp file and determine we don't need to actually parse > and return the type because the context doesn't match. There is no need for > the skeleton unit and the address tables and lines tables in the type lookup > case. It allows us to traverse the DIE tree in the .dwp file without ever > needing to possibly do an expensive lookup by DWO ID for DWARF4, when it > actually isn't needed. We have an accessor to allow us to get the skeleton > unit if and only if we need it, and yes, most times it will be filled in > already. But when it isn't we can easily access it. Huh, fair enough - bit surprising, but it works. Still worried about doing things a bit too lazily, more complicated states to deal with through the debugger, but fair enough. Thanks for walking me through it! https://github.com/llvm/llvm-project/pull/79544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix a crash when using .dwp files and make type lookup reliable with the index cache (PR #79544)
dwblaikie wrote: > Even if we want to have the skeleton compile unit be parsed first, we would > need to know this from any accelerator table entry, wether it be from > .debug_names or from our internal manual index. True enough, but I think letting this become a lazy property post-construction is a bit more problematic as it might lead to more complicated invariants/situations further downstream. By making it a precondition that any split unit, on construction, has the association with the skeleton unit - there would be fewer "interesting" situations later where the lookup occurs when someone isn't expecting it. The split unit isn't useful without the skeleton for addresses, etc, so it's not like delaying the skeleton lookup provides better performance, etc. > Our internal manual index creates a DIERef object with a magic .dwp file > index + a DIE offset within the .debug_info.dwo in the dwp file. (side note: it'd be great if the internal manual index was closer to .debug_names (especially the serialized version - it'd be good if that /was/ .debug_names) - to ensure consistency (& at least last I checked the cached manual index - the debugger startup time was impacted by loading that index off-disk, when the .debug_names/.apple_names indexes don't need to be loaded from disk) & fewer support paths/less code to maintain) Presumably the entry would still need to mention which CU in the .dwp file it comes from - and presumably the non-split entries in this index would also mention which CU they come from (by offset in the .debug_info section)? (because reading a DIE without knowing wihch CU it's in isn't helpful - you need CU properties, etc) (but I'm not an lldb maintainer, so can take all this with a few grains of salt) https://github.com/llvm/llvm-project/pull/79544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix expressions that involve nested structs/classes/unions. (PR #77029)
dwblaikie wrote: How'd this work before your recent changes, then - when each repeated query would get one level further down in the nesting? How'd that work given the clang limitations you're describing? In any case, the extra clang requirements here seem like they might be of uncertain cost/maintainability (if it's only updating one place that everyone calls through - great, but if it's updating multiple callers, and the risk that new callers miss this handling - that seems like a maintainability problem) which is worrying. So, I'd be uncertain about that direction without more info - and with that info, if it is just one codepath, yeah, maybe it's quick enough to address the regression. But if it'd require substantial reengineering to clang to get this working - this seems an unfortunate regression to carry in the interim, and it might be worth reverting to the previous (differently buggy, but at least work-around-able) behavior. And if this would require updating many callers in clang (unless such updates include an API change that would make it difficult to accidentally introduce a new caller that didn't handle things correctly for lldb), I'd worry about the future stability of such a change & might be inclined towards the less efficient "search all the DWARF" thing. https://github.com/llvm/llvm-project/pull/77029 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFIndex] Use IDX_parent to implement GetFullyQualifiedType query (PR #79932)
@@ -218,6 +219,104 @@ void DebugNamesDWARFIndex::GetCompleteObjCClass(
m_fallback.GetCompleteObjCClass(class_name, must_be_implementation,
callback);
}
+namespace {
+using Entry = llvm::DWARFDebugNames::Entry;
+
+/// If `entry` and all of its parents have an `IDX_parent`, use that
information
+/// to build and return a list of at most `max_parents` parent Entries.
+/// `entry` itself is not included in the list.
+/// If any parent does not have an `IDX_parent`, or the Entry data is
corrupted,
+/// nullopt is returned.
+static std::optional>
+getParentChain(Entry entry, uint32_t max_parents) {
+ llvm::SmallVector parent_entries;
+
+ do {
+if (!entry.hasParentInformation())
+ return std::nullopt;
+
+llvm::Expected> parent = entry.getParentDIEEntry();
+if (!parent) { // Bad data.
+ consumeError(parent.takeError());
+ return std::nullopt;
+}
+
+// Last parent in the chain
+if (!parent->has_value())
+ break;
+
+parent_entries.push_back(**parent);
+entry = **parent;
+ } while (parent_entries.size() < max_parents);
+
+ return parent_entries;
+}
+} // namespace
+
+void DebugNamesDWARFIndex::GetFullyQualifiedType(
+const DWARFDeclContext ,
+llvm::function_ref callback) {
+ if (context.GetSize() == 0)
+return;
+
+ // Fallback: use the base class implementation.
+ auto fallback_impl = [&](const DebugNames::Entry ) {
+return ProcessEntry(entry, [&](DWARFDIE die) {
+ return GetFullyQualifiedTypeImpl(context, die, callback);
+});
+ };
+
+ llvm::StringRef leaf_name = context[0].name;
+ llvm::SmallVector parent_names;
+ for (auto idx : llvm::seq(1, context.GetSize()))
+parent_names.emplace_back(context[idx].name);
+
+ for (const DebugNames::Entry :
+ m_debug_names_up->equal_range(leaf_name)) {
+if (!isType(entry.tag()))
+ continue;
+
+// Grab at most one extra parent, subsequent parents are not necessary to
+// test equality.
+auto parent_chain = getParentChain(entry, parent_names.size() + 1);
+
+if (!parent_chain) {
+ if (!fallback_impl(entry))
+return;
+ continue;
+}
+
+if (SameParentChain(parent_names, *parent_chain) &&
+!ProcessEntry(entry, callback))
+ return;
dwblaikie wrote:
FWIW as-written does lean a bit towards the LLVM documented style (
https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code
) - but I wouldn't be too fussed either way.
https://github.com/llvm/llvm-project/pull/79932
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFIndex] Use IDX_parent to implement GetFullyQualifiedType query (PR #79932)
@@ -0,0 +1,210 @@
+//===-- DWARFDIETest.cpp
--=---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/SymbolFile/DWARF/DWARFDIE.h"
+#include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h"
+#include "Plugins/SymbolFile/DWARF/DWARFDeclContext.h"
+#include "Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h"
+#include "TestingSupport/Symbol/YAMLModuleTester.h"
+#include "llvm/ADT/STLExtras.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::plugin::dwarf;
+using StringRef = llvm::StringRef;
+
+void check_num_matches(DebugNamesDWARFIndex , int expected_num_matches,
+ llvm::ArrayRef ctx_entries) {
+ DWARFDeclContext ctx(ctx_entries);
+ int num_matches = 0;
+ auto increment_matches = [&](DWARFDIE die) {
+num_matches++;
+return true;
+ };
+
+ index.GetFullyQualifiedType(ctx, increment_matches);
dwblaikie wrote:
Probably inline the lambda at the call site?
```
index.GetFullyQualifiedType(ctx, [*](DWARFDIE die) {
num_matches++;
return true;
});
```
https://github.com/llvm/llvm-project/pull/79932
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFIndex] Use IDX_parent to implement GetFullyQualifiedType query (PR #79932)
@@ -218,6 +219,104 @@ void DebugNamesDWARFIndex::GetCompleteObjCClass(
m_fallback.GetCompleteObjCClass(class_name, must_be_implementation,
callback);
}
+namespace {
+using Entry = llvm::DWARFDebugNames::Entry;
+
+/// If `entry` and all of its parents have an `IDX_parent`, use that
information
+/// to build and return a list of at most `max_parents` parent Entries.
+/// `entry` itself is not included in the list.
+/// If any parent does not have an `IDX_parent`, or the Entry data is
corrupted,
+/// nullopt is returned.
+static std::optional>
+getParentChain(Entry entry, uint32_t max_parents) {
+ llvm::SmallVector parent_entries;
+
+ do {
+if (!entry.hasParentInformation())
+ return std::nullopt;
+
+llvm::Expected> parent = entry.getParentDIEEntry();
+if (!parent) { // Bad data.
+ consumeError(parent.takeError());
+ return std::nullopt;
+}
+
+// Last parent in the chain
+if (!parent->has_value())
+ break;
+
+parent_entries.push_back(**parent);
+entry = **parent;
+ } while (parent_entries.size() < max_parents);
+
+ return parent_entries;
+}
+} // namespace
+
+void DebugNamesDWARFIndex::GetFullyQualifiedType(
+const DWARFDeclContext ,
+llvm::function_ref callback) {
+ if (context.GetSize() == 0)
+return;
+
+ // Fallback: use the base class implementation.
+ auto fallback_impl = [&](const DebugNames::Entry ) {
+return ProcessEntry(entry, [&](DWARFDIE die) {
+ return GetFullyQualifiedTypeImpl(context, die, callback);
+});
+ };
+
+ llvm::StringRef leaf_name = context[0].name;
+ llvm::SmallVector parent_names;
+ for (auto idx : llvm::seq(1, context.GetSize()))
+parent_names.emplace_back(context[idx].name);
+
+ for (const DebugNames::Entry :
+ m_debug_names_up->equal_range(leaf_name)) {
+if (!isType(entry.tag()))
+ continue;
+
+// Grab at most one extra parent, subsequent parents are not necessary to
+// test equality.
+auto parent_chain = getParentChain(entry, parent_names.size() + 1);
+
+if (!parent_chain) {
+ if (!fallback_impl(entry))
+return;
+ continue;
+}
+
+if (SameParentChain(parent_names, *parent_chain) &&
+!ProcessEntry(entry, callback))
+ return;
+ }
+}
+
+bool DebugNamesDWARFIndex::SameParentChain(
+llvm::ArrayRef parent_names,
+llvm::ArrayRef parent_entries) const {
+
+ if (parent_entries.size() != parent_names.size())
+return false;
+
+ auto SameAsEntryATName = [this](llvm::StringRef name,
+ const DebugNames::Entry ) {
+auto maybe_dieoffset = entry.getDIEUnitOffset();
+if (!maybe_dieoffset)
+ return false;
+auto die_ref = ToDIERef(entry);
+if (!die_ref)
+ return false;
+return name == m_debug_info.PeekDIEName(*die_ref);
+ };
+
dwblaikie wrote:
Similarly, a lambda that's only called once doesn't seem to carry its weight -
might be simpler to put it in the loop.
https://github.com/llvm/llvm-project/pull/79932
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFIndex] Use IDX_parent to implement GetFullyQualifiedType query (PR #79932)
@@ -218,6 +219,104 @@ void DebugNamesDWARFIndex::GetCompleteObjCClass(
m_fallback.GetCompleteObjCClass(class_name, must_be_implementation,
callback);
}
+namespace {
+using Entry = llvm::DWARFDebugNames::Entry;
+
+/// If `entry` and all of its parents have an `IDX_parent`, use that
information
+/// to build and return a list of at most `max_parents` parent Entries.
+/// `entry` itself is not included in the list.
+/// If any parent does not have an `IDX_parent`, or the Entry data is
corrupted,
+/// nullopt is returned.
+static std::optional>
+getParentChain(Entry entry, uint32_t max_parents) {
+ llvm::SmallVector parent_entries;
+
+ do {
+if (!entry.hasParentInformation())
+ return std::nullopt;
+
+llvm::Expected> parent = entry.getParentDIEEntry();
+if (!parent) { // Bad data.
+ consumeError(parent.takeError());
+ return std::nullopt;
+}
+
+// Last parent in the chain
+if (!parent->has_value())
+ break;
+
+parent_entries.push_back(**parent);
+entry = **parent;
+ } while (parent_entries.size() < max_parents);
+
+ return parent_entries;
+}
+} // namespace
+
+void DebugNamesDWARFIndex::GetFullyQualifiedType(
+const DWARFDeclContext ,
+llvm::function_ref callback) {
+ if (context.GetSize() == 0)
+return;
+
+ // Fallback: use the base class implementation.
+ auto fallback_impl = [&](const DebugNames::Entry ) {
+return ProcessEntry(entry, [&](DWARFDIE die) {
+ return GetFullyQualifiedTypeImpl(context, die, callback);
+});
+ };
dwblaikie wrote:
Not sure this is worth putting in a lambda? Might be simpler to write it inline
in the one place it's used?
https://github.com/llvm/llvm-project/pull/79932
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFIndex] Use IDX_parent to implement GetFullyQualifiedType query (PR #79932)
@@ -0,0 +1,210 @@
+//===-- DWARFDIETest.cpp
--=---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/SymbolFile/DWARF/DWARFDIE.h"
+#include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h"
+#include "Plugins/SymbolFile/DWARF/DWARFDeclContext.h"
+#include "Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h"
+#include "TestingSupport/Symbol/YAMLModuleTester.h"
+#include "llvm/ADT/STLExtras.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::plugin::dwarf;
+using StringRef = llvm::StringRef;
+
+void check_num_matches(DebugNamesDWARFIndex , int expected_num_matches,
+ llvm::ArrayRef ctx_entries) {
+ DWARFDeclContext ctx(ctx_entries);
+ int num_matches = 0;
+ auto increment_matches = [&](DWARFDIE die) {
+num_matches++;
+return true;
+ };
+
+ index.GetFullyQualifiedType(ctx, increment_matches);
+ ASSERT_EQ(num_matches, expected_num_matches);
+}
+
+TEST(DWARFDebugNamesIndexTest, FullyQualifiedQueryWithIDXParent) {
+ const char *yamldata = R"(
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data:ELFDATA2LSB
+ Type:ET_EXEC
+ Machine: EM_386
+DWARF:
+ debug_str:
+- '1'
+- '2'
+- '3'
+ debug_abbrev:
+- Table:
+# We intentionally don't nest types in debug_info: if the nesting is
not
+# inferred from debug_names, we want the test to fail.
+- Code:0x1
+ Tag: DW_TAG_compile_unit
+ Children:DW_CHILDREN_yes
+- Code:0x2
+ Tag: DW_TAG_class_type
+ Children:DW_CHILDREN_no
+ Attributes:
+- Attribute: DW_AT_name
+ Form:DW_FORM_strp
+ debug_info:
+- Version: 4
+ AddrSize:8
+ Entries:
+- AbbrCode:0x1
+- AbbrCode:0x2
+ Values:
+- Value: 0x0 # Name "1"
+- AbbrCode:0x2
+ Values:
+- Value: 0x2 # Name "2"
+- AbbrCode:0x2
+ Values:
+- Value: 0x4 # Name "3"
+- AbbrCode:0x0
+ debug_names:
+Abbreviations:
+- Code: 0x11
+ Tag: DW_TAG_class_type
+ Indices:
+- Idx: DW_IDX_parent
+ Form: DW_FORM_flag_present
+- Idx: DW_IDX_die_offset
+ Form: DW_FORM_ref4
+- Code: 0x22
+ Tag: DW_TAG_class_type
+ Indices:
+- Idx: DW_IDX_parent
+ Form: DW_FORM_ref4
+- Idx: DW_IDX_die_offset
+ Form: DW_FORM_ref4
+Entries:
+- Name: 0x0 # strp to Name1
+ Code: 0x11
+ Values:
+- 0xc # Die offset to entry named "1"
+- Name: 0x2 # strp to Name2
+ Code: 0x22
+ Values:
+- 0x0 # Parent = First entry ("1")
+- 0x11 # Die offset to entry named "1:2"
+- Name: 0x4 # strp to Name3
+ Code: 0x22
+ Values:
+- 0x6 # Parent = Second entry ("1::2")
+- 0x16 # Die offset to entry named "1::2::3"
+- Name: 0x4 # strp to Name3
+ Code: 0x11
+ Values:
+- 0x16 # Die offset to entry named "3"
+)";
+
+ YAMLModuleTester t(yamldata);
+ auto *symbol_file =
+ llvm::cast(t.GetModule()->GetSymbolFile());
+ auto *index = static_cast(symbol_file->getIndex());
+ ASSERT_NE(index, nullptr);
+
+ auto make_entry = [](const char *c) {
+return DWARFDeclContext::Entry(dwarf::DW_TAG_class_type, c);
+ };
+ check_num_matches(*index, 1, {make_entry("1")});
+ check_num_matches(*index, 1, {make_entry("2"), make_entry("1")});
+ check_num_matches(*index, 1,
+{make_entry("3"), make_entry("2"), make_entry("1")});
+ check_num_matches(*index, 0, {make_entry("2")});
+ check_num_matches(*index, 1, {make_entry("3")});
+}
+
+TEST(DWARFDebugNamesIndexTest, FullyQualifiedQueryWithoutIDXParent) {
+ const char *yamldata = R"(
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data:ELFDATA2LSB
+ Type:ET_EXEC
+ Machine: EM_386
+DWARF:
+ debug_str:
+- '1'
+- '2'
+ debug_abbrev:
+- Table:
+- Code:0x1
+ Tag: DW_TAG_compile_unit
+ Children:DW_CHILDREN_yes
+- Code:0x2
+ Tag: DW_TAG_class_type
+ Children:DW_CHILDREN_yes
+ Attributes:
+- Attribute: DW_AT_name
+ Form:DW_FORM_strp
+- Code:0x3
+ Tag: DW_TAG_class_type
+ Children:DW_CHILDREN_no
+
[Lldb-commits] [lldb] [lldb] Fix expressions that involve nested structs/classes/unions. (PR #77029)
dwblaikie wrote: > > Thanks for the fix! I did test this patch with both our regression issue, > > and also allowing us to remove the double-lookup working around #53904 and > > can confirm it addressed both issues. Thanks again! > > Glad it worked! It will also be a huge improvement in the expression parser > and avoid these weird expression parser bugs where it doesn't work sometimes > and does other times, so it was totally worth fixing. I am also glad to know > that the type lookups are being so much more efficient. Having a repro case > is always the best way to help us fix bugs, so thanks for posting the details > so we can repro easily. Turns out it actually got worse, see https://github.com/llvm/llvm-project/issues/79668 - now whether or not a given nested type is accessible via name lookup depends on which copy of the outer type is loaded first & there's no workaround that I know of - the lookup then fails/succeeds permanently. Be good to get this addressed - as now it's not possible to lookup these nested members at all reliably (even the "repeat the lookup" workaround doesn't work - maybe there are other workarounds? But none that I can think of) https://github.com/llvm/llvm-project/pull/77029 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix a crash when using .dwp files and make type lookup reliable with the index cache (PR #79544)
dwblaikie wrote: I'm not following all of this, but it appears to be based on the premise that it's OK that sometimes split units inside a DWP file are parsed before their skeleton unit? Why is that OK/when/why/where is that happening? I'd think any case where that happens would be a performance (& possibly correctness bug) & it'd be better to maintain the invariant that the only way you ever parse a split unit is starting from the skeleton - rather than adding maps/etc to be able to backtrack to parsing the skeleton when you already have the split unit. https://github.com/llvm/llvm-project/pull/79544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [lldb] [c++20] P1907R1: Support for generalized non-type template arguments of scalar type. (PR #78041)
@@ -5401,6 +5409,8 @@ std::string CGDebugInfo::GetName(const Decl *D, bool
Qualified) const {
// feasible some day.
return TA.getAsIntegral().getBitWidth() <= 64 &&
IsReconstitutableType(TA.getIntegralType());
+ case TemplateArgument::StructuralValue:
+return false;
dwblaikie wrote:
Sorry for the delays in the debug info review for this - it is on my list :/
https://github.com/llvm/llvm-project/pull/78041
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] GetClangDeclForDIE: don't create VarDecl for static data members (PR #77155)
@@ -0,0 +1,23 @@ +# UNSUPPORTED: system-darwin, system-windows dwblaikie wrote: That's actually a great question, but I think for now the answer with lldb is "no, expression evaluation doesn't work without a running process" - but I was just commenting on an internal bug where folks were having trouble investigating core dumps with some of our pretty printers that evaluate expressions that happen to be constants that gdb can evaluate on a core/without a running process, but it seems lldb can't do that... So while that sounds like a good thing, my understanding is that lldb can't do that at the moment. https://github.com/llvm/llvm-project/pull/77155 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] GetClangDeclForDIE: don't create VarDecl for static data members (PR #77155)
@@ -0,0 +1,23 @@
+# UNSUPPORTED: system-darwin, system-windows
+
+# In DWARFv5, C++ static data members are represented
+# as DW_TAG_variable. We make sure LLDB's expression
+# evaluator doesn't crash when trying to parse such
+# a DW_TAG_variable DIE, whose parent DIE is only
+# a forward declaration.
+
+# RUN: %clangxx_host %S/Inputs/dwo-static-data-member.cpp \
+# RUN: -g -gdwarf-5 -gsplit-dwarf -o %t
+# RUN: %lldb %t -s %s -o exit 2>&1 | FileCheck %s
+
+breakpoint set -n main
+process launch
+
+# CHECK: Process {{.*}} stopped
+
+# There is no definition for NoCtor anywhere
+# in the debug-info, so LLDB can't evaluate
+# this expression.
+expression NoCtor::i
+# CHECK-LABEL: expression NoCtor::i
+# CHECK: use of undeclared identifier 'NoCtor'
dwblaikie wrote:
(totally side note: Pity about how this fails
(a) gdb can evaluate this, so it's a bug/limitation in lldb (though it sounds
like lldb-eval maybe can handle this, hopefully, and possibly without the need
for a running process either)
(b) `NoCtor` isn't undeclared - it's declared but not defined, but even saying
that wouldn't be quite accurate/wouldn't explain why it can't be evaluated by
the debugger, since the info for the `i` member is present
But that's just me rambling/not relevant to this bug/regression/immediate issue)
https://github.com/llvm/llvm-project/pull/77155
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix expressions that involve nested structs/classes/unions. (PR #77029)
dwblaikie wrote: Thanks for the fix! I did test this patch with both our regression issue, and also allowing us to remove the double-lookup working around #53904 and can confirm it addressed both issues. Thanks again! https://github.com/llvm/llvm-project/pull/77029 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Make only one function that needs to be implemented when searching for types (PR #74786)
dwblaikie wrote: @clayborg any thoughts on this ^ ? https://github.com/llvm/llvm-project/pull/74786 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Make only one function that needs to be implemented when searching for types (PR #74786)
dwblaikie wrote: FWIW, we're seeing this breaking a pretty printer in google - previously a lookup was able to find `absl::cord_internal::RefcountAndFlags::Flags::kNumFlags` ( https://github.com/abseil/abseil-cpp/blob/8184f16e898fcb13b310b40430e13ba40679cf0e/absl/strings/internal/cord_internal.h#L202 ) (the name components are, in order: namespace, namespace, class, unscoped enumeration, enumerator) The code we have (working via [gala](https://github.com/sivachandra/gala)) has been doing this query twice to workaround this bug: https://github.com/llvm/llvm-project/issues/53904 - but it looks like since this change, that workaround has even broken and the name is no longer found? I don't have a fully isolated repro yet, but figured I'd start with raising the flag in case anyone recognizes this. https://github.com/llvm/llvm-project/pull/74786 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 5bc1adf - Revert "lldb: Cache string hash during ConstString pool queries/insertions"
Author: David Blaikie
Date: 2023-12-14T17:44:18Z
New Revision: 5bc1adff69315dcef670e9fcbe04067b5d5963fb
URL:
https://github.com/llvm/llvm-project/commit/5bc1adff69315dcef670e9fcbe04067b5d5963fb
DIFF:
https://github.com/llvm/llvm-project/commit/5bc1adff69315dcef670e9fcbe04067b5d5963fb.diff
LOG: Revert "lldb: Cache string hash during ConstString pool queries/insertions"
Underlying StringMap API for providing a hash has caused some problems
(observed a crash in lld) - so reverting this until I can figure out/fix
what's going on there.
This reverts commit 52ba075571958e2fec8d871ddfa1ef56486b86d3.
This reverts commit 2e197602305be18b963928e6ae024a004a95af6d.
Added:
Modified:
lldb/source/Utility/ConstString.cpp
llvm/lib/Support/StringMap.cpp
Removed:
diff --git a/lldb/source/Utility/ConstString.cpp
b/lldb/source/Utility/ConstString.cpp
index ea897dc611cc94..4535771adfb735 100644
--- a/lldb/source/Utility/ConstString.cpp
+++ b/lldb/source/Utility/ConstString.cpp
@@ -77,10 +77,10 @@ class Pool {
return 0;
}
- StringPoolValueType GetMangledCounterpart(const char *ccstr) {
+ StringPoolValueType GetMangledCounterpart(const char *ccstr) const {
if (ccstr != nullptr) {
- const PoolEntry = selectPool(llvm::StringRef(ccstr));
- llvm::sys::SmartScopedReader rlock(pool.m_mutex);
+ const uint8_t h = hash(llvm::StringRef(ccstr));
+ llvm::sys::SmartScopedReader rlock(m_string_pools[h].m_mutex);
return GetStringMapEntryFromKeyData(ccstr).getValue();
}
return nullptr;
@@ -100,20 +100,19 @@ class Pool {
const char *GetConstCStringWithStringRef(llvm::StringRef string_ref) {
if (string_ref.data()) {
- const uint32_t string_hash = StringPool::hash(string_ref);
- PoolEntry = selectPool(string_hash);
+ const uint8_t h = hash(string_ref);
{
-llvm::sys::SmartScopedReader rlock(pool.m_mutex);
-auto it = pool.m_string_map.find(string_ref, string_hash);
-if (it != pool.m_string_map.end())
+llvm::sys::SmartScopedReader rlock(m_string_pools[h].m_mutex);
+auto it = m_string_pools[h].m_string_map.find(string_ref);
+if (it != m_string_pools[h].m_string_map.end())
return it->getKeyData();
}
- llvm::sys::SmartScopedWriter wlock(pool.m_mutex);
+ llvm::sys::SmartScopedWriter wlock(m_string_pools[h].m_mutex);
StringPoolEntryType =
- *pool.m_string_map
- .insert(std::make_pair(string_ref, nullptr), string_hash)
+ *m_string_pools[h]
+ .m_string_map.insert(std::make_pair(string_ref, nullptr))
.first;
return entry.getKeyData();
}
@@ -126,14 +125,12 @@ class Pool {
const char *demangled_ccstr = nullptr;
{
- const uint32_t demangled_hash = StringPool::hash(demangled);
- PoolEntry = selectPool(demangled_hash);
- llvm::sys::SmartScopedWriter wlock(pool.m_mutex);
+ const uint8_t h = hash(demangled);
+ llvm::sys::SmartScopedWriter wlock(m_string_pools[h].m_mutex);
// Make or update string pool entry with the mangled counterpart
- StringPool = pool.m_string_map;
- StringPoolEntryType =
- *map.try_emplace_with_hash(demangled, demangled_hash).first;
+ StringPool = m_string_pools[h].m_string_map;
+ StringPoolEntryType = *map.try_emplace(demangled).first;
entry.second = mangled_ccstr;
@@ -144,8 +141,8 @@ class Pool {
{
// Now assign the demangled const string as the counterpart of the
// mangled const string...
- PoolEntry = selectPool(llvm::StringRef(mangled_ccstr));
- llvm::sys::SmartScopedWriter wlock(pool.m_mutex);
+ const uint8_t h = hash(llvm::StringRef(mangled_ccstr));
+ llvm::sys::SmartScopedWriter wlock(m_string_pools[h].m_mutex);
GetStringMapEntryFromKeyData(mangled_ccstr).setValue(demangled_ccstr);
}
@@ -174,20 +171,17 @@ class Pool {
}
protected:
+ uint8_t hash(llvm::StringRef s) const {
+uint32_t h = llvm::djbHash(s);
+return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff;
+ }
+
struct PoolEntry {
mutable llvm::sys::SmartRWMutex m_mutex;
StringPool m_string_map;
};
std::array m_string_pools;
-
- PoolEntry (const llvm::StringRef ) {
-return selectPool(StringPool::hash(s));
- }
-
- PoolEntry (uint32_t h) {
-return m_string_pools[((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff];
- }
};
// Frameworks and dylibs aren't supposed to have global C++ initializers so we
@@ -203,7 +197,7 @@ static Pool () {
static Pool *g_string_pool = nullptr;
llvm::call_once(g_pool_initialization_flag,
- []() { g_string_pool = new Pool(); });
+ []() { g_string_pool = new Pool(); });
return *g_string_pool;
}
diff --git a/llvm/lib/Support/StringMap.cpp
[Lldb-commits] [lldb] 2e19760 - lldb: Cache string hash during ConstString pool queries/insertions
Author: David Blaikie
Date: 2023-12-12T00:07:08Z
New Revision: 2e197602305be18b963928e6ae024a004a95af6d
URL:
https://github.com/llvm/llvm-project/commit/2e197602305be18b963928e6ae024a004a95af6d
DIFF:
https://github.com/llvm/llvm-project/commit/2e197602305be18b963928e6ae024a004a95af6d.diff
LOG: lldb: Cache string hash during ConstString pool queries/insertions
lldb was rehashing the string 3 times (once to determine which StringMap
to use, once to query the StringMap, once to insert) on insertion (twice
on successful lookup).
This patch allows the lldb to benefit from hash improvements in LLVM
(from djbHash to xxh3).
Though further changes would be needed to cache this value to disk - we
shouldn't rely on the StringMap::hash remaining the same in the
future/this value should not be serialized to disk. If we want cache
this value StringMap should take a hashing template parameter to allow
for a fixed hash to be requested.
Added:
Modified:
lldb/source/Utility/ConstString.cpp
Removed:
diff --git a/lldb/source/Utility/ConstString.cpp
b/lldb/source/Utility/ConstString.cpp
index 4535771adfb735..ea897dc611cc94 100644
--- a/lldb/source/Utility/ConstString.cpp
+++ b/lldb/source/Utility/ConstString.cpp
@@ -77,10 +77,10 @@ class Pool {
return 0;
}
- StringPoolValueType GetMangledCounterpart(const char *ccstr) const {
+ StringPoolValueType GetMangledCounterpart(const char *ccstr) {
if (ccstr != nullptr) {
- const uint8_t h = hash(llvm::StringRef(ccstr));
- llvm::sys::SmartScopedReader rlock(m_string_pools[h].m_mutex);
+ const PoolEntry = selectPool(llvm::StringRef(ccstr));
+ llvm::sys::SmartScopedReader rlock(pool.m_mutex);
return GetStringMapEntryFromKeyData(ccstr).getValue();
}
return nullptr;
@@ -100,19 +100,20 @@ class Pool {
const char *GetConstCStringWithStringRef(llvm::StringRef string_ref) {
if (string_ref.data()) {
- const uint8_t h = hash(string_ref);
+ const uint32_t string_hash = StringPool::hash(string_ref);
+ PoolEntry = selectPool(string_hash);
{
-llvm::sys::SmartScopedReader rlock(m_string_pools[h].m_mutex);
-auto it = m_string_pools[h].m_string_map.find(string_ref);
-if (it != m_string_pools[h].m_string_map.end())
+llvm::sys::SmartScopedReader rlock(pool.m_mutex);
+auto it = pool.m_string_map.find(string_ref, string_hash);
+if (it != pool.m_string_map.end())
return it->getKeyData();
}
- llvm::sys::SmartScopedWriter wlock(m_string_pools[h].m_mutex);
+ llvm::sys::SmartScopedWriter wlock(pool.m_mutex);
StringPoolEntryType =
- *m_string_pools[h]
- .m_string_map.insert(std::make_pair(string_ref, nullptr))
+ *pool.m_string_map
+ .insert(std::make_pair(string_ref, nullptr), string_hash)
.first;
return entry.getKeyData();
}
@@ -125,12 +126,14 @@ class Pool {
const char *demangled_ccstr = nullptr;
{
- const uint8_t h = hash(demangled);
- llvm::sys::SmartScopedWriter wlock(m_string_pools[h].m_mutex);
+ const uint32_t demangled_hash = StringPool::hash(demangled);
+ PoolEntry = selectPool(demangled_hash);
+ llvm::sys::SmartScopedWriter wlock(pool.m_mutex);
// Make or update string pool entry with the mangled counterpart
- StringPool = m_string_pools[h].m_string_map;
- StringPoolEntryType = *map.try_emplace(demangled).first;
+ StringPool = pool.m_string_map;
+ StringPoolEntryType =
+ *map.try_emplace_with_hash(demangled, demangled_hash).first;
entry.second = mangled_ccstr;
@@ -141,8 +144,8 @@ class Pool {
{
// Now assign the demangled const string as the counterpart of the
// mangled const string...
- const uint8_t h = hash(llvm::StringRef(mangled_ccstr));
- llvm::sys::SmartScopedWriter wlock(m_string_pools[h].m_mutex);
+ PoolEntry = selectPool(llvm::StringRef(mangled_ccstr));
+ llvm::sys::SmartScopedWriter wlock(pool.m_mutex);
GetStringMapEntryFromKeyData(mangled_ccstr).setValue(demangled_ccstr);
}
@@ -171,17 +174,20 @@ class Pool {
}
protected:
- uint8_t hash(llvm::StringRef s) const {
-uint32_t h = llvm::djbHash(s);
-return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff;
- }
-
struct PoolEntry {
mutable llvm::sys::SmartRWMutex m_mutex;
StringPool m_string_map;
};
std::array m_string_pools;
+
+ PoolEntry (const llvm::StringRef ) {
+return selectPool(StringPool::hash(s));
+ }
+
+ PoolEntry (uint32_t h) {
+return m_string_pools[((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff];
+ }
};
// Frameworks and dylibs aren't supposed to have global C++ initializers so we
@@ -197,7 +203,7 @@ static Pool () {
static Pool *g_string_pool = nullptr;
[Lldb-commits] [lldb] [clang] [clang][DebugInfo] Revert "emit definitions for constant-initialized static data-members" (PR #74580)
dwblaikie wrote: > To support access to such constants from LLDB we'll most likely have to have > to make LLDB find the constants by looking at the containing class first. Tangentially related to: https://discourse.llvm.org/t/rfc-improve-dwarf-5-debug-names-type-lookup-parsing-speed/74151/31?u=dblaikie Clang/LLVM do know the difference between type-invariant members and type variant ones (type invariant members are in the `members` list of the `DICompositeType` and variant members have a `scope` that refers to the type but don't appear in the `members` list) - would it be reasonable to not include the invariant members in the accelerator table, but only include the variant ones? Invariant ones can be found by finding any instance of the type in the index, then looking at its members - and for variant members it'd be useful to have them in the index. (though, honestly, I'm not sure how lldb and gdb handle that situation - last time I tested it with gdb, it just saw two different copies of the type and didn't try to unify/aggregate all the variant members... but lldb only creates one copy of the type, so don't know what it does if you've got, say, two member function template instantiations for different template parameters in two different translation units/compile units) https://github.com/llvm/llvm-project/pull/74580 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Emit DIE's size in bits when size is not a multiple of 8 (PR #69741)
dwblaikie wrote: > > I'm arguing it doesn't fit it particularly well. We use it for bit fields - > > which are pretty special, for instance, but it seems like this thing isn't > > quite like that - it does have a whole byte size (if you allocated an array > > of them, for instance, I'm guessing they're a byte each, right?) but then > > has some padding bits that can be reused in some circumstances? That's why > > I'm saying it seems like it fits more closely to the struct padding > > representation. > > Swift is really clever at packing at packing aggregate types. For example, > the discriminator bits for enums are always stored in unused bits of the > payload type. For a contrived example, the type Optional has a size > of 3 bits. Sure enough - C++ tail padding does similar things: https://godbolt.org/z/4jsYWK6ev - where t2's member is packed into the tail padding of t1, but still leaves more tail padding for t3. (it's not identical, of course - it's somewhere between C++ tail padding and the ad-hoc stuff we have in LLVM for PointerIntPair, where multiple of those can be nested together and use the remaining bits for another element) https://github.com/llvm/llvm-project/pull/69741 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Emit DIE's size in bits when size is not a multiple of 8 (PR #69741)
dwblaikie wrote: > > I guess one question that might be relevant - does Swift have something > > like sizeof and what result does it give for these sort of types with bits > > to spare? > > You can't actually use that with these types as these are special compiler > builtin types which aren't actually accessible in source code. Perhaps observable indirectly? > > But like I said - it seems like structs with tail padding are similar to > > this situation - we still describe the whole size of the struct, because > > that's used for creating arrays of instances, ABI passing, etc. But the > > tail padding can still be used, in certain circumstances, when laying out a > > derived class. We encode this as the POD-ness of the type, and so if you > > wanted to create a class that derived from one described in DWARF you could > > do so & would know whether or not to put the derived class's members into > > the tail padding of the base or not. > > I understand the rationale of basing this on precedent, but in this case in > this case we should break from it for two reasons: > > * DW_AT_BIT_SIZE is already a standardized attribute in Dwarf that fits this > use case. I'm arguing it doesn't fit it particularly well. We use it for bit fields - which are pretty special, for instance, but it seems like this thing isn't quite like that - it does have a whole byte size (if you allocated an array of them, for instance, I'm guessing they're a byte each, right?) but then has some padding bits that can be reused in some circumstances? That's why I'm saying it seems like it fits more closely to the struct padding representation. > * Round up to the nearest byte would lose information, which can be kept with > fairly minimal downsides in my opinion. Seems like it'd still need to be special cased, right? The consumer would go "oh, this has a bit size, but if we want an array of them, or to allocate them for ABI purposes, etc, I have to round it up to the nearest byte"? or something like that. Some pointers to documentation about these types, and the range of uses/instances there are might be handy (like is this a general concept? Or is it only one type that uses this (`bool` equivalent, with 7 padding bytes unused) or a class of types (a small finite list of them? Unbounded (like if I put a bool in my custom struct - does my custom struct end up with a bit size too?)) https://github.com/llvm/llvm-project/pull/69741 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Emit DIE's size in bits when size is not a multiple of 8 (PR #69741)
dwblaikie wrote: > @dwblaikie I talked with Adrian, his idea is to emit the size in bits > whenever it does not cleanly fit in a byte. His point (and I agree) is that > there may be tools that can use this bit size, and if we always round up when > emitting the DWARF we will lose this information, with no way to recover it. > I updated the patch to implement this behavior. Yeah - ultimately it's pretty much up to you folks how you encode Swift and ObjC things - so take anything I'm saying as advice but not requirement. But it feels a bit at-odds with precedent in the way other things that seem similar to me are encoded in DWARF already, and precedent is pretty much most/all we have to go with in DWARF. I guess one question that might be relevant - does Swift have something like `sizeof` and what result does it give for these sort of types with bits to spare? But like I said - it seems like structs with tail padding are similar to this situation - we still describe the whole size of the struct, because that's used for creating arrays of instances, ABI passing, etc. But the tail padding can still be used, in certain circumstances, when laying out a derived class. We encode this as the POD-ness of the type, and so if you wanted to create a class that derived from one described in DWARF you could do so & would know whether or not to put the derived class's members into the tail padding of the base or not. https://github.com/llvm/llvm-project/pull/69741 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][DebugInfo] Emit global variable definitions for static data members with constant initializers (PR #70639)
dwblaikie wrote: > 2) always put DW_AT_const_value in DW_TAG_member. My understanding is that this is not possible. Dependent initializer expressions can't be evaluated in all cases - we're only allowed to evaluate them in the places the language allows us to, otherwise we might produce the wrong answer. For non-dependent initializer expressions I think we could produce the answer in all cases. And if we want type descriptions that are consistent (valuable for use with Type Units, and with @avl-llvm's DWARFLinker work) and we want to include static member variables in those descriptions, then we could put the constant value in the declaration of the member in cases where the initializer is non-dependent. But we'd still have to put it out in a definition in the dependent cases (if we want consistency). And if we have to put it out in a separate definition DIE anyway - probably good to do that consistently so there's fewer special cases? Admittedly there are other reasons type definitions are inconsistent (eg: implicit special members, nested types, and member function template specializations (& I guess static member variable template specializations)) - and we could move static variables out of the authoritative type definitions the same way we do for those cases. We can see this in type units (the type unit never has these entities in it, but the skeleton unit that references the type unit does have these features) - I'd expect something like that to be what @avl-llvm will want to do with DWARFLinker - though without type units in input, I'm not sure how easily it'll be to determine that those are the variant parts that shouldn't go in the canonical type definition... https://github.com/llvm/llvm-project/pull/70639 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [clang] [clang][DebugInfo] Emit global variable definitions for static data members with constant initializers (PR #70639)
dwblaikie wrote: > That's true, if defined in a header, we'll emit a DW_TAG_variable for the > constant in each compile unit the header is included in. GCC does do the > right thing and only emit the definition DIE in a single CU. We should > probably do the same. Though not sure at which level we want to catch that. Which variable are you discussing here, `val1` or `val2`? For `val1`, we could not emit the constant value and only emit the real definition (there's /some/ risk here - non-ODR uses (or otherwise optimized away uses) of the variable may mean that the object file that defines the variable won't be linked in - so we'd miss the constant value) For `val2` the variable is effectively `inline` and doesn't have a home, so there's no one place that we can emit the `DW_TAG_variable` out-of-line definition... https://github.com/llvm/llvm-project/pull/70639 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][DebugInfo] Emit global variable definitions for static data members with constant initializers (PR #70639)
https://github.com/dwblaikie approved this pull request. Few minor issues, but looks good to me. (you metnioned somewhere an lldb issue I think with this patch when the value is removed from the static member declaration inside the class? If that's a problem - probably hold off on committing this until lldb's been fixed so this doesn't regress things? And document the dependence clearly in the commit message) https://github.com/llvm/llvm-project/pull/70639 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][DebugInfo] Emit global variable definitions for static data members with constant initializers (PR #70639)
@@ -0,0 +1,87 @@
+// RUN: %clangxx -target arm64-apple-macosx11.0.0 -g %s -emit-llvm -S -o - |
FileCheck --check-prefixes=CHECK %s
+
+enum class Enum : int {
+ VAL = -1
+};
+
+struct Empty {};
+
+constexpr auto func() { return 25; }
+
+struct Foo {
+static constexpr int cexpr_int = func();
+static constexpr int cexpr_int2 = func() + 1;
+static constexpr float cexpr_float = 2.0 + 1.0;
+static constexpr Enum cexpr_enum = Enum::VAL;
+static constexpr Empty cexpr_empty{};
+static inlineEnum inline_enum = Enum::VAL;
+
+template
+static constexpr T cexpr_template{};
+};
+
+int main() {
+Foo f;
+
+// Force global variable definitions to be emitted.
+(void)::cexpr_int;
+(void)::cexpr_empty;
+
+return Foo::cexpr_int + Foo::cexpr_float
+ + (int)Foo::cexpr_enum + Foo::cexpr_template;
+}
+
+// CHECK: @{{.*}}cexpr_int{{.*}} =
+// CHECK-SAME: !dbg ![[INT_GLOBAL:[0-9]+]]
+
+// CHECK: @{{.*}}cexpr_empty{{.*}} =
+// CHECK-SAME!dbg ![[EMPTY_GLOBAL:[0-9]+]]
+
+// CHECK: !DIGlobalVariableExpression(var: ![[INT_VAR:[0-9]+]], expr:
!DIExpression())
dwblaikie wrote:
Ah, that was the intent (the line 27 taking address-of)? Maybe give the
variable a more descriptive name to make it clear this is testing the case
where the variable has a definition emitted to the IR
https://github.com/llvm/llvm-project/pull/70639
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][DebugInfo] Emit global variable definitions for static data members with constant initializers (PR #70639)
@@ -5883,6 +5907,18 @@ void CGDebugInfo::finalize() {
DBuilder.replaceTemporary(std::move(FwdDecl), cast(Repl));
}
+ for (auto const *VD : StaticDataMemberDefinitionsToEmit) {
+assert(VD->isStaticDataMember());
+
+if (auto It = DeclCache.find(VD); It != DeclCache.end())
dwblaikie wrote:
DeclCache.contains might be simpler to use/read here?
https://github.com/llvm/llvm-project/pull/70639
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][DebugInfo] Emit global variable definitions for static data members with constant initializers (PR #70639)
@@ -67,15 +67,15 @@ int C::a = 4; // CHECK-NOT:size: // CHECK-NOT:align: // CHECK-NOT:offset: -// CHECK-SAME: flags: DIFlagStaticMember, -// CHECK-SAME: extraData: i1 true) +// CHECK-SAME: flags: DIFlagStaticMember +// CHECK-NOT:extraData: dwblaikie wrote: Perhaps this test case could be extended to check the constatn values on the definition metadata? & /maybe/ we don't need a new test case? (or maybe we do, but perhaps they could be added to this file, rather than another one?) https://github.com/llvm/llvm-project/pull/70639 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [clang] [clang][DebugInfo] Emit global variable definitions for static data members with constant initializers (PR #70639)
dwblaikie wrote: size report sounds generally OK - but there's a bunch of what look like unrelated or strange results in there, perhaps they weren't compared at exactly the same version? (like why did the apple_names size go down? How did the .text and .debuG_line size change at all? ) https://github.com/llvm/llvm-project/pull/70639 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [clang][DebugInfo] Emit global variable definitions for static data members with constant initializers (PR #70639)
dwblaikie wrote: > > > Should not we remove constant initializer from the type definition if we > > > have a DW_TAG_variable with a DW_AT_const_value now? > > > > > > Yep, +1 to this. This is where the type normalization benefit would come > > from - no longer having this const value on the member declaration, but > > only on the definition. > > Makes sense. I'll create a separate PR for that, unless people are fine with > doing that as part of this change If it's not too much code might make sense to do it as part of this change - so the size impacts are more clear, for instance. https://github.com/llvm/llvm-project/pull/70639 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [clang][DebugInfo] Emit global variable definitions for static data members with constant initializers (PR #70639)
dwblaikie wrote: > Summing the size of all *.o files in my local debug Clang build increased by > 0.7% with this patch That's a bit significant. Any chance you could get a per-section breakdown/growth on that? (& exact flags - is this with compressed debug info, for instance? Or not?) I use `bloaty` for this ( https://github.com/google/bloaty ) - though it's a bit involved to do this to sum over all the object files - summing up the results from per-object file, etc... https://github.com/llvm/llvm-project/pull/70639 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [clang][DebugInfo] Emit global variable definitions for static data members with constant initializers (PR #70639)
dwblaikie wrote: > Should not we remove constant initializer from the type definition if we have > a DW_TAG_variable with a DW_AT_const_value now? Yep, +1 to this. This is where the type normalization benefit would come from - no longer having this const value on the member declaration, but only on the definition. https://github.com/llvm/llvm-project/pull/70639 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [clang][DebugInfo] Emit global variable definitions for static data members with constant initializers (PR #70639)
@@ -0,0 +1,87 @@
+// RUN: %clangxx -target arm64-apple-macosx11.0.0 -g %s -emit-llvm -S -o - |
FileCheck --check-prefixes=CHECK %s
+
+enum class Enum : int {
+ VAL = -1
+};
+
+struct Empty {};
+
+constexpr auto func() { return 25; }
+
+struct Foo {
+static constexpr int cexpr_int = func();
+static constexpr int cexpr_int2 = func() + 1;
+static constexpr float cexpr_float = 2.0 + 1.0;
+static constexpr Enum cexpr_enum = Enum::VAL;
+static constexpr Empty cexpr_empty{};
+static inlineEnum inline_enum = Enum::VAL;
+
+template
+static constexpr T cexpr_template{};
+};
+
+int main() {
+Foo f;
+
+// Force global variable definitions to be emitted.
+(void)::cexpr_int;
+(void)::cexpr_empty;
+
+return Foo::cexpr_int + Foo::cexpr_float
+ + (int)Foo::cexpr_enum + Foo::cexpr_template;
+}
+
+// CHECK: @{{.*}}cexpr_int{{.*}} =
+// CHECK-SAME: !dbg ![[INT_GLOBAL:[0-9]+]]
+
+// CHECK: @{{.*}}cexpr_empty{{.*}} =
+// CHECK-SAME!dbg ![[EMPTY_GLOBAL:[0-9]+]]
+
+// CHECK: !DIGlobalVariableExpression(var: ![[INT_VAR:[0-9]+]], expr:
!DIExpression())
dwblaikie wrote:
Why doesn't this have a value/expression? (but INT_VAR2 does)
https://github.com/llvm/llvm-project/pull/70639
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve debug names index fetching global variables performance (PR #70231)
@@ -129,8 +129,19 @@ void DebugNamesDWARFIndex::GetGlobalVariables(
DWARFUnit , llvm::function_ref callback) {
uint64_t cu_offset = cu.GetOffset();
bool found_entry_for_cu = false;
- for (const DebugNames::NameIndex : *m_debug_names_up) {
-for (DebugNames::NameTableEntry nte: ni) {
+ for (const DebugNames::NameIndex : *m_debug_names_up) {
+// Check if this name index contains an entry for the given CU.
+bool cu_matches = false;
+for (uint32_t i = 0; i < ni.getCUCount(); ++i) {
+ if (ni.getCUOffset(i) == cu_offset) {
dwblaikie wrote:
Oh, also, if you kept the result (more like a `llvm::find_if` as @bulbazord was
suggesting, rather than my `llvm::none_of` here) of this search, you could save
a small amount of time (no need to indirect through the index and reapply
relocations to get the CU offset) by using `getCUInedx()` and comparing that to
the index you would've found in this search - down on line 139/150
https://github.com/llvm/llvm-project/pull/70231
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve debug names index fetching global variables performance (PR #70231)
@@ -129,8 +129,19 @@ void DebugNamesDWARFIndex::GetGlobalVariables(
DWARFUnit , llvm::function_ref callback) {
uint64_t cu_offset = cu.GetOffset();
bool found_entry_for_cu = false;
- for (const DebugNames::NameIndex : *m_debug_names_up) {
-for (DebugNames::NameTableEntry nte: ni) {
+ for (const DebugNames::NameIndex : *m_debug_names_up) {
+// Check if this name index contains an entry for the given CU.
+bool cu_matches = false;
+for (uint32_t i = 0; i < ni.getCUCount(); ++i) {
+ if (ni.getCUOffset(i) == cu_offset) {
dwblaikie wrote:
If NameIndex exposed the CUOffsets as a range (which seems pretty
easy/reasonable for it to do - ah, because it requires potentially applying
relocations it'd probably require a custom iterator - maybe a mapped iterator
would be adequate & easy to do) then this could be written as:
```
if (llvm::none_of(ni.CUOffsets(), [&](uint64_t off) { return off == cu_offset;
}))
continue;
```
I /think/ `CUOffsets()` would look something roughly like this:
```
auto CUOffsets() const {
assert(TU < Hdr.CompUnitCount);
const unsigned SectionOffsetSize = dwarf::getDwarfOffsetByteSize(Hdr.Format);
uint64_t Offset = CUsBase + SectionOffsetSize * CU;
auto R = /* Guess we need some sort of generator to produce the values
[CUsBase, CUsBase+SectionOffsetSize*Hdr.CompUnitCount) in SectionOffsetSize
increments...
some enhancement to llvm::seq that takes a stride size would be suitable */
return llvm::map_range(llvm::seq(CUsBase,
[&](uint64_t Offset) {
return Section.AccelSection.getRelocatedValue(SectionOffsetSize,
);
});
}
```
https://github.com/llvm/llvm-project/pull/70231
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFCI] Remove duplicated code in DWARFParser (PR #69531)
https://github.com/dwblaikie approved this pull request. https://github.com/llvm/llvm-project/pull/69531 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFCI] Remove duplicated code in DWARFParser (PR #69531)
@@ -73,137 +69,18 @@ bool DWARFDebugInfoEntry::Extract(const DWARFDataExtractor
,
m_tag = abbrevDecl->getTag();
m_has_children = abbrevDecl->hasChildren();
// Skip all data in the .debug_info or .debug_types for the attributes
- dw_form_t form;
for (const auto : abbrevDecl->attributes()) {
-form = attribute.Form;
-std::optional fixed_skip_size =
-DWARFFormValue::GetFixedSize(form, cu);
-if (fixed_skip_size)
- offset += *fixed_skip_size;
-else {
- bool form_is_indirect = false;
- do {
-form_is_indirect = false;
-uint32_t form_size = 0;
-switch (form) {
-// Blocks if inlined data that have a length field and the data bytes
-// inlined in the .debug_info/.debug_types
-case DW_FORM_exprloc:
-case DW_FORM_block:
- form_size = data.GetULEB128();
- break;
-case DW_FORM_block1:
- form_size = data.GetU8_unchecked();
- break;
-case DW_FORM_block2:
- form_size = data.GetU16_unchecked();
- break;
-case DW_FORM_block4:
- form_size = data.GetU32_unchecked();
- break;
-
-// Inlined NULL terminated C-strings
-case DW_FORM_string:
- data.GetCStr();
- break;
-
-// Compile unit address sized values
-case DW_FORM_addr:
- form_size = cu->GetAddressByteSize();
- break;
-case DW_FORM_ref_addr:
- if (cu->GetVersion() <= 2)
-form_size = cu->GetAddressByteSize();
- else
-form_size = 4;
- break;
+if (DWARFFormValue::SkipValue(attribute.Form, data, offset_ptr, cu))
+ continue;
-// 0 sized form
-case DW_FORM_flag_present:
- form_size = 0;
- break;
-
-// 1 byte values
-case DW_FORM_addrx1:
-case DW_FORM_data1:
-case DW_FORM_flag:
-case DW_FORM_ref1:
-case DW_FORM_strx1:
- form_size = 1;
- break;
-
-// 2 byte values
-case DW_FORM_addrx2:
-case DW_FORM_data2:
-case DW_FORM_ref2:
-case DW_FORM_strx2:
- form_size = 2;
- break;
-
-// 3 byte values
-case DW_FORM_addrx3:
-case DW_FORM_strx3:
- form_size = 3;
- break;
-
-// 4 byte values
-case DW_FORM_addrx4:
-case DW_FORM_data4:
-case DW_FORM_ref4:
-case DW_FORM_strx4:
- form_size = 4;
- break;
-
-// 8 byte values
-case DW_FORM_data8:
-case DW_FORM_ref8:
-case DW_FORM_ref_sig8:
- form_size = 8;
- break;
-
-// signed or unsigned LEB 128 values
-case DW_FORM_addrx:
-case DW_FORM_loclistx:
-case DW_FORM_rnglistx:
-case DW_FORM_sdata:
-case DW_FORM_udata:
-case DW_FORM_ref_udata:
-case DW_FORM_GNU_addr_index:
-case DW_FORM_GNU_str_index:
-case DW_FORM_strx:
- data.Skip_LEB128();
dwblaikie wrote:
Ah, right - of course. Thanks for walking me through it!
https://github.com/llvm/llvm-project/pull/69531
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFCI] Remove duplicated code in DWARFParser (PR #69531)
@@ -73,137 +69,18 @@ bool DWARFDebugInfoEntry::Extract(const DWARFDataExtractor
,
m_tag = abbrevDecl->getTag();
m_has_children = abbrevDecl->hasChildren();
// Skip all data in the .debug_info or .debug_types for the attributes
- dw_form_t form;
for (const auto : abbrevDecl->attributes()) {
-form = attribute.Form;
-std::optional fixed_skip_size =
-DWARFFormValue::GetFixedSize(form, cu);
-if (fixed_skip_size)
- offset += *fixed_skip_size;
-else {
- bool form_is_indirect = false;
- do {
-form_is_indirect = false;
-uint32_t form_size = 0;
-switch (form) {
-// Blocks if inlined data that have a length field and the data bytes
-// inlined in the .debug_info/.debug_types
-case DW_FORM_exprloc:
-case DW_FORM_block:
- form_size = data.GetULEB128();
- break;
-case DW_FORM_block1:
- form_size = data.GetU8_unchecked();
- break;
-case DW_FORM_block2:
- form_size = data.GetU16_unchecked();
- break;
-case DW_FORM_block4:
- form_size = data.GetU32_unchecked();
- break;
-
-// Inlined NULL terminated C-strings
-case DW_FORM_string:
- data.GetCStr();
- break;
-
-// Compile unit address sized values
-case DW_FORM_addr:
- form_size = cu->GetAddressByteSize();
- break;
-case DW_FORM_ref_addr:
- if (cu->GetVersion() <= 2)
-form_size = cu->GetAddressByteSize();
- else
-form_size = 4;
- break;
+if (DWARFFormValue::SkipValue(attribute.Form, data, offset_ptr, cu))
+ continue;
-// 0 sized form
-case DW_FORM_flag_present:
- form_size = 0;
- break;
-
-// 1 byte values
-case DW_FORM_addrx1:
-case DW_FORM_data1:
-case DW_FORM_flag:
-case DW_FORM_ref1:
-case DW_FORM_strx1:
- form_size = 1;
- break;
-
-// 2 byte values
-case DW_FORM_addrx2:
-case DW_FORM_data2:
-case DW_FORM_ref2:
-case DW_FORM_strx2:
- form_size = 2;
- break;
-
-// 3 byte values
-case DW_FORM_addrx3:
-case DW_FORM_strx3:
- form_size = 3;
- break;
-
-// 4 byte values
-case DW_FORM_addrx4:
-case DW_FORM_data4:
-case DW_FORM_ref4:
-case DW_FORM_strx4:
- form_size = 4;
- break;
-
-// 8 byte values
-case DW_FORM_data8:
-case DW_FORM_ref8:
-case DW_FORM_ref_sig8:
- form_size = 8;
- break;
-
-// signed or unsigned LEB 128 values
-case DW_FORM_addrx:
-case DW_FORM_loclistx:
-case DW_FORM_rnglistx:
-case DW_FORM_sdata:
-case DW_FORM_udata:
-case DW_FORM_ref_udata:
-case DW_FORM_GNU_addr_index:
-case DW_FORM_GNU_str_index:
-case DW_FORM_strx:
- data.Skip_LEB128();
dwblaikie wrote:
One loss seems to be that the other code uses the `get` rather than `skip`
version of the LEB parsing DataExtractor functions - so maybe those should be
changed/improved (or, if that optimization isn't worthwhile, perhaps we can
remove the skip versions)?
Otherwies all the cases seemed to be handled equivalently between the removed
code and the new code path.
https://github.com/llvm/llvm-project/pull/69531
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-vscode] Display a more descriptive summary for containers and pointers (PR #65514)
@@ -132,6 +132,84 @@ std::vector GetStrings(const
llvm::json::Object *obj,
return strs;
}
+/// Create a short summary for a container that contains the summary of its
+/// first children, so that the user can get a glimpse of its contents at a
+/// glance.
+static std::optional
+GetSyntheticSummaryForContainer(lldb::SBValue ) {
+ if (v.TypeIsPointerType() || !v.MightHaveChildren())
+return std::nullopt;
+ /// As this operation can be potentially slow, we limit the total time spent
+ /// fetching children to a few ms.
+ const auto max_evaluation_time = std::chrono::milliseconds(10);
+ /// We don't want to generate a extremely long summary string, so we limit
its
+ /// length.
+ const size_t max_length = 32;
+
+ auto start = std::chrono::steady_clock::now();
+ std::string summary;
+ llvm::raw_string_ostream os(summary);
+ os << "{";
+
+ llvm::StringRef separator = "";
+
+ for (size_t i = 0, e = v.GetNumChildren(); i < e; ++i) {
dwblaikie wrote:
Thanks for the context - how's all this compare to other debuggers? (like
Visual Studio, for example - what's it show by default for the types?)
https://github.com/llvm/llvm-project/pull/65514
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-vscode] Display a more descriptive summary for containers and pointers (PR #65514)
@@ -132,6 +132,84 @@ std::vector GetStrings(const
llvm::json::Object *obj,
return strs;
}
+/// Create a short summary for a container that contains the summary of its
+/// first children, so that the user can get a glimpse of its contents at a
+/// glance.
+static std::optional
+GetSyntheticSummaryForContainer(lldb::SBValue ) {
+ if (v.TypeIsPointerType() || !v.MightHaveChildren())
+return std::nullopt;
+ /// As this operation can be potentially slow, we limit the total time spent
+ /// fetching children to a few ms.
+ const auto max_evaluation_time = std::chrono::milliseconds(10);
+ /// We don't want to generate a extremely long summary string, so we limit
its
+ /// length.
+ const size_t max_length = 32;
+
+ auto start = std::chrono::steady_clock::now();
+ std::string summary;
+ llvm::raw_string_ostream os(summary);
+ os << "{";
+
+ llvm::StringRef separator = "";
+
+ for (size_t i = 0, e = v.GetNumChildren(); i < e; ++i) {
dwblaikie wrote:
I'm not sure I understand. `v.GetNumChildren()` wouldn't need to do any more
work for a case with "10 class pointer variables", right? You can determine how
many children this type has (10) without completing the types those pointers
point to?
Or is it later code (like line 168, etc) that try to get the summary of those
pointer member variables that are an issue? (though getting the summary of a
pointer shouldn't automatically dereference the pointer/complete its pointed-to
type, should it?)
https://github.com/llvm/llvm-project/pull/65514
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add `target modules dump separate-debug-info` (PR #66035)
dwblaikie wrote: > N_SO is the stab moniker for a source file, and N_OSO is the object file > associated with that source file (N_SO was in Sun's implementation, we made > up N_OSO). Most nm''s leave off the N_ when they print stabs, so then this > became just OSO... > We don't do DWO on Darwin, and nobody but Darwin does OSO's, so for now I > don't think you need to worry about mixing of the entities in these reports. Ah, thanks for the context - makes sense. https://github.com/llvm/llvm-project/pull/66035 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add `target modules dump separate-debug-info` (PR #66035)
dwblaikie wrote: (aside: isn't the SBAPI meant to be the thing to use if you want to script stuff, rather than having formatted output/scraping that? - but sounds like the output has converged on a table rather than JSON anyway? So maybe a moot point) Re: errors, OSO (what does that stand for anyway) - yeah, maybe separate tables for different data? (one table for OSO, one for DWO, one for errors? (or two for errors, one for OSO errors and one for DWO errors?)) https://github.com/llvm/llvm-project/pull/66035 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-vscode] Display a more descriptive summary for containers and pointers (PR #65514)
@@ -132,6 +132,84 @@ std::vector GetStrings(const
llvm::json::Object *obj,
return strs;
}
+/// Create a short summary for a container that contains the summary of its
+/// first children, so that the user can get a glimpse of its contents at a
+/// glance.
+static std::optional
+GetSyntheticSummaryForContainer(lldb::SBValue ) {
+ if (v.TypeIsPointerType() || !v.MightHaveChildren())
+return std::nullopt;
+ /// As this operation can be potentially slow, we limit the total time spent
+ /// fetching children to a few ms.
+ const auto max_evaluation_time = std::chrono::milliseconds(10);
+ /// We don't want to generate a extremely long summary string, so we limit
its
+ /// length.
+ const size_t max_length = 32;
+
+ auto start = std::chrono::steady_clock::now();
+ std::string summary;
+ llvm::raw_string_ostream os(summary);
+ os << "{";
+
+ llvm::StringRef separator = "";
+
+ for (size_t i = 0, e = v.GetNumChildren(); i < e; ++i) {
dwblaikie wrote:
What do other debuggers do? (like Visual Studio itself)
& not sure that `GetNumChildren` should cause indefinite recursive expansion -
could limit this feature to just going one level deep, for instance. But
certainly worth comparing to other debuggers to see what sort of
heuristics/rules/guidelines are good in terms of the amount of data expanded by
default V unexpanded.
(& yeah, if everything's expanded all the way down, that's probably not good)
https://github.com/llvm/llvm-project/pull/65514
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 4e429fd - Few linter fixes
Author: David Blaikie
Date: 2023-07-31T18:52:57Z
New Revision: 4e429fd2a72511bc3c0a20e1b12f735863e615df
URL:
https://github.com/llvm/llvm-project/commit/4e429fd2a72511bc3c0a20e1b12f735863e615df
DIFF:
https://github.com/llvm/llvm-project/commit/4e429fd2a72511bc3c0a20e1b12f735863e615df.diff
LOG: Few linter fixes
size() > 0 -> !empty
indentation
mismatched names on parameters in decls/defs
const on value return types
Added:
Modified:
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
Removed:
diff --git
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
index d298af92ba5e62..920a5eba20abd9 100644
---
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
+++
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
@@ -77,7 +77,7 @@ class ClassDescriptorV2 : public
ObjCLanguageRuntime::ClassDescriptor {
void GetIVarInformation();
private:
- static const uint32_t RW_REALIZED = (1 << 31);
+ static const uint32_t RW_REALIZED = (1u << 31);
struct objc_class_t {
ObjCLanguageRuntime::ObjCISA m_isa = 0; // The class's metaclass.
@@ -173,7 +173,8 @@ class ClassDescriptorV2 : public
ObjCLanguageRuntime::ClassDescriptor {
}
bool Read(Process *process, lldb::addr_t addr,
- lldb::addr_t relative_method_lists_base_addr, bool, bool);
+ lldb::addr_t relative_selector_base_addr, bool is_small,
+ bool has_direct_sel);
};
struct ivar_list_t {
diff --git
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index d5357b94d68edc..a50cdc88cd0124 100644
---
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1399,7 +1399,7 @@ class RemoteNXMapTable {
return *this;
}
-const element operator*() const {
+element operator*() const {
if (m_index == -1) {
// TODO find a way to make this an error, but not an assert
return element();
@@ -2739,7 +2739,7 @@ lldb::addr_t
AppleObjCRuntimeV2::LookupRuntimeSymbol(ConstString name) {
std::pair class_and_ivar =
ivar_skipped_prefix.split('.');
- if (class_and_ivar.first.size() && class_and_ivar.second.size()) {
+ if (!class_and_ivar.first.empty() && !class_and_ivar.second.empty()) {
const ConstString class_name_cs(class_and_ivar.first);
ClassDescriptorSP descriptor =
ObjCLanguageRuntime::GetClassDescriptorFromClassName(class_name_cs);
diff --git
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
index 678865ecd9186f..c9d0b3a907b54b 100644
---
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
+++
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
@@ -65,12 +65,12 @@ class AppleObjCRuntimeV2 : public AppleObjCRuntime {
return ObjCRuntimeVersions::eAppleObjC_V2;
}
- size_t GetByteOffsetForIvar(CompilerType _qual_type,
+ size_t GetByteOffsetForIvar(CompilerType _ast_type,
const char *ivar_name) override;
void UpdateISAToDescriptorMapIfNeeded() override;
- ClassDescriptorSP GetClassDescriptor(ValueObject _value) override;
+ ClassDescriptorSP GetClassDescriptor(ValueObject ) override;
ClassDescriptorSP GetClassDescriptorFromISA(ObjCISA isa) override;
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index 8f9d9173af9fbf..d81b634be87b1e 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -2867,7 +2867,7 @@ bool IRTranslator::translateVAArg(const User ,
MachineIRBuilder ) {
}
bool IRTranslator::translateUnreachable(const User , MachineIRBuilder
) {
-if (!MF->getTarget().Options.TrapUnreachable)
+ if (!MF->getTarget().Options.TrapUnreachable)
return true;
auto = cast(U);
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 87ba56137728b3..7191e89d36071b 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++
[Lldb-commits] [lldb] ed7be0d - lldb: Fix cross-cu-reference test to explicitly request that feature
Author: David Blaikie Date: 2023-06-01T00:35:39Z New Revision: ed7be0d4d17b5d1470587643cd5c55157414bb9c URL: https://github.com/llvm/llvm-project/commit/ed7be0d4d17b5d1470587643cd5c55157414bb9c DIFF: https://github.com/llvm/llvm-project/commit/ed7be0d4d17b5d1470587643cd5c55157414bb9c.diff LOG: lldb: Fix cross-cu-reference test to explicitly request that feature Added: Modified: lldb/test/Shell/SymbolFile/DWARF/x86/split-dwarf-multiple-cu.ll Removed: diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/split-dwarf-multiple-cu.ll b/lldb/test/Shell/SymbolFile/DWARF/x86/split-dwarf-multiple-cu.ll index 9c2348750cfb3..0cc8b1f1b8e85 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/x86/split-dwarf-multiple-cu.ll +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/split-dwarf-multiple-cu.ll @@ -1,7 +1,7 @@ ; Check handling of dwo files with multiple compile units. Right now this is not ; supported, but it should not cause us to crash or misbehave either... -; RUN: llc %s -filetype=obj -o %t.o --split-dwarf-file=%t.o +; RUN: llc %s -filetype=obj -o %t.o --split-dwarf-file=%t.o -split-dwarf-cross-cu-references ; RUN: %lldb %t.o -o "image lookup -s x1 -v" -o "image lookup -s x2 -v" -b | FileCheck %s ; CHECK: image lookup -s x1 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 40501f1 - buildbot-based-debugging a Microsoft lldb test XPASS
Author: David Blaikie Date: 2022-10-04T21:38:33Z New Revision: 40501f1b4139e472094ac0d4102923d2730515a1 URL: https://github.com/llvm/llvm-project/commit/40501f1b4139e472094ac0d4102923d2730515a1 DIFF: https://github.com/llvm/llvm-project/commit/40501f1b4139e472094ac0d4102923d2730515a1.diff LOG: buildbot-based-debugging a Microsoft lldb test XPASS Let's see if it's been fixed by my recent ABI fixes... Added: Modified: lldb/test/API/commands/expression/xvalue/TestXValuePrinting.py Removed: diff --git a/lldb/test/API/commands/expression/xvalue/TestXValuePrinting.py b/lldb/test/API/commands/expression/xvalue/TestXValuePrinting.py index 8ef0654b6444..e89ba6b7b62f 100644 --- a/lldb/test/API/commands/expression/xvalue/TestXValuePrinting.py +++ b/lldb/test/API/commands/expression/xvalue/TestXValuePrinting.py @@ -5,7 +5,6 @@ class ExprXValuePrintingTestCase(TestBase): -@expectedFailureAll(oslist=["windows"], archs=["i[3-6]86", "x86_64"], bugnumber="llvm.org/pr21765") def test(self): """Printing an xvalue should work.""" self.build() ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 9fc6565 - fold assert-only variable into assert to address non-assert -Wunused-variable
Author: David Blaikie Date: 2022-08-17T04:10:59Z New Revision: 9fc65658f5e5a0af5821f4945d78a9f6b8fd368f URL: https://github.com/llvm/llvm-project/commit/9fc65658f5e5a0af5821f4945d78a9f6b8fd368f DIFF: https://github.com/llvm/llvm-project/commit/9fc65658f5e5a0af5821f4945d78a9f6b8fd368f.diff LOG: fold assert-only variable into assert to address non-assert -Wunused-variable Added: Modified: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp Removed: diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index f815f3ad7d41..e298d7fee421 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -7188,8 +7188,7 @@ GetNthTemplateArgument(const clang::ClassTemplateSpecializationDecl *decl, // (including the ones preceding the parameter pack). const auto = args[last_idx]; const size_t pack_idx = idx - last_idx; - const size_t pack_size = pack.pack_size(); - assert(pack_idx < pack_size && "parameter pack index out-of-bounds"); + assert(pack_idx < pack.pack_size() && "parameter pack index out-of-bounds"); return _elements()[pack_idx]; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 72d9390 - Add a little extra test coverage for simple template names
Author: David Blaikie
Date: 2022-07-08T00:12:29Z
New Revision: 72d9390778966d4f87ec4b1de63c107b2fd46b9a
URL:
https://github.com/llvm/llvm-project/commit/72d9390778966d4f87ec4b1de63c107b2fd46b9a
DIFF:
https://github.com/llvm/llvm-project/commit/72d9390778966d4f87ec4b1de63c107b2fd46b9a.diff
LOG: Add a little extra test coverage for simple template names
This would fail with an overly naive approach to simple template
name (clang's -gsimple-template-names) since the names wouldn't be
unique per specialization, creating ambiguity/chance that a query for
one specialization would find another.
Added:
Modified:
lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
Removed:
diff --git
a/lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
b/lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
index b596611b15f2f..81a8876743474 100644
--- a/lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
+++ b/lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
@@ -14,6 +14,11 @@ def assertComplete(self, typename):
self.assertTrue(found_type.IsValid())
self.assertTrue(found_type.IsTypeComplete())
+def assertIsNotPresent(self, typename):
+""" Asserts that the type with the given name is not found. """
+found_type = self.target().FindFirstType(typename)
+self.assertFalse(found_type.IsValid())
+
def assertCompleteWithVar(self, typename):
""" Asserts that the type with the given name is complete. """
found_type = self.target().FindFirstType(typename)
@@ -41,6 +46,7 @@ def test_forward_declarations(self):
self.assertCompleteWithVar("DefinedClass")
self.assertCompleteWithVar("DefinedClassTypedef")
self.assertCompleteWithVar("DefinedTemplateClass")
+self.assertIsNotPresent("DefinedTemplateClass")
# Record types without a defining declaration are not complete.
self.assertPointeeIncomplete("FwdClass *", "fwd_class")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] b92c334 - Remove dead code: TypeMap::RemoveMismatchedTypes(TypeClass type_class)
Author: David Blaikie
Date: 2022-07-07T20:46:14Z
New Revision: b92c33495aeda7d7fa7f5d3f518c59cc5785fd9f
URL:
https://github.com/llvm/llvm-project/commit/b92c33495aeda7d7fa7f5d3f518c59cc5785fd9f
DIFF:
https://github.com/llvm/llvm-project/commit/b92c33495aeda7d7fa7f5d3f518c59cc5785fd9f.diff
LOG: Remove dead code: TypeMap::RemoveMismatchedTypes(TypeClass type_class)
Added:
Modified:
lldb/include/lldb/Symbol/TypeMap.h
lldb/source/Symbol/TypeMap.cpp
Removed:
diff --git a/lldb/include/lldb/Symbol/TypeMap.h
b/lldb/include/lldb/Symbol/TypeMap.h
index 064e2cc948b6..c200ccb9844f 100644
--- a/lldb/include/lldb/Symbol/TypeMap.h
+++ b/lldb/include/lldb/Symbol/TypeMap.h
@@ -53,15 +53,10 @@ class TypeMap {
bool Remove(const lldb::TypeSP _sp);
- void RemoveMismatchedTypes(llvm::StringRef qualified_typename,
- bool exact_match);
-
void RemoveMismatchedTypes(llvm::StringRef type_scope,
llvm::StringRef type_basename,
lldb::TypeClass type_class, bool exact_match);
- void RemoveMismatchedTypes(lldb::TypeClass type_class);
-
private:
typedef collection::iterator iterator;
typedef collection::const_iterator const_iterator;
diff --git a/lldb/source/Symbol/TypeMap.cpp b/lldb/source/Symbol/TypeMap.cpp
index 7217a29fc002..0d5f6d53e5a0 100644
--- a/lldb/source/Symbol/TypeMap.cpp
+++ b/lldb/source/Symbol/TypeMap.cpp
@@ -127,20 +127,6 @@ void TypeMap::Dump(Stream *s, bool show_context,
lldb::DescriptionLevel level) {
}
}
-void TypeMap::RemoveMismatchedTypes(llvm::StringRef qualified_typename,
-bool exact_match) {
- llvm::StringRef type_scope;
- llvm::StringRef type_basename;
- TypeClass type_class = eTypeClassAny;
- if (!Type::GetTypeScopeAndBasename(qualified_typename, type_scope,
- type_basename, type_class)) {
-type_basename = qualified_typename;
-type_scope = "";
- }
- return RemoveMismatchedTypes(type_scope, type_basename, type_class,
- exact_match);
-}
-
void TypeMap::RemoveMismatchedTypes(llvm::StringRef type_scope,
llvm::StringRef type_basename,
TypeClass type_class, bool exact_match) {
@@ -213,25 +199,3 @@ void TypeMap::RemoveMismatchedTypes(llvm::StringRef
type_scope,
}
m_types.swap(matching_types);
}
-
-void TypeMap::RemoveMismatchedTypes(TypeClass type_class) {
- if (type_class == eTypeClassAny)
-return;
-
- // Our "collection" type currently is a std::map which doesn't have any good
- // way to iterate and remove items from the map so we currently just make a
- // new list and add all of the matching types to it, and then swap it into
- // m_types at the end
- collection matching_types;
-
- iterator pos, end = m_types.end();
-
- for (pos = m_types.begin(); pos != end; ++pos) {
-Type *the_type = pos->second.get();
-TypeClass match_type_class =
-the_type->GetForwardCompilerType().GetTypeClass();
-if (match_type_class & type_class)
- matching_types.insert(*pos);
- }
- m_types.swap(matching_types);
-}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 6edbde1 - Simplify some AsCString usage that was also explicitly handling default
Author: David Blaikie
Date: 2022-07-07T20:27:05Z
New Revision: 6edbde100132f5dc025bed64059d9fb770abd19e
URL:
https://github.com/llvm/llvm-project/commit/6edbde100132f5dc025bed64059d9fb770abd19e
DIFF:
https://github.com/llvm/llvm-project/commit/6edbde100132f5dc025bed64059d9fb770abd19e.diff
LOG: Simplify some AsCString usage that was also explicitly handling default
Added:
Modified:
lldb/source/Core/Module.cpp
Removed:
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 43d1cdb5bd38..893e20837124 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -144,9 +144,7 @@ Module::Module(const ModuleSpec _spec)
module_spec.GetArchitecture().GetArchitectureName(),
module_spec.GetFileSpec().GetPath().c_str(),
module_spec.GetObjectName().IsEmpty() ? "" : "(",
- module_spec.GetObjectName().IsEmpty()
- ? ""
- : module_spec.GetObjectName().AsCString(""),
+ module_spec.GetObjectName().AsCString(""),
module_spec.GetObjectName().IsEmpty() ? "" : ")");
auto data_sp = module_spec.GetData();
@@ -254,8 +252,7 @@ Module::Module(const FileSpec _spec, const ArchSpec
,
LLDB_LOGF(log, "%p Module::Module((%s) '%s%s%s%s')",
static_cast(this), m_arch.GetArchitectureName(),
m_file.GetPath().c_str(), m_object_name.IsEmpty() ? "" : "(",
- m_object_name.IsEmpty() ? "" : m_object_name.AsCString(""),
- m_object_name.IsEmpty() ? "" : ")");
+ m_object_name.AsCString(""), m_object_name.IsEmpty() ? "" : ")");
}
Module::Module() : m_file_has_changed(false), m_first_file_changed_log(false) {
@@ -283,8 +280,7 @@ Module::~Module() {
LLDB_LOGF(log, "%p Module::~Module((%s) '%s%s%s%s')",
static_cast(this), m_arch.GetArchitectureName(),
m_file.GetPath().c_str(), m_object_name.IsEmpty() ? "" : "(",
- m_object_name.IsEmpty() ? "" : m_object_name.AsCString(""),
- m_object_name.IsEmpty() ? "" : ")");
+ m_object_name.AsCString(""), m_object_name.IsEmpty() ? "" : ")");
// Release any auto pointers before we start tearing down our member
// variables since the object file and symbol files might need to make
// function calls back into this module object. The ordering is important
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 856ebe9 - Retrieve as StringRef since that's how it'll be used
Author: David Blaikie
Date: 2022-07-07T20:13:36Z
New Revision: 856ebe9e8247698095a66f98647ee5d7cb12f337
URL:
https://github.com/llvm/llvm-project/commit/856ebe9e8247698095a66f98647ee5d7cb12f337
DIFF:
https://github.com/llvm/llvm-project/commit/856ebe9e8247698095a66f98647ee5d7cb12f337.diff
LOG: Retrieve as StringRef since that's how it'll be used
Added:
Modified:
lldb/source/Core/Module.cpp
Removed:
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index c05b40c4362e..43d1cdb5bd38 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1016,7 +1016,7 @@ void Module::FindTypes(
FindTypes_Impl(name, CompilerDeclContext(), UINT_MAX,
searched_symbol_files, typesmap);
if (exact_match) {
-typesmap.RemoveMismatchedTypes(type_scope, name.AsCString(""),
+typesmap.RemoveMismatchedTypes(type_scope, name.GetStringRef(),
type_class, exact_match);
}
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 65cac0e - Use StringRef to avoid unnecessary copies into std::strings
Author: David Blaikie
Date: 2022-07-07T19:50:12Z
New Revision: 65cac0ed9266e3551663358de677161ce25a25bf
URL:
https://github.com/llvm/llvm-project/commit/65cac0ed9266e3551663358de677161ce25a25bf
DIFF:
https://github.com/llvm/llvm-project/commit/65cac0ed9266e3551663358de677161ce25a25bf.diff
LOG: Use StringRef to avoid unnecessary copies into std::strings
Added:
Modified:
lldb/include/lldb/Symbol/TypeList.h
lldb/include/lldb/Symbol/TypeMap.h
lldb/source/Core/Module.cpp
lldb/source/Symbol/TypeList.cpp
lldb/source/Symbol/TypeMap.cpp
Removed:
diff --git a/lldb/include/lldb/Symbol/TypeList.h
b/lldb/include/lldb/Symbol/TypeList.h
index 03390858025b..403469c989f5 100644
--- a/lldb/include/lldb/Symbol/TypeList.h
+++ b/lldb/include/lldb/Symbol/TypeList.h
@@ -49,10 +49,11 @@ class TypeList {
void ForEach(std::function const );
- void RemoveMismatchedTypes(const char *qualified_typename, bool exact_match);
+ void RemoveMismatchedTypes(llvm::StringRef qualified_typename,
+ bool exact_match);
- void RemoveMismatchedTypes(const std::string _scope,
- const std::string _basename,
+ void RemoveMismatchedTypes(llvm::StringRef type_scope,
+ llvm::StringRef type_basename,
lldb::TypeClass type_class, bool exact_match);
void RemoveMismatchedTypes(lldb::TypeClass type_class);
diff --git a/lldb/include/lldb/Symbol/TypeMap.h
b/lldb/include/lldb/Symbol/TypeMap.h
index ede54c1a09d4..064e2cc948b6 100644
--- a/lldb/include/lldb/Symbol/TypeMap.h
+++ b/lldb/include/lldb/Symbol/TypeMap.h
@@ -53,10 +53,11 @@ class TypeMap {
bool Remove(const lldb::TypeSP _sp);
- void RemoveMismatchedTypes(const char *qualified_typename, bool exact_match);
+ void RemoveMismatchedTypes(llvm::StringRef qualified_typename,
+ bool exact_match);
- void RemoveMismatchedTypes(const std::string _scope,
- const std::string _basename,
+ void RemoveMismatchedTypes(llvm::StringRef type_scope,
+ llvm::StringRef type_basename,
lldb::TypeClass type_class, bool exact_match);
void RemoveMismatchedTypes(lldb::TypeClass type_class);
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 41c21e1dc326..c05b40c4362e 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1000,8 +1000,7 @@ void Module::FindTypes(
FindTypes_Impl(type_basename_const_str, CompilerDeclContext(), max_matches,
searched_symbol_files, typesmap);
if (typesmap.GetSize())
- typesmap.RemoveMismatchedTypes(std::string(type_scope),
- std::string(type_basename), type_class,
+ typesmap.RemoveMismatchedTypes(type_scope, type_basename, type_class,
exact_match);
} else {
// The type is not in a namespace/class scope, just search for it by
@@ -1011,15 +1010,13 @@ void Module::FindTypes(
// class prefix (like "struct", "class", "union", "typedef" etc).
FindTypes_Impl(ConstString(type_basename), CompilerDeclContext(),
UINT_MAX, searched_symbol_files, typesmap);
- typesmap.RemoveMismatchedTypes(std::string(type_scope),
- std::string(type_basename), type_class,
+ typesmap.RemoveMismatchedTypes(type_scope, type_basename, type_class,
exact_match);
} else {
FindTypes_Impl(name, CompilerDeclContext(), UINT_MAX,
searched_symbol_files, typesmap);
if (exact_match) {
-std::string name_str(name.AsCString(""));
-typesmap.RemoveMismatchedTypes(std::string(type_scope), name_str,
+typesmap.RemoveMismatchedTypes(type_scope, name.AsCString(""),
type_class, exact_match);
}
}
diff --git a/lldb/source/Symbol/TypeList.cpp b/lldb/source/Symbol/TypeList.cpp
index ace715d933ea..494e59e3a0fc 100644
--- a/lldb/source/Symbol/TypeList.cpp
+++ b/lldb/source/Symbol/TypeList.cpp
@@ -97,7 +97,7 @@ void TypeList::Dump(Stream *s, bool show_context) {
}
}
-void TypeList::RemoveMismatchedTypes(const char *qualified_typename,
+void TypeList::RemoveMismatchedTypes(llvm::StringRef qualified_typename,
bool exact_match) {
llvm::StringRef type_scope;
llvm::StringRef type_basename;
@@ -107,13 +107,12 @@ void TypeList::RemoveMismatchedTypes(const char
*qualified_typename,
type_basename = qualified_typename;
type_scope = "";
}
- return RemoveMismatchedTypes(std::string(type_scope),
- std::string(type_basename), type_class,
+ return RemoveMismatchedTypes(type_scope, type_basename,
[Lldb-commits] [lldb] 8b280df - Rough guess at fixing lldb tests to handle Clang defaulting to DWARFv5
Author: David Blaikie Date: 2022-01-23T21:24:25-08:00 New Revision: 8b280df504b97a13d06a929fbc85348903456fdd URL: https://github.com/llvm/llvm-project/commit/8b280df504b97a13d06a929fbc85348903456fdd DIFF: https://github.com/llvm/llvm-project/commit/8b280df504b97a13d06a929fbc85348903456fdd.diff LOG: Rough guess at fixing lldb tests to handle Clang defaulting to DWARFv5 Added: Modified: lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-dwo-cross-reference.cpp Removed: diff --git a/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py b/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py index 4b9a814764158..f4ae1fc015569 100644 --- a/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py +++ b/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py @@ -15,4 +15,4 @@ lldbinline.MakeInlineTest(__file__, globals(), decorators=decorators+[skipIf(debug_info="dsym")], name="BasicEntryValues_GNU", -build_dict=dict(CXXFLAGS_EXTRAS="-O2 -ggdb")) +build_dict=dict(CXXFLAGS_EXTRAS="-O2 -ggdb -gdwarf-4")) diff --git a/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py b/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py index cbdf40e2416f7..19aad2ab1ec32 100644 --- a/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py +++ b/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py @@ -7,5 +7,5 @@ lldbinline.MakeInlineTest(__file__, globals(), name="UnambiguousTailCalls_V5", build_dict=dict(CFLAGS_EXTRAS="-O2 -glldb"), decorators=decor) lldbinline.MakeInlineTest(__file__, globals(), name="UnambiguousTailCalls_GNU", -build_dict=dict(CFLAGS_EXTRAS="-O2 -ggdb"), +build_dict=dict(CFLAGS_EXTRAS="-O2 -ggdb -gdwarf-4"), decorators=decor+[decorators.skipIf(debug_info="dsym")]) diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-dwo-cross-reference.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-dwo-cross-reference.cpp index 29adff62cd1ee..0e29cb3e7f16e 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-dwo-cross-reference.cpp +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-dwo-cross-reference.cpp @@ -7,8 +7,8 @@ // RUN: -fdebug-types-section -gsplit-dwarf -c -o %t1.o -DONE // RUN: %clang %s -target x86_64-pc-linux -fno-standalone-debug -g \ // RUN: -fdebug-types-section -gsplit-dwarf -c -o %t2.o -DTWO -// RUN: llvm-dwarfdump %t1.dwo -debug-types | FileCheck --check-prefix=ONEUNIT %s -// RUN: llvm-dwarfdump %t2.dwo -debug-types | FileCheck --check-prefix=ONEUNIT %s +// RUN: llvm-dwarfdump %t1.dwo -debug-types -debug-info | FileCheck --check-prefix=ONEUNIT %s +// RUN: llvm-dwarfdump %t2.dwo -debug-types -debug-info | FileCheck --check-prefix=ONEUNIT %s // RUN: ld.lld %t1.o %t2.o -o %t // RUN: %lldb %t -o "target var a b **b.a" -b | FileCheck %s ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] 58473d8 - [lldb] Use LLDB_VERSION_STRING instead of CLANG_VERSION_STRING
Any chance of test coverage?
On Mon, Dec 13, 2021 at 4:58 PM Jonas Devlieghere via lldb-commits <
lldb-commits@lists.llvm.org> wrote:
>
> Author: Jonas Devlieghere
> Date: 2021-12-13T16:58:39-08:00
> New Revision: 58473d84e0c7796de5dcfd3153e5d5cc8ad034b3
>
> URL:
> https://github.com/llvm/llvm-project/commit/58473d84e0c7796de5dcfd3153e5d5cc8ad034b3
> DIFF:
> https://github.com/llvm/llvm-project/commit/58473d84e0c7796de5dcfd3153e5d5cc8ad034b3.diff
>
> LOG: [lldb] Use LLDB_VERSION_STRING instead of CLANG_VERSION_STRING
>
> Added:
>
>
> Modified:
> lldb/source/Version/Version.cpp
>
> Removed:
>
>
>
>
>
> diff --git a/lldb/source/Version/Version.cpp
> b/lldb/source/Version/Version.cpp
> index ae2d83bd3e5f8..b391fe1eacd80 100644
> --- a/lldb/source/Version/Version.cpp
> +++ b/lldb/source/Version/Version.cpp
> @@ -15,7 +15,7 @@ static const char *GetLLDBVersion() {
> #ifdef LLDB_FULL_VERSION_STRING
>return LLDB_FULL_VERSION_STRING;
> #else
> - return "lldb version " CLANG_VERSION_STRING;
> + return "lldb version " LLDB_VERSION_STRING;
> #endif
> }
>
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] 6438a52 - Revert "[clang] retain type sugar in auto / template argument deduction"
Thanks for the details! On Sun, Nov 14, 2021 at 3:39 PM Matheus Izvekov wrote: > Oops, my bad! > > The reason for reverting this is that this is a recent commit, and > there are reports of breakage building libANGLE which have not been > investigated. > > See https://reviews.llvm.org/D110216 for details / future developments. > > Thanks! > > On Mon, Nov 15, 2021 at 12:33 AM David Blaikie wrote: > > > > > > > > On Sun, Nov 14, 2021 at 3:30 PM Matheus Izvekov via lldb-commits < > lldb-commits@lists.llvm.org> wrote: > >> > >> > >> Author: Matheus Izvekov > >> Date: 2021-11-15T00:29:05+01:00 > >> New Revision: 6438a52df1c7f36952b6126ff7b978861b76ad45 > >> > >> URL: > https://github.com/llvm/llvm-project/commit/6438a52df1c7f36952b6126ff7b978861b76ad45 > >> DIFF: > https://github.com/llvm/llvm-project/commit/6438a52df1c7f36952b6126ff7b978861b76ad45.diff > >> > >> LOG: Revert "[clang] retain type sugar in auto / template argument > deduction" > >> > >> This reverts commit 4d8fff477e024698facd89741cc6cf996708d598. > > > > > > Would be good to include some details about the reason for reverting > patches in the commit message - helps folks identify whether a particular > revert will address issues they might be investigating, and increases the > chance folks might avoid the same mistakes if they're pursuing a similar > direction in the future. > ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] 6438a52 - Revert "[clang] retain type sugar in auto / template argument deduction"
On Sun, Nov 14, 2021 at 3:30 PM Matheus Izvekov via lldb-commits < lldb-commits@lists.llvm.org> wrote: > > Author: Matheus Izvekov > Date: 2021-11-15T00:29:05+01:00 > New Revision: 6438a52df1c7f36952b6126ff7b978861b76ad45 > > URL: > https://github.com/llvm/llvm-project/commit/6438a52df1c7f36952b6126ff7b978861b76ad45 > DIFF: > https://github.com/llvm/llvm-project/commit/6438a52df1c7f36952b6126ff7b978861b76ad45.diff > > LOG: Revert "[clang] retain type sugar in auto / template argument > deduction" > > This reverts commit 4d8fff477e024698facd89741cc6cf996708d598. > Would be good to include some details about the reason for reverting patches in the commit message - helps folks identify whether a particular revert will address issues they might be investigating, and increases the chance folks might avoid the same mistakes if they're pursuing a similar direction in the future. ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] e6b3233 - Cleanup a few more PR36048 skips
Author: David Blaikie
Date: 2021-10-29T14:10:42-07:00
New Revision: e6b323379e3195ba98bacc9216fe38025bda6104
URL:
https://github.com/llvm/llvm-project/commit/e6b323379e3195ba98bacc9216fe38025bda6104
DIFF:
https://github.com/llvm/llvm-project/commit/e6b323379e3195ba98bacc9216fe38025bda6104.diff
LOG: Cleanup a few more PR36048 skips
Hopefully these were also fixed bi r343545 /
7fd4513920d2fed533ad420976529ef43eb42a35
Differential Revision: https://reviews.llvm.org/D112165
Added:
Modified:
lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
lldb/test/API/functionalities/data-formatter/data-formatter-globals/TestDataFormatterGlobals.py
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py
Removed:
diff --git
a/lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
b/lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
index 9d5c6ee262fee..bc681825d4a3b 100644
---
a/lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
+++
b/lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
@@ -20,8 +20,6 @@ def setUp(self):
# Find the line number to break at.
self.line = line_number('main.cpp', '// Set break point at this line.')
-@skipIf(debug_info="gmodules",
-bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048;)
def test_with_run_command(self):
"""Test that that file and class static variables display correctly."""
self.build()
diff --git
a/lldb/test/API/functionalities/data-formatter/data-formatter-globals/TestDataFormatterGlobals.py
b/lldb/test/API/functionalities/data-formatter/data-formatter-globals/TestDataFormatterGlobals.py
index 72fe2fb188528..fe9a17e4e61fd 100644
---
a/lldb/test/API/functionalities/data-formatter/data-formatter-globals/TestDataFormatterGlobals.py
+++
b/lldb/test/API/functionalities/data-formatter/data-formatter-globals/TestDataFormatterGlobals.py
@@ -20,8 +20,6 @@ def setUp(self):
# Find the line number to break at.
self.line = line_number('main.cpp', '// Set break point at this line.')
-@skipIf(debug_info="gmodules",
-bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048;)
def test_with_run_command(self):
"""Test that that file and class static variables display correctly."""
self.build()
diff --git
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py
index 584eabd389587..8de749d74f035 100644
---
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py
+++
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py
@@ -27,8 +27,6 @@ def setUp(self):
'// Set fourth break point at this line.')
@add_test_categories(["libc++"])
-@skipIf(debug_info="gmodules",
-bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048;)
def test_with_run_command(self):
"""Test that that file and class static variables display correctly."""
self.build()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] 71cfce8 - [lldb] Fix TestCompressedVectors after array type name change
Thanks for the fix! (is there a buildbot email I should've got (or did and failed to understand) about this?) On Fri, Oct 22, 2021 at 1:16 AM Raphael Isemann via lldb-commits < lldb-commits@lists.llvm.org> wrote: > > Author: Raphael Isemann > Date: 2021-10-22T10:15:53+02:00 > New Revision: 71cfce832054dfea8e79769f15a3fdc05d23b733 > > URL: > https://github.com/llvm/llvm-project/commit/71cfce832054dfea8e79769f15a3fdc05d23b733 > DIFF: > https://github.com/llvm/llvm-project/commit/71cfce832054dfea8e79769f15a3fdc05d23b733.diff > > LOG: [lldb] Fix TestCompressedVectors after array type name change > > aee49255074fd4ef38d97e6e70cbfbf2f9fd0fa7 turns array names such as `int > [1]` > into `int[1]` (without the space). This probably breaks some user > formatters, > but let's first get this test running while this is being discussed. > > Added: > > > Modified: > > lldb/test/API/functionalities/data-formatter/compactvectors/TestCompactVectors.py > > Removed: > > > > > > diff --git > a/lldb/test/API/functionalities/data-formatter/compactvectors/TestCompactVectors.py > b/lldb/test/API/functionalities/data-formatter/compactvectors/TestCompactVectors.py > index 4b36d02da14df..d7d5579b5fdbc 100644 > --- > a/lldb/test/API/functionalities/data-formatter/compactvectors/TestCompactVectors.py > +++ > b/lldb/test/API/functionalities/data-formatter/compactvectors/TestCompactVectors.py > @@ -49,8 +49,8 @@ def cleanup(): > substrs=[ > '(vFloat) valueFL = (1.25, 0, 0.25, 0)', > '(vDouble) valueDL = (1.25, 2.25)', > -'(int16_t [8]) valueI16 = (1, 0, 4, 0, 0, 1, 0, 4)', > -'(int32_t [4]) valueI32 = (1, 0, 4, 0)', > +'(int16_t[8]) valueI16 = (1, 0, 4, 0, 0, 1, 0, 4)', > +'(int32_t[4]) valueI32 = (1, 0, 4, 0)', > '(vUInt8) valueU8 = (0x01, 0x00, 0x04, 0x00, 0x00, 0x01, > 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00)', > '(vUInt16) valueU16 = (1, 0, 4, 0, 0, 1, 0, 4)', > '(vUInt32) valueU32 = (1, 2, 3, 4)', > > > > ___ > lldb-commits mailing list > lldb-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits > ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 64f002c - Follow-up fixes for aee49255074fd4ef38d97e6e70cbfbf2f9fd0fa7
Author: David Blaikie
Date: 2021-10-21T13:00:56-07:00
New Revision: 64f002c6d36d674a924c6116ec0b2d731cc3981c
URL:
https://github.com/llvm/llvm-project/commit/64f002c6d36d674a924c6116ec0b2d731cc3981c
DIFF:
https://github.com/llvm/llvm-project/commit/64f002c6d36d674a924c6116ec0b2d731cc3981c.diff
LOG: Follow-up fixes for aee49255074fd4ef38d97e6e70cbfbf2f9fd0fa7
Added:
Modified:
lldb/test/Shell/SymbolFile/PDB/typedefs.test
lldb/test/Shell/SymbolFile/PDB/udt-layout.test
Removed:
diff --git a/lldb/test/Shell/SymbolFile/PDB/typedefs.test
b/lldb/test/Shell/SymbolFile/PDB/typedefs.test
index 5f70d87823049..a0106dd381c81 100644
--- a/lldb/test/Shell/SymbolFile/PDB/typedefs.test
+++ b/lldb/test/Shell/SymbolFile/PDB/typedefs.test
@@ -16,7 +16,7 @@ CHECK: SymbolFile pdb ([[MOD]])
CHECK-DAG: name = "char32_t", size = 4, compiler_type = {{.*}} char32_t
CHECK-DAG: name = "char16_t", size = 2, compiler_type = {{.*}} char16_t
CHECK-DAG: Type{{.*}} , name = "unsigned long", size = 4, compiler_type =
{{.*}} unsigned long
-CHECK-DAG: Type{{.*}} , size = 40, compiler_type = {{.*}} unsigned long [10]
+CHECK-DAG: Type{{.*}} , size = 40, compiler_type = {{.*}} unsigned long[10]
CHECK-DAG: Type{{.*}} , name = "ULongArrayTypedef", compiler_type = {{.*}}
typedef ULongArrayTypedef
; Note: compiler_type of `long double` is represented by the one for `double`
diff --git a/lldb/test/Shell/SymbolFile/PDB/udt-layout.test
b/lldb/test/Shell/SymbolFile/PDB/udt-layout.test
index f114c200d0219..84414cbf8440d 100644
--- a/lldb/test/Shell/SymbolFile/PDB/udt-layout.test
+++ b/lldb/test/Shell/SymbolFile/PDB/udt-layout.test
@@ -3,7 +3,7 @@ RUN: %build --compiler=clang-cl --output=%t.exe
%S/Inputs/UdtLayoutTest.cpp
RUN: %lldb -b -s %S/Inputs/UdtLayoutTest.script -- %t.exe | FileCheck %s
CHECK:(int) C::abc = 123
-CHECK:(List [16]) ls = {
+CHECK:(List[16]) ls = {
CHECK: [15] = {
CHECK:Prev = nullptr
CHECK:Next = nullptr
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] d723ad5 - Enable libc++ in the build for libcxx initializerlist pretty printers
Author: David Blaikie Date: 2021-10-21T11:35:59-07:00 New Revision: d723ad5bcf7133c3f3950ebb63edf4ddfee56581 URL: https://github.com/llvm/llvm-project/commit/d723ad5bcf7133c3f3950ebb63edf4ddfee56581 DIFF: https://github.com/llvm/llvm-project/commit/d723ad5bcf7133c3f3950ebb63edf4ddfee56581.diff LOG: Enable libc++ in the build for libcxx initializerlist pretty printers Differential Revision: https://reviews.llvm.org/D112163 Added: Modified: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/Makefile lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py Removed: diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/Makefile b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/Makefile index e78030cbf7528..98af672c70fbe 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/Makefile +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/Makefile @@ -1,4 +1,6 @@ CXX_SOURCES := main.cpp CXXFLAGS_EXTRAS := -std=c++11 +USE_LIBCPP := 1 + include Makefile.rules diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py index 5fef10e6d3ede..16b64210d6592 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py @@ -14,11 +14,7 @@ class InitializerListTestCase(TestBase): mydir = TestBase.compute_mydir(__file__) -@skipIfWindows # libc++ not ported to Windows yet -@skipIf(compiler="gcc") -@expectedFailureAll( -oslist=["linux"], -bugnumber="fails on clang 3.5 and tot") +@add_test_categories(["libc++"]) def test(self): """Test that that file and class static variables display correctly.""" self.build() ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
