Fix multiple bugs: Python FFI mismatches, mutex leak, thread state, double notify, and misc (#1026)

Author: 3rdit

api/python/debuggercontroller.py

@@ -83,7 +83,7 @@ def __ne__(self, other):
         return not (self == other)
 
     def __hash__(self):
-        return hash((self.pid, self.pid))
+        return hash((self.pid, self.name))
 
     def __setattr__(self, name, value):
         try:
@@ -1586,7 +1586,7 @@ def detach(self) -> None:
         """
         Detach the target, and let it execute on its own.
         """
-        dbgcore.BNDebuggerQuit(self.handle)
+        dbgcore.BNDebuggerDetach(self.handle)
 
     def pause(self) -> None:
         """
@@ -2072,12 +2072,12 @@ def pid_attach(self) -> int:
 
         ``pid_attach`` is only useful for connecting to a running process using PID.
 
-        :getter: returns the remote port
-        :setter: sets the remote port
+        :getter: returns the PID to attach to
+        :setter: sets the PID to attach to
         """
         return dbgcore.BNDebuggerGetPIDAttach(self.handle)
 
-    @remote_port.setter
+    @pid_attach.setter
     def pid_attach(self, pid: int) -> None:
         dbgcore.BNDebuggerSetPIDAttach(self.handle, pid)
 
@@ -2494,7 +2494,9 @@ def set_adapter_property(self, name: Union[str, bytes], value: binaryninja.metad
         return dbgcore.BNDebuggerSetAdapterProperty(self.handle, name, handle)
 
     def get_addr_info(self, addr: int):
-        return dbgcore.BNDebuggerGetAddressInformation(self.handle, addr)
+        buffer = addr.to_bytes(64, byteorder='little', signed=False)
+        c_buffer = (ctypes.c_ubyte * 64)(*buffer)
+        return dbgcore.BNDebuggerGetAddressInformation(self.handle, c_buffer)
 
     @property
     def is_first_launch(self):

core/adapters/dbgengttdadapter.cpp

@@ -116,7 +116,7 @@ bool DbgEngTTDAdapter::Start()
 
 	auto handle = GetModuleHandleA("dbgeng.dll");
 	if (handle == nullptr)
-		false;
+		return false;
 
 	//    HRESULT DebugCreate(
 	//    [in]  REFIID InterfaceId,

core/adapters/gdbadapter.cpp

@@ -1376,7 +1376,7 @@ std::string GdbAdapter::InvokeBackendCommand(const std::string& command)
 	if (command.substr(0, 4) == "mon ")
 		return RunMonitorCommand(command.substr(4));
 	else if (command.substr(0, 8) == "monitor ")
-		return RunMonitorCommand(command.substr(4));
+		return RunMonitorCommand(command.substr(8));
 
 	auto reply = this->m_rspConnector->TransmitAndReceive(RspData(command));
 	return reply.AsString();

core/debuggercontroller.cpp

@@ -1259,7 +1259,6 @@ DebugStopReason DebuggerController::RunToAndWaitInternal(const std::vector<uint6
 		}
 	}
 
-	NotifyStopped(reason);
 	return reason;
 }
 
@@ -1286,7 +1285,6 @@ DebugStopReason DebuggerController::RunToReverseAndWaitInternal(const std::vecto
 		}
 	}
 
-	NotifyStopped(reason);
 	return reason;
 }
 
@@ -1530,7 +1528,11 @@ void DebuggerController::DetachAndWait()
 		locked = true;
 
 	if (!m_state->IsConnected())
+	{
+		if (locked)
+			m_targetControlMutex.unlock();
 		return;
+	}
 
 	// TODO: return whether the operation is successful
 	ExecuteAdapterAndWait(DebugAdapterDetach);
@@ -1559,7 +1561,11 @@ void DebuggerController::QuitAndWait()
 		locked = true;
 
 	if (!m_state->IsConnected())
+	{
+		if (locked)
+			m_targetControlMutex.unlock();
 		return;
+	}
 
 	if (m_state->IsRunning())
 	{

core/debuggerstate.cpp

@@ -330,12 +330,13 @@ bool DebuggerThreads::SuspendThread(std::uint32_t tid)
 	if (!adapter)
 		return false;
 
-	auto threads = GetAllThreads();
-	auto thread = std::find_if(threads.begin(), threads.end(), [&](DebugThread const& t) {
+	std::unique_lock lock(m_threadsMutex);
+
+	auto thread = std::find_if(m_threads.begin(), m_threads.end(), [&](DebugThread const& t) {
 		return t.m_tid == tid;
 	});
 
-	if (thread == threads.end())
+	if (thread == m_threads.end())
 		return false;
 
 
@@ -360,12 +361,13 @@ bool DebuggerThreads::ResumeThread(std::uint32_t tid)
 	if (!adapter)
 		return false;
 
-	auto threads = GetAllThreads();
-	auto thread = std::find_if(threads.begin(), threads.end(), [&](DebugThread const& t) {
+	std::unique_lock lock(m_threadsMutex);
+
+	auto thread = std::find_if(m_threads.begin(), m_threads.end(), [&](DebugThread const& t) {
 		return t.m_tid == tid;
 	});
 
-	if (thread == threads.end())
+	if (thread == m_threads.end())
 		return false;
 
 	if (!thread->m_isFrozen)

ui/threadframes.h

@@ -99,7 +99,7 @@ class FrameItem
 	uint64_t m_fp {};
 
 	QList<FrameItem*> m_childItems;
-	FrameItem* m_parentItem;
+	FrameItem* m_parentItem {nullptr};
 };
 
 Q_DECLARE_METATYPE(FrameItem);