llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Janet Yang (qxy11) <details> <summary>Changes</summary> ## Summary: This change introduces a DAPSessionManager to enable multiple DAP sessions to share debugger instances when needed, for things like child process debugging and some scripting hooks that create dynamically new targets. Changes include: - Add DAPSessionManager singleton to track and coordinate all active DAP sessions - Support attaching to an existing target via its globally unique target ID (targetId parameter) - Share debugger instances across sessions when new targets are created dynamically - Refactor event thread management to allow sharing event threads between sessions - Add eBroadcastBitNewTargetCreated event to notify when new targets are created - Extract session names from target creation events - Defer debugger initialization from 'initialize' request to 'launch'/'attach' requests. The only time the debugger is used currently in between its creation in `InitializeRequestHandler` and the `Launch` or `Attach` requests is during the `TelemetryDispatcher` destruction call at the end of the `DAP::HandleObject` call, so this is safe. This enables scenarios when new targets are created dynamically so that the debug adapter can automatically start a new debug session for the spawned target while sharing the debugger instance. ## Tests: The refactoring maintains backward compatibility. All existing DAP test cases pass. Also added a few basic unit tests for DAPSessionManager ``` >> ninja DAPTests >> ./tools/lldb/unittests/DAP/DAPTests >>./bin/llvm-lit -v ../llvm-project/lldb/test/API/tools/lldb-dap/ ``` --- Patch is 56.92 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/163653.diff 21 Files Affected: - (modified) lldb/include/lldb/API/SBTarget.h (+3) - (modified) lldb/include/lldb/Target/Target.h (+9) - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+3) - (modified) lldb/source/API/SBTarget.cpp (+8) - (modified) lldb/source/Target/Target.cpp (+32-2) - (modified) lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py (+13) - (modified) lldb/tools/lldb-dap/CMakeLists.txt (+1) - (modified) lldb/tools/lldb-dap/DAP.cpp (+190-41) - (modified) lldb/tools/lldb-dap/DAP.h (+24-1) - (added) lldb/tools/lldb-dap/DAPSessionManager.cpp (+168) - (added) lldb/tools/lldb-dap/DAPSessionManager.h (+119) - (modified) lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp (+22-4) - (modified) lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp (+2-56) - (modified) lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp (+4) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+2-1) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+3) - (modified) lldb/tools/lldb-dap/package.json (+4) - (modified) lldb/tools/lldb-dap/tool/lldb-dap.cpp (+16-37) - (modified) lldb/unittests/DAP/CMakeLists.txt (+1) - (added) lldb/unittests/DAP/DAPSessionManagerTest.cpp (+177) - (modified) llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn (+1) ``````````diff diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 173fd05b54a13..1d251763a826a 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -44,6 +44,7 @@ class LLDB_API SBTarget { eBroadcastBitWatchpointChanged = (1 << 3), eBroadcastBitSymbolsLoaded = (1 << 4), eBroadcastBitSymbolsChanged = (1 << 5), + eBroadcastBitNewTargetCreated = (1 << 6), }; // Constructors @@ -69,6 +70,8 @@ class LLDB_API SBTarget { static lldb::SBModule GetModuleAtIndexFromEvent(const uint32_t idx, const lldb::SBEvent &event); + static const char *GetSessionNameFromEvent(const SBEvent &event); + static const char *GetBroadcasterClassName(); lldb::SBProcess GetProcess(); diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index f4a09237ce897..86349897762f4 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -537,6 +537,7 @@ class Target : public std::enable_shared_from_this<Target>, eBroadcastBitWatchpointChanged = (1 << 3), eBroadcastBitSymbolsLoaded = (1 << 4), eBroadcastBitSymbolsChanged = (1 << 5), + eBroadcastBitNewTargetCreated = (1 << 6), }; // These two functions fill out the Broadcaster interface: @@ -556,6 +557,11 @@ class Target : public std::enable_shared_from_this<Target>, TargetEventData(const lldb::TargetSP &target_sp, const ModuleList &module_list); + TargetEventData(const lldb::TargetSP &target_sp, std::string session_name); + + TargetEventData(const lldb::TargetSP &target_sp, + const ModuleList &module_list, std::string session_name); + ~TargetEventData() override; static llvm::StringRef GetFlavorString(); @@ -564,6 +570,8 @@ class Target : public std::enable_shared_from_this<Target>, return TargetEventData::GetFlavorString(); } + static llvm::StringRef GetSessionNameFromEvent(const Event *event_ptr); + void Dump(Stream *s) const override; static const TargetEventData *GetEventDataFromEvent(const Event *event_ptr); @@ -579,6 +587,7 @@ class Target : public std::enable_shared_from_this<Target>, private: lldb::TargetSP m_target_sp; ModuleList m_module_list; + std::string m_session_name; TargetEventData(const TargetEventData &) = delete; const TargetEventData &operator=(const TargetEventData &) = delete; diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py index 8eb64b4ab8b2b..b52aaf558aa24 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py @@ -785,6 +785,7 @@ def request_attach( *, program: Optional[str] = None, pid: Optional[int] = None, + targetId: Optional[int] = None, waitFor=False, initCommands: Optional[list[str]] = None, preRunCommands: Optional[list[str]] = None, @@ -804,6 +805,8 @@ def request_attach( args_dict["pid"] = pid if program is not None: args_dict["program"] = program + if targetId is not None: + args_dict["targetId"] = targetId if waitFor: args_dict["waitFor"] = waitFor args_dict["initCommands"] = self.init_commands diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index 98d10aa07c53f..0f4508c772ee4 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -145,6 +145,14 @@ SBModule SBTarget::GetModuleAtIndexFromEvent(const uint32_t idx, return SBModule(module_list.GetModuleAtIndex(idx)); } +const char *SBTarget::GetSessionNameFromEvent(const SBEvent &event) { + LLDB_INSTRUMENT_VA(event); + + return ConstString( + Target::TargetEventData::GetSessionNameFromEvent(event.get())) + .AsCString(); +} + const char *SBTarget::GetBroadcasterClassName() { LLDB_INSTRUMENT(); diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index e224a12e33463..a13e11c4b3a05 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -193,6 +193,7 @@ Target::Target(Debugger &debugger, const ArchSpec &target_arch, SetEventName(eBroadcastBitModulesUnloaded, "modules-unloaded"); SetEventName(eBroadcastBitWatchpointChanged, "watchpoint-changed"); SetEventName(eBroadcastBitSymbolsLoaded, "symbols-loaded"); + SetEventName(eBroadcastBitNewTargetCreated, "new-target-created"); CheckInWithManager(); @@ -5169,11 +5170,21 @@ void TargetProperties::SetDebugUtilityExpression(bool debug) { // Target::TargetEventData Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp) - : EventData(), m_target_sp(target_sp), m_module_list() {} + : TargetEventData(target_sp, ModuleList(), "") {} Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp, const ModuleList &module_list) - : EventData(), m_target_sp(target_sp), m_module_list(module_list) {} + : TargetEventData(target_sp, module_list, "") {} + +Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp, + std::string session_name) + : TargetEventData(target_sp, ModuleList(), std::move(session_name)) {} + +Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp, + const ModuleList &module_list, + std::string session_name) + : EventData(), m_target_sp(target_sp), m_module_list(module_list), + m_session_name(std::move(session_name)) {} Target::TargetEventData::~TargetEventData() = default; @@ -5209,6 +5220,25 @@ TargetSP Target::TargetEventData::GetTargetFromEvent(const Event *event_ptr) { return target_sp; } +llvm::StringRef +Target::TargetEventData::GetSessionNameFromEvent(const Event *event_ptr) { + const TargetEventData *event_data = GetEventDataFromEvent(event_ptr); + if (!event_data) + return llvm::StringRef(); + + if (!event_data->m_session_name.empty()) + return event_data->m_session_name; + + // Generate default session name if not provided + if (event_data->m_target_sp) { + lldb::user_id_t target_id = event_data->m_target_sp->GetGloballyUniqueID(); + std::string default_name = llvm::formatv("Session {0}", target_id).str(); + return ConstString(default_name).GetStringRef(); + } + + return llvm::StringRef(); +} + ModuleList Target::TargetEventData::GetModuleListFromEvent(const Event *event_ptr) { ModuleList module_list; diff --git a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py index 5331a9f94ef1f..6ca286c12abd2 100644 --- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py +++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py @@ -72,3 +72,16 @@ def test_by_name_waitFor(self): self.spawn_thread.start() self.attach(program=program, waitFor=True) self.continue_and_verify_pid() + + def test_attach_with_invalid_targetId(self): + """ + Test that attaching with an invalid targetId fails with the expected + error message. + """ + self.build_and_create_debug_adapter() + + resp = self.attach(targetId=99999, expectFailure=True) + self.assertFalse(resp["success"]) + self.assertIn( + "Unable to find existing debugger", resp["body"]["error"]["format"] + ) diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt index 7db334ca56bcf..2fbdc73a79978 100644 --- a/lldb/tools/lldb-dap/CMakeLists.txt +++ b/lldb/tools/lldb-dap/CMakeLists.txt @@ -12,6 +12,7 @@ add_lldb_library(lldbDAP DAP.cpp DAPError.cpp DAPLog.cpp + DAPSessionManager.cpp EventHelper.cpp ExceptionBreakpoint.cpp FifoFiles.cpp diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index f76656e98ca01..dc681336637e0 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "DAP.h" +#include "CommandPlugins.h" #include "DAPLog.h" #include "EventHelper.h" #include "ExceptionBreakpoint.h" @@ -241,10 +242,12 @@ llvm::Error DAP::ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr) { } void DAP::StopEventHandlers() { - if (event_thread.joinable()) { - broadcaster.BroadcastEventByType(eBroadcastBitStopEventThread); - event_thread.join(); - } + event_thread_sp.reset(); + + // Clean up expired event threads from the session manager. + DAPSessionManager::GetInstance().ReleaseExpiredEventThreads(); + + // Still handle the progress thread normally since it's per-DAP instance. if (progress_event_thread.joinable()) { broadcaster.BroadcastEventByType(eBroadcastBitStopProgressThread); progress_event_thread.join(); @@ -804,7 +807,8 @@ void DAP::SetTarget(const lldb::SBTarget target) { lldb::SBTarget::eBroadcastBitModulesLoaded | lldb::SBTarget::eBroadcastBitModulesUnloaded | lldb::SBTarget::eBroadcastBitSymbolsLoaded | - lldb::SBTarget::eBroadcastBitSymbolsChanged); + lldb::SBTarget::eBroadcastBitSymbolsChanged | + lldb::SBTarget::eBroadcastBitNewTargetCreated); listener.StartListeningForEvents(this->broadcaster, eBroadcastBitStopEventThread); } @@ -1289,13 +1293,91 @@ protocol::Capabilities DAP::GetCustomCapabilities() { } void DAP::StartEventThread() { - event_thread = std::thread(&DAP::EventThread, this); + // Get event thread for this debugger (creates it if it doesn't exist). + event_thread_sp = DAPSessionManager::GetInstance().GetEventThreadForDebugger( + debugger, this); } void DAP::StartProgressEventThread() { progress_event_thread = std::thread(&DAP::ProgressEventThread, this); } +void DAP::StartEventThreads() { + if (clientFeatures.contains(eClientFeatureProgressReporting)) + StartProgressEventThread(); + + StartEventThread(); +} + +llvm::Error DAP::InitializeDebugger(std::optional<uint32_t> target_id) { + // Initialize debugger instance (shared or individual). + if (target_id) { + std::optional<lldb::SBDebugger> shared_debugger = + DAPSessionManager::GetInstance().GetSharedDebugger(*target_id); + // If the target ID is not valid, then we won't find a debugger. + if (!shared_debugger) { + return llvm::createStringError( + "Unable to find existing debugger for target ID"); + } + debugger = shared_debugger.value(); + StartEventThreads(); + return llvm::Error::success(); + } + + debugger = lldb::SBDebugger::Create(/*argument_name=*/false); + + // Configure input/output/error file descriptors. + debugger.SetInputFile(in); + target = debugger.GetDummyTarget(); + + llvm::Expected<int> out_fd = out.GetWriteFileDescriptor(); + if (!out_fd) + return out_fd.takeError(); + debugger.SetOutputFile(lldb::SBFile(*out_fd, "w", false)); + + llvm::Expected<int> err_fd = err.GetWriteFileDescriptor(); + if (!err_fd) + return err_fd.takeError(); + debugger.SetErrorFile(lldb::SBFile(*err_fd, "w", false)); + + // The sourceInitFile option is not part of the DAP specification. It is an + // extension used by the test suite to prevent sourcing `.lldbinit` and + // changing its behavior. The CLI flag --no-lldbinit takes precedence over + // the DAP parameter. + bool should_source_init_files = !no_lldbinit && sourceInitFile; + if (should_source_init_files) { + debugger.SkipLLDBInitFiles(false); + debugger.SkipAppInitFiles(false); + lldb::SBCommandReturnObject init; + auto interp = debugger.GetCommandInterpreter(); + interp.SourceInitFileInGlobalDirectory(init); + interp.SourceInitFileInHomeDirectory(init); + } + + // Run initialization commands. + if (llvm::Error err = RunPreInitCommands()) + return err; + + auto cmd = debugger.GetCommandInterpreter().AddMultiwordCommand( + "lldb-dap", "Commands for managing lldb-dap."); + + if (clientFeatures.contains(eClientFeatureStartDebuggingRequest)) { + cmd.AddCommand( + "start-debugging", new StartDebuggingCommand(*this), + "Sends a startDebugging request from the debug adapter to the client " + "to start a child debug session of the same type as the caller."); + } + + cmd.AddCommand( + "repl-mode", new ReplModeCommand(*this), + "Get or set the repl behavior of lldb-dap evaluation requests."); + cmd.AddCommand("send-event", new SendEventCommand(*this), + "Sends an DAP event to the client."); + + StartEventThreads(); + return llvm::Error::success(); +} + void DAP::ProgressEventThread() { lldb::SBListener listener("lldb-dap.progress.listener"); debugger.GetBroadcaster().AddListener( @@ -1374,6 +1456,14 @@ void DAP::EventThread() { const auto event_mask = event.GetType(); if (lldb::SBProcess::EventIsProcessEvent(event)) { lldb::SBProcess process = lldb::SBProcess::GetProcessFromEvent(event); + // Find the DAP instance that owns this process's target. + DAP *dap_instance = DAPSessionManager::FindDAP(process.GetTarget()); + if (!dap_instance) { + DAP_LOG(log, "Unable to find DAP instance for process {0}", + process.GetProcessID()); + continue; + } + if (event_mask & lldb::SBProcess::eBroadcastBitStateChanged) { auto state = lldb::SBProcess::GetStateFromEvent(event); switch (state) { @@ -1390,89 +1480,138 @@ void DAP::EventThread() { // Only report a stopped event if the process was not // automatically restarted. if (!lldb::SBProcess::GetRestartedFromEvent(event)) { - SendStdOutStdErr(*this, process); - if (llvm::Error err = SendThreadStoppedEvent(*this)) - DAP_LOG_ERROR(log, std::move(err), + SendStdOutStdErr(*dap_instance, process); + if (llvm::Error err = SendThreadStoppedEvent(*dap_instance)) + DAP_LOG_ERROR(dap_instance->log, std::move(err), "({1}) reporting thread stopped: {0}", - m_client_name); + dap_instance->m_client_name); } break; case lldb::eStateRunning: case lldb::eStateStepping: - WillContinue(); - SendContinuedEvent(*this); + dap_instance->WillContinue(); + SendContinuedEvent(*dap_instance); break; case lldb::eStateExited: lldb::SBStream stream; process.GetStatus(stream); - SendOutput(OutputType::Console, stream.GetData()); + dap_instance->SendOutput(OutputType::Console, stream.GetData()); // When restarting, we can get an "exited" event for the process we // just killed with the old PID, or even with no PID. In that case // we don't have to terminate the session. if (process.GetProcessID() == LLDB_INVALID_PROCESS_ID || - process.GetProcessID() == restarting_process_id) { - restarting_process_id = LLDB_INVALID_PROCESS_ID; + process.GetProcessID() == dap_instance->restarting_process_id) { + dap_instance->restarting_process_id = LLDB_INVALID_PROCESS_ID; } else { // Run any exit LLDB commands the user specified in the // launch.json - RunExitCommands(); - SendProcessExitedEvent(*this, process); - SendTerminatedEvent(); + dap_instance->RunExitCommands(); + SendProcessExitedEvent(*dap_instance, process); + dap_instance->SendTerminatedEvent(); done = true; } break; } } else if ((event_mask & lldb::SBProcess::eBroadcastBitSTDOUT) || (event_mask & lldb::SBProcess::eBroadcastBitSTDERR)) { - SendStdOutStdErr(*this, process); + SendStdOutStdErr(*dap_instance, process); } } else if (lldb::SBTarget::EventIsTargetEvent(event)) { if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded || event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded || event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded || event_mask & lldb::SBTarget::eBroadcastBitSymbolsChanged) { + lldb::SBTarget event_target = + lldb::SBTarget::GetTargetFromEvent(event); + // Find the DAP instance that owns this target. + DAP *dap_instance = DAPSessionManager::FindDAP(event_target); + if (!dap_instance) + continue; + const uint32_t num_modules = lldb::SBTarget::GetNumModulesFromEvent(event); const bool remove_module = event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded; - std::lock_guard<std::mutex> guard(modules_mutex); + std::lock_guard<std::mutex> guard(dap_instance->modules_mutex); for (uint32_t i = 0; i < num_modules; ++i) { lldb::SBModule module = lldb::SBTarget::GetModuleAtIndexFromEvent(i, event); std::optional<protocol::Module> p_module = - CreateModule(target, module, remove_module); + CreateModule(dap_instance->target, module, remove_module); if (!p_module) continue; llvm::StringRef module_id = p_module->id; - const bool module_exists = modules.contains(module_id); + const bool module_exists = + dap_instance->modules.contains(module_id); if (remove_module && module_exists) { - modules.erase(module_id); - Send(protocol::Event{ + dap_instance->modules.erase(module_id); + dap_instance->Send(protocol::Event{ "module", ModuleEventBody{std::move(p_module).value(), ModuleEventBody::eReasonRemoved}}); } else if (module_exists) { - Send(protocol::Event{ + dap_instance->Send(protocol::Event{ "module", ModuleEventBody{std::move(p_module).value(), ModuleEventBody::eReasonChanged}}); } else if (!remove_module) { - modules.insert(module_id); - Send(protocol::Event{ + dap_instance->modules.insert(module_id); + dap_instance->Send(protocol::Event{ "module", ModuleEventBody{std::move(p_module).value(), ModuleEventBody::eReasonNew}}); } } + } else if (event_mask & lldb::SBTarget::eBroadcastBitNewTargetCreated) { + auto target = lldb::SBTarget::GetTargetFromEvent(event); + + // Generate unique target ID and set the shared debugger. + uint32_t target_id = target.GetGloballyUniqueID(); + DAPSessionManager::GetInstance().SetSharedDebugger(target_id, + debugger); + + // We create an attach config that will select the unique + // target ID of the created target. The DAP instance will attach to + // this existing target and the debug session will be ready to go. + llvm::json::Object attach_config; + + // If we have a process name, add command to attach to the same + // process name. + attach_config.try_emplace("type", "lldb"); + attach_config.try_emplace("targetId", target_id); + const char *session_name = + lldb::SBTarget::GetSessionNameFromEvent(event); + attach_config.try_emplace("name", session_name); + + // 2. Construct the main 'startDebugging' request arguments. + llvm::json::Object start_debugging_args{ + {"request", "attach"}, + {"configuratio... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/163653 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
