v0.0.131 — GC infrastructure batch (closes #487 and #348)#571
Conversation
Closes #487 and #348. Two related GC bugs in vera/codegen/assembly.py fixed in a single batch. #487 — $alloc grows by 1 page regardless of need ================================================ Pre-fix: when heap_ptr + total > memory.size * 65536, $alloc called memory.grow 1 unconditionally. A single allocation > ~64 KB past the current memory boundary fell through to the bump-allocate and trapped on out-of-bounds memory access. Post-fix: compute pages_needed = ceil(((heap_ptr + total) - memory.size * 65536) / 65536) and grow by that many pages. Verified against the issue's reproducer: two 50K-element Array<Int> (~800 KB total) now allocates cleanly. #348 — GC worklist silent overflow ================================== Pre-fix: 16 KiB / 4096-entry worklist; both push branches (Phase 2 seed and Phase 2b mark scan) silently dropped pushes when full, leaving reachable objects unmarked, which the sweep phase then freed — a real use-after-free hole for object graphs holding more than ~4K reachable pointers. Post-fix: - Worklist quadrupled to 64 KiB (16 384 entries) for ~4× headroom - Both push branches now `unreachable` on overflow rather than silently dropping. Any residual overflow is a clean WASM trap, not silent corruption. Eliminating the trap entirely via iterative deepening or dynamic worklist growth is tracked separately for follow-up. Tests ===== Two new test classes, 5 tests total: TestLargeAllocGrow487: test_50k_int_array_alloc_succeeds (returns 50000, was: trap) test_single_large_alloc_smaller_than_old_limit (regression pin) TestWorklistOverflow348: test_moderate_graph_with_gc_pressure (1000-element Array<Box>) test_worklist_size_quadrupled_in_wat (gc_heap_start = 81920) test_worklist_overflow_traps_in_wat (≥6 i32.ge_u in $gc_collect) Two existing tests updated for the new heap base: test_heap_ptr_starts_after_strings (32773 → 81925) test_heap_ptr_zero_without_strings (32768 → 81920) Test count: 3,689 → 3,694. Surfaced #570 ============= Writing the natural runtime regression test for #348 (a 5000-element Array<Box>) hit a separate pre-existing bug: array_map's per-element gc_shadow_push isn't unwound between iterations, exhausting the 16 KiB / 4096-entry shadow stack at ~4000 elements. Filed as #570 and tracked in KNOWN_ISSUES.md for follow-up; the #348 runtime regression is covered by a 1000-element graph (within the shadow stack budget) plus structural WAT pins. Co-Authored-By: Claude <noreply@anthropic.invalid>
📝 WalkthroughWalkthroughVersion 0.0.131 release bump that applies GC infrastructure fixes: allocator OOM path grows by the computed number of WASM pages, GC mark worklist capacity increased from 16 KiB to 64 KiB with overflow now trapping via ChangesGC Infrastructure & Release
Sequence Diagram(s)sequenceDiagram
participant Codegen as Codegen (assembly)
participant WASM as WASM Module
participant Memory as Linear Memory
participant GC as GC Worklist
participant Tests as Test Harness
Codegen->>WASM: emit `$alloc` with computed pages_needed logic
Tests->>WASM: invoke allocation path (large alloc)
WASM->>Memory: call memory.size, compare heap vs boundary
WASM->>Memory: memory.grow(pages_needed)
alt memory.grow succeeds
WASM->>GC: seed shadow-stack into worklist
GC->>GC: push entries, check wl_ptr >= wl_end
alt within bounds
GC-->>WASM: continue marking
WASM-->>Tests: allocation/collect completes successfully
else overflow
GC-->>WASM: unreachable (trap)
WASM-->>Tests: trap observed
end
else memory.grow fails
WASM-->>Tests: trap observed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #571 +/- ##
=======================================
Coverage 90.96% 90.96%
=======================================
Files 59 59
Lines 22588 22588
Branches 259 259
=======================================
Hits 20547 20547
Misses 2034 2034
Partials 7 7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_codegen.py`:
- Around line 1661-1662: Combine the two assertions into one that matches the
$heap_ptr global declaration and its constant value together in result.wat (e.g.
use a single regex or substring that ensures "global $heap_ptr" is followed by
the "i32.const 81925" initializer), update the assertion that currently checks
"global $heap_ptr" and "i32.const 81925" separately to this single check, and
apply the same change to the other occurrence around line 1674 so both tests
assert the $heap_ptr declaration and value in one match.
- Around line 15140-15179: Add a new unit test to exercise the 64 KiB
page-boundary for pages_needed rounding: create a test (e.g.,
test_page_boundary_alloc_rounding) alongside test_50k_int_array_alloc_succeeds
and test_single_large_alloc_smaller_than_old_limit that calls array_range with a
length chosen so the allocation shortage is right at or just over 65536 bytes
(for example lengths that produce shortage == 65535, 65536, and 65537) and
assert correct values via _run and indexing (like reading the last element).
This will pin the ceil(shortage/65536) edge case and ensure multi-page grow math
handles off-by-one at the 64 KiB boundary.
- Around line 15291-15305: The test is too coarse-grained by asserting
ge_u_count >= 6 and can miss regressions in the specific worklist overflow
guard; update the assertion in tests/test_codegen.py to look for the exact
opcode sequence that represents the overflow guard (the sequence containing
local.get $wl_ptr, local.get $wl_end, i32.ge_u, if, unreachable) and assert that
this full sequence occurs exactly twice (or at least twice) inside the
$gc_collect body rather than counting all i32.ge_u occurrences; locate the
$gc_collect string and replace the ge_u_count check with a search/match for the
concatenated sequence (or a regex that tolerates minimal whitespace) and assert
its occurrence count is >= 2 to ensure both overflow checks are present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9af70727-9a35-4ebf-ad3f-59c50cc5ab87
⛔ Files ignored due to path filters (5)
docs/index.htmlis excluded by!docs/**docs/index.mdis excluded by!docs/**docs/llms-full.txtis excluded by!docs/**docs/llms.txtis excluded by!docs/**uv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (10)
CHANGELOG.mdHISTORY.mdKNOWN_ISSUES.mdREADME.mdROADMAP.mdTESTING.mdpyproject.tomltests/test_codegen.pyvera/__init__.pyvera/codegen/assembly.py
All three inline findings on PR #571 were valid; applied as suggested. Inline (3): 1. tests/test_codegen.py 1661-1662 + 1674 — combined $heap_ptr assertion. Pre-fix: assert "global $heap_ptr" in result.wat assert "i32.const 81925" in result.wat could both pass even if the i32.const 81925 came from somewhere else in the WAT (e.g. an unrelated future constant). Post-fix: match the entire decl + initializer in one substring: '(global $heap_ptr (export "heap_ptr") (mut i32) (i32.const 81925))' Applied to both test_heap_ptr_starts_after_strings (81925) and test_heap_ptr_zero_without_strings (81920). 2. tests/test_codegen.py 15140-15179 — added test_page_boundary_alloc_rounding to TestLargeAllocGrow487. Pins the (shortage + 65535) >> 16 ceiling math at the 64 KiB boundary against off-by-one regressions. Three subcases: - 8192 i64s = 65536 bytes (exactly 1 page) - 8193 i64s = 65544 bytes (1 byte over → must round up to 2) - 16384 i64s = 131072 bytes (exactly 2 pages) Each allocates fresh and reads the last element to force the access to land in the new memory. 3. tests/test_codegen.py 15291-15305 — replaced the coarse ge_u_count >= 6 check with an exact regex match for the full overflow-guard opcode sequence: local.get $wl_ptr local.get $wl_end i32.ge_u if unreachable The pre-fix shape (i32.lt_u + push) wouldn't match this pattern, so a regression where one or both push branches reverted to silent-drop would now fail the test cleanly. Test count: 3,694 → 3,695 (1 new test in TestLargeAllocGrow487). ruff S clean, mypy clean, full pytest passes. Co-Authored-By: Claude <noreply@anthropic.invalid>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@HISTORY.md`:
- Line 268: Update the v0.0.131 HISTORY.md entry (the row containing "v0.0.131 |
5 May") to append a note that this PR surfaced issue `#570` (array_map
shadow-stack overflow around ~4,000 elements) which remains unfixed/for
follow-up, and also mention the heap base offset change (gc_heap_start 32,768 →
81,920 bytes) so test-suite expectations are clear; keep the existing references
to `#487` and `#348` and phrase the new text succinctly to match the release-note
style used in that row.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 459f7da4-8935-4f4e-8016-bb58a4c51918
📒 Files selected for processing (5)
HISTORY.mdREADME.mdROADMAP.mdTESTING.mdtests/test_codegen.py
…ichment
Appended to the v0.0.131 HISTORY.md row:
- heap base offset change (gc_heap_start 32,768 -> 81,920 bytes
from the worklist resize) so test-suite expectations are clear
- note that this PR surfaced #570 (array_map shadow-stack
overflow at ~4,000 elements), tracked for follow-up
Existing #487 and #348 references kept; phrasing matches the
release-note style of adjacent rows.
Co-Authored-By: Claude <noreply@anthropic.invalid>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@HISTORY.md`:
- Line 301: Update the tests count for v0.0.131 in HISTORY.md by replacing the
incorrect value "3,695" with the correct value "3,496" in the table cell (the
row containing "Tests" and the v0.0.131 entry currently showing 3,695).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 14826ea8-b2f9-438a-bff5-9f765e1f5070
📒 Files selected for processing (1)
HISTORY.md
Summary
$alloc1-page grow → multi-page grow) and #348 (GC worklist silent overflow → trap + 4× capacity) in a single batch.vera/codegen/assembly.py(_emit_allocand_emit_gc_collect+ thegc_worklist_sizeconstant).TestLargeAllocGrow487(2) andTestWorklistOverflow348(3); test count 3,689 → 3,694.array_mapshadow-stack overflow at ~4 000 elements) while writing the natural runtime regression test for GC worklist overflow for deeply nested object graphs #348 — filed and tracked in KNOWN_ISSUES.md for follow-up.#487 —
$allocmulti-page growPre-fix, when
heap_ptr + total > memory.size * 65536,$alloccalledmemory.grow 1regardless of how many pages were actually needed. A single allocation request > ~64 KB past the current memory boundary fell through to the bump-allocate and trapped on out-of-bounds memory access.Post-fix: compute
pages_needed = ceil(((heap_ptr + total) - memory.size * 65536) / 65536)and grow by that many pages in a single call. The math reduces to the pre-fix single-page grow when shortage ≤ 65 535 (regression pin viatest_single_large_alloc_smaller_than_old_limit).Verified against the issue's reproducer (two 50 K-element
Array<Int>arrays, ~800 KB total) — pre-fix this trapped at index access, post-fix it returns 50000.#348 — GC worklist size + overflow trap
Pre-fix, the mark-phase worklist was 16 KiB / 4 096 entries; both push branches (Phase 2 seed and Phase 2b mark scan) silently dropped pushes when full, leaving reachable objects unmarked which the sweep phase then freed — a real use-after-free hole for object graphs holding more than ~4 K reachable pointers from a single root.
Post-fix:
gc_worklist_size = 65536). Reasonable program shapes don't reach the cap.unreachableon overflow rather than silently dropping. Any residual overflow is a clean WASM trap, not silent corruption.The
gc_heap_startglobal shifts from 32 768 → 81 920 bytes (16 KiB stack + 64 KiB worklist). Two existing tests (test_heap_ptr_starts_after_strings,test_heap_ptr_zero_without_strings) updated for the new offset.Eliminating the trap entirely via iterative deepening or dynamic worklist growth is a deliberate non-goal here; trade-off is the design ratchet (header-bit changes for proper recovery) vs. a clean trap and 4× headroom that covers reasonable program shapes today.
Tests
TestLargeAllocGrow487TestWorklistOverflow348Array<Box>runtime; structural pins ongc_heap_start = 81920and≥6 i32.ge_uin$gc_collectThe natural "wide-graph" runtime regression for #348 (a 5 000-element
Array<Box>) was blocked by #570 —array_map's per-elementgc_shadow_pushisn't unwound between iterations, exhausting the shadow stack at ~4 000 elements. Verified pre-existing onmain(separate subsystem). The 1 K-element runtime test plus structural WAT pins cover the post-fix shape; the 5 K case can move to a runtime test once #570 is fixed.Test plan
mypy vera/cleanruff check --select S vera/cleanCloses #487
Closes #348
Summary by CodeRabbit