diff --git a/include/z64player.h b/include/z64player.h index 277af318b2..1d5244d286 100644 --- a/include/z64player.h +++ b/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); diff --git a/src/boot/yaz0.c b/src/boot/yaz0.c index 2583a0ffc2..70e42e77ba 100644 --- a/src/boot/yaz0.c +++ b/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); diff --git a/src/code/z_message_nes.c b/src/code/z_message_nes.c index 5ce20df6ec..eeea224953 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 2f3e06f595..c32f604d75 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; diff --git a/src/overlays/actors/ovl_En_Dnq/z_en_dnq.c b/src/overlays/actors/ovl_En_Dnq/z_en_dnq.c index 0199adbe30..a23fce2aa2 100644 --- a/src/overlays/actors/ovl_En_Dnq/z_en_dnq.c +++ b/src/overlays/actors/ovl_En_Dnq/z_en_dnq.c @@ -456,9 +456,18 @@ 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. + // Avoid UB: Add an explicit array member rather than relying on 0 padding. + DEKU_KING_ANIM_IDLE, +#endif }; s32 cueChannel; u32 cueId; diff --git a/src/overlays/actors/ovl_player_actor/z_player.c b/src/overlays/actors/ovl_player_actor/z_player.c index 8fc696f2ab..96e096fe22 100644 --- a/src/overlays/actors/ovl_player_actor/z_player.c +++ b/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 },