Skip to content

Commit 98129d1

Browse files
authored
Fix typing of struct RMW ops with null references (#7214)
When the struct reference is null, there is no struct type from which to look up the proper field type when finalizing a struct RMW or cmpxchg expression. We previously reused code for this case from StructGet, but unlike StructGet, the RMW operations have additional operands that can constrain the type of the retrieved struct field. Take these operands into account when computing a type for the RMW expressions.
1 parent 7a258b3 commit 98129d1

File tree

3 files changed

+125
-8
lines changed

3 files changed

+125
-8
lines changed

scripts/test/fuzzing.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
'typed_continuations_suspend.wast',
8181
# the fuzzer does not support struct RMW ops
8282
'gc-atomics.wast',
83+
'gc-atomics-null-refs.wast',
8384
# contains too many segments to run in a wasm VM
8485
'limit-segments_disable-bulk-memory.wast',
8586
# https://github.com/WebAssembly/binaryen/issues/7176

src/wasm/wasm.cpp

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1168,10 +1168,10 @@ void StructRMW::finalize() {
11681168
if (ref->type == Type::unreachable || value->type == Type::unreachable) {
11691169
type = Type::unreachable;
11701170
} else if (ref->type.isNull()) {
1171-
// See comment on CallRef for explanation.
1172-
if (type.isRef()) {
1173-
type = Type(type.getHeapType().getBottom(), NonNullable);
1174-
}
1171+
// We have no struct type to read the field off of, but the most precise
1172+
// possible option is the type of the value we are using to make the
1173+
// modification.
1174+
type = value->type;
11751175
} else {
11761176
type = ref->type.getHeapType().getStruct().fields[index].type;
11771177
}
@@ -1182,10 +1182,9 @@ void StructCmpxchg::finalize() {
11821182
replacement->type == Type::unreachable) {
11831183
type = Type::unreachable;
11841184
} else if (ref->type.isNull()) {
1185-
// See comment on CallRef for explanation.
1186-
if (type.isRef()) {
1187-
type = Type(type.getHeapType().getBottom(), NonNullable);
1188-
}
1185+
// Like StructRMW, but the most precise possible field type is the LUB of
1186+
// the expected and replacement values.
1187+
type = Type::getLeastUpperBound(expected->type, replacement->type);
11891188
} else {
11901189
type = ref->type.getHeapType().getStruct().fields[index].type;
11911190
}
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.
2+
3+
;; RUN: wasm-opt -all %s -S -o - | filecheck %s
4+
;; RUN: wasm-opt -all %s --roundtrip -S -o - | filecheck %s --check-prefix=RTRIP
5+
6+
(module
7+
(type $struct-i32 (struct (field (mut i32))))
8+
(type $struct-eq (struct (field (mut eqref))))
9+
10+
;; CHECK: (func $rmw-null (type $1) (result i32)
11+
;; CHECK-NEXT: (block ;; (replaces unreachable StructRMW we can't emit)
12+
;; CHECK-NEXT: (drop
13+
;; CHECK-NEXT: (ref.null none)
14+
;; CHECK-NEXT: )
15+
;; CHECK-NEXT: (drop
16+
;; CHECK-NEXT: (i32.const 1)
17+
;; CHECK-NEXT: )
18+
;; CHECK-NEXT: (unreachable)
19+
;; CHECK-NEXT: )
20+
;; CHECK-NEXT: )
21+
;; RTRIP: (func $rmw-null (type $1) (result i32)
22+
;; RTRIP-NEXT: (drop
23+
;; RTRIP-NEXT: (ref.null none)
24+
;; RTRIP-NEXT: )
25+
;; RTRIP-NEXT: (drop
26+
;; RTRIP-NEXT: (i32.const 1)
27+
;; RTRIP-NEXT: )
28+
;; RTRIP-NEXT: (unreachable)
29+
;; RTRIP-NEXT: )
30+
(func $rmw-null (result i32)
31+
(struct.atomic.rmw.add $struct-i32 0
32+
(ref.null none)
33+
(i32.const 1)
34+
)
35+
)
36+
37+
;; CHECK: (func $cmpxchg-null (type $0) (result i31ref)
38+
;; CHECK-NEXT: (block ;; (replaces unreachable StructCmpxchg we can't emit)
39+
;; CHECK-NEXT: (drop
40+
;; CHECK-NEXT: (ref.null none)
41+
;; CHECK-NEXT: )
42+
;; CHECK-NEXT: (drop
43+
;; CHECK-NEXT: (ref.null none)
44+
;; CHECK-NEXT: )
45+
;; CHECK-NEXT: (drop
46+
;; CHECK-NEXT: (ref.i31
47+
;; CHECK-NEXT: (i32.const 0)
48+
;; CHECK-NEXT: )
49+
;; CHECK-NEXT: )
50+
;; CHECK-NEXT: (unreachable)
51+
;; CHECK-NEXT: )
52+
;; CHECK-NEXT: )
53+
;; RTRIP: (func $cmpxchg-null (type $0) (result i31ref)
54+
;; RTRIP-NEXT: (drop
55+
;; RTRIP-NEXT: (ref.null none)
56+
;; RTRIP-NEXT: )
57+
;; RTRIP-NEXT: (drop
58+
;; RTRIP-NEXT: (ref.null none)
59+
;; RTRIP-NEXT: )
60+
;; RTRIP-NEXT: (drop
61+
;; RTRIP-NEXT: (ref.i31
62+
;; RTRIP-NEXT: (i32.const 0)
63+
;; RTRIP-NEXT: )
64+
;; RTRIP-NEXT: )
65+
;; RTRIP-NEXT: (unreachable)
66+
;; RTRIP-NEXT: )
67+
(func $cmpxchg-null (result i31ref)
68+
;; This is technically invalid because the result should be eqref, but we
69+
;; lose that type information after parsing. Test that we compute a
70+
;; sufficiently general result type from the expected and replacement
71+
;; values.
72+
(struct.atomic.rmw.cmpxchg $struct-eq 0
73+
(ref.null none)
74+
(ref.null none)
75+
(ref.i31 (i32.const 0))
76+
)
77+
)
78+
79+
;; CHECK: (func $cmpxchg-null-2 (type $0) (result i31ref)
80+
;; CHECK-NEXT: (block ;; (replaces unreachable StructCmpxchg we can't emit)
81+
;; CHECK-NEXT: (drop
82+
;; CHECK-NEXT: (ref.null none)
83+
;; CHECK-NEXT: )
84+
;; CHECK-NEXT: (drop
85+
;; CHECK-NEXT: (ref.i31
86+
;; CHECK-NEXT: (i32.const 0)
87+
;; CHECK-NEXT: )
88+
;; CHECK-NEXT: )
89+
;; CHECK-NEXT: (drop
90+
;; CHECK-NEXT: (ref.null none)
91+
;; CHECK-NEXT: )
92+
;; CHECK-NEXT: (unreachable)
93+
;; CHECK-NEXT: )
94+
;; CHECK-NEXT: )
95+
;; RTRIP: (func $cmpxchg-null-2 (type $0) (result i31ref)
96+
;; RTRIP-NEXT: (drop
97+
;; RTRIP-NEXT: (ref.null none)
98+
;; RTRIP-NEXT: )
99+
;; RTRIP-NEXT: (drop
100+
;; RTRIP-NEXT: (ref.i31
101+
;; RTRIP-NEXT: (i32.const 0)
102+
;; RTRIP-NEXT: )
103+
;; RTRIP-NEXT: )
104+
;; RTRIP-NEXT: (drop
105+
;; RTRIP-NEXT: (ref.null none)
106+
;; RTRIP-NEXT: )
107+
;; RTRIP-NEXT: (unreachable)
108+
;; RTRIP-NEXT: )
109+
(func $cmpxchg-null-2 (result i31ref)
110+
;; Same as above, but with the expected and replacement types reversed.
111+
(struct.atomic.rmw.cmpxchg $struct-eq 0
112+
(ref.null none)
113+
(ref.i31 (i32.const 0))
114+
(ref.null none)
115+
)
116+
)
117+
)

0 commit comments

Comments
 (0)