From 3e0fea509d29ca7eaec5ca6ba76d6ba7d21c3f5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 14 May 2025 09:29:31 +0200 Subject: [PATCH] Improve sanity checks for vertex ranges in GPU --- GPU/GPUCommon.cpp | 26 +++++++++++--------------- GPU/GPUCommon.h | 2 +- GPU/GPUCommonHW.cpp | 14 ++++++++++---- GPU/GPUState.h | 1 - 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/GPU/GPUCommon.cpp b/GPU/GPUCommon.cpp index 1f922011b5..fc14cb8e53 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -1197,8 +1197,10 @@ void GPUCommon::Execute_BoundingBox(u32 op, u32 diff) { // Approximate based on timings of several counts on a PSP. cyclesExecuted += count * 22; - const bool useInds = (gstate.vertType & GE_VTYPE_IDX_MASK) != 0; - VertexDecoder *dec = drawEngineCommon_->GetVertexDecoder(gstate.vertType); + const u32 vertType = gstate.vertType; + + const bool useInds = (vertType & GE_VTYPE_IDX_MASK) != 0; + const VertexDecoder *dec = drawEngineCommon_->GetVertexDecoder(vertType); int bytesRead = (useInds ? 1 : dec->VertexSize()) * count; if (!Memory::IsValidRange(gstate_c.vertexAddr, bytesRead)) { @@ -1207,23 +1209,17 @@ void GPUCommon::Execute_BoundingBox(u32 op, u32 diff) { currentList->bboxResult = true; return; } - - const void *control_points = Memory::GetPointerUnchecked(gstate_c.vertexAddr); - if (!control_points) { - ERROR_LOG_REPORT_ONCE(boundingbox, Log::G3D, "Invalid verts in bounding box check"); - currentList->bboxResult = true; - return; - } + const void *control_points = Memory::GetPointerUnchecked(gstate_c.vertexAddr); // we checked the range above. const void *inds = nullptr; if (useInds) { - int indexShift = ((gstate.vertType & GE_VTYPE_IDX_MASK) >> GE_VTYPE_IDX_SHIFT) - 1; - inds = Memory::GetPointerUnchecked(gstate_c.indexAddr); - if (!inds || !Memory::IsValidRange(gstate_c.indexAddr, count << indexShift)) { + const int indexSizeShift = ((vertType & GE_VTYPE_IDX_MASK) >> GE_VTYPE_IDX_SHIFT) - 1; + if (!Memory::IsValidRange(gstate_c.indexAddr, count << indexSizeShift)) { ERROR_LOG_REPORT_ONCE(boundingboxInds, Log::G3D, "Invalid inds in bounding box check"); currentList->bboxResult = true; return; } + inds = Memory::GetPointerUnchecked(gstate_c.indexAddr); } // Test if the bounding box is within the drawing region. @@ -1231,12 +1227,12 @@ void GPUCommon::Execute_BoundingBox(u32 op, u32 diff) { if (count > 0x200) { // The second to last set of 0x100 is checked (even for odd counts.) size_t skipSize = (count - 0x200) * dec->VertexSize(); - currentList->bboxResult = drawEngineCommon_->TestBoundingBox((const uint8_t *)control_points + skipSize, inds, 0x100, dec, gstate.vertType); + currentList->bboxResult = drawEngineCommon_->TestBoundingBox((const uint8_t *)control_points + skipSize, inds, 0x100, dec, vertType); } else if (count > 0x100) { int checkSize = count - 0x100; - currentList->bboxResult = drawEngineCommon_->TestBoundingBox(control_points, inds, checkSize, dec, gstate.vertType); + currentList->bboxResult = drawEngineCommon_->TestBoundingBox(control_points, inds, checkSize, dec, vertType); } else { - currentList->bboxResult = drawEngineCommon_->TestBoundingBox(control_points, inds, count, dec, gstate.vertType); + currentList->bboxResult = drawEngineCommon_->TestBoundingBox(control_points, inds, count, dec, vertType); } AdvanceVerts(gstate.vertType, count, bytesRead); } diff --git a/GPU/GPUCommon.h b/GPU/GPUCommon.h index fa5d5672b2..2b4827ac21 100644 --- a/GPU/GPUCommon.h +++ b/GPU/GPUCommon.h @@ -448,7 +448,7 @@ protected: void AdvanceVerts(u32 vertType, int count, int bytesRead) { if ((vertType & GE_VTYPE_IDX_MASK) != GE_VTYPE_IDX_NONE) { - int indexShift = ((vertType & GE_VTYPE_IDX_MASK) >> GE_VTYPE_IDX_SHIFT) - 1; + const int indexShift = ((vertType & GE_VTYPE_IDX_MASK) >> GE_VTYPE_IDX_SHIFT) - 1; gstate_c.indexAddr += count << indexShift; } else { gstate_c.vertexAddr += bytesRead; diff --git a/GPU/GPUCommonHW.cpp b/GPU/GPUCommonHW.cpp index d5639bfb7b..0b3ffa5e11 100644 --- a/GPU/GPUCommonHW.cpp +++ b/GPU/GPUCommonHW.cpp @@ -985,14 +985,14 @@ void GPUCommonHW::Execute_Prim(u32 op, u32 diff) { const void *verts = Memory::GetPointerUnchecked(gstate_c.vertexAddr); const void *inds = nullptr; - bool isTriangle = IsTrianglePrim(prim); + const bool isTriangle = IsTrianglePrim(prim); bool canExtend = isTriangle; u32 vertexType = gstate.vertType; if ((vertexType & GE_VTYPE_IDX_MASK) != GE_VTYPE_IDX_NONE) { u32 indexAddr = gstate_c.indexAddr; - u32 indexSize = (vertexType & GE_VTYPE_IDX_MASK) >> GE_VTYPE_IDX_SHIFT; - if (!Memory::IsValidRange(indexAddr, count * indexSize)) { + const int indexShift = ((vertexType & GE_VTYPE_IDX_MASK) >> GE_VTYPE_IDX_SHIFT) - 1; + if (!Memory::IsValidRange(indexAddr, count << indexShift)) { ERROR_LOG(Log::G3D, "Bad index address %08x (%d)!", indexAddr, count); return; } @@ -1109,7 +1109,13 @@ void GPUCommonHW::Execute_Prim(u32 op, u32 diff) { verts = Memory::GetPointerUnchecked(gstate_c.vertexAddr); inds = nullptr; if ((vertexType & GE_VTYPE_IDX_MASK) != GE_VTYPE_IDX_NONE) { - inds = Memory::GetPointerUnchecked(gstate_c.indexAddr); + const u32 indexAddr = gstate_c.indexAddr; + const int indexShift = ((vertexType & GE_VTYPE_IDX_MASK) >> GE_VTYPE_IDX_SHIFT) - 1; + if (!Memory::IsValidRange(gstate_c.indexAddr, count << indexShift)) { + // Bad index range. Let's give up the fast loop. + goto bail; + } + inds = Memory::GetPointerUnchecked(indexAddr); } else { // We can extend again after submitting a normal draw. canExtend = isTriangle; diff --git a/GPU/GPUState.h b/GPU/GPUState.h index b25bd443b2..0cb0517028 100644 --- a/GPU/GPUState.h +++ b/GPU/GPUState.h @@ -420,7 +420,6 @@ struct GPUgstate { int getTransferHeight() const { return ((transfersize >> 10) & 0x3FF) + 1; } int getTransferBpp() const { return (transferstart & 1) ? 4 : 2; } - void FastLoadBoneMatrix(u32 addr); // Real data in the context ends here