From 51f3b387da9bf0241ae55a6de5d00738b780b19c Mon Sep 17 00:00:00 2001
From: Xusheng <xusheng@vector35.com>
Date: Tue, 6 Aug 2024 20:27:35 +0800
Subject: [PATCH] Only discard the debugger state cache after the target has
 stopped. Fix https://github.com/Vector35/debugger/issues/587

---
 core/debuggercontroller.cpp |  6 +--
 core/debuggerstate.cpp      | 82 ++++++++++++++++++++++++++-----------
 core/debuggerstate.h        | 19 ++++++++-
 3 files changed, 77 insertions(+), 30 deletions(-)

diff --git a/core/debuggercontroller.cpp b/core/debuggercontroller.cpp
index ae0c5b2..3ee95c0 100644
--- a/core/debuggercontroller.cpp
+++ b/core/debuggercontroller.cpp
@@ -1509,11 +1509,11 @@ void DebuggerController::EventHandler(const DebuggerEvent& event)
 		// 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;
 	}
 	case TargetExitedEventType:
 		m_exitCode = event.data.exitData.exitCode;
+		m_state->MarkDirty();
 	case QuitDebuggingEventType:
 	case DetachedEventType:
 	case LaunchFailureEventType:
@@ -1534,6 +1534,7 @@ void DebuggerController::EventHandler(const DebuggerEvent& event)
 	}
 	case TargetStoppedEventType:
 	{
+		m_state->MarkDirty();
 		m_state->UpdateCaches();
 		m_state->SetConnectionStatus(DebugAdapterConnectedStatus);
 		m_state->SetExecutionStatus(DebugAdapterPausedStatus);
@@ -1705,9 +1706,6 @@ DataBuffer DebuggerController::ReadMemory(std::uintptr_t address, std::size_t si
 	if (!m_state->IsConnected())
 		return DataBuffer {};
 
-	if (m_state->IsRunning())
-		return DataBuffer {};
-
 	DebuggerMemory* memory = m_state->GetMemory();
 	if (!memory)
 		return DataBuffer {};
diff --git a/core/debuggerstate.cpp b/core/debuggerstate.cpp
index 090fe42..ee0f421 100644
--- a/core/debuggerstate.cpp
+++ b/core/debuggerstate.cpp
@@ -618,8 +618,62 @@ DebuggerMemory::DebuggerMemory(DebuggerState* state) : m_state(state) {}
 void DebuggerMemory::MarkDirty()
 {
 	std::unique_lock<std::recursive_mutex> memoryLock(m_memoryMutex);
-	m_valueCache.clear();
-	m_errorCache.clear();
+	for (auto& it: m_valueCache)
+	{
+		if (it.second.status == UpToDateStatus)
+			it.second.status = OutOfDateStatus;
+		else
+			it.second.status = DefaultStatus;
+	}
+}
+
+
+DataBuffer DebuggerMemory::ReadBlock(uint64_t block)
+{
+	auto iter = m_valueCache.find(block);
+	if (iter != m_valueCache.end())
+	{
+		switch (iter->second.status)
+		{
+		case FailedToReadStatus:
+			return {};
+		case OutOfDateStatus:
+		{
+			if (m_state->IsConnected() && m_state->IsRunning())
+			{
+				// The cache is old but the target is running, return old value
+				return iter->second.value;
+			}
+			// Break out and try to read the new value
+			break;
+		}
+		case UpToDateStatus:
+		{
+			// Cache is up-to-date, return the value
+			return iter->second.value;
+		}
+		case DefaultStatus:
+			// There is no useful information about the status, break out and try to read it
+			break;
+		}
+	}
+
+	// Try to read the memory value from the backend
+	if (m_state->IsConnected() && !m_state->IsRunning())
+	{
+		// The cache is old and the target is stopped, try to update the cache value
+		DataBuffer buffer = m_state->GetAdapter()->ReadMemory(block, 0x100);
+		if (buffer.GetLength() > 0)
+		{
+			// Successfully updated
+			m_valueCache[block] = {buffer, UpToDateStatus};
+			return buffer;
+		}
+	}
+
+	// Update failed
+	m_valueCache[block] = {{}, FailedToReadStatus};
+	return {};
 }
 
 
@@ -639,30 +693,10 @@ DataBuffer DebuggerMemory::ReadMemory(uint64_t offset, size_t len)
 	// List of 256-byte block addresses to read into the cache to fully cover this region
 	for (uint64_t block = cacheStart; block < cacheEnd; block += 0x100)
 	{
-		// If any block cannot be read, then return false
-		if (m_errorCache.find(block) != m_errorCache.end())
-		{
+		auto cached = ReadBlock(block);
+		if (cached.GetLength() == 0)
 			return result;
-		}
-
-		auto iter = m_valueCache.find(block);
-		if (iter == m_valueCache.end())
-		{
-			// The ReadMemory() function should return the number of bytes read
-			DataBuffer buffer = m_state->GetAdapter()->ReadMemory(block, 0x100);
-			// TODO: what if the buffer's size is smaller than 0x100
-			if (buffer.GetLength() > 0)
-			{
-				m_valueCache[block] = buffer;
-			}
-			else
-			{
-				m_errorCache.insert(block);
-				return result;
-			}
-		}
 
-		DataBuffer cached = m_valueCache[block];
 		if (offset + len < block + cached.GetLength())
 		{
 			// Last block
diff --git a/core/debuggerstate.h b/core/debuggerstate.h
index 6886472..bcec78f 100644
--- a/core/debuggerstate.h
+++ b/core/debuggerstate.h
@@ -116,18 +116,33 @@ namespace BinaryNinjaDebugger {
 		bool ResumeThread(std::uint32_t tid);
 	};
 
+	enum MemoryByteCacheStatus
+	{
+		DefaultStatus,
+		UpToDateStatus,
+		OutOfDateStatus,
+		FailedToReadStatus
+	};
+
+
+	struct MemoryBytesCache
+	{
+		DataBuffer value;
+		MemoryByteCacheStatus status;
+	};
+
 
 	class DebuggerMemory
 	{
 		DebuggerState* m_state;
-		std::map<uint64_t, DataBuffer> m_valueCache;
-		std::set<uint64_t> m_errorCache;
+		std::map<uint64_t, MemoryBytesCache> m_valueCache;
 		std::recursive_mutex m_memoryMutex;
 
 	public:
 		DebuggerMemory(DebuggerState* state);
 
 		void MarkDirty();
+		DataBuffer ReadBlock(uint64_t block);
 		DataBuffer ReadMemory(uint64_t offset, size_t len);
 		bool WriteMemory(std::uintptr_t address, const DataBuffer& buffer);
 	};