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>

include/z64player.h

@@ -1421,7 +1421,7 @@ void Player_SetAutoLockOnActor(struct PlayState* play, Actor* actor);
 s32 Player_SetBButtonAmmo(struct PlayState* play, s32 ammo);
 bool Player_IsBurningStickInRange(struct PlayState* play, Vec3f* pos, f32 xzRange, f32 yRange);
 u8 Player_GetStrength(void);
-PlayerMask Player_GetMask(struct PlayState* play);
+s32 Player_GetMask(struct PlayState* play);
 void Player_RemoveMask(struct PlayState* play);
 bool Player_HasMirrorShieldEquipped(struct PlayState* play);
 bool Player_IsHoldingMirrorShield(struct PlayState* play);

src/boot/yaz0.c

@@ -112,7 +112,7 @@ s32 Yaz0_DecompressImpl(u8* src, u8* dst) {
             // N = chunkSize; B = back offset
             // 3 bytes 0B BB NN
             // 2 bytes NB BB
-            chunkSize = (nibble == 0) ? *src++ + 0x12 : nibble + 2;
+            chunkSize = (nibble == 0) ? (u32)(*src++ + 0x12) : nibble + 2;
 
             do {
                 *dst++ = *(backPtr++ - 1);

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
 }

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;

src/overlays/actors/ovl_En_Dnq/z_en_dnq.c

@@ -456,9 +456,21 @@ void func_80A52FB8(EnDnq* this, PlayState* play) {
 
 void EnDnq_HandleCutscene(EnDnq* this, PlayState* play) {
     static s32 sCsAnimIndex[] = {
-        DEKU_KING_ANIM_IDLE,          DEKU_KING_ANIM_IDLE_MORPH,
-        DEKU_KING_ANIM_SURPRISE,      DEKU_KING_ANIM_JUMPED_ON_START,
-        DEKU_KING_ANIM_JUMPED_ON_END, DEKU_KING_ANIM_JUMPED_ON_END_MORPH,
+        DEKU_KING_ANIM_IDLE,
+        DEKU_KING_ANIM_IDLE_MORPH,
+        DEKU_KING_ANIM_SURPRISE,
+        DEKU_KING_ANIM_JUMPED_ON_START,
+        DEKU_KING_ANIM_JUMPED_ON_END,
+        DEKU_KING_ANIM_JUMPED_ON_END_MORPH,
+#ifdef AVOID_UB
+        //! @bug Z2_DEKU_KINGCutsceneData_008258 provides a cue id of 6, reading out-of-bounds
+        //! of this array that happens to be 0 padding.
+        //! Note that adding another idle anim entry still matches, since what follows this array is
+        //! section padding full of 0s. However in MM3D the data in this file is ordered differently,
+        //! proving that the array was originally missing an entry.
+        // Avoid UB: Add an explicit array member rather than relying on 0 padding.
+        DEKU_KING_ANIM_IDLE,
+#endif
     };
     s32 cueChannel;
     u32 cueId;

src/overlays/actors/ovl_player_actor/z_player.c

@@ -5671,7 +5671,7 @@ void func_80833864(PlayState* play, Player* this, PlayerMeleeWeaponAnimation mel
     }
 
     // Accumulate consecutive slashes to do the "third slash" types
-    if ((meleeWeaponAnim != this->meleeWeaponAnimation) || (this->unk_ADD >= 3)) {
+    if (((s32)meleeWeaponAnim != this->meleeWeaponAnimation) || (this->unk_ADD >= 3)) {
         this->unk_ADD = 0;
     }
 
@@ -11406,7 +11406,7 @@ void func_808425B4(Player* this) {
  *     - Tatl C-up icon for hints
  */
 void Player_UpdateInterface(PlayState* play, Player* this) {
-    DoAction doActionB;
+    s32 doActionB;
     s32 sp38;
 
     if (this != GET_PLAYER(play)) {
@@ -18584,12 +18584,19 @@ void func_80855218(PlayState* play, Player* this, struct_8085D910** arg2) {
     }
 }
 
+//! @bug This array may be indexed with PLAYER_FORM_HUMAN, causing an out-of-bounds access
 u16 D_8085D908[] = {
     WEEKEVENTREG_30_80, // PLAYER_FORM_FIERCE_DEITY
     WEEKEVENTREG_30_20, // PLAYER_FORM_GORON
     WEEKEVENTREG_30_40, // PLAYER_FORM_ZORA
     WEEKEVENTREG_30_10, // PLAYER_FORM_DEKU
+#ifdef AVOID_UB
+    // Avoid UB: Provide the data that would be read by indexing this with PLAYER_FORM_HUMAN.
+    // Both this array and D_8085D910 are read-only so this is not expected to change.
+    WEEKEVENTREG_16_02 | WEEKEVENTREG_16_08, // PLAYER_FORM_HUMAN
+#endif
 };
+
 struct_8085D910 D_8085D910[] = {
     { 0x10, 0xA, 0x3B, 0x3F },
     { 9, 0x32, 0xA, 0xD },