Fix TypeGeneralizing stack corruption and crash#8310
Merged
Conversation
Two bugs in the experimental TypeGeneralizing pass: 1. visitStructSet unconditionally pushed the field type onto the backward analysis stack, even for non-reference fields (i32, f64). Since non-ref producers (visitConst, etc.) are no-ops that don't pop, this corrupted the stack alignment, causing subsequent local.get visitors to pop wrong type requirements. Add an isRef() guard to match the pattern used by visitArraySet, visitStructNew, and handleCall. 2. visitRefAs called type.getHeapType() on Type::none (returned by pop() when the stack is empty), triggering an assertion failure. This happens when ref.as_non_null's result is dropped. Add a Type::none check to propagate "no requirement" through RefAs.
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
2 participants
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
Two bugs in the experimental
TypeGeneralizingpass's backward analysis:1.
visitStructSetpushes non-ref field types onto the stackvisitStructSetunconditionally pushes the struct field type as a type requirement onto the backward analysis stack (line 690). When the field is a non-reference type (i32, f64, etc.), this corrupts the stack because non-ref producers (likevisitConst,visitBinary) are no-ops that don't pop. The spurious non-ref value on the stack causes subsequentpop()calls to retrieve wrong type requirements.The analogous methods all correctly guard with
isRef():visitArraySet(line 792):if (elemType.isRef())visitStructNew(line 620):if (field.type.isRef())handleCall(line 364):if (param.isRef())Fix: Add
if (fieldType.isRef())guard before pushing.2.
visitRefAscrashes onType::nonefrom empty stackWhen the backward analysis stack is empty (no downstream consumer imposes a type requirement),
pop()returnsType::none.visitRefAsthen callstype.getHeapType()onType::none, triggeringassert(isRef())— crashing on anyref.as_non_nullwhose result is dropped.Fix: Check for
Type::nonebefore accessing heap type, and propagate "no requirement" through.Both bugs are in the experimental (not-yet-sound) pass and do not affect production optimization pipelines.
Test plan
type-generalizing-fixes.wastcovering:struct.seton non-ref field followed by ref field (stack alignment)drop(ref.as_non_null(...))(empty stack crash)drop(any.convert_extern(extern.convert_any(...)))(empty stack with convert ops)