fix(compile): object-slot number locals that may hold undefined (#367)#371
Merged
Conversation
A `: number` local unsoundly assigned an `any`/`undefined` value (e.g. `let x: number = undefined as any`) can hold the runtime `undefined` sentinel, yet `return x` is statically `number` — so the static #344 detection (which keys on the return value's own type being any/unknown) never fires. Worse, the compiler gives such a local an unboxed `double` slot, which coerces the sentinel to NaN at the *store*, before the return is even reached. After a function/arrow/method/accessor body with a number/boolean return is type-checked, run an order-independent taint pass: compute (to a fixpoint, transitively through other tainted locals) the set of locals that may hold the sentinel, then - flag returns of a tainted local so the return slot widens to object (the existing #344 ReturnSlotAnalysis seam), and - flag the tainted locals' declarations so CanUseUnboxedLocal gives them an object slot instead of an unboxed double. Order-independent, so a taint reaching an earlier return via a loop back-edge is still caught. Over-approximate (no narrowing): at worst a needless object slot, never a wrong value. Clean number locals keep their sound unboxed slot. Residuals (filed separately): inferred-return functions, void/non-return observations (console.log, arithmetic), and reassigned number parameters share the same double-slot root cause but are outside #367's return scope.
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
1 participant
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.
Summary
Closes #367 — the residual of #344 where a
number/boolean-typed local unsoundly assigned anany/undefinedvalue, then returned, compiled toNaN/falseinstead ofundefined.#344fixed the case where the return value's own static type isany/unknown. This one is invisible to that detection:return xwherex: numberis staticallynumber. Two distinct slots corrupt the sentinel:doubleslot coercesundefined→NaNat the store (let x: number = 0; x = undefined as any).ReturnSlotAnalysisseam).Approach
After a function/arrow/method/accessor body with a
number/booleanreturn is type-checked (so every sub-expression type is in theTypeMap), run an order-independent taint pass (ReturnLocalTaintCollector+ a fixpoint inMarkUndefinedReachableLocalReturns):any/undefinedassignments, grown transitively through other tainted locals (let z: number = y).object.CanUseUnboxedLocalgives them anobjectslot instead of an unboxeddouble.Order-independent, so a taint reaching an earlier return via a loop back-edge is still caught. Over-approximate (no narrowing): at worst a needless object slot, never a wrong value. Clean number locals keep their sound unboxed
doubleslot (verified by a guard test). The reusedValueMayBeUndefinedSentinelpredicate already recurses through grouping/ternary/||/&&pass-throughs; this PR also adds the missing??(NullishCoalescing) case.Tests
5 new cases in
ILVerificationTests(each asserts compiled IL verifies, runs correctly, and matches the interpreter): local-with-any-init, transitive taint, loop back-edge, method+getter, and a sound-body guard. Full suite: 11137 passed, 0 failed.Out of scope (filed as follow-up)
The same unboxed-
double-slot root cause also corrupts: inferred-return functions, non-return observations (console.log/arithmetic in void functions), and reassignednumberparameters. These are outside #367's return scope — filed separately.