Skip to content

Narrowing after an early exit; a narrowed field is read from the field - #383

Merged
ASDAlexander77 merged 1 commit into
mainfrom
narrow-after-early-exit
Sep 28, 2026
Merged

ASDAlexander77 merged 1 commit into
mainfrom
narrow-after-early-exit

Conversation

@ASDAlexander77

Copy link
Copy Markdown
Owner

Two related narrowing gaps, both found in Event.ts of the BrowserLib sources attached to #231:

remove(callback: EventListener): void {
    if (this.head === null) return;              // did not narrow this.head for what follows:
    if (this.head.callback === callback) {       // 'ts.PropertyRef' op operand #0 must be … but got union
        this.head = this.head.next;              // and inside a narrowed branch: "saving to constant object"
        …

1. Narrowing after an early exit

An if without an else, whose body always returns, throws, breaks or continues, now applies its else narrowing after the if, for the rest of the enclosing block. This is what TypeScript does.

  • When the body counts as always exiting (statementAlwaysExits):
    • it is a return, throw, break or continue;
    • or it is a block containing one;
    • or it is an if/else whose branches both always exit.
  • When it applies: only inside a function, and not when the condition is a compile-time constant.
  • Scope: each block (mlirGen(ts::Block), and the try body of mlirGenBlockWithUnwindCleanup) now opens its own SafeTypesMapScopeT, so a narrowing made in a block ends with it. Without that, a narrowed field would outlive its function, the same leak Narrowing in an else branch ends with the branch #377 fixed for else.

2. A narrowed field is read from the field

safeTypesMap mapped a narrowed field (this.head after this.head !== null) to the value the narrowing cast, and every later read returned that same value. That caused three problems:

  • an assignment to the field failed with "saving to constant object", because the left side was the cast value and not the field;
  • a read after an assignment returned the old value;
  • the value belongs to the region where it was cast, which is the use-after-free class that Narrowing in an else branch ends with the branch #377 had to scope away.

safeTypesMap now keeps only the narrowed type:

  • A read loads the field as usual and then casts it with castToNarrowedType. That new helper does what addSafeCastStatement already did: GetValueFromUnionOp for a union, ValueOp for an optional, UnboxOp for any, and cast otherwise.
  • The left side of an assignment (mlirGenAssignedPropertyAccess) skips the narrowing and writes the field itself.

Tests

  • New 00safe_cast_early_exit.ts (compile, jit, and the rc/none corpus). It covers:

    • if (… === null) return; on a field, and on a parameter with throw;
    • continue in a for…of;
    • the linked-list remove from Event.ts, assigning to the narrowed field and walking with cur.next;
    • a read after assigning to a narrowed field, which must see the new value.

    The old build crashes on it (0xC0000005).

  • Full release suite: 2993/2993 passed, including the owned-unions and ownership-verifier tests, which exercise the union narrowing path.

  • TypeScriptCompilerDefaultLib tests, release jit and compile: 156/156 each.

Not in this PR

  • An assignment does not re-narrow: after this.head = this.head.next, a later this.head.v in the same narrowed block is still cast as non-null. TypeScript would reject that read in strict mode.
  • if (x !== null) { … } else return;, where the else is the exiting side, does not narrow after the if yet.

🤖 Generated with Claude Code

`if (x === null) return;` now narrows x for the rest of the block, as an else
branch does: an if without else whose body always returns, throws, breaks or
continues applies its else narrowing after the if. Each block opens its own
narrowing scope, so a narrowing made in it ends with it.

A narrowed field (`this.head` after `this.head !== null`) was mapped to the
value the narrowing cast, and every later read reused that value. An
assignment to the field then failed ("saving to constant object"), a read
after it returned the old value, and a value used outside its region was the
use-after-free #377 fixed for else. safeTypesMap now keeps only the narrowed
type: a read loads the field again and casts it the way addSafeCastStatement
does (castToNarrowedType), and an assignment writes the field itself.

Found in the BrowserLib sources attached to #231 (Event.ts `remove`).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ASDAlexander77
ASDAlexander77 merged commit b4bf2bb into main Sep 28, 2026
2 checks passed
@ASDAlexander77
ASDAlexander77 deleted the narrow-after-early-exit branch September 28, 2026 08:27
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.

1 participant