From 2b069011bea3fca93440afc62c57a4b2d7a1dbde Mon Sep 17 00:00:00 2001
From: Tharo <17233964+Thar0@users.noreply.github.com>
Date: Mon, 20 Jan 2025 22:52:03 +0000
Subject: Provide AVOID_UB for some bugs found in GCC compiler testing (#1785)

* Provide AVOID_UB for some bugs found in GCC compiler testing

Co-authored-by: Fig02 <fig02srl@gmail.com>

* Format

* Fix silly typo

* Mention MM3D in en_dnq bug comment

---------

Co-authored-by: Fig02 <fig02srl@gmail.com>
---
 src/code/z_message_nes.c | 19 +++++++++++++++++++
 src/code/z_player_lib.c  | 13 ++++++++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

(limited to 'src/code')

diff --git a/src/code/z_message_nes.c b/src/code/z_message_nes.c
index 5ce20df6e..eeea22495 100644
--- a/src/code/z_message_nes.c
+++ b/src/code/z_message_nes.c
@@ -1005,8 +1005,23 @@ void Message_DecodeNES(PlayState* play) {
     s16 value;
     u32 timeToMoonCrash;
     s16 i;
+#ifndef AVOID_UB
+    // UB: digits is accessed out-of-bounds below (see bug annotation).
+    // On the IDO compiler the stack in memory is in the reverse
+    // order to variable declarations, so this ends up accessing
+    // numLines.
     s16 numLines;
     s16 digits[4];
+#else
+    // Make this behavior consistent across compilers that allocate
+    // stack differently.
+    struct {
+        s16 digits[4];
+        s16 numLines;
+    } forceLayout;
+#define numLines (forceLayout.numLines)
+#define digits (forceLayout.digits)
+#endif
     s16 spC6 = 0;
     u16 sfxHi;
     f32 var_fs0;
@@ -1940,4 +1955,8 @@ void Message_DecodeNES(PlayState* play) {
         decodedBufPos++;
         msgCtx->msgBufPos++;
     }
+#ifdef AVOID_UB
+#undef numLines
+#undef digits
+#endif
 }
diff --git a/src/code/z_player_lib.c b/src/code/z_player_lib.c
index 2f3e06f59..c32f604d7 100644
--- a/src/code/z_player_lib.c
+++ b/src/code/z_player_lib.c
@@ -1307,6 +1307,17 @@ struct_80124618 D_801C0560[] = {
     { 2, { 95, 95, 100 } },
     { 3, { 105, 105, 100 } },
     { 5, { 102, 102, 102 } },
+#ifdef AVOID_UB
+    //! @bug gPlayerAnim_pz_gakkiplay uses this array with a frame count
+    //! of up to (and including) 6, which is larger than the last
+    //! keyframe frame number (5). This causes it to continue to read into
+    //! the next array in search of a keyframe that bounds frame 6.
+    // Avoid UB: Provide extra data elements that would be read in an
+    // out-of-bounds read from the next array. Both are read-only so are
+    // not expected to change.
+    { 0, { 100, 100, 100 } },
+    { 9, { 100, 100, 100 } },
+#endif
 };
 struct_80124618 D_801C0580[] = {
     { 0, { 100, 100, 100 } }, { 9, { 100, 100, 100 } }, { 10, { 150, 150, 150 } },
@@ -1581,7 +1592,7 @@ u8 Player_GetStrength(void) {
     return sPlayerStrengths[GET_PLAYER_FORM];
 }
 
-PlayerMask Player_GetMask(PlayState* play) {
+s32 Player_GetMask(PlayState* play) {
     Player* player = GET_PLAYER(play);
 
     return player->currentMask;
-- 
cgit v1.2.3