Skip to content

fix(types,ir): loop-grown arrays widen to mixed before the body is processed (#452)#475

Open
mirchaemanuel wants to merge 2 commits into
illegalstudio:mainfrom
mirchaemanuel:fix/452-mixed-array-growth-corruption
Open

fix(types,ir): loop-grown arrays widen to mixed before the body is processed (#452)#475
mirchaemanuel wants to merge 2 commits into
illegalstudio:mainfrom
mirchaemanuel:fix/452-mixed-array-growth-corruption

Conversation

@mirchaemanuel

Copy link
Copy Markdown
Contributor

Summary

Fixes #452. Growing an array inside a loop across heterogeneous push sites ($vals[] = 1; $vals[] = 2.0;) corrupted the heap: reading the affected element produced garbage or a SIGSEGV, and the string/float flavour silently corrupted values and leaked.

Root cause

Both the type checker and EIR lowering process loop bodies in a single pass, so each push site is typed and lowered against the types in force the first time through:

  • site A ($vals[] = 1) is lowered against array<never> → a raw scalar array_push;
  • site B ($vals[] = 2.0) widens the element type and emits the array_to_mixed promotion.

The loop back edge brings the promoted array<mixed> around, so from iteration 2 the stale-typed site A writes an unboxed i64 into boxed-cell storage. Any value-read of that element dereferences the raw scalar as a Mixed cell pointer → SIGSEGV (a raw string payload happens to survive reads, which is why the string flavour corrupted silently instead). Diagnosed via a deterministic bisection: the minimal repro is 2 iterations + echo $vals[2] — no foreach needed.

Fix

A shared prescan (src/types/checker/loop_widening.rs) walks a loop body's statements recursively, collects its $name[] = value push sites, and reports the locals whose indexed-array element type joins to mixed across all sites (never adopts the other side, equal types stay, anything else widens). Then:

  • the checker widens those locals to array<mixed> in the TypeEnv before checking each loop body (while/do-while/for/foreach; for foreach after the value/key bindings so a push of the loop variable joins with its real element type);
  • EIR lowering widens its local_types the same way and materializes the promotion once before the loop (in-place ArrayToMixed + storeback), so every push site inside the body boxes its value.

Same-type pushes and never → T growth are untouched: on those paths the generated IR is byte-identical to main (verified) — the widening only fires on the representation-changing join to mixed. As a bonus, loops that previously re-promoted per iteration now hoist a single conversion out of the loop.

Verification

  • 6 regression tests in tests/codegen/array_basics.rs: element read, foreach sum, int+string flavour, float-first ordering, push under if, while-loop variant (the walker's recursion branches).
  • Adversarially reviewed: repro family (2–5 iterations, while/do-while/nested loops, push under if/try, foreach value-var push) all match PHP with a clean --heap-debug; iterate-and-push snapshot semantics, conditional break before the promoting site, COW copies between iterations, and continue skipping a site all verified against PHP.
  • No regressions: arrays + runtime_gc + optimizer (1231 tests), control/types/loops (502), error_tests (993), lib — all green; build warning-free.

Found during review (pre-existing, not this change)

  • The parser rejects an array push (and comma-separated expressions) in the for update clause — filed separately with a repro.

…ocessed

Fixes illegalstudio#452. Both the type checker and EIR lowering process loop bodies in a
single pass, so with $vals[] = 1; $vals[] = 2.0; in a loop the earlier push
site was typed/lowered against the pre-promotion element type (array<never>/
array<int>) and emitted a raw scalar push. On iterations >= 2 the back edge
brings the promoted array<mixed> around, so the raw push wrote an unboxed
scalar into boxed-cell storage; reading that element dereferenced the scalar
as a Mixed cell pointer (SIGSEGV), or silently corrupted values and leaked in
the string/float flavour.

A shared prescan (types::checker::loop_widening) collects the body's
$name[] = value pushes recursively and reports locals whose element type
joins to mixed. The checker widens those locals in the TypeEnv before
checking each loop body; EIR lowering widens its local_types and materializes
the promotion once before the loop (in-place ArrayToMixed + storeback), so
every push site boxes its value. Same-type pushes and never->T growth keep
their current lowering (no action unless the join is mixed). The foreach
prescan binds the loop value/key variables to their real element types so a
push of the loop variable does not over-widen.

Regression tests in tests/codegen/array_basics.rs (element read, foreach sum,
int+string flavour, float-first ordering).

(cherry picked from commit c3b449b)
@mirchaemanuel mirchaemanuel force-pushed the fix/452-mixed-array-growth-corruption branch from c3b449b to c7831bd Compare July 6, 2026 12:35
…d defaults

CI caught a false positive: the prescan treated any pushed variable unknown at
loop entry as Mixed evidence, spuriously widening same-typed rebuild loops. The
compiler-synthesized MultipleIterator::detachIterator body rebuilds an
array<Iterator> through a loop-defined variable ($candidate =
$this->iterators[$i]; $newIterators[] = $candidate;), and the widened
array<mixed> then failed the typed-property storeback — every program calling
detachIterator stopped compiling.

Two layers:
- loop_widening: a pushed variable now joins its loop-entry type with
  self-evident literal assignments found inside the body ($x = 1; $a[] = $x
  still widens); a variable whose only in-loop sources are non-literal yields
  no evidence and is excluded from the join (poison entries guard partial
  evidence). Unknown never defaults to Mixed.
- ir_lower lookup closure: returned Some(Mixed) for undeclared names via the
  local_type fallback, so the checker skipped but ir_lower still widened; it
  now returns None for names without a declared slot, mirroring the checker's
  env.get.

Regression tests: the MultipleIterator detach shape compiles and runs, and the
literal-through-variable heterogeneous push still widens (array_basics.rs).
Full sweep green including the spl suite (1803 codegen + 993 error tests).
@mirchaemanuel

Copy link
Copy Markdown
Contributor Author

CI fix pushed (c1d71ae3) — the failing test_multiple_iterator_contains_detach_and_assoc_null_info_error was a real defect in this PR, thanks for the run.

What was wrong: the widening prescan treated any pushed variable unknown at loop entry as mixed evidence. The synthesized MultipleIterator::detachIterator body rebuilds an array<Iterator> through exactly such a variable ($candidate = $this->iterators[$i]; $newIterators[] = $candidate;), so the array was spuriously widened to array<mixed> and its typed-property storeback stopped compiling — for every program touching detachIterator.

The fix: unknown never defaults to mixed. A pushed variable joins its loop-entry type with self-evident literal assignments found inside the body (so $x = 1; $vals[] = $x; $y = 2.0; $vals[] = $y; still widens); non-literal in-loop sources yield no evidence and are excluded from the join. This needed two layers — the shared prescan, and ir_lower's lookup closure, which was silently turning "unknown name" into Some(Mixed) via the local_type fallback while the checker's env.get correctly said None.

Regression tests added for both directions (the detach shape compiles and runs; the literal-through-variable heterogeneous push still widens). Full sweep green including the spl suite this time (1803 codegen + 993 error tests).

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.

Growing a Mixed-promoted array in a loop then iterating+consuming it corrupts the heap

1 participant