From d972a36748861e4c9c6f24d686e2895f3ebe61a5 Mon Sep 17 00:00:00 2001 From: Xusheng Date: Thu, 23 Mar 2023 11:25:17 +0800 Subject: [PATCH] Make the launch operation async for LLDB --- core/adapters/lldbadapter.cpp | 40 ++++++++++-------- core/debuggercontroller.cpp | 77 ++++++++++++++++------------------- core/debuggercontroller.h | 3 +- 3 files changed, 58 insertions(+), 62 deletions(-) diff --git a/core/adapters/lldbadapter.cpp b/core/adapters/lldbadapter.cpp index 1a89583d..2c5eadc1 100644 --- a/core/adapters/lldbadapter.cpp +++ b/core/adapters/lldbadapter.cpp @@ -157,7 +157,12 @@ bool LldbAdapter::Execute(const std::string& path, const LaunchConfigurations& c bool LldbAdapter::ExecuteWithArgs(const std::string& path, const std::string& args, const std::string& workingDir, const LaunchConfigurations& configs) { - m_debugger.SetAsync(false); + m_debugger.SetAsync(true); + + // We must start the event listener before calling CreateTarget, since CreateTarget will send out the initial + // batch of module load events. + std::thread thread([&]() { EventListener(); }); + thread.detach(); SBError err; @@ -198,9 +203,6 @@ bool LldbAdapter::ExecuteWithArgs(const std::string& path, const std::string& ar // target. ApplyBreakpoints(); - std::thread thread([&]() { EventListener(); }); - thread.detach(); - if (Settings::Instance()->Get("debugger.stopAtEntryPoint") && m_hasEntryFunction) AddBreakpoint(ModuleNameAndOffset(configs.inputFile, m_entryPoint - m_start)); @@ -224,7 +226,7 @@ bool LldbAdapter::ExecuteWithArgs(const std::string& path, const std::string& ar PostDebuggerEvent(evt); m_process = m_target.GetProcess(); - if (!m_process.IsValid() || (m_process.GetState() != StateType::eStateStopped) || (result.rfind("error: ", 0) == 0)) + if (!m_process.IsValid() || (m_process.GetState() == StateType::eStateInvalid) || (result.rfind("error: ", 0) == 0)) { auto it = result.find_last_not_of('\n'); result.erase(it + 1); @@ -235,14 +237,16 @@ bool LldbAdapter::ExecuteWithArgs(const std::string& path, const std::string& ar PostDebuggerEvent(event); return false; } - m_debugger.SetAsync(true); return true; } bool LldbAdapter::Attach(std::uint32_t pid) { - m_debugger.SetAsync(false); + m_debugger.SetAsync(true); + + std::thread thread([&]() { EventListener(); }); + thread.detach(); SBError err; @@ -279,12 +283,9 @@ bool LldbAdapter::Attach(std::uint32_t pid) m_targetActive = true; ApplyBreakpoints(); - std::thread thread([&]() { EventListener(); }); - thread.detach(); - SBAttachInfo info(pid); m_process = m_target.Attach(info, err); - if (!m_process.IsValid() || (m_process.GetState() != StateType::eStateStopped) || err.Fail()) + if (!m_process.IsValid() || (m_process.GetState() == StateType::eStateInvalid) || err.Fail()) { DebuggerEvent event; event.type = ErrorEventType; @@ -294,14 +295,21 @@ bool LldbAdapter::Attach(std::uint32_t pid) PostDebuggerEvent(event); return false; } - m_debugger.SetAsync(true); + + DebuggerEvent dbgevt; + dbgevt.type = TargetStoppedEventType; + dbgevt.data.targetStoppedData.reason = InitialBreakpoint; + PostDebuggerEvent(dbgevt); return true; } bool LldbAdapter::Connect(const std::string& server, std::uint32_t port) { - m_debugger.SetAsync(false); + m_debugger.SetAsync(true); + + std::thread thread([&]() { EventListener(); }); + thread.detach(); SBError err; @@ -338,9 +346,6 @@ bool LldbAdapter::Connect(const std::string& server, std::uint32_t port) m_targetActive = true; ApplyBreakpoints(); - std::thread thread([&]() { EventListener(); }); - thread.detach(); - if (Settings::Instance()->Get("debugger.stopAtEntryPoint") && m_hasEntryFunction) AddBreakpoint(ModuleNameAndOffset(m_originalFileName, m_entryPoint - m_start)); @@ -350,7 +355,7 @@ bool LldbAdapter::Connect(const std::string& server, std::uint32_t port) if (!m_processPlugin.empty() && m_processPlugin != "debugserver/lldb") plugin = m_processPlugin.c_str(); m_process = m_target.ConnectRemote(listener, url.c_str(), plugin, err); - if (!m_process.IsValid() || (m_process.GetState() != StateType::eStateStopped) || err.Fail()) + if (!m_process.IsValid() || (m_process.GetState() == StateType::eStateInvalid) || err.Fail()) { DebuggerEvent event; event.type = ErrorEventType; @@ -360,7 +365,6 @@ bool LldbAdapter::Connect(const std::string& server, std::uint32_t port) PostDebuggerEvent(event); return false; } - m_debugger.SetAsync(true); return true; } diff --git a/core/debuggercontroller.cpp b/core/debuggercontroller.cpp index fdbc1246..43ddfa96 100644 --- a/core/debuggercontroller.cpp +++ b/core/debuggercontroller.cpp @@ -119,21 +119,12 @@ bool DebuggerController::Launch() return false; m_inputFileLoaded = false; + m_initialBreakpointSeen = false; m_state->MarkDirty(); - bool result = Execute(); - if (result) - { - m_state->SetConnectionStatus(DebugAdapterConnectedStatus); - m_state->SetExecutionStatus(DebugAdapterPausedStatus); - HandleInitialBreakpoint(); - } - else - { - // TODO: Right now, the LLDBAdapter directly notifies about the failure. In the future, we should let the - // backend return both an error value and an error string. And the error will be notified at API level. - // NotifyError("Failed to launch the target"); - } - return result; + if (!CreateDebuggerBinaryView()) + return false; + + return Execute(); } @@ -146,15 +137,12 @@ bool DebuggerController::Attach(int32_t pid) return false; m_inputFileLoaded = false; + m_initialBreakpointSeen = false; m_state->MarkDirty(); - bool result = m_adapter->Attach(pid); - if (result) - { - m_state->SetConnectionStatus(DebugAdapterConnectedStatus); - m_state->SetExecutionStatus(DebugAdapterPausedStatus); - HandleInitialBreakpoint(); - } - return result; + if (!CreateDebuggerBinaryView()) + return false; + + return m_adapter->Attach(pid); } @@ -613,18 +601,15 @@ DebugStopReason DebuggerController::RunToAndWait(const std::vector& re } -void DebuggerController::HandleInitialBreakpoint() +bool DebuggerController::CreateDebuggerBinaryView() { - m_state->UpdateCaches(); - - // Create the debugger view BinaryViewTypeRef viewType = BinaryViewType::GetByName("Debugger"); if (!viewType) - return; + return false; BinaryViewRef liveView = viewType->Create(m_data); if (!liveView) - return; + return false; // The bvt does not set the arch and platform for the created binary view. We must set them explicitly. // TODO: in the future, when we add support for using the debugger without a base binary view (i.e., the m_data in @@ -634,7 +619,7 @@ void DebuggerController::HandleInitialBreakpoint() liveView->SetDefaultPlatform(m_data->GetDefaultPlatform()); SetLiveView(liveView); - NotifyStopped(DebugStopReason::InitialBreakpoint); + return true; } @@ -741,23 +726,12 @@ void DebuggerController::Connect() return; m_inputFileLoaded = false; + m_initialBreakpointSeen = false; m_state->MarkDirty(); m_state->SetConnectionStatus(DebugAdapterConnectingStatus); NotifyEvent(ConnectEventType); bool ok = m_adapter->Connect(m_state->GetRemoteHost(), m_state->GetRemotePort()); - - if (ok) - { - m_state->MarkDirty(); - m_state->SetConnectionStatus(DebugAdapterConnectedStatus); - m_state->SetExecutionStatus(DebugAdapterPausedStatus); - HandleInitialBreakpoint(); - } - else - { - LogWarn("Failed to connect to the target"); - } } @@ -1064,6 +1038,8 @@ void DebuggerController::EventHandler(const DebuggerEvent& event) case ResumeEventType: case StepIntoEventType: { + // Todo: this is just a temporary workaround. Otherwise, the connection status would not be set properly + m_state->SetConnectionStatus(DebugAdapterConnectedStatus); m_state->SetExecutionStatus(DebugAdapterRunningStatus); m_state->MarkDirty(); break; @@ -1073,6 +1049,8 @@ void DebuggerController::EventHandler(const DebuggerEvent& event) case QuitDebuggingEventType: case DetachedEventType: { + m_inputFileLoaded = false; + m_initialBreakpointSeen = false; m_liveView->GetFile()->UnregisterViewOfType("Debugger", m_liveView); SetLiveView(nullptr); m_state->SetConnectionStatus(DebugAdapterNotConnectedStatus); @@ -1082,6 +1060,7 @@ void DebuggerController::EventHandler(const DebuggerEvent& event) case TargetStoppedEventType: { m_state->UpdateCaches(); + m_state->SetConnectionStatus(DebugAdapterConnectedStatus); m_state->SetExecutionStatus(DebugAdapterPausedStatus); m_lastIP = m_currentIP; m_currentIP = m_state->IP(); @@ -1162,12 +1141,19 @@ void DebuggerController::PostDebuggerEvent(const DebuggerEvent& event) m_lastAdapterStopEventConsumed = false; ExecuteOnMainThreadAndWait([&]() { + DebuggerEvent eventToSend = event; + if ((eventToSend.type == TargetStoppedEventType) && !m_initialBreakpointSeen) + { + m_initialBreakpointSeen = true; + eventToSend.data.targetStoppedData.reason = InitialBreakpoint; + } + for (const DebuggerEventCallback& cb : eventCallbacks) { if (m_disabledCallbacks.find(cb.index) != m_disabledCallbacks.end()) continue; - cb.function(event); + cb.function(eventToSend); } // If the current event is an AdapterStoppedEvent, and it is not consumed by any callback, then the adapter @@ -1176,6 +1162,11 @@ void DebuggerController::PostDebuggerEvent(const DebuggerEvent& event) { DebuggerEvent stopEvent = event; stopEvent.type = TargetStoppedEventType; + if (!m_initialBreakpointSeen) + { + m_initialBreakpointSeen = true; + stopEvent.data.targetStoppedData.reason = InitialBreakpoint; + } for (const DebuggerEventCallback& cb : eventCallbacks) { if (m_disabledCallbacks.find(cb.index) != m_disabledCallbacks.end()) @@ -1336,7 +1327,7 @@ uint32_t DebuggerController::GetExitCode() void DebuggerController::WriteStdIn(const std::string message) { - if (m_adapter && m_state->IsConnected()) + if (m_adapter && m_state->IsRunning()) { m_adapter->WriteStdin(message); } diff --git a/core/debuggercontroller.h b/core/debuggercontroller.h index 5785db33..228a8137 100644 --- a/core/debuggercontroller.h +++ b/core/debuggercontroller.h @@ -91,12 +91,13 @@ namespace BinaryNinjaDebugger { bool m_lastAdapterStopEventConsumed = true; bool m_inputFileLoaded = false; + bool m_initialBreakpointSeen = false; void EventHandler(const DebuggerEvent& event); void UpdateStackVariables(); void AddRegisterValuesToExpressionParser(); bool CreateDebugAdapter(); - void HandleInitialBreakpoint(); + bool CreateDebuggerBinaryView(); void SetLiveView(BinaryViewRef view) { m_liveView = view; }