https://github.com/royitaqi updated https://github.com/llvm/llvm-project/pull/89868
>From 079a550481d4cdcb69ad01c376b5e1f0632a07d6 Mon Sep 17 00:00:00 2001 From: Roy Shi <roy...@meta.com> Date: Tue, 23 Apr 2024 18:10:21 -0700 Subject: [PATCH 1/5] Allow multiple destroy callbacks in `SBDebugger::SetDestroyCallback()` --- lldb/include/lldb/Core/Debugger.h | 4 ++-- lldb/source/Core/Debugger.cpp | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h index 418c2403d020f4..20884f954ec7db 100644 --- a/lldb/include/lldb/Core/Debugger.h +++ b/lldb/include/lldb/Core/Debugger.h @@ -731,8 +731,8 @@ class Debugger : public std::enable_shared_from_this<Debugger>, lldb::TargetSP m_dummy_target_sp; Diagnostics::CallbackID m_diagnostics_callback_id; - lldb_private::DebuggerDestroyCallback m_destroy_callback = nullptr; - void *m_destroy_callback_baton = nullptr; + std::vector<std::pair<lldb_private::DebuggerDestroyCallback, void *>> + m_destroy_callback_and_baton; uint32_t m_interrupt_requested = 0; ///< Tracks interrupt requests std::mutex m_interrupt_mutex; diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index 19b3cf3bbf46b1..0ebdf2b0a0f970 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -743,10 +743,11 @@ DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback, } void Debugger::HandleDestroyCallback() { - if (m_destroy_callback) { - m_destroy_callback(GetID(), m_destroy_callback_baton); - m_destroy_callback = nullptr; + const lldb::user_id_t user_id = GetID(); + for (const auto &callback_and_baton : m_destroy_callback_and_baton) { + callback_and_baton.first(user_id, callback_and_baton.second); } + m_destroy_callback_and_baton.clear(); } void Debugger::Destroy(DebuggerSP &debugger_sp) { @@ -1427,8 +1428,7 @@ void Debugger::SetLoggingCallback(lldb::LogOutputCallback log_callback, void Debugger::SetDestroyCallback( lldb_private::DebuggerDestroyCallback destroy_callback, void *baton) { - m_destroy_callback = destroy_callback; - m_destroy_callback_baton = baton; + m_destroy_callback_and_baton.emplace_back(destroy_callback, baton); } static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id, >From 771b52723be8d0ffecaf8f0818105cfdb82ba332 Mon Sep 17 00:00:00 2001 From: Roy Shi <roy...@meta.com> Date: Tue, 23 Apr 2024 21:05:58 -0700 Subject: [PATCH 2/5] Fix code format --- lldb/include/lldb/Core/Debugger.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h index 20884f954ec7db..af025219b0bc12 100644 --- a/lldb/include/lldb/Core/Debugger.h +++ b/lldb/include/lldb/Core/Debugger.h @@ -732,7 +732,7 @@ class Debugger : public std::enable_shared_from_this<Debugger>, Diagnostics::CallbackID m_diagnostics_callback_id; std::vector<std::pair<lldb_private::DebuggerDestroyCallback, void *>> - m_destroy_callback_and_baton; + m_destroy_callback_and_baton; uint32_t m_interrupt_requested = 0; ///< Tracks interrupt requests std::mutex m_interrupt_mutex; >From d1f13cad8a3789a994572459893b32a225ba3e1b Mon Sep 17 00:00:00 2001 From: Roy Shi <roy...@meta.com> Date: Wed, 24 Apr 2024 11:55:16 -0700 Subject: [PATCH 3/5] Add `AddDestroyCallback()` and `ClearDestroyCallback()` --- lldb/include/lldb/Core/Debugger.h | 11 +++++++++++ lldb/source/Core/Debugger.cpp | 12 +++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h index af025219b0bc12..5b3e433f09c68e 100644 --- a/lldb/include/lldb/Core/Debugger.h +++ b/lldb/include/lldb/Core/Debugger.h @@ -568,10 +568,21 @@ class Debugger : public std::enable_shared_from_this<Debugger>, static void ReportSymbolChange(const ModuleSpec &module_spec); + /// Add a callback for when the debugger is destroyed. A list is maintained + /// internally. + void + AddDestroyCallback(lldb_private::DebuggerDestroyCallback destroy_callback, + void *baton); + + /// Clear the list of callbacks, then add the callback. void SetDestroyCallback(lldb_private::DebuggerDestroyCallback destroy_callback, void *baton); + /// Clear the list of callbacks. + void + ClearDestroyCallback(); + /// Manually start the global event handler thread. It is useful to plugins /// that directly use the \a lldb_private namespace and want to use the /// debugger's default event handler thread instead of defining their own. diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index 0ebdf2b0a0f970..159913642f253e 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -1426,11 +1426,21 @@ void Debugger::SetLoggingCallback(lldb::LogOutputCallback log_callback, std::make_shared<CallbackLogHandler>(log_callback, baton); } -void Debugger::SetDestroyCallback( +void Debugger::AddDestroyCallback( lldb_private::DebuggerDestroyCallback destroy_callback, void *baton) { m_destroy_callback_and_baton.emplace_back(destroy_callback, baton); } +void Debugger::SetDestroyCallback( + lldb_private::DebuggerDestroyCallback destroy_callback, void *baton) { + ClearDestroyCallback(); + AddDestroyCallback(destroy_callback, baton); +} + +void Debugger::ClearDestroyCallback() { + m_destroy_callback_and_baton.clear(); +} + static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id, std::string title, std::string details, uint64_t completed, uint64_t total, >From f536124f6f31ac72baf601fd438f21a04ac20691 Mon Sep 17 00:00:00 2001 From: Roy Shi <roy...@meta.com> Date: Wed, 24 Apr 2024 14:18:25 -0700 Subject: [PATCH 4/5] Reflect API changes in `SBDebugger` class and add test --- lldb/include/lldb/API/SBDebugger.h | 7 +++- lldb/source/API/SBDebugger.cpp | 26 +++++++++--- .../python_api/debugger/TestDebuggerAPI.py | 42 +++++++++++++++++++ 3 files changed, 69 insertions(+), 6 deletions(-) diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h index cf5409a12a056a..2501f8efb45ad5 100644 --- a/lldb/include/lldb/API/SBDebugger.h +++ b/lldb/include/lldb/API/SBDebugger.h @@ -194,7 +194,7 @@ class LLDB_API SBDebugger { lldb::SBCommandInterpreter GetCommandInterpreter(); void HandleCommand(const char *command); - + void RequestInterrupt(); void CancelInterruptRequest(); bool InterruptRequested(); @@ -321,9 +321,14 @@ class LLDB_API SBDebugger { void SetLoggingCallback(lldb::LogOutputCallback log_callback, void *baton); + void AddDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback, + void *baton); + void SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback, void *baton); + void ClearDestroyCallback(); + #ifndef SWIG LLDB_DEPRECATED_FIXME("Use DispatchInput(const void *, size_t)", "DispatchInput(const void *, size_t)") diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp index fbcf30e67fc1cd..754e3d27024a9f 100644 --- a/lldb/source/API/SBDebugger.cpp +++ b/lldb/source/API/SBDebugger.cpp @@ -1686,6 +1686,15 @@ void SBDebugger::SetLoggingCallback(lldb::LogOutputCallback log_callback, } } +void SBDebugger::AddDestroyCallback( + lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) { + LLDB_INSTRUMENT_VA(this, destroy_callback, baton); + if (m_opaque_sp) { + return m_opaque_sp->AddDestroyCallback( + destroy_callback, baton); + } +} + void SBDebugger::SetDestroyCallback( lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) { LLDB_INSTRUMENT_VA(this, destroy_callback, baton); @@ -1695,6 +1704,13 @@ void SBDebugger::SetDestroyCallback( } } +void SBDebugger::ClearDestroyCallback() { + LLDB_INSTRUMENT_VA(this); + if (m_opaque_sp) { + return m_opaque_sp->ClearDestroyCallback(); + } +} + SBTrace SBDebugger::LoadTraceFromFile(SBError &error, const SBFileSpec &trace_description_file) { @@ -1704,20 +1720,20 @@ SBDebugger::LoadTraceFromFile(SBError &error, void SBDebugger::RequestInterrupt() { LLDB_INSTRUMENT_VA(this); - + if (m_opaque_sp) - m_opaque_sp->RequestInterrupt(); + m_opaque_sp->RequestInterrupt(); } void SBDebugger::CancelInterruptRequest() { LLDB_INSTRUMENT_VA(this); - + if (m_opaque_sp) - m_opaque_sp->CancelInterruptRequest(); + m_opaque_sp->CancelInterruptRequest(); } bool SBDebugger::InterruptRequested() { LLDB_INSTRUMENT_VA(this); - + if (m_opaque_sp) return m_opaque_sp->InterruptRequested(); return false; diff --git a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py index 522de2466012ed..46c62ea222a2c8 100644 --- a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py +++ b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py @@ -161,3 +161,45 @@ def foo(dbg_id): original_dbg_id = self.dbg.GetID() self.dbg.Destroy(self.dbg) self.assertEqual(destroy_dbg_id, original_dbg_id) + + def test_AddDestroyCallback(self): + destroy_dbg_id = None + called = [] + + def foo(dbg_id): + # Need nonlocal to modify closure variable. + nonlocal destroy_dbg_id + destroy_dbg_id = dbg_id + + nonlocal called + called += ['foo'] + + def bar(dbg_id): + # Need nonlocal to modify closure variable. + nonlocal destroy_dbg_id + destroy_dbg_id = dbg_id + + nonlocal called + called += ['bar'] + + self.dbg.AddDestroyCallback(foo) + self.dbg.AddDestroyCallback(bar) + + original_dbg_id = self.dbg.GetID() + self.dbg.Destroy(self.dbg) + self.assertEqual(destroy_dbg_id, original_dbg_id) + self.assertEqual(called, ['bar', 'foo']) + + def test_ClearDestroyCallback(self): + destroy_dbg_id = None + + def foo(dbg_id): + # Need nonlocal to modify closure variable. + nonlocal destroy_dbg_id + destroy_dbg_id = dbg_id + + self.dbg.AddDestroyCallback(foo) + self.dbg.ClearDestroyCallback() + + self.dbg.Destroy(self.dbg) + self.assertEqual(destroy_dbg_id, None) >From 661dc1c27279d35ea9bf30a76fd1cbce045b37c7 Mon Sep 17 00:00:00 2001 From: Roy Shi <roy...@meta.com> Date: Wed, 24 Apr 2024 14:47:52 -0700 Subject: [PATCH 5/5] Update test --- .../python_api/debugger/TestDebuggerAPI.py | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py index 46c62ea222a2c8..cbcf60587a89aa 100644 --- a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py +++ b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py @@ -163,32 +163,25 @@ def foo(dbg_id): self.assertEqual(destroy_dbg_id, original_dbg_id) def test_AddDestroyCallback(self): - destroy_dbg_id = None + original_dbg_id = self.dbg.GetID() called = [] def foo(dbg_id): # Need nonlocal to modify closure variable. - nonlocal destroy_dbg_id - destroy_dbg_id = dbg_id - nonlocal called - called += ['foo'] + called += [('foo', dbg_id)] def bar(dbg_id): # Need nonlocal to modify closure variable. - nonlocal destroy_dbg_id - destroy_dbg_id = dbg_id - nonlocal called - called += ['bar'] + called += [('bar', dbg_id)] self.dbg.AddDestroyCallback(foo) self.dbg.AddDestroyCallback(bar) - - original_dbg_id = self.dbg.GetID() self.dbg.Destroy(self.dbg) - self.assertEqual(destroy_dbg_id, original_dbg_id) - self.assertEqual(called, ['bar', 'foo']) + + # Should first call `foo()`, then `bar()` + self.assertEqual(called, [('foo', original_dbg_id), ('bar', original_dbg_id)]) def test_ClearDestroyCallback(self): destroy_dbg_id = None @@ -200,6 +193,7 @@ def foo(dbg_id): self.dbg.AddDestroyCallback(foo) self.dbg.ClearDestroyCallback() - self.dbg.Destroy(self.dbg) + + # `foo()` should never be called self.assertEqual(destroy_dbg_id, None) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits