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 <michaelbuc...@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 <michaelbuc...@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); } } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits