fix(qcommon,game): cast LHS of left shifts to unsigned to avoid signed-int UB#219
Open
blackden wants to merge 1 commit into
Open
fix(qcommon,game): cast LHS of left shifts to unsigned to avoid signed-int UB#219blackden wants to merge 1 commit into
blackden wants to merge 1 commit into
Conversation
…d-int UB UBSAN flagged 5 left-shift patterns where a positive int value, when shifted, becomes representable only as an unsigned int — undefined behavior by C11 §6.5.7p4. All sites use bit masks where the eventual storage type is treated as a bit pattern, so casting the LHS to unsigned is bit-identical on 2's-complement (i.e., everywhere) but standards-clean. Sites fixed: - code/qcommon/q_shared.c COM_BitCheck/Set/Clear: `1 << bitNum` where bitNum may reach 31. - code/qcommon/msg.c MSG_Write/ReadDeltaPlayerstate: 27 occurrences of `1 << i` where i iterates over MAX_STATS / MAX_PERSISTANT / MAX_POWERUPS / MAX_PERKS / MAX_WEAPONS / etc., which can reach 31. Wire format is bit-identical — verified by inspection. - code/game/g_misc.c SP_dlight: `(i / 4) << 24` where i/4 routinely exceeds 127, causing the shift result to overflow signed int. No behavior change on supported targets.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes 5 instances of signed-integer left-shift UB caught by UBSAN on a
-fsanitize=undefinedbuild of the macOS arm64 port. All sites use bit masks where the integer is treated as a bit pattern, so casting the LHS tounsignedis bit-identical on 2's-complement (i.e., everywhere) but standards-clean.code/qcommon/q_shared.cCOM_BitCheck / COM_BitSet / COM_BitClear —1 << bitNumwherebitNummay reach 31.code/qcommon/msg.cMSG_Write/ReadDeltaPlayerstate — 27 sites of1 << iwhereiiterates overMAX_STATS/MAX_PERSISTANT/MAX_POWERUPS/MAX_PERKS/MAX_WEAPONS/MAX_AMMO/ etc., which can reach 31. Wire format is bit-identical (verified by inspection and by the UBSAN before/after in the test plan).code/game/g_misc.cSP_dlight color pack —( i / 4 ) << 24wherei/4routinely exceeds 127.All three are vanilla-Q3-lineage code; the same lines exist in the id Software 2010 RTCW SP source drop, so this PR is also a candidate to forward to ioquake3 / RTCW MP downstream.
Test plan
left shift of N by M places cannot be represented in type 'int'hits → 0map escape1+give all+s_useOpenAL 0+snd_restart+find/cmdlist/imagelist/savegame/disconnect/quit); no functional regression observedMSG_*DeltaPlayerstatechanges touch the snapshot serialisation path)Found during a broader UBSAN triage of the macOS arm64 build alongside the other macOS PRs (#216, #217, #218). A companion PR follows for a separate UBSAN finding in cgame.
🤖 Generated with Claude Code