llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) <details> <summary>Changes</summary> This patch is in preparation for the `__compressed_pair` refactor in https://github.com/llvm/llvm-project/pull/76756. This is mostly reviewable now. With the new layout we no longer need to unwrap the `__compressed_pair`. Instead, we just need to look for child members. E.g., to get to the underlying pointer of `std::unique_ptr` we no longer do, ``` GetFirstValueOfCXXCompressedPair(GetChildMemberWithName("__ptr_")) ``` but instead do ``` GetChildMemberWithName("__ptr_") ``` We need to be slightly careful because previously the `__compressed_pair` had a member called `__value_`, whereas now `__value_` might be a member of the class that used to hold the `__compressed_pair`. So before unwrapping the pair, we added checks for `isOldCompressedLayout` (not sure yet whether folding this check into `GetFirstValueOfCXXCompressedPair` is better). --- Patch is 26.60 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/96538.diff 9 Files Affected: - (modified) lldb/examples/synthetic/libcxx.py (+20-6) - (modified) lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp (+63-22) - (modified) lldb/source/Plugins/Language/CPlusPlus/LibCxx.h (+2-1) - (modified) lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp (+43-29) - (modified) lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp (+31-10) - (modified) lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp (+103-63) - (modified) lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp (+19-19) - (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py (+1-1) - (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/main.cpp (+1) ``````````diff 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 feaa51a96843a..7d3b2410a7296 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 <optional> #include <tuple> @@ -34,6 +35,32 @@ using namespace lldb; using namespace lldb_private; using namespace lldb_private::formatters; +static void consumeInlineNamespace(llvm::StringRef &name) { + // Delete past an inline namespace, if any: __[a-zA-Z0-9_]+:: + auto scratch = name; + if (scratch.consume_front("__") && std::isalnum(scratch[0])) { + scratch = scratch.drop_while([](char c) { return std::isalnum(c); }); + if (scratch.consume_front("::")) { + // Successfully consumed a namespace. + name = scratch; + } + } +} + +bool lldb_private::formatters::isOldCompressedPairLayout( + ValueObject &pair_obj) { + return isStdTemplate(pair_obj.GetTypeName(), "__compressed_pair"); +} + +bool lldb_private::formatters::isStdTemplate(ConstString type_name, + llvm::StringRef type) { + llvm::StringRef name = type_name.GetStringRef(); + // The type name may be prefixed with `std::__<inline-namespace>::`. + if (name.consume_front("std::")) + consumeInlineNamespace(name); + return name.consume_front(type) && name.starts_with("<"); +} + lldb::ValueObjectSP lldb_private::formatters::GetChildMemberWithName( ValueObject &obj, llvm::ArrayRef<ConstString> alternative_names) { for (ConstString name : alternative_names) { @@ -53,7 +80,7 @@ lldb_private::formatters::GetFirstValueOfLibCXXCompressedPair( if (first_child) value = first_child->GetChildMemberWithName("__value_"); if (!value) { - // pre-r300140 member name + // pre-c88580c member name value = pair.GetChildMemberWithName("__first_"); } return value; @@ -70,7 +97,7 @@ lldb_private::formatters::GetSecondValueOfLibCXXCompressedPair( } } if (!value) { - // pre-r300140 member name + // pre-c88580c member name value = pair.GetChildMemberWithName("__second_"); } return value; @@ -176,7 +203,9 @@ bool lldb_private::formatters::LibcxxUniquePointerSummaryProvider( if (!ptr_sp) return false; - ptr_sp = GetFirstValueOfLibCXXCompressedPair(*ptr_sp); + if (isOldCompressedPairLayout(*ptr_sp)) + ptr_sp = GetFirstValueOfLibCXXCompressedPair(*ptr_sp); + if (!ptr_sp) return false; @@ -363,13 +392,22 @@ lldb_private::formatters::LibcxxUniquePtrSyntheticFrontEnd::Update() { // Retrieve the actual pointer and the deleter, and clone them to give them // user-friendly names. - ValueObjectSP value_pointer_sp = GetFirstValueOfLibCXXCompressedPair(*ptr_sp); - if (value_pointer_sp) - m_value_ptr_sp = value_pointer_sp->Clone(ConstString("pointer")); + if (isOldCompressedPairLayout(*ptr_sp)) { + if (ValueObjectSP value_pointer_sp = + GetFirstValueOfLibCXXCompressedPair(*ptr_sp)) + m_value_ptr_sp = value_pointer_sp->Clone(ConstString("pointer")); + + if (ValueObjectSP deleter_sp = + GetSecondValueOfLibCXXCompressedPair(*ptr_sp)) + m_deleter_sp = deleter_sp->Clone(ConstString("deleter")); + } else { + m_value_ptr_sp = ptr_sp->Clone(ConstString("pointer")); - ValueObjectSP deleter_sp = GetSecondValueOfLibCXXCompressedPair(*ptr_sp); - if (deleter_sp) - m_deleter_sp = deleter_sp->Clone(ConstString("deleter")); + if (ValueObjectSP deleter_sp = + valobj_sp->GetChildMemberWithName("__deleter_")) + if (deleter_sp->GetNumChildrenIgnoringErrors() > 0) + m_deleter_sp = deleter_sp->Clone(ConstString("deleter")); + } return lldb::ChildCacheState::eRefetch; } @@ -407,24 +445,27 @@ namespace { enum class StringLayout { CSD, DSC }; } +static ValueObjectSP ExtractLibCxxStringData(ValueObject &valobj) { + if (auto rep_sp = valobj.GetChildMemberWithName("__rep_")) + return rep_sp; + + ValueObjectSP valobj_r_sp = valobj.GetChildMemberWithName("__r_"); + if (!valobj_r_sp || !valobj_r_sp->GetError().Success()) + return nullptr; + + if (!isOldCompressedPairLayout(*valobj_r_sp)) + return nullptr; + + return GetFirstValueOfLibCXXCompressedPair(*valobj_r_sp); +} + /// Determine the size in bytes of \p valobj (a libc++ std::string object) and /// extract its data payload. Return the size + payload pair. // TODO: Support big-endian architectures. static std::optional<std::pair<uint64_t, ValueObjectSP>> ExtractLibcxxStringInfo(ValueObject &valobj) { - ValueObjectSP valobj_r_sp = valobj.GetChildMemberWithName("__r_"); - if (!valobj_r_sp || !valobj_r_sp->GetError().Success()) - return {}; - - // __r_ is a compressed_pair of the actual data and the allocator. The data we - // want is in the first base class. - ValueObjectSP valobj_r_base_sp = valobj_r_sp->GetChildAtIndex(0); - if (!valobj_r_base_sp) - return {}; - - ValueObjectSP valobj_rep_sp = - valobj_r_base_sp->GetChildMemberWithName("__value_"); - if (!valobj_rep_sp) + ValueObjectSP valobj_rep_sp = ExtractLibCxxStringData(valobj); + if (!valobj_rep_sp || !valobj_rep_sp->GetError().Success()) return {}; ValueObjectSP l = valobj_rep_sp->GetChildMemberWithName("__l"); diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h index 5307b5251ca84..dad033611b38d 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h @@ -25,7 +25,8 @@ GetChildMemberWithName(ValueObject &obj, lldb::ValueObjectSP GetFirstValueOfLibCXXCompressedPair(ValueObject &pair); lldb::ValueObjectSP GetSecondValueOfLibCXXCompressedPair(ValueObject &pair); - +bool isOldCompressedPairLayout(ValueObject &pair_obj); +bool isStdTemplate(ConstString type_name, llvm::StringRef type); bool LibcxxStringSummaryProviderASCII( ValueObject &valobj, Stream &stream, diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp index d7cfeb30557c3..4479f592fc2d2 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp @@ -17,6 +17,7 @@ #include "lldb/Utility/Endian.h" #include "lldb/Utility/Status.h" #include "lldb/Utility/Stream.h" +#include "lldb/lldb-enumerations.h" using namespace lldb; using namespace lldb_private; @@ -294,12 +295,17 @@ lldb::ChildCacheState ForwardListFrontEnd::Update() { ValueObjectSP impl_sp(m_backend.GetChildMemberWithName("__before_begin_")); if (!impl_sp) - return lldb::ChildCacheState::eRefetch; - impl_sp = GetFirstValueOfLibCXXCompressedPair(*impl_sp); + return ChildCacheState::eRefetch; + + if (isOldCompressedPairLayout(*impl_sp)) + impl_sp = GetFirstValueOfLibCXXCompressedPair(*impl_sp); + if (!impl_sp) - return lldb::ChildCacheState::eRefetch; + return ChildCacheState::eRefetch; + m_head = impl_sp->GetChildMemberWithName("__next_").get(); - return lldb::ChildCacheState::eRefetch; + + return ChildCacheState::eRefetch; } ListFrontEnd::ListFrontEnd(lldb::ValueObjectSP valobj_sp) @@ -313,34 +319,42 @@ llvm::Expected<uint32_t> ListFrontEnd::CalculateNumChildren() { return m_count; if (!m_head || !m_tail || m_node_address == 0) return 0; - ValueObjectSP size_alloc(m_backend.GetChildMemberWithName("__size_alloc_")); - if (size_alloc) { - ValueObjectSP value = GetFirstValueOfLibCXXCompressedPair(*size_alloc); - if (value) { - m_count = value->GetValueAsUnsigned(UINT32_MAX); - } + + ValueObjectSP size_node_sp(m_backend.GetChildMemberWithName("__size_")); + if (!size_node_sp) { + size_node_sp = m_backend.GetChildMemberWithName( + "__size_alloc_"); // pre-compressed_pair rework + + if (!isOldCompressedPairLayout(*size_node_sp)) + return llvm::createStringError("Unexpected std::list layout: expected " + "old __compressed_pair layout."); + + size_node_sp = GetFirstValueOfLibCXXCompressedPair(*size_node_sp); } - if (m_count != UINT32_MAX) { + + if (size_node_sp) + m_count = size_node_sp->GetValueAsUnsigned(UINT32_MAX); + + if (m_count != UINT32_MAX) return m_count; - } else { - uint64_t next_val = m_head->GetValueAsUnsigned(0); - uint64_t prev_val = m_tail->GetValueAsUnsigned(0); - if (next_val == 0 || prev_val == 0) - return 0; - if (next_val == m_node_address) - return 0; - if (next_val == prev_val) - return 1; - uint64_t size = 2; - ListEntry current(m_head); - while (current.next() && current.next().value() != m_node_address) { - size++; - current = current.next(); - if (size > m_list_capping_size) - break; - } - return m_count = (size - 1); + + uint64_t next_val = m_head->GetValueAsUnsigned(0); + uint64_t prev_val = m_tail->GetValueAsUnsigned(0); + if (next_val == 0 || prev_val == 0) + return 0; + if (next_val == m_node_address) + return 0; + if (next_val == prev_val) + return 1; + uint64_t size = 2; + ListEntry current(m_head); + while (current.next() && current.next().value() != m_node_address) { + size++; + current = current.next(); + if (size > m_list_capping_size) + break; } + return m_count = (size - 1); } lldb::ValueObjectSP ListFrontEnd::GetChildAtIndex(uint32_t idx) { diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp index 5106a63d531f8..b89afd6a388ab 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp @@ -202,6 +202,8 @@ class LibcxxStdMapSyntheticFrontEnd : public SyntheticChildrenFrontEnd { size_t GetIndexOfChildWithName(ConstString name) override; private: + llvm::Expected<uint32_t> CalculateNumChildrenForOldCompressedPairLayout(); + /// Returns the ValueObject for the __tree_node type that /// holds the key/value pair of the node at index \ref idx. /// @@ -254,6 +256,29 @@ lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd:: Update(); } +llvm::Expected<uint32_t> +lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd:: + CalculateNumChildrenForOldCompressedPairLayout() { + ValueObjectSP node_sp(m_tree->GetChildMemberWithName("__pair3_")); + if (!node_sp) + return 0; + + // TODO: or should this just be: assert + // (!isOldCompressedPairLayout(*node_sp)); + if (!isOldCompressedPairLayout(*node_sp)) + return llvm::createStringError("Unexpected std::map layout: expected " + "old __compressed_pair layout."); + + node_sp = GetFirstValueOfLibCXXCompressedPair(*node_sp); + + if (!node_sp) + return 0; + + m_count = node_sp->GetValueAsUnsigned(0); + + return m_count; +} + llvm::Expected<uint32_t> lldb_private::formatters:: LibcxxStdMapSyntheticFrontEnd::CalculateNumChildren() { if (m_count != UINT32_MAX) @@ -262,17 +287,12 @@ llvm::Expected<uint32_t> lldb_private::formatters:: if (m_tree == nullptr) return 0; - ValueObjectSP size_node(m_tree->GetChildMemberWithName("__pair3_")); - if (!size_node) - return 0; - - size_node = GetFirstValueOfLibCXXCompressedPair(*size_node); - - if (!size_node) - return 0; + if (auto node_sp = m_tree->GetChildMemberWithName("__size_")) { + m_count = node_sp->GetValueAsUnsigned(0); + return m_count; + } - m_count = size_node->GetValueAsUnsigned(0); - return m_count; + return CalculateNumChildrenForOldCompressedPairLayout(); } ValueObjectSP @@ -371,6 +391,7 @@ lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::Update() { m_tree = m_backend.GetChildMemberWithName("__tree_").get(); if (!m_tree) return lldb::ChildCacheState::eRefetch; + m_root_node = m_tree->GetChildMemberWithName("__begin_node_").get(); m_node_ptr_type = m_tree->GetCompilerType().GetDirectNestedTypeWithName("__node_pointer"); diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp index 93e7f4f4fd86c..2f65c726aa51a 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp @@ -19,6 +19,7 @@ #include "lldb/Utility/Status.h" #include "lldb/Utility/Stream.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Error.h" using namespace lldb; using namespace lldb_private; @@ -44,6 +45,10 @@ class LibcxxStdUnorderedMapSyntheticFrontEnd size_t GetIndexOfChildWithName(ConstString name) override; private: + CompilerType GetNodeType(); + CompilerType GetElementType(CompilerType node_type); + llvm::Expected<size_t> CalculateNumChildrenImpl(ValueObject &table); + CompilerType m_element_type; CompilerType m_node_type; ValueObject *m_tree = nullptr; @@ -91,29 +96,53 @@ llvm::Expected<uint32_t> lldb_private::formatters:: return m_num_elements; } -static void consumeInlineNamespace(llvm::StringRef &name) { - // Delete past an inline namespace, if any: __[a-zA-Z0-9_]+:: - auto scratch = name; - if (scratch.consume_front("__") && std::isalnum(scratch[0])) { - scratch = scratch.drop_while([](char c) { return std::isalnum(c); }); - if (scratch.consume_front("::")) { - // Successfully consumed a namespace. - name = scratch; - } - } +static bool isUnorderedMap(ConstString type_name) { + return isStdTemplate(type_name, "unordered_map") || + isStdTemplate(type_name, "unordered_multimap"); } -static bool isStdTemplate(ConstString type_name, llvm::StringRef type) { - llvm::StringRef name = type_name.GetStringRef(); - // The type name may be prefixed with `std::__<inline-namespace>::`. - if (name.consume_front("std::")) - consumeInlineNamespace(name); - return name.consume_front(type) && name.starts_with("<"); +CompilerType lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEnd:: + GetElementType(CompilerType node_type) { + CompilerType element_type = node_type.GetTypeTemplateArgument(0); + + // This synthetic provider is used for both unordered_(multi)map and + // unordered_(multi)set. For unordered_map, the element type has an + // additional type layer, an internal struct (`__hash_value_type`) + // that wraps a std::pair. Peel away the internal wrapper type - whose + // structure is of no value to users, to expose the std::pair. This + // matches the structure returned by the std::map synthetic provider. + if (isUnorderedMap(m_backend.GetTypeName())) { + std::string name; + CompilerType field_type = + element_type.GetFieldAtIndex(0, name, nullptr, nullptr, nullptr); + CompilerType actual_type = field_type.GetTypedefedType(); + if (isStdTemplate(actual_type.GetTypeName(), "pair")) + element_type = actual_type; + } + + return element_type; } -static bool isUnorderedMap(ConstString type_name) { - return isStdTemplate(type_name, "unordered_map") || - isStdTemplate(type_name, "unordered_multimap"); +CompilerType lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEnd:: + GetNodeType() { + auto node_sp = m_backend.GetChildAtNamePath({"__table_", "__first_node_"}); + + if (!node_sp) { + auto p1_sp = m_backend.GetChildAtNamePath({"__table_", "__p1_"}); + if (!p1_sp) + return {}; + + if (!isOldCompressedPairLayout(*p1_sp)) + return {}; + + node_sp = GetFirstValueOfLibCXXCompressedPair(*p1_sp); + if (!node_sp) + return {}; + } + + assert(node_sp); + + return node_sp->GetCompilerType().GetTypeTemplateArgument(0).GetPointeeType(); } lldb::ValueObjectSP lldb_private::formatters:: @@ -136,36 +165,12 @@ lldb::ValueObjectSP lldb_private::formatters:: ValueObjectSP hash_sp = node_sp->GetChildMemberWithName("__hash_"); if (!hash_sp || !value_sp) { if (!m_element_type) { - auto p1_sp = m_backend.GetChildAtNamePath({"__table_", "__p1_"}); - if (!p1_sp) - return nullptr; - - ValueObjectSP first_sp = GetFirstValueOfLibCXXCompressedPair(*p1_sp); - if (!first_sp) + m_node_type = GetNodeType(); + if (!m_node_type) return nullptr; - m_element_type = first_sp->GetCompilerType(); - m_element_type = m_element_type.GetTypeTemplateArgument(0); - m_element_type = m_element_type.GetPointeeType(); - m_node_type = m_element_type; - m_element_type = m_element_type.GetTypeTemplateArgument(0); - // This synthetic provider is used for both unordered_(multi)map and - // unordered_(multi)set. For unordered_map, the element type has an - // additional type layer, an internal struct (`__hash_value_type`) - // that wraps a std::pair. Peel away the internal wrapper type - whose - // structure is of no value to users, to expose the std::pair. This - // matches the structure returned by the std::map synthetic provider. - if (isUnorderedMap(m_backend.GetTypeName())) { - std::string name; - CompilerType field_type = m_element_type.GetFieldAtIndex( - 0, name, nullptr, nullptr, nullptr); - CompilerType actual_type = field_type.GetTypedefedType(); - if (isStdTemplate(actual_type.GetTypeName(), "pair")) - m_element_type = actual_type; - } + m_element_type = GetElementType(m_node_type); } - if (!m_node_type) - return nullptr; node_sp = m_next_element->Cast(m_node_type.GetPointerType()) ->Dereference(error); if (!node_sp || error.Fail()) @@ -217,6 +222,49 @@ lldb::ValueObjectSP lldb_private::formatters:: ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/96538 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
