Skip to content

fix(vm): mask infix shift counts & 63 — kill << >> undefined behavior#443

Merged
InauguralPhysicist merged 1 commit into
mainfrom
fix/infix-shift-mask-ub
Jul 5, 2026
Merged

fix(vm): mask infix shift counts & 63 — kill << >> undefined behavior#443
InauguralPhysicist merged 1 commit into
mainfrom
fix/infix-shift-mask-ub

Conversation

@InauguralPhysicist

Copy link
Copy Markdown
Collaborator

What

The bit_shl/bit_shr builtins mask their shift count to [0,63], but the infix <</>> operators did not. A count ≥ 64 (or negative) on the int64 path was C undefined behavior1 << 64 is UB, not a defined result. docs/LANGUAGE_CONTRACT.md promises "shifts are defined, not UB"; the infix path silently violated its own contract.

No suite case exercised a count ≥ 64, so UBSan stayed green over a real latent bug. Surfaced during a documentation review that flagged the LANGUAGE_CONTRACT bitwise section as stale (it also still said int32 — a separate doc PR).

Fix

Refactor INT_BINOPINT_BINOP_R (parameterized RHS) so the shift ops apply (int64_t)_bd & 63 while & | ^ keep the plain operand. The JIT bails to the interpreter for OP_SHL/OP_SHR, so the VM macro is the only fix site.

Verification

  • tests/test_bitwise.eigs gains infix ≥ 64 cases proving parity with the builtins: 1 << 64 == 1, 1 << 65 == 2, 1 << 100 == 2^36, 256 >> 64 == 256.
  • Full suite 2519/2519, ASan+UBSan clean.
  • x86-64 output is unchanged (the hardware already masked mod-64) — the fix makes the behavior defined rather than accidental.

🤖 Generated with Claude Code

The bit_shl/bit_shr builtins already masked their shift count to [0,63]
(builtins.c), but the infix `<<`/`>>` operators in INT_BINOP did not — a
count >= 64 (or negative) on the int64 path was C undefined behavior.
docs/LANGUAGE_CONTRACT.md promises "shifts are defined, not UB"; the infix
path silently violated it. No suite case used a count >= 64, so UBSan never
tripped over the latent bug (found during a documentation review that flagged
the contract as stale).

Refactor INT_BINOP into INT_BINOP_R (parameterized RHS) so the shift ops mask
`(int64_t)_bd & 63` while `& | ^` keep the plain operand. Add infix >= 64 cases
to test_bitwise.eigs proving parity with the builtins (1 << 64 == 1,
1 << 100 == 2^36, 256 >> 64 == 256). x86-64 output is unchanged (hardware
already masked mod-64); the behavior is now defined instead of accidental. The
JIT bails to the interpreter for OP_SHL/OP_SHR, so this is the only fix site.

Full suite 2519/2519, ASan+UBSan clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@InauguralPhysicist InauguralPhysicist merged commit 41d5b92 into main Jul 5, 2026
17 checks passed
@InauguralPhysicist InauguralPhysicist deleted the fix/infix-shift-mask-ub branch July 5, 2026 06:09
InauguralPhysicist added a commit that referenced this pull request Jul 5, 2026
…ly hang (#451)

Overnight 2026-07-05, 5 of ~15 CI runs hung in tsan_seeded_race.eigs
and burned the 6h default job timeout before being auto-cancelled —
including code-touching runs (#443) and pure docs pushes, so the hang
is content-independent. All 9 race-free slice tests had passed in
every hung run; the hang is downstream of the DELIBERATE lost-update
race (two threads set_at the same slot; both can decref the displaced
element — double-decref corruption may legally spin). 0/30 local
repros on the 2-core box vs ~1/3 on 4-core runners — consistent with
the CI-TSan-sees-more rule.

The gate now bounds every run with timeout (default 120s,
TSAN_RUN_TIMEOUT to override):
- a race-free slice test that times out FAILs loudly in minutes as a
  liveness bug (was: silent 6h runner burn),
- the seeded race may be killed after its warnings are counted —
  TSan emits warnings as they happen, so detection (the property the
  gate asserts) survives the kill,
- ci.yml adds timeout-minutes: 20 as the job-level backstop.

tsan_warnings returns via globals, deliberately not echo-and-capture:
w=$(fn) runs fn in a subshell and drops LAST_RC, silently disabling
the hang branch — caught by a planted-hang probe, which now validates
both branches (clean pass exit 0, planted hang exit 1).

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant