From ad43b032534094b1acfa1e4a9c0717ecc75e0f47 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 31 Mar 2024 21:53:51 +0200 Subject: [PATCH] HW: Remove calls to GetPointer Typically when someone uses GetPointer, it's because they want to read from a range of memory. GetPointer is unsafe to use for this. While it does check that the passed-in address is valid, it doesn't know the size of the range that will be accessed, so it can't check that the end address is valid. The safer alternative GetPointerForRange should be used instead. Note that there is still the problem of many callers not checking for nullptr. This is part 2 of a series of changes removing the use of GetPointer throughout the code base. After this, VideoCommon is the one major part of Dolphin that remains. --- Source/Core/Core/HW/AddressSpace.cpp | 6 ++--- Source/Core/Core/HW/DSP.cpp | 2 +- Source/Core/Core/HW/DSPHLE/UCodes/UCodes.cpp | 5 +++-- Source/Core/Core/HW/DSPLLE/DSPHost.cpp | 2 +- .../Core/Core/HW/EXI/EXI_DeviceEthernet.cpp | 2 +- .../Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp | 4 ++-- Source/Core/Core/HW/EXI/EXI_DeviceModem.cpp | 4 ++-- Source/Core/Core/HW/GPFifo.cpp | 10 ++------- Source/Core/Core/HW/Memmap.cpp | 22 ++++++++++++++----- Source/Core/Core/PowerPC/GDBStub.cpp | 4 ++-- Source/Core/Core/PowerPC/MMU.cpp | 15 +++---------- 11 files changed, 36 insertions(+), 40 deletions(-) diff --git a/Source/Core/Core/HW/AddressSpace.cpp b/Source/Core/Core/HW/AddressSpace.cpp index 4ced120a50..e98ce0f104 100644 --- a/Source/Core/Core/HW/AddressSpace.cpp +++ b/Source/Core/Core/HW/AddressSpace.cpp @@ -150,14 +150,14 @@ struct EffectiveAddressSpaceAccessors : Accessors return false; } - u8* page_ptr = memory.GetPointer(*page_physical_address); + std::size_t chunk_size = std::min(0x1000 - offset, needle_size); + u8* page_ptr = memory.GetPointerForRange(*page_physical_address + offset, chunk_size); if (page_ptr == nullptr) { return false; } - std::size_t chunk_size = std::min(0x1000 - offset, needle_size); - if (memcmp(needle_start, page_ptr + offset, chunk_size) != 0) + if (memcmp(needle_start, page_ptr, chunk_size) != 0) { return false; } diff --git a/Source/Core/Core/HW/DSP.cpp b/Source/Core/Core/HW/DSP.cpp index 30064953a8..869c0f42b5 100644 --- a/Source/Core/Core/HW/DSP.cpp +++ b/Source/Core/Core/HW/DSP.cpp @@ -430,7 +430,7 @@ void DSPManager::UpdateAudioDMA() // external audio fifo in the emulator, to be mixed with the disc // streaming output. auto& memory = m_system.GetMemory(); - void* address = memory.GetPointer(m_audio_dma.current_source_address); + void* address = memory.GetPointerForRange(m_audio_dma.current_source_address, 32); AudioCommon::SendAIBuffer(m_system, reinterpret_cast(address), 8); if (m_audio_dma.remaining_blocks_count != 0) diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/UCodes.cpp b/Source/Core/Core/HW/DSPHLE/UCodes/UCodes.cpp index 7617c0088b..b3d2afd30e 100644 --- a/Source/Core/Core/HW/DSPHLE/UCodes/UCodes.cpp +++ b/Source/Core/Core/HW/DSPHLE/UCodes/UCodes.cpp @@ -190,8 +190,9 @@ void UCodeInterface::PrepareBootUCode(u32 mail) if (Config::Get(Config::MAIN_DUMP_UCODE)) { - DSP::DumpDSPCode(memory.GetPointer(m_next_ucode.iram_mram_addr), m_next_ucode.iram_size, - ector_crc); + const u8* pointer = + memory.GetPointerForRange(m_next_ucode.iram_mram_addr, m_next_ucode.iram_size); + DSP::DumpDSPCode(pointer, m_next_ucode.iram_size, ector_crc); } DEBUG_LOG_FMT(DSPHLE, "PrepareBootUCode {:#010x}", ector_crc); diff --git a/Source/Core/Core/HW/DSPLLE/DSPHost.cpp b/Source/Core/Core/HW/DSPLLE/DSPHost.cpp index 7ba08a57ec..39507c98fe 100644 --- a/Source/Core/Core/HW/DSPLLE/DSPHost.cpp +++ b/Source/Core/Core/HW/DSPLLE/DSPHost.cpp @@ -76,7 +76,7 @@ void CodeLoaded(DSPCore& dsp, u32 addr, size_t size) { auto& system = Core::System::GetInstance(); auto& memory = system.GetMemory(); - CodeLoaded(dsp, memory.GetPointer(addr), size); + CodeLoaded(dsp, memory.GetPointerForRange(addr, size), size); } void CodeLoaded(DSPCore& dsp, const u8* ptr, size_t size) diff --git a/Source/Core/Core/HW/EXI/EXI_DeviceEthernet.cpp b/Source/Core/Core/HW/EXI/EXI_DeviceEthernet.cpp index fb2660ed04..202dc557f8 100644 --- a/Source/Core/Core/HW/EXI/EXI_DeviceEthernet.cpp +++ b/Source/Core/Core/HW/EXI/EXI_DeviceEthernet.cpp @@ -231,7 +231,7 @@ void CEXIETHERNET::DMAWrite(u32 addr, u32 size) transfer.address == BBA_WRTXFIFOD) { auto& memory = m_system.GetMemory(); - DirectFIFOWrite(memory.GetPointer(addr), size); + DirectFIFOWrite(memory.GetPointerForRange(addr, size), size); } else { diff --git a/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp b/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp index cfc76f27db..13a06da652 100644 --- a/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp +++ b/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp @@ -525,7 +525,7 @@ void CEXIMemoryCard::DoState(PointerWrap& p) void CEXIMemoryCard::DMARead(u32 addr, u32 size) { auto& memory = m_system.GetMemory(); - m_memory_card->Read(m_address, size, memory.GetPointer(addr)); + m_memory_card->Read(m_address, size, memory.GetPointerForRange(addr, size)); if ((m_address + size) % Memcard::BLOCK_SIZE == 0) { @@ -543,7 +543,7 @@ void CEXIMemoryCard::DMARead(u32 addr, u32 size) void CEXIMemoryCard::DMAWrite(u32 addr, u32 size) { auto& memory = m_system.GetMemory(); - m_memory_card->Write(m_address, size, memory.GetPointer(addr)); + m_memory_card->Write(m_address, size, memory.GetPointerForRange(addr, size)); if (((m_address + size) % Memcard::BLOCK_SIZE) == 0) { diff --git a/Source/Core/Core/HW/EXI/EXI_DeviceModem.cpp b/Source/Core/Core/HW/EXI/EXI_DeviceModem.cpp index 17d4a7c484..fbe75e7a84 100644 --- a/Source/Core/Core/HW/EXI/EXI_DeviceModem.cpp +++ b/Source/Core/Core/HW/EXI/EXI_DeviceModem.cpp @@ -127,7 +127,7 @@ void CEXIModem::DMAWrite(u32 addr, u32 size) else { auto& memory = m_system.GetMemory(); - HandleWriteModemTransfer(memory.GetPointer(addr), size); + HandleWriteModemTransfer(memory.GetPointerForRange(addr, size), size); } } @@ -195,7 +195,7 @@ void CEXIModem::DMARead(u32 addr, u32 size) else { auto& memory = m_system.GetMemory(); - HandleReadModemTransfer(memory.GetPointer(addr), size); + HandleReadModemTransfer(memory.GetPointerForRange(addr, size), size); } } diff --git a/Source/Core/Core/HW/GPFifo.cpp b/Source/Core/Core/HW/GPFifo.cpp index dc2d6e2f60..0de434b93d 100644 --- a/Source/Core/Core/HW/GPFifo.cpp +++ b/Source/Core/Core/HW/GPFifo.cpp @@ -90,24 +90,18 @@ void GPFifoManager::UpdateGatherPipe() size_t pipe_count = GetGatherPipeCount(); size_t processed; - u8* cur_mem = memory.GetPointer(processor_interface.m_fifo_cpu_write_pointer); for (processed = 0; pipe_count >= GATHER_PIPE_SIZE; processed += GATHER_PIPE_SIZE) { // copy the GatherPipe - memcpy(cur_mem, m_gather_pipe + processed, GATHER_PIPE_SIZE); + memory.CopyToEmu(processor_interface.m_fifo_cpu_write_pointer, m_gather_pipe + processed, + GATHER_PIPE_SIZE); pipe_count -= GATHER_PIPE_SIZE; // increase the CPUWritePointer if (processor_interface.m_fifo_cpu_write_pointer == processor_interface.m_fifo_cpu_end) - { processor_interface.m_fifo_cpu_write_pointer = processor_interface.m_fifo_cpu_base; - cur_mem = memory.GetPointer(processor_interface.m_fifo_cpu_write_pointer); - } else - { - cur_mem += GATHER_PIPE_SIZE; processor_interface.m_fifo_cpu_write_pointer += GATHER_PIPE_SIZE; - } system.GetCommandProcessor().GatherPipeBursted(); } diff --git a/Source/Core/Core/HW/Memmap.cpp b/Source/Core/Core/HW/Memmap.cpp index 02d31c8272..9950fb5200 100644 --- a/Source/Core/Core/HW/Memmap.cpp +++ b/Source/Core/Core/HW/Memmap.cpp @@ -462,18 +462,28 @@ void MemoryManager::Memset(u32 address, u8 value, size_t size) std::string MemoryManager::GetString(u32 em_address, size_t size) { - const char* ptr = reinterpret_cast(GetPointer(em_address)); - if (ptr == nullptr) - return ""; + std::string result; if (size == 0) // Null terminated string. { - return std::string(ptr); + while (true) + { + const u8 value = Read_U8(em_address); + if (value == 0) + break; + + result.push_back(value); + ++em_address; + } + return result; } else // Fixed size string, potentially null terminated or null padded. { - size_t length = strnlen(ptr, size); - return std::string(ptr, length); + result.resize(size); + CopyFromEmu(result.data(), em_address, size); + size_t length = strnlen(result.data(), size); + result.resize(length); + return result; } } diff --git a/Source/Core/Core/PowerPC/GDBStub.cpp b/Source/Core/Core/PowerPC/GDBStub.cpp index 5ab058ef82..a0d658eb1f 100644 --- a/Source/Core/Core/PowerPC/GDBStub.cpp +++ b/Source/Core/Core/PowerPC/GDBStub.cpp @@ -831,7 +831,7 @@ static void ReadMemory(const Core::CPUThreadGuard& guard) auto& system = Core::System::GetInstance(); auto& memory = system.GetMemory(); - u8* data = memory.GetPointer(addr); + u8* data = memory.GetPointerForRange(addr, len); Mem2hex(reply, data, len); reply[len * 2] = '\0'; SendReply((char*)reply); @@ -858,7 +858,7 @@ static void WriteMemory(const Core::CPUThreadGuard& guard) auto& system = Core::System::GetInstance(); auto& memory = system.GetMemory(); - u8* dst = memory.GetPointer(addr); + u8* dst = memory.GetPointerForRange(addr, len); Hex2mem(dst, s_cmd_bfr + i + 1, len); SendReply("OK"); } diff --git a/Source/Core/Core/PowerPC/MMU.cpp b/Source/Core/Core/PowerPC/MMU.cpp index 59f574ad65..0291010bad 100644 --- a/Source/Core/Core/PowerPC/MMU.cpp +++ b/Source/Core/Core/PowerPC/MMU.cpp @@ -1044,18 +1044,11 @@ void MMU::DMA_LCToMemory(const u32 mem_address, const u32 cache_address, const u } const u8* src = m_memory.GetL1Cache() + (cache_address & 0x3FFFF); - u8* dst = m_memory.GetPointer(mem_address); - if (dst == nullptr) - return; - - memcpy(dst, src, 32 * num_blocks); + m_memory.CopyToEmu(mem_address, src, 32 * num_blocks); } void MMU::DMA_MemoryToLC(const u32 cache_address, const u32 mem_address, const u32 num_blocks) { - const u8* src = m_memory.GetPointer(mem_address); - u8* dst = m_memory.GetL1Cache() + (cache_address & 0x3FFFF); - // No known game uses this; here for completeness. // TODO: Refactor. if ((mem_address & 0x0F000000) == 0x08000000) @@ -1081,10 +1074,8 @@ void MMU::DMA_MemoryToLC(const u32 cache_address, const u32 mem_address, const u return; } - if (src == nullptr) - return; - - memcpy(dst, src, 32 * num_blocks); + u8* dst = m_memory.GetL1Cache() + (cache_address & 0x3FFFF); + m_memory.CopyFromEmu(dst, mem_address, 32 * num_blocks); } static bool TranslateBatAddress(const BatTable& bat_table, u32* address, bool* wi)