[Lldb-commits] [lldb] 1912879 - Reland "[lldb][DWARFASTParserClang] Attach linkage name to ctors/dtors if missing"
Author: Michael Buch Date: 2023-02-12T10:14:43Z New Revision: 19128792e2aa320c1a149f7f93638cbd7f3c83c6 URL: https://github.com/llvm/llvm-project/commit/19128792e2aa320c1a149f7f93638cbd7f3c83c6 DIFF: https://github.com/llvm/llvm-project/commit/19128792e2aa320c1a149f7f93638cbd7f3c83c6.diff LOG: Reland "[lldb][DWARFASTParserClang] Attach linkage name to ctors/dtors if missing" This relands the commit previously reverted in `d2cc2c5610ffa78736aa99512bc85a85417efb0a` due to failures on Linux when debugging split-debug-info enabled executables. The problem was we called `SymbolFileDWARF::FindFunctions` directly instead of `Module::FindFunctions` which resulted in a nullptr dereference because the backing `SymbolFileDWARFDwo` didn't have an index attached to it. The relanded version calls `Module::FindFunctions` instead. Differential Revision: https://reviews.llvm.org/D143652 Added: lldb/test/API/lang/cpp/external_ctor_dtor_lookup/Makefile lldb/test/API/lang/cpp/external_ctor_dtor_lookup/TestExternalCtorDtorLookup.py lldb/test/API/lang/cpp/external_ctor_dtor_lookup/lib.h lldb/test/API/lang/cpp/external_ctor_dtor_lookup/main.cpp Modified: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Removed: diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 4429b4fcae2a0..8401d648c6704 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -888,6 +888,46 @@ ConvertDWARFCallingConventionToClang(const ParsedDWARFTypeAttributes &attrs) { return clang::CC_C; } +/// Given a DIE with an external definition (and thus no linkage name) +/// find the definitions by lookup into the DWARF name index. +/// We check the DW_AT_specification for each DIE in the index with +/// the same name as the specified 'die' until we find one that references +/// 'die'. Then return that linkage name. If no such DIE is found in the index, +/// returns nullptr. +static char const *FindLinkageName(DWARFDIE die) { + auto *dwarf = die.GetDWARF(); + if (!dwarf) +return nullptr; + + ConstString func_name(die.GetName()); + if (!func_name) +return nullptr; + + SymbolContextList sc_list; + Module::LookupInfo lookup_info(func_name, + FunctionNameType::eFunctionNameTypeMethod | + FunctionNameType::eFunctionNameTypeFull, + LanguageType::eLanguageTypeUnknown); + dwarf->GetObjectFile()->GetModule()->FindFunctions(lookup_info, {}, {}, + sc_list); + + for (auto const &sc : sc_list.SymbolContexts()) { +if (auto *func = sc.function) { + auto func_die = dwarf->GetDIE(func->GetID()); + if (!func_die.IsValid()) +continue; + + auto spec_die = + func_die.GetAttributeValueAsReferenceDIE(DW_AT_specification); + if (spec_die.IsValid() && spec_die == die) { +return func->GetMangled().GetMangledName().AsCString(); + } +} + } + + return nullptr; +} + TypeSP DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, ParsedDWARFTypeAttributes &attrs) { Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups); @@ -1116,6 +1156,9 @@ TypeSP DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, if (attrs.accessibility == eAccessNone) attrs.accessibility = eAccessPublic; + if (!attrs.mangled_name && attrs.storage == clang::SC_Extern) +attrs.mangled_name = FindLinkageName(die); + clang::CXXMethodDecl *cxx_method_decl = m_ast.AddMethodToCXXRecordType( class_opaque_type.GetOpaqueQualType(), diff --git a/lldb/test/API/lang/cpp/external_ctor_dtor_lookup/Makefile b/lldb/test/API/lang/cpp/external_ctor_dtor_lookup/Makefile new file mode 100644 index 0..eba15476332f9 --- /dev/null +++ b/lldb/test/API/lang/cpp/external_ctor_dtor_lookup/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/lang/cpp/external_ctor_dtor_lookup/TestExternalCtorDtorLookup.py b/lldb/test/API/lang/cpp/external_ctor_dtor_lookup/TestExternalCtorDtorLookup.py new file mode 100644 index 0..e436252800a79 --- /dev/null +++ b/lldb/test/API/lang/cpp/external_ctor_dtor_lookup/TestExternalCtorDtorLookup.py @@ -0,0 +1,31 @@ +""" +Test that we can constructors/destructors +without a linkage name because they are +marked DW_AT_external and the fallback +mangled-name-guesser in LLDB doesn't account +for ABI tags. +""" + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest
[Lldb-commits] [PATCH] D143842: [lldb][test] Add check for Xcode binutils version to test-runner
Michael137 created this revision. Herald added a project: All. Michael137 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Some tests in LLDB require the use of binutils tools. On Darwin, we need to use the tools that ship with Xcode. Otherwise we risk crashing in unpredictable ways. This patch adds a check for this and bails early if the user has a non-Xcode binutils in their path before the Xcode ones. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D143842 Files: lldb/packages/Python/lldbsuite/test/dotest.py Index: lldb/packages/Python/lldbsuite/test/dotest.py === --- lldb/packages/Python/lldbsuite/test/dotest.py +++ lldb/packages/Python/lldbsuite/test/dotest.py @@ -869,6 +869,28 @@ if platform not in ["freebsd", "linux", "netbsd"]: configuration.skip_categories.append("fork") +def checkDarwinBinutilsPath(): +from lldbsuite.test import lldbplatformutil + +if not lldbplatformutil.platformIsDarwin(): +return + +cmd = ["strip", "--version"] +process = subprocess.Popen( +cmd, +stdout=subprocess.PIPE, +stderr=subprocess.STDOUT) +cmd_output = process.stdout.read() +output_str = cmd_output.decode("utf-8") +if "GNU strip (GNU Binutils)" in output_str: +print(""" +The tool 'strip' in your path is not the one from the Xcode +toolchain. Please make sure the Xcode tools are before any +other tools in your path. Run `xcode -f strip` for the correct +toolchain path. +Exiting... +""") +sys.exit(0) def run_suite(): # On MacOS X, check to make sure that domain for com.apple.DebugSymbols defaults @@ -956,6 +978,7 @@ checkDebugServerSupport() checkObjcSupport() checkForkVForkSupport() +checkDarwinBinutilsPath() skipped_categories_list = ", ".join(configuration.skip_categories) print("Skipping the following test categories: {}".format(configuration.skip_categories)) Index: lldb/packages/Python/lldbsuite/test/dotest.py === --- lldb/packages/Python/lldbsuite/test/dotest.py +++ lldb/packages/Python/lldbsuite/test/dotest.py @@ -869,6 +869,28 @@ if platform not in ["freebsd", "linux", "netbsd"]: configuration.skip_categories.append("fork") +def checkDarwinBinutilsPath(): +from lldbsuite.test import lldbplatformutil + +if not lldbplatformutil.platformIsDarwin(): +return + +cmd = ["strip", "--version"] +process = subprocess.Popen( +cmd, +stdout=subprocess.PIPE, +stderr=subprocess.STDOUT) +cmd_output = process.stdout.read() +output_str = cmd_output.decode("utf-8") +if "GNU strip (GNU Binutils)" in output_str: +print(""" +The tool 'strip' in your path is not the one from the Xcode +toolchain. Please make sure the Xcode tools are before any +other tools in your path. Run `xcode -f strip` for the correct +toolchain path. +Exiting... +""") +sys.exit(0) def run_suite(): # On MacOS X, check to make sure that domain for com.apple.DebugSymbols defaults @@ -956,6 +978,7 @@ checkDebugServerSupport() checkObjcSupport() checkForkVForkSupport() +checkDarwinBinutilsPath() skipped_categories_list = ", ".join(configuration.skip_categories) print("Skipping the following test categories: {}".format(configuration.skip_categories)) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D143842: [lldb][test] Add check for Xcode binutils version to test-runner
tschuett added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:889 +toolchain. Please make sure the Xcode tools are before any +other tools in your path. Run `xcode -f strip` for the correct +toolchain path. Did you mean `xcrun -f strip`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143842/new/ https://reviews.llvm.org/D143842 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D143842: [lldb][test] Add check for Xcode binutils version to test-runner
Michael137 added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:889 +toolchain. Please make sure the Xcode tools are before any +other tools in your path. Run `xcode -f strip` for the correct +toolchain path. tschuett wrote: > Did you mean `xcrun -f strip`? yup! good catch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143842/new/ https://reviews.llvm.org/D143842 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D143842: [lldb][test] Add check for Xcode binutils version to test-runner
Michael137 added a comment. My preference would be to check explicitly whether the tool is from the xcode toolchain. But the tools dont really have a distinguishable version (in fact the `strip` from Xcode doesnt have a version commandline flag. Perhaps that’s deliberate and a good way to disbiguate homebrew vs xcode?) Another option would be to check that homebrew/binutils only appears after xcode in the user path Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143842/new/ https://reviews.llvm.org/D143842 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D143398: [lldb][DWARFASTParserClang] Correctly resolve imported namespaces during expression evaluation
Michael137 reopened this revision. Michael137 added a comment. This revision is now accepted and ready to land. Reopening to Ireland slightly adjusted patch. New patch makes sure we populate the `ManualDWARFIndex` with `DW_TAG_imported_declaration`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143398/new/ https://reviews.llvm.org/D143398 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D143398: [lldb][DWARFASTParserClang] Correctly resolve imported namespaces during expression evaluation
Michael137 updated this revision to Diff 496774. Michael137 added a comment. Herald added a subscriber: arphaman. - Populate `ManualDWARFIndex` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143398/new/ https://reviews.llvm.org/D143398 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp lldb/test/API/commands/expression/namespace-alias/Makefile lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py lldb/test/API/commands/expression/namespace-alias/main.cpp Index: lldb/test/API/commands/expression/namespace-alias/main.cpp === --- /dev/null +++ lldb/test/API/commands/expression/namespace-alias/main.cpp @@ -0,0 +1,21 @@ +namespace A { +inline namespace _A { +namespace B { +namespace C { +int a = -1; + +int func() { return 0; } +} // namespace C +} // namespace B + +namespace C = B::C; +namespace D = B::C; + +} // namespace _A +} // namespace A + +namespace E = A; +namespace F = E::C; +namespace G = F; + +int main(int argc, char **argv) { return A::B::C::a; } Index: lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py === --- /dev/null +++ lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py @@ -0,0 +1,37 @@ +""" +Test that we correctly handle namespace +expression evaluation through namespace +aliases. +""" + +import lldb + +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + +class TestInlineNamespace(TestBase): +def do_test(self, params): +self.build() + +lldbutil.run_to_source_breakpoint(self, + "return A::B::C::a", lldb.SBFileSpec("main.cpp")) + +self.expect_expr("A::C::a", result_type="int", result_value="-1") +self.expect_expr("A::D::a", result_type="int", result_value="-1") + +self.expect_expr("A::C::func()", result_type="int", result_value="0") +self.expect_expr("A::D::func()", result_type="int", result_value="0") + +self.expect_expr("E::C::a", result_type="int", result_value="-1") +self.expect_expr("E::D::a", result_type="int", result_value="-1") +self.expect_expr("F::a", result_type="int", result_value="-1") +self.expect_expr("G::a", result_type="int", result_value="-1") + +@skipIf(debug_info=no_match(["dsym"])) +def test_dsym(self): +self.do_test({}) + +@skipIf(debug_info="dsym") +def test_dwarf(self): +self.do_test(dict(CFLAGS_EXTRAS="-gdwarf-5 -gpubnames")) Index: lldb/test/API/commands/expression/namespace-alias/Makefile === --- /dev/null +++ lldb/test/API/commands/expression/namespace-alias/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp === --- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp +++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp @@ -21,6 +21,7 @@ #include "lldb/Utility/DataExtractor.h" #include "lldb/Utility/Stream.h" #include "lldb/Utility/Timer.h" +#include "llvm/BinaryFormat/Dwarf.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/ThreadPool.h" #include @@ -208,6 +209,7 @@ case DW_TAG_enumeration_type: case DW_TAG_inlined_subroutine: case DW_TAG_namespace: +case DW_TAG_imported_declaration: case DW_TAG_string_type: case DW_TAG_structure_type: case DW_TAG_subprogram: @@ -354,6 +356,7 @@ break; case DW_TAG_namespace: +case DW_TAG_imported_declaration: if (name) set.namespaces.Insert(ConstString(name), ref); break; Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h === --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h +++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h @@ -132,6 +132,17 @@ clang::NamespaceDecl *ResolveNamespaceDIE(const DWARFDIE &die); + /// Returns the namespace decl that a DW_TAG_imported_declaration imports. + /// + /// \param[in] die The import declaration to resolve. If the DIE is not a + ///DW_TAG_imported_declaration the behaviour is undefined. + /// + /// \returns The decl corresponding to the namespace that the specified + /// 'die' imports. If the imported entity is not a namespace + /// or another import declaration, returns nullptr. If an error + /// occurs, returns nullptr. + clang::NamespaceDecl *ResolveImportedDeclarationDIE(const DWARFDIE &die);
[Lldb-commits] [PATCH] D143398: [lldb][DWARFASTParserClang] Correctly resolve imported namespaces during expression evaluation
Michael137 updated this revision to Diff 496775. Michael137 added a comment. - Remove redundant header Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143398/new/ https://reviews.llvm.org/D143398 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp lldb/test/API/commands/expression/namespace-alias/Makefile lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py lldb/test/API/commands/expression/namespace-alias/main.cpp Index: lldb/test/API/commands/expression/namespace-alias/main.cpp === --- /dev/null +++ lldb/test/API/commands/expression/namespace-alias/main.cpp @@ -0,0 +1,21 @@ +namespace A { +inline namespace _A { +namespace B { +namespace C { +int a = -1; + +int func() { return 0; } +} // namespace C +} // namespace B + +namespace C = B::C; +namespace D = B::C; + +} // namespace _A +} // namespace A + +namespace E = A; +namespace F = E::C; +namespace G = F; + +int main(int argc, char **argv) { return A::B::C::a; } Index: lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py === --- /dev/null +++ lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py @@ -0,0 +1,37 @@ +""" +Test that we correctly handle namespace +expression evaluation through namespace +aliases. +""" + +import lldb + +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + +class TestInlineNamespace(TestBase): +def do_test(self, params): +self.build() + +lldbutil.run_to_source_breakpoint(self, + "return A::B::C::a", lldb.SBFileSpec("main.cpp")) + +self.expect_expr("A::C::a", result_type="int", result_value="-1") +self.expect_expr("A::D::a", result_type="int", result_value="-1") + +self.expect_expr("A::C::func()", result_type="int", result_value="0") +self.expect_expr("A::D::func()", result_type="int", result_value="0") + +self.expect_expr("E::C::a", result_type="int", result_value="-1") +self.expect_expr("E::D::a", result_type="int", result_value="-1") +self.expect_expr("F::a", result_type="int", result_value="-1") +self.expect_expr("G::a", result_type="int", result_value="-1") + +@skipIf(debug_info=no_match(["dsym"])) +def test_dsym(self): +self.do_test({}) + +@skipIf(debug_info="dsym") +def test_dwarf(self): +self.do_test(dict(CFLAGS_EXTRAS="-gdwarf-5 -gpubnames")) Index: lldb/test/API/commands/expression/namespace-alias/Makefile === --- /dev/null +++ lldb/test/API/commands/expression/namespace-alias/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp === --- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp +++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp @@ -208,6 +208,7 @@ case DW_TAG_enumeration_type: case DW_TAG_inlined_subroutine: case DW_TAG_namespace: +case DW_TAG_imported_declaration: case DW_TAG_string_type: case DW_TAG_structure_type: case DW_TAG_subprogram: @@ -354,6 +355,7 @@ break; case DW_TAG_namespace: +case DW_TAG_imported_declaration: if (name) set.namespaces.Insert(ConstString(name), ref); break; Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h === --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h +++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h @@ -132,6 +132,17 @@ clang::NamespaceDecl *ResolveNamespaceDIE(const DWARFDIE &die); + /// Returns the namespace decl that a DW_TAG_imported_declaration imports. + /// + /// \param[in] die The import declaration to resolve. If the DIE is not a + ///DW_TAG_imported_declaration the behaviour is undefined. + /// + /// \returns The decl corresponding to the namespace that the specified + /// 'die' imports. If the imported entity is not a namespace + /// or another import declaration, returns nullptr. If an error + /// occurs, returns nullptr. + clang::NamespaceDecl *ResolveImportedDeclarationDIE(const DWARFDIE &die); + bool ParseTemplateDIE(const DWARFDIE &die, lldb_private::TypeSystemClang::TemplateParameterInfos &template_param_infos); Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
[Lldb-commits] [lldb] 129eb5b - [lldb] Add the ability to provide a message to a progress event update
Author: Jonas Devlieghere Date: 2023-02-12T11:17:58-08:00 New Revision: 129eb5bcab91a12ed3c4712279f201834ae2d8e1 URL: https://github.com/llvm/llvm-project/commit/129eb5bcab91a12ed3c4712279f201834ae2d8e1 DIFF: https://github.com/llvm/llvm-project/commit/129eb5bcab91a12ed3c4712279f201834ae2d8e1.diff LOG: [lldb] Add the ability to provide a message to a progress event update Consider the following example as motivation. Say you have to load symbols for 3 dynamic libraries: `libFoo`, `libBar` and `libBaz`. Currently, there are two ways to report process for this operation: 1. As 3 separate progress instances. In this case you create a progress instance with the message "Loading symbols: libFoo", "Loading symbols: libBar", and "Loading symbols: libBaz" respectively. Each progress event gets a unique ID and therefore cannot be correlated by the consumer. 2. As 1 progress instance with 3 units of work. The title would be "Loading symbols" and you call Progress::Increment for each of the libraries. The 3 progress events share the same ID and can easily be correlated, however, in the current design, there's no way to include the name of the libraries. The second approach is preferred when the amount of work is known in advance, because determinate progress can be reported (i.e. x out of y operations completed). An additional benefit is that the progress consumer can decide to ignore certain progress updates by their ID if they are deemed to noisy, which isn't trivial for the first approach due to the use of different progress IDs. This patch adds the ability to add a message (detail) to a progress event update. For the example described above, progress can now be displayed as shown: [1/3] Loading symbols: libFoo [2/3] Loading symbols: libBar [3/3] Loading symbols: libBaz Differential revision: https://reviews.llvm.org/D143690 Added: Modified: lldb/include/lldb/Core/Debugger.h lldb/include/lldb/Core/DebuggerEvents.h lldb/include/lldb/Core/Progress.h lldb/source/API/SBDebugger.cpp lldb/source/Core/Debugger.cpp lldb/source/Core/DebuggerEvents.cpp lldb/source/Core/Progress.cpp Removed: diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h index dd3e6c061fcf6..9f83a37bc75cc 100644 --- a/lldb/include/lldb/Core/Debugger.h +++ b/lldb/include/lldb/Core/Debugger.h @@ -484,8 +484,9 @@ class Debugger : public std::enable_shared_from_this, /// debugger identifier that this progress should be delivered to. If this /// optional parameter does not have a value, the progress will be /// delivered to all debuggers. - static void ReportProgress(uint64_t progress_id, const std::string &message, - uint64_t completed, uint64_t total, + static void ReportProgress(uint64_t progress_id, std::string title, + std::string details, uint64_t completed, + uint64_t total, std::optional debugger_id); static void ReportDiagnosticImpl(DiagnosticEventData::Type type, diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h index 0dade0f9629a0..f9ae67efac323 100644 --- a/lldb/include/lldb/Core/DebuggerEvents.h +++ b/lldb/include/lldb/Core/DebuggerEvents.h @@ -21,10 +21,11 @@ class Stream; class ProgressEventData : public EventData { public: - ProgressEventData(uint64_t progress_id, const std::string &message, + ProgressEventData(uint64_t progress_id, std::string title, std::string update, uint64_t completed, uint64_t total, bool debugger_specific) - : m_message(message), m_id(progress_id), m_completed(completed), -m_total(total), m_debugger_specific(debugger_specific) {} + : m_title(std::move(title)), m_details(std::move(update)), +m_id(progress_id), m_completed(completed), m_total(total), +m_debugger_specific(debugger_specific) {} static ConstString GetFlavorString(); @@ -41,12 +42,30 @@ class ProgressEventData : public EventData { bool IsFinite() const { return m_total != UINT64_MAX; } uint64_t GetCompleted() const { return m_completed; } uint64_t GetTotal() const { return m_total; } - const std::string &GetMessage() const { return m_message; } + std::string GetMessage() const { +std::string message = m_title; +if (!m_details.empty()) { + message.append(": "); + message.append(m_details); +} +return message; + } + const std::string &GetTitle() const { return m_title; } + const std::string &GetDetails() const { return m_details; } bool IsDebuggerSpecific() const { return m_debugger_specific; } private: - std::string m_message; + /// The title of this progress event. The value is expected to remain stable + /// for a given progress ID. + std::string m_ti
[Lldb-commits] [PATCH] D143690: [lldb] Add the ability to provide a message to a progress event update
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG129eb5bcab91: [lldb] Add the ability to provide a message to a progress event update (authored by JDevlieghere). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143690/new/ https://reviews.llvm.org/D143690 Files: lldb/include/lldb/Core/Debugger.h lldb/include/lldb/Core/DebuggerEvents.h lldb/include/lldb/Core/Progress.h lldb/source/API/SBDebugger.cpp lldb/source/Core/Debugger.cpp lldb/source/Core/DebuggerEvents.cpp lldb/source/Core/Progress.cpp Index: lldb/source/Core/Progress.cpp === --- lldb/source/Core/Progress.cpp +++ lldb/source/Core/Progress.cpp @@ -36,7 +36,7 @@ } } -void Progress::Increment(uint64_t amount) { +void Progress::Increment(uint64_t amount, std::string update) { if (amount > 0) { std::lock_guard guard(m_mutex); // Watch out for unsigned overflow and make sure we don't increment too @@ -45,16 +45,16 @@ m_completed = m_total; else m_completed += amount; -ReportProgress(); +ReportProgress(update); } } -void Progress::ReportProgress() { +void Progress::ReportProgress(std::string update) { if (!m_complete) { // Make sure we only send one notification that indicates the progress is // complete. m_complete = m_completed == m_total; -Debugger::ReportProgress(m_id, m_title, m_completed, m_total, - m_debugger_id); +Debugger::ReportProgress(m_id, m_title, std::move(update), m_completed, + m_total, m_debugger_id); } } Index: lldb/source/Core/DebuggerEvents.cpp === --- lldb/source/Core/DebuggerEvents.cpp +++ lldb/source/Core/DebuggerEvents.cpp @@ -33,7 +33,9 @@ } void ProgressEventData::Dump(Stream *s) const { - s->Printf(" id = %" PRIu64 ", message = \"%s\"", m_id, m_message.c_str()); + s->Printf(" id = %" PRIu64 ", title = \"%s\"", m_id, m_title.c_str()); + if (!m_details.empty()) +s->Printf(", details = \"%s\"", m_details.c_str()); if (m_completed == 0 || m_completed == m_total) s->Printf(", type = %s", m_completed == 0 ? "start" : "end"); else @@ -58,6 +60,8 @@ return {}; auto dictionary_sp = std::make_shared(); + dictionary_sp->AddStringItem("title", progress_data->GetTitle()); + dictionary_sp->AddStringItem("details", progress_data->GetDetails()); dictionary_sp->AddStringItem("message", progress_data->GetMessage()); dictionary_sp->AddIntegerItem("progress_id", progress_data->GetID()); dictionary_sp->AddIntegerItem("completed", progress_data->GetCompleted()); Index: lldb/source/Core/Debugger.cpp === --- lldb/source/Core/Debugger.cpp +++ lldb/source/Core/Debugger.cpp @@ -1286,7 +1286,7 @@ } static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id, - const std::string &message, + std::string title, std::string details, uint64_t completed, uint64_t total, bool is_debugger_specific) { // Only deliver progress events if we have any progress listeners. @@ -1294,13 +1294,15 @@ if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type)) return; EventSP event_sp(new Event( - event_type, new ProgressEventData(progress_id, message, completed, total, -is_debugger_specific))); + event_type, + new ProgressEventData(progress_id, std::move(title), std::move(details), +completed, total, is_debugger_specific))); debugger.GetBroadcaster().BroadcastEvent(event_sp); } -void Debugger::ReportProgress(uint64_t progress_id, const std::string &message, - uint64_t completed, uint64_t total, +void Debugger::ReportProgress(uint64_t progress_id, std::string title, + std::string details, uint64_t completed, + uint64_t total, std::optional debugger_id) { // Check if this progress is for a specific debugger. if (debugger_id) { @@ -1308,8 +1310,9 @@ // still exists. DebuggerSP debugger_sp = FindDebuggerWithID(*debugger_id); if (debugger_sp) - PrivateReportProgress(*debugger_sp, progress_id, message, completed, -total, /*is_debugger_specific*/ true); + PrivateReportProgress(*debugger_sp, progress_id, std::move(title), +std::move(details), completed, total, +/*is_debugger_specific*/ true); return; } // The progress event is no