Skip to content

Commit e3adbad

Browse files
authored
Fix TypeGeneralizing stack corruption and crash (WebAssembly#8310)
## Summary Two bugs in the experimental `TypeGeneralizing` pass's backward analysis: ### 1. `visitStructSet` pushes non-ref field types onto the stack `visitStructSet` unconditionally 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 (like `visitConst`, `visitBinary`) are no-ops that don't pop. The spurious non-ref value on the stack causes subsequent `pop()` 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. `visitRefAs` crashes on `Type::none` from empty stack When the backward analysis stack is empty (no downstream consumer imposes a type requirement), `pop()` returns `Type::none`. `visitRefAs` then calls `type.getHeapType()` on `Type::none`, triggering `assert(isRef())` — crashing on any `ref.as_non_null` whose result is dropped. **Fix:** Check for `Type::none` before 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 - [x] New lit test `type-generalizing-fixes.wast` covering: - `struct.set` on 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) - [x] All 309 unit tests pass
1 parent f5ea49a commit e3adbad

File tree

2 files changed

+92
-1
lines changed

2 files changed

+92
-1
lines changed

src/passes/TypeGeneralizing.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,10 @@ struct TransferFn : OverriddenVisitor<TransferFn> {
687687
}
688688
auto generalized = generalizeStructType(type, curr->index);
689689
push(Type(generalized, Nullable));
690-
push(generalized.getStruct().fields[curr->index].type);
690+
auto fieldType = generalized.getStruct().fields[curr->index].type;
691+
if (fieldType.isRef()) {
692+
push(fieldType);
693+
}
691694
}
692695

693696
void visitStructRMW(StructRMW* curr) { WASM_UNREACHABLE("TODO"); }
@@ -861,6 +864,11 @@ struct TransferFn : OverriddenVisitor<TransferFn> {
861864

862865
void visitRefAs(RefAs* curr) {
863866
auto type = pop();
867+
if (type == Type::none) {
868+
// No downstream requirement, so no requirement for the input either.
869+
push(Type::none);
870+
return;
871+
}
864872
switch (curr->op) {
865873
case RefAsNonNull:
866874
push(Type(type.getHeapType(), Nullable));
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
2+
3+
;; RUN: foreach %s %t wasm-opt --experimental-type-generalizing -all -S -o - | filecheck %s
4+
5+
;; Test that visitStructSet correctly handles structs with non-ref fields
6+
;; without corrupting the backward analysis stack.
7+
(module
8+
;; CHECK: (type $s (struct (field $x (mut i32)) (field $r (mut anyref))))
9+
(type $s (struct (field $x (mut i32)) (field $r (mut anyref))))
10+
11+
;; CHECK: (type $1 (func (param (ref null $s) anyref)))
12+
13+
;; CHECK: (func $struct-set-nonref-field (type $1) (param $obj (ref null $s)) (param $r anyref)
14+
;; CHECK-NEXT: (struct.set $s $x
15+
;; CHECK-NEXT: (local.get $obj)
16+
;; CHECK-NEXT: (i32.const 10)
17+
;; CHECK-NEXT: )
18+
;; CHECK-NEXT: (struct.set $s $r
19+
;; CHECK-NEXT: (local.get $obj)
20+
;; CHECK-NEXT: (local.get $r)
21+
;; CHECK-NEXT: )
22+
;; CHECK-NEXT: )
23+
(func $struct-set-nonref-field (param $obj (ref null $s)) (param $r anyref)
24+
;; Setting a non-ref field (i32) should not push i32 onto the type
25+
;; requirement stack, which would corrupt subsequent ref type tracking.
26+
(struct.set $s $x
27+
(local.get $obj)
28+
(i32.const 10)
29+
)
30+
(struct.set $s $r
31+
(local.get $obj)
32+
(local.get $r)
33+
)
34+
)
35+
)
36+
37+
;; Test that visitRefAs handles the case where the stack is empty (no
38+
;; downstream type requirement) without crashing.
39+
(module
40+
;; CHECK: (type $0 (func (param anyref)))
41+
42+
;; CHECK: (func $ref-as-non-null-dropped (type $0) (param $x anyref)
43+
;; CHECK-NEXT: (drop
44+
;; CHECK-NEXT: (ref.as_non_null
45+
;; CHECK-NEXT: (local.get $x)
46+
;; CHECK-NEXT: )
47+
;; CHECK-NEXT: )
48+
;; CHECK-NEXT: )
49+
(func $ref-as-non-null-dropped (param $x anyref)
50+
;; The result of ref.as_non_null is dropped, so no downstream type
51+
;; requirement exists. This should not crash.
52+
(drop
53+
(ref.as_non_null
54+
(local.get $x)
55+
)
56+
)
57+
)
58+
)
59+
60+
;; Test extern.convert_any with dropped result.
61+
(module
62+
;; CHECK: (type $0 (func (param anyref)))
63+
64+
;; CHECK: (func $any-convert-extern-dropped (type $0) (param $x anyref)
65+
;; CHECK-NEXT: (drop
66+
;; CHECK-NEXT: (any.convert_extern
67+
;; CHECK-NEXT: (extern.convert_any
68+
;; CHECK-NEXT: (local.get $x)
69+
;; CHECK-NEXT: )
70+
;; CHECK-NEXT: )
71+
;; CHECK-NEXT: )
72+
;; CHECK-NEXT: )
73+
(func $any-convert-extern-dropped (param $x anyref)
74+
;; Dropped extern.convert_any / any.convert_extern chain should not crash.
75+
(drop
76+
(any.convert_extern
77+
(extern.convert_any
78+
(local.get $x)
79+
)
80+
)
81+
)
82+
)
83+
)

0 commit comments

Comments
 (0)