[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::map formatter (PR #97579)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/97579 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify libc++ std::map::iterator formatter (PR #97713)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/97713 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::map formatter (PR #97579)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97579
>From 223606fa569d4d9ced91461ec10e63ef414ad15e Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 3 Jul 2024 11:43:47 +0200
Subject: [PATCH] [lldb][DataFormatter] Simplify std::map formatter
Depends on:
* https://github.com/llvm/llvm-project/pull/97544
* https://github.com/llvm/llvm-project/pull/97549
* https://github.com/llvm/llvm-project/pull/97551
This patch tries to simplify the way in which the
`std::map` formatter goes from the root `__tree`
pointer to a specific key/value pair.
Previously we would synthesize a structure that
mimicked what `__iter_pointer` looked like
in memory, then called `GetChildCompilerTypeAtIndex`
on it to find the byte offset into that structure
at which the pair was located at, and finally
use that offset through a call to `GetSyntheticChildAtOffset`
to retrieve that pair. Not only was this logic hard to follow,
and encoded the libc++ layout in non-obvious ways, it was also
fragile to alignment miscalculations
(https://github.com/llvm/llvm-project/pull/97443); this would
break after the new layout of std::map landed as part of
inhttps://github.com/llvm/llvm-project/issues/93069.
Instead, this patch simply casts the `__iter_pointer` to
the `__node_pointer` and uses a straightforward
`GetChildMemberWithName("__value_")` to get to the key/value
we care about. This allows us to get rid of some support
infrastructure/class state.
Ideally we would fix the underlying alignment issue, but
this unblocks the libc++ refactor in the interim,
while also benefitting the formatter in terms of readability (in my opinion).
---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 172 +-
1 file changed, 48 insertions(+), 124 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index c2bb3555908bee..093e182df11889 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -18,11 +18,31 @@
#include "lldb/Utility/Status.h"
#include "lldb/Utility/Stream.h"
#include "lldb/lldb-forward.h"
+#include
+#include
+#include
using namespace lldb;
using namespace lldb_private;
using namespace lldb_private::formatters;
+// The flattened layout of the std::__tree_iterator::__ptr_ looks
+// as follows:
+//
+// The following shows the contiguous block of memory:
+//
+//+-+ class __tree_end_node
+// __ptr_ | pointer __left_;|
+//+-+ class __tree_node_base
+//| pointer __right_; |
+//| __parent_pointer __parent_; |
+//| bool __is_black_; |
+//+-+ class __tree_node
+//| __node_value_type __value_; | <<< our key/value pair
+//+-+
+//
+// where __ptr_ has type __iter_pointer.
+
class MapEntry {
public:
MapEntry() = default;
@@ -181,10 +201,6 @@ class LibcxxStdMapSyntheticFrontEnd : public
SyntheticChildrenFrontEnd {
size_t GetIndexOfChildWithName(ConstString name) override;
private:
- bool GetDataType();
-
- void GetValueOffset(const lldb::ValueObjectSP );
-
/// Returns the ValueObject for the __tree_node type that
/// holds the key/value pair of the node at index \ref idx.
///
@@ -203,8 +219,7 @@ class LibcxxStdMapSyntheticFrontEnd : public
SyntheticChildrenFrontEnd {
ValueObject *m_tree = nullptr;
ValueObject *m_root_node = nullptr;
- CompilerType m_element_type;
- uint32_t m_skip_size = UINT32_MAX;
+ CompilerType m_node_ptr_type;
size_t m_count = UINT32_MAX;
std::map m_iterators;
};
@@ -234,7 +249,7 @@ class LibCxxMapIteratorSyntheticFrontEnd : public
SyntheticChildrenFrontEnd {
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::
LibcxxStdMapSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp), m_element_type(), m_iterators() {
+: SyntheticChildrenFrontEnd(*valobj_sp) {
if (valobj_sp)
Update();
}
@@ -260,146 +275,52 @@ llvm::Expected lldb_private::formatters::
return m_count;
}
-bool lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetDataType() {
- if (m_element_type.IsValid())
-return true;
- m_element_type.Clear();
- ValueObjectSP deref;
- Status error;
- deref = m_root_node->Dereference(error);
- if (!deref || error.Fail())
-return false;
- deref = deref->GetChildMemberWithName("__value_");
- if (deref) {
-m_element_type = deref->GetCompilerType();
-return true;
- }
- deref = m_backend.GetChildAtNamePath({"__tree_", "__pair3_"});
- if (!deref)
-return false;
- m_element_type = deref->GetCompilerType()
- .GetTypeTemplateArgument(1)
- .GetTypeTemplateArgument(1);
- if (m_element_type) {
-std::string name;
-uint64_t
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::unordered_map::iterator formatter (PR #97754)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/97754 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::unordered_map::iterator formatter (PR #97754)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97754
>From b10f76bd6d02106e80315a70a7b72461cb6f2a99 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 4 Jul 2024 13:35:21 +0200
Subject: [PATCH 1/3] [lldb][DataFormatter] Move std::unordered_map::iterator
formatter into LibCxxUnorderedMap.cpp
---
.../Plugins/Language/CPlusPlus/LibCxx.cpp | 149 -
.../Plugins/Language/CPlusPlus/LibCxx.h | 54 +
.../Language/CPlusPlus/LibCxxUnorderedMap.cpp | 196 ++
3 files changed, 200 insertions(+), 199 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 05cfa0568c25d..feaa51a96843a 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -202,155 +202,6 @@ bool
lldb_private::formatters::LibcxxUniquePointerSummaryProvider(
return true;
}
-lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
-LibCxxUnorderedMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp) {
- if (valobj_sp)
-Update();
-}
-
-lldb::ChildCacheState lldb_private::formatters::
-LibCxxUnorderedMapIteratorSyntheticFrontEnd::Update() {
- m_pair_sp.reset();
- m_iter_ptr = nullptr;
-
- ValueObjectSP valobj_sp = m_backend.GetSP();
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- TargetSP target_sp(valobj_sp->GetTargetSP());
-
- if (!target_sp)
-return lldb::ChildCacheState::eRefetch;
-
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- auto exprPathOptions = ValueObject::GetValueForExpressionPathOptions()
- .DontCheckDotVsArrowSyntax()
- .SetSyntheticChildrenTraversal(
-
ValueObject::GetValueForExpressionPathOptions::
- SyntheticChildrenTraversal::None);
-
- // This must be a ValueObject* because it is a child of the ValueObject we
- // are producing children for it if were a ValueObjectSP, we would end up
- // with a loop (iterator -> synthetic -> child -> parent == iterator) and
- // that would in turn leak memory by never allowing the ValueObjects to die
- // and free their memory.
- m_iter_ptr =
- valobj_sp
- ->GetValueForExpressionPath(".__i_.__node_", nullptr, nullptr,
- exprPathOptions, nullptr)
- .get();
-
- if (m_iter_ptr) {
-auto iter_child(valobj_sp->GetChildMemberWithName("__i_"));
-if (!iter_child) {
- m_iter_ptr = nullptr;
- return lldb::ChildCacheState::eRefetch;
-}
-
-CompilerType node_type(iter_child->GetCompilerType()
- .GetTypeTemplateArgument(0)
- .GetPointeeType());
-
-CompilerType pair_type(node_type.GetTypeTemplateArgument(0));
-
-std::string name;
-uint64_t bit_offset_ptr;
-uint32_t bitfield_bit_size_ptr;
-bool is_bitfield_ptr;
-
-pair_type = pair_type.GetFieldAtIndex(
-0, name, _offset_ptr, _bit_size_ptr, _bitfield_ptr);
-if (!pair_type) {
- m_iter_ptr = nullptr;
- return lldb::ChildCacheState::eRefetch;
-}
-
-uint64_t addr = m_iter_ptr->GetValueAsUnsigned(LLDB_INVALID_ADDRESS);
-m_iter_ptr = nullptr;
-
-if (addr == 0 || addr == LLDB_INVALID_ADDRESS)
- return lldb::ChildCacheState::eRefetch;
-
-auto ts = pair_type.GetTypeSystem();
-auto ast_ctx = ts.dyn_cast_or_null();
-if (!ast_ctx)
- return lldb::ChildCacheState::eRefetch;
-
-// Mimick layout of std::__hash_iterator::__node_ and read it in
-// from process memory.
-//
-// The following shows the contiguous block of memory:
-//
-// +-+ class __hash_node_base
-// __node_ | __next_pointer __next_; |
-// +-+ class __hash_node
-// | size_t __hash_; |
-// | __node_value_type __value_; | <<< our key/value pair
-// +-+
-//
-CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-llvm::StringRef(),
-{{"__next_",
- ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"__hash_", ast_ctx->GetBasicType(lldb::eBasicTypeUnsignedLongLong)},
- {"__value_", pair_type}});
-std::optional size = tree_node_type.GetByteSize(nullptr);
-if (!size)
- return lldb::ChildCacheState::eRefetch;
-WritableDataBufferSP buffer_sp(new DataBufferHeap(*size, 0));
-ProcessSP process_sp(target_sp->GetProcessSP());
-Status error;
-process_sp->ReadMemory(addr, buffer_sp->GetBytes(),
- buffer_sp->GetByteSize(), error);
-if (error.Fail())
- return lldb::ChildCacheState::eRefetch;
-
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify libc++ std::map::iterator formatter (PR #97713)
@@ -456,3 +477,97 @@
lldb_private::formatters::LibcxxStdMapSyntheticFrontEndCreator(
CXXSyntheticChildren *, lldb::ValueObjectSP valobj_sp) {
return (valobj_sp ? new LibcxxStdMapSyntheticFrontEnd(valobj_sp) : nullptr);
}
+
+lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::
+LibCxxMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
+: SyntheticChildrenFrontEnd(*valobj_sp) {
+ if (valobj_sp)
+Update();
+}
+
+lldb::ChildCacheState
+lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::Update() {
+ m_pair_sp.reset();
+
+ ValueObjectSP valobj_sp = m_backend.GetSP();
+ if (!valobj_sp)
+return lldb::ChildCacheState::eRefetch;
+
+ TargetSP target_sp(valobj_sp->GetTargetSP());
+ if (!target_sp)
+return lldb::ChildCacheState::eRefetch;
+
+ auto tree_iter_sp = valobj_sp->GetChildMemberWithName("__i_");
+ if (!tree_iter_sp)
+return lldb::ChildCacheState::eRefetch;
+
+ auto node_pointer_type =
+ tree_iter_sp->GetCompilerType().GetDirectNestedTypeWithName(
+ "__node_pointer");
+ if (!node_pointer_type.IsValid())
+return lldb::ChildCacheState::eRefetch;
+
+ auto iter_pointer_sp = tree_iter_sp->GetChildMemberWithName("__ptr_");
+ if (!iter_pointer_sp)
+return lldb::ChildCacheState::eRefetch;
+
+ auto node_pointer_sp = iter_pointer_sp->Cast(node_pointer_type);
+ if (!node_pointer_sp)
+return lldb::ChildCacheState::eRefetch;
+
+ Status err;
+ node_pointer_sp = node_pointer_sp->Dereference(err);
+ if (!node_pointer_sp || err.Fail())
+return lldb::ChildCacheState::eRefetch;
+
+ auto key_value_sp = node_pointer_sp->GetChildMemberWithName("__value_");
+ if (!key_value_sp)
+return lldb::ChildCacheState::eRefetch;
+
+ key_value_sp = key_value_sp->Clone(ConstString("name"));
+ if (key_value_sp->GetNumChildrenIgnoringErrors() == 1) {
+auto child0_sp = key_value_sp->GetChildAtIndex(0);
+if (child0_sp &&
+(child0_sp->GetName() == "__cc_" || child0_sp->GetName() == "__cc"))
+ key_value_sp = child0_sp->Clone(ConstString("pair"));
+ }
+
+ m_pair_sp = key_value_sp;
+
+ return lldb::ChildCacheState::eRefetch;
+}
+
+llvm::Expected lldb_private::formatters::
+LibCxxMapIteratorSyntheticFrontEnd::CalculateNumChildren() {
+ return 2;
+}
+
+lldb::ValueObjectSP
+lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::GetChildAtIndex(
+uint32_t idx) {
+ if (!m_pair_sp)
+return nullptr;
+
+ return m_pair_sp->GetChildAtIndex(idx);
+}
+
+bool lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::
+MightHaveChildren() {
+ return true;
+}
+
+size_t lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::
+GetIndexOfChildWithName(ConstString name) {
+ if (name == "first")
Michael137 wrote:
yup good idea!
https://github.com/llvm/llvm-project/pull/97713
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify libc++ std::map::iterator formatter (PR #97713)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97713
>From f6b3e6055a9e2263f61e3f70d7a97ddbb7db5ab0 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 4 Jul 2024 09:30:19 +0200
Subject: [PATCH 1/7] [lldb][DataFormatter][NFC] Move std::map iterator
formatter into LibCxxMap.cpp
The two formatters follow very similar techniques to retrieve data out of the
map. We're changing this for `std::map` in
https://github.com/llvm/llvm-project/pull/97579 and plan to change it in the
same way for the iterator formatter. Having them in the same place will allow
us to re-use some of the logic (and we won't have to repeat some of the
clarification comments).
---
.../Plugins/Language/CPlusPlus/LibCxx.cpp | 188 --
.../Plugins/Language/CPlusPlus/LibCxx.h | 29 +--
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 187 +
3 files changed, 191 insertions(+), 213 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index ad467c3966e609..05cfa0568c25d4 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -202,194 +202,6 @@ bool
lldb_private::formatters::LibcxxUniquePointerSummaryProvider(
return true;
}
-/*
- (lldb) fr var ibeg --raw --ptr-depth 1
- (std::__1::__map_iterator,
- std::__1::allocator > >, std::__1::__tree_node,
- std::__1::allocator > >, void *> *, long> >) ibeg = {
- __i_ = {
- __ptr_ = 0x000100103870 {
- std::__1::__tree_node_base = {
- std::__1::__tree_end_node *> = {
- __left_ = 0x
- }
- __right_ = 0x
- __parent_ = 0x0001001038b0
- __is_black_ = true
- }
- __value_ = {
- first = 0
- second = { std::string }
- */
-
-lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::
-LibCxxMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp), m_pair_ptr(), m_pair_sp() {
- if (valobj_sp)
-Update();
-}
-
-lldb::ChildCacheState
-lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::Update() {
- m_pair_sp.reset();
- m_pair_ptr = nullptr;
-
- ValueObjectSP valobj_sp = m_backend.GetSP();
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- TargetSP target_sp(valobj_sp->GetTargetSP());
-
- if (!target_sp)
-return lldb::ChildCacheState::eRefetch;
-
- // this must be a ValueObject* because it is a child of the ValueObject we
- // are producing children for it if were a ValueObjectSP, we would end up
- // with a loop (iterator -> synthetic -> child -> parent == iterator) and
- // that would in turn leak memory by never allowing the ValueObjects to die
- // and free their memory
- m_pair_ptr = valobj_sp
- ->GetValueForExpressionPath(
- ".__i_.__ptr_->__value_", nullptr, nullptr,
- ValueObject::GetValueForExpressionPathOptions()
- .DontCheckDotVsArrowSyntax()
- .SetSyntheticChildrenTraversal(
- ValueObject::GetValueForExpressionPathOptions::
- SyntheticChildrenTraversal::None),
- nullptr)
- .get();
-
- if (!m_pair_ptr) {
-m_pair_ptr = valobj_sp
- ->GetValueForExpressionPath(
- ".__i_.__ptr_", nullptr, nullptr,
- ValueObject::GetValueForExpressionPathOptions()
- .DontCheckDotVsArrowSyntax()
- .SetSyntheticChildrenTraversal(
-
ValueObject::GetValueForExpressionPathOptions::
- SyntheticChildrenTraversal::None),
- nullptr)
- .get();
-if (m_pair_ptr) {
- auto __i_(valobj_sp->GetChildMemberWithName("__i_"));
- if (!__i_) {
-m_pair_ptr = nullptr;
-return lldb::ChildCacheState::eRefetch;
- }
- CompilerType pair_type(
- __i_->GetCompilerType().GetTypeTemplateArgument(0));
- std::string name;
- uint64_t bit_offset_ptr;
- uint32_t bitfield_bit_size_ptr;
- bool is_bitfield_ptr;
- pair_type = pair_type.GetFieldAtIndex(
- 0, name, _offset_ptr, _bit_size_ptr, _bitfield_ptr);
- if (!pair_type) {
-m_pair_ptr = nullptr;
-return lldb::ChildCacheState::eRefetch;
- }
-
- auto addr(m_pair_ptr->GetValueAsUnsigned(LLDB_INVALID_ADDRESS));
- m_pair_ptr = nullptr;
- if (addr && addr != LLDB_INVALID_ADDRESS) {
-auto ts = pair_type.GetTypeSystem();
-auto ast_ctx = ts.dyn_cast_or_null();
-if (!ast_ctx)
- return lldb::ChildCacheState::eRefetch;
-
-// Mimick layout of std::__tree_iterator::__ptr_ and read it in
-// from process memory.
-//
-// The
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify libc++ std::map::iterator formatter (PR #97713)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97713
>From f6b3e6055a9e2263f61e3f70d7a97ddbb7db5ab0 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 4 Jul 2024 09:30:19 +0200
Subject: [PATCH 1/6] [lldb][DataFormatter][NFC] Move std::map iterator
formatter into LibCxxMap.cpp
The two formatters follow very similar techniques to retrieve data out of the
map. We're changing this for `std::map` in
https://github.com/llvm/llvm-project/pull/97579 and plan to change it in the
same way for the iterator formatter. Having them in the same place will allow
us to re-use some of the logic (and we won't have to repeat some of the
clarification comments).
---
.../Plugins/Language/CPlusPlus/LibCxx.cpp | 188 --
.../Plugins/Language/CPlusPlus/LibCxx.h | 29 +--
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 187 +
3 files changed, 191 insertions(+), 213 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index ad467c3966e60..05cfa0568c25d 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -202,194 +202,6 @@ bool
lldb_private::formatters::LibcxxUniquePointerSummaryProvider(
return true;
}
-/*
- (lldb) fr var ibeg --raw --ptr-depth 1
- (std::__1::__map_iterator,
- std::__1::allocator > >, std::__1::__tree_node,
- std::__1::allocator > >, void *> *, long> >) ibeg = {
- __i_ = {
- __ptr_ = 0x000100103870 {
- std::__1::__tree_node_base = {
- std::__1::__tree_end_node *> = {
- __left_ = 0x
- }
- __right_ = 0x
- __parent_ = 0x0001001038b0
- __is_black_ = true
- }
- __value_ = {
- first = 0
- second = { std::string }
- */
-
-lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::
-LibCxxMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp), m_pair_ptr(), m_pair_sp() {
- if (valobj_sp)
-Update();
-}
-
-lldb::ChildCacheState
-lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::Update() {
- m_pair_sp.reset();
- m_pair_ptr = nullptr;
-
- ValueObjectSP valobj_sp = m_backend.GetSP();
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- TargetSP target_sp(valobj_sp->GetTargetSP());
-
- if (!target_sp)
-return lldb::ChildCacheState::eRefetch;
-
- // this must be a ValueObject* because it is a child of the ValueObject we
- // are producing children for it if were a ValueObjectSP, we would end up
- // with a loop (iterator -> synthetic -> child -> parent == iterator) and
- // that would in turn leak memory by never allowing the ValueObjects to die
- // and free their memory
- m_pair_ptr = valobj_sp
- ->GetValueForExpressionPath(
- ".__i_.__ptr_->__value_", nullptr, nullptr,
- ValueObject::GetValueForExpressionPathOptions()
- .DontCheckDotVsArrowSyntax()
- .SetSyntheticChildrenTraversal(
- ValueObject::GetValueForExpressionPathOptions::
- SyntheticChildrenTraversal::None),
- nullptr)
- .get();
-
- if (!m_pair_ptr) {
-m_pair_ptr = valobj_sp
- ->GetValueForExpressionPath(
- ".__i_.__ptr_", nullptr, nullptr,
- ValueObject::GetValueForExpressionPathOptions()
- .DontCheckDotVsArrowSyntax()
- .SetSyntheticChildrenTraversal(
-
ValueObject::GetValueForExpressionPathOptions::
- SyntheticChildrenTraversal::None),
- nullptr)
- .get();
-if (m_pair_ptr) {
- auto __i_(valobj_sp->GetChildMemberWithName("__i_"));
- if (!__i_) {
-m_pair_ptr = nullptr;
-return lldb::ChildCacheState::eRefetch;
- }
- CompilerType pair_type(
- __i_->GetCompilerType().GetTypeTemplateArgument(0));
- std::string name;
- uint64_t bit_offset_ptr;
- uint32_t bitfield_bit_size_ptr;
- bool is_bitfield_ptr;
- pair_type = pair_type.GetFieldAtIndex(
- 0, name, _offset_ptr, _bit_size_ptr, _bitfield_ptr);
- if (!pair_type) {
-m_pair_ptr = nullptr;
-return lldb::ChildCacheState::eRefetch;
- }
-
- auto addr(m_pair_ptr->GetValueAsUnsigned(LLDB_INVALID_ADDRESS));
- m_pair_ptr = nullptr;
- if (addr && addr != LLDB_INVALID_ADDRESS) {
-auto ts = pair_type.GetTypeSystem();
-auto ast_ctx = ts.dyn_cast_or_null();
-if (!ast_ctx)
- return lldb::ChildCacheState::eRefetch;
-
-// Mimick layout of std::__tree_iterator::__ptr_ and read it in
-// from process memory.
-//
-// The
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::unordered_map::iterator formatter (PR #97754)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97754
>From b10f76bd6d02106e80315a70a7b72461cb6f2a99 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 4 Jul 2024 13:35:21 +0200
Subject: [PATCH 1/2] [lldb][DataFormatter] Move std::unordered_map::iterator
formatter into LibCxxUnorderedMap.cpp
---
.../Plugins/Language/CPlusPlus/LibCxx.cpp | 149 -
.../Plugins/Language/CPlusPlus/LibCxx.h | 54 +
.../Language/CPlusPlus/LibCxxUnorderedMap.cpp | 196 ++
3 files changed, 200 insertions(+), 199 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 05cfa0568c25d4..feaa51a96843ab 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -202,155 +202,6 @@ bool
lldb_private::formatters::LibcxxUniquePointerSummaryProvider(
return true;
}
-lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
-LibCxxUnorderedMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp) {
- if (valobj_sp)
-Update();
-}
-
-lldb::ChildCacheState lldb_private::formatters::
-LibCxxUnorderedMapIteratorSyntheticFrontEnd::Update() {
- m_pair_sp.reset();
- m_iter_ptr = nullptr;
-
- ValueObjectSP valobj_sp = m_backend.GetSP();
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- TargetSP target_sp(valobj_sp->GetTargetSP());
-
- if (!target_sp)
-return lldb::ChildCacheState::eRefetch;
-
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- auto exprPathOptions = ValueObject::GetValueForExpressionPathOptions()
- .DontCheckDotVsArrowSyntax()
- .SetSyntheticChildrenTraversal(
-
ValueObject::GetValueForExpressionPathOptions::
- SyntheticChildrenTraversal::None);
-
- // This must be a ValueObject* because it is a child of the ValueObject we
- // are producing children for it if were a ValueObjectSP, we would end up
- // with a loop (iterator -> synthetic -> child -> parent == iterator) and
- // that would in turn leak memory by never allowing the ValueObjects to die
- // and free their memory.
- m_iter_ptr =
- valobj_sp
- ->GetValueForExpressionPath(".__i_.__node_", nullptr, nullptr,
- exprPathOptions, nullptr)
- .get();
-
- if (m_iter_ptr) {
-auto iter_child(valobj_sp->GetChildMemberWithName("__i_"));
-if (!iter_child) {
- m_iter_ptr = nullptr;
- return lldb::ChildCacheState::eRefetch;
-}
-
-CompilerType node_type(iter_child->GetCompilerType()
- .GetTypeTemplateArgument(0)
- .GetPointeeType());
-
-CompilerType pair_type(node_type.GetTypeTemplateArgument(0));
-
-std::string name;
-uint64_t bit_offset_ptr;
-uint32_t bitfield_bit_size_ptr;
-bool is_bitfield_ptr;
-
-pair_type = pair_type.GetFieldAtIndex(
-0, name, _offset_ptr, _bit_size_ptr, _bitfield_ptr);
-if (!pair_type) {
- m_iter_ptr = nullptr;
- return lldb::ChildCacheState::eRefetch;
-}
-
-uint64_t addr = m_iter_ptr->GetValueAsUnsigned(LLDB_INVALID_ADDRESS);
-m_iter_ptr = nullptr;
-
-if (addr == 0 || addr == LLDB_INVALID_ADDRESS)
- return lldb::ChildCacheState::eRefetch;
-
-auto ts = pair_type.GetTypeSystem();
-auto ast_ctx = ts.dyn_cast_or_null();
-if (!ast_ctx)
- return lldb::ChildCacheState::eRefetch;
-
-// Mimick layout of std::__hash_iterator::__node_ and read it in
-// from process memory.
-//
-// The following shows the contiguous block of memory:
-//
-// +-+ class __hash_node_base
-// __node_ | __next_pointer __next_; |
-// +-+ class __hash_node
-// | size_t __hash_; |
-// | __node_value_type __value_; | <<< our key/value pair
-// +-+
-//
-CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-llvm::StringRef(),
-{{"__next_",
- ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"__hash_", ast_ctx->GetBasicType(lldb::eBasicTypeUnsignedLongLong)},
- {"__value_", pair_type}});
-std::optional size = tree_node_type.GetByteSize(nullptr);
-if (!size)
- return lldb::ChildCacheState::eRefetch;
-WritableDataBufferSP buffer_sp(new DataBufferHeap(*size, 0));
-ProcessSP process_sp(target_sp->GetProcessSP());
-Status error;
-process_sp->ReadMemory(addr, buffer_sp->GetBytes(),
- buffer_sp->GetByteSize(), error);
-if (error.Fail())
- return lldb::ChildCacheState::eRefetch;
-
[Lldb-commits] [lldb] [lldb][DataFormatter] Move std::unordered_map::iterator formatter into LibCxxUnorderedMap.cpp (PR #97752)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/97752 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Remove support for old std::map layout (PR #97549)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/97549 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify libc++ std::map::iterator formatter (PR #97713)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97713
>From f6b3e6055a9e2263f61e3f70d7a97ddbb7db5ab0 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 4 Jul 2024 09:30:19 +0200
Subject: [PATCH 1/2] [lldb][DataFormatter][NFC] Move std::map iterator
formatter into LibCxxMap.cpp
The two formatters follow very similar techniques to retrieve data out of the
map. We're changing this for `std::map` in
https://github.com/llvm/llvm-project/pull/97579 and plan to change it in the
same way for the iterator formatter. Having them in the same place will allow
us to re-use some of the logic (and we won't have to repeat some of the
clarification comments).
---
.../Plugins/Language/CPlusPlus/LibCxx.cpp | 188 --
.../Plugins/Language/CPlusPlus/LibCxx.h | 29 +--
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 187 +
3 files changed, 191 insertions(+), 213 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index ad467c3966e60..05cfa0568c25d 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -202,194 +202,6 @@ bool
lldb_private::formatters::LibcxxUniquePointerSummaryProvider(
return true;
}
-/*
- (lldb) fr var ibeg --raw --ptr-depth 1
- (std::__1::__map_iterator,
- std::__1::allocator > >, std::__1::__tree_node,
- std::__1::allocator > >, void *> *, long> >) ibeg = {
- __i_ = {
- __ptr_ = 0x000100103870 {
- std::__1::__tree_node_base = {
- std::__1::__tree_end_node *> = {
- __left_ = 0x
- }
- __right_ = 0x
- __parent_ = 0x0001001038b0
- __is_black_ = true
- }
- __value_ = {
- first = 0
- second = { std::string }
- */
-
-lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::
-LibCxxMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp), m_pair_ptr(), m_pair_sp() {
- if (valobj_sp)
-Update();
-}
-
-lldb::ChildCacheState
-lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::Update() {
- m_pair_sp.reset();
- m_pair_ptr = nullptr;
-
- ValueObjectSP valobj_sp = m_backend.GetSP();
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- TargetSP target_sp(valobj_sp->GetTargetSP());
-
- if (!target_sp)
-return lldb::ChildCacheState::eRefetch;
-
- // this must be a ValueObject* because it is a child of the ValueObject we
- // are producing children for it if were a ValueObjectSP, we would end up
- // with a loop (iterator -> synthetic -> child -> parent == iterator) and
- // that would in turn leak memory by never allowing the ValueObjects to die
- // and free their memory
- m_pair_ptr = valobj_sp
- ->GetValueForExpressionPath(
- ".__i_.__ptr_->__value_", nullptr, nullptr,
- ValueObject::GetValueForExpressionPathOptions()
- .DontCheckDotVsArrowSyntax()
- .SetSyntheticChildrenTraversal(
- ValueObject::GetValueForExpressionPathOptions::
- SyntheticChildrenTraversal::None),
- nullptr)
- .get();
-
- if (!m_pair_ptr) {
-m_pair_ptr = valobj_sp
- ->GetValueForExpressionPath(
- ".__i_.__ptr_", nullptr, nullptr,
- ValueObject::GetValueForExpressionPathOptions()
- .DontCheckDotVsArrowSyntax()
- .SetSyntheticChildrenTraversal(
-
ValueObject::GetValueForExpressionPathOptions::
- SyntheticChildrenTraversal::None),
- nullptr)
- .get();
-if (m_pair_ptr) {
- auto __i_(valobj_sp->GetChildMemberWithName("__i_"));
- if (!__i_) {
-m_pair_ptr = nullptr;
-return lldb::ChildCacheState::eRefetch;
- }
- CompilerType pair_type(
- __i_->GetCompilerType().GetTypeTemplateArgument(0));
- std::string name;
- uint64_t bit_offset_ptr;
- uint32_t bitfield_bit_size_ptr;
- bool is_bitfield_ptr;
- pair_type = pair_type.GetFieldAtIndex(
- 0, name, _offset_ptr, _bit_size_ptr, _bitfield_ptr);
- if (!pair_type) {
-m_pair_ptr = nullptr;
-return lldb::ChildCacheState::eRefetch;
- }
-
- auto addr(m_pair_ptr->GetValueAsUnsigned(LLDB_INVALID_ADDRESS));
- m_pair_ptr = nullptr;
- if (addr && addr != LLDB_INVALID_ADDRESS) {
-auto ts = pair_type.GetTypeSystem();
-auto ast_ctx = ts.dyn_cast_or_null();
-if (!ast_ctx)
- return lldb::ChildCacheState::eRefetch;
-
-// Mimick layout of std::__tree_iterator::__ptr_ and read it in
-// from process memory.
-//
-// The
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::map formatter (PR #97579)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97579
>From 223606fa569d4d9ced91461ec10e63ef414ad15e Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 3 Jul 2024 11:43:47 +0200
Subject: [PATCH] [lldb][DataFormatter] Simplify std::map formatter
Depends on:
* https://github.com/llvm/llvm-project/pull/97544
* https://github.com/llvm/llvm-project/pull/97549
* https://github.com/llvm/llvm-project/pull/97551
This patch tries to simplify the way in which the
`std::map` formatter goes from the root `__tree`
pointer to a specific key/value pair.
Previously we would synthesize a structure that
mimicked what `__iter_pointer` looked like
in memory, then called `GetChildCompilerTypeAtIndex`
on it to find the byte offset into that structure
at which the pair was located at, and finally
use that offset through a call to `GetSyntheticChildAtOffset`
to retrieve that pair. Not only was this logic hard to follow,
and encoded the libc++ layout in non-obvious ways, it was also
fragile to alignment miscalculations
(https://github.com/llvm/llvm-project/pull/97443); this would
break after the new layout of std::map landed as part of
inhttps://github.com/llvm/llvm-project/issues/93069.
Instead, this patch simply casts the `__iter_pointer` to
the `__node_pointer` and uses a straightforward
`GetChildMemberWithName("__value_")` to get to the key/value
we care about. This allows us to get rid of some support
infrastructure/class state.
Ideally we would fix the underlying alignment issue, but
this unblocks the libc++ refactor in the interim,
while also benefitting the formatter in terms of readability (in my opinion).
---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 172 +-
1 file changed, 48 insertions(+), 124 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index c2bb3555908be..093e182df1188 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -18,11 +18,31 @@
#include "lldb/Utility/Status.h"
#include "lldb/Utility/Stream.h"
#include "lldb/lldb-forward.h"
+#include
+#include
+#include
using namespace lldb;
using namespace lldb_private;
using namespace lldb_private::formatters;
+// The flattened layout of the std::__tree_iterator::__ptr_ looks
+// as follows:
+//
+// The following shows the contiguous block of memory:
+//
+//+-+ class __tree_end_node
+// __ptr_ | pointer __left_;|
+//+-+ class __tree_node_base
+//| pointer __right_; |
+//| __parent_pointer __parent_; |
+//| bool __is_black_; |
+//+-+ class __tree_node
+//| __node_value_type __value_; | <<< our key/value pair
+//+-+
+//
+// where __ptr_ has type __iter_pointer.
+
class MapEntry {
public:
MapEntry() = default;
@@ -181,10 +201,6 @@ class LibcxxStdMapSyntheticFrontEnd : public
SyntheticChildrenFrontEnd {
size_t GetIndexOfChildWithName(ConstString name) override;
private:
- bool GetDataType();
-
- void GetValueOffset(const lldb::ValueObjectSP );
-
/// Returns the ValueObject for the __tree_node type that
/// holds the key/value pair of the node at index \ref idx.
///
@@ -203,8 +219,7 @@ class LibcxxStdMapSyntheticFrontEnd : public
SyntheticChildrenFrontEnd {
ValueObject *m_tree = nullptr;
ValueObject *m_root_node = nullptr;
- CompilerType m_element_type;
- uint32_t m_skip_size = UINT32_MAX;
+ CompilerType m_node_ptr_type;
size_t m_count = UINT32_MAX;
std::map m_iterators;
};
@@ -234,7 +249,7 @@ class LibCxxMapIteratorSyntheticFrontEnd : public
SyntheticChildrenFrontEnd {
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::
LibcxxStdMapSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp), m_element_type(), m_iterators() {
+: SyntheticChildrenFrontEnd(*valobj_sp) {
if (valobj_sp)
Update();
}
@@ -260,146 +275,52 @@ llvm::Expected lldb_private::formatters::
return m_count;
}
-bool lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetDataType() {
- if (m_element_type.IsValid())
-return true;
- m_element_type.Clear();
- ValueObjectSP deref;
- Status error;
- deref = m_root_node->Dereference(error);
- if (!deref || error.Fail())
-return false;
- deref = deref->GetChildMemberWithName("__value_");
- if (deref) {
-m_element_type = deref->GetCompilerType();
-return true;
- }
- deref = m_backend.GetChildAtNamePath({"__tree_", "__pair3_"});
- if (!deref)
-return false;
- m_element_type = deref->GetCompilerType()
- .GetTypeTemplateArgument(1)
- .GetTypeTemplateArgument(1);
- if (m_element_type) {
-std::string name;
-uint64_t bit_offset_ptr;
[Lldb-commits] [lldb] [lldb][DataFormatter] Clean up LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair (PR #97551)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/97551 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::map formatter (PR #97579)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97579
>From 9dabd3a399f37789b6a9bc7578b76e738c344f1d Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 3 Jul 2024 10:55:40 +0200
Subject: [PATCH 1/2] [lldb][DataFormatter][NFC] Factor out MapIterator logic
into separate helper
This patch factors all the logic for advancing the `MapIterator`
out of `GetChildAtIndex`. This, in my opinion, helps readability,
and will be useful for upcoming cleanups in this area.
While here, some drive-by changes:
* added a couple of clarification comments
* fixed a variable name typo
* turned the `return lldb::ValueObjectSP()` into `return nullptr`
* added an assertion to make sure we keep the iterator cache in a valid
state
---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 115 +++---
1 file changed, 72 insertions(+), 43 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 44fe294ced722..370dfa35e7703 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -17,6 +17,7 @@
#include "lldb/Utility/Endian.h"
#include "lldb/Utility/Status.h"
#include "lldb/Utility/Stream.h"
+#include "lldb/lldb-forward.h"
using namespace lldb;
using namespace lldb_private;
@@ -184,6 +185,22 @@ class LibcxxStdMapSyntheticFrontEnd : public
SyntheticChildrenFrontEnd {
void GetValueOffset(const lldb::ValueObjectSP );
+ /// Returns the ValueObject for the __tree_node type that
+ /// holds the key/value pair of the node at index \ref idx.
+ ///
+ /// \param[in] idx The child index that we're looking to get
+ ///the key/value pair for.
+ ///
+ /// \param[in] max_depth The maximum search depth after which
+ /// we stop trying to find the key/value
+ /// pair for.
+ ///
+ /// \returns On success, returns the ValueObjectSP corresponding
+ /// to the __tree_node's __value_ member (which holds
+ /// the key/value pair the formatter wants to display).
+ /// On failure, will return nullptr.
+ ValueObjectSP GetKeyValuePair(size_t idx, size_t max_depth);
+
ValueObject *m_tree = nullptr;
ValueObject *m_root_node = nullptr;
CompilerType m_element_type;
@@ -299,75 +316,88 @@ void
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
}
}
-lldb::ValueObjectSP
-lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex(
-uint32_t idx) {
- static ConstString g_cc_("__cc_"), g_cc("__cc");
- static ConstString g_nc("__nc");
- uint32_t num_children = CalculateNumChildrenIgnoringErrors();
- if (idx >= num_children)
-return lldb::ValueObjectSP();
- if (m_tree == nullptr || m_root_node == nullptr)
-return lldb::ValueObjectSP();
-
- MapIterator iterator(m_root_node, num_children);
+ValueObjectSP
+lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair(
+size_t idx, size_t max_depth) {
+ MapIterator iterator(m_root_node, max_depth);
const bool need_to_skip = (idx > 0);
- size_t actual_advancde = idx;
+ size_t actual_advance = idx;
if (need_to_skip) {
+// If we have already created the iterator for the previous
+// index, we can start from there and advance by 1.
auto cached_iterator = m_iterators.find(idx - 1);
if (cached_iterator != m_iterators.end()) {
iterator = cached_iterator->second;
- actual_advancde = 1;
+ actual_advance = 1;
}
}
- ValueObjectSP iterated_sp(iterator.advance(actual_advancde));
- if (!iterated_sp) {
+ ValueObjectSP iterated_sp(iterator.advance(actual_advance));
+ if (!iterated_sp)
// this tree is garbage - stop
-m_tree =
-nullptr; // this will stop all future searches until an Update()
happens
-return iterated_sp;
- }
+return nullptr;
- if (!GetDataType()) {
-m_tree = nullptr;
-return lldb::ValueObjectSP();
- }
+ if (!GetDataType())
+return nullptr;
if (!need_to_skip) {
Status error;
iterated_sp = iterated_sp->Dereference(error);
-if (!iterated_sp || error.Fail()) {
- m_tree = nullptr;
- return lldb::ValueObjectSP();
-}
+if (!iterated_sp || error.Fail())
+ return nullptr;
+
GetValueOffset(iterated_sp);
auto child_sp = iterated_sp->GetChildMemberWithName("__value_");
-if (child_sp)
+if (child_sp) {
+ // Old layout (pre 089a7cc5dea)
iterated_sp = child_sp;
-else
+} else {
iterated_sp = iterated_sp->GetSyntheticChildAtOffset(
m_skip_size, m_element_type, true);
-if (!iterated_sp) {
- m_tree = nullptr;
- return lldb::ValueObjectSP();
}
+
+if (!iterated_sp)
+ return nullptr;
} else {
// because of the way our debug info is made, we need to read item 0
// first so that we can cache information used to
[Lldb-commits] [lldb] [lldb][DataFormatter] Remove support for old std::map layout (PR #97549)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97549
>From 5dc61f0721746359cbaa70e5f50dd15de4a1f082 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 3 Jul 2024 12:06:49 +0200
Subject: [PATCH 1/2] [lldb][DataFormatter] Remove support for old std::map
layout
We currently supported the layout from pre-2016 (before the layout
change in 14caaddd3f08e798dcd9ac0ddfc).
We have another upcoming layout change in `__tree` and `map` (as part of
require rewriting parts of this formatter. Removing the support will
make those changes more straightforward to review/maintain.
Being backward compatible would be great but we have no tests that
actually verify that the old layout still works (and our oldest
matrix bot tests clang-15). If anyone feels strongly about keeping
this layout, we could possibly factor out that logic and keep it around.
---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 76 ---
1 file changed, 30 insertions(+), 46 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 6c2bc1a34137a..141b525da063b 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -248,11 +248,6 @@ bool
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetDataType() {
deref = m_root_node->Dereference(error);
if (!deref || error.Fail())
return false;
- deref = deref->GetChildMemberWithName("__value_");
- if (deref) {
-m_element_type = deref->GetCompilerType();
-return true;
- }
deref = m_backend.GetChildAtNamePath({"__tree_", "__pair3_"});
if (!deref)
return false;
@@ -280,40 +275,35 @@ void
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
return;
if (!node)
return;
+
CompilerType node_type(node->GetCompilerType());
- uint64_t bit_offset;
- if (node_type.GetIndexOfFieldWithName("__value_", nullptr, _offset) !=
- UINT32_MAX) {
-// Old layout (pre d05b10ab4fc65)
-m_skip_size = bit_offset / 8u;
- } else {
-auto ast_ctx =
node_type.GetTypeSystem().dyn_cast_or_null();
-if (!ast_ctx)
- return;
-CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-llvm::StringRef(),
-{{"ptr0",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"ptr1",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"ptr2",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
- {"payload", (m_element_type.GetCompleteType(), m_element_type)}});
-std::string child_name;
-uint32_t child_byte_size;
-int32_t child_byte_offset = 0;
-uint32_t child_bitfield_bit_size;
-uint32_t child_bitfield_bit_offset;
-bool child_is_base_class;
-bool child_is_deref_of_parent;
-uint64_t language_flags;
-auto child_type =
-llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
-nullptr, 4, true, true, true, child_name, child_byte_size,
-child_byte_offset, child_bitfield_bit_size,
-child_bitfield_bit_offset, child_is_base_class,
-child_is_deref_of_parent, nullptr, language_flags));
-if (child_type && child_type->IsValid())
- m_skip_size = (uint32_t)child_byte_offset;
- }
+ auto ast_ctx = node_type.GetTypeSystem().dyn_cast_or_null();
+ if (!ast_ctx)
+return;
+
+ CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
+ llvm::StringRef(),
+ {{"ptr0", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"ptr1", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"ptr2", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
+ {"payload", (m_element_type.GetCompleteType(), m_element_type)}});
+ std::string child_name;
+ uint32_t child_byte_size;
+ int32_t child_byte_offset = 0;
+ uint32_t child_bitfield_bit_size;
+ uint32_t child_bitfield_bit_offset;
+ bool child_is_base_class;
+ bool child_is_deref_of_parent;
+ uint64_t language_flags;
+ auto child_type =
+ llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
+ nullptr, 4, true, true, true, child_name, child_byte_size,
+ child_byte_offset, child_bitfield_bit_size,
child_bitfield_bit_offset,
+ child_is_base_class, child_is_deref_of_parent, nullptr,
+ language_flags));
+ if (child_type && child_type->IsValid())
+m_skip_size = (uint32_t)child_byte_offset;
}
ValueObjectSP
@@ -348,14 +338,8 @@
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair(
return nullptr;
GetValueOffset(iterated_sp);
-auto child_sp = iterated_sp->GetChildMemberWithName("__value_");
-if (child_sp) {
- // Old
[Lldb-commits] [lldb] [lldb] Make variant formatter work with libstdc++-14 (PR #97568)
Michael137 wrote: > > LGTM > > Been wondering if we should follow the same principle as we did with > > `std::string` and simulate the layouts for all the STL types, keeping a > > record of them as they change. So we know we don't break the old layouts. > > Of course it has a separate maintenance cost > > Yeah, I think the biggest hurdle is the initial work in getting a stripped > down version of the class in place. The problem is that the container types > use a lot of template metaprogramming to implement various optimizations like > empty base class and whatnot. Copying that verbatim would make the test huge > (and in the case of libstdc++ its probably not even possible), and recreating > it from scratch is not trivial. I believe that after I wrote the std::string > test this way I tried to also do the same for some of the container types (it > may have actually been variant), and just gave up after I saw hold long it > would take. > > Nonetheless, I think there's definitely value in tests like this (for one, > going through the implementation in this way makes it obvious how many corner > cases need testing), and it's nice to see there's interest for these tests. > I'll keep that in mind when working on future changes. Agreed, they can get pretty unwieldy. I'll see what the `std::(unordered_)map` simulator could look like since we're making changes there at the moment https://github.com/llvm/llvm-project/pull/97568 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Clean up LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair (PR #97551)
Michael137 wrote: > Why do we even have the `GetValueOffset` function? Wouldn't it be possible to > dredge the actual type describing the node layout from somewhere (a template > argument of something, or a return value of some method?) Agreed? that’s actually what I’m planning to do in https://github.com/llvm/llvm-project/pull/97579 (by getting the type via a typedef). This patch was kind of an intermediate step to make the followup more straightforward to review, but now looking at it, I’m not sure there’s value in that. I guess we could just skip this and do the refactor in one go https://github.com/llvm/llvm-project/pull/97551 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix printing of unsigned enum bitfields when they contain the max value (PR #96202)
Michael137 wrote:
FYI, the new test seems to be failing on the matrix LLDB bot that's testing
DWARFv2:
```
==
FAIL: test_bitfield_enums_dsym (TestBitfieldEnums.TestBitfieldEnum)
--
Traceback (most recent call last):
File
"/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
line 1756, in test_method
return attrvalue(self)
File
"/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/llvm-project/lldb/test/API/commands/expression/bitfield_enums/TestBitfieldEnums.py",
line 20, in test_bitfield_enums
self.expect_expr(
File
"/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
line 2512, in expect_expr
value_check.check_value(self, eval_result, str(eval_result))
File
"/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
line 326, in check_value
self.check_value_children(test_base, val, error_msg)
File
"/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
line 349, in check_value_children
expected_child.check_value(test_base, actual_child, child_error)
File
"/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
line 311, in check_value
test_base.assertEqual(self.expect_value, val.GetValue(), this_error_msg)
AssertionError: 'max' != '-1'
- max
+ -1
: Checking child with index 5:
(BitfieldStruct) $0 = {
signed_min = min
signed_other = -1
signed_max = max
unsigned_min = min
unsigned_other = 1
unsigned_max = -1
}
Checking SBValue: (UnsignedEnum:2) unsigned_max = -1
```
https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-matrix/459/execution/node/59/log/?consoleFull
(both the dSYM and non-dSYM variants of the test are failing)
AFAICT, this seems to just be an existing bug in the DWARFv2 support that this
test uncovered.
With my system LLDB (compiling the test with `-gdwarf-2`):
```
(lldb) v bfs
(BitfieldStruct) bfs = {
signed_min = min
signed_other = -1
signed_max = max
unsigned_min = min
unsigned_other = 1
unsigned_max = -1
}
```
https://github.com/llvm/llvm-project/pull/96202
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::unordered_map::iterator formatter (PR #97754)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97754
>From b10f76bd6d02106e80315a70a7b72461cb6f2a99 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 4 Jul 2024 13:35:21 +0200
Subject: [PATCH 1/2] [lldb][DataFormatter] Move std::unordered_map::iterator
formatter into LibCxxUnorderedMap.cpp
---
.../Plugins/Language/CPlusPlus/LibCxx.cpp | 149 -
.../Plugins/Language/CPlusPlus/LibCxx.h | 54 +
.../Language/CPlusPlus/LibCxxUnorderedMap.cpp | 196 ++
3 files changed, 200 insertions(+), 199 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 05cfa0568c25d..feaa51a96843a 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -202,155 +202,6 @@ bool
lldb_private::formatters::LibcxxUniquePointerSummaryProvider(
return true;
}
-lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
-LibCxxUnorderedMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp) {
- if (valobj_sp)
-Update();
-}
-
-lldb::ChildCacheState lldb_private::formatters::
-LibCxxUnorderedMapIteratorSyntheticFrontEnd::Update() {
- m_pair_sp.reset();
- m_iter_ptr = nullptr;
-
- ValueObjectSP valobj_sp = m_backend.GetSP();
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- TargetSP target_sp(valobj_sp->GetTargetSP());
-
- if (!target_sp)
-return lldb::ChildCacheState::eRefetch;
-
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- auto exprPathOptions = ValueObject::GetValueForExpressionPathOptions()
- .DontCheckDotVsArrowSyntax()
- .SetSyntheticChildrenTraversal(
-
ValueObject::GetValueForExpressionPathOptions::
- SyntheticChildrenTraversal::None);
-
- // This must be a ValueObject* because it is a child of the ValueObject we
- // are producing children for it if were a ValueObjectSP, we would end up
- // with a loop (iterator -> synthetic -> child -> parent == iterator) and
- // that would in turn leak memory by never allowing the ValueObjects to die
- // and free their memory.
- m_iter_ptr =
- valobj_sp
- ->GetValueForExpressionPath(".__i_.__node_", nullptr, nullptr,
- exprPathOptions, nullptr)
- .get();
-
- if (m_iter_ptr) {
-auto iter_child(valobj_sp->GetChildMemberWithName("__i_"));
-if (!iter_child) {
- m_iter_ptr = nullptr;
- return lldb::ChildCacheState::eRefetch;
-}
-
-CompilerType node_type(iter_child->GetCompilerType()
- .GetTypeTemplateArgument(0)
- .GetPointeeType());
-
-CompilerType pair_type(node_type.GetTypeTemplateArgument(0));
-
-std::string name;
-uint64_t bit_offset_ptr;
-uint32_t bitfield_bit_size_ptr;
-bool is_bitfield_ptr;
-
-pair_type = pair_type.GetFieldAtIndex(
-0, name, _offset_ptr, _bit_size_ptr, _bitfield_ptr);
-if (!pair_type) {
- m_iter_ptr = nullptr;
- return lldb::ChildCacheState::eRefetch;
-}
-
-uint64_t addr = m_iter_ptr->GetValueAsUnsigned(LLDB_INVALID_ADDRESS);
-m_iter_ptr = nullptr;
-
-if (addr == 0 || addr == LLDB_INVALID_ADDRESS)
- return lldb::ChildCacheState::eRefetch;
-
-auto ts = pair_type.GetTypeSystem();
-auto ast_ctx = ts.dyn_cast_or_null();
-if (!ast_ctx)
- return lldb::ChildCacheState::eRefetch;
-
-// Mimick layout of std::__hash_iterator::__node_ and read it in
-// from process memory.
-//
-// The following shows the contiguous block of memory:
-//
-// +-+ class __hash_node_base
-// __node_ | __next_pointer __next_; |
-// +-+ class __hash_node
-// | size_t __hash_; |
-// | __node_value_type __value_; | <<< our key/value pair
-// +-+
-//
-CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-llvm::StringRef(),
-{{"__next_",
- ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"__hash_", ast_ctx->GetBasicType(lldb::eBasicTypeUnsignedLongLong)},
- {"__value_", pair_type}});
-std::optional size = tree_node_type.GetByteSize(nullptr);
-if (!size)
- return lldb::ChildCacheState::eRefetch;
-WritableDataBufferSP buffer_sp(new DataBufferHeap(*size, 0));
-ProcessSP process_sp(target_sp->GetProcessSP());
-Status error;
-process_sp->ReadMemory(addr, buffer_sp->GetBytes(),
- buffer_sp->GetByteSize(), error);
-if (error.Fail())
- return lldb::ChildCacheState::eRefetch;
-
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::unordered_map::iterator formatter (PR #97754)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97754
>From b10f76bd6d02106e80315a70a7b72461cb6f2a99 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 4 Jul 2024 13:35:21 +0200
Subject: [PATCH 1/2] [lldb][DataFormatter] Move std::unordered_map::iterator
formatter into LibCxxUnorderedMap.cpp
---
.../Plugins/Language/CPlusPlus/LibCxx.cpp | 149 -
.../Plugins/Language/CPlusPlus/LibCxx.h | 54 +
.../Language/CPlusPlus/LibCxxUnorderedMap.cpp | 196 ++
3 files changed, 200 insertions(+), 199 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 05cfa0568c25d..feaa51a96843a 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -202,155 +202,6 @@ bool
lldb_private::formatters::LibcxxUniquePointerSummaryProvider(
return true;
}
-lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
-LibCxxUnorderedMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp) {
- if (valobj_sp)
-Update();
-}
-
-lldb::ChildCacheState lldb_private::formatters::
-LibCxxUnorderedMapIteratorSyntheticFrontEnd::Update() {
- m_pair_sp.reset();
- m_iter_ptr = nullptr;
-
- ValueObjectSP valobj_sp = m_backend.GetSP();
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- TargetSP target_sp(valobj_sp->GetTargetSP());
-
- if (!target_sp)
-return lldb::ChildCacheState::eRefetch;
-
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- auto exprPathOptions = ValueObject::GetValueForExpressionPathOptions()
- .DontCheckDotVsArrowSyntax()
- .SetSyntheticChildrenTraversal(
-
ValueObject::GetValueForExpressionPathOptions::
- SyntheticChildrenTraversal::None);
-
- // This must be a ValueObject* because it is a child of the ValueObject we
- // are producing children for it if were a ValueObjectSP, we would end up
- // with a loop (iterator -> synthetic -> child -> parent == iterator) and
- // that would in turn leak memory by never allowing the ValueObjects to die
- // and free their memory.
- m_iter_ptr =
- valobj_sp
- ->GetValueForExpressionPath(".__i_.__node_", nullptr, nullptr,
- exprPathOptions, nullptr)
- .get();
-
- if (m_iter_ptr) {
-auto iter_child(valobj_sp->GetChildMemberWithName("__i_"));
-if (!iter_child) {
- m_iter_ptr = nullptr;
- return lldb::ChildCacheState::eRefetch;
-}
-
-CompilerType node_type(iter_child->GetCompilerType()
- .GetTypeTemplateArgument(0)
- .GetPointeeType());
-
-CompilerType pair_type(node_type.GetTypeTemplateArgument(0));
-
-std::string name;
-uint64_t bit_offset_ptr;
-uint32_t bitfield_bit_size_ptr;
-bool is_bitfield_ptr;
-
-pair_type = pair_type.GetFieldAtIndex(
-0, name, _offset_ptr, _bit_size_ptr, _bitfield_ptr);
-if (!pair_type) {
- m_iter_ptr = nullptr;
- return lldb::ChildCacheState::eRefetch;
-}
-
-uint64_t addr = m_iter_ptr->GetValueAsUnsigned(LLDB_INVALID_ADDRESS);
-m_iter_ptr = nullptr;
-
-if (addr == 0 || addr == LLDB_INVALID_ADDRESS)
- return lldb::ChildCacheState::eRefetch;
-
-auto ts = pair_type.GetTypeSystem();
-auto ast_ctx = ts.dyn_cast_or_null();
-if (!ast_ctx)
- return lldb::ChildCacheState::eRefetch;
-
-// Mimick layout of std::__hash_iterator::__node_ and read it in
-// from process memory.
-//
-// The following shows the contiguous block of memory:
-//
-// +-+ class __hash_node_base
-// __node_ | __next_pointer __next_; |
-// +-+ class __hash_node
-// | size_t __hash_; |
-// | __node_value_type __value_; | <<< our key/value pair
-// +-+
-//
-CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-llvm::StringRef(),
-{{"__next_",
- ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"__hash_", ast_ctx->GetBasicType(lldb::eBasicTypeUnsignedLongLong)},
- {"__value_", pair_type}});
-std::optional size = tree_node_type.GetByteSize(nullptr);
-if (!size)
- return lldb::ChildCacheState::eRefetch;
-WritableDataBufferSP buffer_sp(new DataBufferHeap(*size, 0));
-ProcessSP process_sp(target_sp->GetProcessSP());
-Status error;
-process_sp->ReadMemory(addr, buffer_sp->GetBytes(),
- buffer_sp->GetByteSize(), error);
-if (error.Fail())
- return lldb::ChildCacheState::eRefetch;
-
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::unordered_map::iterator formatter (PR #97754)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/97754 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::unordered_map::iterator formatter (PR #97754)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97754
>From b10f76bd6d02106e80315a70a7b72461cb6f2a99 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 4 Jul 2024 13:35:21 +0200
Subject: [PATCH 1/2] [lldb][DataFormatter] Move std::unordered_map::iterator
formatter into LibCxxUnorderedMap.cpp
---
.../Plugins/Language/CPlusPlus/LibCxx.cpp | 149 -
.../Plugins/Language/CPlusPlus/LibCxx.h | 54 +
.../Language/CPlusPlus/LibCxxUnorderedMap.cpp | 196 ++
3 files changed, 200 insertions(+), 199 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 05cfa0568c25d4..feaa51a96843ab 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -202,155 +202,6 @@ bool
lldb_private::formatters::LibcxxUniquePointerSummaryProvider(
return true;
}
-lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
-LibCxxUnorderedMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp) {
- if (valobj_sp)
-Update();
-}
-
-lldb::ChildCacheState lldb_private::formatters::
-LibCxxUnorderedMapIteratorSyntheticFrontEnd::Update() {
- m_pair_sp.reset();
- m_iter_ptr = nullptr;
-
- ValueObjectSP valobj_sp = m_backend.GetSP();
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- TargetSP target_sp(valobj_sp->GetTargetSP());
-
- if (!target_sp)
-return lldb::ChildCacheState::eRefetch;
-
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- auto exprPathOptions = ValueObject::GetValueForExpressionPathOptions()
- .DontCheckDotVsArrowSyntax()
- .SetSyntheticChildrenTraversal(
-
ValueObject::GetValueForExpressionPathOptions::
- SyntheticChildrenTraversal::None);
-
- // This must be a ValueObject* because it is a child of the ValueObject we
- // are producing children for it if were a ValueObjectSP, we would end up
- // with a loop (iterator -> synthetic -> child -> parent == iterator) and
- // that would in turn leak memory by never allowing the ValueObjects to die
- // and free their memory.
- m_iter_ptr =
- valobj_sp
- ->GetValueForExpressionPath(".__i_.__node_", nullptr, nullptr,
- exprPathOptions, nullptr)
- .get();
-
- if (m_iter_ptr) {
-auto iter_child(valobj_sp->GetChildMemberWithName("__i_"));
-if (!iter_child) {
- m_iter_ptr = nullptr;
- return lldb::ChildCacheState::eRefetch;
-}
-
-CompilerType node_type(iter_child->GetCompilerType()
- .GetTypeTemplateArgument(0)
- .GetPointeeType());
-
-CompilerType pair_type(node_type.GetTypeTemplateArgument(0));
-
-std::string name;
-uint64_t bit_offset_ptr;
-uint32_t bitfield_bit_size_ptr;
-bool is_bitfield_ptr;
-
-pair_type = pair_type.GetFieldAtIndex(
-0, name, _offset_ptr, _bit_size_ptr, _bitfield_ptr);
-if (!pair_type) {
- m_iter_ptr = nullptr;
- return lldb::ChildCacheState::eRefetch;
-}
-
-uint64_t addr = m_iter_ptr->GetValueAsUnsigned(LLDB_INVALID_ADDRESS);
-m_iter_ptr = nullptr;
-
-if (addr == 0 || addr == LLDB_INVALID_ADDRESS)
- return lldb::ChildCacheState::eRefetch;
-
-auto ts = pair_type.GetTypeSystem();
-auto ast_ctx = ts.dyn_cast_or_null();
-if (!ast_ctx)
- return lldb::ChildCacheState::eRefetch;
-
-// Mimick layout of std::__hash_iterator::__node_ and read it in
-// from process memory.
-//
-// The following shows the contiguous block of memory:
-//
-// +-+ class __hash_node_base
-// __node_ | __next_pointer __next_; |
-// +-+ class __hash_node
-// | size_t __hash_; |
-// | __node_value_type __value_; | <<< our key/value pair
-// +-+
-//
-CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-llvm::StringRef(),
-{{"__next_",
- ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"__hash_", ast_ctx->GetBasicType(lldb::eBasicTypeUnsignedLongLong)},
- {"__value_", pair_type}});
-std::optional size = tree_node_type.GetByteSize(nullptr);
-if (!size)
- return lldb::ChildCacheState::eRefetch;
-WritableDataBufferSP buffer_sp(new DataBufferHeap(*size, 0));
-ProcessSP process_sp(target_sp->GetProcessSP());
-Status error;
-process_sp->ReadMemory(addr, buffer_sp->GetBytes(),
- buffer_sp->GetByteSize(), error);
-if (error.Fail())
- return lldb::ChildCacheState::eRefetch;
-
[Lldb-commits] [lldb] [lldb] Support new libc++ __compressed_pair layout (PR #96538)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/96538
>From 0d39f1ecfb9643f944aa1352d4a307e5638ab08f Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Mon, 29 Jan 2024 16:23:16 +
Subject: [PATCH 1/2] [lldb] Support new libc++ __compressed_pair layout
This patch is in preparation for the `__compressed_pair`
refactor in https://github.com/llvm/llvm-project/pull/76756.
This gets the formatter tests to at least pass.
Currently in draft because there's still some cleanup to be done.
---
lldb/examples/synthetic/libcxx.py | 26 --
.../Plugins/Language/CPlusPlus/LibCxx.cpp | 61 ++
.../Plugins/Language/CPlusPlus/LibCxxList.cpp | 83 ---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 68 +++
.../Language/CPlusPlus/LibCxxUnorderedMap.cpp | 62 +-
.../Language/CPlusPlus/LibCxxVector.cpp | 40 +
.../list/TestDataFormatterGenericList.py | 3 +-
.../libcxx/string/simulator/main.cpp | 1 +
.../TestDataFormatterLibcxxUniquePtr.py | 7 +-
9 files changed, 246 insertions(+), 105 deletions(-)
diff --git a/lldb/examples/synthetic/libcxx.py
b/lldb/examples/synthetic/libcxx.py
index 474aaa428fa23a..060ff901008497 100644
--- a/lldb/examples/synthetic/libcxx.py
+++ b/lldb/examples/synthetic/libcxx.py
@@ -721,6 +721,12 @@ def _get_value_of_compressed_pair(self, pair):
def update(self):
logger = lldb.formatters.Logger.Logger()
try:
+has_compressed_pair_layout = True
+alloc_valobj = self.valobj.GetChildMemberWithName("__alloc_")
+size_valobj = self.valobj.GetChildMemberWithName("__size_")
+if alloc_valobj.IsValid() and size_valobj.IsValid():
+has_compressed_pair_layout = False
+
# A deque is effectively a two-dim array, with fixed width.
# 'map' contains pointers to the rows of this array. The
# full memory area allocated by the deque is delimited
@@ -734,9 +740,13 @@ def update(self):
# variable tells which element in this NxM array is the 0th
# one, and the 'size' element gives the number of elements
# in the deque.
-count = self._get_value_of_compressed_pair(
-self.valobj.GetChildMemberWithName("__size_")
-)
+if has_compressed_pair_layout:
+count = self._get_value_of_compressed_pair(
+self.valobj.GetChildMemberWithName("__size_")
+)
+else:
+count = size_valobj.GetValueAsUnsigned(0)
+
# give up now if we cant access memory reliably
if self.block_size < 0:
logger.write("block_size < 0")
@@ -748,9 +758,13 @@ def update(self):
self.map_begin = map_.GetChildMemberWithName("__begin_")
map_begin = self.map_begin.GetValueAsUnsigned(0)
map_end =
map_.GetChildMemberWithName("__end_").GetValueAsUnsigned(0)
-map_endcap = self._get_value_of_compressed_pair(
-map_.GetChildMemberWithName("__end_cap_")
-)
+
+if has_compressed_pair_layout:
+map_endcap = self._get_value_of_compressed_pair(
+map_.GetChildMemberWithName("__end_cap_")
+)
+else:
+map_endcap =
map_.GetChildMemberWithName("__end_cap_").GetValueAsUnsigned(0)
# check consistency
if not map_first <= map_begin <= map_end <= map_endcap:
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 0f9f93b727ce86..1eee1d9cec7e82 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -27,6 +27,7 @@
#include "Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h"
#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
#include
#include
@@ -176,9 +177,9 @@ bool
lldb_private::formatters::LibcxxUniquePointerSummaryProvider(
if (!ptr_sp)
return false;
- ptr_sp = GetFirstValueOfLibCXXCompressedPair(*ptr_sp);
- if (!ptr_sp)
-return false;
+ if (ValueObjectSP compressed_pair_value__sp =
+ GetFirstValueOfLibCXXCompressedPair(*ptr_sp))
+ptr_sp = std::move(compressed_pair_value__sp);
if (ptr_sp->GetValueAsUnsigned(0) == 0) {
stream.Printf("nullptr");
@@ -701,15 +702,28 @@
lldb_private::formatters::LibcxxUniquePtrSyntheticFrontEnd::Update() {
if (!ptr_sp)
return lldb::ChildCacheState::eRefetch;
+ bool has_compressed_pair_layout = true;
+ ValueObjectSP deleter_sp(valobj_sp->GetChildMemberWithName("__deleter_"));
+ if (deleter_sp)
+has_compressed_pair_layout = false;
+
// Retrieve the actual pointer and the deleter, and clone them to give them
// user-friendly
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::unordered_map::iterator formatter (PR #97754)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/97754
Depends on https://github.com/llvm/llvm-project/pull/97752
This patch changes the way we retrieve the key/value pair in the
`std::unordered_map::iterator` formatter (similar to how we are changing it for
`std::map::iterator` in https://github.com/llvm/llvm-project/pull/97713, the
motivations being the same).
The `std::unordered_map` already does it this way, so we align the iterator
counterpart to do the same.
>From b10f76bd6d02106e80315a70a7b72461cb6f2a99 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 4 Jul 2024 13:35:21 +0200
Subject: [PATCH 1/2] [lldb][DataFormatter] Move std::unordered_map::iterator
formatter into LibCxxUnorderedMap.cpp
---
.../Plugins/Language/CPlusPlus/LibCxx.cpp | 149 -
.../Plugins/Language/CPlusPlus/LibCxx.h | 54 +
.../Language/CPlusPlus/LibCxxUnorderedMap.cpp | 196 ++
3 files changed, 200 insertions(+), 199 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 05cfa0568c25d..feaa51a96843a 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -202,155 +202,6 @@ bool
lldb_private::formatters::LibcxxUniquePointerSummaryProvider(
return true;
}
-lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
-LibCxxUnorderedMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp) {
- if (valobj_sp)
-Update();
-}
-
-lldb::ChildCacheState lldb_private::formatters::
-LibCxxUnorderedMapIteratorSyntheticFrontEnd::Update() {
- m_pair_sp.reset();
- m_iter_ptr = nullptr;
-
- ValueObjectSP valobj_sp = m_backend.GetSP();
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- TargetSP target_sp(valobj_sp->GetTargetSP());
-
- if (!target_sp)
-return lldb::ChildCacheState::eRefetch;
-
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- auto exprPathOptions = ValueObject::GetValueForExpressionPathOptions()
- .DontCheckDotVsArrowSyntax()
- .SetSyntheticChildrenTraversal(
-
ValueObject::GetValueForExpressionPathOptions::
- SyntheticChildrenTraversal::None);
-
- // This must be a ValueObject* because it is a child of the ValueObject we
- // are producing children for it if were a ValueObjectSP, we would end up
- // with a loop (iterator -> synthetic -> child -> parent == iterator) and
- // that would in turn leak memory by never allowing the ValueObjects to die
- // and free their memory.
- m_iter_ptr =
- valobj_sp
- ->GetValueForExpressionPath(".__i_.__node_", nullptr, nullptr,
- exprPathOptions, nullptr)
- .get();
-
- if (m_iter_ptr) {
-auto iter_child(valobj_sp->GetChildMemberWithName("__i_"));
-if (!iter_child) {
- m_iter_ptr = nullptr;
- return lldb::ChildCacheState::eRefetch;
-}
-
-CompilerType node_type(iter_child->GetCompilerType()
- .GetTypeTemplateArgument(0)
- .GetPointeeType());
-
-CompilerType pair_type(node_type.GetTypeTemplateArgument(0));
-
-std::string name;
-uint64_t bit_offset_ptr;
-uint32_t bitfield_bit_size_ptr;
-bool is_bitfield_ptr;
-
-pair_type = pair_type.GetFieldAtIndex(
-0, name, _offset_ptr, _bit_size_ptr, _bitfield_ptr);
-if (!pair_type) {
- m_iter_ptr = nullptr;
- return lldb::ChildCacheState::eRefetch;
-}
-
-uint64_t addr = m_iter_ptr->GetValueAsUnsigned(LLDB_INVALID_ADDRESS);
-m_iter_ptr = nullptr;
-
-if (addr == 0 || addr == LLDB_INVALID_ADDRESS)
- return lldb::ChildCacheState::eRefetch;
-
-auto ts = pair_type.GetTypeSystem();
-auto ast_ctx = ts.dyn_cast_or_null();
-if (!ast_ctx)
- return lldb::ChildCacheState::eRefetch;
-
-// Mimick layout of std::__hash_iterator::__node_ and read it in
-// from process memory.
-//
-// The following shows the contiguous block of memory:
-//
-// +-+ class __hash_node_base
-// __node_ | __next_pointer __next_; |
-// +-+ class __hash_node
-// | size_t __hash_; |
-// | __node_value_type __value_; | <<< our key/value pair
-// +-+
-//
-CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-llvm::StringRef(),
-{{"__next_",
- ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"__hash_", ast_ctx->GetBasicType(lldb::eBasicTypeUnsignedLongLong)},
- {"__value_", pair_type}});
-std::optional size =
[Lldb-commits] [lldb] [lldb][DataFormatter] Move std::unordered_map::iterator formatter into LibCxxUnorderedMap.cpp (PR #97752)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/97752
Similar to how we moved the `std::map::iterator` formatter in
https://github.com/llvm/llvm-project/pull/97687, do the same for
`std::unordered_map::iterator`.
Again the `unordered_map` and `unordered_map::iterator` formatters try to do
very similar things: retrieve data out of the map. The iterator formatter does
this in a fragile way (similar to how `std::map` does it, see
https://github.com/llvm/llvm-project/pull/97579). Thus we will be refactoring
the `std::unordered_map::iterator` in upcoming patches. Having it in
`LibCxxUnorderedMap` will allow us to re-use some of the logic (and we won't
have to repeat some of the clarification comments).
>From f62e9688284af9295aeee0987398805884c37a4f Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 4 Jul 2024 13:35:21 +0200
Subject: [PATCH] [lldb][DataFormatter] Move std::unordered_map::iterator
formatter into LibCxxUnorderedMap.cpp
---
.../Plugins/Language/CPlusPlus/LibCxx.cpp | 149 -
.../Plugins/Language/CPlusPlus/LibCxx.h | 54 +
.../Language/CPlusPlus/LibCxxUnorderedMap.cpp | 196 ++
3 files changed, 200 insertions(+), 199 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 05cfa0568c25d4..feaa51a96843ab 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -202,155 +202,6 @@ bool
lldb_private::formatters::LibcxxUniquePointerSummaryProvider(
return true;
}
-lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
-LibCxxUnorderedMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp) {
- if (valobj_sp)
-Update();
-}
-
-lldb::ChildCacheState lldb_private::formatters::
-LibCxxUnorderedMapIteratorSyntheticFrontEnd::Update() {
- m_pair_sp.reset();
- m_iter_ptr = nullptr;
-
- ValueObjectSP valobj_sp = m_backend.GetSP();
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- TargetSP target_sp(valobj_sp->GetTargetSP());
-
- if (!target_sp)
-return lldb::ChildCacheState::eRefetch;
-
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- auto exprPathOptions = ValueObject::GetValueForExpressionPathOptions()
- .DontCheckDotVsArrowSyntax()
- .SetSyntheticChildrenTraversal(
-
ValueObject::GetValueForExpressionPathOptions::
- SyntheticChildrenTraversal::None);
-
- // This must be a ValueObject* because it is a child of the ValueObject we
- // are producing children for it if were a ValueObjectSP, we would end up
- // with a loop (iterator -> synthetic -> child -> parent == iterator) and
- // that would in turn leak memory by never allowing the ValueObjects to die
- // and free their memory.
- m_iter_ptr =
- valobj_sp
- ->GetValueForExpressionPath(".__i_.__node_", nullptr, nullptr,
- exprPathOptions, nullptr)
- .get();
-
- if (m_iter_ptr) {
-auto iter_child(valobj_sp->GetChildMemberWithName("__i_"));
-if (!iter_child) {
- m_iter_ptr = nullptr;
- return lldb::ChildCacheState::eRefetch;
-}
-
-CompilerType node_type(iter_child->GetCompilerType()
- .GetTypeTemplateArgument(0)
- .GetPointeeType());
-
-CompilerType pair_type(node_type.GetTypeTemplateArgument(0));
-
-std::string name;
-uint64_t bit_offset_ptr;
-uint32_t bitfield_bit_size_ptr;
-bool is_bitfield_ptr;
-
-pair_type = pair_type.GetFieldAtIndex(
-0, name, _offset_ptr, _bit_size_ptr, _bitfield_ptr);
-if (!pair_type) {
- m_iter_ptr = nullptr;
- return lldb::ChildCacheState::eRefetch;
-}
-
-uint64_t addr = m_iter_ptr->GetValueAsUnsigned(LLDB_INVALID_ADDRESS);
-m_iter_ptr = nullptr;
-
-if (addr == 0 || addr == LLDB_INVALID_ADDRESS)
- return lldb::ChildCacheState::eRefetch;
-
-auto ts = pair_type.GetTypeSystem();
-auto ast_ctx = ts.dyn_cast_or_null();
-if (!ast_ctx)
- return lldb::ChildCacheState::eRefetch;
-
-// Mimick layout of std::__hash_iterator::__node_ and read it in
-// from process memory.
-//
-// The following shows the contiguous block of memory:
-//
-// +-+ class __hash_node_base
-// __node_ | __next_pointer __next_; |
-// +-+ class __hash_node
-// | size_t __hash_; |
-// | __node_value_type __value_; | <<< our key/value pair
-// +-+
-//
-CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-llvm::StringRef(),
-{{"__next_",
[Lldb-commits] [lldb] [lldb][DataFormatter][NFC] Move std::map iterator formatter into LibCxxMap.cpp (PR #97687)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/97687 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify libc++ std::map::iterator formatter (PR #97713)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/97713 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Lldb/map iterator refactor (PR #97713)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/97713
Depends on https://github.com/llvm/llvm-project/pull/97687
Similar to https://github.com/llvm/llvm-project/pull/97579, this patch
simplifies the way in which we retrieve the key/value pair of a `std::map` (in
this case of the `std::map::iterator`).
We do this for the same reason: not only was the old logic hard to follow, and
encoded the libc++ layout in non-obvious ways, it was also fragile to alignment
miscalculations (https://github.com/llvm/llvm-project/pull/97443); this would
break once the new layout of std::map landed as part of
https://github.com/llvm/llvm-project/issues/93069.
Instead, this patch simply casts the `__iter_pointer` to the `__node_pointer`
and uses a straightforward `GetChildMemberWithName("__value_")` to get to the
key/value we care about.
We can eventually re-use the core-part of the `std::map` and
`std::map::iterator` formatters. But it will be an easier to change to review
once both simplifications landed.
>From f6b3e6055a9e2263f61e3f70d7a97ddbb7db5ab0 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 4 Jul 2024 09:30:19 +0200
Subject: [PATCH 1/2] [lldb][DataFormatter][NFC] Move std::map iterator
formatter into LibCxxMap.cpp
The two formatters follow very similar techniques to retrieve data out of the
map. We're changing this for `std::map` in
https://github.com/llvm/llvm-project/pull/97579 and plan to change it in the
same way for the iterator formatter. Having them in the same place will allow
us to re-use some of the logic (and we won't have to repeat some of the
clarification comments).
---
.../Plugins/Language/CPlusPlus/LibCxx.cpp | 188 --
.../Plugins/Language/CPlusPlus/LibCxx.h | 29 +--
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 187 +
3 files changed, 191 insertions(+), 213 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index ad467c3966e60..05cfa0568c25d 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -202,194 +202,6 @@ bool
lldb_private::formatters::LibcxxUniquePointerSummaryProvider(
return true;
}
-/*
- (lldb) fr var ibeg --raw --ptr-depth 1
- (std::__1::__map_iterator,
- std::__1::allocator > >, std::__1::__tree_node,
- std::__1::allocator > >, void *> *, long> >) ibeg = {
- __i_ = {
- __ptr_ = 0x000100103870 {
- std::__1::__tree_node_base = {
- std::__1::__tree_end_node *> = {
- __left_ = 0x
- }
- __right_ = 0x
- __parent_ = 0x0001001038b0
- __is_black_ = true
- }
- __value_ = {
- first = 0
- second = { std::string }
- */
-
-lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::
-LibCxxMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp), m_pair_ptr(), m_pair_sp() {
- if (valobj_sp)
-Update();
-}
-
-lldb::ChildCacheState
-lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::Update() {
- m_pair_sp.reset();
- m_pair_ptr = nullptr;
-
- ValueObjectSP valobj_sp = m_backend.GetSP();
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- TargetSP target_sp(valobj_sp->GetTargetSP());
-
- if (!target_sp)
-return lldb::ChildCacheState::eRefetch;
-
- // this must be a ValueObject* because it is a child of the ValueObject we
- // are producing children for it if were a ValueObjectSP, we would end up
- // with a loop (iterator -> synthetic -> child -> parent == iterator) and
- // that would in turn leak memory by never allowing the ValueObjects to die
- // and free their memory
- m_pair_ptr = valobj_sp
- ->GetValueForExpressionPath(
- ".__i_.__ptr_->__value_", nullptr, nullptr,
- ValueObject::GetValueForExpressionPathOptions()
- .DontCheckDotVsArrowSyntax()
- .SetSyntheticChildrenTraversal(
- ValueObject::GetValueForExpressionPathOptions::
- SyntheticChildrenTraversal::None),
- nullptr)
- .get();
-
- if (!m_pair_ptr) {
-m_pair_ptr = valobj_sp
- ->GetValueForExpressionPath(
- ".__i_.__ptr_", nullptr, nullptr,
- ValueObject::GetValueForExpressionPathOptions()
- .DontCheckDotVsArrowSyntax()
- .SetSyntheticChildrenTraversal(
-
ValueObject::GetValueForExpressionPathOptions::
- SyntheticChildrenTraversal::None),
- nullptr)
- .get();
-if (m_pair_ptr) {
- auto __i_(valobj_sp->GetChildMemberWithName("__i_"));
- if (!__i_) {
-m_pair_ptr = nullptr;
[Lldb-commits] [lldb] [lldb][DataFormatter][NFC] Move std::map iterator formatter into LibCxxMap.cpp (PR #97687)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/97687
The two formatters follow very similar techniques to retrieve data out of the
map. We're changing this for `std::map` in
https://github.com/llvm/llvm-project/pull/97579 and plan to change it in the
same way for the iterator formatter. Having them in the same place will allow
us to re-use some of the logic (and we won't have to repeat some of the
clarification comments).
>From f6b3e6055a9e2263f61e3f70d7a97ddbb7db5ab0 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 4 Jul 2024 09:30:19 +0200
Subject: [PATCH] [lldb][DataFormatter][NFC] Move std::map iterator formatter
into LibCxxMap.cpp
The two formatters follow very similar techniques to retrieve data out of the
map. We're changing this for `std::map` in
https://github.com/llvm/llvm-project/pull/97579 and plan to change it in the
same way for the iterator formatter. Having them in the same place will allow
us to re-use some of the logic (and we won't have to repeat some of the
clarification comments).
---
.../Plugins/Language/CPlusPlus/LibCxx.cpp | 188 --
.../Plugins/Language/CPlusPlus/LibCxx.h | 29 +--
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 187 +
3 files changed, 191 insertions(+), 213 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index ad467c3966e609..05cfa0568c25d4 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -202,194 +202,6 @@ bool
lldb_private::formatters::LibcxxUniquePointerSummaryProvider(
return true;
}
-/*
- (lldb) fr var ibeg --raw --ptr-depth 1
- (std::__1::__map_iterator,
- std::__1::allocator > >, std::__1::__tree_node,
- std::__1::allocator > >, void *> *, long> >) ibeg = {
- __i_ = {
- __ptr_ = 0x000100103870 {
- std::__1::__tree_node_base = {
- std::__1::__tree_end_node *> = {
- __left_ = 0x
- }
- __right_ = 0x
- __parent_ = 0x0001001038b0
- __is_black_ = true
- }
- __value_ = {
- first = 0
- second = { std::string }
- */
-
-lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::
-LibCxxMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp), m_pair_ptr(), m_pair_sp() {
- if (valobj_sp)
-Update();
-}
-
-lldb::ChildCacheState
-lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::Update() {
- m_pair_sp.reset();
- m_pair_ptr = nullptr;
-
- ValueObjectSP valobj_sp = m_backend.GetSP();
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
- TargetSP target_sp(valobj_sp->GetTargetSP());
-
- if (!target_sp)
-return lldb::ChildCacheState::eRefetch;
-
- // this must be a ValueObject* because it is a child of the ValueObject we
- // are producing children for it if were a ValueObjectSP, we would end up
- // with a loop (iterator -> synthetic -> child -> parent == iterator) and
- // that would in turn leak memory by never allowing the ValueObjects to die
- // and free their memory
- m_pair_ptr = valobj_sp
- ->GetValueForExpressionPath(
- ".__i_.__ptr_->__value_", nullptr, nullptr,
- ValueObject::GetValueForExpressionPathOptions()
- .DontCheckDotVsArrowSyntax()
- .SetSyntheticChildrenTraversal(
- ValueObject::GetValueForExpressionPathOptions::
- SyntheticChildrenTraversal::None),
- nullptr)
- .get();
-
- if (!m_pair_ptr) {
-m_pair_ptr = valobj_sp
- ->GetValueForExpressionPath(
- ".__i_.__ptr_", nullptr, nullptr,
- ValueObject::GetValueForExpressionPathOptions()
- .DontCheckDotVsArrowSyntax()
- .SetSyntheticChildrenTraversal(
-
ValueObject::GetValueForExpressionPathOptions::
- SyntheticChildrenTraversal::None),
- nullptr)
- .get();
-if (m_pair_ptr) {
- auto __i_(valobj_sp->GetChildMemberWithName("__i_"));
- if (!__i_) {
-m_pair_ptr = nullptr;
-return lldb::ChildCacheState::eRefetch;
- }
- CompilerType pair_type(
- __i_->GetCompilerType().GetTypeTemplateArgument(0));
- std::string name;
- uint64_t bit_offset_ptr;
- uint32_t bitfield_bit_size_ptr;
- bool is_bitfield_ptr;
- pair_type = pair_type.GetFieldAtIndex(
- 0, name, _offset_ptr, _bit_size_ptr, _bitfield_ptr);
- if (!pair_type) {
-m_pair_ptr = nullptr;
-return lldb::ChildCacheState::eRefetch;
- }
-
- auto addr(m_pair_ptr->GetValueAsUnsigned(LLDB_INVALID_ADDRESS));
-
[Lldb-commits] [lldb] [lldb][DataFormatter] Remove support for old std::map layout (PR #97549)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97549
>From 5dc61f0721746359cbaa70e5f50dd15de4a1f082 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 3 Jul 2024 12:06:49 +0200
Subject: [PATCH 1/2] [lldb][DataFormatter] Remove support for old std::map
layout
We currently supported the layout from pre-2016 (before the layout
change in 14caaddd3f08e798dcd9ac0ddfc).
We have another upcoming layout change in `__tree` and `map` (as part of
require rewriting parts of this formatter. Removing the support will
make those changes more straightforward to review/maintain.
Being backward compatible would be great but we have no tests that
actually verify that the old layout still works (and our oldest
matrix bot tests clang-15). If anyone feels strongly about keeping
this layout, we could possibly factor out that logic and keep it around.
---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 76 ---
1 file changed, 30 insertions(+), 46 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 6c2bc1a34137a..141b525da063b 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -248,11 +248,6 @@ bool
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetDataType() {
deref = m_root_node->Dereference(error);
if (!deref || error.Fail())
return false;
- deref = deref->GetChildMemberWithName("__value_");
- if (deref) {
-m_element_type = deref->GetCompilerType();
-return true;
- }
deref = m_backend.GetChildAtNamePath({"__tree_", "__pair3_"});
if (!deref)
return false;
@@ -280,40 +275,35 @@ void
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
return;
if (!node)
return;
+
CompilerType node_type(node->GetCompilerType());
- uint64_t bit_offset;
- if (node_type.GetIndexOfFieldWithName("__value_", nullptr, _offset) !=
- UINT32_MAX) {
-// Old layout (pre d05b10ab4fc65)
-m_skip_size = bit_offset / 8u;
- } else {
-auto ast_ctx =
node_type.GetTypeSystem().dyn_cast_or_null();
-if (!ast_ctx)
- return;
-CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-llvm::StringRef(),
-{{"ptr0",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"ptr1",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"ptr2",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
- {"payload", (m_element_type.GetCompleteType(), m_element_type)}});
-std::string child_name;
-uint32_t child_byte_size;
-int32_t child_byte_offset = 0;
-uint32_t child_bitfield_bit_size;
-uint32_t child_bitfield_bit_offset;
-bool child_is_base_class;
-bool child_is_deref_of_parent;
-uint64_t language_flags;
-auto child_type =
-llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
-nullptr, 4, true, true, true, child_name, child_byte_size,
-child_byte_offset, child_bitfield_bit_size,
-child_bitfield_bit_offset, child_is_base_class,
-child_is_deref_of_parent, nullptr, language_flags));
-if (child_type && child_type->IsValid())
- m_skip_size = (uint32_t)child_byte_offset;
- }
+ auto ast_ctx = node_type.GetTypeSystem().dyn_cast_or_null();
+ if (!ast_ctx)
+return;
+
+ CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
+ llvm::StringRef(),
+ {{"ptr0", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"ptr1", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"ptr2", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
+ {"payload", (m_element_type.GetCompleteType(), m_element_type)}});
+ std::string child_name;
+ uint32_t child_byte_size;
+ int32_t child_byte_offset = 0;
+ uint32_t child_bitfield_bit_size;
+ uint32_t child_bitfield_bit_offset;
+ bool child_is_base_class;
+ bool child_is_deref_of_parent;
+ uint64_t language_flags;
+ auto child_type =
+ llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
+ nullptr, 4, true, true, true, child_name, child_byte_size,
+ child_byte_offset, child_bitfield_bit_size,
child_bitfield_bit_offset,
+ child_is_base_class, child_is_deref_of_parent, nullptr,
+ language_flags));
+ if (child_type && child_type->IsValid())
+m_skip_size = (uint32_t)child_byte_offset;
}
ValueObjectSP
@@ -348,14 +338,8 @@
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair(
return nullptr;
GetValueOffset(iterated_sp);
-auto child_sp = iterated_sp->GetChildMemberWithName("__value_");
-if (child_sp) {
- // Old
[Lldb-commits] [lldb] [lldb][DataFormatter] Remove support for old std::map layout (PR #97549)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97549
>From 5dc61f0721746359cbaa70e5f50dd15de4a1f082 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 3 Jul 2024 12:06:49 +0200
Subject: [PATCH 1/2] [lldb][DataFormatter] Remove support for old std::map
layout
We currently supported the layout from pre-2016 (before the layout
change in 14caaddd3f08e798dcd9ac0ddfc).
We have another upcoming layout change in `__tree` and `map` (as part of
require rewriting parts of this formatter. Removing the support will
make those changes more straightforward to review/maintain.
Being backward compatible would be great but we have no tests that
actually verify that the old layout still works (and our oldest
matrix bot tests clang-15). If anyone feels strongly about keeping
this layout, we could possibly factor out that logic and keep it around.
---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 76 ---
1 file changed, 30 insertions(+), 46 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 6c2bc1a34137a..141b525da063b 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -248,11 +248,6 @@ bool
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetDataType() {
deref = m_root_node->Dereference(error);
if (!deref || error.Fail())
return false;
- deref = deref->GetChildMemberWithName("__value_");
- if (deref) {
-m_element_type = deref->GetCompilerType();
-return true;
- }
deref = m_backend.GetChildAtNamePath({"__tree_", "__pair3_"});
if (!deref)
return false;
@@ -280,40 +275,35 @@ void
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
return;
if (!node)
return;
+
CompilerType node_type(node->GetCompilerType());
- uint64_t bit_offset;
- if (node_type.GetIndexOfFieldWithName("__value_", nullptr, _offset) !=
- UINT32_MAX) {
-// Old layout (pre d05b10ab4fc65)
-m_skip_size = bit_offset / 8u;
- } else {
-auto ast_ctx =
node_type.GetTypeSystem().dyn_cast_or_null();
-if (!ast_ctx)
- return;
-CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-llvm::StringRef(),
-{{"ptr0",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"ptr1",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"ptr2",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
- {"payload", (m_element_type.GetCompleteType(), m_element_type)}});
-std::string child_name;
-uint32_t child_byte_size;
-int32_t child_byte_offset = 0;
-uint32_t child_bitfield_bit_size;
-uint32_t child_bitfield_bit_offset;
-bool child_is_base_class;
-bool child_is_deref_of_parent;
-uint64_t language_flags;
-auto child_type =
-llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
-nullptr, 4, true, true, true, child_name, child_byte_size,
-child_byte_offset, child_bitfield_bit_size,
-child_bitfield_bit_offset, child_is_base_class,
-child_is_deref_of_parent, nullptr, language_flags));
-if (child_type && child_type->IsValid())
- m_skip_size = (uint32_t)child_byte_offset;
- }
+ auto ast_ctx = node_type.GetTypeSystem().dyn_cast_or_null();
+ if (!ast_ctx)
+return;
+
+ CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
+ llvm::StringRef(),
+ {{"ptr0", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"ptr1", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"ptr2", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
+ {"payload", (m_element_type.GetCompleteType(), m_element_type)}});
+ std::string child_name;
+ uint32_t child_byte_size;
+ int32_t child_byte_offset = 0;
+ uint32_t child_bitfield_bit_size;
+ uint32_t child_bitfield_bit_offset;
+ bool child_is_base_class;
+ bool child_is_deref_of_parent;
+ uint64_t language_flags;
+ auto child_type =
+ llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
+ nullptr, 4, true, true, true, child_name, child_byte_size,
+ child_byte_offset, child_bitfield_bit_size,
child_bitfield_bit_offset,
+ child_is_base_class, child_is_deref_of_parent, nullptr,
+ language_flags));
+ if (child_type && child_type->IsValid())
+m_skip_size = (uint32_t)child_byte_offset;
}
ValueObjectSP
@@ -348,14 +338,8 @@
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair(
return nullptr;
GetValueOffset(iterated_sp);
-auto child_sp = iterated_sp->GetChildMemberWithName("__value_");
-if (child_sp) {
- // Old
[Lldb-commits] [lldb] 30df629 - [lldb][DataFormatter][NFC] Remove duplicate null-check in std::map iterator formatter
Author: Michael Buch
Date: 2024-07-04T08:51:28+02:00
New Revision: 30df62992e890310550259afbe458b845c0d6b89
URL:
https://github.com/llvm/llvm-project/commit/30df62992e890310550259afbe458b845c0d6b89
DIFF:
https://github.com/llvm/llvm-project/commit/30df62992e890310550259afbe458b845c0d6b89.diff
LOG: [lldb][DataFormatter][NFC] Remove duplicate null-check in std::map
iterator formatter
The nullness is already checked a few lines before this.
Added:
Modified:
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
Removed:
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 0f9f93b727ce8..ad467c3966e60 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -245,9 +245,6 @@
lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::Update() {
if (!target_sp)
return lldb::ChildCacheState::eRefetch;
- if (!valobj_sp)
-return lldb::ChildCacheState::eRefetch;
-
// this must be a ValueObject* because it is a child of the ValueObject we
// are producing children for it if were a ValueObjectSP, we would end up
// with a loop (iterator -> synthetic -> child -> parent == iterator) and
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Remove support for old std::map layout (PR #97549)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97549
>From 2e36d3d7670298fb296d82af2fdc9de8c32a48c3 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 3 Jul 2024 12:06:49 +0200
Subject: [PATCH] [lldb][DataFormatter] Remove support for old std::map layout
We currently supported the layout from pre-2016 (before the layout
change in 14caaddd3f08e798dcd9ac0ddfc).
We have another upcoming layout change in `__tree` and `map` (as part of
require rewriting parts of this formatter. Removing the support will
make those changes more straightforward to review/maintain.
Being backward compatible would be great but we have no tests that
actually verify that the old layout still works (and our oldest
matrix bot tests clang-15). If anyone feels strongly about keeping
this layout, we could possibly factor out that logic and keep it around.
---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 76 ---
1 file changed, 30 insertions(+), 46 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 6c2bc1a34137a..141b525da063b 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -248,11 +248,6 @@ bool
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetDataType() {
deref = m_root_node->Dereference(error);
if (!deref || error.Fail())
return false;
- deref = deref->GetChildMemberWithName("__value_");
- if (deref) {
-m_element_type = deref->GetCompilerType();
-return true;
- }
deref = m_backend.GetChildAtNamePath({"__tree_", "__pair3_"});
if (!deref)
return false;
@@ -280,40 +275,35 @@ void
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
return;
if (!node)
return;
+
CompilerType node_type(node->GetCompilerType());
- uint64_t bit_offset;
- if (node_type.GetIndexOfFieldWithName("__value_", nullptr, _offset) !=
- UINT32_MAX) {
-// Old layout (pre d05b10ab4fc65)
-m_skip_size = bit_offset / 8u;
- } else {
-auto ast_ctx =
node_type.GetTypeSystem().dyn_cast_or_null();
-if (!ast_ctx)
- return;
-CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-llvm::StringRef(),
-{{"ptr0",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"ptr1",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"ptr2",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
- {"payload", (m_element_type.GetCompleteType(), m_element_type)}});
-std::string child_name;
-uint32_t child_byte_size;
-int32_t child_byte_offset = 0;
-uint32_t child_bitfield_bit_size;
-uint32_t child_bitfield_bit_offset;
-bool child_is_base_class;
-bool child_is_deref_of_parent;
-uint64_t language_flags;
-auto child_type =
-llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
-nullptr, 4, true, true, true, child_name, child_byte_size,
-child_byte_offset, child_bitfield_bit_size,
-child_bitfield_bit_offset, child_is_base_class,
-child_is_deref_of_parent, nullptr, language_flags));
-if (child_type && child_type->IsValid())
- m_skip_size = (uint32_t)child_byte_offset;
- }
+ auto ast_ctx = node_type.GetTypeSystem().dyn_cast_or_null();
+ if (!ast_ctx)
+return;
+
+ CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
+ llvm::StringRef(),
+ {{"ptr0", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"ptr1", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"ptr2", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
+ {"payload", (m_element_type.GetCompleteType(), m_element_type)}});
+ std::string child_name;
+ uint32_t child_byte_size;
+ int32_t child_byte_offset = 0;
+ uint32_t child_bitfield_bit_size;
+ uint32_t child_bitfield_bit_offset;
+ bool child_is_base_class;
+ bool child_is_deref_of_parent;
+ uint64_t language_flags;
+ auto child_type =
+ llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
+ nullptr, 4, true, true, true, child_name, child_byte_size,
+ child_byte_offset, child_bitfield_bit_size,
child_bitfield_bit_offset,
+ child_is_base_class, child_is_deref_of_parent, nullptr,
+ language_flags));
+ if (child_type && child_type->IsValid())
+m_skip_size = (uint32_t)child_byte_offset;
}
ValueObjectSP
@@ -348,14 +338,8 @@
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair(
return nullptr;
GetValueOffset(iterated_sp);
-auto child_sp = iterated_sp->GetChildMemberWithName("__value_");
-if (child_sp) {
- // Old layout
[Lldb-commits] [lldb] [lldb][DataFormatter] Clean up LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair (PR #97551)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97551
>From 27fb4d207722e12ffd88df4ce5095859782f62ce Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 3 Jul 2024 11:43:47 +0200
Subject: [PATCH] [lldb][DataFormatter] Clean up
LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair
This patch cleans up the core of the `std::map` libc++ formatter.
Depends on https://github.com/llvm/llvm-project/pull/97544 and
https://github.com/llvm/llvm-project/pull/97549.
Changes:
* Renamed `m_skip_size` to `m_value_type_offset` to better
describe what it's actually for.
* Made updating `m_skip_size` (now `m_value_type_offset`)
isolated to `GetKeyValuePair` (instead of doing so in the
`GetValueOffset` helper).
* We don't need special logic for the 0th index, so I merged the
two `need_to_skip` branches.
---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 133 --
1 file changed, 55 insertions(+), 78 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 6c2bc1a34137a..e12704ce28443 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -18,6 +18,8 @@
#include "lldb/Utility/Status.h"
#include "lldb/Utility/Stream.h"
#include "lldb/lldb-forward.h"
+#include
+#include
using namespace lldb;
using namespace lldb_private;
@@ -183,7 +185,7 @@ class LibcxxStdMapSyntheticFrontEnd : public
SyntheticChildrenFrontEnd {
private:
bool GetDataType();
- void GetValueOffset(const lldb::ValueObjectSP );
+ std::optional GetValueOffset();
/// Returns the ValueObject for the __tree_node type that
/// holds the key/value pair of the node at index \ref idx.
@@ -204,7 +206,7 @@ class LibcxxStdMapSyntheticFrontEnd : public
SyntheticChildrenFrontEnd {
ValueObject *m_tree = nullptr;
ValueObject *m_root_node = nullptr;
CompilerType m_element_type;
- uint32_t m_skip_size = UINT32_MAX;
+ uint32_t m_value_type_offset = UINT32_MAX;
size_t m_count = UINT32_MAX;
std::map m_iterators;
};
@@ -274,46 +276,43 @@ bool
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetDataType() {
}
}
-void lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
-const lldb::ValueObjectSP ) {
- if (m_skip_size != UINT32_MAX)
-return;
- if (!node)
-return;
- CompilerType node_type(node->GetCompilerType());
- uint64_t bit_offset;
- if (node_type.GetIndexOfFieldWithName("__value_", nullptr, _offset) !=
- UINT32_MAX) {
-// Old layout (pre d05b10ab4fc65)
-m_skip_size = bit_offset / 8u;
- } else {
-auto ast_ctx =
node_type.GetTypeSystem().dyn_cast_or_null();
-if (!ast_ctx)
- return;
-CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-llvm::StringRef(),
-{{"ptr0",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"ptr1",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"ptr2",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
- {"payload", (m_element_type.GetCompleteType(), m_element_type)}});
-std::string child_name;
-uint32_t child_byte_size;
-int32_t child_byte_offset = 0;
-uint32_t child_bitfield_bit_size;
-uint32_t child_bitfield_bit_offset;
-bool child_is_base_class;
-bool child_is_deref_of_parent;
-uint64_t language_flags;
-auto child_type =
-llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
-nullptr, 4, true, true, true, child_name, child_byte_size,
-child_byte_offset, child_bitfield_bit_size,
-child_bitfield_bit_offset, child_is_base_class,
-child_is_deref_of_parent, nullptr, language_flags));
-if (child_type && child_type->IsValid())
- m_skip_size = (uint32_t)child_byte_offset;
- }
+std::optional
+lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset() {
+ if (!m_tree)
+return std::nullopt;
+
+ auto ast_ctx = m_tree->GetCompilerType()
+ .GetTypeSystem()
+ .dyn_cast_or_null();
+ if (!ast_ctx)
+return std::nullopt;
+
+ CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
+ llvm::StringRef(),
+ {{"ptr0", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"ptr1", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"ptr2", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
+ {"payload", (m_element_type.GetCompleteType(), m_element_type)}});
+ std::string child_name;
+ uint32_t child_byte_size;
+ int32_t child_byte_offset = 0;
+ uint32_t child_bitfield_bit_size;
+ uint32_t child_bitfield_bit_offset;
+ bool child_is_base_class;
+ bool
[Lldb-commits] [lldb] [lldb][DataFormatter][NFC] Factor out MapIterator logic into separate helper (PR #97544)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/97544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::map formatter (PR #97579)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/97579 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::map formatter (PR #97579)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/97579 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::map formatter (PR #97579)
Michael137 wrote: This is currently a bit hard to review because it's based off of other branches that are in review. So feel free to hold off on reviewing until the dependencies landed https://github.com/llvm/llvm-project/pull/97579 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::map formatter (PR #97579)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/97579
Depends on:
* https://github.com/llvm/llvm-project/pull/97544
* https://github.com/llvm/llvm-project/pull/97549
* https://github.com/llvm/llvm-project/pull/97551
This patch tries to simplify the way in which the
`std::map` formatter goes from the root `__tree`
pointer to a specific key/value pair.
Previously we would synthesize a structure that
mimicked what `__iter_pointer` looked like
in memory, then called `GetChildCompilerTypeAtIndex`
on it to find the byte offset into that structure
at which the pair was located at, and finally
use that offset through a call to `GetSyntheticChildAtOffset`
to retrieve that pair. Not only was this logic hard to follow,
and encoded the libc++ layout in non-obvious ways, it was also
fragile to alignment miscalculations
(https://github.com/llvm/llvm-project/pull/97443); this would
break after the new layout of std::map landed as part of
inhttps://github.com/https://github.com/llvm/llvm-project/issues/93069.
Instead, this patch simply casts the `__iter_pointer` to
the `__node_pointer` and uses a straightforward
`GetChildMemberWithName("__value_")` to get to the key/value
we care about. This allows us to get rid of some support
infrastructure/class state.
Ideally we would fix the underlying alignment issue, but
this unblocks the libc++ refactor in the interim,
while also benefitting the formatter in terms of readability (in my opinion).
>From 9dabd3a399f37789b6a9bc7578b76e738c344f1d Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 3 Jul 2024 10:55:40 +0200
Subject: [PATCH 1/3] [lldb][DataFormatter][NFC] Factor out MapIterator logic
into separate helper
This patch factors all the logic for advancing the `MapIterator`
out of `GetChildAtIndex`. This, in my opinion, helps readability,
and will be useful for upcoming cleanups in this area.
While here, some drive-by changes:
* added a couple of clarification comments
* fixed a variable name typo
* turned the `return lldb::ValueObjectSP()` into `return nullptr`
* added an assertion to make sure we keep the iterator cache in a valid
state
---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 115 +++---
1 file changed, 72 insertions(+), 43 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 44fe294ced722..370dfa35e7703 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -17,6 +17,7 @@
#include "lldb/Utility/Endian.h"
#include "lldb/Utility/Status.h"
#include "lldb/Utility/Stream.h"
+#include "lldb/lldb-forward.h"
using namespace lldb;
using namespace lldb_private;
@@ -184,6 +185,22 @@ class LibcxxStdMapSyntheticFrontEnd : public
SyntheticChildrenFrontEnd {
void GetValueOffset(const lldb::ValueObjectSP );
+ /// Returns the ValueObject for the __tree_node type that
+ /// holds the key/value pair of the node at index \ref idx.
+ ///
+ /// \param[in] idx The child index that we're looking to get
+ ///the key/value pair for.
+ ///
+ /// \param[in] max_depth The maximum search depth after which
+ /// we stop trying to find the key/value
+ /// pair for.
+ ///
+ /// \returns On success, returns the ValueObjectSP corresponding
+ /// to the __tree_node's __value_ member (which holds
+ /// the key/value pair the formatter wants to display).
+ /// On failure, will return nullptr.
+ ValueObjectSP GetKeyValuePair(size_t idx, size_t max_depth);
+
ValueObject *m_tree = nullptr;
ValueObject *m_root_node = nullptr;
CompilerType m_element_type;
@@ -299,75 +316,88 @@ void
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
}
}
-lldb::ValueObjectSP
-lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex(
-uint32_t idx) {
- static ConstString g_cc_("__cc_"), g_cc("__cc");
- static ConstString g_nc("__nc");
- uint32_t num_children = CalculateNumChildrenIgnoringErrors();
- if (idx >= num_children)
-return lldb::ValueObjectSP();
- if (m_tree == nullptr || m_root_node == nullptr)
-return lldb::ValueObjectSP();
-
- MapIterator iterator(m_root_node, num_children);
+ValueObjectSP
+lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair(
+size_t idx, size_t max_depth) {
+ MapIterator iterator(m_root_node, max_depth);
const bool need_to_skip = (idx > 0);
- size_t actual_advancde = idx;
+ size_t actual_advance = idx;
if (need_to_skip) {
+// If we have already created the iterator for the previous
+// index, we can start from there and advance by 1.
auto cached_iterator = m_iterators.find(idx - 1);
if (cached_iterator != m_iterators.end()) {
iterator = cached_iterator->second;
- actual_advancde = 1;
+ actual_advance = 1;
}
}
-
[Lldb-commits] [lldb] [lldb][DataFormatter] Remove support for old std::map layout (PR #97549)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97549
>From e80ca1531751eb6750eb65fcec75c760e0282792 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 3 Jul 2024 12:06:49 +0200
Subject: [PATCH 1/2] [lldb][DataFormatter] Remove support for old std::map
layout
We currently supported the layout from pre-2016 (before the layout
change in 14caaddd3f08e798dcd9ac0ddfc).
We have another upcoming layout change in `__tree` and `map` (as part of
require rewriting parts of this formatter. Removing the support will
make those changes more straightforward to review/maintain.
Being backward compatible would be great but we have no tests that
actually verify that the old layout still works (and our oldest
matrix bot tests clang-15). If anyone feels strongly about keeping
this layout, we could possibly factor out that logic and keep it around.
---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 68 ---
1 file changed, 29 insertions(+), 39 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 44fe294ced722..7eb6a3637acd2 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -264,39 +264,33 @@ void
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
if (!node)
return;
CompilerType node_type(node->GetCompilerType());
- uint64_t bit_offset;
- if (node_type.GetIndexOfFieldWithName("__value_", nullptr, _offset) !=
- UINT32_MAX) {
-// Old layout (pre 089a7cc5dea)
-m_skip_size = bit_offset / 8u;
- } else {
-auto ast_ctx =
node_type.GetTypeSystem().dyn_cast_or_null();
-if (!ast_ctx)
- return;
-CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-llvm::StringRef(),
-{{"ptr0",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"ptr1",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"ptr2",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
- {"payload", (m_element_type.GetCompleteType(), m_element_type)}});
-std::string child_name;
-uint32_t child_byte_size;
-int32_t child_byte_offset = 0;
-uint32_t child_bitfield_bit_size;
-uint32_t child_bitfield_bit_offset;
-bool child_is_base_class;
-bool child_is_deref_of_parent;
-uint64_t language_flags;
-auto child_type =
-llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
-nullptr, 4, true, true, true, child_name, child_byte_size,
-child_byte_offset, child_bitfield_bit_size,
-child_bitfield_bit_offset, child_is_base_class,
-child_is_deref_of_parent, nullptr, language_flags));
-if (child_type && child_type->IsValid())
- m_skip_size = (uint32_t)child_byte_offset;
- }
+ auto ast_ctx = node_type.GetTypeSystem().dyn_cast_or_null();
+ if (!ast_ctx)
+return;
+
+ CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
+ llvm::StringRef(),
+ {{"ptr0", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"ptr1", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"ptr2", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
+ {"payload", (m_element_type.GetCompleteType(), m_element_type)}});
+ std::string child_name;
+ uint32_t child_byte_size;
+ int32_t child_byte_offset = 0;
+ uint32_t child_bitfield_bit_size;
+ uint32_t child_bitfield_bit_offset;
+ bool child_is_base_class;
+ bool child_is_deref_of_parent;
+ uint64_t language_flags;
+ auto child_type =
+ llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
+ nullptr, 4, true, true, true, child_name, child_byte_size,
+ child_byte_offset, child_bitfield_bit_size,
child_bitfield_bit_offset,
+ child_is_base_class, child_is_deref_of_parent, nullptr,
+ language_flags));
+ if (child_type && child_type->IsValid())
+m_skip_size = (uint32_t)child_byte_offset;
}
lldb::ValueObjectSP
@@ -343,12 +337,8 @@
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex(
return lldb::ValueObjectSP();
}
GetValueOffset(iterated_sp);
-auto child_sp = iterated_sp->GetChildMemberWithName("__value_");
-if (child_sp)
- iterated_sp = child_sp;
-else
- iterated_sp = iterated_sp->GetSyntheticChildAtOffset(
- m_skip_size, m_element_type, true);
+iterated_sp = iterated_sp->GetSyntheticChildAtOffset(m_skip_size,
+ m_element_type, true);
if (!iterated_sp) {
m_tree = nullptr;
return lldb::ValueObjectSP();
>From 3f760be3995be0b3d96eab8b95a9e8ff710232a0 Mon Sep
[Lldb-commits] [lldb] [lldb] Fix printing of unsigned enum bitfields when they contain the max value (PR #96202)
https://github.com/Michael137 approved this pull request. good catch, LGTM https://github.com/llvm/llvm-project/pull/96202 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Print "0x0" for bitfield like enums where the value is 0 (PR #97557)
https://github.com/Michael137 approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/97557 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Print empty enums as if they were unrecognised normal enums (PR #97553)
https://github.com/Michael137 approved this pull request. makes sense https://github.com/llvm/llvm-project/pull/97553 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make variant formatter work with libstdc++-14 (PR #97568)
https://github.com/Michael137 approved this pull request. LGTM Been wondering if we should follow the same principle as we did with `std::string` and simulate the layouts for all the STL types, keeping a record of them as they change. So we know we don't break the old layouts. Of course it has a separate maintenance cost https://github.com/llvm/llvm-project/pull/97568 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Clean up LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair (PR #97551)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/97551 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Clean up LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair (PR #97551)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/97551
This patch cleans up the core of the `std::map` libc++ formatter.
Depends on https://github.com/llvm/llvm-project/pull/97544 and
https://github.com/llvm/llvm-project/pull/97549.
Changes:
* Renamed `m_skip_size` to `m_value_type_offset` to better
describe what it's actually for.
* Made updating `m_skip_size` (now `m_value_type_offset`)
isolated to `GetKeyValuePair` (instead of doing so in the
`GetValueOffset` helper).
* We don't need special logic for the 0th index, so I merged the
two `need_to_skip` branches.
>From 9dabd3a399f37789b6a9bc7578b76e738c344f1d Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 3 Jul 2024 10:55:40 +0200
Subject: [PATCH 1/2] [lldb][DataFormatter][NFC] Factor out MapIterator logic
into separate helper
This patch factors all the logic for advancing the `MapIterator`
out of `GetChildAtIndex`. This, in my opinion, helps readability,
and will be useful for upcoming cleanups in this area.
While here, some drive-by changes:
* added a couple of clarification comments
* fixed a variable name typo
* turned the `return lldb::ValueObjectSP()` into `return nullptr`
* added an assertion to make sure we keep the iterator cache in a valid
state
---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 115 +++---
1 file changed, 72 insertions(+), 43 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 44fe294ced722..370dfa35e7703 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -17,6 +17,7 @@
#include "lldb/Utility/Endian.h"
#include "lldb/Utility/Status.h"
#include "lldb/Utility/Stream.h"
+#include "lldb/lldb-forward.h"
using namespace lldb;
using namespace lldb_private;
@@ -184,6 +185,22 @@ class LibcxxStdMapSyntheticFrontEnd : public
SyntheticChildrenFrontEnd {
void GetValueOffset(const lldb::ValueObjectSP );
+ /// Returns the ValueObject for the __tree_node type that
+ /// holds the key/value pair of the node at index \ref idx.
+ ///
+ /// \param[in] idx The child index that we're looking to get
+ ///the key/value pair for.
+ ///
+ /// \param[in] max_depth The maximum search depth after which
+ /// we stop trying to find the key/value
+ /// pair for.
+ ///
+ /// \returns On success, returns the ValueObjectSP corresponding
+ /// to the __tree_node's __value_ member (which holds
+ /// the key/value pair the formatter wants to display).
+ /// On failure, will return nullptr.
+ ValueObjectSP GetKeyValuePair(size_t idx, size_t max_depth);
+
ValueObject *m_tree = nullptr;
ValueObject *m_root_node = nullptr;
CompilerType m_element_type;
@@ -299,75 +316,88 @@ void
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
}
}
-lldb::ValueObjectSP
-lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex(
-uint32_t idx) {
- static ConstString g_cc_("__cc_"), g_cc("__cc");
- static ConstString g_nc("__nc");
- uint32_t num_children = CalculateNumChildrenIgnoringErrors();
- if (idx >= num_children)
-return lldb::ValueObjectSP();
- if (m_tree == nullptr || m_root_node == nullptr)
-return lldb::ValueObjectSP();
-
- MapIterator iterator(m_root_node, num_children);
+ValueObjectSP
+lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair(
+size_t idx, size_t max_depth) {
+ MapIterator iterator(m_root_node, max_depth);
const bool need_to_skip = (idx > 0);
- size_t actual_advancde = idx;
+ size_t actual_advance = idx;
if (need_to_skip) {
+// If we have already created the iterator for the previous
+// index, we can start from there and advance by 1.
auto cached_iterator = m_iterators.find(idx - 1);
if (cached_iterator != m_iterators.end()) {
iterator = cached_iterator->second;
- actual_advancde = 1;
+ actual_advance = 1;
}
}
- ValueObjectSP iterated_sp(iterator.advance(actual_advancde));
- if (!iterated_sp) {
+ ValueObjectSP iterated_sp(iterator.advance(actual_advance));
+ if (!iterated_sp)
// this tree is garbage - stop
-m_tree =
-nullptr; // this will stop all future searches until an Update()
happens
-return iterated_sp;
- }
+return nullptr;
- if (!GetDataType()) {
-m_tree = nullptr;
-return lldb::ValueObjectSP();
- }
+ if (!GetDataType())
+return nullptr;
if (!need_to_skip) {
Status error;
iterated_sp = iterated_sp->Dereference(error);
-if (!iterated_sp || error.Fail()) {
- m_tree = nullptr;
- return lldb::ValueObjectSP();
-}
+if (!iterated_sp || error.Fail())
+ return nullptr;
+
GetValueOffset(iterated_sp);
auto child_sp = iterated_sp->GetChildMemberWithName("__value_");
-
[Lldb-commits] [lldb] [lldb][DataFormatter] Remove support for old std::map layout (PR #97549)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/97549 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Remove support for old std::map layout (PR #97549)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/97549 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter] Remove support for old std::map layout (PR #97549)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/97549
We currently supported the layout from pre-2016 (before the layout
change in
[14caaddd3f08e798dcd9ac0ddfc](https://github.com/llvm/llvm-project/commit/14caaddd3f08e798dcd9ac0ddfc)).
We have another upcoming layout change in `__tree` and `map` (as part of
require rewriting parts of this formatter. Removing the support will
make those changes more straightforward to review/maintain.
Being backward compatible would be great but we have no tests that
actually verify that the old layout still works (and our oldest
matrix bot tests clang-15). If anyone feels strongly about keeping
this layout, we could possibly factor out that logic and keep it around.
>From e80ca1531751eb6750eb65fcec75c760e0282792 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 3 Jul 2024 12:06:49 +0200
Subject: [PATCH] [lldb][DataFormatter] Remove support for old std::map layout
We currently supported the layout from pre-2016 (before the layout
change in 14caaddd3f08e798dcd9ac0ddfc).
We have another upcoming layout change in `__tree` and `map` (as part of
require rewriting parts of this formatter. Removing the support will
make those changes more straightforward to review/maintain.
Being backward compatible would be great but we have no tests that
actually verify that the old layout still works (and our oldest
matrix bot tests clang-15). If anyone feels strongly about keeping
this layout, we could possibly factor out that logic and keep it around.
---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 68 ---
1 file changed, 29 insertions(+), 39 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 44fe294ced722..7eb6a3637acd2 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -264,39 +264,33 @@ void
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
if (!node)
return;
CompilerType node_type(node->GetCompilerType());
- uint64_t bit_offset;
- if (node_type.GetIndexOfFieldWithName("__value_", nullptr, _offset) !=
- UINT32_MAX) {
-// Old layout (pre 089a7cc5dea)
-m_skip_size = bit_offset / 8u;
- } else {
-auto ast_ctx =
node_type.GetTypeSystem().dyn_cast_or_null();
-if (!ast_ctx)
- return;
-CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-llvm::StringRef(),
-{{"ptr0",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"ptr1",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"ptr2",
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
- {"payload", (m_element_type.GetCompleteType(), m_element_type)}});
-std::string child_name;
-uint32_t child_byte_size;
-int32_t child_byte_offset = 0;
-uint32_t child_bitfield_bit_size;
-uint32_t child_bitfield_bit_offset;
-bool child_is_base_class;
-bool child_is_deref_of_parent;
-uint64_t language_flags;
-auto child_type =
-llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
-nullptr, 4, true, true, true, child_name, child_byte_size,
-child_byte_offset, child_bitfield_bit_size,
-child_bitfield_bit_offset, child_is_base_class,
-child_is_deref_of_parent, nullptr, language_flags));
-if (child_type && child_type->IsValid())
- m_skip_size = (uint32_t)child_byte_offset;
- }
+ auto ast_ctx = node_type.GetTypeSystem().dyn_cast_or_null();
+ if (!ast_ctx)
+return;
+
+ CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
+ llvm::StringRef(),
+ {{"ptr0", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"ptr1", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"ptr2", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+ {"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
+ {"payload", (m_element_type.GetCompleteType(), m_element_type)}});
+ std::string child_name;
+ uint32_t child_byte_size;
+ int32_t child_byte_offset = 0;
+ uint32_t child_bitfield_bit_size;
+ uint32_t child_bitfield_bit_offset;
+ bool child_is_base_class;
+ bool child_is_deref_of_parent;
+ uint64_t language_flags;
+ auto child_type =
+ llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
+ nullptr, 4, true, true, true, child_name, child_byte_size,
+ child_byte_offset, child_bitfield_bit_size,
child_bitfield_bit_offset,
+ child_is_base_class, child_is_deref_of_parent, nullptr,
+ language_flags));
+ if (child_type && child_type->IsValid())
+m_skip_size = (uint32_t)child_byte_offset;
}
lldb::ValueObjectSP
@@ -343,12 +337,8 @@
[Lldb-commits] [lldb] [lldb][DataFormatter][NFC] Factor out MapIterator logic into separate helper (PR #97544)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97544
>From 9dabd3a399f37789b6a9bc7578b76e738c344f1d Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 3 Jul 2024 10:55:40 +0200
Subject: [PATCH 1/2] [lldb][DataFormatter][NFC] Factor out MapIterator logic
into separate helper
This patch factors all the logic for advancing the `MapIterator`
out of `GetChildAtIndex`. This, in my opinion, helps readability,
and will be useful for upcoming cleanups in this area.
While here, some drive-by changes:
* added a couple of clarification comments
* fixed a variable name typo
* turned the `return lldb::ValueObjectSP()` into `return nullptr`
* added an assertion to make sure we keep the iterator cache in a valid
state
---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 115 +++---
1 file changed, 72 insertions(+), 43 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 44fe294ced722..370dfa35e7703 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -17,6 +17,7 @@
#include "lldb/Utility/Endian.h"
#include "lldb/Utility/Status.h"
#include "lldb/Utility/Stream.h"
+#include "lldb/lldb-forward.h"
using namespace lldb;
using namespace lldb_private;
@@ -184,6 +185,22 @@ class LibcxxStdMapSyntheticFrontEnd : public
SyntheticChildrenFrontEnd {
void GetValueOffset(const lldb::ValueObjectSP );
+ /// Returns the ValueObject for the __tree_node type that
+ /// holds the key/value pair of the node at index \ref idx.
+ ///
+ /// \param[in] idx The child index that we're looking to get
+ ///the key/value pair for.
+ ///
+ /// \param[in] max_depth The maximum search depth after which
+ /// we stop trying to find the key/value
+ /// pair for.
+ ///
+ /// \returns On success, returns the ValueObjectSP corresponding
+ /// to the __tree_node's __value_ member (which holds
+ /// the key/value pair the formatter wants to display).
+ /// On failure, will return nullptr.
+ ValueObjectSP GetKeyValuePair(size_t idx, size_t max_depth);
+
ValueObject *m_tree = nullptr;
ValueObject *m_root_node = nullptr;
CompilerType m_element_type;
@@ -299,75 +316,88 @@ void
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
}
}
-lldb::ValueObjectSP
-lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex(
-uint32_t idx) {
- static ConstString g_cc_("__cc_"), g_cc("__cc");
- static ConstString g_nc("__nc");
- uint32_t num_children = CalculateNumChildrenIgnoringErrors();
- if (idx >= num_children)
-return lldb::ValueObjectSP();
- if (m_tree == nullptr || m_root_node == nullptr)
-return lldb::ValueObjectSP();
-
- MapIterator iterator(m_root_node, num_children);
+ValueObjectSP
+lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair(
+size_t idx, size_t max_depth) {
+ MapIterator iterator(m_root_node, max_depth);
const bool need_to_skip = (idx > 0);
- size_t actual_advancde = idx;
+ size_t actual_advance = idx;
if (need_to_skip) {
+// If we have already created the iterator for the previous
+// index, we can start from there and advance by 1.
auto cached_iterator = m_iterators.find(idx - 1);
if (cached_iterator != m_iterators.end()) {
iterator = cached_iterator->second;
- actual_advancde = 1;
+ actual_advance = 1;
}
}
- ValueObjectSP iterated_sp(iterator.advance(actual_advancde));
- if (!iterated_sp) {
+ ValueObjectSP iterated_sp(iterator.advance(actual_advance));
+ if (!iterated_sp)
// this tree is garbage - stop
-m_tree =
-nullptr; // this will stop all future searches until an Update()
happens
-return iterated_sp;
- }
+return nullptr;
- if (!GetDataType()) {
-m_tree = nullptr;
-return lldb::ValueObjectSP();
- }
+ if (!GetDataType())
+return nullptr;
if (!need_to_skip) {
Status error;
iterated_sp = iterated_sp->Dereference(error);
-if (!iterated_sp || error.Fail()) {
- m_tree = nullptr;
- return lldb::ValueObjectSP();
-}
+if (!iterated_sp || error.Fail())
+ return nullptr;
+
GetValueOffset(iterated_sp);
auto child_sp = iterated_sp->GetChildMemberWithName("__value_");
-if (child_sp)
+if (child_sp) {
+ // Old layout (pre 089a7cc5dea)
iterated_sp = child_sp;
-else
+} else {
iterated_sp = iterated_sp->GetSyntheticChildAtOffset(
m_skip_size, m_element_type, true);
-if (!iterated_sp) {
- m_tree = nullptr;
- return lldb::ValueObjectSP();
}
+
+if (!iterated_sp)
+ return nullptr;
} else {
// because of the way our debug info is made, we need to read item 0
// first so that we can cache information used to
[Lldb-commits] [lldb] [lldb][DataFormatter][NFC] Factor out MapIterator logic into separate helper (PR #97544)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/97544
This patch factors all the logic for advancing the `MapIterator` out of
`GetChildAtIndex`. This, in my opinion, helps readability, and will be useful
for upcoming cleanups in this area.
While here, some drive-by changes:
* added a couple of clarification comments
* fixed a variable name typo
* turned the `return lldb::ValueObjectSP()` into `return nullptr`
* added an assertion to make sure we keep the iterator cache in a valid state
>From 9dabd3a399f37789b6a9bc7578b76e738c344f1d Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 3 Jul 2024 10:55:40 +0200
Subject: [PATCH] [lldb][DataFormatter][NFC] Factor out MapIterator logic into
separate helper
This patch factors all the logic for advancing the `MapIterator`
out of `GetChildAtIndex`. This, in my opinion, helps readability,
and will be useful for upcoming cleanups in this area.
While here, some drive-by changes:
* added a couple of clarification comments
* fixed a variable name typo
* turned the `return lldb::ValueObjectSP()` into `return nullptr`
* added an assertion to make sure we keep the iterator cache in a valid
state
---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 115 +++---
1 file changed, 72 insertions(+), 43 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 44fe294ced722..370dfa35e7703 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -17,6 +17,7 @@
#include "lldb/Utility/Endian.h"
#include "lldb/Utility/Status.h"
#include "lldb/Utility/Stream.h"
+#include "lldb/lldb-forward.h"
using namespace lldb;
using namespace lldb_private;
@@ -184,6 +185,22 @@ class LibcxxStdMapSyntheticFrontEnd : public
SyntheticChildrenFrontEnd {
void GetValueOffset(const lldb::ValueObjectSP );
+ /// Returns the ValueObject for the __tree_node type that
+ /// holds the key/value pair of the node at index \ref idx.
+ ///
+ /// \param[in] idx The child index that we're looking to get
+ ///the key/value pair for.
+ ///
+ /// \param[in] max_depth The maximum search depth after which
+ /// we stop trying to find the key/value
+ /// pair for.
+ ///
+ /// \returns On success, returns the ValueObjectSP corresponding
+ /// to the __tree_node's __value_ member (which holds
+ /// the key/value pair the formatter wants to display).
+ /// On failure, will return nullptr.
+ ValueObjectSP GetKeyValuePair(size_t idx, size_t max_depth);
+
ValueObject *m_tree = nullptr;
ValueObject *m_root_node = nullptr;
CompilerType m_element_type;
@@ -299,75 +316,88 @@ void
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
}
}
-lldb::ValueObjectSP
-lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex(
-uint32_t idx) {
- static ConstString g_cc_("__cc_"), g_cc("__cc");
- static ConstString g_nc("__nc");
- uint32_t num_children = CalculateNumChildrenIgnoringErrors();
- if (idx >= num_children)
-return lldb::ValueObjectSP();
- if (m_tree == nullptr || m_root_node == nullptr)
-return lldb::ValueObjectSP();
-
- MapIterator iterator(m_root_node, num_children);
+ValueObjectSP
+lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair(
+size_t idx, size_t max_depth) {
+ MapIterator iterator(m_root_node, max_depth);
const bool need_to_skip = (idx > 0);
- size_t actual_advancde = idx;
+ size_t actual_advance = idx;
if (need_to_skip) {
+// If we have already created the iterator for the previous
+// index, we can start from there and advance by 1.
auto cached_iterator = m_iterators.find(idx - 1);
if (cached_iterator != m_iterators.end()) {
iterator = cached_iterator->second;
- actual_advancde = 1;
+ actual_advance = 1;
}
}
- ValueObjectSP iterated_sp(iterator.advance(actual_advancde));
- if (!iterated_sp) {
+ ValueObjectSP iterated_sp(iterator.advance(actual_advance));
+ if (!iterated_sp)
// this tree is garbage - stop
-m_tree =
-nullptr; // this will stop all future searches until an Update()
happens
-return iterated_sp;
- }
+return nullptr;
- if (!GetDataType()) {
-m_tree = nullptr;
-return lldb::ValueObjectSP();
- }
+ if (!GetDataType())
+return nullptr;
if (!need_to_skip) {
Status error;
iterated_sp = iterated_sp->Dereference(error);
-if (!iterated_sp || error.Fail()) {
- m_tree = nullptr;
- return lldb::ValueObjectSP();
-}
+if (!iterated_sp || error.Fail())
+ return nullptr;
+
GetValueOffset(iterated_sp);
auto child_sp = iterated_sp->GetChildMemberWithName("__value_");
-if (child_sp)
+if (child_sp) {
+ // Old layout (pre 089a7cc5dea)
[Lldb-commits] [lldb] da62f5f - [lldb][DataFormatter][NFC] std::map: Add comments and other minor cleanups
Author: Michael Buch
Date: 2024-07-03T10:33:39+02:00
New Revision: da62f5f8dfe4d4196191b40dc41e1ef2de1bf5cb
URL:
https://github.com/llvm/llvm-project/commit/da62f5f8dfe4d4196191b40dc41e1ef2de1bf5cb
DIFF:
https://github.com/llvm/llvm-project/commit/da62f5f8dfe4d4196191b40dc41e1ef2de1bf5cb.diff
LOG: [lldb][DataFormatter][NFC] std::map: Add comments and other minor cleanups
Added:
Modified:
lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
Removed:
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 96d9bcc3f2cd7..2a241e3764b19 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -80,17 +80,10 @@ class MapEntry {
class MapIterator {
public:
- MapIterator() = default;
- MapIterator(MapEntry entry, size_t depth = 0)
- : m_entry(std::move(entry)), m_max_depth(depth), m_error(false) {}
- MapIterator(ValueObjectSP entry, size_t depth = 0)
- : m_entry(std::move(entry)), m_max_depth(depth), m_error(false) {}
- MapIterator(const MapIterator )
- : m_entry(rhs.m_entry), m_max_depth(rhs.m_max_depth), m_error(false) {}
MapIterator(ValueObject *entry, size_t depth = 0)
: m_entry(entry), m_max_depth(depth), m_error(false) {}
- MapIterator =(const MapIterator &) = default;
+ MapIterator() = default;
ValueObjectSP value() { return m_entry.GetEntry(); }
@@ -108,7 +101,9 @@ class MapIterator {
return m_entry.GetEntry();
}
-protected:
+private:
+ /// Mimicks libc++'s __tree_next algorithm, which libc++ uses
+ /// in its __tree_iteartor::operator++.
void next() {
if (m_entry.null())
return;
@@ -133,7 +128,7 @@ class MapIterator {
m_entry = MapEntry(m_entry.parent());
}
-private:
+ /// Mimicks libc++'s __tree_min algorithm.
MapEntry tree_min(MapEntry x) {
if (x.null())
return MapEntry();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] e89890e - [lldb][DataFormatter][NFC] std::map: minor restructuring in GetChildAtIndex to use early-return
Author: Michael Buch
Date: 2024-07-03T10:34:16+02:00
New Revision: e89890e8e510f2b76c8c4a2b2a6fc323b1e837ad
URL:
https://github.com/llvm/llvm-project/commit/e89890e8e510f2b76c8c4a2b2a6fc323b1e837ad
DIFF:
https://github.com/llvm/llvm-project/commit/e89890e8e510f2b76c8c4a2b2a6fc323b1e837ad.diff
LOG: [lldb][DataFormatter][NFC] std::map: minor restructuring in
GetChildAtIndex to use early-return
Added:
Modified:
lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
Removed:
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 2a241e3764b19..44fe294ced722 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -267,6 +267,7 @@ void
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
uint64_t bit_offset;
if (node_type.GetIndexOfFieldWithName("__value_", nullptr, _offset) !=
UINT32_MAX) {
+// Old layout (pre 089a7cc5dea)
m_skip_size = bit_offset / 8u;
} else {
auto ast_ctx =
node_type.GetTypeSystem().dyn_cast_or_null();
@@ -328,45 +329,47 @@
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex(
nullptr; // this will stop all future searches until an Update()
happens
return iterated_sp;
}
- if (GetDataType()) {
-if (!need_to_skip) {
- Status error;
- iterated_sp = iterated_sp->Dereference(error);
- if (!iterated_sp || error.Fail()) {
-m_tree = nullptr;
-return lldb::ValueObjectSP();
- }
- GetValueOffset(iterated_sp);
- auto child_sp = iterated_sp->GetChildMemberWithName("__value_");
- if (child_sp)
-iterated_sp = child_sp;
- else
-iterated_sp = iterated_sp->GetSyntheticChildAtOffset(
-m_skip_size, m_element_type, true);
- if (!iterated_sp) {
-m_tree = nullptr;
-return lldb::ValueObjectSP();
- }
-} else {
- // because of the way our debug info is made, we need to read item 0
- // first so that we can cache information used to generate other elements
- if (m_skip_size == UINT32_MAX)
-GetChildAtIndex(0);
- if (m_skip_size == UINT32_MAX) {
-m_tree = nullptr;
-return lldb::ValueObjectSP();
- }
+
+ if (!GetDataType()) {
+m_tree = nullptr;
+return lldb::ValueObjectSP();
+ }
+
+ if (!need_to_skip) {
+Status error;
+iterated_sp = iterated_sp->Dereference(error);
+if (!iterated_sp || error.Fail()) {
+ m_tree = nullptr;
+ return lldb::ValueObjectSP();
+}
+GetValueOffset(iterated_sp);
+auto child_sp = iterated_sp->GetChildMemberWithName("__value_");
+if (child_sp)
+ iterated_sp = child_sp;
+else
iterated_sp = iterated_sp->GetSyntheticChildAtOffset(
m_skip_size, m_element_type, true);
- if (!iterated_sp) {
-m_tree = nullptr;
-return lldb::ValueObjectSP();
- }
+if (!iterated_sp) {
+ m_tree = nullptr;
+ return lldb::ValueObjectSP();
}
} else {
-m_tree = nullptr;
-return lldb::ValueObjectSP();
+// because of the way our debug info is made, we need to read item 0
+// first so that we can cache information used to generate other elements
+if (m_skip_size == UINT32_MAX)
+ GetChildAtIndex(0);
+if (m_skip_size == UINT32_MAX) {
+ m_tree = nullptr;
+ return lldb::ValueObjectSP();
+}
+iterated_sp = iterated_sp->GetSyntheticChildAtOffset(m_skip_size,
+ m_element_type, true);
+if (!iterated_sp) {
+ m_tree = nullptr;
+ return lldb::ValueObjectSP();
+}
}
+
// at this point we have a valid
// we need to copy current_sp into a new object otherwise we will end up with
// all items named __value_
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] aa0851a - [lldb][DataFormatter][NFC] Remove redundant variables in std::map formatter
Author: Michael Buch
Date: 2024-07-03T10:33:39+02:00
New Revision: aa0851a5a6fd0c8d66dfd8b259c215dba3fabd1e
URL:
https://github.com/llvm/llvm-project/commit/aa0851a5a6fd0c8d66dfd8b259c215dba3fabd1e
DIFF:
https://github.com/llvm/llvm-project/commit/aa0851a5a6fd0c8d66dfd8b259c215dba3fabd1e.diff
LOG: [lldb][DataFormatter][NFC] Remove redundant variables in std::map formatter
Redundant since:
```
commit be3be28b5d5c97de1c26bf069e0b82043d938f30
Author: Enrico Granata
Date: Mon Oct 3 23:33:00 2016 +
Changes to the std::multimap formatter to make it work against trunk libc++
Fixes rdar://28237486
llvm-svn: 283160
```
Added:
Modified:
lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
Removed:
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 0929d49e55eac..96d9bcc3f2cd7 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -30,7 +30,6 @@ class MapEntry {
: m_entry_sp(entry ? entry->GetSP() : ValueObjectSP()) {}
ValueObjectSP left() const {
-static ConstString g_left("__left_");
if (!m_entry_sp)
return m_entry_sp;
return m_entry_sp->GetSyntheticChildAtOffset(
@@ -38,7 +37,6 @@ class MapEntry {
}
ValueObjectSP right() const {
-static ConstString g_right("__right_");
if (!m_entry_sp)
return m_entry_sp;
return m_entry_sp->GetSyntheticChildAtOffset(
@@ -47,7 +45,6 @@ class MapEntry {
}
ValueObjectSP parent() const {
-static ConstString g_parent("__parent_");
if (!m_entry_sp)
return m_entry_sp;
return m_entry_sp->GetSyntheticChildAtOffset(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] SBThread::StepInstruction shouldn't discard other plans (PR #97493)
https://github.com/Michael137 approved this pull request. LGTM, thanks for fixing this https://github.com/llvm/llvm-project/pull/97493 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][RecordLayoutBuilder] Be stricter about inferring packed-ness in ExternalLayouts (PR #97443)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/97443 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][RecordLayoutBuilder] Be stricter about inferring packed-ness in ExternalLayouts (PR #97443)
@@ -2250,14 +2246,18 @@ void ItaniumRecordLayoutBuilder::UpdateAlignment(
}
}
-uint64_t
-ItaniumRecordLayoutBuilder::updateExternalFieldOffset(const FieldDecl *Field,
- uint64_t ComputedOffset)
{
+uint64_t ItaniumRecordLayoutBuilder::updateExternalFieldOffset(
+const FieldDecl *Field, uint64_t ComputedOffset, uint64_t PreviousOffset) {
uint64_t ExternalFieldOffset = External.getExternalFieldOffset(Field);
- if (InferAlignment && ExternalFieldOffset < ComputedOffset) {
-// The externally-supplied field offset is before the field offset we
-// computed. Assume that the structure is packed.
+ // If the externally-supplied field offset is before the field offset we
+ // computed. Check against the previous field offset to make sure we don't
+ // misinterpret overlapping fields as packedness of the structure.
+ const bool assume_packed = ExternalFieldOffset > 0 &&
+ ExternalFieldOffset < ComputedOffset &&
+ ExternalFieldOffset > PreviousOffset;
Michael137 wrote:
Technically "overlapping" would have to account for size of the previous field
https://github.com/llvm/llvm-project/pull/97443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][RecordLayoutBuilder] Be stricter about inferring packed-ness in ExternalLayouts (PR #97443)
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/97443 This patch is motivated by the LLDB support required for: https://github.com/llvm/llvm-project/issues/93069 In the presence of `[[no_unique_address]]`, LLDB may ask Clang to lay out types with overlapping field offsets. Because we don't have attributes such as `packed` or `no_unique_address` in the LLDB AST, the `RecordLayoutBuilder` supports an `InferAlignment` mode, which, in the past, attempted to detect layouts which came from packed structures in order to provide a conservative estimate of alignment for it (since `DW_AT_alignment` isn't emitted unless explicitly changed with `alignas`, etc.). However, in the presence of overlapping fields due to `no_unique_address`, `InferAlignment` would set the alignment of structures to `1` for which that's incorrect. This poses issues in some LLDB formatters that synthesize new Clang types and rely on the layout builder to get the `FieldOffset` of structures right that we did have DWARF offsets for. The result of this is that if we get the alignment wrong, LLDB reads out garbage data from the wrong field offsets. There are a couple of solutions to this that we considered: 1. Make LLDB formatters not do the above, and make them more robust to inaccurate alignment. 2. Remove `InferAlignment` entirely and rely on Clang emitting `DW_AT_alignment` for packed structures. 3. Remove `InferAlignment` and detect packedness from within LLDB. 4. Make the `InferAlignment` logic account for overlapping fields. Option (1) turned out quite hairy and it's not clear we can achieve this with the tools available for certain STL formatters (particularly `std::map`). But I would still very much like to simplify this if we can. Option (2) wouldn't help with GCC-compiled binaries, and if we can get away with LLDB not needing the alignment, then we wouldn't need to increase debug-info size. Option (3), AFAICT, would require us to reimplement some of the layout logic in the layout builder. Would be great if we can avoid this added complexity. Option (4) seemed like the best option in the interim. As part of this change I also removed one of the `InferAlignment` blocks. The test-cases associated with this code-path pass regardless, and from the description of the change that introduced it it's not clear why specifically the base offsets would influence the `Alignment` field, and how it would imply packedness. But happy to be proven wrong. Ultimately it would be great if we can get rid of the `InferAlignment` infrastructure and support our use-cases in LLDB or DWARF instead. >From 3a718c75d0458b7aece72f2ba8e5aa5a68815237 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Tue, 2 Jul 2024 18:43:34 +0200 Subject: [PATCH] [clang][RecordLayoutBuilder] Be stricter about inferring packed-ness in ExternalLayouts This patch is motivated by the LLDB support required for: https://github.com/llvm/llvm-project/issues/93069 In the presence of `[[no_unique_address]]`, LLDB may ask Clang to lay out types with overlapping field offsets. Because we don't have attributes such as `packed` or `no_unique_address` in the LLDB AST, the `RecordLayoutBuilder` supports an `InferAlignment` mode, which, in the past, attempted to detect layouts which came from packed structures in order to provide a conservative estimate of alignment for it (since `DW_AT_alignment` isn't emitted unless explicitly changed with `alignas`, etc.). However, in the presence of overlapping fields due to `no_unique_address`, `InferAlignment` would set the alignment of structures to `1` for which that's incorrect. This poses issues in some LLDB formatters that synthesize new Clang types and rely on the layout builder to get the `FieldOffset` of structures right that we did have DWARF offsets for. The result of this is that if we get the alignment wrong, LLDB reads out garbage data from the wrong field offsets. There are a couple of solutions to this that we considered: 1. Make LLDB formatters not do the above, and make them more robust to inaccurate alignment. 2. Remove `InferAlignment` entirely and rely on Clang emitting `DW_AT_alignment` for packed structures. 3. Remove `InferAlignment` and detect packedness from within LLDB. 4. Make the `InferAlignment` logic account for overlapping fields. Option (1) turned out quite hairy and it's not clear we can achieve this with the tools available for certain STL formatters (particularly `std::map`). But I would still very much like to simplify this if we can. Option (2) wouldn't help with GCC-compiled binaries, and if we can get away with LLDB not needing the alignment, then we wouldn't need to increase debug-info size. Option (3), AFAICT, would require us to reimplement some of the layout logic in the layout builder. Would be great if we can avoid this added complexity. Option (4) seemed like the best option in the interim. As part of this change I also removed one
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Allow transparent lookup through anonymous namespaces (PR #97275)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/97275 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Allow transparent lookup through anonymous namespaces (PR #97275)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97275
>From f6c801efec331a832f2f10386be9cc14c8bb9565 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Mon, 1 Jul 2024 11:46:37 +0200
Subject: [PATCH 1/2] [lldb][TypeSystemClang] Allow transparent lookup through
anonymous namespaces
This patch allows expressions to reference entities in
anonymous namespaces. Previously this would have resulted in:
```
(lldb) expr foo::FooAnonymousVar
error: :1:6: no member named 'FooAnonymousVar' in namespace
'foo'
1 | foo::FooAnonymousVar
| ~^
```
We already allow such lookups through inline namespaces, and for
the purposes of lookup, anonymous namespaces shouldn't behave
any different.
Fixes https://github.com/llvm/llvm-project/issues/96963.
---
.../TypeSystem/Clang/TypeSystemClang.cpp | 18 ++
.../API/lang/cpp/namespace/TestNamespace.py | 5 +
lldb/test/API/lang/cpp/namespace/main.cpp | 19 ++-
3 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index cd1c500d9aa29..093d27a92d718 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -9484,14 +9484,24 @@ bool TypeSystemClang::DeclContextIsContainedInLookup(
auto *decl_ctx = (clang::DeclContext *)opaque_decl_ctx;
auto *other = (clang::DeclContext *)other_opaque_decl_ctx;
+ // If we have an inline or anonymous namespace, then the lookup of the
+ // parent context also includes those namespace contents.
+ auto is_transparent_lookup_allowed = [](clang::DeclContext *DC) {
+if (DC->isInlineNamespace())
+ return true;
+
+if (auto const *NS = dyn_cast(DC))
+ return NS->isAnonymousNamespace();
+
+return false;
+ };
+
do {
// A decl context always includes its own contents in its lookup.
if (decl_ctx == other)
return true;
-
-// If we have an inline namespace, then the lookup of the parent context
-// also includes the inline namespace contents.
- } while (other->isInlineNamespace() && (other = other->getParent()));
+ } while (is_transparent_lookup_allowed(other) &&
+ (other = other->getParent()));
return false;
}
diff --git a/lldb/test/API/lang/cpp/namespace/TestNamespace.py
b/lldb/test/API/lang/cpp/namespace/TestNamespace.py
index d747e2be77c8e..83bfe173658cc 100644
--- a/lldb/test/API/lang/cpp/namespace/TestNamespace.py
+++ b/lldb/test/API/lang/cpp/namespace/TestNamespace.py
@@ -253,3 +253,8 @@ def test_with_run_command(self):
self.expect_expr(
"((::B::Bar*)&::B::bar)->x()", result_type="int", result_value="42"
)
+
+self.expect_expr("InAnon1::var_in_anon", result_type="int",
result_value="10")
+self.expect_expr("InAnon1::InAnon2::var_in_anon", result_type="int",
result_value="5")
+self.expect_expr("InAnon1::inline_ns::var_in_anon", result_type="int",
result_value="15")
+self.expect_expr("InAnon1::inline_ns::InAnon2::var_in_anon",
result_type="int", result_value="5")
diff --git a/lldb/test/API/lang/cpp/namespace/main.cpp
b/lldb/test/API/lang/cpp/namespace/main.cpp
index 6a8efa160766b..2edfab8437639 100644
--- a/lldb/test/API/lang/cpp/namespace/main.cpp
+++ b/lldb/test/API/lang/cpp/namespace/main.cpp
@@ -127,6 +127,22 @@ struct Foo {
};
} // namespace NS2
+namespace {
+namespace InAnon1 {
+int var_in_anon = 10;
+namespace {
+inline namespace inline_ns {
+int var_in_anon = 15;
+namespace InAnon2 {
+namespace {
+int var_in_anon = 5;
+} // namespace
+} // namespace InAnon2
+} // namespace inline_ns
+} // namespace
+} // namespace InAnon1
+} // namespace
+
int
main (int argc, char const *argv[])
{
@@ -140,5 +156,6 @@ main (int argc, char const *argv[])
::B::Bar bb;
A::B::Bar ab;
return Foo::myfunc(12) + bb.x() + ab.y() + NS1::NS2::Foo{}.bar() +
- NS2::Foo{}.bar();
+ NS2::Foo{}.bar() + InAnon1::var_in_anon +
+ InAnon1::InAnon2::var_in_anon + InAnon1::inline_ns::var_in_anon;
}
>From 27734b12297e268905f87573c90077f2f616ea70 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Mon, 1 Jul 2024 12:19:03 +0200
Subject: [PATCH 2/2] fixup! python format
---
lldb/test/API/lang/cpp/namespace/TestNamespace.py | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/lldb/test/API/lang/cpp/namespace/TestNamespace.py
b/lldb/test/API/lang/cpp/namespace/TestNamespace.py
index 83bfe173658cc..84891b322180c 100644
--- a/lldb/test/API/lang/cpp/namespace/TestNamespace.py
+++ b/lldb/test/API/lang/cpp/namespace/TestNamespace.py
@@ -255,6 +255,14 @@ def test_with_run_command(self):
)
self.expect_expr("InAnon1::var_in_anon", result_type="int",
result_value="10")
-self.expect_expr("InAnon1::InAnon2::var_in_anon", result_type="int",
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Allow transparent lookup through anonymous namespaces (PR #97275)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/97275
This patch allows expressions to reference entities in anonymous namespaces.
Previously this would have resulted in:
```
(lldb) expr foo::FooAnonymousVar
error: :1:6: no member named 'FooAnonymousVar' in namespace
'foo'
1 | foo::FooAnonymousVar
| ~^
```
We already allow such lookups through inline namespaces, and for the purposes
of lookup, anonymous namespaces shouldn't behave any different.
Fixes https://github.com/llvm/llvm-project/issues/96963.
>From f6c801efec331a832f2f10386be9cc14c8bb9565 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Mon, 1 Jul 2024 11:46:37 +0200
Subject: [PATCH] [lldb][TypeSystemClang] Allow transparent lookup through
anonymous namespaces
This patch allows expressions to reference entities in
anonymous namespaces. Previously this would have resulted in:
```
(lldb) expr foo::FooAnonymousVar
error: :1:6: no member named 'FooAnonymousVar' in namespace
'foo'
1 | foo::FooAnonymousVar
| ~^
```
We already allow such lookups through inline namespaces, and for
the purposes of lookup, anonymous namespaces shouldn't behave
any different.
Fixes https://github.com/llvm/llvm-project/issues/96963.
---
.../TypeSystem/Clang/TypeSystemClang.cpp | 18 ++
.../API/lang/cpp/namespace/TestNamespace.py | 5 +
lldb/test/API/lang/cpp/namespace/main.cpp | 19 ++-
3 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index cd1c500d9aa29..093d27a92d718 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -9484,14 +9484,24 @@ bool TypeSystemClang::DeclContextIsContainedInLookup(
auto *decl_ctx = (clang::DeclContext *)opaque_decl_ctx;
auto *other = (clang::DeclContext *)other_opaque_decl_ctx;
+ // If we have an inline or anonymous namespace, then the lookup of the
+ // parent context also includes those namespace contents.
+ auto is_transparent_lookup_allowed = [](clang::DeclContext *DC) {
+if (DC->isInlineNamespace())
+ return true;
+
+if (auto const *NS = dyn_cast(DC))
+ return NS->isAnonymousNamespace();
+
+return false;
+ };
+
do {
// A decl context always includes its own contents in its lookup.
if (decl_ctx == other)
return true;
-
-// If we have an inline namespace, then the lookup of the parent context
-// also includes the inline namespace contents.
- } while (other->isInlineNamespace() && (other = other->getParent()));
+ } while (is_transparent_lookup_allowed(other) &&
+ (other = other->getParent()));
return false;
}
diff --git a/lldb/test/API/lang/cpp/namespace/TestNamespace.py
b/lldb/test/API/lang/cpp/namespace/TestNamespace.py
index d747e2be77c8e..83bfe173658cc 100644
--- a/lldb/test/API/lang/cpp/namespace/TestNamespace.py
+++ b/lldb/test/API/lang/cpp/namespace/TestNamespace.py
@@ -253,3 +253,8 @@ def test_with_run_command(self):
self.expect_expr(
"((::B::Bar*)&::B::bar)->x()", result_type="int", result_value="42"
)
+
+self.expect_expr("InAnon1::var_in_anon", result_type="int",
result_value="10")
+self.expect_expr("InAnon1::InAnon2::var_in_anon", result_type="int",
result_value="5")
+self.expect_expr("InAnon1::inline_ns::var_in_anon", result_type="int",
result_value="15")
+self.expect_expr("InAnon1::inline_ns::InAnon2::var_in_anon",
result_type="int", result_value="5")
diff --git a/lldb/test/API/lang/cpp/namespace/main.cpp
b/lldb/test/API/lang/cpp/namespace/main.cpp
index 6a8efa160766b..2edfab8437639 100644
--- a/lldb/test/API/lang/cpp/namespace/main.cpp
+++ b/lldb/test/API/lang/cpp/namespace/main.cpp
@@ -127,6 +127,22 @@ struct Foo {
};
} // namespace NS2
+namespace {
+namespace InAnon1 {
+int var_in_anon = 10;
+namespace {
+inline namespace inline_ns {
+int var_in_anon = 15;
+namespace InAnon2 {
+namespace {
+int var_in_anon = 5;
+} // namespace
+} // namespace InAnon2
+} // namespace inline_ns
+} // namespace
+} // namespace InAnon1
+} // namespace
+
int
main (int argc, char const *argv[])
{
@@ -140,5 +156,6 @@ main (int argc, char const *argv[])
::B::Bar bb;
A::B::Bar ab;
return Foo::myfunc(12) + bb.x() + ab.y() + NS1::NS2::Foo{}.bar() +
- NS2::Foo{}.bar();
+ NS2::Foo{}.bar() + InAnon1::var_in_anon +
+ InAnon1::InAnon2::var_in_anon + InAnon1::inline_ns::var_in_anon;
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Add tests for alignof on class with overlapping bases (PR #97068)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/97068 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Add tests for alignof on class with overlapping bases (PR #97068)
Michael137 wrote: Only test failure on buildkite is `TestConcurrentVFork.py`, which is unrelated. Merging https://github.com/llvm/llvm-project/pull/97068 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Remove duplicate testcase names in API test-suite (PR #97043)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/97043 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Add tests for alignof on class with overlapping bases (PR #97068)
@@ -0,0 +1,30 @@
+// XFAIL: *
+
+// RUN: %clangxx_host -gdwarf -o %t %s
+// RUN: %lldb %t \
+// RUN: -o "expr alignof(OverlappingDerived)" \
+// RUN: -o "expr sizeof(OverlappingDerived)" \
+// RUN: -o exit | FileCheck %s
+
+// CHECK: (lldb) expr alignof(OverlappingDerived)
+// CHECK-NEXT: ${{.*}} = 4
Michael137 wrote:
good point, done
https://github.com/llvm/llvm-project/pull/97068
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Add tests for alignof on class with overlapping bases (PR #97068)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97068
>From 1c924c866cc2de5e9e1d84ce0b185e9dacefd4ec Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Fri, 28 Jun 2024 15:29:54 +0100
Subject: [PATCH 1/2] [lldb][test] Add tests for alignof on class with
overlapping bases
Follow-up to https://github.com/llvm/llvm-project/pull/96932
Adds XFAILed test where LLDB incorrectly infers the
alignment of a derived class whose bases are overlapping
due to [[no_unique_address]]. Specifically, the `InferAlignment`
code-path of the `ItaniumRecordLayoutBuilder` assumes that overlapping
base offsets imply a packed structure and thus sets alignment
to 1. See discussion in https://github.com/llvm/llvm-project/pull/93809.
Also adds test where LLDB correctly infers an alignment
of `1` when a packed base class is overlapping with other bases.
Lastly, there were a couple of `alignof` inconsistencies which
I encapsulated in an XFAIL-ed `packed-alignof.cpp`.
---
.../no_unique_address-base-alignment.cpp | 30 ++
.../Shell/SymbolFile/DWARF/packed-alignof.cpp | 41 +++
lldb/test/Shell/SymbolFile/DWARF/packed.cpp | 14 +++
3 files changed, 85 insertions(+)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/no_unique_address-base-alignment.cpp
create mode 100644 lldb/test/Shell/SymbolFile/DWARF/packed-alignof.cpp
diff --git
a/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-base-alignment.cpp
b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-base-alignment.cpp
new file mode 100644
index 0..634d461e6ff19
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-base-alignment.cpp
@@ -0,0 +1,30 @@
+// XFAIL: *
+
+// RUN: %clangxx_host -gdwarf -o %t %s
+// RUN: %lldb %t \
+// RUN: -o "expr alignof(OverlappingDerived)" \
+// RUN: -o "expr sizeof(OverlappingDerived)" \
+// RUN: -o exit | FileCheck %s
+
+// CHECK: (lldb) expr alignof(OverlappingDerived)
+// CHECK-NEXT: ${{.*}} = 4
+// CHECK: (lldb) expr sizeof(OverlappingDerived)
+// CHECK-NEXT: ${{.*}} = 4
+
+struct Empty {};
+
+struct OverlappingBase {
+ [[no_unique_address]] Empty e;
+};
+static_assert(sizeof(OverlappingBase) == 1);
+static_assert(alignof(OverlappingBase) == 1);
+
+struct Base {
+ int mem;
+};
+
+struct OverlappingDerived : Base, OverlappingBase {};
+static_assert(alignof(OverlappingDerived) == 4);
+static_assert(sizeof(OverlappingDerived) == 4);
+
+int main() { OverlappingDerived d; }
diff --git a/lldb/test/Shell/SymbolFile/DWARF/packed-alignof.cpp
b/lldb/test/Shell/SymbolFile/DWARF/packed-alignof.cpp
new file mode 100644
index 0..a15e88f0b65df
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/packed-alignof.cpp
@@ -0,0 +1,41 @@
+// XFAIL: *
+//
+// RUN: %clangxx_host -gdwarf -o %t %s
+// RUN: %lldb %t \
+// RUN: -o "expr alignof(base)" \
+// RUN: -o "expr alignof(packed_base)" \
+// RUN: -o "expr alignof(derived)" \
+// RUN: -o "expr sizeof(derived)" \
+// RUN: -o exit | FileCheck %s
+
+// CHECK: (lldb) expr alignof(base)
+// CHECK-NEXT: ${{.*}} = 4
+// CHECK: (lldb) expr alignof(packed_base)
+// CHECK-NEXT: ${{.*}} = 1
+// CHECK: (lldb) expr alignof(derived)
+// CHECK-NEXT: ${{.*}} = 2
+// CHECK: (lldb) expr sizeof(derived)
+// CHECK-NEXT: ${{.*}} = 16
+
+struct __attribute__((packed)) packed {
+ int x;
+ char y;
+ int z;
+} g_packed_struct;
+
+// LLDB incorrectly calculates alignof(base)
+struct foo {};
+struct base : foo { int x; };
+static_assert(alignof(base) == 4);
+
+// LLDB incorrectly calculates alignof(packed_base)
+struct __attribute__((packed)) packed_base { int x; };
+static_assert(alignof(packed_base) == 1);
+
+struct derived : packed, packed_base {
+ short s;
+} g_derived;
+static_assert(alignof(derived) == 2);
+static_assert(sizeof(derived) == 16);
+
+int main() {}
diff --git a/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
b/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
index 640a2e0454c92..135808f46d7ea 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
@@ -4,6 +4,8 @@
// RUN: -o "expr sizeof(packed)" \
// RUN: -o "expr alignof(packed_and_aligned)" \
// RUN: -o "expr sizeof(packed_and_aligned)" \
+// RUN: -o "expr alignof(derived)" \
+// RUN: -o "expr sizeof(derived)" \
// RUN: -o exit | FileCheck %s
// CHECK: (lldb) expr alignof(packed)
@@ -16,6 +18,11 @@
// CHECK: (lldb) expr sizeof(packed_and_aligned)
// CHECK-NEXT: ${{.*}} = 16
+// CHECK: (lldb) expr alignof(derived)
+// CHECK-NEXT: ${{.*}} = 1
+// CHECK: (lldb) expr sizeof(derived)
+// CHECK-NEXT: ${{.*}} = 13
+
struct __attribute__((packed)) packed {
int x;
char y;
@@ -32,4 +39,11 @@ struct __attribute__((packed, aligned(16)))
packed_and_aligned {
static_assert(alignof(packed_and_aligned) == 16);
static_assert(sizeof(packed_and_aligned) == 16);
+struct __attribute__((packed)) packed_base { int x; };
[Lldb-commits] [lldb] [lldb][test] Remove duplicate testcase names in API test-suite (PR #97043)
Michael137 wrote: > LGTM. This has definitely come up in the past. If you feel motivated, I'm > sure there must be a way to detect this issue in Python and we could have > assert/warning/error that captures this at the dotest level. Agreed, making it part of `dotest` would be amazing. Maybe someone with better python knowledge has some ideas (@medismailben @kastiglione ?). In the meantime I'll have a think of how one might do that https://github.com/llvm/llvm-project/pull/97043 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Add tests for alignof on class with overlapping bases (PR #97068)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/97068
Follow-up to https://github.com/llvm/llvm-project/pull/96932
Adds XFAILed test where LLDB incorrectly infers the alignment of a derived
class whose bases are overlapping due to [[no_unique_address]]. Specifically,
the `InferAlignment` code-path of the `ItaniumRecordLayoutBuilder` assumes that
overlapping base offsets imply a packed structure and thus sets alignment to 1.
See discussion in https://github.com/llvm/llvm-project/pull/93809.
Also adds test where LLDB correctly infers an alignment of `1` when a packed
base class is overlapping with other bases.
Lastly, there were a couple of `alignof` inconsistencies which I encapsulated
in an XFAIL-ed `packed-alignof.cpp`.
>From 1c924c866cc2de5e9e1d84ce0b185e9dacefd4ec Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Fri, 28 Jun 2024 15:29:54 +0100
Subject: [PATCH] [lldb][test] Add tests for alignof on class with overlapping
bases
Follow-up to https://github.com/llvm/llvm-project/pull/96932
Adds XFAILed test where LLDB incorrectly infers the
alignment of a derived class whose bases are overlapping
due to [[no_unique_address]]. Specifically, the `InferAlignment`
code-path of the `ItaniumRecordLayoutBuilder` assumes that overlapping
base offsets imply a packed structure and thus sets alignment
to 1. See discussion in https://github.com/llvm/llvm-project/pull/93809.
Also adds test where LLDB correctly infers an alignment
of `1` when a packed base class is overlapping with other bases.
Lastly, there were a couple of `alignof` inconsistencies which
I encapsulated in an XFAIL-ed `packed-alignof.cpp`.
---
.../no_unique_address-base-alignment.cpp | 30 ++
.../Shell/SymbolFile/DWARF/packed-alignof.cpp | 41 +++
lldb/test/Shell/SymbolFile/DWARF/packed.cpp | 14 +++
3 files changed, 85 insertions(+)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/no_unique_address-base-alignment.cpp
create mode 100644 lldb/test/Shell/SymbolFile/DWARF/packed-alignof.cpp
diff --git
a/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-base-alignment.cpp
b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-base-alignment.cpp
new file mode 100644
index 0..634d461e6ff19
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-base-alignment.cpp
@@ -0,0 +1,30 @@
+// XFAIL: *
+
+// RUN: %clangxx_host -gdwarf -o %t %s
+// RUN: %lldb %t \
+// RUN: -o "expr alignof(OverlappingDerived)" \
+// RUN: -o "expr sizeof(OverlappingDerived)" \
+// RUN: -o exit | FileCheck %s
+
+// CHECK: (lldb) expr alignof(OverlappingDerived)
+// CHECK-NEXT: ${{.*}} = 4
+// CHECK: (lldb) expr sizeof(OverlappingDerived)
+// CHECK-NEXT: ${{.*}} = 4
+
+struct Empty {};
+
+struct OverlappingBase {
+ [[no_unique_address]] Empty e;
+};
+static_assert(sizeof(OverlappingBase) == 1);
+static_assert(alignof(OverlappingBase) == 1);
+
+struct Base {
+ int mem;
+};
+
+struct OverlappingDerived : Base, OverlappingBase {};
+static_assert(alignof(OverlappingDerived) == 4);
+static_assert(sizeof(OverlappingDerived) == 4);
+
+int main() { OverlappingDerived d; }
diff --git a/lldb/test/Shell/SymbolFile/DWARF/packed-alignof.cpp
b/lldb/test/Shell/SymbolFile/DWARF/packed-alignof.cpp
new file mode 100644
index 0..a15e88f0b65df
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/packed-alignof.cpp
@@ -0,0 +1,41 @@
+// XFAIL: *
+//
+// RUN: %clangxx_host -gdwarf -o %t %s
+// RUN: %lldb %t \
+// RUN: -o "expr alignof(base)" \
+// RUN: -o "expr alignof(packed_base)" \
+// RUN: -o "expr alignof(derived)" \
+// RUN: -o "expr sizeof(derived)" \
+// RUN: -o exit | FileCheck %s
+
+// CHECK: (lldb) expr alignof(base)
+// CHECK-NEXT: ${{.*}} = 4
+// CHECK: (lldb) expr alignof(packed_base)
+// CHECK-NEXT: ${{.*}} = 1
+// CHECK: (lldb) expr alignof(derived)
+// CHECK-NEXT: ${{.*}} = 2
+// CHECK: (lldb) expr sizeof(derived)
+// CHECK-NEXT: ${{.*}} = 16
+
+struct __attribute__((packed)) packed {
+ int x;
+ char y;
+ int z;
+} g_packed_struct;
+
+// LLDB incorrectly calculates alignof(base)
+struct foo {};
+struct base : foo { int x; };
+static_assert(alignof(base) == 4);
+
+// LLDB incorrectly calculates alignof(packed_base)
+struct __attribute__((packed)) packed_base { int x; };
+static_assert(alignof(packed_base) == 1);
+
+struct derived : packed, packed_base {
+ short s;
+} g_derived;
+static_assert(alignof(derived) == 2);
+static_assert(sizeof(derived) == 16);
+
+int main() {}
diff --git a/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
b/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
index 640a2e0454c92..135808f46d7ea 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
@@ -4,6 +4,8 @@
// RUN: -o "expr sizeof(packed)" \
// RUN: -o "expr alignof(packed_and_aligned)" \
// RUN: -o "expr sizeof(packed_and_aligned)" \
+// RUN: -o "expr alignof(derived)" \
+//
[Lldb-commits] [lldb] [lldb][test] Add test-cases for packed/aligned structures (PR #96932)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/96932 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 1e01e0c - [lldb][test][NFC] TestWatchpointConditionCmd.py: remove BOM character
Author: Michael Buch Date: 2024-06-28T14:10:08+01:00 New Revision: 1e01e0c19ace6c9b165ee0cbbcd24ab55d27d8e0 URL: https://github.com/llvm/llvm-project/commit/1e01e0c19ace6c9b165ee0cbbcd24ab55d27d8e0 DIFF: https://github.com/llvm/llvm-project/commit/1e01e0c19ace6c9b165ee0cbbcd24ab55d27d8e0.diff LOG: [lldb][test][NFC] TestWatchpointConditionCmd.py: remove BOM character Missed this file in https://github.com/llvm/llvm-project/pull/97045 Added: Modified: lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py Removed: diff --git a/lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py b/lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py index 9e8db7025219d..3a60859b9ce7d 100644 --- a/lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py +++ b/lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py @@ -1,4 +1,4 @@ -""" +""" Test watchpoint modify command to set condition on a watchpoint. """ ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test][NFC] Remove BOM characters from tests (PR #97045)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/97045 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test][NFC] Remove BOM characters from tests (PR #97045)
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/97045 These handful of tests had a BOM (Byte order mark) at the beginning of the file. This marker is unnecessary in our test files. The main motivation for this is that the `ast` python module breaks when passing a file to it with a BOM marker (and might break other tooling which doesn't expect it). E.g.,: ``` """Test that lldb command 'process signal SIGUSR1' to send a signal to the inferior works.""" ^ SyntaxError: invalid non-printable character U+FEFF ``` If anyone is aware of a good reason to keep it, happy to drop this. >From 51f45aca3cdd36c2183fc3ebd2da3e199e048cdd Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 28 Jun 2024 13:09:32 +0100 Subject: [PATCH] [lldb][test][NFC] Remove BOM characters from tests These handful of tests had a BOM (Byte order mark) at the beginning of the file. This marker is unnecessary in our test files. The main motivation for this is that the `ast` python module breaks when passing a file to it with a BOM marker (and might break other tooling which doesn't expect it). E.g.,: ``` """Test that lldb command 'process signal SIGUSR1' to send a signal to the inferior works.""" ^ SyntaxError: invalid non-printable character U+FEFF ``` If anyone is aware of a good reason to keep it, happy to drop this. --- .../commands/expression/call-restarts/TestCallThatRestarts.py | 2 +- .../test/API/functionalities/load_lazy/TestLoadUsingLazyBind.py | 2 +- lldb/test/API/functionalities/load_unload/TestLoadUnload.py | 2 +- .../API/functionalities/load_using_paths/TestLoadUsingPaths.py | 2 +- .../location-list-lookup/TestLocationListLookup.py | 2 +- lldb/test/API/functionalities/signal/TestSendSignal.py | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lldb/test/API/commands/expression/call-restarts/TestCallThatRestarts.py b/lldb/test/API/commands/expression/call-restarts/TestCallThatRestarts.py index ca08591aedb39..a108ffe485efc 100644 --- a/lldb/test/API/commands/expression/call-restarts/TestCallThatRestarts.py +++ b/lldb/test/API/commands/expression/call-restarts/TestCallThatRestarts.py @@ -1,4 +1,4 @@ -""" +""" Test calling a function that hits a signal set to auto-restart, make sure the call completes. """ diff --git a/lldb/test/API/functionalities/load_lazy/TestLoadUsingLazyBind.py b/lldb/test/API/functionalities/load_lazy/TestLoadUsingLazyBind.py index 2783aaff1a0ed..0fa7bc7edf995 100644 --- a/lldb/test/API/functionalities/load_lazy/TestLoadUsingLazyBind.py +++ b/lldb/test/API/functionalities/load_lazy/TestLoadUsingLazyBind.py @@ -1,4 +1,4 @@ -""" +""" Test that SBProcess.LoadImageUsingPaths uses RTLD_LAZY """ diff --git a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py index 2208e520f1d56..e52fb8c87377f 100644 --- a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py +++ b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py @@ -1,4 +1,4 @@ -""" +""" Test that breakpoint by symbol name works correctly with dynamic libs. """ diff --git a/lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py b/lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py index 6ee99f84adda8..c423236b219f4 100644 --- a/lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py +++ b/lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py @@ -1,4 +1,4 @@ -""" +""" Test that SBProcess.LoadImageUsingPaths works correctly. """ diff --git a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py index c5f4a7c6dedc1..84033daff7730 100644 --- a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py +++ b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py @@ -1,4 +1,4 @@ -"""Test that lldb picks the correct DWARF location list entry with a return-pc out of bounds.""" +"""Test that lldb picks the correct DWARF location list entry with a return-pc out of bounds.""" import lldb from lldbsuite.test.decorators import * diff --git a/lldb/test/API/functionalities/signal/TestSendSignal.py b/lldb/test/API/functionalities/signal/TestSendSignal.py index 50435572c4d83..5b6a50a461cdf 100644 --- a/lldb/test/API/functionalities/signal/TestSendSignal.py +++ b/lldb/test/API/functionalities/signal/TestSendSignal.py @@ -1,4 +1,4 @@ -"""Test that lldb command 'process signal SIGUSR1' to send a signal to the inferior works.""" +"""Test that lldb command 'process signal SIGUSR1' to send a signal to the inferior works.""" import lldb ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Remove duplicate testcase names in API test-suite (PR #97043)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/97043
>From 0c1f5c2240c64cfd69afcf819c69cca1270e5d8a Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Fri, 28 Jun 2024 12:49:55 +0100
Subject: [PATCH 1/2] [lldb][test] Remove duplicate testcase names in API
test-suite
In one of my recent PRs I mistakenly had two test-cases with
the same name, preventing one of them to run. Since it's an
easy mistake to make (e.g., copy pasting existing test-cases),
I ran following sanity-check script over `lldb/test/API`, which
found couple of tests which were losing coverage because of
this (or in some cases simply had duplicate tests):
```
import ast
import sys
filename = sys.argv[1]
print(f'Checking {filename}...')
tree = ast.parse(open(filename, 'r').read())
for node in ast.walk(tree):
if not isinstance(node, ast.ClassDef):
continue
func_names = []
for child in ast.iter_child_nodes(node):
if isinstance(child, ast.FunctionDef):
func_names.append(child.name)
seen_func_names = set()
duplicate_func_names = []
for name in func_names:
if name in seen_func_names:
duplicate_func_names.append(name)
else:
seen_func_names.add(name)
if len(duplicate_func_names) != 0:
print(f'Multiple func names found:\n\t{duplicate_func_names}\n\tclass
{node.name}\n\tfile: {filename}')
```
This patch fixes these cases.
---
.../log/invalid-args/TestInvalidArgsLog.py| 2 +-
.../backtrace/TestThreadBacktraceRepeat.py| 2 -
.../TestTraceStartStopMultipleThreads.py | 37 ---
.../completion/TestCompletion.py | 6 ---
.../gdb_remote_client/TestIOSSimulator.py | 2 +-
.../python_os_plugin/TestPythonOSPlugin.py| 2 +-
.../API/lang/cpp/diamond/TestCppDiamond.py| 2 +-
.../TestMembersAndLocalsWithSameName.py | 1 -
.../cxx-bridged-po/TestObjCXXBridgedPO.py | 2 +-
.../rust/enum-structs/TestRustEnumStructs.py | 2 +-
lldb/test/API/test_utils/TestDecorators.py| 4 +-
11 files changed, 8 insertions(+), 54 deletions(-)
diff --git a/lldb/test/API/commands/log/invalid-args/TestInvalidArgsLog.py
b/lldb/test/API/commands/log/invalid-args/TestInvalidArgsLog.py
index 583c68d7bfacc..dbcd2d60d021a 100644
--- a/lldb/test/API/commands/log/invalid-args/TestInvalidArgsLog.py
+++ b/lldb/test/API/commands/log/invalid-args/TestInvalidArgsLog.py
@@ -25,7 +25,7 @@ def test_disable_empty(self):
)
@no_debug_info_test
-def test_enable_empty(self):
+def test_enable_invalid_path(self):
invalid_path = os.path.join("this", "is", "not", "a", "valid", "path")
self.expect(
"log enable lldb all -f " + invalid_path,
diff --git
a/lldb/test/API/commands/thread/backtrace/TestThreadBacktraceRepeat.py
b/lldb/test/API/commands/thread/backtrace/TestThreadBacktraceRepeat.py
index 9678bd42999b3..571024417560f 100644
--- a/lldb/test/API/commands/thread/backtrace/TestThreadBacktraceRepeat.py
+++ b/lldb/test/API/commands/thread/backtrace/TestThreadBacktraceRepeat.py
@@ -16,8 +16,6 @@ def test_thread_backtrace_one_thread(self):
"""Run a simplified version of the test that just hits one breakpoint
and
doesn't care about synchronizing the two threads - hopefully this will
run on more systems."""
-
-def test_thread_backtrace_one_thread(self):
self.build()
(
self.inferior_target,
diff --git
a/lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
b/lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
index c41e85fd670ba..12f99f07c78a8 100644
---
a/lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
+++
b/lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
@@ -31,43 +31,6 @@ def testStartMultipleLiveThreads(self):
self.traceStopProcess()
-@skipIf(oslist=no_match(["linux"]), archs=no_match(["i386", "x86_64"]))
-@testSBAPIAndCommands
-def testStartMultipleLiveThreadsWithStops(self):
-self.build()
-exe = self.getBuildArtifact("a.out")
-
-self.dbg.CreateTarget(exe)
-
-self.expect("b main")
-self.expect("b 6")
-self.expect("b 11")
-
-self.expect("r")
-self.traceStartProcess()
-
-# We'll see here the first thread
-self.expect("continue")
-
-# We are in thread 2
-self.expect("thread trace dump instructions", substrs=["main.cpp:9"])
-self.expect("thread trace dump instructions 2", substrs=["main.cpp:9"])
-
-# We stop tracing it
-self.expect("thread trace stop 2")
-
-# The trace is still in memory
-self.expect("thread trace dump instructions 2", substrs=["main.cpp:9"])
-
-# We'll stop at the next breakpoint, thread 2 will be still alive, but
not traced. Thread 3 will be traced
-
[Lldb-commits] [lldb] [lldb][test] Remove duplicate testcase names in API test-suite (PR #97043)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/97043
In one of my recent PRs I mistakenly had two test-cases with the same name,
preventing one of them to run. Since it's an easy mistake to make (e.g., copy
pasting existing test-cases), I ran following sanity-check script over
`lldb/test/API`, which found couple of tests which were losing coverage because
of this (or in some cases simply had duplicate tests):
```
import ast
import sys
filename = sys.argv[1]
print(f'Checking {filename}...')
tree = ast.parse(open(filename, 'r').read())
for node in ast.walk(tree):
if not isinstance(node, ast.ClassDef):
continue
func_names = []
for child in ast.iter_child_nodes(node):
if isinstance(child, ast.FunctionDef):
func_names.append(child.name)
seen_func_names = set()
duplicate_func_names = []
for name in func_names:
if name in seen_func_names:
duplicate_func_names.append(name)
else:
seen_func_names.add(name)
if len(duplicate_func_names) != 0:
print(f'Multiple func names found:\n\t{duplicate_func_names}\n\tclass
{node.name}\n\tfile: {filename}')
```
This patch fixes these cases.
>From 0c1f5c2240c64cfd69afcf819c69cca1270e5d8a Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Fri, 28 Jun 2024 12:49:55 +0100
Subject: [PATCH] [lldb][test] Remove duplicate testcase names in API
test-suite
In one of my recent PRs I mistakenly had two test-cases with
the same name, preventing one of them to run. Since it's an
easy mistake to make (e.g., copy pasting existing test-cases),
I ran following sanity-check script over `lldb/test/API`, which
found couple of tests which were losing coverage because of
this (or in some cases simply had duplicate tests):
```
import ast
import sys
filename = sys.argv[1]
print(f'Checking {filename}...')
tree = ast.parse(open(filename, 'r').read())
for node in ast.walk(tree):
if not isinstance(node, ast.ClassDef):
continue
func_names = []
for child in ast.iter_child_nodes(node):
if isinstance(child, ast.FunctionDef):
func_names.append(child.name)
seen_func_names = set()
duplicate_func_names = []
for name in func_names:
if name in seen_func_names:
duplicate_func_names.append(name)
else:
seen_func_names.add(name)
if len(duplicate_func_names) != 0:
print(f'Multiple func names found:\n\t{duplicate_func_names}\n\tclass
{node.name}\n\tfile: {filename}')
```
This patch fixes these cases.
---
.../log/invalid-args/TestInvalidArgsLog.py| 2 +-
.../backtrace/TestThreadBacktraceRepeat.py| 2 -
.../TestTraceStartStopMultipleThreads.py | 37 ---
.../completion/TestCompletion.py | 6 ---
.../gdb_remote_client/TestIOSSimulator.py | 2 +-
.../python_os_plugin/TestPythonOSPlugin.py| 2 +-
.../API/lang/cpp/diamond/TestCppDiamond.py| 2 +-
.../TestMembersAndLocalsWithSameName.py | 1 -
.../cxx-bridged-po/TestObjCXXBridgedPO.py | 2 +-
.../rust/enum-structs/TestRustEnumStructs.py | 2 +-
lldb/test/API/test_utils/TestDecorators.py| 4 +-
11 files changed, 8 insertions(+), 54 deletions(-)
diff --git a/lldb/test/API/commands/log/invalid-args/TestInvalidArgsLog.py
b/lldb/test/API/commands/log/invalid-args/TestInvalidArgsLog.py
index 583c68d7bfacc..dbcd2d60d021a 100644
--- a/lldb/test/API/commands/log/invalid-args/TestInvalidArgsLog.py
+++ b/lldb/test/API/commands/log/invalid-args/TestInvalidArgsLog.py
@@ -25,7 +25,7 @@ def test_disable_empty(self):
)
@no_debug_info_test
-def test_enable_empty(self):
+def test_enable_invalid_path(self):
invalid_path = os.path.join("this", "is", "not", "a", "valid", "path")
self.expect(
"log enable lldb all -f " + invalid_path,
diff --git
a/lldb/test/API/commands/thread/backtrace/TestThreadBacktraceRepeat.py
b/lldb/test/API/commands/thread/backtrace/TestThreadBacktraceRepeat.py
index 9678bd42999b3..571024417560f 100644
--- a/lldb/test/API/commands/thread/backtrace/TestThreadBacktraceRepeat.py
+++ b/lldb/test/API/commands/thread/backtrace/TestThreadBacktraceRepeat.py
@@ -16,8 +16,6 @@ def test_thread_backtrace_one_thread(self):
"""Run a simplified version of the test that just hits one breakpoint
and
doesn't care about synchronizing the two threads - hopefully this will
run on more systems."""
-
-def test_thread_backtrace_one_thread(self):
self.build()
(
self.inferior_target,
diff --git
a/lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
b/lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
index c41e85fd670ba..12f99f07c78a8 100644
---
a/lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
+++
[Lldb-commits] [lldb] [lldb][test] Add test-cases for packed/aligned structures (PR #96932)
@@ -0,0 +1,25 @@ +// XFAIL: * + +// RUN: %clangxx_host -gdwarf -o %t %s +// RUN: %lldb %t \ +// RUN: -o "b main" \ Michael137 wrote: good catch, removed https://github.com/llvm/llvm-project/pull/96932 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Add test-cases for packed/aligned structures (PR #96932)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/96932
>From d2c28706769f89bf9f0b53071726bb59c6205ce8 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 27 Jun 2024 17:16:50 +0100
Subject: [PATCH 1/2] [lldb][test] Add test-cases for packed structures
Adds test that checks whether LLDB correctly infers the
alignment of packed structures. Specifically, the
`InferAlignment` code-path of the `ItaniumRecordLayoutBuilder`
where it assumes that overlapping field offsets imply a
packed structure and thus sets alignment to `1`. See discussion
in https://github.com/llvm/llvm-project/pull/93809.
While here, also added a test-case where we check alignment of
a class whose base has an explicit `DW_AT_alignment
(those don't get transitively propagated in DWARF, but don't seem
like a problem for LLDB).
Lastly, also added an XFAIL-ed tests where the aforementioned
`InferAlignment` kicks in for overlapping fields (but in this
case incorrectly since the structure isn't actually packed).
---
.../TestAlignAsBaseClass.py | 1 +
.../API/lang/cpp/alignas_base_class/main.cpp | 7
.../DWARF/no_unique_address-alignment.cpp | 25 +
lldb/test/Shell/SymbolFile/DWARF/packed.cpp | 36 +++
4 files changed, 69 insertions(+)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/no_unique_address-alignment.cpp
create mode 100644 lldb/test/Shell/SymbolFile/DWARF/packed.cpp
diff --git a/lldb/test/API/lang/cpp/alignas_base_class/TestAlignAsBaseClass.py
b/lldb/test/API/lang/cpp/alignas_base_class/TestAlignAsBaseClass.py
index 7d97b0c42b7e1..362fc2740bf52 100644
--- a/lldb/test/API/lang/cpp/alignas_base_class/TestAlignAsBaseClass.py
+++ b/lldb/test/API/lang/cpp/alignas_base_class/TestAlignAsBaseClass.py
@@ -16,3 +16,4 @@ def test(self):
# Verify specified class alignments.
self.expect_expr("alignof(B2)", result_value="8")
self.expect_expr("alignof(EmptyClassAlign8)", result_value="8")
+self.expect_expr("alignof(Derived)", result_value="8")
diff --git a/lldb/test/API/lang/cpp/alignas_base_class/main.cpp
b/lldb/test/API/lang/cpp/alignas_base_class/main.cpp
index 9d37554957ba3..a37919deaebdc 100644
--- a/lldb/test/API/lang/cpp/alignas_base_class/main.cpp
+++ b/lldb/test/API/lang/cpp/alignas_base_class/main.cpp
@@ -13,4 +13,11 @@ D d3g;
struct alignas(8) EmptyClassAlign8 {
} t;
+struct alignas(8) __attribute__((packed)) AlignedAndPackedBase {
+} foo;
+
+struct Derived : AlignedAndPackedBase {
+} bar;
+static_assert(alignof(Derived) == 8);
+
int main() {}
diff --git a/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-alignment.cpp
b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-alignment.cpp
new file mode 100644
index 0..6e544f40625df
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-alignment.cpp
@@ -0,0 +1,25 @@
+// XFAIL: *
+
+// RUN: %clangxx_host -gdwarf -o %t %s
+// RUN: %lldb %t \
+// RUN: -o "b main" \
+// RUN: -o "expr alignof(OverlappingFields)" \
+// RUN: -o "expr sizeof(OverlappingFields)" \
+// RUN: -o exit | FileCheck %s
+
+// CHECK: (lldb) expr alignof(OverlappingFields)
+// CHECK-NEXT: ${{.*}} = 4
+// CHECK: (lldb) expr sizeof(OverlappingFields)
+// CHECK-NEXT: ${{.*}} = 8
+
+struct Empty {};
+
+struct OverlappingFields {
+ char y;
+ [[no_unique_address]] Empty e;
+ int z;
+} g_packed_struct;
+static_assert(alignof(OverlappingFields) == 4);
+static_assert(sizeof(OverlappingFields) == 8);
+
+int main() {}
diff --git a/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
b/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
new file mode 100644
index 0..fb94a834d48a6
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
@@ -0,0 +1,36 @@
+// RUN: %clangxx_host -gdwarf -o %t %s
+// RUN: %lldb %t \
+// RUN: -o "b main" \
+// RUN: -o "expr alignof(packed)" \
+// RUN: -o "expr sizeof(packed)" \
+// RUN: -o "expr alignof(packed_and_aligned)" \
+// RUN: -o "expr sizeof(packed_and_aligned)" \
+// RUN: -o exit | FileCheck %s
+
+// CHECK: (lldb) expr alignof(packed)
+// CHECK-NEXT: ${{.*}} = 1
+// CHECK: (lldb) expr sizeof(packed)
+// CHECK-NEXT: ${{.*}} = 9
+
+// CHECK: (lldb) expr alignof(packed_and_aligned)
+// CHECK-NEXT: ${{.*}} = 16
+// CHECK: (lldb) expr sizeof(packed_and_aligned)
+// CHECK-NEXT: ${{.*}} = 16
+
+struct __attribute__((packed)) packed {
+ int x;
+ char y;
+ int z;
+} g_packed_struct;
+static_assert(alignof(packed) == 1);
+static_assert(sizeof(packed) == 9);
+
+struct __attribute__((packed, aligned(16))) packed_and_aligned {
+ int x;
+ char y;
+ int z;
+} g_packed_and_aligned_struct;
+static_assert(alignof(packed_and_aligned) == 16);
+static_assert(sizeof(packed_and_aligned) == 16);
+
+int main() {}
>From a7b91858c8f9e09b455a581469155152f2dea11e Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Fri, 28 Jun 2024 09:08:20 +0100
Subject: [PATCH 2/2] fixup! don't set redundant
[Lldb-commits] [lldb] [lldb][test] Add test-cases for packed/aligned structures (PR #96932)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/96932 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Add test-cases for packed/aligned structures (PR #96932)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/96932
>From d2c28706769f89bf9f0b53071726bb59c6205ce8 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 27 Jun 2024 17:16:50 +0100
Subject: [PATCH] [lldb][test] Add test-cases for packed structures
Adds test that checks whether LLDB correctly infers the
alignment of packed structures. Specifically, the
`InferAlignment` code-path of the `ItaniumRecordLayoutBuilder`
where it assumes that overlapping field offsets imply a
packed structure and thus sets alignment to `1`. See discussion
in https://github.com/llvm/llvm-project/pull/93809.
While here, also added a test-case where we check alignment of
a class whose base has an explicit `DW_AT_alignment
(those don't get transitively propagated in DWARF, but don't seem
like a problem for LLDB).
Lastly, also added an XFAIL-ed tests where the aforementioned
`InferAlignment` kicks in for overlapping fields (but in this
case incorrectly since the structure isn't actually packed).
---
.../TestAlignAsBaseClass.py | 1 +
.../API/lang/cpp/alignas_base_class/main.cpp | 7
.../DWARF/no_unique_address-alignment.cpp | 25 +
lldb/test/Shell/SymbolFile/DWARF/packed.cpp | 36 +++
4 files changed, 69 insertions(+)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/no_unique_address-alignment.cpp
create mode 100644 lldb/test/Shell/SymbolFile/DWARF/packed.cpp
diff --git a/lldb/test/API/lang/cpp/alignas_base_class/TestAlignAsBaseClass.py
b/lldb/test/API/lang/cpp/alignas_base_class/TestAlignAsBaseClass.py
index 7d97b0c42b7e1..362fc2740bf52 100644
--- a/lldb/test/API/lang/cpp/alignas_base_class/TestAlignAsBaseClass.py
+++ b/lldb/test/API/lang/cpp/alignas_base_class/TestAlignAsBaseClass.py
@@ -16,3 +16,4 @@ def test(self):
# Verify specified class alignments.
self.expect_expr("alignof(B2)", result_value="8")
self.expect_expr("alignof(EmptyClassAlign8)", result_value="8")
+self.expect_expr("alignof(Derived)", result_value="8")
diff --git a/lldb/test/API/lang/cpp/alignas_base_class/main.cpp
b/lldb/test/API/lang/cpp/alignas_base_class/main.cpp
index 9d37554957ba3..a37919deaebdc 100644
--- a/lldb/test/API/lang/cpp/alignas_base_class/main.cpp
+++ b/lldb/test/API/lang/cpp/alignas_base_class/main.cpp
@@ -13,4 +13,11 @@ D d3g;
struct alignas(8) EmptyClassAlign8 {
} t;
+struct alignas(8) __attribute__((packed)) AlignedAndPackedBase {
+} foo;
+
+struct Derived : AlignedAndPackedBase {
+} bar;
+static_assert(alignof(Derived) == 8);
+
int main() {}
diff --git a/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-alignment.cpp
b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-alignment.cpp
new file mode 100644
index 0..6e544f40625df
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-alignment.cpp
@@ -0,0 +1,25 @@
+// XFAIL: *
+
+// RUN: %clangxx_host -gdwarf -o %t %s
+// RUN: %lldb %t \
+// RUN: -o "b main" \
+// RUN: -o "expr alignof(OverlappingFields)" \
+// RUN: -o "expr sizeof(OverlappingFields)" \
+// RUN: -o exit | FileCheck %s
+
+// CHECK: (lldb) expr alignof(OverlappingFields)
+// CHECK-NEXT: ${{.*}} = 4
+// CHECK: (lldb) expr sizeof(OverlappingFields)
+// CHECK-NEXT: ${{.*}} = 8
+
+struct Empty {};
+
+struct OverlappingFields {
+ char y;
+ [[no_unique_address]] Empty e;
+ int z;
+} g_packed_struct;
+static_assert(alignof(OverlappingFields) == 4);
+static_assert(sizeof(OverlappingFields) == 8);
+
+int main() {}
diff --git a/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
b/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
new file mode 100644
index 0..fb94a834d48a6
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
@@ -0,0 +1,36 @@
+// RUN: %clangxx_host -gdwarf -o %t %s
+// RUN: %lldb %t \
+// RUN: -o "b main" \
+// RUN: -o "expr alignof(packed)" \
+// RUN: -o "expr sizeof(packed)" \
+// RUN: -o "expr alignof(packed_and_aligned)" \
+// RUN: -o "expr sizeof(packed_and_aligned)" \
+// RUN: -o exit | FileCheck %s
+
+// CHECK: (lldb) expr alignof(packed)
+// CHECK-NEXT: ${{.*}} = 1
+// CHECK: (lldb) expr sizeof(packed)
+// CHECK-NEXT: ${{.*}} = 9
+
+// CHECK: (lldb) expr alignof(packed_and_aligned)
+// CHECK-NEXT: ${{.*}} = 16
+// CHECK: (lldb) expr sizeof(packed_and_aligned)
+// CHECK-NEXT: ${{.*}} = 16
+
+struct __attribute__((packed)) packed {
+ int x;
+ char y;
+ int z;
+} g_packed_struct;
+static_assert(alignof(packed) == 1);
+static_assert(sizeof(packed) == 9);
+
+struct __attribute__((packed, aligned(16))) packed_and_aligned {
+ int x;
+ char y;
+ int z;
+} g_packed_and_aligned_struct;
+static_assert(alignof(packed_and_aligned) == 16);
+static_assert(sizeof(packed_and_aligned) == 16);
+
+int main() {}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Add test-cases for packed/aligned structures (PR #96932)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/96932
Adds test that checks that LLDB correctly infers the alignment of packed
structures. Specifically, the
`InferAlignment` code-path of the `ItaniumRecordLayoutBuilder` where it assumes
that overlapping field offsets imply a packed structure and thus sets alignment
to `1`. See discussion in https://github.com/llvm/llvm-project/pull/93809.
Also adds two XFAIL-ed tests:
1. where LLDB doesn't correctly infer the alignment of a derived class whose
base has an explicit `DW_AT_alignment. See
https://github.com/llvm/llvm-project/issues/73623.
2. where the aforementioned `InferAlignment` kicks in for overlapping fields
(but in this case incorrectly since the structure isn't actually packed).
>From 2113f052dd69cc1773f642e06339e5a13599090c Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 27 Jun 2024 17:16:50 +0100
Subject: [PATCH] [lldb][test] Add test-cases for packed structures
Adds test that checks that LLDB correctly infers the
alignment of packed structures. Specifically, the
`InferAlignment` code-path of the `ItaniumRecordLayoutBuilder`
where it assumes that overlapping field offsets imply a
packed structure and thus sets alignment to `1`. See discussion
in https://github.com/llvm/llvm-project/pull/93809.
Also adds two XFAIL-ed tests:
1. where LLDB doesn't correctly infer the alignment of a derived class whose
base has
an explicit `DW_AT_alignment. See
https://github.com/llvm/llvm-project/issues/73623.
2. where the aforementioned `InferAlignment` kicks in for
overlapping fields (but in this case incorrectly since
the structure isn't actually packed).
---
.../TestAlignAsBaseClass.py | 8 +
.../API/lang/cpp/alignas_base_class/main.cpp | 6
.../DWARF/no_unique_address-alignment.cpp | 25 +
lldb/test/Shell/SymbolFile/DWARF/packed.cpp | 36 +++
4 files changed, 75 insertions(+)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/no_unique_address-alignment.cpp
create mode 100644 lldb/test/Shell/SymbolFile/DWARF/packed.cpp
diff --git a/lldb/test/API/lang/cpp/alignas_base_class/TestAlignAsBaseClass.py
b/lldb/test/API/lang/cpp/alignas_base_class/TestAlignAsBaseClass.py
index 7d97b0c42b7e1..f9c99d15e5891 100644
--- a/lldb/test/API/lang/cpp/alignas_base_class/TestAlignAsBaseClass.py
+++ b/lldb/test/API/lang/cpp/alignas_base_class/TestAlignAsBaseClass.py
@@ -16,3 +16,11 @@ def test(self):
# Verify specified class alignments.
self.expect_expr("alignof(B2)", result_value="8")
self.expect_expr("alignof(EmptyClassAlign8)", result_value="8")
+
+@no_debug_info_test
+
@expectedFailureAll(bugnumber="https://github.com/llvm/llvm-project/issues/73623;)
+def test(self):
+self.build()
+self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+
+self.expect_expr("alignof(Derived)", result_value="8")
diff --git a/lldb/test/API/lang/cpp/alignas_base_class/main.cpp
b/lldb/test/API/lang/cpp/alignas_base_class/main.cpp
index 9d37554957ba3..fb922c42edfc3 100644
--- a/lldb/test/API/lang/cpp/alignas_base_class/main.cpp
+++ b/lldb/test/API/lang/cpp/alignas_base_class/main.cpp
@@ -13,4 +13,10 @@ D d3g;
struct alignas(8) EmptyClassAlign8 {
} t;
+struct alignas(8) __attribute__((packed)) AlignedAndPackedBase {} foo;
+
+struct Derived : AlignedAndPackedBase {
+} bar;
+static_assert(alignof(Derived) == 8);
+
int main() {}
diff --git a/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-alignment.cpp
b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-alignment.cpp
new file mode 100644
index 0..79199a9237a63
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-alignment.cpp
@@ -0,0 +1,25 @@
+// XFAIL: *
+
+// RUN: %clangxx_host -gdwarf -o %t %s
+// RUN: %lldb %t \
+// RUN: -o "b main" \
+// RUN: -o "expr alignof(OverlappingFields)" \
+// RUN: -o "expr sizeof(OverlappingFields)" \
+// RUN: -o exit | FileCheck %s
+
+// CHECK: (lldb) expr alignof(OverlappingFields)
+// CHECK-NEXT: ${{.*}} = 4
+// CHECK: (lldb) expr sizeof(OverlappingFields)
+// CHECK-NEXT: ${{.*}} = 8
+
+struct Empty {};
+
+struct OverlappingFields {
+char y;
+[[no_unique_address]] Empty e;
+int z;
+} g_packed_struct;
+static_assert(alignof(OverlappingFields) == 4);
+static_assert(sizeof(OverlappingFields) == 8);
+
+int main() {}
diff --git a/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
b/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
new file mode 100644
index 0..53b5ee624472c
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/packed.cpp
@@ -0,0 +1,36 @@
+// RUN: %clangxx_host -gdwarf -o %t %s
+// RUN: %lldb %t \
+// RUN: -o "b main" \
+// RUN: -o "expr alignof(packed)" \
+// RUN: -o "expr sizeof(packed)" \
+// RUN: -o "expr alignof(packed_and_aligned)" \
+// RUN: -o "expr sizeof(packed_and_aligned)" \
+// RUN: -o exit | FileCheck %s
+
+// CHECK: (lldb)
[Lldb-commits] [lldb] [lldb] Support new libc++ __compressed_pair layout (PR #96538)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/96538
>From 3b4d9629a68c9e75dfd139ee2745bf00db979ecd Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Mon, 29 Jan 2024 16:23:16 +
Subject: [PATCH 1/2] [lldb] Support new libc++ __compressed_pair layout
This patch is in preparation for the `__compressed_pair`
refactor in https://github.com/llvm/llvm-project/pull/76756.
This gets the formatter tests to at least pass.
Currently in draft because there's still some cleanup to be done.
---
lldb/examples/synthetic/libcxx.py | 26 --
.../Plugins/Language/CPlusPlus/LibCxx.cpp | 61 ++
.../Plugins/Language/CPlusPlus/LibCxxList.cpp | 83 ---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 68 +++
.../Language/CPlusPlus/LibCxxUnorderedMap.cpp | 73 +++-
.../Language/CPlusPlus/LibCxxVector.cpp | 40 +
.../list/TestDataFormatterGenericList.py | 3 +-
.../libcxx/string/simulator/main.cpp | 1 +
.../TestDataFormatterLibcxxUniquePtr.py | 7 +-
9 files changed, 256 insertions(+), 106 deletions(-)
diff --git a/lldb/examples/synthetic/libcxx.py
b/lldb/examples/synthetic/libcxx.py
index 474aaa428fa23..060ff90100849 100644
--- a/lldb/examples/synthetic/libcxx.py
+++ b/lldb/examples/synthetic/libcxx.py
@@ -721,6 +721,12 @@ def _get_value_of_compressed_pair(self, pair):
def update(self):
logger = lldb.formatters.Logger.Logger()
try:
+has_compressed_pair_layout = True
+alloc_valobj = self.valobj.GetChildMemberWithName("__alloc_")
+size_valobj = self.valobj.GetChildMemberWithName("__size_")
+if alloc_valobj.IsValid() and size_valobj.IsValid():
+has_compressed_pair_layout = False
+
# A deque is effectively a two-dim array, with fixed width.
# 'map' contains pointers to the rows of this array. The
# full memory area allocated by the deque is delimited
@@ -734,9 +740,13 @@ def update(self):
# variable tells which element in this NxM array is the 0th
# one, and the 'size' element gives the number of elements
# in the deque.
-count = self._get_value_of_compressed_pair(
-self.valobj.GetChildMemberWithName("__size_")
-)
+if has_compressed_pair_layout:
+count = self._get_value_of_compressed_pair(
+self.valobj.GetChildMemberWithName("__size_")
+)
+else:
+count = size_valobj.GetValueAsUnsigned(0)
+
# give up now if we cant access memory reliably
if self.block_size < 0:
logger.write("block_size < 0")
@@ -748,9 +758,13 @@ def update(self):
self.map_begin = map_.GetChildMemberWithName("__begin_")
map_begin = self.map_begin.GetValueAsUnsigned(0)
map_end =
map_.GetChildMemberWithName("__end_").GetValueAsUnsigned(0)
-map_endcap = self._get_value_of_compressed_pair(
-map_.GetChildMemberWithName("__end_cap_")
-)
+
+if has_compressed_pair_layout:
+map_endcap = self._get_value_of_compressed_pair(
+map_.GetChildMemberWithName("__end_cap_")
+)
+else:
+map_endcap =
map_.GetChildMemberWithName("__end_cap_").GetValueAsUnsigned(0)
# check consistency
if not map_first <= map_begin <= map_end <= map_endcap:
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index b0e6fb7d6f5af..928b790317b6e 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -27,6 +27,7 @@
#include "Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h"
#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
#include
#include
@@ -176,9 +177,9 @@ bool
lldb_private::formatters::LibcxxUniquePointerSummaryProvider(
if (!ptr_sp)
return false;
- ptr_sp = GetFirstValueOfLibCXXCompressedPair(*ptr_sp);
- if (!ptr_sp)
-return false;
+ if (ValueObjectSP compressed_pair_value__sp =
+ GetFirstValueOfLibCXXCompressedPair(*ptr_sp))
+ptr_sp = std::move(compressed_pair_value__sp);
if (ptr_sp->GetValueAsUnsigned(0) == 0) {
stream.Printf("nullptr");
@@ -701,15 +702,28 @@
lldb_private::formatters::LibcxxUniquePtrSyntheticFrontEnd::Update() {
if (!ptr_sp)
return lldb::ChildCacheState::eRefetch;
+ bool has_compressed_pair_layout = true;
+ ValueObjectSP deleter_sp(valobj_sp->GetChildMemberWithName("__deleter_"));
+ if (deleter_sp)
+has_compressed_pair_layout = false;
+
// Retrieve the actual pointer and the deleter, and clone them to give them
// user-friendly
[Lldb-commits] [clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)
Michael137 wrote:
> Here's the smallest patch that would put explicit alignment on any packed
> structure:
>
> ```
> diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp
> b/clang/lib/CodeGen/CGDebugInfo.cpp
> index a072475ba770..bbb13ddd593b 100644
> --- a/clang/lib/CodeGen/CGDebugInfo.cpp
> +++ b/clang/lib/CodeGen/CGDebugInfo.cpp
> @@ -64,7 +64,7 @@ static uint32_t getTypeAlignIfRequired(const Type *Ty,
> const ASTContext ) {
>// MaxFieldAlignmentAttr is the attribute added to types
>// declared after #pragma pack(n).
>if (auto *Decl = Ty->getAsRecordDecl())
> -if (Decl->hasAttr())
> +if (Decl->hasAttr() ||
> Decl->hasAttr())
>return TI.Align;
>
>return 0;
> ```
>
> But I don't think that's the right approach - I think what we should do is
> compute the natural alignment of the structure, then compare that to the
> actual alignment - and if they differ, we should put an explicit alignment on
> the structure. This avoids the risk that other alignment-influencing effects
> might be missed (and avoids the case of putting alignment on a structure
> that, when packed, just has the same alignment anyway - which is a minor
> issue, but nice to get right (eg: packed struct of a single char probably
> shouldn't have an explicit alignment - since it's the same as the implicit
> alignment anyway))
Thanks for the analysis! If we can emit alignment for packed attributes
consistently then we probably can get rid of most of the `InferAlignment` logic
in the `RecordLayoutBuilder` (it seems to me most of that logic was put
introduced there for the purpose of packed structs), which would address the
issue I saw with laying out `[[no_unique_address]]` fields. Trying this now
https://github.com/llvm/llvm-project/pull/93809
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)
@@ -2192,87 +2097,82 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE ,
ClangASTImporter::LayoutInfo layout_info;
std::vector contained_type_dies;
- if (die.HasChildren()) {
-const bool type_is_objc_object_or_interface =
-TypeSystemClang::IsObjCObjectOrInterfaceType(clang_type);
-if (type_is_objc_object_or_interface) {
- // For objective C we don't start the definition when the class is
- // created.
- TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-}
-
-AccessType default_accessibility = eAccessNone;
-if (tag == DW_TAG_structure_type) {
- default_accessibility = eAccessPublic;
-} else if (tag == DW_TAG_union_type) {
- default_accessibility = eAccessPublic;
-} else if (tag == DW_TAG_class_type) {
- default_accessibility = eAccessPrivate;
-}
-
-std::vector> bases;
-// Parse members and base classes first
-std::vector member_function_dies;
-
-DelayedPropertyList delayed_properties;
-ParseChildMembers(die, clang_type, bases, member_function_dies,
- contained_type_dies, delayed_properties,
- default_accessibility, layout_info);
-
-// Now parse any methods if there were any...
-for (const DWARFDIE : member_function_dies)
- dwarf->ResolveType(die);
-
-if (type_is_objc_object_or_interface) {
- ConstString class_name(clang_type.GetTypeName());
- if (class_name) {
-dwarf->GetObjCMethods(class_name, [&](DWARFDIE method_die) {
- method_die.ResolveType();
- return true;
-});
+ if (die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0))
+return false; // No definition, cannot complete.
-for (DelayedAddObjCClassProperty : delayed_properties)
- property.Finalize();
- }
-}
+ // Start the definition if the type is not being defined already. This can
+ // happen (e.g.) when adding nested types to a class type -- see
+ // PrepareContextToReceiveMembers.
+ if (!clang_type.IsBeingDefined())
+TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-if (!bases.empty()) {
- // Make sure all base classes refer to complete types and not forward
- // declarations. If we don't do this, clang will crash with an
- // assertion in the call to clang_type.TransferBaseClasses()
- for (const auto _class : bases) {
-clang::TypeSourceInfo *type_source_info =
-base_class->getTypeSourceInfo();
-if (type_source_info)
- TypeSystemClang::RequireCompleteType(
- m_ast.GetType(type_source_info->getType()));
- }
+ AccessType default_accessibility = eAccessNone;
+ if (tag == DW_TAG_structure_type) {
+default_accessibility = eAccessPublic;
+ } else if (tag == DW_TAG_union_type) {
+default_accessibility = eAccessPublic;
+ } else if (tag == DW_TAG_class_type) {
+default_accessibility = eAccessPrivate;
+ }
+
+ std::vector> bases;
+ // Parse members and base classes first
+ std::vector member_function_dies;
- m_ast.TransferBaseClasses(clang_type.GetOpaqueQualType(),
-std::move(bases));
+ DelayedPropertyList delayed_properties;
+ ParseChildMembers(die, clang_type, bases, member_function_dies,
+contained_type_dies, delayed_properties,
+default_accessibility, layout_info);
+
+ // Now parse any methods if there were any...
+ for (const DWARFDIE : member_function_dies)
+dwarf->ResolveType(die);
+
+ if (TypeSystemClang::IsObjCObjectOrInterfaceType(clang_type)) {
+ConstString class_name(clang_type.GetTypeName());
+if (class_name) {
+ dwarf->GetObjCMethods(class_name, [&](DWARFDIE method_die) {
+method_die.ResolveType();
+return true;
+ });
+
+ for (DelayedAddObjCClassProperty : delayed_properties)
+property.Finalize();
}
}
+ if (!bases.empty()) {
+// Make sure all base classes refer to complete types and not forward
+// declarations. If we don't do this, clang will crash with an
+// assertion in the call to clang_type.TransferBaseClasses()
+for (const auto _class : bases) {
+ clang::TypeSourceInfo *type_source_info =
base_class->getTypeSourceInfo();
+ if (type_source_info)
+TypeSystemClang::RequireCompleteType(
+m_ast.GetType(type_source_info->getType()));
+}
+
+m_ast.TransferBaseClasses(clang_type.GetOpaqueQualType(),
std::move(bases));
+ }
+
m_ast.AddMethodOverridesForCXXRecordType(clang_type.GetOpaqueQualType());
TypeSystemClang::BuildIndirectFields(clang_type);
TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
- if (!layout_info.field_offsets.empty() || !layout_info.base_offsets.empty()
||
- !layout_info.vbase_offsets.empty()) {
Michael137 wrote:
Agreed, don't really see an issue with removing this.
Based on the commits
[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)
@@ -1893,72 +1849,21 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext ,
dwarf->GetUniqueDWARFASTTypeMap().Insert(unique_typename,
*unique_ast_entry_up);
- if (!attrs.is_forward_declaration) {
-// Always start the definition for a class type so that if the class
-// has child classes or types that require the class to be created
-// for use as their decl contexts the class will be ready to accept
-// these child definitions.
-if (!def_die.HasChildren()) {
- // No children for this struct/union/class, lets finish it
- if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
-TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
- } else {
-dwarf->GetObjectFile()->GetModule()->ReportError(
-
-"DWARF DIE {0:x16} named \"{1}\" was not able to start its "
-"definition.\nPlease file a bug and attach the file at the "
-"start of this error message",
-def_die.GetID(), attrs.name.GetCString());
- }
-
- // Setting authority byte size and alignment for empty structures.
- //
- // If the byte size or alignmenet of the record is specified then
- // overwrite the ones that would be computed by Clang.
- // This is only needed as LLDB's TypeSystemClang is always in C++ mode,
- // but some compilers such as GCC and Clang give empty structs a size of 0
- // in C mode (in contrast to the size of 1 for empty structs that would
be
- // computed in C++ mode).
- if (attrs.byte_size || attrs.alignment) {
-clang::RecordDecl *record_decl =
-TypeSystemClang::GetAsRecordDecl(clang_type);
-if (record_decl) {
- ClangASTImporter::LayoutInfo layout;
- layout.bit_size = attrs.byte_size.value_or(0) * 8;
- layout.alignment = attrs.alignment.value_or(0) * 8;
- GetClangASTImporter().SetRecordLayout(record_decl, layout);
-}
- }
-} else if (clang_type_was_created) {
- // Start the definition if the class is not objective C since the
- // underlying decls respond to isCompleteDefinition(). Objective
- // C decls don't respond to isCompleteDefinition() so we can't
- // start the declaration definition right away. For C++
- // class/union/structs we want to start the definition in case the
- // class is needed as the declaration context for a contained class
- // or type without the need to complete that type..
-
- if (attrs.class_language != eLanguageTypeObjC &&
- attrs.class_language != eLanguageTypeObjC_plus_plus)
-TypeSystemClang::StartTagDeclarationDefinition(clang_type);
-
- // Leave this as a forward declaration until we need to know the
- // details of the type. lldb_private::Type will automatically call
- // the SymbolFile virtual function
- // "SymbolFileDWARF::CompleteType(Type *)" When the definition
- // needs to be defined.
- assert(!dwarf->GetForwardDeclCompilerTypeToDIE().count(
- ClangUtil::RemoveFastQualifiers(clang_type)
- .GetOpaqueQualType()) &&
- "Type already in the forward declaration map!");
- // Can't assume m_ast.GetSymbolFile() is actually a
- // SymbolFileDWARF, it can be a SymbolFileDWARFDebugMap for Apple
- // binaries.
- dwarf->GetForwardDeclCompilerTypeToDIE().try_emplace(
- ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType(),
- *def_die.GetDIERef());
- m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true);
-}
+ if (clang_type_was_created) {
Michael137 wrote:
Could there be any consequence of doing this for empty types now? We might end
up with some lookups into empty types? Since we're going to turn off the
external storage bit in `CompleteRecordTypeFromDWARF` this is probably fine
though?
https://github.com/llvm/llvm-project/pull/96755
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)
https://github.com/Michael137 approved this pull request. This is a really nice cleanup! It actually aligns almost exactly with [our in-progress version of this](https://github.com/llvm/llvm-project/blob/caacb57a46f34bf663fa5ab2190b361ce29b255b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp). LGTM Agree with your point about `PrepareContextToReceiveMembers`. We can add those as needed. In our version of this I had to only add it in `ParseSubroutine`, as you've also effectively done. https://github.com/llvm/llvm-project/pull/96755 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Don't start class definitions in ParseStructureLikeDIE (PR #96755)
https://github.com/Michael137 commented: Without having reviewed this in detail yet this I like the goal, thanks for doing this. This aligns with what https://github.com/llvm/llvm-project/pull/95100 is trying to do. There we similarly want to make sure that we start and complete definitions in the same place. So I'm hoping that patch rebases more-or-less cleanly on this :) https://github.com/llvm/llvm-project/pull/96755 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Unique enums parsed from declarations (PR #96751)
https://github.com/Michael137 approved this pull request. https://github.com/llvm/llvm-project/pull/96751 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)
Michael137 wrote:
While fixing the libc++ formatters in preparation for the [compressed_pair
change](https://github.com/llvm/llvm-project/issues/93069), i encountered
another issue which I'm not sure entirely how to best reconcile. There's [this
assumption in
`RecordLayoutBuilder`](https://github.com/llvm/llvm-project/blob/f782ff8fc6426890863be0791a9ace2394da3887/clang/lib/AST/RecordLayoutBuilder.cpp#L2258-L2264)
for external layouts, where, if we encounter overlapping field offsets, we
assume the structure is packed and set the alignment back to `1`:
```
uint64_t
ItaniumRecordLayoutBuilder::updateExternalFieldOffset(const FieldDecl *Field,
uint64_t ComputedOffset) {
uint64_t ExternalFieldOffset = External.getExternalFieldOffset(Field);
if (InferAlignment && ExternalFieldOffset < ComputedOffset) {
// The externally-supplied field offset is before the field offset we
// computed. Assume that the structure is packed.
Alignment = CharUnits::One();
PreferredAlignment = CharUnits::One();
InferAlignment = false;
}
...
```
The assumption in that comment doesn't hold for layouts where the overlap
occurred because of `[[no_unique_address]]`. In those cases, the alignment of
`1` will prevent us from aligning the `FieldOffset` correctly and cause LLDB to
read out the fields incorrectly.
We can't guard this with `Field->isZeroSize()` because we don't have the
attribute in the AST.
Can we infer packedness more accurately here?
Or maybe that's just putting a bandaid on a bigger underlying issue
https://github.com/llvm/llvm-project/pull/93809
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)
Michael137 wrote: > I have the whole thing implemented (complete with test cases). If you really > want to dig through the complete code, you can see it at > https://github.com/cmtice/llvm-project/tree/DIL-work-new/ (note that I will > be cleaning up the Parser & Evaluator code before actually being put into a > PR). Thanks! that'll be a useful reference > Besides managing that risk (if this fades out for whatever reason, there's > less dead code around), I think this would also make it easier to review, as > we could have patches like "add operator/ to the DIL", which would focus > solely on that, and also come with associated tests. I think it would also > make it easier to discuss things like whether we want to add modifying > operations (operator++) or obviously language-specific features > (dynamic_cast), both of which I think are very good questions. Makes sense! https://github.com/llvm/llvm-project/pull/95738 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][LibCxx] Move incorrect nullptr check (PR #96635)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/96635 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][LibCxx] Move incorrect nullptr check (PR #96635)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/96635 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][LibCxx] Move incorrect nullptr check (PR #96635)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/96635
Found this while skimming this code. Don't have a reproducible test case for
this but the nullptr check should clearly occur before we try to dereference
`location_sp`.
>From 87edb6b9ba8b48e1bcddd2d73314cdb8f4e0a73b Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Tue, 25 Jun 2024 14:25:07 +0100
Subject: [PATCH] [lldb][LibCxx] Move incorrect nullptr check
Found this while skimming this code. Don't have a
reproducible test case for this but the nullptr
check should clearly occur before we try to dereference
`location_sp`.
---
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index b0e6fb7d6f5af..0f9f93b727ce8 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -808,6 +808,9 @@ ExtractLibcxxStringInfo(ValueObject ) {
size = (layout == StringLayout::DSC) ? size_mode_value
: ((size_mode_value >> 1) % 256);
+if (!location_sp)
+ return {};
+
// When the small-string optimization takes place, the data must fit in the
// inline string buffer (23 bytes on x86_64/Darwin). If it doesn't, it's
// likely that the string isn't initialized and we're reading garbage.
@@ -815,7 +818,7 @@ ExtractLibcxxStringInfo(ValueObject ) {
const std::optional max_bytes =
location_sp->GetCompilerType().GetByteSize(
exe_ctx.GetBestExecutionContextScope());
-if (!max_bytes || size > *max_bytes || !location_sp)
+if (!max_bytes || size > *max_bytes)
return {};
return std::make_pair(size, location_sp);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][LibCxx] Move incorrect nullptr check (PR #96635)
https://github.com/Michael137 ready_for_review https://github.com/llvm/llvm-project/pull/96635 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Support new libc++ __compressed_pair layout (PR #96538)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/96538
This patch is in preparation for the `__compressed_pair` refactor in
https://github.com/llvm/llvm-project/pull/76756.
This gets the formatter tests to at least pass.
Currently in draft because there's still some cleanup to be done.
>From 3b4d9629a68c9e75dfd139ee2745bf00db979ecd Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Mon, 29 Jan 2024 16:23:16 +
Subject: [PATCH] [lldb] Support new libc++ __compressed_pair layout
This patch is in preparation for the `__compressed_pair`
refactor in https://github.com/llvm/llvm-project/pull/76756.
This gets the formatter tests to at least pass.
Currently in draft because there's still some cleanup to be done.
---
lldb/examples/synthetic/libcxx.py | 26 --
.../Plugins/Language/CPlusPlus/LibCxx.cpp | 61 ++
.../Plugins/Language/CPlusPlus/LibCxxList.cpp | 83 ---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 68 +++
.../Language/CPlusPlus/LibCxxUnorderedMap.cpp | 73 +++-
.../Language/CPlusPlus/LibCxxVector.cpp | 40 +
.../list/TestDataFormatterGenericList.py | 3 +-
.../libcxx/string/simulator/main.cpp | 1 +
.../TestDataFormatterLibcxxUniquePtr.py | 7 +-
9 files changed, 256 insertions(+), 106 deletions(-)
diff --git a/lldb/examples/synthetic/libcxx.py
b/lldb/examples/synthetic/libcxx.py
index 474aaa428fa23..060ff90100849 100644
--- a/lldb/examples/synthetic/libcxx.py
+++ b/lldb/examples/synthetic/libcxx.py
@@ -721,6 +721,12 @@ def _get_value_of_compressed_pair(self, pair):
def update(self):
logger = lldb.formatters.Logger.Logger()
try:
+has_compressed_pair_layout = True
+alloc_valobj = self.valobj.GetChildMemberWithName("__alloc_")
+size_valobj = self.valobj.GetChildMemberWithName("__size_")
+if alloc_valobj.IsValid() and size_valobj.IsValid():
+has_compressed_pair_layout = False
+
# A deque is effectively a two-dim array, with fixed width.
# 'map' contains pointers to the rows of this array. The
# full memory area allocated by the deque is delimited
@@ -734,9 +740,13 @@ def update(self):
# variable tells which element in this NxM array is the 0th
# one, and the 'size' element gives the number of elements
# in the deque.
-count = self._get_value_of_compressed_pair(
-self.valobj.GetChildMemberWithName("__size_")
-)
+if has_compressed_pair_layout:
+count = self._get_value_of_compressed_pair(
+self.valobj.GetChildMemberWithName("__size_")
+)
+else:
+count = size_valobj.GetValueAsUnsigned(0)
+
# give up now if we cant access memory reliably
if self.block_size < 0:
logger.write("block_size < 0")
@@ -748,9 +758,13 @@ def update(self):
self.map_begin = map_.GetChildMemberWithName("__begin_")
map_begin = self.map_begin.GetValueAsUnsigned(0)
map_end =
map_.GetChildMemberWithName("__end_").GetValueAsUnsigned(0)
-map_endcap = self._get_value_of_compressed_pair(
-map_.GetChildMemberWithName("__end_cap_")
-)
+
+if has_compressed_pair_layout:
+map_endcap = self._get_value_of_compressed_pair(
+map_.GetChildMemberWithName("__end_cap_")
+)
+else:
+map_endcap =
map_.GetChildMemberWithName("__end_cap_").GetValueAsUnsigned(0)
# check consistency
if not map_first <= map_begin <= map_end <= map_endcap:
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index b0e6fb7d6f5af..928b790317b6e 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -27,6 +27,7 @@
#include "Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h"
#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
#include
#include
@@ -176,9 +177,9 @@ bool
lldb_private::formatters::LibcxxUniquePointerSummaryProvider(
if (!ptr_sp)
return false;
- ptr_sp = GetFirstValueOfLibCXXCompressedPair(*ptr_sp);
- if (!ptr_sp)
-return false;
+ if (ValueObjectSP compressed_pair_value__sp =
+ GetFirstValueOfLibCXXCompressedPair(*ptr_sp))
+ptr_sp = std::move(compressed_pair_value__sp);
if (ptr_sp->GetValueAsUnsigned(0) == 0) {
stream.Printf("nullptr");
@@ -701,15 +702,28 @@
lldb_private::formatters::LibcxxUniquePtrSyntheticFrontEnd::Update() {
if (!ptr_sp)
return lldb::ChildCacheState::eRefetch;
+ bool has_compressed_pair_layout = true;
+
[Lldb-commits] [lldb] [lldb/DWARF] Remove parsing recursion when searching for definition DIEs (PR #96484)
Michael137 wrote: Latest update LGTM https://github.com/llvm/llvm-project/pull/96484 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Remove parsing recursion when searching for definition DIEs (PR #96484)
https://github.com/Michael137 approved this pull request. Makes sense to me Do we make sure that the type lookup map is updated so we don't re-parse when calling `ParseTypeFromDWARF` with either the declaration or the definition DIE? https://github.com/llvm/llvm-project/pull/96484 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Optimize DIEToType handling (PR #96308)
https://github.com/Michael137 approved this pull request. Nice, this is much more understandable, thanks! https://github.com/llvm/llvm-project/pull/96308 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/95738 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/95738 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)
@@ -0,0 +1,165 @@
+(* LLDB Debug Expressions, a subset of C++ *)
+(* Insired by https://www.nongnu.org/hcb *)
+
+expression = assignment_expression ;
+
+assignment_expression = conditional_expression
+logical_or_expression assignment_operator
assignment_expression ;
+
+assignment_operator = "="
+| "*="
+| "/="
+| "%="
+| "+="
+| "-="
+| ">>="
+| "<<="
+| "&="
+| "^="
+| "|=" ;
+
+conditional_expression = logical_or_expression
+ | logical_or_expression "?" expression ":"
assignment_expression ;
+
+logical_or_expression = logical_and_expression {"||" logical_and_expression} ;
+
+logical_and_expression = inclusive_or_expression {"&&"
inclusive_or_expression} ;
+
+inclusive_or_expression = exclusive_or_expression {"|"
exclusive_or_expression} ;
+
+exclusive_or_expression = and_expression {"^" and_expression} ;
+
+and_expression = equality_expression {"&" equality_expression} ;
+
+equality_expression = relational_expression {"==" relational_expression}
+| relational_expression {"!=" relational_expression} ;
+
+relational_expression = shift_expression {"<" shift_expression}
+ | shift_expression {">" shift_expression}
+ | shift_expression {"<=" shift_expression}
+ | shift_expression {">=" shift_expression} ;
+
+shift_expression = additive_expression {"<<" additive_expression}
+ | additive_expression {">>" additive_expression} ;
+
+additive_expression = multiplicative_expression {"+" multiplicative_expression}
+| multiplicative_expression {"-"
multiplicative_expression} ;
+
+multiplicative_expression = cast_expression {"*" cast_expression}
+ | cast_expression {"/" cast_expression}
+ | cast_expression {"%" cast_expression} ;
+
+cast_expression = unary_expression
+| "(" type_id ")" cast_expression ;
+
+unary_expression = postfix_expression
+ | "++" cast_expression
+ | "--" cast_expression
+ | unary_operator cast_expression
+ | "sizeof" unary_expression
+ | "sizeof" "(" type_id ")" ;
+
+unary_operator = "*" | "&" | "+" | "-" | "!" | "~" ;
+
+postfix_expression = primary_expression
+ | postfix_expression "[" expression "]"
+ | postfix_expression "." id_expression
+ | postfix_expression "->" id_expression
+ | postfix_expression "++"
+ | postfix_expression "--"
+ | static_cast "<" type_id ">" "(" expression ")"
+ | dynamic_cast "<" type_id ">" "(" expression ")"
+ | reinterpret_cast "<" type_id ">" "(" expression ")" ;
Michael137 wrote:
Wouldn't `C-style` casts suffice?
https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
