mirror/Shipwright
1
0
Fork 0
mirror of https://github.com/HarbourMasters/Shipwright synced 2026-10-14 07:43:40 -04:00
Code Issues Packages Projects Releases Wiki Activity

Fix undefined left shifts of 1 by 31 (#6640)

Browse Source 
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.
This commit is contained in:
David Racine
2026-10-09 09:49:46 -04:00
committed by GitHub
parent 1595a94df3
commit 6e7a9b47f0
4 changed files with 32 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
soh/include/z64player.h
+2 -2
View File
@@ -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)
soh/src/code/z_actor.c
+24 -24
View File
@@ -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) {
soh/src/code/z_inventory.c
+5 -5
View File
@@ -5,12 +5,12 @@
#include "textures/parameter_static/parameter_static.h"
#include <soh_assets.h>
// 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 };
soh/src/overlays/actors/ovl_En_Horse/z_en_horse.h
+1 -1
View File
@@ -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;