[SharedCache] Simplify MMappedFileAccessor::Read* methods

This change is mostly motivated by simplifying the code, but it also
brings minor correctness and performance benefits.

1. The pointer returned by mmap is stored as a uint8_t* rather than
   void* as that is how it is used. This reduces how often it needs to
   be cast to a different type before it is used.
2. Read methods for primitives delegate to a new Read template function
   that in turn delegates to the general-purpose `Read(void* dest, size_t
   address, size_t length)`. This improves the consistency of bounds
   checking and simplifies the code. The compiler is more than willing
   to inline this so we get less repetition with no overhead.
3. ReadNullTermString now uses std::find to find the nul byte and
   directly constructs the string from that range of bytes. This removes
   an unnecessary allocation that was previously being forced by the use
   reserve followed by shrink_to_fit. It also avoids repeated
   reallocation for longer strings as they grew past the the reserved
   size as they were being built up a character at a time.

1 files changed, 31 insertions, 52 deletions

diff --git a/view/sharedcache/core/VM.cpp b/view/sharedcache/core/VM.cpp
index 358a1dd8..4a3acf66 100644
--- a/view/sharedcache/core/VM.cpp
+++ b/view/sharedcache/core/VM.cpp
@@ -149,12 +149,12 @@ void MMAP::Map()
 		return;
 	}
 
-	_mmap = MapViewOfFile(
+	_mmap = static_cast<uint8_t*>(MapViewOfFile(
 		hMapping,                    // handle to the file mapping object
 		FILE_MAP_COPY,         		 // desired access
 		0,                           // file offset (high-order DWORD)
 		0,                           // file offset (low-order DWORD)
-		0);                          // number of bytes to map (0 = entire file)
+		0));                         // number of bytes to map (0 = entire file)
 
 	if (_mmap == nullptr)
 	{
@@ -174,13 +174,14 @@ void MMAP::Map()
 	len = ftell(fd);
 	fseek(fd, 0L, SEEK_SET);
 
-	_mmap = mmap(nullptr, len, PROT_READ | PROT_WRITE, MAP_PRIVATE, fileno(fd), 0u);
-	if (_mmap == MAP_FAILED)
+	void *result = mmap(nullptr, len, PROT_READ | PROT_WRITE, MAP_PRIVATE, fileno(fd), 0u);
+	if (result == MAP_FAILED)
 	{
 		// Handle error
 		return;
 	}
 
+	_mmap = static_cast<uint8_t*>(result);
 	mapped = true;
 #endif
 }
@@ -376,93 +377,72 @@ MMappedFileAccessor::~MMappedFileAccessor()
 
 void MMappedFileAccessor::WritePointer(size_t address, size_t pointer)
 {
-	((size_t*)(&((uint8_t*)m_mmap._mmap)[address]))[0] = pointer;
+	*(size_t*)&m_mmap._mmap[address] = pointer;
+}
+
+template <typename T>
+T MMappedFileAccessor::Read(size_t address) {
+	T result;
+	Read(&result, address, sizeof(T));
+	return result;
 }
 
 std::string MMappedFileAccessor::ReadNullTermString(size_t address)
 {
 	if (address > m_mmap.len)
 		return "";
-	size_t max = m_mmap.len;
-	size_t i = address;
-	std::string str;
-	str.reserve(140);
-	while (i < max)
-	{
-		char c = ((char*)(&((uint8_t*)m_mmap._mmap)[i]))[0];
-		if (c == 0)
-			break;
-		str += c;
-		i++;
-	}
-	str.shrink_to_fit();
-	return str;
+	auto start = m_mmap._mmap + address;
+	auto end = m_mmap._mmap + m_mmap.len;
+	auto nul = std::find(start, end, 0);
+	return std::string(start, nul);
 }
 
 uint8_t MMappedFileAccessor::ReadUChar(size_t address)
 {
-	if (address > m_mmap.len)
-		throw MappingReadException();
-	return ((uint8_t*)(&(((uint8_t*)m_mmap._mmap)[address])))[0];
+	return Read<uint8_t>(address);
 }
 
 int8_t MMappedFileAccessor::ReadChar(size_t address)
 {
-	if (address > m_mmap.len)
-		throw MappingReadException();
-	return ((int8_t*)(&(((uint8_t*)m_mmap._mmap)[address])))[0];
+	return Read<int8_t>(address);
 }
 
 uint16_t MMappedFileAccessor::ReadUShort(size_t address)
 {
-	if (address > m_mmap.len)
-		throw MappingReadException();
-	return ((uint16_t*)(&(((uint8_t*)m_mmap._mmap)[address])))[0];
+	return Read<uint16_t>(address);
 }
 
 int16_t MMappedFileAccessor::ReadShort(size_t address)
 {
-	if (address > m_mmap.len)
-		throw MappingReadException();
-	return ((int16_t*)(&(((uint8_t*)m_mmap._mmap)[address])))[0];
+	return Read<int16_t>(address);
 }
 
 uint32_t MMappedFileAccessor::ReadUInt32(size_t address)
 {
-	if (address > m_mmap.len)
-		throw MappingReadException();
-	return ((uint32_t*)(&(((uint8_t*)m_mmap._mmap)[address])))[0];
+	return Read<uint32_t>(address);
 }
 
 int32_t MMappedFileAccessor::ReadInt32(size_t address)
 {
-	if (address > m_mmap.len)
-		throw MappingReadException();
-	return ((int32_t*)(&(((uint8_t*)m_mmap._mmap)[address])))[0];
+	return Read<int32_t>(address);
 }
 
 uint64_t MMappedFileAccessor::ReadULong(size_t address)
 {
-	if (address > m_mmap.len)
-		throw MappingReadException();
-	return ((uint64_t*)(&(((uint8_t*)m_mmap._mmap)[address])))[0];
+	return Read<uint64_t>(address);
 }
 
 int64_t MMappedFileAccessor::ReadLong(size_t address)
 {
-	if (address > m_mmap.len)
-		throw MappingReadException();
-	return ((int64_t*)(&(((uint8_t*)m_mmap._mmap)[address])))[0];
+	return Read<int64_t>(address);
 }
 
 BinaryNinja::DataBuffer MMappedFileAccessor::ReadBuffer(size_t address, size_t length)
 {
-	if (address > m_mmap.len)
+	if (m_mmap.len <= length || address > m_mmap.len - length)
 		throw MappingReadException();
-	if (address + length > m_mmap.len)
-		throw MappingReadException();
-	void* data = (void*)(&(((uint8_t*)m_mmap._mmap)[address]));
-	return BinaryNinja::DataBuffer(data, length);
+
+	return BinaryNinja::DataBuffer(&m_mmap._mmap[address], length);
 }
 
 std::pair<const uint8_t*, const uint8_t*> MMappedFileAccessor::ReadSpan(size_t address, size_t length)
@@ -478,11 +458,10 @@ std::pair<const uint8_t*, const uint8_t*> MMappedFileAccessor::ReadSpan(size_t a
 
 void MMappedFileAccessor::Read(void* dest, size_t address, size_t length)
 {
-	if (address > m_mmap.len)
+	if (m_mmap.len <= length || address > m_mmap.len - length)
 		throw MappingReadException();
-	if (address + length > m_mmap.len)
-		throw MappingReadException();
-	memcpy(dest, (void*)&(((uint8_t*)m_mmap._mmap)[address]), length);
+
+	memcpy(dest, &m_mmap._mmap[address], length);
 }