Skip to content

[GTO] Update struct field indices after localizing operands - #9108

Merged
tlively merged 1 commit into
mainfrom
fix-gto-placeholder-oob
Sep 16, 2026
Merged

tlively merged 1 commit into
mainfrom
fix-gto-placeholder-oob

Conversation

@tlively

@tlively tlively commented Sep 16, 2026

Copy link
Copy Markdown
Member

PR #9043 ("Avoid newly exposing prototypes in GTO") introduced prepending
an i8 placeholder field at index 0 of exposed no-proto descriptors when
their first field becomes prototype-exposing. When all fields of such a
descriptor are kept, the new index of the last field equals the old
struct's fields.size().

Because updateInstructions runs before updateTypes (so expression
heap types still refer to the pre-optimization Struct definitions), and
FieldRemover is a PostWalker, child struct.get, struct.rmw, and
struct.cmpxchg instructions had their index mutated in-place to the
new index before an enclosing struct.new or struct.set ran
ChildLocalizer or getResultOfFirst. When EffectAnalyzer subsequently
inspected those child instructions, readsStruct accessed
type.getStruct().fields[index] using the old HeapType and the new
index, causing an out-of-bounds assertion failure (__n < this->size())
or reading the wrong field's mutability.

Fix this by splitting instruction updating into two passes executed in a
single nested PassRunner invocation (FieldRemover followed by
IndexUpdater), along with sequential runOnModuleCode runs.
FieldRemover removes/reorders struct.new operands and replaces
removed struct.sets while all struct instructions still have their old
field indices matching their old HeapTypes, and IndexUpdater then
updates the field indices on all remaining struct instructions.

PR #9043 ("Avoid newly exposing prototypes in GTO") introduced prepending
an i8 placeholder field at index 0 of exposed no-proto descriptors when
their first field becomes prototype-exposing. When all fields of such a
descriptor are kept, the new index of the last field equals the old
struct's `fields.size()`.

Because `updateInstructions` runs before `updateTypes` (so expression
heap types still refer to the pre-optimization `Struct` definitions), and
`FieldRemover` is a `PostWalker`, child `struct.get`, `struct.rmw`, and
`struct.cmpxchg` instructions had their `index` mutated in-place to the
new index before an enclosing `struct.new` or `struct.set` ran
`ChildLocalizer` or `getResultOfFirst`. When `EffectAnalyzer` subsequently
inspected those child instructions, `readsStruct` accessed
`type.getStruct().fields[index]` using the old `HeapType` and the new
`index`, causing an out-of-bounds assertion failure (`__n < this->size()`)
or reading the wrong field's mutability.

Fix this by splitting instruction updating into two passes executed in a
single nested `PassRunner` invocation (`FieldRemover` followed by
`IndexUpdater`), along with sequential `runOnModuleCode` runs.
`FieldRemover` removes/reorders `struct.new` operands and replaces
removed `struct.set`s while all struct instructions still have their old
field indices matching their old `HeapType`s, and `IndexUpdater` then
updates the field indices on all remaining struct instructions.
@tlively
tlively requested a review from a team as a code owner September 16, 2026 15:30
@tlively
tlively requested review from stevenfontanella and removed request for a team September 16, 2026 15:30
@tlively
tlively merged commit 5b07da1 into main Sep 16, 2026
16 checks passed
@tlively
tlively deleted the fix-gto-placeholder-oob branch September 16, 2026 18:29
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.

2 participants