-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[lldb][Formatters] Consistently unwrap pointer element_type in std::shared_ptr formatters #147340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[lldb][Formatters] Consistently unwrap pointer element_type in std::shared_ptr formatters #147340
Conversation
…hared_ptr formatters
| lldb_private::formatters::LibcxxSharedPtrSyntheticFrontEnd:: | ||
| GetIndexOfChildWithName(ConstString name) { | ||
| if (name == "__ptr_" || name == "pointer") | ||
| if (name == "pointer") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The __ptr_ here feels redundant. Happy to do this in a separate PR though.
|
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesCurrently when we explicitly dereference a std::shared_ptr, both the libstdc++ and libc++ formatters will cast the type of the synthetic pointer child to whatever the However, when we print (or dereference) This patch changes both formatters to store the casted type. Then Full diff: https://github.com/llvm/llvm-project/pull/147340.diff 5 Files Affected:
diff --git a/lldb/include/lldb/DataFormatters/FormattersHelpers.h b/lldb/include/lldb/DataFormatters/FormattersHelpers.h
index 6a47cf37eac64..c7ce199452e1d 100644
--- a/lldb/include/lldb/DataFormatters/FormattersHelpers.h
+++ b/lldb/include/lldb/DataFormatters/FormattersHelpers.h
@@ -63,6 +63,11 @@ void DumpCxxSmartPtrPointerSummary(Stream &stream, ValueObject &ptr,
bool ContainerSizeSummaryProvider(ValueObject &valobj, Stream &stream,
const TypeSummaryOptions &options);
+/// Return the ValueObjectSP of the underlying pointer member whose type
+/// is a desugared 'std::shared_ptr::element_type *'.
+lldb::ValueObjectSP GetCxxSmartPtrElementPointerType(ValueObject &ptr,
+ ValueObject &container);
+
Address GetArrayAddressOrPointerValue(ValueObject &valobj);
time_t GetOSXEpoch();
diff --git a/lldb/source/DataFormatters/FormattersHelpers.cpp b/lldb/source/DataFormatters/FormattersHelpers.cpp
index 028fc5da73dc0..104c2ce6f1c0a 100644
--- a/lldb/source/DataFormatters/FormattersHelpers.cpp
+++ b/lldb/source/DataFormatters/FormattersHelpers.cpp
@@ -151,3 +151,16 @@ bool lldb_private::formatters::ContainerSizeSummaryProvider(
return FormatEntity::FormatStringRef("size=${svar%#}", stream, nullptr,
nullptr, nullptr, &valobj, false, false);
}
+
+ValueObjectSP lldb_private::formatters::GetCxxSmartPtrElementPointerType(
+ ValueObject &ptr, ValueObject &container) {
+ auto container_type = container.GetCompilerType().GetNonReferenceType();
+ if (!container_type)
+ return nullptr;
+
+ auto arg = container_type.GetTypeTemplateArgument(0);
+ if (!arg)
+ return nullptr;
+
+ return ptr.Cast(arg.GetPointerType());
+}
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 3079de4936a85..f73a77836034f 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -264,11 +264,7 @@ lldb_private::formatters::LibcxxSharedPtrSyntheticFrontEnd::GetChildAtIndex(
if (idx == 1) {
Status status;
- auto value_type_sp = valobj_sp->GetCompilerType()
- .GetTypeTemplateArgument(0)
- .GetPointerType();
- ValueObjectSP cast_ptr_sp = m_ptr_obj->Cast(value_type_sp);
- ValueObjectSP value_sp = cast_ptr_sp->Dereference(status);
+ ValueObjectSP value_sp = m_ptr_obj->Dereference(status);
if (status.Success())
return value_sp;
}
@@ -293,7 +289,11 @@ lldb_private::formatters::LibcxxSharedPtrSyntheticFrontEnd::Update() {
if (!ptr_obj_sp)
return lldb::ChildCacheState::eRefetch;
- m_ptr_obj = ptr_obj_sp->Clone(ConstString("pointer")).get();
+ auto cast_ptr_sp = GetCxxSmartPtrElementPointerType(*ptr_obj_sp, *valobj_sp);
+ if (!cast_ptr_sp)
+ return lldb::ChildCacheState::eRefetch;
+
+ m_ptr_obj = cast_ptr_sp->Clone(ConstString("pointer")).get();
lldb::ValueObjectSP cntrl_sp(valobj_sp->GetChildMemberWithName("__cntrl_"));
@@ -305,7 +305,7 @@ lldb_private::formatters::LibcxxSharedPtrSyntheticFrontEnd::Update() {
llvm::Expected<size_t>
lldb_private::formatters::LibcxxSharedPtrSyntheticFrontEnd::
GetIndexOfChildWithName(ConstString name) {
- if (name == "__ptr_" || name == "pointer")
+ if (name == "pointer")
return 0;
if (name == "object" || name == "$$dereference$$")
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index 84442273aebd4..d66b8339809be 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -273,11 +273,7 @@ LibStdcppSharedPtrSyntheticFrontEnd::GetChildAtIndex(uint32_t idx) {
return nullptr;
Status status;
- auto value_type_sp = valobj_sp->GetCompilerType()
- .GetTypeTemplateArgument(0)
- .GetPointerType();
- ValueObjectSP cast_ptr_sp = m_ptr_obj->Cast(value_type_sp);
- ValueObjectSP value_sp = cast_ptr_sp->Dereference(status);
+ ValueObjectSP value_sp = m_ptr_obj->Dereference(status);
if (status.Success())
return value_sp;
}
@@ -297,7 +293,11 @@ lldb::ChildCacheState LibStdcppSharedPtrSyntheticFrontEnd::Update() {
if (!ptr_obj_sp)
return lldb::ChildCacheState::eRefetch;
- m_ptr_obj = ptr_obj_sp->Clone(ConstString("pointer")).get();
+ auto cast_ptr_sp = GetCxxSmartPtrElementPointerType(*ptr_obj_sp, *valobj_sp);
+ if (!cast_ptr_sp)
+ return lldb::ChildCacheState::eRefetch;
+
+ m_ptr_obj = cast_ptr_sp->Clone(ConstString("pointer")).get();
return lldb::ChildCacheState::eRefetch;
}
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/shared_ptr/TestDataFormatterStdSharedPtr.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/shared_ptr/TestDataFormatterStdSharedPtr.py
index 8b641c9643958..3d8569da0332e 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/shared_ptr/TestDataFormatterStdSharedPtr.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/shared_ptr/TestDataFormatterStdSharedPtr.py
@@ -99,6 +99,10 @@ def do_test(self):
"wie", type="std::weak_ptr<int>", summary="nullptr strong=2 weak=2"
)
+ self.expect_var_path("si.pointer", type="int *")
+ self.expect_var_path("*si.pointer", type="int", value="47")
+ self.expect_var_path("si.object", type="int", value="47")
+
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")
|
|
I'm not entirely comfortable with putting the (obviously c++ specific) helper function into the DataFormatters) library. There's no reason to call this from outside the c++ language plugin, right? What would you say to putting this into some header inside the plugin (or creating a new one if we have nothing suitable)? |
Yup that makes sense! |
Put it into |
labath
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find Generic.h a bit... generic, but it's there, so ship it.
|
|
||
| /// Return the ValueObjectSP of the underlying pointer member whose type | ||
| /// is a desugared 'std::shared_ptr::element_type *'. | ||
| lldb::ValueObjectSP GetCxxSmartPtrElementPointerType(ValueObject &ptr, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GetDesugaredSmartPointerValue ?
…hared_ptr formatters (llvm#147340) Follow-up to llvm#147165 (review) Currently when we explicitly dereference a std::shared_ptr, both the libstdc++ and libc++ formatters will cast the type of the synthetic pointer child to whatever the `std::shared_ptr::element_type` is aliased to. E.g., ``` (lldb) v p (std::shared_ptr<int>) p = 10 strong=1 weak=0 { pointer = 0x000000010016c6a0 } (lldb) v *p (int) *p = 10 ``` However, when we print (or dereference) `p.pointer`, the type devolves to something less user-friendly: ``` (lldb) v p.pointer (std::shared_ptr<int>::element_type *) p.pointer = 0x000000010016c6a0 (lldb) v *p.pointer (std::shared_ptr<int>::element_type) *p.pointer = 10 ``` This patch changes both formatters to store the casted type. Then `GetChildAtIndex` will consistently use the unwrapped type. (cherry picked from commit f999918)
Follow-up to #147165 (review)
Currently when we explicitly dereference a std::shared_ptr, both the libstdc++ and libc++ formatters will cast the type of the synthetic pointer child to whatever the
std::shared_ptr::element_typeis aliased to. E.g.,However, when we print (or dereference)
p.pointer, the type devolves to something less user-friendly:This patch changes both formatters to store the casted type. Then
GetChildAtIndexwill consistently use the unwrapped type.