[Lldb-commits] [lldb] [lldb][Formatters] Do not recursively dereference pointer type when creating formatter candicates list. (PR #124048)
Zequan Wu via lldb-commits
lldb-commits at lists.llvm.org
Wed Jan 22 18:53:36 PST 2025
https://github.com/ZequanWu created https://github.com/llvm/llvm-project/pull/124048
Currently, the type `T`'s formatter will be matched for `T`, `T*`, `T**` and so on. This is inconsistent with the behaviour of `SBValue::GetChildMemberWithName` which can only dereference the pointer type at most once if the sbvalue is a pointer type, because the API eventually calls `TypeSystemClang::GetIndexOfChildMemberWithName` (https://github.com/llvm/llvm-project/blob/3ef90f843fee74ff811ef88246734475f50e2073/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp#L6927-L6934) for C/C++.
The current behaviour might cause crash on pretty printers. If the pretty printer calls `SBValue::GetChildMemberWithName` when compute the synthetic children or summary string and its type is `T**`, this might cause crash as it will return nullptr or None in this case.
An example is the built-in pretty printer for libstdc++ `std::optional` when it calls `GetChildMemberWithName` on a nullptr returned from the previous call to `GetChildMemberWithName` (https://github.com/llvm/llvm-project/blob/3ef90f843fee74ff811ef88246734475f50e2073/lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp#L74-L75):
```
$ cat main.cpp
#include <optional>
int main() {
std::optional<int> o_null;
auto po_null = &o_null;
auto ppo_null = &po_null;
auto pppo_null = &ppo_null;
return 0;
}
$ clang++ -g main.cpp && lldb -o "b 8" -o "r" -o "v pppo_null"
[lldb crash]
```
After this change, data formatter for T can only be used for `T` and `T*` (if skip pointer is set to false).
>From 1948805894e006d84fbb78299574b3c7618959d8 Mon Sep 17 00:00:00 2001
From: Zequan Wu <zequanwu at google.com>
Date: Wed, 22 Jan 2025 18:32:11 -0800
Subject: [PATCH] [lldb][Formatters] Do not recursively dereference pointer
type when creating formatter candicates list.
---
lldb/include/lldb/DataFormatters/FormatManager.h | 3 ++-
lldb/source/DataFormatters/FormatManager.cpp | 10 ++++++----
.../ptr_ref_typedef/TestPtrRef2Typedef.py | 12 ++++++++++++
.../data-formatter/ptr_ref_typedef/main.cpp | 3 +++
4 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/lldb/include/lldb/DataFormatters/FormatManager.h b/lldb/include/lldb/DataFormatters/FormatManager.h
index db2fe99c44cafc..9329ee930125a6 100644
--- a/lldb/include/lldb/DataFormatters/FormatManager.h
+++ b/lldb/include/lldb/DataFormatters/FormatManager.h
@@ -163,7 +163,7 @@ class FormatManager : public IFormatChangeListener {
GetPossibleMatches(ValueObject &valobj, lldb::DynamicValueType use_dynamic) {
FormattersMatchVector matches;
GetPossibleMatches(valobj, valobj.GetCompilerType(), use_dynamic, matches,
- FormattersMatchCandidate::Flags(), true);
+ FormattersMatchCandidate::Flags(), true, true);
return matches;
}
@@ -180,6 +180,7 @@ class FormatManager : public IFormatChangeListener {
lldb::DynamicValueType use_dynamic,
FormattersMatchVector &entries,
FormattersMatchCandidate::Flags current_flags,
+ bool dereference_ptr = true,
bool root_level = false);
std::atomic<uint32_t> m_last_revision;
diff --git a/lldb/source/DataFormatters/FormatManager.cpp b/lldb/source/DataFormatters/FormatManager.cpp
index 3b891cecb1c8b9..d6d6935f3e002c 100644
--- a/lldb/source/DataFormatters/FormatManager.cpp
+++ b/lldb/source/DataFormatters/FormatManager.cpp
@@ -174,7 +174,8 @@ void FormatManager::DisableAllCategories() {
void FormatManager::GetPossibleMatches(
ValueObject &valobj, CompilerType compiler_type,
lldb::DynamicValueType use_dynamic, FormattersMatchVector &entries,
- FormattersMatchCandidate::Flags current_flags, bool root_level) {
+ FormattersMatchCandidate::Flags current_flags, bool dereference_ptr,
+ bool root_level) {
compiler_type = compiler_type.GetTypeForFormatters();
ConstString type_name(compiler_type.GetTypeName());
// A ValueObject that couldn't be made correctly won't necessarily have a
@@ -222,14 +223,15 @@ void FormatManager::GetPossibleMatches(
if (compiler_type.IsPointerType()) {
CompilerType non_ptr_type = compiler_type.GetPointeeType();
- GetPossibleMatches(valobj, non_ptr_type, use_dynamic, entries,
- current_flags.WithStrippedPointer());
+ if (dereference_ptr)
+ GetPossibleMatches(valobj, non_ptr_type, use_dynamic, entries,
+ current_flags.WithStrippedPointer(), false);
if (non_ptr_type.IsTypedefType()) {
CompilerType deffed_pointed_type =
non_ptr_type.GetTypedefedType().GetPointerType();
// this is not exactly the usual meaning of stripping typedefs
GetPossibleMatches(valobj, deffed_pointed_type, use_dynamic, entries,
- current_flags.WithStrippedTypedef());
+ current_flags.WithStrippedTypedef(), dereference_ptr);
}
}
diff --git a/lldb/test/API/functionalities/data-formatter/ptr_ref_typedef/TestPtrRef2Typedef.py b/lldb/test/API/functionalities/data-formatter/ptr_ref_typedef/TestPtrRef2Typedef.py
index f70162bf285839..fea40252a2a75c 100644
--- a/lldb/test/API/functionalities/data-formatter/ptr_ref_typedef/TestPtrRef2Typedef.py
+++ b/lldb/test/API/functionalities/data-formatter/ptr_ref_typedef/TestPtrRef2Typedef.py
@@ -53,3 +53,15 @@ def cleanup():
# the match.
self.expect("frame variable y", substrs=["(Foo &", ") y = 0x", "IntLRef"])
self.expect("frame variable z", substrs=["(Foo &&", ") z = 0x", "IntRRef"])
+
+ # Test lldb doesn't dereference pointer more than once.
+ # xp has type Foo**, so it can only uses summary for Foo* or int*, not
+ # summary for Foo or int.
+ self.expect("frame variable xp", substrs=["(Foo **) xp = 0x", "IntPointer"])
+
+ self.runCmd('type summary delete "int *"')
+ self.runCmd('type summary add --cascade true -s "MyInt" "int"')
+ self.expect("frame variable xp", substrs=["(Foo **) xp = 0x"])
+
+ self.runCmd('type summary add --cascade true -s "FooPointer" "Foo *"')
+ self.expect("frame variable xp", substrs=["(Foo **) xp = 0x", "FooPointer"])
diff --git a/lldb/test/API/functionalities/data-formatter/ptr_ref_typedef/main.cpp b/lldb/test/API/functionalities/data-formatter/ptr_ref_typedef/main.cpp
index 13102106551086..41b1d282344b91 100644
--- a/lldb/test/API/functionalities/data-formatter/ptr_ref_typedef/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/ptr_ref_typedef/main.cpp
@@ -5,6 +5,9 @@ int main() {
Foo* x = &lval;
Foo& y = lval;
Foo&& z = 1;
+
+ // Test lldb doesn't dereference pointer more than once.
+ Foo** xp = &x;
return 0; // Set breakpoint here
}
More information about the lldb-commits
mailing list