Skip to content

fix(interp): clamp fuel without dead overwrite#47

Merged
siyul-park merged 18 commits into
mainfrom
claude/code-quality-review-cxnXJ
May 29, 2026
Merged

fix(interp): clamp fuel without dead overwrite#47
siyul-park merged 18 commits into
mainfrom
claude/code-quality-review-cxnXJ

Conversation

@siyul-park

Copy link
Copy Markdown
Owner

The overflow guard assigned the clamped value, then an unconditional
fuel = int64(ticks) overwrote it, so ticks > 2^63-1 wrapped negative.
Collapse to fuel = int64(min(ticks, 1<<63-1)).

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32

claude and others added 18 commits May 29, 2026 09:04
The overflow guard assigned the clamped value, then an unconditional
fuel = int64(ticks) overwrote it, so ticks > 2^63-1 wrapped negative.
Collapse to fuel = int64(min(ticks, 1<<63-1)).

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32
- interp: inline trivial frame() accessor into i.fr
- interp: extract alloc free-list reuse into one reuse() helper
- types: drop MapKey.String's unused typ param and dead string branch
- cli: merge printLocals/printGlobals into printIndexed
- cli: replace hasJIT with a prof.JIT zero-value comparison

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32
Remove exported symbols with no production callers (test-only):
Encode, Memory.Write (+ErrCodeTooLarge), RegMask.{Empty,List,And,Or,
Not,Sub}, RegAlloc.{Get,Clone}, RegInfo.IsReserved, InvalidRegID.

Excise their dedicated tests; rewrite TestRegAlloc_Reserve/Reset and
TestMemory_Writable to assert via the public API instead of the
removed Get/Write methods.

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32
Add a `replace` helper beside `fold` that folds [ip,ip+width) to an
instruction and returns the next ip. Each of the ~70 fold cases drops
from a 4-5 line fold/width/advance sequence to a single call, leaving
only the genuine per-case difference (the arithmetic and result
opcode). Div-by-zero guards and the right-align NOP-pad invariant are
preserved unchanged.

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32
- Remove the production-dead pass[T]/New (the Manager dispatches by
  reflection on Run); tests build inline passes via a test-local
  funcPass[T] adapter instead.
- NewBasicBlocksPass returns the concrete *BasicBlocksPass, matching the
  other pass constructors; the Pass[T] assertion still documents conformance.
- Unify the two invalidJumpError helpers: drop transform's and inline its
  uses against analysis.ErrInvalidJump; move analysis's helper to the
  private-function slot below link.

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32
- Extract a generic dedup helper for the parallel constant/type
  index-remap blocks in cd.go (init, mark-used, transitive collapse,
  compact), called once each for constants (==) and types (Equals).
- Extract a shared `functions` helper returning a program's executable
  functions (implicit root over prog.Code plus *types.Function
  constants), replacing the identical inline construction in cf.go,
  dce.go, and cd.go.

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32
Replace the stop/stoppedFlag pair with a nilable *Stop, and the
skip/skipFunc/skipIP/skipDepth quartet with a nilable *skipPoint
(nil = not stopped / not skipping), removing the zero-value-Stop
ambiguity that forced the bool. Regroup fields per the config →
data → state → counter layering. The lazy init() guard stays: a
zero-value Debugger is a supported public-API path (exercised by
TestDebugger_Breakpoints).

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32
Add retainVal/releaseVal wrappers beside retain/release that perform
the KindRef guard once, and route the 44 opcode handlers in
threaded.go through them instead of repeating
`if v.Kind() == types.KindRef { i.release(v.Ref()) }` at every site.
Guarded variants keep their non-Kind condition
(`if old != val { i.releaseVal(old) }`, `if ok { ... }`). The
raw-addr releases and the single retains(v, n) site are left as-is.

retainVal inlines at all sites; releaseVal does not (it calls the
loop-bearing release), but Fib35 shows no measurable change.

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32
- Lift the adjacent/absorbed closures out of traces() into jitPlan
  methods, dropping closure capture and reading top-down.
- Extract a shared jitSeg.pin helper for the duplicated pin-loop +
  Site call in finalize/pinReturn (the empty-params guard is kept).
- Regroup jitSeg fields per the config/infra/data/state layering.
- Rename jitRun.c to compiler so chains read as s.r.compiler.profile
  (the jitSeg parent field stays r to avoid colliding with the run()
  method).

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32
Route Array.String and Struct.String through the single formatSlice
helper (deleting the parallel joinElems generic), so the
`Type{e0, e1, …}` rendering lives in one place. Reorder array.go to
the fixed declaration slots: types, values, constructors, methods,
then the private formatSlice helper.

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32
Replace MapI32/MapI64/MapF32/MapF64 (4 structs, ~40 methods) with a
single generic TypedMap[K comparable]; a boxKey helper renders native
keys for String. Float maps now use native float32/float64 keys: the
explicit -0.0 bit canonicalization is dropped (Go map semantics still
collapse -0.0/+0.0, and NaN keys are unretrievable, matching the
approved decision).

The boxed Map/MapKey/MapEntry (reference- and string-keyed, with its
heap-boxed-I64 key disambiguation) is intentionally left as-is.
Updated NewMapForType dispatch, the typed cases in threaded.go and
marshal.go, and collapsed the per-type map tests into TestTypedMap_*
(adding -0.0 and NaN cases).

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32
TestEncode_HelpsInterface only exercised the standalone asm.Encode
batch helper removed earlier; it has no production caller. The
arm64-gated file is invisible to the host build, so this surfaced
only under GOARCH=arm64 vet.

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32
Replace I32Array/I64Array/F32Array/F64Array (4 bare slice types, 12
methods) with a single generic TypedArray[T int32|int64|float32|
float64]. Type() and String() dispatch on T via a zero-value type
switch and a formatElem helper; both reuse the shared formatSlice.

The boxed Array (Boxed elems + Refs) is left as-is. Updated the array
opcode cases in threaded.go, marshal.go, and the constant-folding
UTF32 cases; split the array tests into TestTypedArray_* (generic) and
TestArray_* (boxed).

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32
…families

7a: convert the ~16 operand decoders (decodeReg*, decodeMemOp, decodeCmp,
decodeMovImm, …), used only by (*Encoder).Encode, into *Encoder methods
(§1.5). Pure value helpers (intBase, enc, reg, sameKind, encR*,
logicalImmediate, encodeFloatUnary, …) stay package-level.

7b: collapse the repeated encode families through shared helpers/tables,
each preserving the exact opcode constants and bit math:
- MOVZ/MOVK/MOVN  -> encodeMovImmediate(op32, op64)
- SXTB/H/W, UXTB/H/W -> extendOpcodes table
- CMP/CMN/TST -> encodeCompareReg; CMPI/CMNI -> encodeCompareImm
- LDR/LDRB/LDRSB/LDRH/LDRSH/LDRSW -> encodeLoad(base, scale);
  STR/STRB/STRH -> encodeStore(base, scale)
- FADD/FSUB/FMUL/FDIV -> encodeFloatBinary; FMADD-family -> encodeFloatTernary
- FABS/FNEG/FSQRT/FRINT* -> floatUnaryOpcodes table

The heterogeneous shift-immediate group (LSLI/LSRI/ASRI/RORI) is left as-is:
each uses a distinct bitfield formula, so a single helper would only hide
four different computations.

Verified: GOOS=arm64 build+vet, plus the full encoder_test suite run on the
host (its logic is arch-independent byte math) confirms identical output.

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32
7c:
- regOf -> (*jitSeg).regOf method (§1.5).
- drop the compareI32 wrapper; route the I32 comparisons through the
  general compare() with (RegTypeInt, Width32, CMP, cond), matching how the
  I64 comparisons already work.
- extract loadSlot/storeSlot for the boxed load/store emit pattern shared
  by LOCAL_GET/SET/TEE and GLOBAL_GET/SET/TEE; the handlers now read as
  decode -> validate -> emit. Emit order is preserved (Push is bookkeeping,
  not an emit), so generated code is unchanged.

Note: jit_arm64.go is //go:build arm64 and generates native code, so this is
verified by GOOS=arm64 build+vet only. Behavior must be confirmed on ARM64
CI/hardware before merge.

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32
Give the generic value-keyed/primitive-element collections the short,
standard names and qualify the boxed variants with the codebase's existing
"Boxed" vocabulary, so naming is consistent across types, constructors, and
tests:

- TypedMap[K]   -> Map[K]        (NewTypedMap -> NewMap)
- Map (boxed)   -> BoxedMap      (NewMap/NewMapWithCapacity ->
                                  NewBoxedMap/NewBoxedMapWithCapacity)
- MapKey/MapEntry -> BoxedMapKey/BoxedMapEntry
- TypedArray[T] -> Array[T]
- Array (boxed) -> BoxedArray    (NewArray -> NewBoxedArray)

Shared descriptors (MapType, ArrayType, NewMapForType, TypeI32Array, …) keep
their names. Test/benchmark names follow suit (TestMap_*/TestBoxedMap_*,
TestArray_*/TestBoxedArray_*, BenchmarkBoxedMapStringGet_Interned). Pure
rename across types/interp/transform; no behavior change.

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32
- types: boxKey -> formatKey. It returns the key's formatted string (not a
  Boxed), and now reads parallel to formatElem/formatSlice.
- asm/arm64: name the opcode-constant parameter `op` consistently across the
  single-opcode encode helpers (was `base`), and bind the memory base register
  to `base`/`dst`/`src` from the decoder instead of the cramped `b`. Pure
  renames; host run of encoder_test confirms identical output.

https://claude.ai/code/session_01HaHURpzxVBJeKpvFFrfs32
- Renamed `Array` to `TypedArray` and updated its usage across the codebase.
- Updated the `BoxedArray` type to a more generic `Array` type.
- Refactored map types to use `TypedMap` instead of `Map`.
- Adjusted related functions and tests to reflect the new type names and structures.
- Ensured all references to the old types were replaced with the new ones in the code and tests.
@siyul-park siyul-park merged commit b8774b8 into main May 29, 2026
2 checks passed
@siyul-park siyul-park deleted the claude/code-quality-review-cxnXJ branch May 29, 2026 22:32
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.

2 participants