llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Zequan Wu (ZequanWu) <details> <summary>Changes</summary> Currently, the type `T`'s summary formatter will be matched for `T`, `T*`, `T**` and so on. This is unexpected in many data formatters. Such unhandled cases could cause the data formatter to crash. An example would be the lldb's built-in data formatter for `std::optional`: ``` $ 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] ``` This change adds an options `--pointer-match-depth` to `type summary add` command to allow users to specify how many layer of pointers can be dereferenced at most when matching a summary formatter of type `T`, as Jim suggested [here](https://github.com/llvm/llvm-project/pull/124048/#issuecomment-2611164133). By default, this option has value 1 which means summary formatter for `T` could also be used for `T*` but not `T**` nor beyond. This option is no-op when `--skip-pointers` is set as well. I didn't add such option for `type synthetic add`, `type format add`, `type filter add`, because it useful for those command. Instead, they all have the pointer match depth of 1. When printing a type `T*`, lldb never print the children of `T` even if there is a synthetic formatter registered for `T`. --- Patch is 30.13 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/138209.diff 16 Files Affected: - (modified) lldb/include/lldb/API/SBTypeSummary.h (+4) - (modified) lldb/include/lldb/DataFormatters/FormatClasses.h (+8-2) - (modified) lldb/include/lldb/DataFormatters/FormatManager.h (+2-1) - (modified) lldb/include/lldb/DataFormatters/FormattersContainer.h (+3-5) - (modified) lldb/include/lldb/DataFormatters/TypeFormat.h (+5) - (modified) lldb/include/lldb/DataFormatters/TypeSummary.h (+13-4) - (modified) lldb/include/lldb/DataFormatters/TypeSynthetic.h (+5) - (modified) lldb/source/API/SBTypeSummary.cpp (+16) - (modified) lldb/source/Commands/CommandObjectType.cpp (+19-8) - (modified) lldb/source/Commands/Options.td (+4) - (modified) lldb/source/DataFormatters/FormatManager.cpp (+24-16) - (modified) lldb/source/DataFormatters/TypeCategoryMap.cpp (+3-2) - (modified) lldb/source/DataFormatters/TypeSummary.cpp (+23-14) - (added) lldb/test/API/functionalities/data-formatter/data-formatter-ptr-matching/Makefile (+4) - (added) lldb/test/API/functionalities/data-formatter/data-formatter-ptr-matching/TestDataFormatterPtrMatching.py (+101) - (added) lldb/test/API/functionalities/data-formatter/data-formatter-ptr-matching/main.cpp (+32) ``````````diff diff --git a/lldb/include/lldb/API/SBTypeSummary.h b/lldb/include/lldb/API/SBTypeSummary.h index 40fa146b4e31b..b6869e53a39e7 100644 --- a/lldb/include/lldb/API/SBTypeSummary.h +++ b/lldb/include/lldb/API/SBTypeSummary.h @@ -109,6 +109,10 @@ class SBTypeSummary { void SetFunctionCode(const char *data); + uint32_t GetPtrMatchDepth(); + + void SetPtrMatchDepth(uint32_t ptr_match_depth); + uint32_t GetOptions(); void SetOptions(uint32_t); diff --git a/lldb/include/lldb/DataFormatters/FormatClasses.h b/lldb/include/lldb/DataFormatters/FormatClasses.h index a6bc3a3542536..89d74649ecc64 100644 --- a/lldb/include/lldb/DataFormatters/FormatClasses.h +++ b/lldb/include/lldb/DataFormatters/FormatClasses.h @@ -76,9 +76,10 @@ class FormattersMatchCandidate { FormattersMatchCandidate(ConstString name, ScriptInterpreter *script_interpreter, TypeImpl type, - Flags flags) + Flags flags, uint32_t ptr_stripped_depth = 0) : m_type_name(name), m_script_interpreter(script_interpreter), - m_type(type), m_flags(flags) {} + m_type(type), m_flags(flags), m_ptr_stripped_depth(ptr_stripped_depth) { + } ~FormattersMatchCandidate() = default; @@ -96,6 +97,8 @@ class FormattersMatchCandidate { bool DidStripTypedef() const { return m_flags.stripped_typedef; } + uint32_t GetPtrStrippedDepth() const { return m_ptr_stripped_depth; } + template <class Formatter> bool IsMatch(const std::shared_ptr<Formatter> &formatter_sp) const { if (!formatter_sp) @@ -104,6 +107,8 @@ class FormattersMatchCandidate { return false; if (formatter_sp->SkipsPointers() && DidStripPointer()) return false; + if (formatter_sp->GetPtrMatchDepth() < GetPtrStrippedDepth()) + return false; if (formatter_sp->SkipsReferences() && DidStripReference()) return false; return true; @@ -116,6 +121,7 @@ class FormattersMatchCandidate { ScriptInterpreter *m_script_interpreter; TypeImpl m_type; Flags m_flags; + uint32_t m_ptr_stripped_depth; }; typedef std::vector<FormattersMatchCandidate> FormattersMatchVector; diff --git a/lldb/include/lldb/DataFormatters/FormatManager.h b/lldb/include/lldb/DataFormatters/FormatManager.h index db2fe99c44caf..9e24f7b722407 100644 --- a/lldb/include/lldb/DataFormatters/FormatManager.h +++ b/lldb/include/lldb/DataFormatters/FormatManager.h @@ -180,7 +180,8 @@ class FormatManager : public IFormatChangeListener { lldb::DynamicValueType use_dynamic, FormattersMatchVector &entries, FormattersMatchCandidate::Flags current_flags, - bool root_level = false); + bool root_level = false, + uint32_t ptr_stripped_depth = 0); std::atomic<uint32_t> m_last_revision; FormatCache m_format_cache; diff --git a/lldb/include/lldb/DataFormatters/FormattersContainer.h b/lldb/include/lldb/DataFormatters/FormattersContainer.h index 7898621fd18af..f7465fc65538d 100644 --- a/lldb/include/lldb/DataFormatters/FormattersContainer.h +++ b/lldb/include/lldb/DataFormatters/FormattersContainer.h @@ -193,12 +193,10 @@ template <typename ValueType> class FormattersContainer { bool Get(const FormattersMatchVector &candidates, ValueSP &entry) { for (const FormattersMatchCandidate &candidate : candidates) { if (Get(candidate, entry)) { - if (candidate.IsMatch(entry) == false) { - entry.reset(); - continue; - } else { + if (candidate.IsMatch(entry)) return true; - } + entry.reset(); + continue; } } return false; diff --git a/lldb/include/lldb/DataFormatters/TypeFormat.h b/lldb/include/lldb/DataFormatters/TypeFormat.h index 63d4765bdf270..d242f9083d608 100644 --- a/lldb/include/lldb/DataFormatters/TypeFormat.h +++ b/lldb/include/lldb/DataFormatters/TypeFormat.h @@ -133,6 +133,10 @@ class TypeFormatImpl { void SetOptions(uint32_t value) { m_flags.SetValue(value); } + uint32_t GetPtrMatchDepth() { return m_ptr_match_depth; } + + void SetPtrMatchDepth(uint32_t value) { m_ptr_match_depth = value; } + uint32_t &GetRevision() { return m_my_revision; } enum class Type { eTypeUnknown, eTypeFormat, eTypeEnum }; @@ -150,6 +154,7 @@ class TypeFormatImpl { protected: Flags m_flags; uint32_t m_my_revision = 0; + uint32_t m_ptr_match_depth = 1; private: TypeFormatImpl(const TypeFormatImpl &) = delete; diff --git a/lldb/include/lldb/DataFormatters/TypeSummary.h b/lldb/include/lldb/DataFormatters/TypeSummary.h index f4d5563516ecb..589f68c2ce314 100644 --- a/lldb/include/lldb/DataFormatters/TypeSummary.h +++ b/lldb/include/lldb/DataFormatters/TypeSummary.h @@ -253,6 +253,10 @@ class TypeSummaryImpl { void SetOptions(uint32_t value) { m_flags.SetValue(value); } + uint32_t GetPtrMatchDepth() { return m_ptr_match_depth; } + + void SetPtrMatchDepth(uint32_t value) { m_ptr_match_depth = value; } + // we are using a ValueObject* instead of a ValueObjectSP because we do not // need to hold on to this for extended periods of time and we trust the // ValueObject to stay around for as long as it is required for us to @@ -278,10 +282,12 @@ class TypeSummaryImpl { uint32_t m_my_revision = 0; Flags m_flags; - TypeSummaryImpl(Kind kind, const TypeSummaryImpl::Flags &flags); + TypeSummaryImpl(Kind kind, const TypeSummaryImpl::Flags &flags, + uint32_t ptr_match_depth = 1); private: Kind m_kind; + uint32_t m_ptr_match_depth = 1; TypeSummaryImpl(const TypeSummaryImpl &) = delete; const TypeSummaryImpl &operator=(const TypeSummaryImpl &) = delete; }; @@ -292,7 +298,8 @@ struct StringSummaryFormat : public TypeSummaryImpl { FormatEntity::Entry m_format; Status m_error; - StringSummaryFormat(const TypeSummaryImpl::Flags &flags, const char *f); + StringSummaryFormat(const TypeSummaryImpl::Flags &flags, const char *f, + uint32_t ptr_match_depth = 1); ~StringSummaryFormat() override = default; @@ -328,7 +335,8 @@ struct CXXFunctionSummaryFormat : public TypeSummaryImpl { std::string m_description; CXXFunctionSummaryFormat(const TypeSummaryImpl::Flags &flags, Callback impl, - const char *description); + const char *description, + uint32_t ptr_match_depth = 1); ~CXXFunctionSummaryFormat() override = default; @@ -373,7 +381,8 @@ struct ScriptSummaryFormat : public TypeSummaryImpl { ScriptSummaryFormat(const TypeSummaryImpl::Flags &flags, const char *function_name, - const char *python_script = nullptr); + const char *python_script = nullptr, + uint32_t ptr_match_depth = 1); ~ScriptSummaryFormat() override = default; diff --git a/lldb/include/lldb/DataFormatters/TypeSynthetic.h b/lldb/include/lldb/DataFormatters/TypeSynthetic.h index c8d7d15588065..37f02fb8f7ce5 100644 --- a/lldb/include/lldb/DataFormatters/TypeSynthetic.h +++ b/lldb/include/lldb/DataFormatters/TypeSynthetic.h @@ -273,9 +273,14 @@ class SyntheticChildren { uint32_t &GetRevision() { return m_my_revision; } + uint32_t GetPtrMatchDepth() { return m_ptr_match_depth; } + + void SetPtrMatchDepth(uint32_t value) { m_ptr_match_depth = value; } + protected: uint32_t m_my_revision = 0; Flags m_flags; + uint32_t m_ptr_match_depth = 1; private: SyntheticChildren(const SyntheticChildren &) = delete; diff --git a/lldb/source/API/SBTypeSummary.cpp b/lldb/source/API/SBTypeSummary.cpp index 856ee0ed3175b..d7acb0670018a 100644 --- a/lldb/source/API/SBTypeSummary.cpp +++ b/lldb/source/API/SBTypeSummary.cpp @@ -230,6 +230,22 @@ const char *SBTypeSummary::GetData() { return nullptr; } +uint32_t SBTypeSummary::GetPtrMatchDepth() { + LLDB_INSTRUMENT_VA(this); + + if (!IsValid()) + return 0; + return m_opaque_sp->GetPtrMatchDepth(); +} + +void SBTypeSummary::SetPtrMatchDepth(uint32_t ptr_match_depth) { + LLDB_INSTRUMENT_VA(this); + + if (!IsValid()) + return; + return m_opaque_sp->SetPtrMatchDepth(ptr_match_depth); +} + uint32_t SBTypeSummary::GetOptions() { LLDB_INSTRUMENT_VA(this); diff --git a/lldb/source/Commands/CommandObjectType.cpp b/lldb/source/Commands/CommandObjectType.cpp index 41630b61c2f0e..19cd3ff2972e9 100644 --- a/lldb/source/Commands/CommandObjectType.cpp +++ b/lldb/source/Commands/CommandObjectType.cpp @@ -51,12 +51,13 @@ class ScriptAddOptions { FormatterMatchType m_match_type; ConstString m_name; std::string m_category; + uint32_t m_ptr_match_depth; ScriptAddOptions(const TypeSummaryImpl::Flags &flags, FormatterMatchType match_type, ConstString name, - std::string catg) + std::string catg, uint32_t m_ptr_match_depth) : m_flags(flags), m_match_type(match_type), m_name(name), - m_category(catg) {} + m_category(catg), m_ptr_match_depth(m_ptr_match_depth) {} typedef std::shared_ptr<ScriptAddOptions> SharedPointer; }; @@ -146,6 +147,7 @@ class CommandObjectTypeSummaryAdd : public CommandObjectParsed, std::string m_python_function; bool m_is_add_script = false; std::string m_category; + uint32_t m_ptr_match_depth = 1; }; CommandOptions m_options; @@ -211,7 +213,7 @@ class CommandObjectTypeSummaryAdd : public CommandObjectParsed, TypeSummaryImplSP script_format; script_format = std::make_shared<ScriptSummaryFormat>( options->m_flags, funct_name_str.c_str(), - lines.CopyList(" ").c_str()); + lines.CopyList(" ").c_str(), options->m_ptr_match_depth); Status error; @@ -1178,6 +1180,13 @@ Status CommandObjectTypeSummaryAdd::CommandOptions::SetOptionValue( case 'p': m_flags.SetSkipPointers(true); break; + case 'd': + if (option_arg.getAsInteger(0, m_ptr_match_depth)) { + error = Status::FromErrorStringWithFormat( + "invalid integer value for option '%c': %s", short_option, + option_arg.data()); + } + break; case 'r': m_flags.SetSkipReferences(true); break; @@ -1266,7 +1275,8 @@ bool CommandObjectTypeSummaryAdd::Execute_ScriptSummary( (" " + m_options.m_python_function + "(valobj,internal_dict)"); script_format = std::make_shared<ScriptSummaryFormat>( - m_options.m_flags, funct_name, code.c_str()); + m_options.m_flags, funct_name, code.c_str(), + m_options.m_ptr_match_depth); ScriptInterpreter *interpreter = GetDebugger().GetScriptInterpreter(); @@ -1300,12 +1310,13 @@ bool CommandObjectTypeSummaryAdd::Execute_ScriptSummary( std::string code = " " + m_options.m_python_script; script_format = std::make_shared<ScriptSummaryFormat>( - m_options.m_flags, funct_name_str.c_str(), code.c_str()); + m_options.m_flags, funct_name_str.c_str(), code.c_str(), + m_options.m_ptr_match_depth); } else { // Use an IOHandler to grab Python code from the user auto options = std::make_unique<ScriptAddOptions>( m_options.m_flags, m_options.m_match_type, m_options.m_name, - m_options.m_category); + m_options.m_category, m_options.m_ptr_match_depth); for (auto &entry : command.entries()) { if (entry.ref().empty()) { @@ -1380,8 +1391,8 @@ bool CommandObjectTypeSummaryAdd::Execute_StringSummary( return false; } - std::unique_ptr<StringSummaryFormat> string_format( - new StringSummaryFormat(m_options.m_flags, format_cstr)); + std::unique_ptr<StringSummaryFormat> string_format(new StringSummaryFormat( + m_options.m_flags, format_cstr, m_options.m_ptr_match_depth)); if (!string_format) { result.AppendError("summary creation failed"); return false; diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index 53864ff29327d..9dd7467b4269c 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -1240,6 +1240,10 @@ let Command = "type summary add" in { Desc<"Don't show the value, just show the summary, for this type.">; def type_summary_add_skip_pointers : Option<"skip-pointers", "p">, Desc<"Don't use this format for pointers-to-type objects.">; + def type_summary_add_pointer_match_depth : Option<"pointer-match-depth", "d">, + Arg<"UnsignedInteger">, + Desc<"Specify the maximum pointer depth that this format can be apply to " + "(default to 1). It's only effective when --skip-pointers is not set.">; def type_summary_add_skip_references : Option<"skip-references", "r">, Desc<"Don't use this format for references-to-type objects.">; def type_summary_add_regex : Option<"regex", "x">, diff --git a/lldb/source/DataFormatters/FormatManager.cpp b/lldb/source/DataFormatters/FormatManager.cpp index 3b891cecb1c8b..122f2304ead24 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 root_level, + uint32_t ptr_stripped_depth) { compiler_type = compiler_type.GetTypeForFormatters(); ConstString type_name(compiler_type.GetTypeName()); // A ValueObject that couldn't be made correctly won't necessarily have a @@ -190,46 +191,51 @@ void FormatManager::GetPossibleMatches( sstring.Printf("%s:%d", type_name.AsCString(), valobj.GetBitfieldBitSize()); ConstString bitfieldname(sstring.GetString()); entries.push_back({bitfieldname, script_interpreter, - TypeImpl(compiler_type), current_flags}); + TypeImpl(compiler_type), current_flags, + ptr_stripped_depth}); } if (!compiler_type.IsMeaninglessWithoutDynamicResolution()) { entries.push_back({type_name, script_interpreter, TypeImpl(compiler_type), - current_flags}); + current_flags, ptr_stripped_depth}); ConstString display_type_name(compiler_type.GetTypeName()); if (display_type_name != type_name) entries.push_back({display_type_name, script_interpreter, - TypeImpl(compiler_type), current_flags}); + TypeImpl(compiler_type), current_flags, + ptr_stripped_depth}); } for (bool is_rvalue_ref = true, j = true; j && compiler_type.IsReferenceType(nullptr, &is_rvalue_ref); j = false) { CompilerType non_ref_type = compiler_type.GetNonReferenceType(); GetPossibleMatches(valobj, non_ref_type, use_dynamic, entries, - current_flags.WithStrippedReference()); + current_flags.WithStrippedReference(), root_level, + ptr_stripped_depth); if (non_ref_type.IsTypedefType()) { CompilerType deffed_referenced_type = non_ref_type.GetTypedefedType(); deffed_referenced_type = is_rvalue_ref ? deffed_referenced_type.GetRValueReferenceType() : deffed_referenced_type.GetLValueReferenceType(); // this is not exactly the usual meaning of stripping typedefs - GetPossibleMatches( - valobj, deffed_referenced_type, - use_dynamic, entries, current_flags.WithStrippedTypedef()); + GetPossibleMatches(valobj, deffed_referenced_type, use_dynamic, entries, + current_flags.WithStrippedTypedef(), root_level, + ptr_stripped_depth); } } if (compiler_type.IsPointerType()) { CompilerType non_ptr_type = compiler_type.GetPointeeType(); GetPossibleMatches(valobj, non_ptr_type, use_dynamic, entries, - current_flags.WithStrippedPointer()); + current_flags.WithStrippedPointer(), root_level, + ptr_stripped_depth + 1); 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(), root_level, + ptr_stripped_depth + 1); } } @@ -246,9 +252,9 @@ void FormatManager::GetPossibleMatches( CompilerType deffed_array_type = element_type.GetTypedefedType().GetArrayType(array_size); // this is not exactly the usual meaning of stripping typedefs - GetPossibleMatches( - valobj, deffed_array_type, - use_dynamic, entries, current_flags.WithStrippedTypedef()); + GetPossibleMatches(valobj, deffed_array_type, use_dynamic, entries, + current_flags.WithStrippedTypedef(), root_level, + ptr_stripped_depth); } } @@ -266,7 +272,8 @@ void FormatManager::GetPossibleMatches( if (compiler_type.IsTypedefType()) { CompilerType deffed_type = compiler_type.GetTypedefedType(); GetPossibleMatches(valobj, deffed_type, use_dynamic, entries, - current_flags.WithStrippedTypedef()); + current_flags.WithStrippedTypedef(), root_level, + ptr_stripped_depth); } if (root_level) { @@ -281,7 +288,8 @@ void FormatManager::GetPossibleMatches( if (unqual_compiler_ast_type.GetOpaqueQualType() != compiler_type.GetOpaqueQualType()) GetPossibleMatches(valobj, unqual_compiler_ast_type, use_dynamic, - entries, current_flags); + entries, current_flags, root_level, + ptr_stripped_depth); } while (false); // if all else fails, go to static type @@ -290,7 +298,7 @@ void FormatManager::GetPossibleMatches( if (static_value_sp) GetPossibleMatches(*static_value_sp.get(), static_value_sp->GetCompilerType(), use_dynamic, - entries, current_flags, true); + entries, current_flags, true, ptr_stripped_depth); } } } diff --git a/lldb/source/DataFormatters/TypeCategoryMap.cpp b/lldb/source/DataFormatters/TypeCategoryMap.cpp index ce2cf369b5be5..8682e4b7435e4 100644 --- a/lldb/source/DataFormatters/TypeCategoryMap.cpp +++ b/lldb/source/DataFormatters/TypeCategoryMap.cpp @@ -185,12 +185,13 @@ void TypeCategoryMap::Get(FormattersMatchData &match_data, ImplSP &retval) { for (auto match : match_data.GetMatchesVector()) { LLDB_LOGF( log, - "[%s] candidate match = %s %s %s %s", + "[%s] candidate match = %s %s %s %s ptr-stripped-depth=%u", __FUNCTION__, match.GetTypeName().GetCString(), match.DidStripPointer() ? "strip-pointers" : "no-strip-pointers", match.DidStripReference() ? "strip-reference" : "no-strip-reference", - match.DidStripTypedef() ? "strip-typedef" : "no-strip-typedef"); + match.DidStripTypedef() ? "strip-typedef" : "no-strip-typedef", + match.GetPtrStrippedDepth()); } } diff --git a/lldb/source/DataFormatters/TypeSummary.cpp b/lldb/source/DataFormatters/TypeSummary.cpp index 18bf81aedf2cb..6aa290698cd12 100644 --- a/lldb/source/DataFormatters/TypeSummary.cpp +++ b/lldb/source/DataFormatters/TypeSummary.cpp @@ -43,8 +43,9 @@ TypeSummaryOptions::SetCapping(lldb::TypeSummaryCapping cap) { return *this; } -TypeSummaryImpl::TypeSummaryImpl(Kind kind, const TypeSummaryImpl::Flags &flags) - : m_flags(flags), m_kind(kind) {} +TypeSummaryImpl::TypeSummaryImpl(Kind kind, const TypeSummaryImpl::Flags &flags, + uint32_t ptr_match_depth) + : m_flags(flags), m_kind(kind), m... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/138209 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits