llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Ebuka Ezike (da-viper) <details> <summary>Changes</summary> Handle each event type in a different function --- Full diff: https://github.com/llvm/llvm-project/pull/166948.diff 2 Files Affected: - (modified) lldb/tools/lldb-dap/DAP.cpp (+174-156) - (modified) lldb/tools/lldb-dap/DAP.h (+4) ``````````diff diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index f009a902f79e7..7534e76e885b9 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -1317,7 +1317,7 @@ void DAP::ProgressEventThread() { lldb::SBEvent event; bool done = false; while (!done) { - if (listener.WaitForEvent(1, event)) { + if (listener.WaitForEvent(UINT32_MAX, event)) { const auto event_mask = event.GetType(); if (event.BroadcasterMatchesRef(broadcaster)) { if (event_mask & eBroadcastBitStopProgressThread) { @@ -1375,7 +1375,6 @@ void DAP::ProgressEventThread() { // is required. void DAP::EventThread() { llvm::set_thread_name("lldb.DAP.client." + m_client_name + ".event_handler"); - lldb::SBEvent event; lldb::SBListener listener = debugger.GetListener(); broadcaster.AddListener(listener, eBroadcastBitStopEventThread); debugger.GetBroadcaster().AddListener( @@ -1386,169 +1385,176 @@ void DAP::EventThread() { debugger, lldb::SBThread::GetBroadcasterClassName(), lldb::SBThread::eBroadcastBitStackChanged); + lldb::SBEvent event; bool done = false; while (!done) { - if (listener.WaitForEvent(1, event)) { - const auto event_mask = event.GetType(); - if (lldb::SBProcess::EventIsProcessEvent(event)) { - lldb::SBProcess process = lldb::SBProcess::GetProcessFromEvent(event); - if (event_mask & lldb::SBProcess::eBroadcastBitStateChanged) { - auto state = lldb::SBProcess::GetStateFromEvent(event); - switch (state) { - case lldb::eStateConnected: - case lldb::eStateDetached: - case lldb::eStateInvalid: - case lldb::eStateUnloaded: - break; - case lldb::eStateAttaching: - case lldb::eStateCrashed: - case lldb::eStateLaunching: - case lldb::eStateStopped: - case lldb::eStateSuspended: - // 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), - "({1}) reporting thread stopped: {0}", - m_client_name); - } - break; - case lldb::eStateRunning: - case lldb::eStateStepping: - WillContinue(); - SendContinuedEvent(*this); - break; - case lldb::eStateExited: - lldb::SBStream stream; - process.GetStatus(stream); - 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; - } else { - // Run any exit LLDB commands the user specified in the - // launch.json - RunExitCommands(); - SendProcessExitedEvent(*this, process); - SendTerminatedEvent(); - done = true; - } - break; - } - } else if ((event_mask & lldb::SBProcess::eBroadcastBitSTDOUT) || - (event_mask & lldb::SBProcess::eBroadcastBitSTDERR)) { - SendStdOutStdErr(*this, 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) { - const uint32_t num_modules = - lldb::SBTarget::GetNumModulesFromEvent(event); - const bool remove_module = - event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded; - - // NOTE: Both mutexes must be acquired to prevent deadlock when - // handling `modules_request`, which also requires both locks. - lldb::SBMutex api_mutex = GetAPIMutex(); - const std::scoped_lock<lldb::SBMutex, std::mutex> guard( - api_mutex, modules_mutex); - for (uint32_t i = 0; i < num_modules; ++i) { - lldb::SBModule module = - lldb::SBTarget::GetModuleAtIndexFromEvent(i, event); - - std::optional<protocol::Module> p_module = - CreateModule(target, module, remove_module); - if (!p_module) - continue; - - llvm::StringRef module_id = p_module->id; - - const bool module_exists = modules.contains(module_id); - if (remove_module && module_exists) { - modules.erase(module_id); - Send(protocol::Event{ - "module", ModuleEventBody{std::move(p_module).value(), - ModuleEventBody::eReasonRemoved}}); - } else if (module_exists) { - Send(protocol::Event{ - "module", ModuleEventBody{std::move(p_module).value(), - ModuleEventBody::eReasonChanged}}); - } else if (!remove_module) { - modules.insert(module_id); - Send(protocol::Event{ - "module", ModuleEventBody{std::move(p_module).value(), - ModuleEventBody::eReasonNew}}); - } - } - } - } else if (lldb::SBBreakpoint::EventIsBreakpointEvent(event)) { - if (event_mask & lldb::SBTarget::eBroadcastBitBreakpointChanged) { - auto event_type = - lldb::SBBreakpoint::GetBreakpointEventTypeFromEvent(event); - auto bp = Breakpoint( - *this, lldb::SBBreakpoint::GetBreakpointFromEvent(event)); - // 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 - // going away and the reason is always "changed". - if ((event_type & lldb::eBreakpointEventTypeLocationsAdded || - event_type & lldb::eBreakpointEventTypeLocationsRemoved || - event_type & lldb::eBreakpointEventTypeLocationsResolved) && - bp.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. - protocol::Breakpoint protocol_bp = bp.ToProtocolBreakpoint(); - - // "source" is not needed here, unless we add adapter data to be - // saved by the client. - if (protocol_bp.source && !protocol_bp.source->adapterData) - protocol_bp.source = std::nullopt; - - llvm::json::Object body; - body.try_emplace("breakpoint", protocol_bp); - body.try_emplace("reason", "changed"); - - llvm::json::Object bp_event = CreateEventObject("breakpoint"); - bp_event.try_emplace("body", std::move(body)); - - SendJSON(llvm::json::Value(std::move(bp_event))); - } - } + if (!listener.WaitForEvent(UINT32_MAX, event)) + continue; - } else if (lldb::SBThread::EventIsThreadEvent(event)) { - HandleThreadEvent(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()); - } else if (event.BroadcasterMatchesRef(broadcaster)) { - if (event_mask & eBroadcastBitStopEventThread) { - done = true; - } + const uint32_t event_mask = event.GetType(); + if (lldb::SBProcess::EventIsProcessEvent(event)) { + HandleProcessEvent(event, done); + } else if (lldb::SBTarget::EventIsTargetEvent(event)) { + HandleTargetEvent(event); + } else if (lldb::SBBreakpoint::EventIsBreakpointEvent(event)) { + HandleBreakpointEvent(event); + } else if (lldb::SBThread::EventIsThreadEvent(event)) { + HandleThreadEvent(event); + } else if (event_mask & lldb::eBroadcastBitError || + event_mask & lldb::eBroadcastBitWarning) { + HandleDiagnosticEvent(event); + } else if (event.BroadcasterMatchesRef(broadcaster)) { + if (event_mask & eBroadcastBitStopEventThread) { + done = true; + } + } + } +} + +void DAP::HandleProcessEvent(const lldb::SBEvent &event, bool &process_exited) { + lldb::SBProcess process = lldb::SBProcess::GetProcessFromEvent(event); + const uint32_t event_mask = event.GetType(); + if (event_mask & lldb::SBProcess::eBroadcastBitStateChanged) { + auto state = lldb::SBProcess::GetStateFromEvent(event); + switch (state) { + case lldb::eStateConnected: + case lldb::eStateDetached: + case lldb::eStateInvalid: + case lldb::eStateUnloaded: + break; + case lldb::eStateAttaching: + case lldb::eStateCrashed: + case lldb::eStateLaunching: + case lldb::eStateStopped: + case lldb::eStateSuspended: + // 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), + "({1}) reporting thread stopped: {0}", m_client_name); + } + break; + case lldb::eStateRunning: + case lldb::eStateStepping: + WillContinue(); + SendContinuedEvent(*this); + break; + case lldb::eStateExited: + lldb::SBStream stream; + process.GetStatus(stream); + 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; + } else { + // Run any exit LLDB commands the user specified in the + // launch.json + RunExitCommands(); + SendProcessExitedEvent(*this, process); + SendTerminatedEvent(); + process_exited = true; + } + break; + } + } else if ((event_mask & lldb::SBProcess::eBroadcastBitSTDOUT) || + (event_mask & lldb::SBProcess::eBroadcastBitSTDERR)) { + SendStdOutStdErr(*this, process); + } +} + +void DAP::HandleTargetEvent(const lldb::SBEvent &event) { + const uint32_t event_mask = event.GetType(); + if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded || + event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded || + event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded || + event_mask & lldb::SBTarget::eBroadcastBitSymbolsChanged) { + const uint32_t num_modules = lldb::SBTarget::GetNumModulesFromEvent(event); + const bool remove_module = + event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded; + + // NOTE: Both mutexes must be acquired to prevent deadlock when + // handling `modules_request`, which also requires both locks. + lldb::SBMutex api_mutex = GetAPIMutex(); + const std::scoped_lock<lldb::SBMutex, std::mutex> guard(api_mutex, + modules_mutex); + for (uint32_t i = 0; i < num_modules; ++i) { + lldb::SBModule module = + lldb::SBTarget::GetModuleAtIndexFromEvent(i, event); + + std::optional<protocol::Module> p_module = + CreateModule(target, module, remove_module); + if (!p_module) + continue; + + const llvm::StringRef module_id = p_module->id; + + const bool module_exists = modules.contains(module_id); + if (remove_module && module_exists) { + modules.erase(module_id); + Send(protocol::Event{"module", + ModuleEventBody{std::move(p_module).value(), + ModuleEventBody::eReasonRemoved}}); + } else if (module_exists) { + Send(protocol::Event{"module", + ModuleEventBody{std::move(p_module).value(), + ModuleEventBody::eReasonChanged}}); + } else if (!remove_module) { + modules.insert(module_id); + Send(protocol::Event{"module", + ModuleEventBody{std::move(p_module).value(), + ModuleEventBody::eReasonNew}}); } } } } +void DAP::HandleBreakpointEvent(const lldb::SBEvent &event) { + const uint32_t event_mask = event.GetType(); + if (!(event_mask & lldb::SBTarget::eBroadcastBitBreakpointChanged)) + return; + + auto event_type = lldb::SBBreakpoint::GetBreakpointEventTypeFromEvent(event); + auto bp = + Breakpoint(*this, lldb::SBBreakpoint::GetBreakpointFromEvent(event)); + // 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 + // going away and the reason is always "changed". + if ((event_type & lldb::eBreakpointEventTypeLocationsAdded || + event_type & lldb::eBreakpointEventTypeLocationsRemoved || + event_type & lldb::eBreakpointEventTypeLocationsResolved) && + bp.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. + protocol::Breakpoint protocol_bp = bp.ToProtocolBreakpoint(); + + // "source" is not needed here, unless we add adapter data to be + // saved by the client. + if (protocol_bp.source && !protocol_bp.source->adapterData) + protocol_bp.source = std::nullopt; + + llvm::json::Object body; + body.try_emplace("breakpoint", protocol_bp); + body.try_emplace("reason", "changed"); + + llvm::json::Object bp_event = CreateEventObject("breakpoint"); + bp_event.try_emplace("body", std::move(body)); + + SendJSON(llvm::json::Value(std::move(bp_event))); + } +} + void DAP::HandleThreadEvent(const lldb::SBEvent &event) { - uint32_t event_type = event.GetType(); + const uint32_t event_type = event.GetType(); if (event_type & lldb::SBThread::eBroadcastBitStackChanged) { const lldb::SBThread evt_thread = lldb::SBThread::GetThreadFromEvent(event); @@ -1557,6 +1563,18 @@ void DAP::HandleThreadEvent(const lldb::SBEvent &event) { } } +void DAP::HandleDiagnosticEvent(const lldb::SBEvent &event) { + const lldb::SBStructuredData data = + lldb::SBDebugger::GetDiagnosticFromEvent(event); + if (!data.IsValid()) + return; + + std::string type = GetStringValue(data.GetValueForKey("type")); + std::string message = GetStringValue(data.GetValueForKey("message")); + SendOutput(OutputType::Important, + llvm::formatv("{0}: {1}", type, message).str()); +} + std::vector<protocol::Breakpoint> DAP::SetSourceBreakpoints( const protocol::Source &source, const std::optional<std::vector<protocol::SourceBreakpoint>> &breakpoints) { diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index b4f111e4e720c..5d40341329f34 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -454,7 +454,11 @@ struct DAP final : public DAPTransport::MessageHandler { /// Event threads. /// @{ void EventThread(); + void HandleProcessEvent(const lldb::SBEvent &event, bool &process_exited); + void HandleTargetEvent(const lldb::SBEvent &event); + void HandleBreakpointEvent(const lldb::SBEvent &event); void HandleThreadEvent(const lldb::SBEvent &event); + void HandleDiagnosticEvent(const lldb::SBEvent &event); void ProgressEventThread(); std::thread event_thread; `````````` </details> https://github.com/llvm/llvm-project/pull/166948 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
