From 922389fd6b61ec12d571a0bb1839d7415e693537 Mon Sep 17 00:00:00 2001 From: mzxrules Date: Tue, 3 Jun 2025 23:25:28 -0400 Subject: [PATCH 1/3] Document SysCfb_GetFbPtr bug --- src/code/graph.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/code/graph.c b/src/code/graph.c index 8a4646808f..63c2fdba91 100644 --- a/src/code/graph.c +++ b/src/code/graph.c @@ -141,6 +141,9 @@ void Graph_InitTHGA(GraphicsContext* gfxCtx) { gfxCtx->overlayBuffer = pool->overlayBuffer; gfxCtx->workBuffer = pool->workBuffer; + //! @bug fbIdx is a signed integer that can overflow into the negatives. When compiled with IDO, the remainder + //! operator will yield -1 for odd negative values of fbIdx (i.e. the same as C99 onwards). + //! This results in an out of bounds array access in SysCfb_GetFbPtr due to the negative index value. gfxCtx->curFrameBuffer = SysCfb_GetFbPtr(gfxCtx->fbIdx % 2); gfxCtx->unk_014 = 0; } From 8cbdc701dcb6f5e424832cec42eeb3f13b1d4fbb Mon Sep 17 00:00:00 2001 From: mzxrules Date: Thu, 5 Jun 2025 16:04:13 -0400 Subject: [PATCH 2/3] cleaner comment --- src/code/graph.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/code/graph.c b/src/code/graph.c index 63c2fdba91..34a0a931d7 100644 --- a/src/code/graph.c +++ b/src/code/graph.c @@ -141,9 +141,12 @@ void Graph_InitTHGA(GraphicsContext* gfxCtx) { gfxCtx->overlayBuffer = pool->overlayBuffer; gfxCtx->workBuffer = pool->workBuffer; - //! @bug fbIdx is a signed integer that can overflow into the negatives. When compiled with IDO, the remainder - //! operator will yield -1 for odd negative values of fbIdx (i.e. the same as C99 onwards). + //! @bug fbIdx is a signed integer that can overflow into the negatives. When compiled with a C99+ compiler or IDO, + //! the remainder operator will yield -1 for odd negative values of fbIdx. //! This results in an out of bounds array access in SysCfb_GetFbPtr due to the negative index value. + //! + //! In practice, this isn't issue. In the worst case scenario with the game operating at a consistent 60 FPS, + //! it would take approximately 414.25 days of continuous operation for fbIdx to overflow. gfxCtx->curFrameBuffer = SysCfb_GetFbPtr(gfxCtx->fbIdx % 2); gfxCtx->unk_014 = 0; } From 74a415a643ffc99fc22c910a35747b205309a6aa Mon Sep 17 00:00:00 2001 From: mzxrules Date: Fri, 6 Jun 2025 20:54:51 -0400 Subject: [PATCH 3/3] fixes --- src/code/graph.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/code/graph.c b/src/code/graph.c index 34a0a931d7..6ecf863dbc 100644 --- a/src/code/graph.c +++ b/src/code/graph.c @@ -143,9 +143,10 @@ void Graph_InitTHGA(GraphicsContext* gfxCtx) { //! @bug fbIdx is a signed integer that can overflow into the negatives. When compiled with a C99+ compiler or IDO, //! the remainder operator will yield -1 for odd negative values of fbIdx. - //! This results in an out of bounds array access in SysCfb_GetFbPtr due to the negative index value. + //! This results in an out of bounds array access in SysCfb_GetFbPtr due to the negative index value, + //! which will crash the game. //! - //! In practice, this isn't issue. In the worst case scenario with the game operating at a consistent 60 FPS, + //! In practice, this isn't an issue. In the worst case scenario with the game operating at a consistent 60 FPS, //! it would take approximately 414.25 days of continuous operation for fbIdx to overflow. gfxCtx->curFrameBuffer = SysCfb_GetFbPtr(gfxCtx->fbIdx % 2); gfxCtx->unk_014 = 0;