Skip to content

Fix select missing stack map declarations for GC refs#12862

Open
fitzgen wants to merge 1 commit intobytecodealliance:mainfrom
fitzgen:select-declare-needs-stack-map
Open

Fix select missing stack map declarations for GC refs#12862
fitzgen wants to merge 1 commit intobytecodealliance:mainfrom
fitzgen:select-declare-needs-stack-map

Conversation

@fitzgen
Copy link
Copy Markdown
Member

@fitzgen fitzgen commented Mar 28, 2026

The select and typed select Wasm operators create new SSA values in Cranelift but were not calling declare_value_needs_stack_map on the result when the operand type is a GC reference. This meant the result, when kept on the Wasm operand stack (not stored in a local variable), would not appear in stack maps at subsequent safepoints.

If a GC collection occurred at such a safepoint, the collector would not see the select's result as a live GC root and could free the referenced object, leading to use-after-free.

The fix checks select's operand types for reference types and declares the result as requiring inclusion in stack maps when needed.

The `select` and typed `select` Wasm operators create new SSA values in
Cranelift but were not calling `declare_value_needs_stack_map` on the result
when the operand type is a GC reference. This meant the result, when kept on the
Wasm operand stack (not stored in a local variable), would not appear in stack
maps at subsequent safepoints.

If a GC collection occurred at such a safepoint, the collector would not see the
`select`'s result as a live GC root and could free the referenced object,
leading to use-after-free.

The fix checks `select`'s operand types for reference types and declares the
result as requiring inclusion in stack maps when needed.
@fitzgen fitzgen requested review from a team as code owners March 28, 2026 00:09
@fitzgen fitzgen requested review from cfallin and removed request for a team March 28, 2026 00:09
Copy link
Copy Markdown
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find!

needs_stack_map
}
_ => false,
}) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we wrap this predicate up in another environ helper? Something like environ.ty_needs_stack_map(ty)? The embedded match in a closure in an if is a little awkward otherwise

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