[Lldb-commits] [lldb] [lldb][DataFormatter] Remove support for old std::map layout (PR #97549)
Michael Buch via lldb-commits
lldb-commits at lists.llvm.org
Wed Jul 3 23:57:04 PDT 2024
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 <michaelbuch12 at gmail.com>
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, &bit_offset) !=
- UINT32_MAX) {
- // Old layout (pre d05b10ab4fc65)
- m_skip_size = bit_offset / 8u;
- } else {
- auto ast_ctx = node_type.GetTypeSystem().dyn_cast_or_null<TypeSystemClang>();
- 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<TypeSystemClang>();
+ 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 (pre 089a7cc5dea)
- 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)
return nullptr;
>From fbcfcdfa6872a31a7a6071850a8486b5a0019a08 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 4 Jul 2024 08:56:22 +0200
Subject: [PATCH 2/2] fixup! remove traces of old layout from map iterator too
---
.../Plugins/Language/CPlusPlus/LibCxx.cpp | 146 ++++++++----------
1 file changed, 66 insertions(+), 80 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index ad467c3966e60..7a28f9590d063 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -252,7 +252,7 @@ lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::Update() {
// and free their memory
m_pair_ptr = valobj_sp
->GetValueForExpressionPath(
- ".__i_.__ptr_->__value_", nullptr, nullptr,
+ ".__i_.__ptr_", nullptr, nullptr,
ValueObject::GetValueForExpressionPathOptions()
.DontCheckDotVsArrowSyntax()
.SetSyntheticChildrenTraversal(
@@ -260,88 +260,74 @@ lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::Update() {
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, &bit_offset_ptr, &bitfield_bit_size_ptr, &is_bitfield_ptr);
+ if (!pair_type) {
+ m_pair_ptr = nullptr;
+ return lldb::ChildCacheState::eRefetch;
+ }
- 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, &bit_offset_ptr, &bitfield_bit_size_ptr, &is_bitfield_ptr);
- if (!pair_type) {
- m_pair_ptr = nullptr;
+ 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<TypeSystemClang>();
+ if (!ast_ctx)
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<TypeSystemClang>();
- if (!ast_ctx)
- return lldb::ChildCacheState::eRefetch;
-
- // Mimick layout of std::__tree_iterator::__ptr_ and read it in
- // from process memory.
- //
- // 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
- // +-----------------------------+
- //
- 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", pair_type}});
- std::optional<uint64_t> 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;
- DataExtractor extractor(buffer_sp, process_sp->GetByteOrder(),
- process_sp->GetAddressByteSize());
- auto pair_sp = CreateValueObjectFromData(
- "pair", extractor, valobj_sp->GetExecutionContextRef(),
- tree_node_type);
- if (pair_sp)
- m_pair_sp = pair_sp->GetChildAtIndex(4);
- }
+ // Mimick layout of std::__tree_iterator::__ptr_ and read it in
+ // from process memory.
+ //
+ // 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
+ // +-----------------------------+
+ //
+ 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", pair_type}});
+ std::optional<uint64_t> 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;
+ DataExtractor extractor(buffer_sp, process_sp->GetByteOrder(),
+ process_sp->GetAddressByteSize());
+ auto pair_sp = CreateValueObjectFromData(
+ "pair", extractor, valobj_sp->GetExecutionContextRef(),
+ tree_node_type);
+ if (pair_sp)
+ m_pair_sp = pair_sp->GetChildAtIndex(4);
}
}
More information about the lldb-commits
mailing list