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