https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/146909
>From 9963727a56237f9424642bb1cc12e800640fbd0e Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Thu, 3 Jul 2025 16:07:11 +0100 Subject: [PATCH 1/3] [lldb][DataFormatter] Format libstdc++ unique_ptr like we do libc++ The only difference is that with libc++ the summary string contains the derefernced pointer value. With libstdc++ we currently display the pointer itself, which seems redundant. E.g., ``` (std::unique_ptr<int>) iup = 0x55555556d2b0 { pointer = 0x000055555556d2b0 } (std::unique_ptr<std::basic_string<char> >) sup = 0x55555556d2d0 { pointer = "foobar" } ``` This patch moves the logic into a common helper that's shared between the libc++ and libstdc++ formatters. --- .../lldb/DataFormatters/FormattersHelpers.h | 6 +++ .../DataFormatters/FormattersHelpers.cpp | 22 +++++++++++ .../Plugins/Language/CPlusPlus/LibCxx.cpp | 37 +------------------ .../CPlusPlus/LibStdcppUniquePointer.cpp | 10 +---- .../TestDataFormatterStdUniquePtr.py | 14 +++---- 5 files changed, 39 insertions(+), 50 deletions(-) diff --git a/lldb/include/lldb/DataFormatters/FormattersHelpers.h b/lldb/include/lldb/DataFormatters/FormattersHelpers.h index 42699d0a0b1b1..ea77b43e8da39 100644 --- a/lldb/include/lldb/DataFormatters/FormattersHelpers.h +++ b/lldb/include/lldb/DataFormatters/FormattersHelpers.h @@ -55,6 +55,12 @@ void AddFilter(TypeCategoryImpl::SharedPointer category_sp, std::optional<size_t> ExtractIndexFromString(const char *item_name); +/// Returns \c false if the smart pointer formatters shouldn't continue +/// formatting the rest of the object. Currently this is the case when the +/// pointer is a nullptr. +bool DumpCxxSmartPtrPointerSummary(Stream &stream, ValueObject &ptr, + const TypeSummaryOptions &options); + Address GetArrayAddressOrPointerValue(ValueObject &valobj); time_t GetOSXEpoch(); diff --git a/lldb/source/DataFormatters/FormattersHelpers.cpp b/lldb/source/DataFormatters/FormattersHelpers.cpp index d7b058d91c4a3..022a5d12efae3 100644 --- a/lldb/source/DataFormatters/FormattersHelpers.cpp +++ b/lldb/source/DataFormatters/FormattersHelpers.cpp @@ -126,3 +126,25 @@ lldb_private::formatters::GetArrayAddressOrPointerValue(ValueObject &valobj) { return data_addr.address; } + +bool lldb_private::formatters::DumpCxxSmartPtrPointerSummary( + Stream &stream, ValueObject &ptr, const TypeSummaryOptions &options) { + if (ptr.GetValueAsUnsigned(0) == 0) { + stream.Printf("nullptr"); + return true; + } + + Status error; + ValueObjectSP pointee_sp = ptr.Dereference(error); + // FIXME: should we be handling this as an error? + if (!pointee_sp || !error.Success()) + return false; + + if (!pointee_sp->DumpPrintableRepresentation( + stream, ValueObject::eValueObjectRepresentationStyleSummary, + lldb::eFormatInvalid, + ValueObject::PrintableRepresentationSpecialCases::eDisable, false)) + stream.Printf("ptr = 0x%" PRIx64, ptr.GetValueAsUnsigned(0)); + + return false; +} diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp index 7143089209dd3..995c943ac4da4 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp @@ -166,24 +166,8 @@ bool lldb_private::formatters::LibcxxSmartPointerSummaryProvider( if (!ptr_sp) return false; - if (ptr_sp->GetValueAsUnsigned(0) == 0) { - stream.Printf("nullptr"); + if (!DumpCxxSmartPtrPointerSummary(stream, *ptr_sp, options)) return true; - } else { - bool print_pointee = false; - Status error; - ValueObjectSP pointee_sp = ptr_sp->Dereference(error); - if (pointee_sp && error.Success()) { - if (pointee_sp->DumpPrintableRepresentation( - stream, ValueObject::eValueObjectRepresentationStyleSummary, - lldb::eFormatInvalid, - ValueObject::PrintableRepresentationSpecialCases::eDisable, - false)) - print_pointee = true; - } - if (!print_pointee) - stream.Printf("ptr = 0x%" PRIx64, ptr_sp->GetValueAsUnsigned(0)); - } if (count_sp) stream.Printf(" strong=%" PRIu64, 1 + count_sp->GetValueAsUnsigned(0)); @@ -210,24 +194,7 @@ bool lldb_private::formatters::LibcxxUniquePointerSummaryProvider( if (!ptr_sp) return false; - if (ptr_sp->GetValueAsUnsigned(0) == 0) { - stream.Printf("nullptr"); - return true; - } else { - bool print_pointee = false; - Status error; - ValueObjectSP pointee_sp = ptr_sp->Dereference(error); - if (pointee_sp && error.Success()) { - if (pointee_sp->DumpPrintableRepresentation( - stream, ValueObject::eValueObjectRepresentationStyleSummary, - lldb::eFormatInvalid, - ValueObject::PrintableRepresentationSpecialCases::eDisable, - false)) - print_pointee = true; - } - if (!print_pointee) - stream.Printf("ptr = 0x%" PRIx64, ptr_sp->GetValueAsUnsigned(0)); - } + DumpCxxSmartPtrPointerSummary(stream, *ptr_sp, options); return true; } diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp index b47ff579b6a5e..e570a4bb1a886 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp @@ -156,14 +156,8 @@ bool LibStdcppUniquePtrSyntheticFrontEnd::GetSummary( if (!m_ptr_obj) return false; - bool success; - uint64_t ptr_value = m_ptr_obj->GetValueAsUnsigned(0, &success); - if (!success) - return false; - if (ptr_value == 0) - stream.Printf("nullptr"); - else - stream.Printf("0x%" PRIx64, ptr_value); + DumpCxxSmartPtrPointerSummary(stream, *m_ptr_obj, options); + return true; } diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py index 2301e5edfbd90..600a8669c81ba 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py @@ -31,15 +31,15 @@ def test_with_run_command(self): self.assertTrue(frame.IsValid()) self.expect("frame variable nup", substrs=["nup = nullptr"]) - self.expect("frame variable iup", substrs=["iup = 0x"]) - self.expect("frame variable sup", substrs=["sup = 0x"]) + self.expect("frame variable iup", substrs=["iup = 123"]) + self.expect("frame variable sup", substrs=['sup = "foobar"']) self.expect("frame variable ndp", substrs=["ndp = nullptr"]) self.expect( - "frame variable idp", substrs=["idp = 0x", "deleter = ", "a = 1", "b = 2"] + "frame variable idp", substrs=["idp = 456", "deleter = ", "a = 1", "b = 2"] ) self.expect( - "frame variable sdp", substrs=["sdp = 0x", "deleter = ", "a = 3", "b = 4"] + "frame variable sdp", substrs=['sdp = "baz"', "deleter = ", "a = 3", "b = 4"] ) self.assertEqual( @@ -106,13 +106,13 @@ def test_recursive_unique_ptr(self): substrs=["stopped", "stop reason = breakpoint"], ) - self.expect("frame variable f1->fp", substrs=["fp = 0x"]) + self.expect("frame variable f1->fp", substrs=["fp = Foo @ 0x"]) self.expect( - "frame variable --ptr-depth=1 f1->fp", substrs=["data = 2", "fp = 0x"] + "frame variable --ptr-depth=1 f1->fp", substrs=["data = 2", "fp = Foo @ 0x"] ) self.expect( "frame variable --ptr-depth=2 f1->fp", - substrs=["data = 2", "fp = 0x", "data = 1"], + substrs=["data = 2", "fp = Foo @ 0x", "data = 1"], ) frame = self.frame() >From 63066cc6e1f940f38728ab722ef8a0c72e6c8215 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Thu, 3 Jul 2025 15:49:52 +0100 Subject: [PATCH 2/3] fixup! python format --- .../libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py index 600a8669c81ba..8f57dc88f3187 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py @@ -2,7 +2,6 @@ Test lldb data formatter subsystem. """ - import lldb from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * @@ -39,7 +38,8 @@ def test_with_run_command(self): "frame variable idp", substrs=["idp = 456", "deleter = ", "a = 1", "b = 2"] ) self.expect( - "frame variable sdp", substrs=['sdp = "baz"', "deleter = ", "a = 3", "b = 4"] + "frame variable sdp", + substrs=['sdp = "baz"', "deleter = ", "a = 3", "b = 4"], ) self.assertEqual( >From 84de12d24c7862542a7fe91b47c2721b65a1e968 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Thu, 3 Jul 2025 17:18:04 +0100 Subject: [PATCH 3/3] fixup! remove early-return for empty shared_ptrs; refactor weak/strong count printing --- .../lldb/DataFormatters/FormattersHelpers.h | 7 ++-- .../DataFormatters/FormattersHelpers.cpp | 9 ++--- .../Plugins/Language/CPlusPlus/LibCxx.cpp | 37 +++++++++++++------ .../TestDataFormatterLibcxxSharedPtr.py | 14 +++++++ .../libcxx/shared_ptr/main.cpp | 4 ++ 5 files changed, 50 insertions(+), 21 deletions(-) diff --git a/lldb/include/lldb/DataFormatters/FormattersHelpers.h b/lldb/include/lldb/DataFormatters/FormattersHelpers.h index ea77b43e8da39..97d367afc65d7 100644 --- a/lldb/include/lldb/DataFormatters/FormattersHelpers.h +++ b/lldb/include/lldb/DataFormatters/FormattersHelpers.h @@ -55,10 +55,9 @@ void AddFilter(TypeCategoryImpl::SharedPointer category_sp, std::optional<size_t> ExtractIndexFromString(const char *item_name); -/// Returns \c false if the smart pointer formatters shouldn't continue -/// formatting the rest of the object. Currently this is the case when the -/// pointer is a nullptr. -bool DumpCxxSmartPtrPointerSummary(Stream &stream, ValueObject &ptr, +/// Prints the summary for the pointer value of a C++ +/// std::unique_ptr/std::shared_ptr/std::weak_ptr. +void DumpCxxSmartPtrPointerSummary(Stream &stream, ValueObject &ptr, const TypeSummaryOptions &options); Address GetArrayAddressOrPointerValue(ValueObject &valobj); diff --git a/lldb/source/DataFormatters/FormattersHelpers.cpp b/lldb/source/DataFormatters/FormattersHelpers.cpp index 022a5d12efae3..81c76b06fc47a 100644 --- a/lldb/source/DataFormatters/FormattersHelpers.cpp +++ b/lldb/source/DataFormatters/FormattersHelpers.cpp @@ -127,24 +127,21 @@ lldb_private::formatters::GetArrayAddressOrPointerValue(ValueObject &valobj) { return data_addr.address; } -bool lldb_private::formatters::DumpCxxSmartPtrPointerSummary( +void lldb_private::formatters::DumpCxxSmartPtrPointerSummary( Stream &stream, ValueObject &ptr, const TypeSummaryOptions &options) { if (ptr.GetValueAsUnsigned(0) == 0) { stream.Printf("nullptr"); - return true; + return; } Status error; ValueObjectSP pointee_sp = ptr.Dereference(error); - // FIXME: should we be handling this as an error? if (!pointee_sp || !error.Success()) - return false; + return; if (!pointee_sp->DumpPrintableRepresentation( stream, ValueObject::eValueObjectRepresentationStyleSummary, lldb::eFormatInvalid, ValueObject::PrintableRepresentationSpecialCases::eDisable, false)) stream.Printf("ptr = 0x%" PRIx64, ptr.GetValueAsUnsigned(0)); - - return false; } diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp index 995c943ac4da4..009586f525282 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp @@ -157,23 +157,38 @@ bool lldb_private::formatters::LibcxxSmartPointerSummaryProvider( ValueObjectSP valobj_sp(valobj.GetNonSyntheticValue()); if (!valobj_sp) return false; - ValueObjectSP ptr_sp(valobj_sp->GetChildMemberWithName("__ptr_")); - ValueObjectSP count_sp( - valobj_sp->GetChildAtNamePath({"__cntrl_", "__shared_owners_"})); - ValueObjectSP weakcount_sp( - valobj_sp->GetChildAtNamePath({"__cntrl_", "__shared_weak_owners_"})); - if (!ptr_sp) + ValueObjectSP ptr_sp(valobj_sp->GetChildMemberWithName("__ptr_")); + ValueObjectSP ctrl_sp(valobj_sp->GetChildMemberWithName("__cntrl_")); + if (!ctrl_sp || !ptr_sp) return false; - if (!DumpCxxSmartPtrPointerSummary(stream, *ptr_sp, options)) + DumpCxxSmartPtrPointerSummary(stream, *ptr_sp, options); + + bool success; + uint64_t ctrl_addr = ctrl_sp->GetValueAsUnsigned(0, &success); + // Empty control field. We're done. + if (!success || ctrl_addr == 0) return true; - if (count_sp) - stream.Printf(" strong=%" PRIu64, 1 + count_sp->GetValueAsUnsigned(0)); + if (auto count_sp = ctrl_sp->GetChildMemberWithName("__shared_owners_")) { + bool success; + uint64_t count = count_sp->GetValueAsUnsigned(0, &success); + if (!success) + return false; + + stream.Printf(" strong=%" PRIu64, count + 1); + } - if (weakcount_sp) - stream.Printf(" weak=%" PRIu64, 1 + weakcount_sp->GetValueAsUnsigned(0)); + if (auto weak_count_sp = + ctrl_sp->GetChildMemberWithName("__shared_weak_owners_")) { + bool success; + uint64_t count = weak_count_sp->GetValueAsUnsigned(0, &success); + if (!success) + return false; + + stream.Printf(" weak=%" PRIu64, count + 1); + } return true; } diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py index 61c050b3bfa01..cc3bffca8307a 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py @@ -91,6 +91,20 @@ def test_shared_ptr_variables(self): self.expect_var_path("sp_user->id", type="int", value="30") self.expect_var_path("sp_user->name", type="std::string", summary='"steph"') + #valobj = self.expect_var_path( + # "si", + # type="std::shared_ptr<int> &", + # children=[ValueCheck(name="__ptr_")], + #) + #self.assertRegex(valobj.summary, r"^10( strong=1)? weak=1$") + + #valobj = self.expect_var_path( + # "sie", + # type="std::shared_ptr<int> &", + # children=[ValueCheck(name="__ptr_")], + #) + #self.assertRegex(valobj.summary, r"^10( strong=1)? weak=1$") + self.runCmd("settings set target.experimental.use-DIL true") self.expect_var_path("ptr_node->value", value="1") self.expect_var_path("ptr_node->next->value", value="2") diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/main.cpp index 84a02ec13df1c..3ac86a3ee9843 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/main.cpp @@ -22,5 +22,9 @@ int main() { std::shared_ptr<NodeS>(new NodeS{nullptr, 2}); ptr_node = std::shared_ptr<NodeS>(new NodeS{std::move(ptr_node), 1}); + // Construct empty shared_ptr with non-null control field. + std::shared_ptr<int> si(new int(47)); + std::shared_ptr<int> sie(si, nullptr); + return 0; // break here } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits