From eb22c2f76c08bf02807434047816c2d533f161a8 Mon Sep 17 00:00:00 2001 From: qxy11 Date: Thu, 14 Aug 2025 11:02:58 -0700 Subject: [PATCH 01/20] Integration w/ LLDB-DAP using shared event thread --- lldb/include/lldb/API/SBTarget.h | 1 + lldb/include/lldb/Target/Target.h | 1 + .../Process/gdb-remote/ProcessGDBRemote.cpp | 4 + lldb/source/Target/Target.cpp | 1 + lldb/tools/lldb-dap/DAP.cpp | 240 +++++++++++++++--- lldb/tools/lldb-dap/DAP.h | 66 ++++- .../lldb-dap/Handler/AttachRequestHandler.cpp | 11 +- .../Handler/InitializeRequestHandler.cpp | 11 +- lldb/tools/lldb-dap/tool/lldb-dap.cpp | 48 ++-- 9 files changed, 304 insertions(+), 79 deletions(-) diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 2776a8f9010fe..35d0e2a1412a4 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), + eBroadcastBitNewTargetSpawned = (1 << 6), }; // Constructors diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 62a673f8bbff3..341a0eca9fb79 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -536,6 +536,7 @@ class Target : public std::enable_shared_from_this, eBroadcastBitWatchpointChanged = (1 << 3), eBroadcastBitSymbolsLoaded = (1 << 4), eBroadcastBitSymbolsChanged = (1 << 5), + eBroadcastBitNewTargetSpawned = (1 << 6), }; // These two functions fill out the Broadcaster interface: diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 3fefd83c31040..55757dfa9b665 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -1025,6 +1025,10 @@ Status ProcessGDBRemote::HandleConnectionRequest(const GPUActions &gpu_action) { process_sp->GetTarget().shared_from_this()); LLDB_LOG(log, "ProcessGDBRemote::HandleConnectionRequest(): successfully " "created process!!!"); + auto event_sp = std::make_shared( + Target::eBroadcastBitNewTargetSpawned, + new Target::TargetEventData(gpu_target_sp)); + GetTarget().BroadcastEvent(event_sp); return Status(); } diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 4f39f6018e624..d3a7125709c0d 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -190,6 +190,7 @@ Target::Target(Debugger &debugger, const ArchSpec &target_arch, SetEventName(eBroadcastBitModulesUnloaded, "modules-unloaded"); SetEventName(eBroadcastBitWatchpointChanged, "watchpoint-changed"); SetEventName(eBroadcastBitSymbolsLoaded, "symbols-loaded"); + SetEventName(eBroadcastBitNewTargetSpawned, "new-target-spawned"); CheckInWithManager(); diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index cbd3b14463e25..851a15cddea31 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -85,6 +85,105 @@ const char DEV_NULL[] = "/dev/null"; namespace lldb_dap { +static DAPSessionManager *instance = nullptr; +DAPSessionManager& DAPSessionManager::GetInstance() { + if (!instance) { + instance = new DAPSessionManager(); + } + return *instance; +} + +void DAPSessionManager::RegisterSession(lldb::IOObjectSP io, DAP* dap) { + std::lock_guard lock(sessions_mutex_); + active_sessions_[io] = dap; +} + +void DAPSessionManager::UnregisterSession(lldb::IOObjectSP io) { + std::unique_lock lock(sessions_mutex_); + active_sessions_.erase(io); + + // Clean up shared resources when the last session exits + if (active_sessions_.empty()) { + CleanupSharedResources(); + } + + std::notify_all_at_thread_exit(sessions_condition_, std::move(lock)); +} + +std::vector DAPSessionManager::GetActiveSessions() { + std::lock_guard lock(sessions_mutex_); + std::vector sessions; + for (const auto& [io, dap] : active_sessions_) { + if (dap) { + sessions.push_back(dap); + } + } + return sessions; +} + +void DAPSessionManager::DisconnectAllSessions() { + std::lock_guard lock(sessions_mutex_); + for (const auto& [io, dap] : active_sessions_) { + if (dap) { + if (llvm::Error error = dap->Disconnect()) { + llvm::errs() << "DAP client " << dap->transport.GetClientName() + << " disconnected failed: " + << llvm::toString(std::move(error)) << "\n"; + } + } + } +} + +void DAPSessionManager::WaitForAllSessionsToDisconnect() { + std::unique_lock lock(sessions_mutex_); + sessions_condition_.wait(lock, [this] { return active_sessions_.empty(); }); +} + +std::shared_ptr DAPSessionManager::GetEventThreadForDebugger(lldb::SBDebugger debugger, DAP* requesting_dap) { + lldb::user_id_t debugger_id = debugger.GetID(); + + std::lock_guard lock(sessions_mutex_); + + // Check if we already have a thread (most common case) + auto it = debugger_event_threads_.find(debugger_id); + if (it != debugger_event_threads_.end() && it->second) { + return it->second; + } + + // Create new thread and store it + auto new_thread = std::make_shared(&DAP::EventThread, requesting_dap); + debugger_event_threads_[debugger_id] = new_thread; + return new_thread; +} + +void DAPSessionManager::SetSharedDebugger(lldb::SBDebugger debugger) { + std::lock_guard lock(sessions_mutex_); + shared_debugger_ = debugger; +} + +std::optional DAPSessionManager::GetSharedDebugger() { + std::lock_guard lock(sessions_mutex_); + return shared_debugger_; +} + +DAP* DAPSessionManager::FindDAPForTarget(lldb::SBTarget target) { + std::lock_guard lock(sessions_mutex_); + + for (const auto& [io, dap] : active_sessions_) { + if (dap && dap->target.IsValid() && dap->target == target) { + return dap; + } + } + + return nullptr; +} + +void DAPSessionManager::CleanupSharedResources() { + if (shared_debugger_.has_value() && shared_debugger_->IsValid()) { + shared_debugger_ = std::nullopt; + } +} + static std::string GetStringFromStructuredData(lldb::SBStructuredData &data, const char *key) { lldb::SBStructuredData keyValue = data.GetValueForKey(key); @@ -237,11 +336,17 @@ llvm::Error DAP::ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr) { return llvm::Error::success(); } + void DAP::StopEventHandlers() { - if (event_thread.joinable()) { + // Check if this is the last reference to the shared event thread + if (event_thread_sp && event_thread_sp.use_count() == 1 && event_thread_sp->joinable()) { + // Signal the shared event thread to stop broadcaster.BroadcastEventByType(eBroadcastBitStopEventThread); - event_thread.join(); + + event_thread_sp->join(); } + + // Still handle the progress thread normally since it's per-DAP instance if (progress_event_thread.joinable()) { broadcaster.BroadcastEventByType(eBroadcastBitStopProgressThread); progress_event_thread.join(); @@ -786,7 +891,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::eBroadcastBitNewTargetSpawned); listener.StartListeningForEvents(this->broadcaster, eBroadcastBitStopEventThread); } @@ -1209,7 +1315,8 @@ protocol::Capabilities DAP::GetCapabilities() { } 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() { @@ -1276,6 +1383,7 @@ void DAP::ProgressEventThread() { } } + // All events from the debugger, target, process, thread and frames are // received in this function that runs in its own thread. We are using a // "FILE *" to output packets back to VS Code and they have mutexes in them @@ -1294,6 +1402,10 @@ 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::GetInstance().FindDAPForTarget(process.GetTarget()); + if (!dap_instance) continue; + if (event_mask & lldb::SBProcess::eBroadcastBitStateChanged) { auto state = lldb::SBProcess::GetStateFromEvent(event); switch (state) { @@ -1310,89 +1422,135 @@ 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}", - transport.GetClientName()); + dap_instance->transport.GetClientName()); } 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::GetInstance().FindDAPForTarget(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 guard(modules_mutex); + std::lock_guard guard(dap_instance->modules_mutex); for (uint32_t i = 0; i < num_modules; ++i) { lldb::SBModule module = lldb::SBTarget::GetModuleAtIndexFromEvent(i, event); std::optional 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::eBroadcastBitNewTargetSpawned) { + auto target = lldb::SBTarget::GetTargetFromEvent(event); + auto target_index = debugger.GetIndexOfTarget(target); + + // Set the shared debugger for GPU processes + DAPSessionManager::GetInstance().SetSharedDebugger(debugger); + + llvm::json::Object attach_config; + llvm::json::Array attach_commands; + + attach_commands.push_back(llvm::formatv("target list").str()); + attach_commands.push_back( + llvm::formatv("target select {0}", target_index).str()); + + // 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("name", "GPU Session"); + attach_config.try_emplace("attachCommands", + std::move(attach_commands)); + + // 2. Construct the main 'startDebugging' request arguments. + llvm::json::Object start_debugging_args; + start_debugging_args.try_emplace("request", "attach"); + start_debugging_args.try_emplace("configuration", + std::move(attach_config)); + + // Send the request. Note that this is a reverse request, so you don't + // expect a direct response in the same way as a client request. + SendReverseRequest( + "startDebugging", std::move(start_debugging_args)); } } else if (lldb::SBBreakpoint::EventIsBreakpointEvent(event)) { + lldb::SBBreakpoint bp = lldb::SBBreakpoint::GetBreakpointFromEvent(event); + if (!bp.IsValid()) continue; + + lldb::SBTarget event_target = bp.GetTarget(); + + // Find the DAP instance that owns this target + DAP* dap_instance = DAPSessionManager::GetInstance().FindDAPForTarget(event_target); + if (!dap_instance) continue; + if (event_mask & lldb::SBTarget::eBroadcastBitBreakpointChanged) { auto event_type = lldb::SBBreakpoint::GetBreakpointEventTypeFromEvent(event); - auto bp = Breakpoint( - *this, lldb::SBBreakpoint::GetBreakpointFromEvent(event)); + auto breakpoint = Breakpoint(*dap_instance, bp); // If the breakpoint was set through DAP, it will have the // BreakpointBase::kDAPBreakpointLabel. Regardless of whether // locations were added, removed, or resolved, the breakpoint isn't @@ -1400,13 +1558,13 @@ void DAP::EventThread() { if ((event_type & lldb::eBreakpointEventTypeLocationsAdded || event_type & lldb::eBreakpointEventTypeLocationsRemoved || event_type & lldb::eBreakpointEventTypeLocationsResolved) && - bp.MatchesName(BreakpointBase::kDAPBreakpointLabel)) { + breakpoint.MatchesName(BreakpointBase::kDAPBreakpointLabel)) { // As the DAP client already knows the path of this breakpoint, we // don't need to send it back as part of the "changed" event. This // avoids sending paths that should be source mapped. Note that // CreateBreakpoint doesn't apply source mapping and certain // implementation ignore the source part of this event anyway. - llvm::json::Value source_bp = bp.ToProtocolBreakpoint(); + llvm::json::Value source_bp = breakpoint.ToProtocolBreakpoint(); source_bp.getAsObject()->erase("source"); llvm::json::Object body; @@ -1416,19 +1574,25 @@ void DAP::EventThread() { llvm::json::Object bp_event = CreateEventObject("breakpoint"); bp_event.try_emplace("body", std::move(body)); - SendJSON(llvm::json::Value(std::move(bp_event))); + dap_instance->SendJSON(llvm::json::Value(std::move(bp_event))); } } } else if (event_mask & lldb::eBroadcastBitError || event_mask & lldb::eBroadcastBitWarning) { - lldb::SBStructuredData data = - lldb::SBDebugger::GetDiagnosticFromEvent(event); - if (!data.IsValid()) - continue; - std::string type = GetStringValue(data.GetValueForKey("type")); - std::string message = GetStringValue(data.GetValueForKey("message")); - SendOutput(OutputType::Important, - llvm::formatv("{0}: {1}", type, message).str()); + // Global debugger events - send to all DAP instances + std::vector active_instances = DAPSessionManager::GetInstance().GetActiveSessions(); + for (DAP* dap_instance : active_instances) { + if (!dap_instance) continue; + + lldb::SBStructuredData data = + lldb::SBDebugger::GetDiagnosticFromEvent(event); + if (!data.IsValid()) continue; + + std::string type = GetStringValue(data.GetValueForKey("type")); + std::string message = GetStringValue(data.GetValueForKey("message")); + dap_instance->SendOutput(OutputType::Important, + llvm::formatv("{0}: {1}", type, message).str()); + } } else if (event.BroadcasterMatchesRef(broadcaster)) { if (event_mask & eBroadcastBitStopEventThread) { done = true; diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index af4aabaafaae8..fcf46095f38ec 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -45,6 +45,7 @@ #include #include #include +#include #include #include #include @@ -64,6 +65,65 @@ typedef llvm::DenseMap using AdapterFeature = protocol::AdapterFeature; using ClientFeature = protocol::ClientFeature; +/// Global DAP session manager +class DAPSessionManager { +public: + /// Get the singleton instance of the DAP session manager + static DAPSessionManager& GetInstance(); + + /// Register a DAP session + void RegisterSession(lldb::IOObjectSP io, DAP* dap); + + /// Unregister a DAP session + void UnregisterSession(lldb::IOObjectSP io); + + /// Get all active DAP sessions + std::vector GetActiveSessions(); + + /// Disconnect all active sessions + void DisconnectAllSessions(); + + /// Wait for all sessions to finish disconnecting + void WaitForAllSessionsToDisconnect(); + + /// Set the shared debugger instance (only for GPU processes) + void SetSharedDebugger(lldb::SBDebugger debugger); + + /// Get the shared debugger instance if it exists + std::optional GetSharedDebugger(); + + /// Get or create event thread for a specific debugger + std::shared_ptr GetEventThreadForDebugger(lldb::SBDebugger debugger, DAP* requesting_dap); + + /// Find the DAP instance that owns the given target + DAP* FindDAPForTarget(lldb::SBTarget target); + + /// Clean up shared resources when the last session exits + void CleanupSharedResources(); + +private: + DAPSessionManager() = default; + ~DAPSessionManager() = default; + + // Non-copyable and non-movable + DAPSessionManager(const DAPSessionManager&) = delete; + DAPSessionManager& operator=(const DAPSessionManager&) = delete; + DAPSessionManager(DAPSessionManager&&) = delete; + DAPSessionManager& operator=(DAPSessionManager&&) = delete; + + std::mutex sessions_mutex_; + std::condition_variable sessions_condition_; + std::map active_sessions_; + + /// Optional shared debugger instance set when the native process + /// spawns a new GPU target + std::optional shared_debugger_; + + /// Map from debugger ID to its event thread used for when + /// multiple DAP sessions are using the same debugger instance. + std::map> debugger_event_threads_; +}; + enum class OutputType { Console, Important, Stdout, Stderr, Telemetry }; /// Buffer size for handling output events. @@ -77,6 +137,8 @@ enum DAPBroadcasterBits { enum class ReplMode { Variable = 0, Command, Auto }; struct DAP { + friend class DAPSessionManager; + /// Path to the lldb-dap binary itself. static llvm::StringRef debug_adapter_path; @@ -438,7 +500,9 @@ struct DAP { void EventThread(); void ProgressEventThread(); - std::thread event_thread; + /// Event thread is a shared pointer in case we have a multiple + /// DAP instances sharing the same event thread + std::shared_ptr event_thread_sp; std::thread progress_event_thread; /// @} diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp index 371349a26866e..2e294171bcaeb 100644 --- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp @@ -64,11 +64,12 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { dap.ConfigureSourceMaps(); lldb::SBError error; - lldb::SBTarget target = dap.CreateTarget(error); - if (error.Fail()) - return ToError(error); - - dap.SetTarget(target); + if (!dap.debugger.GetSelectedTarget().IsValid()) { + lldb::SBTarget target = dap.CreateTarget(error); + if (error.Fail()) + return ToError(error); + dap.SetTarget(target); + } // Run any pre run LLDB commands the user specified in the launch.json if (Error err = dap.RunPreRunCommands()) diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp index b499a69876e2c..60d0542273486 100644 --- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp @@ -23,8 +23,15 @@ llvm::Expected InitializeRequestHandler::Run( const InitializeRequestArguments &arguments) const { dap.clientFeatures = arguments.supportedFeatures; - // Do not source init files until in/out/err are configured. - dap.debugger = lldb::SBDebugger::Create(false); + // Check if we already have a shared debugger (eg. for GPU processes), otherwise create individual debugger + auto shared_debugger = DAPSessionManager::GetInstance().GetSharedDebugger(); + if (shared_debugger.has_value()) { + dap.debugger = shared_debugger.value(); + } else { + // Create individual debugger for this DAP instance + dap.debugger = lldb::SBDebugger::Create(false); + } + dap.debugger.SetInputFile(dap.in); dap.target = dap.debugger.GetDummyTarget(); diff --git a/lldb/tools/lldb-dap/tool/lldb-dap.cpp b/lldb/tools/lldb-dap/tool/lldb-dap.cpp index 8bba4162aa7bf..b9a4db386a55a 100644 --- a/lldb/tools/lldb-dap/tool/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/tool/lldb-dap.cpp @@ -282,13 +282,8 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name, g_loop.AddPendingCallback( [](MainLoopBase &loop) { loop.RequestTermination(); }); }); - std::condition_variable dap_sessions_condition; - std::mutex dap_sessions_mutex; - std::map dap_sessions; unsigned int clientCount = 0; - auto handle = listener->Accept(g_loop, [=, &dap_sessions_condition, - &dap_sessions_mutex, &dap_sessions, - &clientCount]( + auto handle = listener->Accept(g_loop, [=, &clientCount]( std::unique_ptr sock) { std::string client_name = llvm::formatv("client_{0}", clientCount++).str(); DAP_LOG(log, "({0}) client connected", client_name); @@ -297,8 +292,7 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name, // Move the client into a background thread to unblock accepting the next // client. - std::thread client([=, &dap_sessions_condition, &dap_sessions_mutex, - &dap_sessions]() { + std::thread client([=]() { llvm::set_thread_name(client_name + ".runloop"); Transport transport(client_name, log, io, io); DAP dap(log, default_repl_mode, pre_init_commands, transport); @@ -309,10 +303,8 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name, return; } - { - std::scoped_lock lock(dap_sessions_mutex); - dap_sessions[io.get()] = &dap; - } + // Register the DAP session with the global manager + DAPSessionManager::GetInstance().RegisterSession(io, &dap); if (auto Err = dap.Loop()) { llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), @@ -321,9 +313,8 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name, } DAP_LOG(log, "({0}) client disconnected", client_name); - std::unique_lock lock(dap_sessions_mutex); - dap_sessions.erase(io.get()); - std::notify_all_at_thread_exit(dap_sessions_condition, std::move(lock)); + // Unregister the DAP session from the global manager + DAPSessionManager::GetInstance().UnregisterSession(io); }); client.detach(); }); @@ -341,26 +332,11 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name, log, "lldb-dap server shutdown requested, disconnecting remaining clients..."); - bool client_failed = false; - { - std::scoped_lock lock(dap_sessions_mutex); - for (auto [sock, dap] : dap_sessions) { - if (llvm::Error error = dap->Disconnect()) { - client_failed = true; - llvm::errs() << "DAP client " << dap->transport.GetClientName() - << " disconnected failed: " - << llvm::toString(std::move(error)) << "\n"; - } - } - } + // Disconnect all active sessions using the global manager + DAPSessionManager::GetInstance().DisconnectAllSessions(); // Wait for all clients to finish disconnecting. - std::unique_lock lock(dap_sessions_mutex); - dap_sessions_condition.wait(lock, [&] { return dap_sessions.empty(); }); - - if (client_failed) - return llvm::make_error( - "disconnecting all clients failed", llvm::inconvertibleErrorCode()); + DAPSessionManager::GetInstance().WaitForAllSessionsToDisconnect(); return llvm::Error::success(); } @@ -560,6 +536,10 @@ int main(int argc, char *argv[]) { return EXIT_FAILURE; } + // Register the DAP session with the global manager for stdio mode + // This is needed for the event handling to find the correct DAP instance + DAPSessionManager::GetInstance().RegisterSession(input, &dap); + // used only by TestVSCode_redirection_to_console.py if (getenv("LLDB_DAP_TEST_STDOUT_STDERR_REDIRECTION") != nullptr) redirection_test(); @@ -569,7 +549,9 @@ int main(int argc, char *argv[]) { llvm::toStringWithoutConsuming(Err)); llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "DAP session error: "); + DAPSessionManager::GetInstance().UnregisterSession(input); return EXIT_FAILURE; } + DAPSessionManager::GetInstance().UnregisterSession(input); return EXIT_SUCCESS; } From 412cf86a27f1e3ca1cadba999e6d98b8ee5ff3b1 Mon Sep 17 00:00:00 2001 From: qxy11 Date: Thu, 14 Aug 2025 11:02:58 -0700 Subject: [PATCH 02/20] Add targetIdx to attach config --- lldb/tools/lldb-dap/DAP.cpp | 5 +++-- .../lldb-dap/Handler/AttachRequestHandler.cpp | 17 ++++++++++++----- .../lldb-dap/Protocol/ProtocolRequests.cpp | 3 ++- lldb/tools/lldb-dap/Protocol/ProtocolRequests.h | 3 +++ lldb/tools/lldb-dap/package.json | 4 ++++ 5 files changed, 24 insertions(+), 8 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 851a15cddea31..7fc02072c39ab 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -1507,10 +1507,10 @@ void DAP::EventThread() { } else if (event_mask & lldb::SBTarget::eBroadcastBitNewTargetSpawned) { auto target = lldb::SBTarget::GetTargetFromEvent(event); auto target_index = debugger.GetIndexOfTarget(target); - + // Set the shared debugger for GPU processes DAPSessionManager::GetInstance().SetSharedDebugger(debugger); - + llvm::json::Object attach_config; llvm::json::Array attach_commands; @@ -1525,6 +1525,7 @@ void DAP::EventThread() { attach_config.try_emplace("name", "GPU Session"); attach_config.try_emplace("attachCommands", std::move(attach_commands)); + attach_config.try_emplace("targetIdx", target_index); // 2. Construct the main 'startDebugging' request arguments. llvm::json::Object start_debugging_args; diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp index 2e294171bcaeb..0096842c258b6 100644 --- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp @@ -64,12 +64,19 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { dap.ConfigureSourceMaps(); lldb::SBError error; - if (!dap.debugger.GetSelectedTarget().IsValid()) { - lldb::SBTarget target = dap.CreateTarget(error); - if (error.Fail()) - return ToError(error); - dap.SetTarget(target); + lldb::SBTarget target; + if (use_shared_debugger) { + lldb::SBTarget target = dap.debugger.GetTargetAtIndex(args.targetIdx); + if (!target.IsValid()) { + error.SetErrorStringWithFormat("invalid target_idx %u in attach config", + args.targetIdx); + } + } else { + target = dap.CreateTarget(error); } + if (error.Fail()) + return ToError(error); + dap.SetTarget(target); // Run any pre run LLDB commands the user specified in the launch.json if (Error err = dap.RunPreRunCommands()) diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp index 29855ca50e9e0..a692bb7dfdffc 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp @@ -315,7 +315,8 @@ bool fromJSON(const json::Value &Params, AttachRequestArguments &ARA, O.mapOptional("waitFor", ARA.waitFor) && O.mapOptional("gdb-remote-port", ARA.gdbRemotePort) && O.mapOptional("gdb-remote-hostname", ARA.gdbRemoteHostname) && - O.mapOptional("coreFile", ARA.coreFile); + O.mapOptional("coreFile", ARA.coreFile) && + O.mapOptional("targetIdx", ARA.targetIdx); } bool fromJSON(const json::Value &Params, ContinueArguments &CA, json::Path P) { diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index c45ee10e77d1c..8bf973e852534 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -345,6 +345,9 @@ struct AttachRequestArguments { /// Path to the core file to debug. std::string coreFile; + /// Index of an existing target to attach to. + uint32_t targetIdx = UINT32_MAX; + /// @} }; bool fromJSON(const llvm::json::Value &, AttachRequestArguments &, diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json index d677a81cc7974..f3b35978adf0f 100644 --- a/lldb/tools/lldb-dap/package.json +++ b/lldb/tools/lldb-dap/package.json @@ -678,6 +678,10 @@ "description": "Custom commands that are executed instead of attaching to a process ID or to a process by name. These commands may optionally create a new target and must perform an attach. A valid process must exist after these commands complete or the \"attach\" will fail.", "default": [] }, + "targetIdx": { + "type": "number", + "description": "The index of an existing target to attach to. Used only for child/GPU process debugging." + }, "initCommands": { "type": "array", "items": { From dfefa98683ccd1f6fb9cb4cbd620e08f8506edff Mon Sep 17 00:00:00 2001 From: qxy11 Date: Thu, 14 Aug 2025 11:02:58 -0700 Subject: [PATCH 03/20] Move debugger construction to Launch/AttachRequest --- lldb/tools/lldb-dap/DAP.cpp | 139 ++++++++++++++---- lldb/tools/lldb-dap/DAP.h | 15 ++ .../lldb-dap/Handler/AttachRequestHandler.cpp | 6 + .../Handler/InitializeRequestHandler.cpp | 62 +------- .../lldb-dap/Handler/LaunchRequestHandler.cpp | 5 + .../Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp | 2 +- 6 files changed, 140 insertions(+), 89 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 7fc02072c39ab..0f1827565aa0e 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" @@ -86,14 +87,14 @@ const char DEV_NULL[] = "/dev/null"; namespace lldb_dap { static DAPSessionManager *instance = nullptr; -DAPSessionManager& DAPSessionManager::GetInstance() { +DAPSessionManager &DAPSessionManager::GetInstance() { if (!instance) { instance = new DAPSessionManager(); } return *instance; } -void DAPSessionManager::RegisterSession(lldb::IOObjectSP io, DAP* dap) { +void DAPSessionManager::RegisterSession(lldb::IOObjectSP io, DAP *dap) { std::lock_guard lock(sessions_mutex_); active_sessions_[io] = dap; } @@ -101,19 +102,19 @@ void DAPSessionManager::RegisterSession(lldb::IOObjectSP io, DAP* dap) { void DAPSessionManager::UnregisterSession(lldb::IOObjectSP io) { std::unique_lock lock(sessions_mutex_); active_sessions_.erase(io); - + // Clean up shared resources when the last session exits if (active_sessions_.empty()) { CleanupSharedResources(); } - + std::notify_all_at_thread_exit(sessions_condition_, std::move(lock)); } -std::vector DAPSessionManager::GetActiveSessions() { +std::vector DAPSessionManager::GetActiveSessions() { std::lock_guard lock(sessions_mutex_); - std::vector sessions; - for (const auto& [io, dap] : active_sessions_) { + std::vector sessions; + for (const auto &[io, dap] : active_sessions_) { if (dap) { sessions.push_back(dap); } @@ -123,7 +124,7 @@ std::vector DAPSessionManager::GetActiveSessions() { void DAPSessionManager::DisconnectAllSessions() { std::lock_guard lock(sessions_mutex_); - for (const auto& [io, dap] : active_sessions_) { + for (const auto &[io, dap] : active_sessions_) { if (dap) { if (llvm::Error error = dap->Disconnect()) { llvm::errs() << "DAP client " << dap->transport.GetClientName() @@ -139,19 +140,22 @@ void DAPSessionManager::WaitForAllSessionsToDisconnect() { sessions_condition_.wait(lock, [this] { return active_sessions_.empty(); }); } -std::shared_ptr DAPSessionManager::GetEventThreadForDebugger(lldb::SBDebugger debugger, DAP* requesting_dap) { +std::shared_ptr +DAPSessionManager::GetEventThreadForDebugger(lldb::SBDebugger debugger, + DAP *requesting_dap) { lldb::user_id_t debugger_id = debugger.GetID(); - + std::lock_guard lock(sessions_mutex_); - + // Check if we already have a thread (most common case) auto it = debugger_event_threads_.find(debugger_id); if (it != debugger_event_threads_.end() && it->second) { return it->second; } - + // Create new thread and store it - auto new_thread = std::make_shared(&DAP::EventThread, requesting_dap); + auto new_thread = + std::make_shared(&DAP::EventThread, requesting_dap); debugger_event_threads_[debugger_id] = new_thread; return new_thread; } @@ -166,10 +170,10 @@ std::optional DAPSessionManager::GetSharedDebugger() { return shared_debugger_; } -DAP* DAPSessionManager::FindDAPForTarget(lldb::SBTarget target) { +DAP *DAPSessionManager::FindDAPForTarget(lldb::SBTarget target) { std::lock_guard lock(sessions_mutex_); - - for (const auto& [io, dap] : active_sessions_) { + + for (const auto &[io, dap] : active_sessions_) { if (dap && dap->target.IsValid() && dap->target == target) { return dap; } @@ -339,13 +343,14 @@ llvm::Error DAP::ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr) { void DAP::StopEventHandlers() { // Check if this is the last reference to the shared event thread - if (event_thread_sp && event_thread_sp.use_count() == 1 && event_thread_sp->joinable()) { + if (event_thread_sp && event_thread_sp.use_count() == 1 && + event_thread_sp->joinable()) { // Signal the shared event thread to stop broadcaster.BroadcastEventByType(eBroadcastBitStopEventThread); - + event_thread_sp->join(); } - + // Still handle the progress thread normally since it's per-DAP instance if (progress_event_thread.joinable()) { broadcaster.BroadcastEventByType(eBroadcastBitStopProgressThread); @@ -891,7 +896,7 @@ void DAP::SetTarget(const lldb::SBTarget target) { lldb::SBTarget::eBroadcastBitModulesLoaded | lldb::SBTarget::eBroadcastBitModulesUnloaded | lldb::SBTarget::eBroadcastBitSymbolsLoaded | - lldb::SBTarget::eBroadcastBitSymbolsChanged | + lldb::SBTarget::eBroadcastBitSymbolsChanged | lldb::SBTarget::eBroadcastBitNewTargetSpawned); listener.StartListeningForEvents(this->broadcaster, eBroadcastBitStopEventThread); @@ -1323,6 +1328,80 @@ void DAP::StartProgressEventThread() { progress_event_thread = std::thread(&DAP::ProgressEventThread, this); } + +llvm::Error DAP::StartEventThreads() { + if (clientFeatures.contains(eClientFeatureProgressReporting)) + StartProgressEventThread(); + + StartEventThread(); + + return llvm::Error::success(); +} + +llvm::Error DAP::InitializeDebugger(bool use_shared_debugger) { + // Initialize debugger instance (shared or individual) + if (use_shared_debugger) { + auto shared_debugger = DAPSessionManager::GetInstance().GetSharedDebugger(); + if (!shared_debugger) { + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "unable to get shared debugger"); + } + debugger = shared_debugger.value(); + return StartEventThreads(); + } + + debugger = lldb::SBDebugger::Create(false); + + // Configure input/output/error file descriptors + debugger.SetInputFile(in); + target = debugger.GetDummyTarget(); + + llvm::Expected out_fd = out.GetWriteFileDescriptor(); + if (!out_fd) + return out_fd.takeError(); + debugger.SetOutputFile(lldb::SBFile(*out_fd, "w", false)); + + llvm::Expected 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. + if (sourceInitFile) { + 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."); + + + return StartEventThreads(); +} + void DAP::ProgressEventThread() { lldb::SBListener listener("lldb-dap.progress.listener"); debugger.GetBroadcaster().AddListener( @@ -1383,7 +1462,6 @@ void DAP::ProgressEventThread() { } } - // All events from the debugger, target, process, thread and frames are // received in this function that runs in its own thread. We are using a // "FILE *" to output packets back to VS Code and they have mutexes in them @@ -1403,9 +1481,11 @@ void DAP::EventThread() { 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::GetInstance().FindDAPForTarget(process.GetTarget()); - if (!dap_instance) continue; - + DAP *dap_instance = DAPSessionManager::GetInstance().FindDAPForTarget( + process.GetTarget()); + if (!dap_instance) + continue; + if (event_mask & lldb::SBProcess::eBroadcastBitStateChanged) { auto state = lldb::SBProcess::GetStateFromEvent(event); switch (state) { @@ -1464,10 +1544,12 @@ void DAP::EventThread() { event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded || event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded || event_mask & lldb::SBTarget::eBroadcastBitSymbolsChanged) { - lldb::SBTarget event_target = lldb::SBTarget::GetTargetFromEvent(event); + lldb::SBTarget event_target = + lldb::SBTarget::GetTargetFromEvent(event); // Find the DAP instance that owns this target - DAP* dap_instance = DAPSessionManager::GetInstance().FindDAPForTarget(event_target); - if (!dap_instance) + DAP *dap_instance = + DAPSessionManager::GetInstance().FindDAPForTarget(event_target); + if (!dap_instance) continue; const uint32_t num_modules = @@ -1487,7 +1569,8 @@ void DAP::EventThread() { llvm::StringRef module_id = p_module->id; - const bool module_exists = dap_instance->modules.contains(module_id); + const bool module_exists = + dap_instance->modules.contains(module_id); if (remove_module && module_exists) { dap_instance->modules.erase(module_id); dap_instance->Send(protocol::Event{ diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index fcf46095f38ec..e9305f422ad61 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -213,6 +213,10 @@ struct DAP { /// The set of features supported by the connected client. llvm::DenseSet clientFeatures; + /// Used by the test suite to prevent sourcing `.lldbinit` and changing its + /// behavior. + bool sourceInitFile = true; + /// The initial thread list upon attaching. std::vector initial_thread_list; @@ -464,6 +468,17 @@ struct DAP { void StartEventThread(); void StartProgressEventThread(); + /// DAP debugger initialization functions + /// @{ + + /// Perform complete DAP initialization in one call + llvm::Error InitializeDebugger(bool use_shared_debugger); + + /// Start event handling threads based on client capabilities + llvm::Error StartEventThreads(); + + /// @} + /// Sets the given protocol `breakpoints` in the given `source`, while /// removing any existing breakpoints in the given source if they are not in /// `breakpoint`. diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp index 0096842c258b6..b4241911d7f7d 100644 --- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp @@ -29,6 +29,12 @@ namespace lldb_dap { /// Since attaching is debugger/runtime specific, the arguments for this request /// are not part of this specification. Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { + // Initialize DAP debugger and related components if not sharing previously launched debugger. + bool use_shared_debugger = args.targetIdx != UINT32_MAX; + if (Error err = dap.InitializeDebugger(use_shared_debugger)) { + return err; + } + // Validate that we have a well formed attach request. if (args.attachCommands.empty() && args.coreFile.empty() && args.configuration.program.empty() && diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp index 60d0542273486..e4f8f31cb7962 100644 --- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp @@ -21,67 +21,9 @@ using namespace lldb_dap::protocol; /// Initialize request; value of command field is 'initialize'. llvm::Expected InitializeRequestHandler::Run( const InitializeRequestArguments &arguments) const { + // Store initialization arguments for later use in Launch/Attach dap.clientFeatures = arguments.supportedFeatures; - - // Check if we already have a shared debugger (eg. for GPU processes), otherwise create individual debugger - auto shared_debugger = DAPSessionManager::GetInstance().GetSharedDebugger(); - if (shared_debugger.has_value()) { - dap.debugger = shared_debugger.value(); - } else { - // Create individual debugger for this DAP instance - dap.debugger = lldb::SBDebugger::Create(false); - } - - dap.debugger.SetInputFile(dap.in); - dap.target = dap.debugger.GetDummyTarget(); - - llvm::Expected out_fd = dap.out.GetWriteFileDescriptor(); - if (!out_fd) - return out_fd.takeError(); - dap.debugger.SetOutputFile(lldb::SBFile(*out_fd, "w", false)); - - llvm::Expected err_fd = dap.err.GetWriteFileDescriptor(); - if (!err_fd) - return err_fd.takeError(); - dap.debugger.SetErrorFile(lldb::SBFile(*err_fd, "w", false)); - - auto interp = dap.debugger.GetCommandInterpreter(); - - // 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. - if (arguments.lldbExtSourceInitFile.value_or(true)) { - dap.debugger.SkipLLDBInitFiles(false); - dap.debugger.SkipAppInitFiles(false); - lldb::SBCommandReturnObject init; - interp.SourceInitFileInGlobalDirectory(init); - interp.SourceInitFileInHomeDirectory(init); - } - - if (llvm::Error err = dap.RunPreInitCommands()) - return err; - - auto cmd = dap.debugger.GetCommandInterpreter().AddMultiwordCommand( - "lldb-dap", "Commands for managing lldb-dap."); - if (arguments.supportedFeatures.contains( - eClientFeatureStartDebuggingRequest)) { - cmd.AddCommand( - "start-debugging", new StartDebuggingCommand(dap), - "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(dap), - "Get or set the repl behavior of lldb-dap evaluation requests."); - cmd.AddCommand("send-event", new SendEventCommand(dap), - "Sends an DAP event to the client."); - - if (arguments.supportedFeatures.contains(eClientFeatureProgressReporting)) - dap.StartProgressEventThread(); - - // Start our event thread so we can receive events from the debugger, target, - // process and more. - dap.StartEventThread(); + dap.sourceInitFile = arguments.lldbExtSourceInitFile.value_or(true); return dap.GetCapabilities(); } diff --git a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp index 553cbeaf849e2..322308068f82c 100644 --- a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp @@ -22,6 +22,11 @@ namespace lldb_dap { /// Launch request; value of command field is 'launch'. Error LaunchRequestHandler::Run(const LaunchRequestArguments &arguments) const { + // Initialize DAP debugger + if (Error err = dap.InitializeDebugger(false /* use_shared_debugger */)) { + return err; + } + // Validate that we have a well formed launch request. if (!arguments.launchCommands.empty() && arguments.console != protocol::eConsoleInternal) diff --git a/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp b/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp index 824fb615967c7..dd62f13658e9c 100644 --- a/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp +++ b/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp @@ -71,7 +71,7 @@ static const char *kGpuLoaderBreakpointIdentifier = "GPU loader breakpoint"; // on the issues and alternatives: // https://github.com/clayborg/llvm-project/pull/20 static const char *kSetDbgApiBreakpointByName = - "_ZN4rocr19_loader_debug_stateEv"; // rocr::_loader_debug_state + nullptr; // rocr::_loader_debug_state static amd_dbgapi_status_t amd_dbgapi_insert_breakpoint_callback( amd_dbgapi_client_process_id_t client_process_id, From bf3fb54f7dcdeb1bbaf8c97d05f603e7faf80297 Mon Sep 17 00:00:00 2001 From: qxy11 Date: Thu, 14 Aug 2025 11:02:58 -0700 Subject: [PATCH 04/20] Add comment, move static variable --- lldb/tools/lldb-dap/DAP.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 0f1827565aa0e..87d2c852de7ac 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -86,8 +86,9 @@ const char DEV_NULL[] = "/dev/null"; namespace lldb_dap { -static DAPSessionManager *instance = nullptr; DAPSessionManager &DAPSessionManager::GetInstance() { + // NOTE: intentional leak to avoid issues with C++ destructor chain + static DAPSessionManager *instance = nullptr; if (!instance) { instance = new DAPSessionManager(); } @@ -340,7 +341,6 @@ llvm::Error DAP::ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr) { return llvm::Error::success(); } - void DAP::StopEventHandlers() { // Check if this is the last reference to the shared event thread if (event_thread_sp && event_thread_sp.use_count() == 1 && @@ -1321,14 +1321,14 @@ protocol::Capabilities DAP::GetCapabilities() { void DAP::StartEventThread() { // Get event thread for this debugger (creates it if it doesn't exist) - event_thread_sp = DAPSessionManager::GetInstance().GetEventThreadForDebugger(debugger, this); + event_thread_sp = DAPSessionManager::GetInstance().GetEventThreadForDebugger( + debugger, this); } void DAP::StartProgressEventThread() { progress_event_thread = std::thread(&DAP::ProgressEventThread, this); } - llvm::Error DAP::StartEventThreads() { if (clientFeatures.contains(eClientFeatureProgressReporting)) StartProgressEventThread(); @@ -1398,7 +1398,6 @@ llvm::Error DAP::InitializeDebugger(bool use_shared_debugger) { cmd.AddCommand("send-event", new SendEventCommand(*this), "Sends an DAP event to the client."); - return StartEventThreads(); } @@ -1594,6 +1593,9 @@ void DAP::EventThread() { // Set the shared debugger for GPU processes DAPSessionManager::GetInstance().SetSharedDebugger(debugger); + // We create "attachCommands" that will select the target that already + // exists in LLDB. The DAP instance will attach to this already + // existing target and the debug session will be ready to go. llvm::json::Object attach_config; llvm::json::Array attach_commands; From 2a3b3b0235520b24cac8704c2ccdf21f1edbafee Mon Sep 17 00:00:00 2001 From: qxy11 Date: Thu, 14 Aug 2025 11:02:58 -0700 Subject: [PATCH 05/20] Undo temp fix for LLDBServerPluginAMDGPU --- .../tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp b/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp index dd62f13658e9c..824fb615967c7 100644 --- a/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp +++ b/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp @@ -71,7 +71,7 @@ static const char *kGpuLoaderBreakpointIdentifier = "GPU loader breakpoint"; // on the issues and alternatives: // https://github.com/clayborg/llvm-project/pull/20 static const char *kSetDbgApiBreakpointByName = - nullptr; // rocr::_loader_debug_state + "_ZN4rocr19_loader_debug_stateEv"; // rocr::_loader_debug_state static amd_dbgapi_status_t amd_dbgapi_insert_breakpoint_callback( amd_dbgapi_client_process_id_t client_process_id, From 58c7548abd50f997245089a7957d74caf6f8f9c7 Mon Sep 17 00:00:00 2001 From: qxy11 Date: Thu, 14 Aug 2025 11:02:58 -0700 Subject: [PATCH 06/20] Add lldb-dap unit tests for gpu --- .../test/tools/lldb-dap/dap_server.py | 84 +++++++++ .../test/tools/lldb-dap/lldbdap_testcase.py | 167 +++++++++++++----- lldb/test/API/tools/lldb-dap/gpu/Makefile | 3 + .../gpu/TestDAP_gpu_reverse_request.py | 113 ++++++++++++ .../API/tools/lldb-dap/gpu/hello_world.hip | 50 ++++++ 5 files changed, 369 insertions(+), 48 deletions(-) create mode 100644 lldb/test/API/tools/lldb-dap/gpu/Makefile create mode 100644 lldb/test/API/tools/lldb-dap/gpu/TestDAP_gpu_reverse_request.py create mode 100644 lldb/test/API/tools/lldb-dap/gpu/hello_world.hip 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 0b09893c7ed5b..8a17ed69dbd31 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 @@ -315,8 +315,28 @@ def _handle_recv_packet(self, packet: Optional[ProtocolMessage]) -> bool: elif packet_type == "response": if packet["command"] == "disconnect": keepGoing = False + elif packet_type == "request": + # This is a reverse request automatically spawned from LLDB (eg. for GPU targets) + command = packet.get("command", "unknown") + self.reverse_requests.append(packet) + if command == "startDebugging": + self._handle_startDebugging_request(packet) + else: + desc = f"unhandled automatic reverse request of type {command}" + raise ValueError(desc) + self._enqueue_recv_packet(packet) return keepGoing + + def _handle_startDebugging_request(self, packet): + response = { + "type": "response", + "request_seq": packet.get("seq", 0), + "success": True, + "command": "startDebugging", + "body": {} + } + self.send_packet(response, set_sequence=True) def _process_continued(self, all_threads_continued: bool): self.frame_scopes = {} @@ -670,6 +690,7 @@ def request_attach( sourceMap: Optional[Union[list[tuple[str, str]], dict[str, str]]] = None, gdbRemotePort: Optional[int] = None, gdbRemoteHostname: Optional[str] = None, + targetIdx: Optional[int] = None, ): args_dict = {} if pid is not None: @@ -703,6 +724,8 @@ def request_attach( args_dict["gdb-remote-port"] = gdbRemotePort if gdbRemoteHostname is not None: args_dict["gdb-remote-hostname"] = gdbRemoteHostname + if targetIdx is not None: + args_dict["targetIdx"] = targetIdx command_dict = {"command": "attach", "type": "request", "arguments": args_dict} return self.send_recv(command_dict) @@ -1333,6 +1356,8 @@ def __init__( ): self.process = None self.connection = None + self.child_dap_sessions: list["DebugAdapterServer"] = [] # Track child sessions for cleanup + if executable is not None: process, connection = DebugAdapterServer.launch( executable=executable, connection=connection, env=env, log_file=log_file @@ -1414,6 +1439,65 @@ def get_pid(self) -> int: if self.process: return self.process.pid return -1 + + def get_child_sessions(self) -> list["DebugAdapterServer"]: + return self.child_dap_sessions + + def _handle_startDebugging_request(self, packet): + """Launch a new DebugAdapterServer with attach config parameters from the packet""" + try: + # Extract arguments from the packet + arguments = packet.get('arguments', {}) + request_type = arguments.get('request', 'attach') # 'attach' or 'launch' + configuration = arguments.get('configuration', {}) + + # Create a new DAP session that launches its own lldb-dap process + child_dap = DebugAdapterServer( + connection=self.connection, + log_file=self.log_file + ) + + # Track the child session for proper cleanup + self.child_dap_sessions.append(child_dap) + + # Initialize the child DAP session + child_dap.request_initialize() + + # Configure the child session based on the request type and configuration + if request_type == 'attach': + # Extract attach-specific parameters + attach_commands = configuration.get('attachCommands', []) + target_idx = configuration.get('targetIdx', None) + + # Send attach request to the child DAP + child_dap.request_attach( + attachCommands=attach_commands, + targetIdx=target_idx, + ) + else: + raise ValueError(f"Unsupported startDebugging request type: {request_type}") + + # Send success response + response = { + "type": "response", + "request_seq": packet.get("seq", 0), + "success": True, + "command": "startDebugging", + "body": {} + } + + except Exception as e: + # Send error response + response = { + "type": "response", + "request_seq": packet.get("seq", 0), + "success": False, + "command": "startDebugging", + "message": f"Failed to start debugging: {str(e)}", + "body": {} + } + + self.send_packet(response, set_sequence=True) def terminate(self): try: diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py index 1567462839748..855a09ffa3320 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py @@ -35,6 +35,119 @@ def create_debug_adapter( env=lldbDAPEnv, ) + def _get_dap_server(self, child_session_index: Optional[int] = None) -> dap_server.DebugAdapterServer: + """Get a specific DAP server instance. + + Args: + child_session_index: Index of child session, or None for main session + + Returns: + The requested DAP server instance + """ + if child_session_index is None: + return self.dap_server + else: + child_sessions = self.dap_server.get_child_sessions() + if child_session_index >= len(child_sessions): + raise IndexError(f"Child session index {child_session_index} out of range. Found {len(child_sessions)} child sessions.") + return child_sessions[child_session_index] + + def _set_source_breakpoints_impl(self, dap_server_instance, source_path, lines, data=None, wait_for_resolve=True): + """Implementation for setting source breakpoints on any DAP server""" + response = dap_server_instance.request_setBreakpoints(Source(source_path), lines, data) + if response is None or not response["success"]: + return [] + breakpoints = response["body"]["breakpoints"] + breakpoint_ids = [] + for breakpoint in breakpoints: + breakpoint_ids.append("%i" % (breakpoint["id"])) + if wait_for_resolve: + self._wait_for_breakpoints_to_resolve_impl(dap_server_instance, breakpoint_ids) + return breakpoint_ids + + def _wait_for_breakpoints_to_resolve_impl(self, dap_server_instance, breakpoint_ids, timeout=None): + """Implementation for waiting for breakpoints to resolve on any DAP server""" + if timeout is None: + timeout = self.DEFAULT_TIMEOUT + unresolved_breakpoints = dap_server_instance.wait_for_breakpoints_to_be_verified(breakpoint_ids, timeout) + self.assertEqual( + len(unresolved_breakpoints), + 0, + f"Expected to resolve all breakpoints. Unresolved breakpoint ids: {unresolved_breakpoints}", + ) + + def _verify_breakpoint_hit_impl(self, dap_server_instance, breakpoint_ids, timeout=None): + """Implementation for verifying breakpoint hit on any DAP server""" + if timeout is None: + timeout = self.DEFAULT_TIMEOUT + stopped_events = dap_server_instance.wait_for_stopped(timeout) + for stopped_event in stopped_events: + if "body" in stopped_event: + body = stopped_event["body"] + if "reason" not in body: + continue + if ( + body["reason"] != "breakpoint" + and body["reason"] != "instruction breakpoint" + ): + continue + if "description" not in body: + continue + # Descriptions for breakpoints will be in the form + # "breakpoint 1.1", so look for any description that matches + # ("breakpoint 1.") in the description field as verification + # that one of the breakpoint locations was hit. DAP doesn't + # allow breakpoints to have multiple locations, but LLDB does. + # So when looking at the description we just want to make sure + # the right breakpoint matches and not worry about the actual + # location. + description = body["description"] + for breakpoint_id in breakpoint_ids: + match_desc = f"breakpoint {breakpoint_id}." + if match_desc in description: + return + self.assertTrue(False, f"breakpoint not hit, stopped_events={stopped_events}") + + def _do_continue_impl(self, dap_server_instance): + """Implementation for continuing execution on any DAP server""" + resp = dap_server_instance.request_continue() + self.assertTrue(resp["success"], f"continue request failed: {resp}") + + # Multi-session methods for operating on specific sessions without switching context + def set_source_breakpoints_on(self, child_session_index: Optional[int], source_path, lines, data=None, wait_for_resolve=True): + """Set source breakpoints on a specific DAP session without switching the active session.""" + return self._set_source_breakpoints_impl( + self._get_dap_server(child_session_index), source_path, lines, data, wait_for_resolve + ) + + def verify_breakpoint_hit_on(self, child_session_index: Optional[int], breakpoint_ids: list[str], timeout=DEFAULT_TIMEOUT): + """Verify breakpoint hit on a specific DAP session without switching the active session.""" + return self._verify_breakpoint_hit_impl( + self._get_dap_server(child_session_index), breakpoint_ids, timeout + ) + + def do_continue_on(self, child_session_index: Optional[int]): + """Continue execution on a specific DAP session without switching the active session.""" + return self._do_continue_impl(self._get_dap_server(child_session_index)) + + def start_server(self, connection): + """ + Start an lldb-dap server process listening on the specified connection. + """ + log_file_path = self.getBuildArtifact("dap.txt") + (process, connection) = dap_server.DebugAdapterServer.launch( + executable=self.lldbDAPExec, + connection=connection, + log_file=log_file_path + ) + + def cleanup(): + process.terminate() + + self.addTearDownHook(cleanup) + + return (process, connection) + def build_and_create_debug_adapter( self, lldbDAPEnv: Optional[dict[str, str]] = None, @@ -59,18 +172,9 @@ def set_source_breakpoints( Each object in data is 1:1 mapping with the entry in lines. It contains optional location/hitCondition/logMessage parameters. """ - response = self.dap_server.request_setBreakpoints( - Source(source_path), lines, data + return self._set_source_breakpoints_impl( + self.dap_server, source_path, lines, data, wait_for_resolve ) - if response is None or not response["success"]: - return [] - breakpoints = response["body"]["breakpoints"] - breakpoint_ids = [] - for breakpoint in breakpoints: - breakpoint_ids.append("%i" % (breakpoint["id"])) - if wait_for_resolve: - self.wait_for_breakpoints_to_resolve(breakpoint_ids) - return breakpoint_ids def set_source_breakpoints_assembly( self, source_reference, lines, data=None, wait_for_resolve=True @@ -113,13 +217,8 @@ def set_function_breakpoints( def wait_for_breakpoints_to_resolve( self, breakpoint_ids: list[str], timeout: Optional[float] = DEFAULT_TIMEOUT ): - unresolved_breakpoints = self.dap_server.wait_for_breakpoints_to_be_verified( - breakpoint_ids, timeout - ) - self.assertEqual( - len(unresolved_breakpoints), - 0, - f"Expected to resolve all breakpoints. Unresolved breakpoint ids: {unresolved_breakpoints}", + return self._wait_for_breakpoints_to_resolve_impl( + self.dap_server, breakpoint_ids, timeout ) def waitUntil(self, condition_callback): @@ -145,33 +244,7 @@ def verify_breakpoint_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT): "breakpoint_ids" should be a list of breakpoint ID strings (["1", "2"]). The return value from self.set_source_breakpoints() or self.set_function_breakpoints() can be passed to this function""" - stopped_events = self.dap_server.wait_for_stopped(timeout) - for stopped_event in stopped_events: - if "body" in stopped_event: - body = stopped_event["body"] - if "reason" not in body: - continue - if ( - body["reason"] != "breakpoint" - and body["reason"] != "instruction breakpoint" - ): - continue - if "description" not in body: - continue - # Descriptions for breakpoints will be in the form - # "breakpoint 1.1", so look for any description that matches - # ("breakpoint 1.") in the description field as verification - # that one of the breakpoint locations was hit. DAP doesn't - # allow breakpoints to have multiple locations, but LLDB does. - # So when looking at the description we just want to make sure - # the right breakpoint matches and not worry about the actual - # location. - description = body["description"] - for breakpoint_id in breakpoint_ids: - match_desc = f"breakpoint {breakpoint_id}." - if match_desc in description: - return - self.assertTrue(False, f"breakpoint not hit, stopped_events={stopped_events}") + self._verify_breakpoint_hit_impl(self.dap_server, breakpoint_ids, timeout) def verify_all_breakpoints_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT): """Wait for the process we are debugging to stop, and verify we hit @@ -384,8 +457,7 @@ def stepOut(self, threadId=None, waitForStop=True, timeout=DEFAULT_TIMEOUT): return None def do_continue(self): # `continue` is a keyword. - resp = self.dap_server.request_continue() - self.assertTrue(resp["success"], f"continue request failed: {resp}") + self._do_continue_impl(self.dap_server) def continue_to_next_stop(self, timeout=DEFAULT_TIMEOUT): self.do_continue() @@ -478,7 +550,6 @@ def launch( **kwargs, ): """Sending launch request to dap""" - # Make sure we disconnect and terminate the DAP debug adapter, # if we throw an exception during the test case def cleanup(): diff --git a/lldb/test/API/tools/lldb-dap/gpu/Makefile b/lldb/test/API/tools/lldb-dap/gpu/Makefile new file mode 100644 index 0000000000000..c3dbfba929f3e --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/gpu/Makefile @@ -0,0 +1,3 @@ +HIP_SOURCES := hello_world.hip + +include Makefile.rules diff --git a/lldb/test/API/tools/lldb-dap/gpu/TestDAP_gpu_reverse_request.py b/lldb/test/API/tools/lldb-dap/gpu/TestDAP_gpu_reverse_request.py new file mode 100644 index 0000000000000..58f3d47a94f36 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/gpu/TestDAP_gpu_reverse_request.py @@ -0,0 +1,113 @@ +""" +Test DAP reverse request functionality for GPU debugging. +Tests the changes that allow creating new DAP targets through reverse requests, +specifically for GPU debugging scenarios using AMD HIP. +""" + +import dap_server +import lldbdap_testcase +from subprocess import Popen, PIPE +from lldbsuite.test.lldbtest import * +from lldbsuite.test.decorators import * + +def _detect_rocm(): + """Detects rocm target.""" + try: + proc = Popen(["rocminfo"], stdout=PIPE, stderr=PIPE) + return "amd" + except Exception: + return None + +def skipUnlessHasROCm(func): + """Decorate the item to skip test unless ROCm is available.""" + + def has_rocm(): + if _detect_rocm() is None: + return "ROCm not available (rocm-smi not found)" + return None + + return skipTestIfFn(has_rocm)(func) + + +class TestDAP_gpu_reverse_request(lldbdap_testcase.DAPTestCaseBase): + """Test DAP session spawning - both basic and GPU scenarios""" + + def setUp(self): + super().setUp() + + @skipUnlessHasROCm + def test_automatic_reverse_request_detection(self): + """Test that we can detect when LLDB automatically sends reverse requests""" + program = self.getBuildArtifact("a.out") + + # Build and launch with settings that might trigger reverse requests + self.build_and_launch( + program + ) + source = "hello_world.hip" + breakpoint_line = line_number(source, "// CPU BREAKPOINT - BEFORE LAUNCH") + self.set_source_breakpoints(source, [breakpoint_line]) + self.continue_to_next_stop() + + reverse_request_count = len(self.dap_server.reverse_requests) + self.assertEqual(reverse_request_count, 1, "Should have received one reverse request") + # If reverse requests were found, validate them + req = self.dap_server.reverse_requests[0] + self.assertIn("command", req, "Reverse request should have command") + self.assertEqual(req["command"], "startDebugging") + + self.assertIn("arguments", req, "Reverse request should have arguments") + self.assertIn("configuration", req["arguments"], "Reverse request should have configuration") + + attach_config = req["arguments"]["configuration"] + self.assertIn("attachCommands", attach_config, "Reverse request should have attachCommands") + self.assertIn("target select 1", attach_config["attachCommands"]) + self.assertIn("name", attach_config, "Attach config should have name") + self.assertIn("GPU Session", attach_config["name"]) + self.assertIn("targetIdx", attach_config, "Attach config should have targetIdx") + self.assertEqual(attach_config["targetIdx"], 1, "Attach config should have targetIdx 1") + + @skipUnlessHasROCm + def test_gpu_breakpoint_hit(self): + """ + Test that we can hit a breakpoint in GPU debugging session spawned through reverse requests. + """ + GPU_SESSION_IDX = 0 + self.build() + log_file_path = self.getBuildArtifact("dap.txt") + # Enable detailed DAP logging to debug any issues + program = self.getBuildArtifact("a.out") + source = "hello_world.hip" + cpu_breakpoint_line = line_number(source, "// CPU BREAKPOINT") + gpu_breakpoint_line = line_number(source, "// GPU BREAKPOINT") + # Launch DAP server + _, connection = self.start_server(connection="listen://localhost:0") + + self.dap_server = dap_server.DebugAdapterServer( + connection=connection, + log_file=log_file_path + ) + self.launch( + program, disconnectAutomatically=False, + ) + # Set CPU breakpoint and stop. + breakpoint_ids = self.set_source_breakpoints(source, [cpu_breakpoint_line]) + + self.continue_to_breakpoints(breakpoint_ids, timeout=self.DEFAULT_TIMEOUT) + # We should have a GPU child session automatically spawned now + self.assertEqual(len(self.dap_server.get_child_sessions()), 1) + # Set breakpoint in GPU session + gpu_breakpoint_ids = self.set_source_breakpoints_on(GPU_SESSION_IDX, source, [gpu_breakpoint_line]) + + # Resume GPU execution after verifying breakpoint hit + self.do_continue_on(0) + + # Continue main session + self.do_continue() + # Verify that the GPU breakpoint is hit in the child session + self.verify_breakpoint_hit_on(GPU_SESSION_IDX, gpu_breakpoint_ids, timeout=self.DEFAULT_TIMEOUT * 2) + + # Manually disconnect sessions + for child_session in self.dap_server.get_child_sessions(): + child_session.request_disconnect() + self.dap_server.request_disconnect() diff --git a/lldb/test/API/tools/lldb-dap/gpu/hello_world.hip b/lldb/test/API/tools/lldb-dap/gpu/hello_world.hip new file mode 100644 index 0000000000000..33ab31e1ca574 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/gpu/hello_world.hip @@ -0,0 +1,50 @@ +#include +#include +#include + +/// \brief Checks if the provided error code is \p hipSuccess and if not, +/// prints an error message to the standard error output and terminates the +/// program with an error code. +constexpr int error_exit_code = -1; +#define HIP_CHECK(condition) \ + { \ + const hipError_t error = condition; \ + if (error != hipSuccess) { \ + fprintf(stderr, "HIP error: \"%s\" at %s:%d\n", \ + hipGetErrorString(error), __FILE__, __LINE__); \ + exit(error_exit_code); \ + } \ + } + +__global__ void add_one(int *data) { + int idx = threadIdx.x; + data[idx] = idx + 1; // GPU BREAKPOINT +} + +int main() { + const int n = 4; + int host_data[n] = {0, 0, 0, 0}; // Initialize to zeros + int *device_data; + + printf("Starting GPU test...\n"); // CPU BREAKPOINT - BEFORE LAUNCH + + // Allocate device memory + HIP_CHECK(hipMalloc(&device_data, n * sizeof(int))); + + // Copy data to device + HIP_CHECK(hipMemcpy(device_data, host_data, n * sizeof(int), hipMemcpyHostToDevice)); + + // Launch kernel on default stream (single block, 4 threads) + add_one<<<1, n>>>(device_data); + + HIP_CHECK(hipDeviceSynchronize()); + + HIP_CHECK(hipMemcpy(host_data, device_data, n * sizeof(int), hipMemcpyDeviceToHost)); + + printf("Results: %d %d %d %d\n", host_data[0], host_data[1], host_data[2], host_data[3]); // CPU BREAKPOINT - AFTER LAUNCH + + // Cleanup + HIP_CHECK(hipFree(device_data)); + + return 0; +} From 770558bc964f67bdbd0ea1c67ffbb8808e74d359 Mon Sep 17 00:00:00 2001 From: qxy11 Date: Thu, 14 Aug 2025 11:05:25 -0700 Subject: [PATCH 07/20] Format --- .../Process/gdb-remote/ProcessGDBRemote.cpp | 6 +-- lldb/tools/lldb-dap/DAP.cpp | 38 +++++++++++-------- lldb/tools/lldb-dap/DAP.h | 30 ++++++++------- .../lldb-dap/Handler/AttachRequestHandler.cpp | 3 +- .../lldb-dap/Handler/LaunchRequestHandler.cpp | 2 +- 5 files changed, 45 insertions(+), 34 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 55757dfa9b665..b44c4058beb29 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -1025,9 +1025,9 @@ Status ProcessGDBRemote::HandleConnectionRequest(const GPUActions &gpu_action) { process_sp->GetTarget().shared_from_this()); LLDB_LOG(log, "ProcessGDBRemote::HandleConnectionRequest(): successfully " "created process!!!"); - auto event_sp = std::make_shared( - Target::eBroadcastBitNewTargetSpawned, - new Target::TargetEventData(gpu_target_sp)); + auto event_sp = + std::make_shared(Target::eBroadcastBitNewTargetSpawned, + new Target::TargetEventData(gpu_target_sp)); GetTarget().BroadcastEvent(event_sp); return Status(); } diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 87d2c852de7ac..88e62b0efaa86 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -1624,15 +1624,19 @@ void DAP::EventThread() { "startDebugging", std::move(start_debugging_args)); } } else if (lldb::SBBreakpoint::EventIsBreakpointEvent(event)) { - lldb::SBBreakpoint bp = lldb::SBBreakpoint::GetBreakpointFromEvent(event); - if (!bp.IsValid()) continue; - + lldb::SBBreakpoint bp = + lldb::SBBreakpoint::GetBreakpointFromEvent(event); + if (!bp.IsValid()) + continue; + lldb::SBTarget event_target = bp.GetTarget(); - + // Find the DAP instance that owns this target - DAP* dap_instance = DAPSessionManager::GetInstance().FindDAPForTarget(event_target); - if (!dap_instance) continue; - + DAP *dap_instance = + DAPSessionManager::GetInstance().FindDAPForTarget(event_target); + if (!dap_instance) + continue; + if (event_mask & lldb::SBTarget::eBroadcastBitBreakpointChanged) { auto event_type = lldb::SBBreakpoint::GetBreakpointEventTypeFromEvent(event); @@ -1666,18 +1670,22 @@ void DAP::EventThread() { } else if (event_mask & lldb::eBroadcastBitError || event_mask & lldb::eBroadcastBitWarning) { // Global debugger events - send to all DAP instances - std::vector active_instances = DAPSessionManager::GetInstance().GetActiveSessions(); - for (DAP* dap_instance : active_instances) { - if (!dap_instance) continue; - + std::vector active_instances = + DAPSessionManager::GetInstance().GetActiveSessions(); + for (DAP *dap_instance : active_instances) { + if (!dap_instance) + continue; + lldb::SBStructuredData data = lldb::SBDebugger::GetDiagnosticFromEvent(event); - if (!data.IsValid()) continue; - + if (!data.IsValid()) + continue; + std::string type = GetStringValue(data.GetValueForKey("type")); std::string message = GetStringValue(data.GetValueForKey("message")); - dap_instance->SendOutput(OutputType::Important, - llvm::formatv("{0}: {1}", type, message).str()); + dap_instance->SendOutput( + OutputType::Important, + llvm::formatv("{0}: {1}", type, message).str()); } } else if (event.BroadcasterMatchesRef(broadcaster)) { if (event_mask & eBroadcastBitStopEventThread) { diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index e9305f422ad61..bada7ededd2be 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -69,16 +69,16 @@ using ClientFeature = protocol::ClientFeature; class DAPSessionManager { public: /// Get the singleton instance of the DAP session manager - static DAPSessionManager& GetInstance(); + static DAPSessionManager &GetInstance(); /// Register a DAP session - void RegisterSession(lldb::IOObjectSP io, DAP* dap); + void RegisterSession(lldb::IOObjectSP io, DAP *dap); /// Unregister a DAP session void UnregisterSession(lldb::IOObjectSP io); /// Get all active DAP sessions - std::vector GetActiveSessions(); + std::vector GetActiveSessions(); /// Disconnect all active sessions void DisconnectAllSessions(); @@ -93,10 +93,11 @@ class DAPSessionManager { std::optional GetSharedDebugger(); /// Get or create event thread for a specific debugger - std::shared_ptr GetEventThreadForDebugger(lldb::SBDebugger debugger, DAP* requesting_dap); + std::shared_ptr + GetEventThreadForDebugger(lldb::SBDebugger debugger, DAP *requesting_dap); /// Find the DAP instance that owns the given target - DAP* FindDAPForTarget(lldb::SBTarget target); + DAP *FindDAPForTarget(lldb::SBTarget target); /// Clean up shared resources when the last session exits void CleanupSharedResources(); @@ -106,22 +107,23 @@ class DAPSessionManager { ~DAPSessionManager() = default; // Non-copyable and non-movable - DAPSessionManager(const DAPSessionManager&) = delete; - DAPSessionManager& operator=(const DAPSessionManager&) = delete; - DAPSessionManager(DAPSessionManager&&) = delete; - DAPSessionManager& operator=(DAPSessionManager&&) = delete; + DAPSessionManager(const DAPSessionManager &) = delete; + DAPSessionManager &operator=(const DAPSessionManager &) = delete; + DAPSessionManager(DAPSessionManager &&) = delete; + DAPSessionManager &operator=(DAPSessionManager &&) = delete; std::mutex sessions_mutex_; std::condition_variable sessions_condition_; - std::map active_sessions_; - + std::map active_sessions_; + /// Optional shared debugger instance set when the native process /// spawns a new GPU target std::optional shared_debugger_; - + /// Map from debugger ID to its event thread used for when /// multiple DAP sessions are using the same debugger instance. - std::map> debugger_event_threads_; + std::map> + debugger_event_threads_; }; enum class OutputType { Console, Important, Stdout, Stderr, Telemetry }; @@ -138,7 +140,7 @@ enum class ReplMode { Variable = 0, Command, Auto }; struct DAP { friend class DAPSessionManager; - + /// Path to the lldb-dap binary itself. static llvm::StringRef debug_adapter_path; diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp index b4241911d7f7d..274a2cf21dc49 100644 --- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp @@ -29,7 +29,8 @@ namespace lldb_dap { /// Since attaching is debugger/runtime specific, the arguments for this request /// are not part of this specification. Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { - // Initialize DAP debugger and related components if not sharing previously launched debugger. + // Initialize DAP debugger and related components if not sharing previously + // launched debugger. bool use_shared_debugger = args.targetIdx != UINT32_MAX; if (Error err = dap.InitializeDebugger(use_shared_debugger)) { return err; diff --git a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp index 322308068f82c..89e1123b18802 100644 --- a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp @@ -22,7 +22,7 @@ namespace lldb_dap { /// Launch request; value of command field is 'launch'. Error LaunchRequestHandler::Run(const LaunchRequestArguments &arguments) const { - // Initialize DAP debugger + // Initialize DAP debugger if (Error err = dap.InitializeDebugger(false /* use_shared_debugger */)) { return err; } From 32f791d5037c567a659b6547d89ae130abeee209 Mon Sep 17 00:00:00 2001 From: qxy11 Date: Thu, 14 Aug 2025 11:42:34 -0700 Subject: [PATCH 08/20] Fix the hip file compilation --- .../API/tools/lldb-dap/gpu/hello_world.hip | 25 ++++++------------- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/gpu/hello_world.hip b/lldb/test/API/tools/lldb-dap/gpu/hello_world.hip index 33ab31e1ca574..2c752819dbb94 100644 --- a/lldb/test/API/tools/lldb-dap/gpu/hello_world.hip +++ b/lldb/test/API/tools/lldb-dap/gpu/hello_world.hip @@ -1,50 +1,39 @@ #include #include #include - -/// \brief Checks if the provided error code is \p hipSuccess and if not, -/// prints an error message to the standard error output and terminates the -/// program with an error code. constexpr int error_exit_code = -1; #define HIP_CHECK(condition) \ { \ - const hipError_t error = condition; \ + const hipError_t error = (condition); \ if (error != hipSuccess) { \ fprintf(stderr, "HIP error: \"%s\" at %s:%d\n", \ hipGetErrorString(error), __FILE__, __LINE__); \ exit(error_exit_code); \ } \ } - __global__ void add_one(int *data) { int idx = threadIdx.x; data[idx] = idx + 1; // GPU BREAKPOINT } - int main() { const int n = 4; - int host_data[n] = {0, 0, 0, 0}; // Initialize to zeros + int host_data[n] = {0, 0, 0, 0}; int *device_data; - printf("Starting GPU test...\n"); // CPU BREAKPOINT - BEFORE LAUNCH - // Allocate device memory HIP_CHECK(hipMalloc(&device_data, n * sizeof(int))); - // Copy data to device HIP_CHECK(hipMemcpy(device_data, host_data, n * sizeof(int), hipMemcpyHostToDevice)); - - // Launch kernel on default stream (single block, 4 threads) + // Launch kernel (single block, 4 threads) add_one<<<1, n>>>(device_data); - + // Check for kernel launch errors + HIP_CHECK(hipGetLastError()); + // Wait for GPU to finish HIP_CHECK(hipDeviceSynchronize()); - + // Copy results back to host HIP_CHECK(hipMemcpy(host_data, device_data, n * sizeof(int), hipMemcpyDeviceToHost)); - printf("Results: %d %d %d %d\n", host_data[0], host_data[1], host_data[2], host_data[3]); // CPU BREAKPOINT - AFTER LAUNCH - // Cleanup HIP_CHECK(hipFree(device_data)); - return 0; } From c41920b53a664ce2a41cf3d079479ddddee3c3f5 Mon Sep 17 00:00:00 2001 From: qxy11 Date: Thu, 14 Aug 2025 12:36:47 -0700 Subject: [PATCH 09/20] Rename instance variables w/ lldb convention --- lldb/tools/lldb-dap/DAP.cpp | 48 ++++++++++++++++++------------------- lldb/tools/lldb-dap/DAP.h | 10 ++++---- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 88e62b0efaa86..8e1989edcee0f 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -96,26 +96,26 @@ DAPSessionManager &DAPSessionManager::GetInstance() { } void DAPSessionManager::RegisterSession(lldb::IOObjectSP io, DAP *dap) { - std::lock_guard lock(sessions_mutex_); - active_sessions_[io] = dap; + std::lock_guard lock(m_sessions_mutex); + m_active_sessions[io] = dap; } void DAPSessionManager::UnregisterSession(lldb::IOObjectSP io) { - std::unique_lock lock(sessions_mutex_); - active_sessions_.erase(io); + std::unique_lock lock(m_sessions_mutex); + m_active_sessions.erase(io); // Clean up shared resources when the last session exits - if (active_sessions_.empty()) { + if (m_active_sessions.empty()) { CleanupSharedResources(); } - std::notify_all_at_thread_exit(sessions_condition_, std::move(lock)); + std::notify_all_at_thread_exit(m_sessions_condition, std::move(lock)); } std::vector DAPSessionManager::GetActiveSessions() { - std::lock_guard lock(sessions_mutex_); + std::lock_guard lock(m_sessions_mutex); std::vector sessions; - for (const auto &[io, dap] : active_sessions_) { + for (const auto &[io, dap] : m_active_sessions) { if (dap) { sessions.push_back(dap); } @@ -124,8 +124,8 @@ std::vector DAPSessionManager::GetActiveSessions() { } void DAPSessionManager::DisconnectAllSessions() { - std::lock_guard lock(sessions_mutex_); - for (const auto &[io, dap] : active_sessions_) { + std::lock_guard lock(m_sessions_mutex); + for (const auto &[io, dap] : m_active_sessions) { if (dap) { if (llvm::Error error = dap->Disconnect()) { llvm::errs() << "DAP client " << dap->transport.GetClientName() @@ -137,8 +137,8 @@ void DAPSessionManager::DisconnectAllSessions() { } void DAPSessionManager::WaitForAllSessionsToDisconnect() { - std::unique_lock lock(sessions_mutex_); - sessions_condition_.wait(lock, [this] { return active_sessions_.empty(); }); + std::unique_lock lock(m_sessions_mutex); + m_sessions_condition.wait(lock, [this] { return m_active_sessions.empty(); }); } std::shared_ptr @@ -146,35 +146,35 @@ DAPSessionManager::GetEventThreadForDebugger(lldb::SBDebugger debugger, DAP *requesting_dap) { lldb::user_id_t debugger_id = debugger.GetID(); - std::lock_guard lock(sessions_mutex_); + std::lock_guard lock(m_sessions_mutex); // Check if we already have a thread (most common case) - auto it = debugger_event_threads_.find(debugger_id); - if (it != debugger_event_threads_.end() && it->second) { + auto it = m_debugger_event_threads.find(debugger_id); + if (it != m_debugger_event_threads.end() && it->second) { return it->second; } // Create new thread and store it auto new_thread = std::make_shared(&DAP::EventThread, requesting_dap); - debugger_event_threads_[debugger_id] = new_thread; + m_debugger_event_threads[debugger_id] = new_thread; return new_thread; } void DAPSessionManager::SetSharedDebugger(lldb::SBDebugger debugger) { - std::lock_guard lock(sessions_mutex_); - shared_debugger_ = debugger; + std::lock_guard lock(m_sessions_mutex); + m_shared_debugger = debugger; } std::optional DAPSessionManager::GetSharedDebugger() { - std::lock_guard lock(sessions_mutex_); - return shared_debugger_; + std::lock_guard lock(m_sessions_mutex); + return m_shared_debugger; } DAP *DAPSessionManager::FindDAPForTarget(lldb::SBTarget target) { - std::lock_guard lock(sessions_mutex_); + std::lock_guard lock(m_sessions_mutex); - for (const auto &[io, dap] : active_sessions_) { + for (const auto &[io, dap] : m_active_sessions) { if (dap && dap->target.IsValid() && dap->target == target) { return dap; } @@ -184,8 +184,8 @@ DAP *DAPSessionManager::FindDAPForTarget(lldb::SBTarget target) { } void DAPSessionManager::CleanupSharedResources() { - if (shared_debugger_.has_value() && shared_debugger_->IsValid()) { - shared_debugger_ = std::nullopt; + if (m_shared_debugger.has_value() && m_shared_debugger->IsValid()) { + m_shared_debugger = std::nullopt; } } diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index bada7ededd2be..2943bd64c1d16 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -112,18 +112,18 @@ class DAPSessionManager { DAPSessionManager(DAPSessionManager &&) = delete; DAPSessionManager &operator=(DAPSessionManager &&) = delete; - std::mutex sessions_mutex_; - std::condition_variable sessions_condition_; - std::map active_sessions_; + std::mutex m_sessions_mutex; + std::condition_variable m_sessions_condition; + std::map m_active_sessions; /// Optional shared debugger instance set when the native process /// spawns a new GPU target - std::optional shared_debugger_; + std::optional m_shared_debugger; /// Map from debugger ID to its event thread used for when /// multiple DAP sessions are using the same debugger instance. std::map> - debugger_event_threads_; + m_debugger_event_threads; }; enum class OutputType { Console, Important, Stdout, Stderr, Telemetry }; From e9dc33b9632cde778a2e96f00c591b5734dd4b52 Mon Sep 17 00:00:00 2001 From: qxy11 Date: Thu, 14 Aug 2025 13:20:31 -0700 Subject: [PATCH 10/20] Stronger mapping from targetIdx to debugger --- lldb/tools/lldb-dap/DAP.cpp | 36 ++++++++++++------- lldb/tools/lldb-dap/DAP.h | 15 ++++---- .../lldb-dap/Handler/AttachRequestHandler.cpp | 9 +++-- .../lldb-dap/Handler/LaunchRequestHandler.cpp | 2 +- 4 files changed, 38 insertions(+), 24 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 8e1989edcee0f..ecfcc32a788aa 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -161,14 +161,21 @@ DAPSessionManager::GetEventThreadForDebugger(lldb::SBDebugger debugger, return new_thread; } -void DAPSessionManager::SetSharedDebugger(lldb::SBDebugger debugger) { +void DAPSessionManager::SetSharedDebugger(uint32_t target_idx, + lldb::SBDebugger debugger) { std::lock_guard lock(m_sessions_mutex); - m_shared_debugger = debugger; + m_target_to_debugger_map[target_idx] = debugger; } -std::optional DAPSessionManager::GetSharedDebugger() { +std::optional +DAPSessionManager::GetSharedDebugger(uint32_t target_idx) { std::lock_guard lock(m_sessions_mutex); - return m_shared_debugger; + auto pos = m_target_to_debugger_map.find(target_idx); + if (pos == m_target_to_debugger_map.end()) + return std::nullopt; + lldb::SBDebugger debugger = pos->second; + m_target_to_debugger_map.erase(pos); + return debugger; } DAP *DAPSessionManager::FindDAPForTarget(lldb::SBTarget target) { @@ -184,9 +191,9 @@ DAP *DAPSessionManager::FindDAPForTarget(lldb::SBTarget target) { } void DAPSessionManager::CleanupSharedResources() { - if (m_shared_debugger.has_value() && m_shared_debugger->IsValid()) { - m_shared_debugger = std::nullopt; - } + // SBDebugger destructors will handle cleanup when the map entries are + // destroyed + m_target_to_debugger_map.clear(); } static std::string GetStringFromStructuredData(lldb::SBStructuredData &data, @@ -1338,13 +1345,15 @@ llvm::Error DAP::StartEventThreads() { return llvm::Error::success(); } -llvm::Error DAP::InitializeDebugger(bool use_shared_debugger) { +llvm::Error DAP::InitializeDebugger(std::optional target_idx) { // Initialize debugger instance (shared or individual) - if (use_shared_debugger) { - auto shared_debugger = DAPSessionManager::GetInstance().GetSharedDebugger(); + if (target_idx) { + auto shared_debugger = + DAPSessionManager::GetInstance().GetSharedDebugger(*target_idx); if (!shared_debugger) { - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "unable to get shared debugger"); + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + "Unable to find existing debugger for target"); } debugger = shared_debugger.value(); return StartEventThreads(); @@ -1591,7 +1600,8 @@ void DAP::EventThread() { auto target_index = debugger.GetIndexOfTarget(target); // Set the shared debugger for GPU processes - DAPSessionManager::GetInstance().SetSharedDebugger(debugger); + DAPSessionManager::GetInstance().SetSharedDebugger(target_index, + debugger); // We create "attachCommands" that will select the target that already // exists in LLDB. The DAP instance will attach to this already diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 2943bd64c1d16..2f7b095a84f1c 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -86,11 +86,11 @@ class DAPSessionManager { /// Wait for all sessions to finish disconnecting void WaitForAllSessionsToDisconnect(); - /// Set the shared debugger instance (only for GPU processes) - void SetSharedDebugger(lldb::SBDebugger debugger); + /// Set the shared debugger instance for a specific target index + void SetSharedDebugger(uint32_t target_idx, lldb::SBDebugger debugger); - /// Get the shared debugger instance if it exists - std::optional GetSharedDebugger(); + /// Get the shared debugger instance for a specific target index + std::optional GetSharedDebugger(uint32_t target_idx); /// Get or create event thread for a specific debugger std::shared_ptr @@ -116,9 +116,9 @@ class DAPSessionManager { std::condition_variable m_sessions_condition; std::map m_active_sessions; - /// Optional shared debugger instance set when the native process + /// Optional map from target index to shared debugger set when the native process /// spawns a new GPU target - std::optional m_shared_debugger; + std::map m_target_to_debugger_map; /// Map from debugger ID to its event thread used for when /// multiple DAP sessions are using the same debugger instance. @@ -474,7 +474,8 @@ struct DAP { /// @{ /// Perform complete DAP initialization in one call - llvm::Error InitializeDebugger(bool use_shared_debugger); + llvm::Error + InitializeDebugger(std::optional target_idx = std::nullopt); /// Start event handling threads based on client capabilities llvm::Error StartEventThreads(); diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp index 274a2cf21dc49..81da7d39fb770 100644 --- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp @@ -17,6 +17,7 @@ #include "lldb/lldb-defines.h" #include "llvm/Support/Error.h" #include "llvm/Support/FileSystem.h" +#include using namespace llvm; using namespace lldb_dap::protocol; @@ -31,8 +32,10 @@ namespace lldb_dap { Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { // Initialize DAP debugger and related components if not sharing previously // launched debugger. - bool use_shared_debugger = args.targetIdx != UINT32_MAX; - if (Error err = dap.InitializeDebugger(use_shared_debugger)) { + std::optional target_idx = + (args.targetIdx == UINT32_MAX) ? std::nullopt + : std::optional{args.targetIdx}; + if (Error err = dap.InitializeDebugger(target_idx)) { return err; } @@ -72,7 +75,7 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { lldb::SBError error; lldb::SBTarget target; - if (use_shared_debugger) { + if (target_idx) { lldb::SBTarget target = dap.debugger.GetTargetAtIndex(args.targetIdx); if (!target.IsValid()) { error.SetErrorStringWithFormat("invalid target_idx %u in attach config", diff --git a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp index 89e1123b18802..fef82dddea909 100644 --- a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp @@ -23,7 +23,7 @@ namespace lldb_dap { /// Launch request; value of command field is 'launch'. Error LaunchRequestHandler::Run(const LaunchRequestArguments &arguments) const { // Initialize DAP debugger - if (Error err = dap.InitializeDebugger(false /* use_shared_debugger */)) { + if (Error err = dap.InitializeDebugger()) { return err; } From 2b5436a0e469bc4d8cf51a3375e22a91d1e7330b Mon Sep 17 00:00:00 2001 From: qxy11 Date: Fri, 15 Aug 2025 00:35:48 -0700 Subject: [PATCH 11/20] Use weak reference to event handler in DAPSessionManager --- lldb/tools/lldb-dap/DAP.cpp | 32 ++++++++++++++++--- lldb/tools/lldb-dap/DAP.h | 9 ++++-- .../lldb-dap/Handler/AttachRequestHandler.cpp | 8 ++--- .../lldb-dap/Protocol/ProtocolRequests.h | 2 +- 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index ecfcc32a788aa..eec4a40c7f14f 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -150,15 +150,19 @@ DAPSessionManager::GetEventThreadForDebugger(lldb::SBDebugger debugger, // Check if we already have a thread (most common case) auto it = m_debugger_event_threads.find(debugger_id); - if (it != m_debugger_event_threads.end() && it->second) { - return it->second; + if (it != m_debugger_event_threads.end()) { + if (auto thread_sp = it->second.lock()) { + return thread_sp; + } + // Our weak pointer has expired + m_debugger_event_threads.erase(it); } // Create new thread and store it - auto new_thread = + auto new_thread_sp = std::make_shared(&DAP::EventThread, requesting_dap); - m_debugger_event_threads[debugger_id] = new_thread; - return new_thread; + m_debugger_event_threads[debugger_id] = new_thread_sp; + return new_thread_sp; } void DAPSessionManager::SetSharedDebugger(uint32_t target_idx, @@ -196,6 +200,19 @@ void DAPSessionManager::CleanupSharedResources() { m_target_to_debugger_map.clear(); } +void DAPSessionManager::ReleaseExpiredEventThreads() { + std::lock_guard lock(m_sessions_mutex); + for (auto it = m_debugger_event_threads.begin(); + it != m_debugger_event_threads.end();) { + // Check if the weak_ptr has expired (no DAP instances are using it anymore) + if (it->second.expired()) { + it = m_debugger_event_threads.erase(it); + } else { + ++it; + } + } +} + static std::string GetStringFromStructuredData(lldb::SBStructuredData &data, const char *key) { lldb::SBStructuredData keyValue = data.GetValueForKey(key); @@ -358,6 +375,11 @@ void DAP::StopEventHandlers() { event_thread_sp->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); diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 2f7b095a84f1c..07975ec748428 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -102,6 +102,9 @@ class DAPSessionManager { /// Clean up shared resources when the last session exits void CleanupSharedResources(); + /// Clean up expired event threads from the collection + void ReleaseExpiredEventThreads(); + private: DAPSessionManager() = default; ~DAPSessionManager() = default; @@ -116,13 +119,13 @@ class DAPSessionManager { std::condition_variable m_sessions_condition; std::map m_active_sessions; - /// Optional map from target index to shared debugger set when the native process - /// spawns a new GPU target + /// Optional map from target index to shared debugger set when the native + /// process spawns a new GPU target std::map m_target_to_debugger_map; /// Map from debugger ID to its event thread used for when /// multiple DAP sessions are using the same debugger instance. - std::map> + std::map> m_debugger_event_threads; }; diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp index 81da7d39fb770..b74c4b65efb90 100644 --- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp @@ -32,9 +32,7 @@ namespace lldb_dap { Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { // Initialize DAP debugger and related components if not sharing previously // launched debugger. - std::optional target_idx = - (args.targetIdx == UINT32_MAX) ? std::nullopt - : std::optional{args.targetIdx}; + std::optional target_idx = args.targetIdx; if (Error err = dap.InitializeDebugger(target_idx)) { return err; } @@ -76,10 +74,10 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { lldb::SBError error; lldb::SBTarget target; if (target_idx) { - lldb::SBTarget target = dap.debugger.GetTargetAtIndex(args.targetIdx); + lldb::SBTarget target = dap.debugger.GetTargetAtIndex(*target_idx); if (!target.IsValid()) { error.SetErrorStringWithFormat("invalid target_idx %u in attach config", - args.targetIdx); + *target_idx); } } else { target = dap.CreateTarget(error); diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index 8bf973e852534..252f75c042f6c 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -346,7 +346,7 @@ struct AttachRequestArguments { std::string coreFile; /// Index of an existing target to attach to. - uint32_t targetIdx = UINT32_MAX; + std::optional targetIdx; /// @} }; From 028edddb11b614cdd7589755b51e526be75724ee Mon Sep 17 00:00:00 2001 From: qxy11 Date: Fri, 15 Aug 2025 13:26:33 -0700 Subject: [PATCH 12/20] Move DAPSessionManager into its own file and std::once in GetInstance() --- lldb/tools/lldb-dap/CMakeLists.txt | 1 + lldb/tools/lldb-dap/DAP.cpp | 127 --------------- lldb/tools/lldb-dap/DAP.h | 64 +------- lldb/tools/lldb-dap/DAPSessionManager.cpp | 149 ++++++++++++++++++ lldb/tools/lldb-dap/DAPSessionManager.h | 94 +++++++++++ .../gn/secondary/lldb/tools/lldb-dap/BUILD.gn | 1 + 6 files changed, 246 insertions(+), 190 deletions(-) create mode 100644 lldb/tools/lldb-dap/DAPSessionManager.cpp create mode 100644 lldb/tools/lldb-dap/DAPSessionManager.h diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt index 4cddfb1bea1c2..81f5baabf1cc4 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 eec4a40c7f14f..3eeff891be498 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -86,133 +86,6 @@ const char DEV_NULL[] = "/dev/null"; namespace lldb_dap { -DAPSessionManager &DAPSessionManager::GetInstance() { - // NOTE: intentional leak to avoid issues with C++ destructor chain - static DAPSessionManager *instance = nullptr; - if (!instance) { - instance = new DAPSessionManager(); - } - return *instance; -} - -void DAPSessionManager::RegisterSession(lldb::IOObjectSP io, DAP *dap) { - std::lock_guard lock(m_sessions_mutex); - m_active_sessions[io] = dap; -} - -void DAPSessionManager::UnregisterSession(lldb::IOObjectSP io) { - std::unique_lock lock(m_sessions_mutex); - m_active_sessions.erase(io); - - // Clean up shared resources when the last session exits - if (m_active_sessions.empty()) { - CleanupSharedResources(); - } - - std::notify_all_at_thread_exit(m_sessions_condition, std::move(lock)); -} - -std::vector DAPSessionManager::GetActiveSessions() { - std::lock_guard lock(m_sessions_mutex); - std::vector sessions; - for (const auto &[io, dap] : m_active_sessions) { - if (dap) { - sessions.push_back(dap); - } - } - return sessions; -} - -void DAPSessionManager::DisconnectAllSessions() { - std::lock_guard lock(m_sessions_mutex); - for (const auto &[io, dap] : m_active_sessions) { - if (dap) { - if (llvm::Error error = dap->Disconnect()) { - llvm::errs() << "DAP client " << dap->transport.GetClientName() - << " disconnected failed: " - << llvm::toString(std::move(error)) << "\n"; - } - } - } -} - -void DAPSessionManager::WaitForAllSessionsToDisconnect() { - std::unique_lock lock(m_sessions_mutex); - m_sessions_condition.wait(lock, [this] { return m_active_sessions.empty(); }); -} - -std::shared_ptr -DAPSessionManager::GetEventThreadForDebugger(lldb::SBDebugger debugger, - DAP *requesting_dap) { - lldb::user_id_t debugger_id = debugger.GetID(); - - std::lock_guard lock(m_sessions_mutex); - - // Check if we already have a thread (most common case) - auto it = m_debugger_event_threads.find(debugger_id); - if (it != m_debugger_event_threads.end()) { - if (auto thread_sp = it->second.lock()) { - return thread_sp; - } - // Our weak pointer has expired - m_debugger_event_threads.erase(it); - } - - // Create new thread and store it - auto new_thread_sp = - std::make_shared(&DAP::EventThread, requesting_dap); - m_debugger_event_threads[debugger_id] = new_thread_sp; - return new_thread_sp; -} - -void DAPSessionManager::SetSharedDebugger(uint32_t target_idx, - lldb::SBDebugger debugger) { - std::lock_guard lock(m_sessions_mutex); - m_target_to_debugger_map[target_idx] = debugger; -} - -std::optional -DAPSessionManager::GetSharedDebugger(uint32_t target_idx) { - std::lock_guard lock(m_sessions_mutex); - auto pos = m_target_to_debugger_map.find(target_idx); - if (pos == m_target_to_debugger_map.end()) - return std::nullopt; - lldb::SBDebugger debugger = pos->second; - m_target_to_debugger_map.erase(pos); - return debugger; -} - -DAP *DAPSessionManager::FindDAPForTarget(lldb::SBTarget target) { - std::lock_guard lock(m_sessions_mutex); - - for (const auto &[io, dap] : m_active_sessions) { - if (dap && dap->target.IsValid() && dap->target == target) { - return dap; - } - } - - return nullptr; -} - -void DAPSessionManager::CleanupSharedResources() { - // SBDebugger destructors will handle cleanup when the map entries are - // destroyed - m_target_to_debugger_map.clear(); -} - -void DAPSessionManager::ReleaseExpiredEventThreads() { - std::lock_guard lock(m_sessions_mutex); - for (auto it = m_debugger_event_threads.begin(); - it != m_debugger_event_threads.end();) { - // Check if the weak_ptr has expired (no DAP instances are using it anymore) - if (it->second.expired()) { - it = m_debugger_event_threads.erase(it); - } else { - ++it; - } - } -} - static std::string GetStringFromStructuredData(lldb::SBStructuredData &data, const char *key) { lldb::SBStructuredData keyValue = data.GetValueForKey(key); diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 07975ec748428..1f1669e4e1677 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -10,6 +10,7 @@ #define LLDB_TOOLS_LLDB_DAP_DAP_H #include "DAPForward.h" +#include "DAPSessionManager.h" #include "ExceptionBreakpoint.h" #include "FunctionBreakpoint.h" #include "InstructionBreakpoint.h" @@ -65,69 +66,6 @@ typedef llvm::DenseMap using AdapterFeature = protocol::AdapterFeature; using ClientFeature = protocol::ClientFeature; -/// Global DAP session manager -class DAPSessionManager { -public: - /// Get the singleton instance of the DAP session manager - static DAPSessionManager &GetInstance(); - - /// Register a DAP session - void RegisterSession(lldb::IOObjectSP io, DAP *dap); - - /// Unregister a DAP session - void UnregisterSession(lldb::IOObjectSP io); - - /// Get all active DAP sessions - std::vector GetActiveSessions(); - - /// Disconnect all active sessions - void DisconnectAllSessions(); - - /// Wait for all sessions to finish disconnecting - void WaitForAllSessionsToDisconnect(); - - /// Set the shared debugger instance for a specific target index - void SetSharedDebugger(uint32_t target_idx, lldb::SBDebugger debugger); - - /// Get the shared debugger instance for a specific target index - std::optional GetSharedDebugger(uint32_t target_idx); - - /// Get or create event thread for a specific debugger - std::shared_ptr - GetEventThreadForDebugger(lldb::SBDebugger debugger, DAP *requesting_dap); - - /// Find the DAP instance that owns the given target - DAP *FindDAPForTarget(lldb::SBTarget target); - - /// Clean up shared resources when the last session exits - void CleanupSharedResources(); - - /// Clean up expired event threads from the collection - void ReleaseExpiredEventThreads(); - -private: - DAPSessionManager() = default; - ~DAPSessionManager() = default; - - // Non-copyable and non-movable - DAPSessionManager(const DAPSessionManager &) = delete; - DAPSessionManager &operator=(const DAPSessionManager &) = delete; - DAPSessionManager(DAPSessionManager &&) = delete; - DAPSessionManager &operator=(DAPSessionManager &&) = delete; - - std::mutex m_sessions_mutex; - std::condition_variable m_sessions_condition; - std::map m_active_sessions; - - /// Optional map from target index to shared debugger set when the native - /// process spawns a new GPU target - std::map m_target_to_debugger_map; - - /// Map from debugger ID to its event thread used for when - /// multiple DAP sessions are using the same debugger instance. - std::map> - m_debugger_event_threads; -}; enum class OutputType { Console, Important, Stdout, Stderr, Telemetry }; diff --git a/lldb/tools/lldb-dap/DAPSessionManager.cpp b/lldb/tools/lldb-dap/DAPSessionManager.cpp new file mode 100644 index 0000000000000..c11b7899ae71d --- /dev/null +++ b/lldb/tools/lldb-dap/DAPSessionManager.cpp @@ -0,0 +1,149 @@ +//===-- DAPSessionManager.cpp ----------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "DAPSessionManager.h" +#include "DAP.h" +#include "lldb/API/SBTarget.h" +#include "llvm/Support/Threading.h" +#include +#include + +namespace lldb_dap { + +DAPSessionManager &DAPSessionManager::GetInstance() { + // NOTE: intentional leak to avoid issues with C++ destructor chain + // Use std::call_once for thread-safe initialization + static std::once_flag initialized; + static DAPSessionManager *instance = nullptr; + + std::call_once(initialized, []() { + instance = new DAPSessionManager(); + }); + + return *instance; +} + +void DAPSessionManager::RegisterSession(lldb::IOObjectSP io, DAP *dap) { + std::lock_guard lock(m_sessions_mutex); + m_active_sessions[io] = dap; +} + +void DAPSessionManager::UnregisterSession(lldb::IOObjectSP io) { + std::unique_lock lock(m_sessions_mutex); + m_active_sessions.erase(io); + + // Clean up shared resources when the last session exits + if (m_active_sessions.empty()) { + CleanupSharedResources(); + } + + std::notify_all_at_thread_exit(m_sessions_condition, std::move(lock)); +} + +std::vector DAPSessionManager::GetActiveSessions() { + std::lock_guard lock(m_sessions_mutex); + std::vector sessions; + for (const auto &[io, dap] : m_active_sessions) { + if (dap) { + sessions.push_back(dap); + } + } + return sessions; +} + +void DAPSessionManager::DisconnectAllSessions() { + std::lock_guard lock(m_sessions_mutex); + for (const auto &[io, dap] : m_active_sessions) { + if (dap) { + if (llvm::Error error = dap->Disconnect()) { + llvm::errs() << "DAP client " << dap->transport.GetClientName() + << " disconnected failed: " + << llvm::toString(std::move(error)) << "\n"; + } + } + } +} + +void DAPSessionManager::WaitForAllSessionsToDisconnect() { + std::unique_lock lock(m_sessions_mutex); + m_sessions_condition.wait(lock, [this] { return m_active_sessions.empty(); }); +} + +std::shared_ptr +DAPSessionManager::GetEventThreadForDebugger(lldb::SBDebugger debugger, + DAP *requesting_dap) { + lldb::user_id_t debugger_id = debugger.GetID(); + + std::lock_guard lock(m_sessions_mutex); + + // Check if we already have a thread (most common case) + auto it = m_debugger_event_threads.find(debugger_id); + if (it != m_debugger_event_threads.end()) { + if (auto thread_sp = it->second.lock()) { + return thread_sp; + } + // Our weak pointer has expired + m_debugger_event_threads.erase(it); + } + + // Create new thread and store it + auto new_thread_sp = + std::make_shared(&DAP::EventThread, requesting_dap); + m_debugger_event_threads[debugger_id] = new_thread_sp; + return new_thread_sp; +} + +void DAPSessionManager::SetSharedDebugger(uint32_t target_idx, + lldb::SBDebugger debugger) { + std::lock_guard lock(m_sessions_mutex); + m_target_to_debugger_map[target_idx] = debugger; +} + +std::optional +DAPSessionManager::GetSharedDebugger(uint32_t target_idx) { + std::lock_guard lock(m_sessions_mutex); + auto pos = m_target_to_debugger_map.find(target_idx); + if (pos == m_target_to_debugger_map.end()) + return std::nullopt; + lldb::SBDebugger debugger = pos->second; + m_target_to_debugger_map.erase(pos); + return debugger; +} + +DAP *DAPSessionManager::FindDAPForTarget(lldb::SBTarget target) { + std::lock_guard lock(m_sessions_mutex); + + for (const auto &[io, dap] : m_active_sessions) { + if (dap && dap->target.IsValid() && dap->target == target) { + return dap; + } + } + + return nullptr; +} + +void DAPSessionManager::CleanupSharedResources() { + // SBDebugger destructors will handle cleanup when the map entries are + // destroyed + m_target_to_debugger_map.clear(); +} + +void DAPSessionManager::ReleaseExpiredEventThreads() { + std::lock_guard lock(m_sessions_mutex); + for (auto it = m_debugger_event_threads.begin(); + it != m_debugger_event_threads.end();) { + // Check if the weak_ptr has expired (no DAP instances are using it anymore) + if (it->second.expired()) { + it = m_debugger_event_threads.erase(it); + } else { + ++it; + } + } +} + +} // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/DAPSessionManager.h b/lldb/tools/lldb-dap/DAPSessionManager.h new file mode 100644 index 0000000000000..5ba921529405a --- /dev/null +++ b/lldb/tools/lldb-dap/DAPSessionManager.h @@ -0,0 +1,94 @@ +//===-- DAPSessionManager.h ------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_TOOLS_LLDB_DAP_DAPSESSIONMANAGER_H +#define LLDB_TOOLS_LLDB_DAP_DAPSESSIONMANAGER_H + +#include "lldb/API/SBDebugger.h" +#include "lldb/API/SBTarget.h" +#include "lldb/lldb-types.h" +#include +#include +#include +#include +#include +#include +#include + +namespace lldb_dap { + +struct DAP; // Forward declaration + +/// Global DAP session manager +/// Global DAP session manager +class DAPSessionManager { +public: + /// Get the singleton instance of the DAP session manager + static DAPSessionManager &GetInstance(); + + /// Register a DAP session + void RegisterSession(lldb::IOObjectSP io, DAP *dap); + + /// Unregister a DAP session + void UnregisterSession(lldb::IOObjectSP io); + + /// Get all active DAP sessions + std::vector GetActiveSessions(); + + /// Disconnect all active sessions + void DisconnectAllSessions(); + + /// Wait for all sessions to finish disconnecting + void WaitForAllSessionsToDisconnect(); + + /// Set the shared debugger instance for a specific target index + void SetSharedDebugger(uint32_t target_idx, lldb::SBDebugger debugger); + + /// Get the shared debugger instance for a specific target index + std::optional GetSharedDebugger(uint32_t target_idx); + + /// Get or create event thread for a specific debugger + std::shared_ptr + GetEventThreadForDebugger(lldb::SBDebugger debugger, DAP *requesting_dap); + + /// Find the DAP instance that owns the given target + DAP *FindDAPForTarget(lldb::SBTarget target); + + /// Clean up shared resources when the last session exits + void CleanupSharedResources(); + + /// Clean up expired event threads from the collection + void ReleaseExpiredEventThreads(); + +private: + DAPSessionManager() = default; + ~DAPSessionManager() = default; + + // Non-copyable and non-movable + DAPSessionManager(const DAPSessionManager &) = delete; + DAPSessionManager &operator=(const DAPSessionManager &) = delete; + DAPSessionManager(DAPSessionManager &&) = delete; + DAPSessionManager &operator=(DAPSessionManager &&) = delete; + + std::mutex m_sessions_mutex; + std::condition_variable m_sessions_condition; + std::map m_active_sessions; + + /// Optional map from target index to shared debugger set when the native + /// process spawns a new GPU target + std::map m_target_to_debugger_map; + + /// Map from debugger ID to its event thread used for when + /// multiple DAP sessions are using the same debugger instance. + std::map> + m_debugger_event_threads; +}; + +} // namespace lldb_dap + +#endif // LLDB_TOOLS_LLDB_DAP_DAPSESSIONMANAGER_H diff --git a/llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn b/llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn index 30a1e03e3bffa..04650130b1cbc 100644 --- a/llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn +++ b/llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn @@ -25,6 +25,7 @@ static_library("lib") { "DAP.cpp", "DAPError.cpp", "DAPLog.cpp", + "DAPSessionManager.cpp", "EventHelper.cpp", "ExceptionBreakpoint.cpp", "FifoFiles.cpp", From 9cc4af8e8a32ee11ade9d283fda01c43e8681364 Mon Sep 17 00:00:00 2001 From: qxy11 Date: Mon, 18 Aug 2025 15:46:49 -0700 Subject: [PATCH 13/20] Address thread safety issue w/ RAII ManagedEventThread Summary: This should allow the shared_ptr destruction to take care of broadcasting events and joining the event thread() on the last reference to it --- lldb/tools/lldb-dap/DAP.cpp | 9 ------- lldb/tools/lldb-dap/DAP.h | 3 +-- lldb/tools/lldb-dap/DAPSessionManager.cpp | 32 ++++++++++++++++------- lldb/tools/lldb-dap/DAPSessionManager.h | 25 ++++++++++++++---- 4 files changed, 43 insertions(+), 26 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 3eeff891be498..1c19688ccbc4b 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -239,15 +239,6 @@ llvm::Error DAP::ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr) { } void DAP::StopEventHandlers() { - // Check if this is the last reference to the shared event thread - if (event_thread_sp && event_thread_sp.use_count() == 1 && - event_thread_sp->joinable()) { - // Signal the shared event thread to stop - broadcaster.BroadcastEventByType(eBroadcastBitStopEventThread); - - event_thread_sp->join(); - } - event_thread_sp.reset(); // Clean up expired event threads from the session manager diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 1f1669e4e1677..aae86682d0f2c 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -66,7 +66,6 @@ typedef llvm::DenseMap using AdapterFeature = protocol::AdapterFeature; using ClientFeature = protocol::ClientFeature; - enum class OutputType { Console, Important, Stdout, Stderr, Telemetry }; /// Buffer size for handling output events. @@ -461,7 +460,7 @@ struct DAP { /// Event thread is a shared pointer in case we have a multiple /// DAP instances sharing the same event thread - std::shared_ptr event_thread_sp; + std::shared_ptr event_thread_sp; std::thread progress_event_thread; /// @} diff --git a/lldb/tools/lldb-dap/DAPSessionManager.cpp b/lldb/tools/lldb-dap/DAPSessionManager.cpp index c11b7899ae71d..24c4e2b744cd3 100644 --- a/lldb/tools/lldb-dap/DAPSessionManager.cpp +++ b/lldb/tools/lldb-dap/DAPSessionManager.cpp @@ -5,9 +5,10 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // //===----------------------------------------------------------------------===// - #include "DAPSessionManager.h" #include "DAP.h" +#include "lldb/API/SBBroadcaster.h" +#include "lldb/API/SBEvent.h" #include "lldb/API/SBTarget.h" #include "llvm/Support/Threading.h" #include @@ -15,6 +16,17 @@ namespace lldb_dap { +ManagedEventThread::ManagedEventThread(lldb::SBBroadcaster broadcaster, + std::thread t) + : m_broadcaster(broadcaster), m_event_thread(std::move(t)) {} + +ManagedEventThread::~ManagedEventThread() { + if (m_event_thread.joinable()) { + m_broadcaster.BroadcastEventByType(eBroadcastBitStopEventThread); + m_event_thread.join(); + } +} + DAPSessionManager &DAPSessionManager::GetInstance() { // NOTE: intentional leak to avoid issues with C++ destructor chain // Use std::call_once for thread-safe initialization @@ -74,26 +86,26 @@ void DAPSessionManager::WaitForAllSessionsToDisconnect() { m_sessions_condition.wait(lock, [this] { return m_active_sessions.empty(); }); } -std::shared_ptr +std::shared_ptr DAPSessionManager::GetEventThreadForDebugger(lldb::SBDebugger debugger, DAP *requesting_dap) { lldb::user_id_t debugger_id = debugger.GetID(); - std::lock_guard lock(m_sessions_mutex); - // Check if we already have a thread (most common case) - auto it = m_debugger_event_threads.find(debugger_id); - if (it != m_debugger_event_threads.end()) { - if (auto thread_sp = it->second.lock()) { + // Try to use shared event thread, if it exists + if (auto it = m_debugger_event_threads.find(debugger_id); + it != m_debugger_event_threads.end()) { + if (auto thread_sp = it->second.lock()) { return thread_sp; } // Our weak pointer has expired m_debugger_event_threads.erase(it); } - // Create new thread and store it - auto new_thread_sp = - std::make_shared(&DAP::EventThread, requesting_dap); + // Create a new event thread and store it + auto new_thread_sp = std::make_shared( + requesting_dap->broadcaster, + std::thread(&DAP::EventThread, requesting_dap)); m_debugger_event_threads[debugger_id] = new_thread_sp; return new_thread_sp; } diff --git a/lldb/tools/lldb-dap/DAPSessionManager.h b/lldb/tools/lldb-dap/DAPSessionManager.h index 5ba921529405a..a46e4c3e4aa00 100644 --- a/lldb/tools/lldb-dap/DAPSessionManager.h +++ b/lldb/tools/lldb-dap/DAPSessionManager.h @@ -9,6 +9,7 @@ #ifndef LLDB_TOOLS_LLDB_DAP_DAPSESSIONMANAGER_H #define LLDB_TOOLS_LLDB_DAP_DAPSESSIONMANAGER_H +#include "lldb/API/SBBroadcaster.h" #include "lldb/API/SBDebugger.h" #include "lldb/API/SBTarget.h" #include "lldb/lldb-types.h" @@ -22,9 +23,24 @@ namespace lldb_dap { -struct DAP; // Forward declaration +// Forward declarations +struct DAP; + +class ManagedEventThread { +public: + // Constructor declaration + ManagedEventThread(lldb::SBBroadcaster broadcaster, std::thread t); + + ~ManagedEventThread(); + + ManagedEventThread(const ManagedEventThread &) = delete; + ManagedEventThread &operator=(const ManagedEventThread &) = delete; + +private: + lldb::SBBroadcaster m_broadcaster; + std::thread m_event_thread; +}; -/// Global DAP session manager /// Global DAP session manager class DAPSessionManager { public: @@ -53,7 +69,7 @@ class DAPSessionManager { std::optional GetSharedDebugger(uint32_t target_idx); /// Get or create event thread for a specific debugger - std::shared_ptr + std::shared_ptr GetEventThreadForDebugger(lldb::SBDebugger debugger, DAP *requesting_dap); /// Find the DAP instance that owns the given target @@ -85,8 +101,7 @@ class DAPSessionManager { /// Map from debugger ID to its event thread used for when /// multiple DAP sessions are using the same debugger instance. - std::map> - m_debugger_event_threads; + std::map> m_debugger_event_threads; }; } // namespace lldb_dap From afd3fe19060a2f190bb6d5ab77c3e1459de2611e Mon Sep 17 00:00:00 2001 From: qxy11 Date: Wed, 10 Sep 2025 22:47:53 -0700 Subject: [PATCH 14/20] Add session name to GPUActions to make it configurable per plugin, and encode the name into the target event --- lldb/include/lldb/API/SBTarget.h | 2 ++ lldb/include/lldb/Target/Target.h | 10 ++++++++ .../lldb/Utility/GPUGDBRemotePackets.h | 2 ++ lldb/source/API/SBTarget.cpp | 8 +++++++ lldb/source/Target/Target.cpp | 23 ++++++++++++++++--- lldb/source/Utility/GPUGDBRemotePackets.cpp | 2 ++ .../Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp | 7 +++++- .../Plugins/AMDGPU/LLDBServerPluginAMDGPU.h | 1 + .../MockGPU/LLDBServerPluginMockGPU.cpp | 1 + 9 files changed, 52 insertions(+), 4 deletions(-) diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 35d0e2a1412a4..0aaca844c6c18 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -70,6 +70,8 @@ class LLDB_API SBTarget { static lldb::SBModule GetModuleAtIndexFromEvent(const uint32_t idx, const lldb::SBEvent &event); + static const char *GetDAPSessionNameFromEvent(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 341a0eca9fb79..712513c198e8f 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -556,6 +556,13 @@ class Target : public std::enable_shared_from_this, TargetEventData(const lldb::TargetSP &target_sp, const ModuleList &module_list); + TargetEventData(const lldb::TargetSP &target_sp, + std::string dap_session_name); + + TargetEventData(const lldb::TargetSP &target_sp, + const ModuleList &module_list, + std::string dap_session_name); + ~TargetEventData() override; static llvm::StringRef GetFlavorString(); @@ -564,6 +571,8 @@ class Target : public std::enable_shared_from_this, return TargetEventData::GetFlavorString(); } + static llvm::StringRef GetDAPSessionNameFromEvent(const Event *event_ptr); + void Dump(Stream *s) const override; static const TargetEventData *GetEventDataFromEvent(const Event *event_ptr); @@ -579,6 +588,7 @@ class Target : public std::enable_shared_from_this, private: lldb::TargetSP m_target_sp; ModuleList m_module_list; + std::string m_dap_session_name = ""; TargetEventData(const TargetEventData &) = delete; const TargetEventData &operator=(const TargetEventData &) = delete; diff --git a/lldb/include/lldb/Utility/GPUGDBRemotePackets.h b/lldb/include/lldb/Utility/GPUGDBRemotePackets.h index 9561b8b4d5f81..718bd918c3bf4 100644 --- a/lldb/include/lldb/Utility/GPUGDBRemotePackets.h +++ b/lldb/include/lldb/Utility/GPUGDBRemotePackets.h @@ -177,6 +177,8 @@ struct GPUActions { /// The name of the plugin. std::string plugin_name; + /// The name to give a DAP session + std::string dap_session_name; /// New breakpoints to set. Nothing to set if this is empty. std::vector breakpoints; /// If a GPU connection is available return a connect URL to use to reverse diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index f26f7951edc6f..fb5213744d8c3 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::GetDAPSessionNameFromEvent(const SBEvent &event) { + LLDB_INSTRUMENT_VA(event); + + return ConstString( + Target::TargetEventData::GetDAPSessionNameFromEvent(event.get())) + .AsCString(); +} + const char *SBTarget::GetBroadcasterClassName() { LLDB_INSTRUMENT(); diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index d3a7125709c0d..6445361f22206 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -5129,13 +5129,22 @@ 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 dap_session_name) + : TargetEventData(target_sp, ModuleList(), std::move(dap_session_name)) {} + +Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp, + const ModuleList &module_list, + std::string dap_session_name) + : EventData(), m_target_sp(target_sp), m_module_list(module_list), + m_dap_session_name(std::move(dap_session_name)) {} Target::TargetEventData::~TargetEventData() = default; @@ -5171,6 +5180,14 @@ TargetSP Target::TargetEventData::GetTargetFromEvent(const Event *event_ptr) { return target_sp; } +llvm::StringRef +Target::TargetEventData::GetDAPSessionNameFromEvent(const Event *event_ptr) { + const TargetEventData *event_data = GetEventDataFromEvent(event_ptr); + if (event_data) + return event_data->m_dap_session_name; + return llvm::StringRef(); +} + ModuleList Target::TargetEventData::GetModuleListFromEvent(const Event *event_ptr) { ModuleList module_list; diff --git a/lldb/source/Utility/GPUGDBRemotePackets.cpp b/lldb/source/Utility/GPUGDBRemotePackets.cpp index f5131318e36b3..1aa1302e17362 100644 --- a/lldb/source/Utility/GPUGDBRemotePackets.cpp +++ b/lldb/source/Utility/GPUGDBRemotePackets.cpp @@ -139,6 +139,7 @@ bool fromJSON(const llvm::json::Value &value, GPUActions &data, llvm::json::Path path) { ObjectMapper o(value, path); return o && o.map("plugin_name", data.plugin_name) && + o.map("dap_session_name", data.dap_session_name) && o.map("breakpoints", data.breakpoints) && o.mapOptional("connect_info", data.connect_info) && o.map("load_libraries", data.load_libraries) && @@ -150,6 +151,7 @@ bool fromJSON(const llvm::json::Value &value, GPUActions &data, llvm::json::Value toJSON(const GPUActions &data) { return json::Value(Object{ {"plugin_name", data.plugin_name}, + {"dap_session_name", data.dap_session_name}, {"breakpoints", data.breakpoints}, {"connect_info", data.connect_info}, {"load_libraries", data.load_libraries}, diff --git a/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp b/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp index 824fb615967c7..b012359aa1231 100644 --- a/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp +++ b/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp @@ -147,6 +147,10 @@ LLDBServerPluginAMDGPU::~LLDBServerPluginAMDGPU() { CloseFDs(); } llvm::StringRef LLDBServerPluginAMDGPU::GetPluginName() { return "amd-gpu"; } +llvm::StringRef LLDBServerPluginAMDGPU::GetDAPSessionName() { + return "AMD GPU Session"; +} + void LLDBServerPluginAMDGPU::CloseFDs() { if (m_fds[0] != -1) { close(m_fds[0]); @@ -415,7 +419,8 @@ std::optional LLDBServerPluginAMDGPU::NativeProcessIsStopping() { "launched successfully"); } actions.connect_info = CreateConnection(); - actions.connect_info->synchronous = true; + actions.connect_info->synchronous = true; + actions.dap_session_name = GetDAPSessionName(); } return actions; } else { diff --git a/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.h b/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.h index 7f657879b6ca4..cdf1780dca4fc 100644 --- a/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.h +++ b/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.h @@ -68,6 +68,7 @@ class LLDBServerPluginAMDGPU : public LLDBServerPlugin { } bool CreateGPUBreakpoint(uint64_t addr); + llvm::StringRef GetDAPSessionName(); // TODO: make this private struct GPUInternalBreakpoinInfo { diff --git a/lldb/tools/lldb-server/Plugins/MockGPU/LLDBServerPluginMockGPU.cpp b/lldb/tools/lldb-server/Plugins/MockGPU/LLDBServerPluginMockGPU.cpp index c031b0136e15e..f49fd44770ef3 100644 --- a/lldb/tools/lldb-server/Plugins/MockGPU/LLDBServerPluginMockGPU.cpp +++ b/lldb/tools/lldb-server/Plugins/MockGPU/LLDBServerPluginMockGPU.cpp @@ -173,6 +173,7 @@ LLDBServerPluginMockGPU::BreakpointWasHit(GPUPluginBreakpointHitArgs &args) { LLDB_LOGF(log, "LLDBServerPluginMockGPU::BreakpointWasHit(\"%s\") disabling breakpoint", bp_identifier.c_str()); response.actions.connect_info = CreateConnection(); + response.actions.dap_session_name = "Mock GPU Session"; // We asked for the symbol "gpu_shlib_load" to be delivered as a symbol // value when the "gpu_initialize" breakpoint was set. So we will use this From aae99212683395b7abc1a8239e04787fe15cdd4b Mon Sep 17 00:00:00 2001 From: qxy11 Date: Wed, 10 Sep 2025 22:47:53 -0700 Subject: [PATCH 15/20] Pass configured session names to reverse request DAP instance, rename broadcast bit to eBroadcastBitNewTargetCreated and p --- lldb/include/lldb/API/SBTarget.h | 2 +- lldb/include/lldb/Target/Target.h | 2 +- .../Process/gdb-remote/ProcessGDBRemote.cpp | 6 +-- lldb/source/Target/Target.cpp | 2 +- lldb/tools/lldb-dap/DAP.cpp | 43 ++++++++++--------- 5 files changed, 28 insertions(+), 27 deletions(-) diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 0aaca844c6c18..e82a097a7d8fc 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -44,7 +44,7 @@ class LLDB_API SBTarget { eBroadcastBitWatchpointChanged = (1 << 3), eBroadcastBitSymbolsLoaded = (1 << 4), eBroadcastBitSymbolsChanged = (1 << 5), - eBroadcastBitNewTargetSpawned = (1 << 6), + eBroadcastBitNewTargetCreated = (1 << 6), }; // Constructors diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 712513c198e8f..af7da77d8ecdc 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -536,7 +536,7 @@ class Target : public std::enable_shared_from_this, eBroadcastBitWatchpointChanged = (1 << 3), eBroadcastBitSymbolsLoaded = (1 << 4), eBroadcastBitSymbolsChanged = (1 << 5), - eBroadcastBitNewTargetSpawned = (1 << 6), + eBroadcastBitNewTargetCreated = (1 << 6), }; // These two functions fill out the Broadcaster interface: diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index b44c4058beb29..b99047816b20f 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -1025,9 +1025,9 @@ Status ProcessGDBRemote::HandleConnectionRequest(const GPUActions &gpu_action) { process_sp->GetTarget().shared_from_this()); LLDB_LOG(log, "ProcessGDBRemote::HandleConnectionRequest(): successfully " "created process!!!"); - auto event_sp = - std::make_shared(Target::eBroadcastBitNewTargetSpawned, - new Target::TargetEventData(gpu_target_sp)); + auto event_sp = std::make_shared( + Target::eBroadcastBitNewTargetCreated, + new Target::TargetEventData(gpu_target_sp, gpu_action.dap_session_name)); GetTarget().BroadcastEvent(event_sp); return Status(); } diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 6445361f22206..052902e202e39 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -190,7 +190,7 @@ Target::Target(Debugger &debugger, const ArchSpec &target_arch, SetEventName(eBroadcastBitModulesUnloaded, "modules-unloaded"); SetEventName(eBroadcastBitWatchpointChanged, "watchpoint-changed"); SetEventName(eBroadcastBitSymbolsLoaded, "symbols-loaded"); - SetEventName(eBroadcastBitNewTargetSpawned, "new-target-spawned"); + SetEventName(eBroadcastBitNewTargetCreated, "new-target-spawned"); CheckInWithManager(); diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 1c19688ccbc4b..6bf21ce13305d 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -790,7 +790,7 @@ void DAP::SetTarget(const lldb::SBTarget target) { lldb::SBTarget::eBroadcastBitModulesUnloaded | lldb::SBTarget::eBroadcastBitSymbolsLoaded | lldb::SBTarget::eBroadcastBitSymbolsChanged | - lldb::SBTarget::eBroadcastBitNewTargetSpawned); + lldb::SBTarget::eBroadcastBitNewTargetCreated); listener.StartListeningForEvents(this->broadcaster, eBroadcastBitStopEventThread); } @@ -1231,15 +1231,15 @@ llvm::Error DAP::StartEventThreads() { return llvm::Error::success(); } -llvm::Error DAP::InitializeDebugger(std::optional target_idx) { +llvm::Error DAP::InitializeDebugger(std::optional target_id) { // Initialize debugger instance (shared or individual) - if (target_idx) { + if (target_id) { auto shared_debugger = - DAPSessionManager::GetInstance().GetSharedDebugger(*target_idx); + DAPSessionManager::GetInstance().GetSharedDebugger(*target_id); if (!shared_debugger) { return llvm::createStringError( llvm::inconvertibleErrorCode(), - "Unable to find existing debugger for target"); + "Unable to find existing debugger for target ID"); } debugger = shared_debugger.value(); return StartEventThreads(); @@ -1481,32 +1481,33 @@ void DAP::EventThread() { ModuleEventBody::eReasonNew}}); } } - } else if (event_mask & lldb::SBTarget::eBroadcastBitNewTargetSpawned) { + } else if (event_mask & lldb::SBTarget::eBroadcastBitNewTargetCreated) { auto target = lldb::SBTarget::GetTargetFromEvent(event); - auto target_index = debugger.GetIndexOfTarget(target); - // Set the shared debugger for GPU processes - DAPSessionManager::GetInstance().SetSharedDebugger(target_index, + // Generate unique target ID and set the shared debugger + uint32_t target_id = target.GetProcess().GetUniqueID(); + DAPSessionManager::GetInstance().SetSharedDebugger(target_id, debugger); - // We create "attachCommands" that will select the target that already - // exists in LLDB. The DAP instance will attach to this already - // existing target and the debug session will be ready to go. + // 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; llvm::json::Array attach_commands; - attach_commands.push_back(llvm::formatv("target list").str()); - attach_commands.push_back( - llvm::formatv("target select {0}", target_index).str()); - // 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("name", "GPU Session"); - attach_config.try_emplace("attachCommands", - std::move(attach_commands)); - attach_config.try_emplace("targetIdx", target_index); + attach_config.try_emplace("targetId", target_id); + const char *session_name = + lldb::SBTarget::GetDAPSessionNameFromEvent(event); + if (session_name && *session_name) { + attach_config.try_emplace("name", session_name); + } else { + std::string default_name = + llvm::formatv("Session {0}", target_id).str(); + attach_config.try_emplace("name", default_name); + } // 2. Construct the main 'startDebugging' request arguments. llvm::json::Object start_debugging_args; From 69d841c0ca23a30400af300be44139e0c9d5615a Mon Sep 17 00:00:00 2001 From: qxy11 Date: Wed, 10 Sep 2025 22:47:53 -0700 Subject: [PATCH 16/20] Assign unique target IDs to reverse attach to --- lldb/include/lldb/API/SBDebugger.h | 3 +++ lldb/include/lldb/Target/TargetList.h | 2 ++ lldb/source/API/SBDebugger.cpp | 10 ++++++++++ lldb/source/Target/TargetList.cpp | 13 +++++++++++++ lldb/tools/lldb-dap/DAP.cpp | 7 +++---- lldb/tools/lldb-dap/DAPSessionManager.cpp | 8 ++++---- lldb/tools/lldb-dap/DAPSessionManager.h | 15 ++++++++------- .../lldb-dap/Handler/AttachRequestHandler.cpp | 19 ++++++++++--------- .../lldb-dap/Protocol/ProtocolRequests.cpp | 2 +- .../lldb-dap/Protocol/ProtocolRequests.h | 4 ++-- 10 files changed, 56 insertions(+), 27 deletions(-) diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h index f77b0c1d7f0ee..8a9ab760d9e0a 100644 --- a/lldb/include/lldb/API/SBDebugger.h +++ b/lldb/include/lldb/API/SBDebugger.h @@ -359,6 +359,9 @@ class LLDB_API SBDebugger { lldb::SBTarget FindTargetWithFileAndArch(const char *filename, const char *arch); + /// Find a target with the specified unique process ID + lldb::SBTarget FindTargetWithUniqueID(uint32_t id); + /// Get the number of targets in the debugger. uint32_t GetNumTargets(); diff --git a/lldb/include/lldb/Target/TargetList.h b/lldb/include/lldb/Target/TargetList.h index 080a6039c7ff8..343fc1676ec30 100644 --- a/lldb/include/lldb/Target/TargetList.h +++ b/lldb/include/lldb/Target/TargetList.h @@ -159,6 +159,8 @@ class TargetList : public Broadcaster { lldb::TargetSP FindTargetWithProcess(lldb_private::Process *process) const; + lldb::TargetSP FindTargetWithUniqueID(uint32_t id) const; + lldb::TargetSP GetTargetSP(Target *target) const; /// Send an async interrupt to one or all processes. diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp index 603e306497841..f4b46cc3b1873 100644 --- a/lldb/source/API/SBDebugger.cpp +++ b/lldb/source/API/SBDebugger.cpp @@ -983,6 +983,16 @@ uint32_t SBDebugger::GetIndexOfTarget(lldb::SBTarget target) { return m_opaque_sp->GetTargetList().GetIndexOfTarget(target.GetSP()); } +SBTarget SBDebugger::FindTargetWithUniqueID(uint32_t id) { + LLDB_INSTRUMENT_VA(this, id); + SBTarget sb_target; + if (m_opaque_sp) { + // No need to lock, the target list is thread safe + sb_target.SetSP(m_opaque_sp->GetTargetList().FindTargetWithUniqueID(id)); + } + return sb_target; +} + SBTarget SBDebugger::FindTargetWithProcessID(lldb::pid_t pid) { LLDB_INSTRUMENT_VA(this, pid); diff --git a/lldb/source/Target/TargetList.cpp b/lldb/source/Target/TargetList.cpp index 7037dc2bea3cc..3bd655eeb680d 100644 --- a/lldb/source/Target/TargetList.cpp +++ b/lldb/source/Target/TargetList.cpp @@ -428,6 +428,19 @@ TargetSP TargetList::FindTargetWithProcess(Process *process) const { return target_sp; } +TargetSP TargetList::FindTargetWithUniqueID(uint32_t id) const { + std::lock_guard guard(m_target_list_mutex); + auto it = llvm::find_if(m_target_list, [id](const TargetSP &item) { + auto *process_ptr = item->GetProcessSP().get(); + return process_ptr && (process_ptr->GetUniqueID() == id); + }); + + if (it != m_target_list.end()) + return *it; + + return TargetSP(); +} + TargetSP TargetList::GetTargetSP(Target *target) const { TargetSP target_sp; if (!target) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 6bf21ce13305d..56ebd824527de 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -1510,10 +1510,9 @@ void DAP::EventThread() { } // 2. Construct the main 'startDebugging' request arguments. - llvm::json::Object start_debugging_args; - start_debugging_args.try_emplace("request", "attach"); - start_debugging_args.try_emplace("configuration", - std::move(attach_config)); + llvm::json::Object start_debugging_args{ + {"request", "attach"}, + {"configuration", std::move(attach_config)}}; // Send the request. Note that this is a reverse request, so you don't // expect a direct response in the same way as a client request. diff --git a/lldb/tools/lldb-dap/DAPSessionManager.cpp b/lldb/tools/lldb-dap/DAPSessionManager.cpp index 24c4e2b744cd3..fca78e241ffe9 100644 --- a/lldb/tools/lldb-dap/DAPSessionManager.cpp +++ b/lldb/tools/lldb-dap/DAPSessionManager.cpp @@ -110,16 +110,16 @@ DAPSessionManager::GetEventThreadForDebugger(lldb::SBDebugger debugger, return new_thread_sp; } -void DAPSessionManager::SetSharedDebugger(uint32_t target_idx, +void DAPSessionManager::SetSharedDebugger(uint32_t target_id, lldb::SBDebugger debugger) { std::lock_guard lock(m_sessions_mutex); - m_target_to_debugger_map[target_idx] = debugger; + m_target_to_debugger_map[target_id] = debugger; } std::optional -DAPSessionManager::GetSharedDebugger(uint32_t target_idx) { +DAPSessionManager::GetSharedDebugger(uint32_t target_id) { std::lock_guard lock(m_sessions_mutex); - auto pos = m_target_to_debugger_map.find(target_idx); + auto pos = m_target_to_debugger_map.find(target_id); if (pos == m_target_to_debugger_map.end()) return std::nullopt; lldb::SBDebugger debugger = pos->second; diff --git a/lldb/tools/lldb-dap/DAPSessionManager.h b/lldb/tools/lldb-dap/DAPSessionManager.h index a46e4c3e4aa00..507bf34e5dd38 100644 --- a/lldb/tools/lldb-dap/DAPSessionManager.h +++ b/lldb/tools/lldb-dap/DAPSessionManager.h @@ -24,7 +24,7 @@ namespace lldb_dap { // Forward declarations -struct DAP; +struct DAP; class ManagedEventThread { public: @@ -62,11 +62,11 @@ class DAPSessionManager { /// Wait for all sessions to finish disconnecting void WaitForAllSessionsToDisconnect(); - /// Set the shared debugger instance for a specific target index - void SetSharedDebugger(uint32_t target_idx, lldb::SBDebugger debugger); + /// Set the shared debugger instance for a unique target ID + void SetSharedDebugger(uint32_t target_id, lldb::SBDebugger debugger); - /// Get the shared debugger instance for a specific target index - std::optional GetSharedDebugger(uint32_t target_idx); + /// Get the shared debugger instance for a unique target ID + std::optional GetSharedDebugger(uint32_t target_id); /// Get or create event thread for a specific debugger std::shared_ptr @@ -95,13 +95,14 @@ class DAPSessionManager { std::condition_variable m_sessions_condition; std::map m_active_sessions; - /// Optional map from target index to shared debugger set when the native + /// Optional map from target ID to shared debugger set when the native /// process spawns a new GPU target std::map m_target_to_debugger_map; /// Map from debugger ID to its event thread used for when /// multiple DAP sessions are using the same debugger instance. - std::map> m_debugger_event_threads; + std::map> + m_debugger_event_threads; }; } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp index b74c4b65efb90..e6293a882d7e0 100644 --- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp @@ -32,19 +32,18 @@ namespace lldb_dap { Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { // Initialize DAP debugger and related components if not sharing previously // launched debugger. - std::optional target_idx = args.targetIdx; - if (Error err = dap.InitializeDebugger(target_idx)) { + std::optional target_id = args.targetId; + if (Error err = dap.InitializeDebugger(target_id)) return err; - } // Validate that we have a well formed attach request. if (args.attachCommands.empty() && args.coreFile.empty() && args.configuration.program.empty() && args.pid == LLDB_INVALID_PROCESS_ID && - args.gdbRemotePort == LLDB_DAP_INVALID_PORT) + args.gdbRemotePort == LLDB_DAP_INVALID_PORT && !target_id.has_value()) return make_error( "expected one of 'pid', 'program', 'attachCommands', " - "'coreFile' or 'gdb-remote-port' to be specified"); + "'coreFile', 'gdb-remote-port', or target_id to be specified"); // Check if we have mutually exclusive arguments. if ((args.pid != LLDB_INVALID_PROCESS_ID) && @@ -73,15 +72,17 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { lldb::SBError error; lldb::SBTarget target; - if (target_idx) { - lldb::SBTarget target = dap.debugger.GetTargetAtIndex(*target_idx); + if (target_id) { + // Use the unique target ID to get the target + target = dap.debugger.FindTargetWithUniqueID(*target_id); if (!target.IsValid()) { - error.SetErrorStringWithFormat("invalid target_idx %u in attach config", - *target_idx); + error.SetErrorStringWithFormat("invalid target_id %u in attach config", + *target_id); } } else { target = dap.CreateTarget(error); } + if (error.Fail()) return ToError(error); dap.SetTarget(target); diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp index a692bb7dfdffc..ccf97f78680fb 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp @@ -316,7 +316,7 @@ bool fromJSON(const json::Value &Params, AttachRequestArguments &ARA, O.mapOptional("gdb-remote-port", ARA.gdbRemotePort) && O.mapOptional("gdb-remote-hostname", ARA.gdbRemoteHostname) && O.mapOptional("coreFile", ARA.coreFile) && - O.mapOptional("targetIdx", ARA.targetIdx); + O.mapOptional("targetId", ARA.targetId); } bool fromJSON(const json::Value &Params, ContinueArguments &CA, json::Path P) { diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index 252f75c042f6c..fe84c429fa21f 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -345,8 +345,8 @@ struct AttachRequestArguments { /// Path to the core file to debug. std::string coreFile; - /// Index of an existing target to attach to. - std::optional targetIdx; + /// Unique ID of an existing target to attach to. + std::optional targetId; /// @} }; From f18bd67ddde4bd13f9ecde2bbfe046722110243a Mon Sep 17 00:00:00 2001 From: qxy11 Date: Wed, 10 Sep 2025 22:47:53 -0700 Subject: [PATCH 17/20] Move and fix tests for unique target ids --- .../test/tools/lldb-dap/dap_server.py | 29 +++++++++--------- .../test/tools/lldb-dap/lldbdap_testcase.py | 24 +++++++-------- .../API/tools/lldb-dap/gpu/{ => amd}/Makefile | 0 .../{ => amd}/TestDAP_gpu_reverse_request.py | 30 +++++++++---------- .../lldb-dap/gpu/{ => amd}/hello_world.hip | 4 +++ 5 files changed, 44 insertions(+), 43 deletions(-) rename lldb/test/API/tools/lldb-dap/gpu/{ => amd}/Makefile (100%) rename lldb/test/API/tools/lldb-dap/gpu/{ => amd}/TestDAP_gpu_reverse_request.py (82%) rename lldb/test/API/tools/lldb-dap/gpu/{ => amd}/hello_world.hip (99%) 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 8a17ed69dbd31..12bf51521f491 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 @@ -12,7 +12,7 @@ import sys import threading import time -from typing import Any, Optional, Union, BinaryIO, TextIO +from typing import Any, Dict, Optional, Union, BinaryIO, TextIO ## DAP type references Event = dict[str, Any] @@ -331,7 +331,7 @@ def _handle_recv_packet(self, packet: Optional[ProtocolMessage]) -> bool: def _handle_startDebugging_request(self, packet): response = { "type": "response", - "request_seq": packet.get("seq", 0), + "request_seq": packet["seq"], "success": True, "command": "startDebugging", "body": {} @@ -690,7 +690,7 @@ def request_attach( sourceMap: Optional[Union[list[tuple[str, str]], dict[str, str]]] = None, gdbRemotePort: Optional[int] = None, gdbRemoteHostname: Optional[str] = None, - targetIdx: Optional[int] = None, + targetId: Optional[int] = None, ): args_dict = {} if pid is not None: @@ -724,8 +724,8 @@ def request_attach( args_dict["gdb-remote-port"] = gdbRemotePort if gdbRemoteHostname is not None: args_dict["gdb-remote-hostname"] = gdbRemoteHostname - if targetIdx is not None: - args_dict["targetIdx"] = targetIdx + if targetId is not None: + args_dict["targetId"] = targetId command_dict = {"command": "attach", "type": "request", "arguments": args_dict} return self.send_recv(command_dict) @@ -1356,7 +1356,7 @@ def __init__( ): self.process = None self.connection = None - self.child_dap_sessions: list["DebugAdapterServer"] = [] # Track child sessions for cleanup + self.child_dap_sessions: Dict[int, "DebugAdapterServer"] = {} # Track child sessions for cleanup if executable is not None: process, connection = DebugAdapterServer.launch( @@ -1440,7 +1440,7 @@ def get_pid(self) -> int: return self.process.pid return -1 - def get_child_sessions(self) -> list["DebugAdapterServer"]: + def get_child_sessions(self) -> Dict[int, "DebugAdapterServer"]: return self.child_dap_sessions def _handle_startDebugging_request(self, packet): @@ -1457,22 +1457,21 @@ def _handle_startDebugging_request(self, packet): log_file=self.log_file ) - # Track the child session for proper cleanup - self.child_dap_sessions.append(child_dap) - - # Initialize the child DAP session - child_dap.request_initialize() - # Configure the child session based on the request type and configuration if request_type == 'attach': + # Initialize the child DAP session + child_dap.request_initialize() + # Extract attach-specific parameters attach_commands = configuration.get('attachCommands', []) - target_idx = configuration.get('targetIdx', None) + target_id = configuration.get('targetId', None) + # Track the child session for proper cleanup + self.child_dap_sessions[target_id] = child_dap # Send attach request to the child DAP child_dap.request_attach( attachCommands=attach_commands, - targetIdx=target_idx, + targetId=target_id, ) else: raise ValueError(f"Unsupported startDebugging request type: {request_type}") diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py index 855a09ffa3320..7c05234fdde0f 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py @@ -35,22 +35,22 @@ def create_debug_adapter( env=lldbDAPEnv, ) - def _get_dap_server(self, child_session_index: Optional[int] = None) -> dap_server.DebugAdapterServer: + def _get_dap_server(self, child_session_id: Optional[int] = None) -> dap_server.DebugAdapterServer: """Get a specific DAP server instance. Args: - child_session_index: Index of child session, or None for main session + child_session_id: Unique id of child session, or None for main session Returns: The requested DAP server instance """ - if child_session_index is None: + if child_session_id is None: return self.dap_server else: child_sessions = self.dap_server.get_child_sessions() - if child_session_index >= len(child_sessions): - raise IndexError(f"Child session index {child_session_index} out of range. Found {len(child_sessions)} child sessions.") - return child_sessions[child_session_index] + if child_session_id not in child_sessions: + raise IndexError(f"Child session id {child_session_id} not found.") + return child_sessions[child_session_id] def _set_source_breakpoints_impl(self, dap_server_instance, source_path, lines, data=None, wait_for_resolve=True): """Implementation for setting source breakpoints on any DAP server""" @@ -114,21 +114,21 @@ def _do_continue_impl(self, dap_server_instance): self.assertTrue(resp["success"], f"continue request failed: {resp}") # Multi-session methods for operating on specific sessions without switching context - def set_source_breakpoints_on(self, child_session_index: Optional[int], source_path, lines, data=None, wait_for_resolve=True): + def set_source_breakpoints_on(self, child_session_id: Optional[int], source_path, lines, data=None, wait_for_resolve=True): """Set source breakpoints on a specific DAP session without switching the active session.""" return self._set_source_breakpoints_impl( - self._get_dap_server(child_session_index), source_path, lines, data, wait_for_resolve + self._get_dap_server(child_session_id), source_path, lines, data, wait_for_resolve ) - def verify_breakpoint_hit_on(self, child_session_index: Optional[int], breakpoint_ids: list[str], timeout=DEFAULT_TIMEOUT): + def verify_breakpoint_hit_on(self, child_session_id: Optional[int], breakpoint_ids: list[str], timeout=DEFAULT_TIMEOUT): """Verify breakpoint hit on a specific DAP session without switching the active session.""" return self._verify_breakpoint_hit_impl( - self._get_dap_server(child_session_index), breakpoint_ids, timeout + self._get_dap_server(child_session_id), breakpoint_ids, timeout ) - def do_continue_on(self, child_session_index: Optional[int]): + def do_continue_on(self, child_session_id: Optional[int]): """Continue execution on a specific DAP session without switching the active session.""" - return self._do_continue_impl(self._get_dap_server(child_session_index)) + return self._do_continue_impl(self._get_dap_server(child_session_id)) def start_server(self, connection): """ diff --git a/lldb/test/API/tools/lldb-dap/gpu/Makefile b/lldb/test/API/tools/lldb-dap/gpu/amd/Makefile similarity index 100% rename from lldb/test/API/tools/lldb-dap/gpu/Makefile rename to lldb/test/API/tools/lldb-dap/gpu/amd/Makefile diff --git a/lldb/test/API/tools/lldb-dap/gpu/TestDAP_gpu_reverse_request.py b/lldb/test/API/tools/lldb-dap/gpu/amd/TestDAP_gpu_reverse_request.py similarity index 82% rename from lldb/test/API/tools/lldb-dap/gpu/TestDAP_gpu_reverse_request.py rename to lldb/test/API/tools/lldb-dap/gpu/amd/TestDAP_gpu_reverse_request.py index 58f3d47a94f36..8c63b3a186d56 100644 --- a/lldb/test/API/tools/lldb-dap/gpu/TestDAP_gpu_reverse_request.py +++ b/lldb/test/API/tools/lldb-dap/gpu/amd/TestDAP_gpu_reverse_request.py @@ -23,13 +23,13 @@ def skipUnlessHasROCm(func): def has_rocm(): if _detect_rocm() is None: - return "ROCm not available (rocm-smi not found)" + return "ROCm not available (rocminfo not found)" return None return skipTestIfFn(has_rocm)(func) -class TestDAP_gpu_reverse_request(lldbdap_testcase.DAPTestCaseBase): +class TestDAPAMDReverseRequest(lldbdap_testcase.DAPTestCaseBase): """Test DAP session spawning - both basic and GPU scenarios""" def setUp(self): @@ -60,19 +60,17 @@ def test_automatic_reverse_request_detection(self): self.assertIn("configuration", req["arguments"], "Reverse request should have configuration") attach_config = req["arguments"]["configuration"] - self.assertIn("attachCommands", attach_config, "Reverse request should have attachCommands") - self.assertIn("target select 1", attach_config["attachCommands"]) self.assertIn("name", attach_config, "Attach config should have name") - self.assertIn("GPU Session", attach_config["name"]) - self.assertIn("targetIdx", attach_config, "Attach config should have targetIdx") - self.assertEqual(attach_config["targetIdx"], 1, "Attach config should have targetIdx 1") + self.assertIn("AMD GPU Session", attach_config["name"]) + self.assertIn("targetId", attach_config, "Attach config should have targetId") + self.assertEqual(attach_config["targetId"], 2, "Attach config should have target id 2") @skipUnlessHasROCm def test_gpu_breakpoint_hit(self): """ Test that we can hit a breakpoint in GPU debugging session spawned through reverse requests. """ - GPU_SESSION_IDX = 0 + GPU_PROCESS_UNIQUE_ID = 2 self.build() log_file_path = self.getBuildArtifact("dap.txt") # Enable detailed DAP logging to debug any issues @@ -90,24 +88,24 @@ def test_gpu_breakpoint_hit(self): self.launch( program, disconnectAutomatically=False, ) + # Set CPU breakpoint and stop. breakpoint_ids = self.set_source_breakpoints(source, [cpu_breakpoint_line]) - self.continue_to_breakpoints(breakpoint_ids, timeout=self.DEFAULT_TIMEOUT) # We should have a GPU child session automatically spawned now self.assertEqual(len(self.dap_server.get_child_sessions()), 1) # Set breakpoint in GPU session - gpu_breakpoint_ids = self.set_source_breakpoints_on(GPU_SESSION_IDX, source, [gpu_breakpoint_line]) - + gpu_breakpoint_ids = self.set_source_breakpoints_on(GPU_PROCESS_UNIQUE_ID, source, [gpu_breakpoint_line]) # Resume GPU execution after verifying breakpoint hit - self.do_continue_on(0) - + self.do_continue_on(GPU_PROCESS_UNIQUE_ID) # Continue main session - self.do_continue() + self.do_continue() + self.dap_server.wait_for_stopped() + self.do_continue() # Verify that the GPU breakpoint is hit in the child session - self.verify_breakpoint_hit_on(GPU_SESSION_IDX, gpu_breakpoint_ids, timeout=self.DEFAULT_TIMEOUT * 2) + self.verify_breakpoint_hit_on(GPU_PROCESS_UNIQUE_ID, gpu_breakpoint_ids, timeout=self.DEFAULT_TIMEOUT * 3) # Manually disconnect sessions - for child_session in self.dap_server.get_child_sessions(): + for child_session in self.dap_server.get_child_sessions().values(): child_session.request_disconnect() self.dap_server.request_disconnect() diff --git a/lldb/test/API/tools/lldb-dap/gpu/hello_world.hip b/lldb/test/API/tools/lldb-dap/gpu/amd/hello_world.hip similarity index 99% rename from lldb/test/API/tools/lldb-dap/gpu/hello_world.hip rename to lldb/test/API/tools/lldb-dap/gpu/amd/hello_world.hip index 2c752819dbb94..63b1e68e0191d 100644 --- a/lldb/test/API/tools/lldb-dap/gpu/hello_world.hip +++ b/lldb/test/API/tools/lldb-dap/gpu/amd/hello_world.hip @@ -1,7 +1,9 @@ #include #include #include + constexpr int error_exit_code = -1; + #define HIP_CHECK(condition) \ { \ const hipError_t error = (condition); \ @@ -11,10 +13,12 @@ constexpr int error_exit_code = -1; exit(error_exit_code); \ } \ } + __global__ void add_one(int *data) { int idx = threadIdx.x; data[idx] = idx + 1; // GPU BREAKPOINT } + int main() { const int n = 4; int host_data[n] = {0, 0, 0, 0}; From 1868fe253436bc76ffb41235c05312731cbca5dc Mon Sep 17 00:00:00 2001 From: qxy11 Date: Wed, 10 Sep 2025 22:59:39 -0700 Subject: [PATCH 18/20] Add static convenience method for FindDAP + lint --- lldb/tools/lldb-dap/DAP.cpp | 9 +++------ lldb/tools/lldb-dap/DAPSessionManager.cpp | 10 ++++------ lldb/tools/lldb-dap/DAPSessionManager.h | 5 +++++ 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 56ebd824527de..017cb03965776 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -1375,8 +1375,7 @@ void DAP::EventThread() { 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::GetInstance().FindDAPForTarget( - process.GetTarget()); + DAP *dap_instance = DAPSessionManager::FindDAP(process.GetTarget()); if (!dap_instance) continue; @@ -1441,8 +1440,7 @@ void DAP::EventThread() { lldb::SBTarget event_target = lldb::SBTarget::GetTargetFromEvent(event); // Find the DAP instance that owns this target - DAP *dap_instance = - DAPSessionManager::GetInstance().FindDAPForTarget(event_target); + DAP *dap_instance = DAPSessionManager::FindDAP(event_target); if (!dap_instance) continue; @@ -1528,8 +1526,7 @@ void DAP::EventThread() { lldb::SBTarget event_target = bp.GetTarget(); // Find the DAP instance that owns this target - DAP *dap_instance = - DAPSessionManager::GetInstance().FindDAPForTarget(event_target); + DAP *dap_instance = DAPSessionManager::FindDAP(event_target); if (!dap_instance) continue; diff --git a/lldb/tools/lldb-dap/DAPSessionManager.cpp b/lldb/tools/lldb-dap/DAPSessionManager.cpp index fca78e241ffe9..7395f5b0ad16d 100644 --- a/lldb/tools/lldb-dap/DAPSessionManager.cpp +++ b/lldb/tools/lldb-dap/DAPSessionManager.cpp @@ -32,11 +32,9 @@ DAPSessionManager &DAPSessionManager::GetInstance() { // Use std::call_once for thread-safe initialization static std::once_flag initialized; static DAPSessionManager *instance = nullptr; - - std::call_once(initialized, []() { - instance = new DAPSessionManager(); - }); - + + std::call_once(initialized, []() { instance = new DAPSessionManager(); }); + return *instance; } @@ -95,7 +93,7 @@ DAPSessionManager::GetEventThreadForDebugger(lldb::SBDebugger debugger, // Try to use shared event thread, if it exists if (auto it = m_debugger_event_threads.find(debugger_id); it != m_debugger_event_threads.end()) { - if (auto thread_sp = it->second.lock()) { + if (auto thread_sp = it->second.lock()) { return thread_sp; } // Our weak pointer has expired diff --git a/lldb/tools/lldb-dap/DAPSessionManager.h b/lldb/tools/lldb-dap/DAPSessionManager.h index 507bf34e5dd38..9eb54f2ffa00f 100644 --- a/lldb/tools/lldb-dap/DAPSessionManager.h +++ b/lldb/tools/lldb-dap/DAPSessionManager.h @@ -75,6 +75,11 @@ class DAPSessionManager { /// Find the DAP instance that owns the given target DAP *FindDAPForTarget(lldb::SBTarget target); + /// Static convenience method for FindDAPForTarget + static DAP *FindDAP(lldb::SBTarget target) { + return GetInstance().FindDAPForTarget(target); + } + /// Clean up shared resources when the last session exits void CleanupSharedResources(); From ffb3a537db4d6aeb89ae557bf63fb218a1334c02 Mon Sep 17 00:00:00 2001 From: qxy11 Date: Fri, 19 Sep 2025 10:35:15 -0700 Subject: [PATCH 19/20] Unique target ids + dap_session_name -> session_name --- lldb/include/lldb/API/SBTarget.h | 2 +- lldb/include/lldb/Target/Target.h | 17 ++++--- .../lldb/Utility/GPUGDBRemotePackets.h | 2 +- lldb/source/API/SBTarget.cpp | 4 +- .../Process/gdb-remote/ProcessGDBRemote.cpp | 2 +- lldb/source/Target/Target.cpp | 12 ++--- lldb/source/Target/TargetList.cpp | 50 ++++++++++--------- lldb/source/Utility/GPUGDBRemotePackets.cpp | 4 +- lldb/tools/lldb-dap/DAP.cpp | 2 +- .../Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp | 4 +- .../Plugins/AMDGPU/LLDBServerPluginAMDGPU.h | 2 +- .../MockGPU/LLDBServerPluginMockGPU.cpp | 2 +- 12 files changed, 55 insertions(+), 48 deletions(-) diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index e82a097a7d8fc..d7ea9dc4ac712 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -70,7 +70,7 @@ class LLDB_API SBTarget { static lldb::SBModule GetModuleAtIndexFromEvent(const uint32_t idx, const lldb::SBEvent &event); - static const char *GetDAPSessionNameFromEvent(const SBEvent &event); + static const char *GetSessionNameFromEvent(const SBEvent &event); static const char *GetBroadcasterClassName(); diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index af7da77d8ecdc..45135d8cf3044 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -556,12 +556,10 @@ class Target : public std::enable_shared_from_this, TargetEventData(const lldb::TargetSP &target_sp, const ModuleList &module_list); - TargetEventData(const lldb::TargetSP &target_sp, - std::string dap_session_name); + TargetEventData(const lldb::TargetSP &target_sp, std::string session_name); TargetEventData(const lldb::TargetSP &target_sp, - const ModuleList &module_list, - std::string dap_session_name); + const ModuleList &module_list, std::string session_name); ~TargetEventData() override; @@ -571,7 +569,7 @@ class Target : public std::enable_shared_from_this, return TargetEventData::GetFlavorString(); } - static llvm::StringRef GetDAPSessionNameFromEvent(const Event *event_ptr); + static llvm::StringRef GetSessionNameFromEvent(const Event *event_ptr); void Dump(Stream *s) const override; @@ -588,7 +586,7 @@ class Target : public std::enable_shared_from_this, private: lldb::TargetSP m_target_sp; ModuleList m_module_list; - std::string m_dap_session_name = ""; + std::string m_session_name = ""; TargetEventData(const TargetEventData &) = delete; const TargetEventData &operator=(const TargetEventData &) = delete; @@ -610,6 +608,12 @@ class Target : public std::enable_shared_from_this, bool IsDummyTarget() const { return m_is_dummy_target; } + /// Get the unique ID for this target. + /// + /// \return + /// The unique ID for this target, or 0 if no ID has been assigned. + uint32_t GetUniqueID() const { return m_target_unique_id; } + const std::string &GetLabel() const { return m_label; } /// Set a label for a target. @@ -1672,6 +1676,7 @@ class Target : public std::enable_shared_from_this, bool m_suppress_stop_hooks; /// Used to not run stop hooks for expressions bool m_is_dummy_target; unsigned m_next_persistent_variable_index = 0; + uint32_t m_target_unique_id = 0; /// The unique ID assigned to this target /// An optional \a lldb_private::Trace object containing processor trace /// information of this target. lldb::TraceSP m_trace_sp; diff --git a/lldb/include/lldb/Utility/GPUGDBRemotePackets.h b/lldb/include/lldb/Utility/GPUGDBRemotePackets.h index 718bd918c3bf4..2d27efb327264 100644 --- a/lldb/include/lldb/Utility/GPUGDBRemotePackets.h +++ b/lldb/include/lldb/Utility/GPUGDBRemotePackets.h @@ -178,7 +178,7 @@ struct GPUActions { /// The name of the plugin. std::string plugin_name; /// The name to give a DAP session - std::string dap_session_name; + std::string session_name; /// New breakpoints to set. Nothing to set if this is empty. std::vector breakpoints; /// If a GPU connection is available return a connect URL to use to reverse diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index fb5213744d8c3..de3dfce85ef86 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -145,11 +145,11 @@ SBModule SBTarget::GetModuleAtIndexFromEvent(const uint32_t idx, return SBModule(module_list.GetModuleAtIndex(idx)); } -const char *SBTarget::GetDAPSessionNameFromEvent(const SBEvent &event) { +const char *SBTarget::GetSessionNameFromEvent(const SBEvent &event) { LLDB_INSTRUMENT_VA(event); return ConstString( - Target::TargetEventData::GetDAPSessionNameFromEvent(event.get())) + Target::TargetEventData::GetSessionNameFromEvent(event.get())) .AsCString(); } diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index b99047816b20f..870987baa7cce 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -1027,7 +1027,7 @@ Status ProcessGDBRemote::HandleConnectionRequest(const GPUActions &gpu_action) { "created process!!!"); auto event_sp = std::make_shared( Target::eBroadcastBitNewTargetCreated, - new Target::TargetEventData(gpu_target_sp, gpu_action.dap_session_name)); + new Target::TargetEventData(gpu_target_sp, gpu_action.session_name)); GetTarget().BroadcastEvent(event_sp); return Status(); } diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 052902e202e39..118f24d102ae0 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -5137,14 +5137,14 @@ Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp, : TargetEventData(target_sp, module_list, "") {} Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp, - std::string dap_session_name) - : TargetEventData(target_sp, ModuleList(), std::move(dap_session_name)) {} + 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 dap_session_name) + std::string session_name) : EventData(), m_target_sp(target_sp), m_module_list(module_list), - m_dap_session_name(std::move(dap_session_name)) {} + m_session_name(std::move(session_name)) {} Target::TargetEventData::~TargetEventData() = default; @@ -5181,10 +5181,10 @@ TargetSP Target::TargetEventData::GetTargetFromEvent(const Event *event_ptr) { } llvm::StringRef -Target::TargetEventData::GetDAPSessionNameFromEvent(const Event *event_ptr) { +Target::TargetEventData::GetSessionNameFromEvent(const Event *event_ptr) { const TargetEventData *event_data = GetEventDataFromEvent(event_ptr); if (event_data) - return event_data->m_dap_session_name; + return event_data->m_session_name; return llvm::StringRef(); } diff --git a/lldb/source/Target/TargetList.cpp b/lldb/source/Target/TargetList.cpp index 3bd655eeb680d..8497cf17e2141 100644 --- a/lldb/source/Target/TargetList.cpp +++ b/lldb/source/Target/TargetList.cpp @@ -176,7 +176,7 @@ Status TargetList::CreateTargetInternal( module_spec.GetArchitecture() = arch; if (module_specs.FindMatchingModuleSpec(module_spec, matching_module_spec)) - update_platform_arch(matching_module_spec.GetArchitecture()); + update_platform_arch(matching_module_spec.GetArchitecture()); } else { // Fat binary. No architecture specified, check if there is // only one platform for all of the architectures. @@ -256,6 +256,9 @@ Status TargetList::CreateTargetInternal(Debugger &debugger, Status error; const bool is_dummy_target = false; + // Global static counter for assigning unique IDs to targets + static uint32_t g_target_unique_id = 0; + ArchSpec arch(specified_arch); if (arch.IsValid()) { @@ -294,7 +297,7 @@ Status TargetList::CreateTargetInternal(Debugger &debugger, if (file.IsRelative() && !user_exe_path.empty()) { llvm::SmallString<64> cwd; - if (! llvm::sys::fs::current_path(cwd)) { + if (!llvm::sys::fs::current_path(cwd)) { FileSpec cwd_file(cwd.c_str()); cwd_file.AppendPathComponent(file); if (FileSystem::Instance().Exists(cwd_file)) @@ -344,6 +347,8 @@ Status TargetList::CreateTargetInternal(Debugger &debugger, if (!target_sp) return error; + target_sp->m_target_unique_id = ++g_target_unique_id; + // Set argv0 with what the user typed, unless the user specified a // directory. If the user specified a directory, then it is probably a // bundle that was resolved and we need to use the resolved bundle path @@ -431,8 +436,7 @@ TargetSP TargetList::FindTargetWithProcess(Process *process) const { TargetSP TargetList::FindTargetWithUniqueID(uint32_t id) const { std::lock_guard guard(m_target_list_mutex); auto it = llvm::find_if(m_target_list, [id](const TargetSP &item) { - auto *process_ptr = item->GetProcessSP().get(); - return process_ptr && (process_ptr->GetUniqueID() == id); + return item->GetUniqueID() == id; }); if (it != m_target_list.end()) @@ -567,29 +571,27 @@ bool TargetList::AnyTargetContainsModule(Module &module) { if (target_sp->GetImages().FindModule(&module)) return true; } - for (const auto &target_sp: m_in_process_target_list) { + for (const auto &target_sp : m_in_process_target_list) { if (target_sp->GetImages().FindModule(&module)) return true; } return false; } - void TargetList::RegisterInProcessTarget(TargetSP target_sp) { - std::lock_guard guard(m_target_list_mutex); - [[maybe_unused]] bool was_added; - std::tie(std::ignore, was_added) = - m_in_process_target_list.insert(target_sp); - assert(was_added && "Target pointer was left in the in-process map"); - } - - void TargetList::UnregisterInProcessTarget(TargetSP target_sp) { - std::lock_guard guard(m_target_list_mutex); - [[maybe_unused]] bool was_present = - m_in_process_target_list.erase(target_sp); - assert(was_present && "Target pointer being removed was not registered"); - } - - bool TargetList::IsTargetInProcess(TargetSP target_sp) { - std::lock_guard guard(m_target_list_mutex); - return m_in_process_target_list.count(target_sp) == 1; - } +void TargetList::RegisterInProcessTarget(TargetSP target_sp) { + std::lock_guard guard(m_target_list_mutex); + [[maybe_unused]] bool was_added; + std::tie(std::ignore, was_added) = m_in_process_target_list.insert(target_sp); + assert(was_added && "Target pointer was left in the in-process map"); +} + +void TargetList::UnregisterInProcessTarget(TargetSP target_sp) { + std::lock_guard guard(m_target_list_mutex); + [[maybe_unused]] bool was_present = m_in_process_target_list.erase(target_sp); + assert(was_present && "Target pointer being removed was not registered"); +} + +bool TargetList::IsTargetInProcess(TargetSP target_sp) { + std::lock_guard guard(m_target_list_mutex); + return m_in_process_target_list.count(target_sp) == 1; +} diff --git a/lldb/source/Utility/GPUGDBRemotePackets.cpp b/lldb/source/Utility/GPUGDBRemotePackets.cpp index 1aa1302e17362..5e8c416971d3b 100644 --- a/lldb/source/Utility/GPUGDBRemotePackets.cpp +++ b/lldb/source/Utility/GPUGDBRemotePackets.cpp @@ -139,7 +139,7 @@ bool fromJSON(const llvm::json::Value &value, GPUActions &data, llvm::json::Path path) { ObjectMapper o(value, path); return o && o.map("plugin_name", data.plugin_name) && - o.map("dap_session_name", data.dap_session_name) && + o.map("session_name", data.session_name) && o.map("breakpoints", data.breakpoints) && o.mapOptional("connect_info", data.connect_info) && o.map("load_libraries", data.load_libraries) && @@ -151,7 +151,7 @@ bool fromJSON(const llvm::json::Value &value, GPUActions &data, llvm::json::Value toJSON(const GPUActions &data) { return json::Value(Object{ {"plugin_name", data.plugin_name}, - {"dap_session_name", data.dap_session_name}, + {"session_name", data.session_name}, {"breakpoints", data.breakpoints}, {"connect_info", data.connect_info}, {"load_libraries", data.load_libraries}, diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 017cb03965776..419c193cb5549 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -1498,7 +1498,7 @@ void DAP::EventThread() { attach_config.try_emplace("type", "lldb"); attach_config.try_emplace("targetId", target_id); const char *session_name = - lldb::SBTarget::GetDAPSessionNameFromEvent(event); + lldb::SBTarget::GetSessionNameFromEvent(event); if (session_name && *session_name) { attach_config.try_emplace("name", session_name); } else { diff --git a/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp b/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp index b012359aa1231..c7cca1b416a14 100644 --- a/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp +++ b/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.cpp @@ -147,7 +147,7 @@ LLDBServerPluginAMDGPU::~LLDBServerPluginAMDGPU() { CloseFDs(); } llvm::StringRef LLDBServerPluginAMDGPU::GetPluginName() { return "amd-gpu"; } -llvm::StringRef LLDBServerPluginAMDGPU::GetDAPSessionName() { +llvm::StringRef LLDBServerPluginAMDGPU::GetSessionName() { return "AMD GPU Session"; } @@ -420,7 +420,7 @@ std::optional LLDBServerPluginAMDGPU::NativeProcessIsStopping() { } actions.connect_info = CreateConnection(); actions.connect_info->synchronous = true; - actions.dap_session_name = GetDAPSessionName(); + actions.session_name = GetSessionName(); } return actions; } else { diff --git a/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.h b/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.h index cdf1780dca4fc..2f0c9dd15b1b5 100644 --- a/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.h +++ b/lldb/tools/lldb-server/Plugins/AMDGPU/LLDBServerPluginAMDGPU.h @@ -68,7 +68,7 @@ class LLDBServerPluginAMDGPU : public LLDBServerPlugin { } bool CreateGPUBreakpoint(uint64_t addr); - llvm::StringRef GetDAPSessionName(); + llvm::StringRef GetSessionName(); // TODO: make this private struct GPUInternalBreakpoinInfo { diff --git a/lldb/tools/lldb-server/Plugins/MockGPU/LLDBServerPluginMockGPU.cpp b/lldb/tools/lldb-server/Plugins/MockGPU/LLDBServerPluginMockGPU.cpp index f49fd44770ef3..58d5a40587922 100644 --- a/lldb/tools/lldb-server/Plugins/MockGPU/LLDBServerPluginMockGPU.cpp +++ b/lldb/tools/lldb-server/Plugins/MockGPU/LLDBServerPluginMockGPU.cpp @@ -173,7 +173,7 @@ LLDBServerPluginMockGPU::BreakpointWasHit(GPUPluginBreakpointHitArgs &args) { LLDB_LOGF(log, "LLDBServerPluginMockGPU::BreakpointWasHit(\"%s\") disabling breakpoint", bp_identifier.c_str()); response.actions.connect_info = CreateConnection(); - response.actions.dap_session_name = "Mock GPU Session"; + response.actions.session_name = "Mock GPU Session"; // We asked for the symbol "gpu_shlib_load" to be delivered as a symbol // value when the "gpu_initialize" breakpoint was set. So we will use this From 01d14b1fa217350c2adfc35eeef771c9a92d8405 Mon Sep 17 00:00:00 2001 From: Janet Yang Date: Fri, 19 Sep 2025 10:35:16 -0700 Subject: [PATCH 20/20] Fix attach request handling --- .../tools/lldb-dap/gpu/amd/TestDAP_gpu_reverse_request.py | 2 -- lldb/tools/lldb-dap/DAP.cpp | 1 - lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp | 6 +++--- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/gpu/amd/TestDAP_gpu_reverse_request.py b/lldb/test/API/tools/lldb-dap/gpu/amd/TestDAP_gpu_reverse_request.py index 8c63b3a186d56..2a6963b9d8d4e 100644 --- a/lldb/test/API/tools/lldb-dap/gpu/amd/TestDAP_gpu_reverse_request.py +++ b/lldb/test/API/tools/lldb-dap/gpu/amd/TestDAP_gpu_reverse_request.py @@ -100,8 +100,6 @@ def test_gpu_breakpoint_hit(self): self.do_continue_on(GPU_PROCESS_UNIQUE_ID) # Continue main session self.do_continue() - self.dap_server.wait_for_stopped() - self.do_continue() # Verify that the GPU breakpoint is hit in the child session self.verify_breakpoint_hit_on(GPU_PROCESS_UNIQUE_ID, gpu_breakpoint_ids, timeout=self.DEFAULT_TIMEOUT * 3) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 419c193cb5549..ebb19f11530ba 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -1491,7 +1491,6 @@ void DAP::EventThread() { // 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; - llvm::json::Array attach_commands; // If we have a process name, add command to attach to the same // process name diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp index e6293a882d7e0..224af5fee3528 100644 --- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp @@ -86,10 +86,10 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { if (error.Fail()) return ToError(error); dap.SetTarget(target); - // Run any pre run LLDB commands the user specified in the launch.json - if (Error err = dap.RunPreRunCommands()) + if (Error err = dap.RunPreRunCommands()) { return err; + } if ((args.pid == LLDB_INVALID_PROCESS_ID || args.gdbRemotePort == LLDB_DAP_INVALID_PORT) && @@ -131,7 +131,7 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { connect_url += std::to_string(args.gdbRemotePort); dap.target.ConnectRemote(listener, connect_url.c_str(), "gdb-remote", error); - } else { + } else if (!target_id.has_value()) { // Attach by pid or process name. lldb::SBAttachInfo attach_info; if (args.pid != LLDB_INVALID_PROCESS_ID)