fix: native compiler segfault when closure mutation error is inside a function#513
Merged
fix: native compiler segfault when closure mutation error is inside a function#513
Conversation
Contributor
Benchmark Results (Linux x86-64)
CLI Tool Benchmarks
|
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
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.
User impact
Before this PR, if you wrote a closure that mutates a captured variable inside a function body, the native self-hosted ChadScript compiler (
.build/chad) would segfault silently instead of printing the "variable captured by closure but reassigned after capture" error. Node-hosted builds (node dist/chad-node.js) were unaffected.After this PR, both paths correctly print the compile error.
Repro
.build/chad build m08.ts→exit 139(SIGSEGV) on the old code,exit 1with a proper error on this PR.The existing top-level fixture (
closure-capture-mutation-error.ts) did not catch this because top-levelAssignmentStatements take a different walker path inclosure-mutation-checker.tsthan function-body ones — only the function-body path triggered the read of the crashing field.Root cause
closure-mutation-checker.ts:100calledthis.reportError(assign.name, assign.loc). But noAssignmentStatementcreation site inparser-tsorparser-nativesets alocfield. Per the CLAUDE.md rule #3, the native compiler derives struct layouts from object-literal creation sites, so the resulting nativeAssignmentStatementstruct has no slot forloc. Readingassign.locGEPs past the end of the struct into garbage memory and segfaults inside the nativeformatCompileErrorpipeline — after correctly detecting the error but before it can be printed.Fix
Defensive one-line change in
closure-mutation-checker.ts: passundefinedinstead ofassign.loctoreportError. The error message itself is preserved; only the optional line-number underline is lost in the native-hosted path. The node-hosted path (which never exercised the crash) is unaffected.Why not plumb
locthrough allAssignmentStatementcreation sites?I tried that first (it's the "proper" fix that would give the native path line-number info too). It triggered a deeper native-compiler bug: adding a
loc: undefinedfield to some creation sites shifts the inferred struct layout in a way that breaksmrAllReturninsafety-checks.ts— the return-path walker starts misreadingstmt.typevia GEP and incorrectly flagstransformExpressionas "does not return a value on all code paths." Fixing that is a bigger native-compiler refactor (unify struct layouts from interface definitions instead of creation sites), which should be a separate PR.Test plan
tests/fixtures/closures/closure-mutation-inside-function.ts— new, captures the exact repro that used to segfaulttests/fixtures/closures/closure-mutation-inside-arrow-body.ts— new, captures the variant where the assignment is inside the arrow bodyclosure-capture-mutation-error.tsstill passes (top-level case)npm run verify:quickpasses (full test suite + stage 1 self-hosting)Followups not in this PR
locthroughAssignmentStatementcreation sites to restore line-number info in the native-hosted error output — blocked on themrAllReturnstruct-layout issue above.stmt.loc/ifStmt.loc/ etc. access on any statement type where no creation site setsloc.codegen/statements/control-flow.tsandcodegen/statements/for-of.tsboth readstmt.locon error paths today — if any of those error paths get exercised on a native self-hosted compile, they will segfault the same way.