llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: nerix (Nerixyz) <details> <summary>Changes</summary> This PR adds type summaries for `std::{string,wstring,u8string,u16string,u32string}` from the MSVC STL. See https://github.com/llvm/llvm-project/issues/24834 for the MSVC STL issue. The following changes were made: - Added a new type summary kind `CXXCompositeSummaryFormat`. It holds a list of child summaries paired with a validator function that checks if a `ValueObject` can be formatted by a child. This feels like it should be done in another PR, but there aren't any tests for it other than the STL formatters (since it's only in C++ it can't be used in Python). - As part of this, the formatter data (callback/format string) from `CXXFunctionSummaryFormat` and `StringSummaryFormat` was extracted to `CXXFunctionSummaryData` and `StringSummaryData` respectively. This allows the composite format to use the same data and `FormatObject` of said data. - `dotest.py` now detects if the MSVC STL is available. It does so by looking at the target triple, which is an additional argument passed from Lit. It specifically checks for `windows-msvc` to not match on `windows-gnu` (i.e. MinGW/Cygwin). - (The main part): Added support for summarizing `std::(w)string` from MSVC's STL. Because the type names from the libstdc++ (pre C++ 11) string types are the same as on MSVC's STL, `CXXCompositeSummaryFormat` is used with two entries, one for MSVC's STL and one for libstdc++. With MSVC's STL, `std::u{8,16,32}string` is also handled. These aren't handled for libstdc++, so I put them in `LoadMsvcStlFormatters`. --- Patch is 53.05 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/143177.diff 21 Files Affected: - (modified) lldb/include/lldb/DataFormatters/StringPrinter.h (+15) - (modified) lldb/include/lldb/DataFormatters/TypeSummary.h (+91-11) - (modified) lldb/include/lldb/DataFormatters/ValueObjectPrinter.h (+1-1) - (modified) lldb/packages/Python/lldbsuite/test/configuration.py (+2) - (modified) lldb/packages/Python/lldbsuite/test/dotest.py (+23) - (modified) lldb/packages/Python/lldbsuite/test/dotest_args.py (+6) - (modified) lldb/packages/Python/lldbsuite/test/test_categories.py (+1) - (modified) lldb/source/API/SBTypeSummary.cpp (+10-1) - (modified) lldb/source/Commands/CommandObjectType.cpp (+4-3) - (modified) lldb/source/DataFormatters/TypeSummary.cpp (+112-31) - (modified) lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt (+1) - (modified) lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp (+115-31) - (added) lldb/source/Plugins/Language/CPlusPlus/MsvcStl.cpp (+140) - (added) lldb/source/Plugins/Language/CPlusPlus/MsvcStl.h (+33) - (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/string/Makefile (+4) - (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/string/TestDataFormatterStdString.py (+118) - (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/string/main.cpp (+40) - (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/u8string/Makefile (+4) - (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/u8string/TestDataFormatterStdU8String.py (+31) - (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/u8string/main.cpp (+14) - (modified) lldb/test/API/lit.cfg.py (+3) ``````````diff diff --git a/lldb/include/lldb/DataFormatters/StringPrinter.h b/lldb/include/lldb/DataFormatters/StringPrinter.h index 4169f53e63f38..4ebe712be60e1 100644 --- a/lldb/include/lldb/DataFormatters/StringPrinter.h +++ b/lldb/include/lldb/DataFormatters/StringPrinter.h @@ -152,6 +152,21 @@ class StringPrinter { template <StringElementType element_type> static bool ReadBufferAndDumpToStream(const ReadBufferAndDumpToStreamOptions &options); + + template <StringElementType element_type> + static constexpr uint64_t ElementByteSize() { + switch (element_type) { + case StringElementType::ASCII: + case StringElementType::UTF8: + return 1; + case StringElementType::UTF16: + return 2; + case StringElementType::UTF32: + return 3; + default: + return 0; + } + } }; } // namespace formatters diff --git a/lldb/include/lldb/DataFormatters/TypeSummary.h b/lldb/include/lldb/DataFormatters/TypeSummary.h index 589f68c2ce314..ea32b31c0b2ae 100644 --- a/lldb/include/lldb/DataFormatters/TypeSummary.h +++ b/lldb/include/lldb/DataFormatters/TypeSummary.h @@ -48,7 +48,14 @@ class TypeSummaryOptions { class TypeSummaryImpl { public: - enum class Kind { eSummaryString, eScript, eBytecode, eCallback, eInternal }; + enum class Kind { + eSummaryString, + eScript, + eBytecode, + eCallback, + eComposite, + eInternal + }; virtual ~TypeSummaryImpl() = default; @@ -292,18 +299,29 @@ class TypeSummaryImpl { const TypeSummaryImpl &operator=(const TypeSummaryImpl &) = delete; }; -// simple string-based summaries, using ${var to show data -struct StringSummaryFormat : public TypeSummaryImpl { +struct StringSummaryData { std::string m_format_str; FormatEntity::Entry m_format; Status m_error; + StringSummaryData(const char *f); + void SetFormat(const char *f); + + bool FormatObject(ValueObject *valobj, std::string &dest, + const TypeSummaryOptions &options, + const TypeSummaryImpl &summary); +}; + +// simple string-based summaries, using ${var to show data +struct StringSummaryFormat : public TypeSummaryImpl { + StringSummaryData m_data; + StringSummaryFormat(const TypeSummaryImpl::Flags &flags, const char *f, uint32_t ptr_match_depth = 1); ~StringSummaryFormat() override = default; - const char *GetSummaryString() const { return m_format_str.c_str(); } + const char *GetSummaryString() const { return m_data.m_format_str.c_str(); } void SetSummaryString(const char *f); @@ -323,15 +341,23 @@ struct StringSummaryFormat : public TypeSummaryImpl { const StringSummaryFormat &operator=(const StringSummaryFormat &) = delete; }; -// summaries implemented via a C++ function -struct CXXFunctionSummaryFormat : public TypeSummaryImpl { +struct CXXFunctionSummaryData { // we should convert these to SBValue and SBStream if we ever cross the // boundary towards the external world - typedef std::function<bool(ValueObject &, Stream &, - const TypeSummaryOptions &)> - Callback; + using Callback = + std::function<bool(ValueObject &, Stream &, const TypeSummaryOptions &)>; Callback m_impl; + + bool FormatObject(ValueObject *valobj, std::string &dest, + const TypeSummaryOptions &options, + const TypeSummaryImpl &summary); +}; + +// summaries implemented via a C++ function +struct CXXFunctionSummaryFormat : public TypeSummaryImpl { + using Callback = CXXFunctionSummaryData::Callback; + CXXFunctionSummaryData m_data; std::string m_description; CXXFunctionSummaryFormat(const TypeSummaryImpl::Flags &flags, Callback impl, @@ -340,11 +366,13 @@ struct CXXFunctionSummaryFormat : public TypeSummaryImpl { ~CXXFunctionSummaryFormat() override = default; - Callback GetBackendFunction() const { return m_impl; } + Callback GetBackendFunction() const { return m_data.m_impl; } const char *GetTextualInfo() const { return m_description.c_str(); } - void SetBackendFunction(Callback cb_func) { m_impl = std::move(cb_func); } + void SetBackendFunction(Callback cb_func) { + m_data.m_impl = std::move(cb_func); + } void SetTextualInfo(const char *descr) { if (descr) @@ -372,6 +400,58 @@ struct CXXFunctionSummaryFormat : public TypeSummaryImpl { operator=(const CXXFunctionSummaryFormat &) = delete; }; +/// A summary that supports multiple type layouts for the same type name. +/// +/// This summary maintains a list of child summaries, each paired with a +/// validator function. An associated validator is used to determine which child +/// is appropriate for formatting a given ValueObject. +struct CXXCompositeSummaryFormat : public TypeSummaryImpl { + using Validator = bool(ValueObject &valobj); + using ChildData = std::variant<CXXFunctionSummaryData, StringSummaryData>; + using Child = std::pair<Validator *, ChildData>; + using Children = llvm::SmallVector<Child, 2>; + + Children m_children; + std::string m_description; + + CXXCompositeSummaryFormat(const TypeSummaryImpl::Flags &flags, + const char *description, Children impls = {}, + uint32_t ptr_match_depth = 1); + + ~CXXCompositeSummaryFormat() override = default; + + const char *GetTextualInfo() const { return m_description.c_str(); } + + CXXCompositeSummaryFormat *Append(Validator *validator, ChildData impl); + + void SetTextualInfo(const char *descr) { + if (descr) + m_description.assign(descr); + else + m_description.clear(); + } + + Children CopyChildren() const; + + bool FormatObject(ValueObject *valobj, std::string &dest, + const TypeSummaryOptions &options) override; + + std::string GetDescription() override; + + static bool classof(const TypeSummaryImpl *S) { + return S->GetKind() == Kind::eComposite; + } + + std::string GetName() override; + + using SharedPointer = std::shared_ptr<CXXCompositeSummaryFormat>; + +private: + CXXCompositeSummaryFormat(const CXXCompositeSummaryFormat &) = delete; + const CXXCompositeSummaryFormat & + operator=(const CXXCompositeSummaryFormat &) = delete; +}; + // Python-based summaries, running script code to show data struct ScriptSummaryFormat : public TypeSummaryImpl { std::string m_function_name; diff --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h index f9deb6ebf8d6d..1f20c3180d37a 100644 --- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h +++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h @@ -165,7 +165,7 @@ class ValueObjectPrinter { std::string m_error; bool m_val_summary_ok; - friend struct StringSummaryFormat; + friend struct StringSummaryData; ValueObjectPrinter(const ValueObjectPrinter &) = delete; const ValueObjectPrinter &operator=(const ValueObjectPrinter &) = delete; diff --git a/lldb/packages/Python/lldbsuite/test/configuration.py b/lldb/packages/Python/lldbsuite/test/configuration.py index b2d91fd211477..4ec892bef69f9 100644 --- a/lldb/packages/Python/lldbsuite/test/configuration.py +++ b/lldb/packages/Python/lldbsuite/test/configuration.py @@ -134,6 +134,8 @@ libcxx_include_target_dir = None libcxx_library_dir = None +target_triple = None + # A plugin whose tests will be enabled, like intel-pt. enabled_plugins = [] diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py b/lldb/packages/Python/lldbsuite/test/dotest.py index d7f274ac4f60e..dd6fbdf8daed4 100644 --- a/lldb/packages/Python/lldbsuite/test/dotest.py +++ b/lldb/packages/Python/lldbsuite/test/dotest.py @@ -299,6 +299,8 @@ def parseOptionsAndInitTestdirs(): configuration.libcxx_library_dir = args.libcxx_library_dir configuration.cmake_build_type = args.cmake_build_type.lower() + configuration.target_triple = args.target_triple + if args.channels: lldbtest_config.channels = args.channels @@ -831,6 +833,26 @@ def checkLibstdcxxSupport(): configuration.skip_categories.append("libstdcxx") +def canRunMsvcStlTests(): + if "windows-msvc" in configuration.target_triple: + return True, "MSVC STL is present on *windows-msvc*" + return ( + False, + f"Don't know how to build with MSVC's STL on {configuration.target_triple}", + ) + + +def checkMsvcStlSupport(): + result, reason = canRunMsvcStlTests() + if result: + return # msvcstl supported + if "msvcstl" in configuration.categories_list: + return # msvcstl category explicitly requested, let it run. + if configuration.verbose: + print(f"msvcstl tests will not be run because: {reason}") + configuration.skip_categories.append("msvcstl") + + def canRunWatchpointTests(): from lldbsuite.test import lldbplatformutil @@ -1044,6 +1066,7 @@ def run_suite(): checkLibcxxSupport() checkLibstdcxxSupport() + checkMsvcStlSupport() checkWatchpointSupport() checkDebugInfoSupport() checkDebugServerSupport() diff --git a/lldb/packages/Python/lldbsuite/test/dotest_args.py b/lldb/packages/Python/lldbsuite/test/dotest_args.py index e9c21388bc213..f47cfc7db357e 100644 --- a/lldb/packages/Python/lldbsuite/test/dotest_args.py +++ b/lldb/packages/Python/lldbsuite/test/dotest_args.py @@ -116,6 +116,12 @@ def create_parser(): "The location of llvm tools used for testing (yaml2obj, FileCheck, etc.)." ), ) + group.add_argument( + "--target-triple", + help=textwrap.dedent( + "The target triple the tests will run on (e.g. x86_64-pc-windows-msvc)." + ), + ) # Test filtering options group = parser.add_argument_group("Test filtering options") diff --git a/lldb/packages/Python/lldbsuite/test/test_categories.py b/lldb/packages/Python/lldbsuite/test/test_categories.py index b585f695adeab..1f6e8a78e0c0d 100644 --- a/lldb/packages/Python/lldbsuite/test/test_categories.py +++ b/lldb/packages/Python/lldbsuite/test/test_categories.py @@ -33,6 +33,7 @@ "lldb-server": "Tests related to lldb-server", "lldb-dap": "Tests for the Debug Adapter Protocol with lldb-dap", "llgs": "Tests for the gdb-server functionality of lldb-server", + "msvcstl": "Test for MSVC STL data formatters", "pexpect": "Tests requiring the pexpect library to be available", "objc": "Tests related to the Objective-C programming language support", "pyapi": "Tests related to the Python API", diff --git a/lldb/source/API/SBTypeSummary.cpp b/lldb/source/API/SBTypeSummary.cpp index 58ec068ab9600..b62e786f522d4 100644 --- a/lldb/source/API/SBTypeSummary.cpp +++ b/lldb/source/API/SBTypeSummary.cpp @@ -370,6 +370,9 @@ bool SBTypeSummary::IsEqualTo(lldb::SBTypeSummary &rhs) { if (IsSummaryString() != rhs.IsSummaryString()) return false; return GetOptions() == rhs.GetOptions(); + case TypeSummaryImpl::Kind::eComposite: + return llvm::dyn_cast<CXXCompositeSummaryFormat>(m_opaque_sp.get()) == + llvm::dyn_cast<CXXCompositeSummaryFormat>(rhs.m_opaque_sp.get()); case TypeSummaryImpl::Kind::eInternal: return (m_opaque_sp.get() == rhs.m_opaque_sp.get()); } @@ -406,7 +409,7 @@ bool SBTypeSummary::CopyOnWrite_Impl() { if (CXXFunctionSummaryFormat *current_summary_ptr = llvm::dyn_cast<CXXFunctionSummaryFormat>(m_opaque_sp.get())) { new_sp = TypeSummaryImplSP(new CXXFunctionSummaryFormat( - GetOptions(), current_summary_ptr->m_impl, + GetOptions(), current_summary_ptr->m_data.m_impl, current_summary_ptr->m_description.c_str())); } else if (ScriptSummaryFormat *current_summary_ptr = llvm::dyn_cast<ScriptSummaryFormat>(m_opaque_sp.get())) { @@ -417,6 +420,12 @@ bool SBTypeSummary::CopyOnWrite_Impl() { llvm::dyn_cast<StringSummaryFormat>(m_opaque_sp.get())) { new_sp = TypeSummaryImplSP(new StringSummaryFormat( GetOptions(), current_summary_ptr->GetSummaryString())); + } else if (CXXCompositeSummaryFormat *current_summary_ptr = + llvm::dyn_cast<CXXCompositeSummaryFormat>(m_opaque_sp.get())) { + new_sp = TypeSummaryImplSP(new CXXCompositeSummaryFormat( + GetOptions(), current_summary_ptr->GetTextualInfo(), + current_summary_ptr->CopyChildren(), + current_summary_ptr->GetPtrMatchDepth())); } SetSP(new_sp); diff --git a/lldb/source/Commands/CommandObjectType.cpp b/lldb/source/Commands/CommandObjectType.cpp index 19cd3ff2972e9..bd6138a860caa 100644 --- a/lldb/source/Commands/CommandObjectType.cpp +++ b/lldb/source/Commands/CommandObjectType.cpp @@ -1397,9 +1397,10 @@ bool CommandObjectTypeSummaryAdd::Execute_StringSummary( result.AppendError("summary creation failed"); return false; } - if (string_format->m_error.Fail()) { - result.AppendErrorWithFormat("syntax error: %s", - string_format->m_error.AsCString("<unknown>")); + if (string_format->m_data.m_error.Fail()) { + result.AppendErrorWithFormat( + "syntax error: %s", + string_format->m_data.m_error.AsCString("<unknown>")); return false; } lldb::TypeSummaryImplSP entry(string_format.release()); diff --git a/lldb/source/DataFormatters/TypeSummary.cpp b/lldb/source/DataFormatters/TypeSummary.cpp index 6aa290698cd12..4830b2c28901a 100644 --- a/lldb/source/DataFormatters/TypeSummary.cpp +++ b/lldb/source/DataFormatters/TypeSummary.cpp @@ -57,21 +57,17 @@ std::string TypeSummaryImpl::GetSummaryKindName() { return "python"; case Kind::eInternal: return "c++"; + case Kind::eComposite: + return "composite"; case Kind::eBytecode: return "bytecode"; } llvm_unreachable("Unknown type kind name"); } -StringSummaryFormat::StringSummaryFormat(const TypeSummaryImpl::Flags &flags, - const char *format_cstr, - uint32_t ptr_match_depth) - : TypeSummaryImpl(Kind::eSummaryString, flags, ptr_match_depth), - m_format_str() { - SetSummaryString(format_cstr); -} +StringSummaryData::StringSummaryData(const char *f) { SetFormat(f); } -void StringSummaryFormat::SetSummaryString(const char *format_cstr) { +void StringSummaryData::SetFormat(const char *format_cstr) { m_format.Clear(); if (format_cstr && format_cstr[0]) { m_format_str = format_cstr; @@ -82,8 +78,19 @@ void StringSummaryFormat::SetSummaryString(const char *format_cstr) { } } -bool StringSummaryFormat::FormatObject(ValueObject *valobj, std::string &retval, - const TypeSummaryOptions &options) { +StringSummaryFormat::StringSummaryFormat(const TypeSummaryImpl::Flags &flags, + const char *format_cstr, + uint32_t ptr_match_depth) + : TypeSummaryImpl(Kind::eSummaryString, flags, ptr_match_depth), + m_data(format_cstr) {} + +void StringSummaryFormat::SetSummaryString(const char *format_cstr) { + m_data.SetFormat(format_cstr); +} + +bool StringSummaryData::FormatObject(ValueObject *valobj, std::string &retval, + const TypeSummaryOptions &options, + const TypeSummaryImpl &summary) { if (!valobj) { retval.assign("NULL ValueObject"); return false; @@ -96,12 +103,12 @@ bool StringSummaryFormat::FormatObject(ValueObject *valobj, std::string &retval, if (frame) sc = frame->GetSymbolContext(lldb::eSymbolContextEverything); - if (IsOneLiner()) { + if (summary.IsOneLiner()) { // We've already checked the case of a NULL valobj above. Let's put in an // assert here to make sure someone doesn't take that out: assert(valobj && "Must have a valid ValueObject to summarize"); ValueObjectPrinter printer(*valobj, &s, DumpValueObjectOptions()); - printer.PrintChildrenOneLiner(HideNames(valobj)); + printer.PrintChildrenOneLiner(summary.HideNames(valobj)); retval = std::string(s.GetString()); return true; } else { @@ -117,40 +124,52 @@ bool StringSummaryFormat::FormatObject(ValueObject *valobj, std::string &retval, } } +bool StringSummaryFormat::FormatObject(ValueObject *valobj, std::string &retval, + const TypeSummaryOptions &options) { + return m_data.FormatObject(valobj, retval, options, *this); +} + std::string StringSummaryFormat::GetDescription() { StreamString sstr; - sstr.Printf("`%s`%s%s%s%s%s%s%s%s%s ptr-match-depth=%u", m_format_str.c_str(), - m_error.Fail() ? " error: " : "", - m_error.Fail() ? m_error.AsCString() : "", - Cascades() ? "" : " (not cascading)", - !DoesPrintChildren(nullptr) ? "" : " (show children)", - !DoesPrintValue(nullptr) ? " (hide value)" : "", - IsOneLiner() ? " (one-line printout)" : "", - SkipsPointers() ? " (skip pointers)" : "", - SkipsReferences() ? " (skip references)" : "", - HideNames(nullptr) ? " (hide member names)" : "", - GetPtrMatchDepth()); + sstr.Printf( + "`%s`%s%s%s%s%s%s%s%s%s ptr-match-depth=%u", m_data.m_format_str.c_str(), + m_data.m_error.Fail() ? " error: " : "", + m_data.m_error.Fail() ? m_data.m_error.AsCString() : "", + Cascades() ? "" : " (not cascading)", + !DoesPrintChildren(nullptr) ? "" : " (show children)", + !DoesPrintValue(nullptr) ? " (hide value)" : "", + IsOneLiner() ? " (one-line printout)" : "", + SkipsPointers() ? " (skip pointers)" : "", + SkipsReferences() ? " (skip references)" : "", + HideNames(nullptr) ? " (hide member names)" : "", GetPtrMatchDepth()); return std::string(sstr.GetString()); } -std::string StringSummaryFormat::GetName() { return m_format_str; } +std::string StringSummaryFormat::GetName() { return m_data.m_format_str; } + +bool CXXFunctionSummaryData::FormatObject(ValueObject *valobj, + std::string &dest, + const TypeSummaryOptions &options, + const TypeSummaryImpl &summary) { + dest.clear(); + StreamString stream; + if (!m_impl || !m_impl(*valobj, stream, options)) + return false; + dest = std::string(stream.GetString()); + return true; +} CXXFunctionSummaryFormat::CXXFunctionSummaryFormat( const TypeSummaryImpl::Flags &flags, Callback impl, const char *description, uint32_t ptr_match_depth) - : TypeSummaryImpl(Kind::eCallback, flags, ptr_match_depth), m_impl(impl), + : TypeSummaryImpl(Kind::eCallback, flags, ptr_match_depth), m_data{impl}, m_description(description ? description : "") {} bool CXXFunctionSummaryFormat::FormatObject(ValueObject *valobj, std::string &dest, const TypeSummaryOptions &options) { - dest.clear(); - StreamString stream; - if (!m_impl || !m_impl(*valobj, stream, options)) - return false; - dest = std::string(stream.GetString()); - return true; + return m_data.FormatObject(valobj, dest, options, *this); } std::string CXXFunctionSummaryFormat::GetDescription() { @@ -169,6 +188,68 @@ std::string CXXFunctionSummaryFormat::GetDescription() { std::string CXXFunctionSummaryFormat::GetName() { return m_description; } +CXXCompositeSummaryFormat::CXXCompositeSummaryFormat( + const TypeSummaryImpl::Flags &flags, const char *description, + Children children, uint32_t ptr_match_depth) + : TypeSummaryImpl(Kind::eComposite, flags, ptr_match_depth), + m_children(std::move(children)) {} + +CXXCompositeSummaryFormat * +CXXCompositeSummaryFormat::Append(Validator *validator, ChildData child) { + m_children.emplace_back(validator, std::move(child)); + return this; +} + +CXXCompositeSummaryFormat::Children +CXXCompositeSummaryFormat::CopyChildren() const { + auto copy = [](const Child &entry) -> Child { + return {entry.first, + std::visit(llvm::makeVisitor( + [](const CXXFunctionSummaryData &fn) -> ChildData { + return fn; + }, + [](const StringSummaryData &string) -> ChildData { + return StringS... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/143177 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits