From c9bbe933dd53fff5d54c2e4f218e4a89f6c58fcd Mon Sep 17 00:00:00 2001 From: Mark Rowe Date: Fri, 22 Nov 2024 19:04:37 -0800 Subject: [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. --- view/sharedcache/core/VM.cpp | 83 +++++++++++++++++--------------------------- 1 file changed, 31 insertions(+), 52 deletions(-) (limited to 'view/sharedcache/core/VM.cpp') 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(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(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 +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(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(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(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(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(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(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(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(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 MMappedFileAccessor::ReadSpan(size_t address, size_t length) @@ -478,11 +458,10 @@ std::pair 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); } -- cgit v1.3.1