From 92d662cb716150892e2cfafd36a31ca39b2bae77 Mon Sep 17 00:00:00 2001 From: rogerman Date: Sat, 18 Feb 2023 10:54:02 -0800 Subject: [PATCH] GFX3D: Fix longstanding potential overflows related to polygon lists by reducing POLYLIST_SIZE from 20000 to 16383. - Historically (ever since commit b1e4934 and commit d5bb6fd), VERTLIST_SIZE (based on POLYLIST_SIZE) could reference an index over 66535, but POLY.vertIndexes has always been an unsigned 16-bit value with a max value of 66535, making for a potential overflow. In practice, an overflow has never happened in the past 15 years because a hardware NDS has a limit of 6144 polygons (or 24576 vertices), and that even a VERTLIST_SIZE of 80000 is way more than plenty to accommodate that. Rather than increasing POLY.vertIndexes to 32-bit, it makes more sense to reduce POLYLIST_SIZE to 16383 to keep VERTLIST_SIZE below 66536. Even with this change, POLYLIST_SIZE and VERTLIST_SIZE should be more than plenty in practice. - Also, now that we're performing polygon clipping for all client 3D renderers (ever since commit e06d11f), we're finally accounting for the fact that the clipped polygon list size is larger than POLYLIST_SIZE. Client 3D renderers have been updated to now reflect this change and avoid theoretical overflow issues that have never actually happened in practice. --- desmume/src/OGLRender.h | 4 ++-- desmume/src/OGLRender_3_2.cpp | 2 +- desmume/src/OGLRender_3_2.h | 2 +- desmume/src/gfx3d.h | 10 +++++----- desmume/src/rasterize.cpp | 2 +- desmume/src/rasterize.h | 4 ++-- desmume/src/render3D.cpp | 2 +- desmume/src/render3D.h | 2 +- 8 files changed, 14 insertions(+), 14 deletions(-) diff --git a/desmume/src/OGLRender.h b/desmume/src/OGLRender.h index becaac9b2..4aa64a3e5 100755 --- a/desmume/src/OGLRender.h +++ b/desmume/src/OGLRender.h @@ -293,7 +293,7 @@ EXTERNOGLEXT(PFNGLDELETESYNCPROC, glDeleteSync) // Core in v3.2 #define OGLRENDER_MINIMUM_DRIVER_VERSION_REQUIRED_MINOR 2 #define OGLRENDER_MINIMUM_DRIVER_VERSION_REQUIRED_REVISION 0 -#define OGLRENDER_VERT_INDEX_BUFFER_COUNT (POLYLIST_SIZE * 6) +#define OGLRENDER_VERT_INDEX_BUFFER_COUNT (CLIPPED_POLYLIST_SIZE * 6) // Assign the FBO attachments for the main geometry render #ifdef OGLRENDER_3_2_H @@ -736,7 +736,7 @@ protected: bool _enableMultisampledRendering; int _selectedMultisampleSize; - bool _isPolyFrontFacing[POLYLIST_SIZE]; + bool _isPolyFrontFacing[CLIPPED_POLYLIST_SIZE]; size_t _clearImageIndex; Render3DError FlushFramebuffer(const FragmentColor *__restrict srcFramebuffer, FragmentColor *__restrict dstFramebufferMain, u16 *__restrict dstFramebuffer16); diff --git a/desmume/src/OGLRender_3_2.cpp b/desmume/src/OGLRender_3_2.cpp index 62a53b3af..39f528c62 100755 --- a/desmume/src/OGLRender_3_2.cpp +++ b/desmume/src/OGLRender_3_2.cpp @@ -1176,7 +1176,7 @@ Render3DError OpenGLRenderer_3_2::CreateGeometryPrograms() // Set up poly states TBO glGenBuffers(1, &OGLRef.tboPolyStatesID); glBindBuffer(GL_TEXTURE_BUFFER, OGLRef.tboPolyStatesID); - glBufferData(GL_TEXTURE_BUFFER, POLYLIST_SIZE * sizeof(OGLPolyStates), NULL, GL_DYNAMIC_DRAW); + glBufferData(GL_TEXTURE_BUFFER, CLIPPED_POLYLIST_SIZE * sizeof(OGLPolyStates), NULL, GL_DYNAMIC_DRAW); glGenTextures(1, &OGLRef.texPolyStatesID); glActiveTexture(GL_TEXTURE0 + OGLTextureUnitID_PolyStates); diff --git a/desmume/src/OGLRender_3_2.h b/desmume/src/OGLRender_3_2.h index 513d81478..af40f69b3 100644 --- a/desmume/src/OGLRender_3_2.h +++ b/desmume/src/OGLRender_3_2.h @@ -71,7 +71,7 @@ protected: bool _isConservativeDepthAMDSupported; GLsync _syncBufferSetup; - CACHE_ALIGN OGLPolyStates _pendingPolyStates[POLYLIST_SIZE]; + CACHE_ALIGN OGLPolyStates _pendingPolyStates[CLIPPED_POLYLIST_SIZE]; virtual Render3DError InitExtensions(); diff --git a/desmume/src/gfx3d.h b/desmume/src/gfx3d.h index ec21b0257..e9f907e9e 100644 --- a/desmume/src/gfx3d.h +++ b/desmume/src/gfx3d.h @@ -535,9 +535,9 @@ bool GFX3D_IsPolyWireframe(const POLY &p); bool GFX3D_IsPolyOpaque(const POLY &p); bool GFX3D_IsPolyTranslucent(const POLY &p); -#define POLYLIST_SIZE 20000 +#define POLYLIST_SIZE 16383 +#define CLIPPED_POLYLIST_SIZE (POLYLIST_SIZE * 2) #define VERTLIST_SIZE (POLYLIST_SIZE * 4) -#define INDEXLIST_SIZE (POLYLIST_SIZE * 4) #include "PACKED.h" @@ -778,7 +778,7 @@ struct GFX3D_GeometryList { PAGE_ALIGN VERT rawVertList[VERTLIST_SIZE]; PAGE_ALIGN POLY rawPolyList[POLYLIST_SIZE]; - PAGE_ALIGN CPoly clippedPolyList[POLYLIST_SIZE]; + PAGE_ALIGN CPoly clippedPolyList[CLIPPED_POLYLIST_SIZE]; size_t rawVertCount; size_t rawPolyCount; @@ -907,8 +907,8 @@ struct GFX3D u32 render3DFrameCount; // Increments when gfx3d_doFlush() is called. Resets every 60 video frames. // Working lists for rendering. - CACHE_ALIGN CPoly clippedPolyUnsortedList[POLYLIST_SIZE * 2]; // Records clipped polygon info on first pass - CACHE_ALIGN u16 indexOfClippedPolyUnsortedList[POLYLIST_SIZE * 2]; + CACHE_ALIGN CPoly clippedPolyUnsortedList[CLIPPED_POLYLIST_SIZE]; // Records clipped polygon info on first pass + CACHE_ALIGN u16 indexOfClippedPolyUnsortedList[CLIPPED_POLYLIST_SIZE]; CACHE_ALIGN float rawPolySortYMin[POLYLIST_SIZE]; // Temp buffer used for processing polygon Y-sorting CACHE_ALIGN float rawPolySortYMax[POLYLIST_SIZE]; // Temp buffer used for processing polygon Y-sorting diff --git a/desmume/src/rasterize.cpp b/desmume/src/rasterize.cpp index 5656fb001..5d076c2f1 100644 --- a/desmume/src/rasterize.cpp +++ b/desmume/src/rasterize.cpp @@ -1733,7 +1733,7 @@ SoftRasterizerRenderer::SoftRasterizerRenderer() _deviceInfo.maxAnisotropy = 1.0f; _deviceInfo.maxSamples = 0; - _clippedPolyList = (CPoly *)malloc_alignedCacheLine(POLYLIST_SIZE * 2 * sizeof(CPoly)); + _clippedPolyList = (CPoly *)malloc_alignedCacheLine(CLIPPED_POLYLIST_SIZE * sizeof(CPoly)); _task = NULL; diff --git a/desmume/src/rasterize.h b/desmume/src/rasterize.h index 3eea54483..6f9c2685e 100644 --- a/desmume/src/rasterize.h +++ b/desmume/src/rasterize.h @@ -186,8 +186,8 @@ public: int _debug_drawClippedUserPoly; CACHE_ALIGN FragmentColor toonColor32LUT[32]; FragmentAttributesBuffer *_framebufferAttributes; - bool isPolyVisible[POLYLIST_SIZE]; - bool isPolyBackFacing[POLYLIST_SIZE]; + bool isPolyVisible[CLIPPED_POLYLIST_SIZE]; + bool isPolyBackFacing[CLIPPED_POLYLIST_SIZE]; GFX3D_State *currentRenderState; bool _enableFragmentSamplingHack; diff --git a/desmume/src/render3D.cpp b/desmume/src/render3D.cpp index ebf002ed5..0beef10d2 100644 --- a/desmume/src/render3D.cpp +++ b/desmume/src/render3D.cpp @@ -240,7 +240,7 @@ Render3D::Render3D() _textureDeposterizeSrcSurface.Height = _textureDeposterizeDstSurface.Height = 1; _textureDeposterizeSrcSurface.Pitch = _textureDeposterizeDstSurface.Pitch = 1; - for (size_t i = 0; i < POLYLIST_SIZE; i++) + for (size_t i = 0; i < CLIPPED_POLYLIST_SIZE; i++) { _textureList[i] = NULL; } diff --git a/desmume/src/render3D.h b/desmume/src/render3D.h index c052dd18e..82fec9011 100644 --- a/desmume/src/render3D.h +++ b/desmume/src/render3D.h @@ -183,7 +183,7 @@ protected: SSurface _textureDeposterizeDstSurface; u32 *_textureUpscaleBuffer; - Render3DTexture *_textureList[POLYLIST_SIZE]; + Render3DTexture *_textureList[CLIPPED_POLYLIST_SIZE]; size_t _clippedPolyCount; size_t _clippedPolyOpaqueCount;