Skip to content

Commit 7478232

Browse files
alexcrichtonalexcrichton
authored
Require memory option if realloc is specified (#2505)
This commit closes a validation gap between `wasm-tools` and the upstream specification by ensuring that if the `realloc` option is specified at all that the `memory` option is additionally specified. This was discovered recently as a discrepancy with the specification itself, and I split this out to be able to sit down and think a bit more about this change. Specifically I've validated that `wit-component` cannot produce a component that uses `realloc` but does not use `memory` by accident. This means that in practice I'm expecting that this won't cause any accidentally-valid components in use today to stop running. Instead there are unlikely to be any components in the wild that this affects. Closes #2503
1 parent 61a7487 commit 7478232

6 files changed

Lines changed: 201 additions & 47 deletions

File tree

‎crates/wasmparser/src/validator/component.rs‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2793,16 +2793,18 @@ impl ComponentState {
27932793

27942794
// Validate `realloc`
27952795
if let Some(realloc_idx) = realloc {
2796-
let addr_type = match memory {
2797-
// If a memory was specified, `realloc` must match its address type.
2798-
Some(memory_idx) => match self.memory_at(memory_idx, offset)?.memory64 {
2799-
true => ValType::I64,
2800-
false => ValType::I32,
2801-
},
2802-
// Backwards compatibility: Assume `i32` memory if none was specified.
2803-
// FIXME(#2503): The spec requires `memory` if `realloc` is specified, but this
2804-
// may break existing code.
2805-
None => ValType::I32,
2796+
let mty = match memory {
2797+
Some(i) => self.memory_at(i, offset)?,
2798+
None => {
2799+
return Err(BinaryReaderError::new(
2800+
"canonical option `realloc` requires `memory` to also be specified",
2801+
offset,
2802+
));
2803+
}
2804+
};
2805+
let addr_type = match mty.memory64 {
2806+
true => ValType::I64,
2807+
false => ValType::I32,
28062808
};
28072809
let ty_id = self.core_function_at(realloc_idx, offset)?;
28082810
let func_ty = types[ty_id].unwrap_func();

‎tests/cli/component-model/async/task-builtins.wast‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,10 @@
8080
(component
8181
(core module $m
8282
(func (export "r") (param i32 i32 i32 i32) (result i32) unreachable)
83+
(memory (export "m") 0)
8384
)
8485
(core instance $m (instantiate $m))
85-
(core func $task-return (canon task.return (result u32) (realloc (func $m "r"))))
86+
(core func $task-return (canon task.return (result u32) (realloc (func $m "r")) (memory $m "m")))
8687
)
8788
"cannot specify `realloc` option on `task.return`")
8889

‎tests/cli/component-model/options.wast‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
;; RUN: wast --assert default --snapshot tests/snapshots % -f custom-page-sizes
2+
3+
(component
4+
(core module $m
5+
(func (export "r") (param i32 i32 i32 i32) (result i32) unreachable)
6+
(memory (export "m") 0)
7+
8+
(func (export "f"))
9+
)
10+
(core instance $i (instantiate $m))
11+
12+
(func (canon lift (core func $i "f")))
13+
(func (canon lift (core func $i "f") (memory $i "m")))
14+
(func (canon lift (core func $i "f") (memory $i "m") (realloc (func $i "r"))))
15+
(func (canon lift (core func $i "f") (realloc (func $i "r")) (memory $i "m")))
16+
)
17+
18+
;; `realloc` requires `memory`
19+
(assert_invalid
20+
(component
21+
(core module $m
22+
(func (export "r") (param i32 i32 i32 i32) (result i32) unreachable)
23+
(func (export "f"))
24+
)
25+
(core instance $i (instantiate $m))
26+
(func (canon lift (core func $i "f") (realloc (func $i "r"))))
27+
)
28+
"canonical option `realloc` requires `memory` to also be specified")
29+
30+
;; even if `realloc` isn't needed, it's still validated
31+
(assert_invalid
32+
(component
33+
(core module $m
34+
(func (export "r"))
35+
(memory (export "m") 0)
36+
(func (export "f"))
37+
)
38+
(core instance $i (instantiate $m))
39+
(func (canon lift (core func $i "f") (realloc (func $i "r")) (memory $i "m")))
40+
)
41+
"canonical option `realloc` uses a core function with an incorrect signature")
42+
43+
;; even if `memory` isn't needed, it's still validated
44+
(assert_invalid
45+
(component
46+
(core module $m
47+
(memory (export "m") 0 (pagesize 1))
48+
(func (export "f"))
49+
)
50+
(core instance $i (instantiate $m))
51+
(func (canon lift (core func $i "f") (memory $i "m")))
52+
)
53+
"mismatch in page size for memories")
54+
(assert_invalid
55+
(component
56+
(core module $m
57+
(memory (export "m") 0 1 shared)
58+
(func (export "f"))
59+
)
60+
(core instance $i (instantiate $m))
61+
(func (canon lift (core func $i "f") (memory $i "m")))
62+
)
63+
"mismatch in the shared flag for memories")
64+
(assert_invalid
65+
(component
66+
(core module $m
67+
(memory (export "m") i64 0)
68+
(func (export "f"))
69+
)
70+
(core instance $i (instantiate $m))
71+
(func (canon lift (core func $i "f") (memory $i "m")))
72+
)
73+
"64-bit memories require the `cm64` feature to be enabled")

‎tests/snapshots/cli/component-model/async/task-builtins.wast.json‎

Lines changed: 36 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -70,237 +70,237 @@
7070
},
7171
{
7272
"type": "module",
73-
"line": 89,
73+
"line": 90,
7474
"filename": "task-builtins.10.wasm",
7575
"module_type": "binary"
7676
},
7777
{
7878
"type": "module",
79-
"line": 101,
79+
"line": 102,
8080
"filename": "task-builtins.11.wasm",
8181
"module_type": "binary"
8282
},
8383
{
8484
"type": "module",
85-
"line": 110,
85+
"line": 111,
8686
"filename": "task-builtins.12.wasm",
8787
"module_type": "binary"
8888
},
8989
{
9090
"type": "assert_invalid",
91-
"line": 118,
91+
"line": 119,
9292
"filename": "task-builtins.13.wasm",
9393
"module_type": "binary",
9494
"text": "type mismatch for export `waitable-set.new` of module instantiation argument ``"
9595
},
9696
{
9797
"type": "module",
98-
"line": 127,
98+
"line": 128,
9999
"filename": "task-builtins.14.wasm",
100100
"module_type": "binary"
101101
},
102102
{
103103
"type": "assert_invalid",
104-
"line": 139,
104+
"line": 140,
105105
"filename": "task-builtins.15.wasm",
106106
"module_type": "binary",
107107
"text": "type mismatch for export `waitable-set.wait` of module instantiation argument ``"
108108
},
109109
{
110110
"type": "module",
111-
"line": 151,
111+
"line": 152,
112112
"filename": "task-builtins.16.wasm",
113113
"module_type": "binary"
114114
},
115115
{
116116
"type": "module",
117-
"line": 162,
117+
"line": 163,
118118
"filename": "task-builtins.17.wasm",
119119
"module_type": "binary"
120120
},
121121
{
122122
"type": "module",
123-
"line": 171,
123+
"line": 172,
124124
"filename": "task-builtins.18.wasm",
125125
"module_type": "binary"
126126
},
127127
{
128128
"type": "assert_invalid",
129-
"line": 183,
129+
"line": 184,
130130
"filename": "task-builtins.19.wasm",
131131
"module_type": "binary",
132132
"text": "type mismatch for export `waitable-set.poll` of module instantiation argument ``"
133133
},
134134
{
135135
"type": "module",
136-
"line": 196,
136+
"line": 197,
137137
"filename": "task-builtins.20.wasm",
138138
"module_type": "binary"
139139
},
140140
{
141141
"type": "assert_invalid",
142-
"line": 204,
142+
"line": 205,
143143
"filename": "task-builtins.21.wasm",
144144
"module_type": "binary",
145145
"text": "type mismatch for export `waitable-set.drop` of module instantiation argument ``"
146146
},
147147
{
148148
"type": "module",
149-
"line": 213,
149+
"line": 214,
150150
"filename": "task-builtins.22.wasm",
151151
"module_type": "binary"
152152
},
153153
{
154154
"type": "assert_invalid",
155-
"line": 221,
155+
"line": 222,
156156
"filename": "task-builtins.23.wasm",
157157
"module_type": "binary",
158158
"text": "type mismatch for export `waitable.join` of module instantiation argument ``"
159159
},
160160
{
161161
"type": "module",
162-
"line": 230,
162+
"line": 231,
163163
"filename": "task-builtins.24.wasm",
164164
"module_type": "binary"
165165
},
166166
{
167167
"type": "assert_invalid",
168-
"line": 240,
168+
"line": 241,
169169
"filename": "task-builtins.25.wasm",
170170
"module_type": "binary",
171171
"text": "type mismatch for export `subtask.drop` of module instantiation argument ``"
172172
},
173173
{
174174
"type": "module",
175-
"line": 251,
175+
"line": 252,
176176
"filename": "task-builtins.26.wasm",
177177
"module_type": "binary"
178178
},
179179
{
180180
"type": "assert_invalid",
181-
"line": 261,
181+
"line": 262,
182182
"filename": "task-builtins.27.wasm",
183183
"module_type": "binary",
184184
"text": "type mismatch for export `subtask.cancel` of module instantiation argument ``"
185185
},
186186
{
187187
"type": "module",
188-
"line": 272,
188+
"line": 273,
189189
"filename": "task-builtins.28.wasm",
190190
"module_type": "binary"
191191
},
192192
{
193193
"type": "module",
194-
"line": 289,
194+
"line": 290,
195195
"filename": "task-builtins.29.wasm",
196196
"module_type": "binary"
197197
},
198198
{
199199
"type": "module",
200-
"line": 297,
200+
"line": 298,
201201
"filename": "task-builtins.30.wasm",
202202
"module_type": "binary"
203203
},
204204
{
205205
"type": "assert_invalid",
206-
"line": 307,
206+
"line": 308,
207207
"filename": "task-builtins.31.wasm",
208208
"module_type": "binary",
209209
"text": "type mismatch for export `thread.yield` of module instantiation argument ``"
210210
},
211211
{
212212
"type": "assert_invalid",
213-
"line": 318,
213+
"line": 319,
214214
"filename": "task-builtins.32.wasm",
215215
"module_type": "binary",
216216
"text": "found: (func (result i32))"
217217
},
218218
{
219219
"type": "assert_invalid",
220-
"line": 325,
220+
"line": 326,
221221
"filename": "task-builtins.33.wasm",
222222
"module_type": "binary",
223223
"text": "found: (func (param i32))"
224224
},
225225
{
226226
"type": "assert_invalid",
227-
"line": 332,
227+
"line": 333,
228228
"filename": "task-builtins.34.wasm",
229229
"module_type": "binary",
230230
"text": "immediate must be zero: 1"
231231
},
232232
{
233233
"type": "assert_invalid",
234-
"line": 336,
234+
"line": 337,
235235
"filename": "task-builtins.35.wasm",
236236
"module_type": "binary",
237237
"text": "immediate must be zero: 1"
238238
},
239239
{
240240
"type": "assert_invalid",
241-
"line": 340,
241+
"line": 341,
242242
"filename": "task-builtins.36.wasm",
243243
"module_type": "binary",
244244
"text": "immediate must be zero: 100"
245245
},
246246
{
247247
"type": "assert_invalid",
248-
"line": 344,
248+
"line": 345,
249249
"filename": "task-builtins.37.wasm",
250250
"module_type": "binary",
251251
"text": "immediate must be zero: 100"
252252
},
253253
{
254254
"type": "assert_invalid",
255-
"line": 350,
255+
"line": 351,
256256
"filename": "task-builtins.38.wasm",
257257
"module_type": "binary",
258258
"text": "64-bit `context.get` requires the component model 64-bit feature"
259259
},
260260
{
261261
"type": "assert_invalid",
262-
"line": 354,
262+
"line": 355,
263263
"filename": "task-builtins.39.wasm",
264264
"module_type": "binary",
265265
"text": "64-bit `context.set` requires the component model 64-bit feature"
266266
},
267267
{
268268
"type": "assert_invalid",
269-
"line": 359,
269+
"line": 360,
270270
"filename": "task-builtins.40.wasm",
271271
"module_type": "binary",
272272
"text": "64-bit `context.get` requires the component model 64-bit feature"
273273
},
274274
{
275275
"type": "assert_invalid",
276-
"line": 366,
276+
"line": 367,
277277
"filename": "task-builtins.41.wasm",
278278
"module_type": "binary",
279279
"text": "64-bit `context.set` requires the component model 64-bit feature"
280280
},
281281
{
282282
"type": "assert_invalid",
283-
"line": 375,
283+
"line": 376,
284284
"filename": "task-builtins.42.wasm",
285285
"module_type": "binary",
286286
"text": "`context.get` only supports `i32` or `i64`"
287287
},
288288
{
289289
"type": "assert_invalid",
290-
"line": 379,
290+
"line": 380,
291291
"filename": "task-builtins.43.wasm",
292292
"module_type": "binary",
293293
"text": "`context.set` only supports `i32` or `i64`"
294294
},
295295
{
296296
"type": "module",
297-
"line": 385,
297+
"line": 386,
298298
"filename": "task-builtins.44.wasm",
299299
"module_type": "binary"
300300
},
301301
{
302302
"type": "module",
303-
"line": 445,
303+
"line": 446,
304304
"filename": "task-builtins.45.wasm",
305305
"module_type": "binary"
306306
}

0 commit comments

Comments
 (0)