From 6e7a9b47f0078eb1ed8da18b5211bfd2e181097f Mon Sep 17 00:00:00 2001 From: David Racine Date: Fri, 9 Oct 2026 09:49:46 -0400 Subject: [PATCH] Fix undefined left shifts of 1 by 31 (#6640) Shifting a signed 1 into the sign bit is undefined, and UBSan reports it as "left shift of 1 by 31 places cannot be represented in type 'int'". The switch-flag helpers in z_actor.c take the bit position at runtime, so every flag from 0x00 to 0x3F goes through the same shift. Flag 0x3F lands on bit 31: the Potion Shop Granny sets and clears it while brewing the Odd Potion, so this is reached in ordinary play, not just in theory. The same shift appears as a constant in gBitFlags, in the two player state flags that occupy bit 31, and in ENHORSE_FLAG_31. Suffixing the 1 with u moves the shift to unsigned, where the value is defined. Every flag field involved is already u32, so the results are unchanged. --- soh/include/z64player.h | 4 +- soh/src/code/z_actor.c | 48 +++++++++---------- soh/src/code/z_inventory.c | 10 ++-- .../overlays/actors/ovl_En_Horse/z_en_horse.h | 2 +- 4 files changed, 32 insertions(+), 32 deletions(-) diff --git a/soh/include/z64player.h b/soh/include/z64player.h index a9c466a1db..2898249960 100644 --- a/soh/include/z64player.h +++ b/soh/include/z64player.h @@ -698,7 +698,7 @@ typedef struct PendingFlag { #define PLAYER_STATE1_IN_ITEM_CS (1 << 28) #define PLAYER_STATE1_IN_CUTSCENE (1 << 29) #define PLAYER_STATE1_LOCK_ON_FORCED_TO_RELEASE (1 << 30) // Lock-on was released automatically, for example by leaving the lock-on leash range -#define PLAYER_STATE1_FLOOR_DISABLED (1 << 31) //Used for grottos +#define PLAYER_STATE1_FLOOR_DISABLED (1u << 31) //Used for grottos #define PLAYER_STATE2_DO_ACTION_GRAB (1 << 0) #define PLAYER_STATE2_CAN_ACCEPT_TALK_OFFER (1 << 1) // Can accept a talk offer. "Speak" or "Check" is shown on the A button. @@ -731,7 +731,7 @@ typedef struct PendingFlag { #define PLAYER_STATE2_IDLE_FIDGET (1 << 28) // Playing a fidget idle animation (under typical circumstances, see `Player_ChooseNextIdleAnim` for more info) #define PLAYER_STATE2_DISABLE_DRAW (1 << 29) #define PLAYER_STATE2_SWORD_LUNGE (1 << 30) -#define PLAYER_STATE2_FORCED_VOID_OUT (1 << 31) +#define PLAYER_STATE2_FORCED_VOID_OUT (1u << 31) #define PLAYER_STATE3_IGNORE_CEILING_FLOOR_WATER (1 << 0) #define PLAYER_STATE3_MIDAIR (1 << 1) diff --git a/soh/src/code/z_actor.c b/soh/src/code/z_actor.c index 77c5abce37..1d2899129e 100644 --- a/soh/src/code/z_actor.c +++ b/soh/src/code/z_actor.c @@ -665,9 +665,9 @@ void Attention_Update(TargetContext* targetCtx, Player* player, Actor* actorArg, */ s32 Flags_GetSwitch(PlayState* play, s32 flag) { if (flag < 0x20) { - return play->actorCtx.flags.swch & (1 << flag); + return play->actorCtx.flags.swch & (1u << flag); } else { - return play->actorCtx.flags.tempSwch & (1 << (flag - 0x20)); + return play->actorCtx.flags.tempSwch & (1u << (flag - 0x20)); } } @@ -677,9 +677,9 @@ s32 Flags_GetSwitch(PlayState* play, s32 flag) { void Flags_SetSwitch(PlayState* play, s32 flag) { u8 previouslyOff = !Flags_GetSwitch(play, flag); if (flag < 0x20) { - play->actorCtx.flags.swch |= (1 << flag); + play->actorCtx.flags.swch |= (1u << flag); } else { - play->actorCtx.flags.tempSwch |= (1 << (flag - 0x20)); + play->actorCtx.flags.tempSwch |= (1u << (flag - 0x20)); } if (previouslyOff) { LUSLOG_INFO("Switch Flag Set - %#x", flag); @@ -693,9 +693,9 @@ void Flags_SetSwitch(PlayState* play, s32 flag) { void Flags_UnsetSwitch(PlayState* play, s32 flag) { u8 previouslyOn = Flags_GetSwitch(play, flag); if (flag < 0x20) { - play->actorCtx.flags.swch &= ~(1 << flag); + play->actorCtx.flags.swch &= ~(1u << flag); } else { - play->actorCtx.flags.tempSwch &= ~(1 << (flag - 0x20)); + play->actorCtx.flags.tempSwch &= ~(1u << (flag - 0x20)); } if (previouslyOn) { LUSLOG_INFO("Switch Flag Unset - %#x", flag); @@ -708,9 +708,9 @@ void Flags_UnsetSwitch(PlayState* play, s32 flag) { */ s32 Flags_GetUnknown(PlayState* play, s32 flag) { if (flag < 0x20) { - return play->actorCtx.flags.unk0 & (1 << flag); + return play->actorCtx.flags.unk0 & (1u << flag); } else { - return play->actorCtx.flags.unk1 & (1 << (flag - 0x20)); + return play->actorCtx.flags.unk1 & (1u << (flag - 0x20)); } } @@ -719,9 +719,9 @@ s32 Flags_GetUnknown(PlayState* play, s32 flag) { */ void Flags_SetUnknown(PlayState* play, s32 flag) { if (flag < 0x20) { - play->actorCtx.flags.unk0 |= (1 << flag); + play->actorCtx.flags.unk0 |= (1u << flag); } else { - play->actorCtx.flags.unk1 |= (1 << (flag - 0x20)); + play->actorCtx.flags.unk1 |= (1u << (flag - 0x20)); } } @@ -730,9 +730,9 @@ void Flags_SetUnknown(PlayState* play, s32 flag) { */ void Flags_UnsetUnknown(PlayState* play, s32 flag) { if (flag < 0x20) { - play->actorCtx.flags.unk0 &= ~(1 << flag); + play->actorCtx.flags.unk0 &= ~(1u << flag); } else { - play->actorCtx.flags.unk1 &= ~(1 << (flag - 0x20)); + play->actorCtx.flags.unk1 &= ~(1u << (flag - 0x20)); } } @@ -740,7 +740,7 @@ void Flags_UnsetUnknown(PlayState* play, s32 flag) { * Tests if current scene chest flag is set. */ s32 Flags_GetTreasure(PlayState* play, s32 flag) { - return play->actorCtx.flags.chest & (1 << flag); + return play->actorCtx.flags.chest & (1u << flag); } /** @@ -748,7 +748,7 @@ s32 Flags_GetTreasure(PlayState* play, s32 flag) { */ void Flags_SetTreasure(PlayState* play, s32 flag) { u8 previouslyOff = !Flags_GetTreasure(play, flag); - play->actorCtx.flags.chest |= (1 << flag); + play->actorCtx.flags.chest |= (1u << flag); if (previouslyOff) { LUSLOG_INFO("Treasure Flag Set - %#x", flag); GameInteractor_ExecuteOnSceneFlagSet(play->sceneNum, FLAG_SCENE_TREASURE, flag); @@ -759,7 +759,7 @@ void Flags_SetTreasure(PlayState* play, s32 flag) { * Tests if current scene clear flag is set. */ s32 Flags_GetClear(PlayState* play, s32 flag) { - return play->actorCtx.flags.clear & (1 << flag); + return play->actorCtx.flags.clear & (1u << flag); } /** @@ -767,7 +767,7 @@ s32 Flags_GetClear(PlayState* play, s32 flag) { */ void Flags_SetClear(PlayState* play, s32 flag) { u8 previouslyOff = !Flags_GetClear(play, flag); - play->actorCtx.flags.clear |= (1 << flag); + play->actorCtx.flags.clear |= (1u << flag); if (previouslyOff) { LUSLOG_INFO("Clear Flag Set - %#x", flag); GameInteractor_ExecuteOnSceneFlagSet(play->sceneNum, FLAG_SCENE_CLEAR, flag); @@ -779,7 +779,7 @@ void Flags_SetClear(PlayState* play, s32 flag) { */ void Flags_UnsetClear(PlayState* play, s32 flag) { u8 previouslyOn = Flags_GetClear(play, flag); - play->actorCtx.flags.clear &= ~(1 << flag); + play->actorCtx.flags.clear &= ~(1u << flag); if (previouslyOn) { LUSLOG_INFO("Clear Flag Unset - %#x", flag); GameInteractor_ExecuteOnSceneFlagUnset(play->sceneNum, FLAG_SCENE_CLEAR, flag); @@ -790,21 +790,21 @@ void Flags_UnsetClear(PlayState* play, s32 flag) { * Tests if current scene temp clear flag is set. */ s32 Flags_GetTempClear(PlayState* play, s32 flag) { - return play->actorCtx.flags.tempClear & (1 << flag); + return play->actorCtx.flags.tempClear & (1u << flag); } /** * Sets current scene temp clear flag. */ void Flags_SetTempClear(PlayState* play, s32 flag) { - play->actorCtx.flags.tempClear |= (1 << flag); + play->actorCtx.flags.tempClear |= (1u << flag); } /** * Unsets current scene temp clear flag. */ void Flags_UnsetTempClear(PlayState* play, s32 flag) { - play->actorCtx.flags.tempClear &= ~(1 << flag); + play->actorCtx.flags.tempClear &= ~(1u << flag); } /** @@ -812,9 +812,9 @@ void Flags_UnsetTempClear(PlayState* play, s32 flag) { */ s32 Flags_GetCollectible(PlayState* play, s32 flag) { if (flag < 0x20) { - return play->actorCtx.flags.collect & (1 << flag); + return play->actorCtx.flags.collect & (1u << flag); } else { - return play->actorCtx.flags.tempCollect & (1 << (flag - 0x20)); + return play->actorCtx.flags.tempCollect & (1u << (flag - 0x20)); } } @@ -825,9 +825,9 @@ void Flags_SetCollectible(PlayState* play, s32 flag) { u8 previouslyOff = !Flags_GetCollectible(play, flag); if (flag != 0) { if (flag < 0x20) { - play->actorCtx.flags.collect |= (1 << flag); + play->actorCtx.flags.collect |= (1u << flag); } else { - play->actorCtx.flags.tempCollect |= (1 << (flag - 0x20)); + play->actorCtx.flags.tempCollect |= (1u << (flag - 0x20)); } } if (previouslyOff) { diff --git a/soh/src/code/z_inventory.c b/soh/src/code/z_inventory.c index f89f20afb7..aeca8f28ee 100644 --- a/soh/src/code/z_inventory.c +++ b/soh/src/code/z_inventory.c @@ -5,12 +5,12 @@ #include "textures/parameter_static/parameter_static.h" #include -// Bit Flag array in which gBitFlags[n] is literally (1 << n) +// Bit Flag array in which gBitFlags[n] is literally (1u << n) u32 gBitFlags[] = { - (1 << 0), (1 << 1), (1 << 2), (1 << 3), (1 << 4), (1 << 5), (1 << 6), (1 << 7), - (1 << 8), (1 << 9), (1 << 10), (1 << 11), (1 << 12), (1 << 13), (1 << 14), (1 << 15), - (1 << 16), (1 << 17), (1 << 18), (1 << 19), (1 << 20), (1 << 21), (1 << 22), (1 << 23), - (1 << 24), (1 << 25), (1 << 26), (1 << 27), (1 << 28), (1 << 29), (1 << 30), (1 << 31), + (1u << 0), (1u << 1), (1u << 2), (1u << 3), (1u << 4), (1u << 5), (1u << 6), (1u << 7), + (1u << 8), (1u << 9), (1u << 10), (1u << 11), (1u << 12), (1u << 13), (1u << 14), (1u << 15), + (1u << 16), (1u << 17), (1u << 18), (1u << 19), (1u << 20), (1u << 21), (1u << 22), (1u << 23), + (1u << 24), (1u << 25), (1u << 26), (1u << 27), (1u << 28), (1u << 29), (1u << 30), (1u << 31), }; u16 gEquipMasks[] = { 0x000F, 0x00F0, 0x0F00, 0xF000 }; diff --git a/soh/src/overlays/actors/ovl_En_Horse/z_en_horse.h b/soh/src/overlays/actors/ovl_En_Horse/z_en_horse.h index 6724faf475..83ac9499e1 100644 --- a/soh/src/overlays/actors/ovl_En_Horse/z_en_horse.h +++ b/soh/src/overlays/actors/ovl_En_Horse/z_en_horse.h @@ -59,7 +59,7 @@ typedef enum { #define ENHORSE_FLAG_28 (1 << 28) /* 0x1000 0000 */ #define ENHORSE_FLAG_29 (1 << 29) /* 0x2000 0000 */ #define ENHORSE_FLAG_30 (1 << 30) /* 0x4000 0000 */ -#define ENHORSE_FLAG_31 (1 << 31) /* 0x8000 0000 */ +#define ENHORSE_FLAG_31 (1u << 31) /* 0x8000 0000 */ struct EnHorse;