From 3246baec4bcb040628b0e385a46ec473521122ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 28 Nov 2022 10:39:04 +0100 Subject: [PATCH 1/2] SoftGPU: Range check block copies. Needs testing to verify if we should copy zeroes instead if the src range is partial, etc, quite a few possible edge cases. Though on its own, this probably fixes the crash in #16427. Still don't understand why that one has issues in hardware renderers though since they do bounds-check the copies. --- GPU/Software/SoftGpu.cpp | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/GPU/Software/SoftGpu.cpp b/GPU/Software/SoftGpu.cpp index 2203c14ab9..7a824fe7b0 100644 --- a/GPU/Software/SoftGpu.cpp +++ b/GPU/Software/SoftGpu.cpp @@ -807,15 +807,38 @@ void SoftGPU::Execute_BlockTransferStart(u32 op, u32 diff) { u32 srcLineStartAddr = srcBasePtr + (srcY * srcStride + srcX) * bpp; u32 dstLineStartAddr = dstBasePtr + (dstY * dstStride + dstX) * bpp; + u32 bytesToCopy = width * height * bpp; + + if (!Memory::IsValidRange(srcLineStartAddr, bytesToCopy)) { + // What should we do here? Memset zeroes to the dest instead? + return; + } + if (!Memory::IsValidRange(dstLineStartAddr, bytesToCopy)) { + // What should we do here? Just not do the write, or partial write if + // some part is in-range? + return; + } + const u8 *srcp = Memory::GetPointer(srcLineStartAddr); u8 *dstp = Memory::GetPointerWrite(dstLineStartAddr); - memcpy(dstp, srcp, width * height * bpp); - GPURecord::NotifyMemcpy(dstLineStartAddr, srcLineStartAddr, width * height * bpp); + memcpy(dstp, srcp, bytesToCopy); + GPURecord::NotifyMemcpy(dstLineStartAddr, srcLineStartAddr, bytesToCopy); } else { for (int y = 0; y < height; y++) { u32 srcLineStartAddr = srcBasePtr + ((y + srcY) * srcStride + srcX) * bpp; u32 dstLineStartAddr = dstBasePtr + ((y + dstY) * dstStride + dstX) * bpp; + u32 bytesToCopy = width * bpp; + if (!Memory::IsValidRange(srcLineStartAddr, bytesToCopy)) { + // What should we do here? Due to the y loop, in this case we might have + // performed a partial copy. Probably fine. + break; + } + if (!Memory::IsValidRange(dstLineStartAddr, bytesToCopy)) { + // What should we do here? Due to the y loop, in this case we might have + // performed a partial copy. Probably fine. + break; + } const u8 *srcp = Memory::GetPointer(srcLineStartAddr); u8 *dstp = Memory::GetPointerWrite(dstLineStartAddr); memcpy(dstp, srcp, width * bpp); From 6c8ccc149e852af8d054a4e14b09d1100639baf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 28 Nov 2022 10:46:45 +0100 Subject: [PATCH 2/2] Stop logspam from bad block transfer corners. --- GPU/GPUCommon.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/GPU/GPUCommon.cpp b/GPU/GPUCommon.cpp index 37654faff5..1560d3fe49 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -3073,11 +3073,11 @@ void GPUCommon::DoBlockTransfer(u32 skipDrawReason) { u32 dstLastAddr = dstBasePtr + ((dstY + height - 1) * dstStride + (dstX + width - 1)) * bpp; if (!Memory::IsValidAddress(srcLastAddr)) { - ERROR_LOG_REPORT(G3D, "Bottom-right corner of source of block transfer is at an invalid address: %08x", srcLastAddr); + ERROR_LOG_N_TIMES(bad_xfer_src, 5, G3D, "Bottom-right corner of source of %dx%d src=(%d, %d) block transfer from buffer at %08x is at an invalid address: %08x. Skipping.", width, height, srcX, srcY, srcBasePtr, srcLastAddr); return; } if (!Memory::IsValidAddress(dstLastAddr)) { - ERROR_LOG_REPORT(G3D, "Bottom-right corner of destination of block transfer is at an invalid address: %08x", srcLastAddr); + ERROR_LOG_N_TIMES(bad_xfer_src, 5, G3D, "Bottom-right corner of destination of %dx%d dst=(%d, %d) block transfer to buffer at %08x is at an invalid address: %08x. Skipping.", width, height, dstX, dstY, dstBasePtr, srcLastAddr); return; }