Skip to content

Commit f7f95ba

Browse files
author
Peter Huene
authored
wit-bindgen-rust: fix lowering and lifting of zero-length lists. (#296)
* wit-bindgen-rust: fix lowering and lifting of zero-length lists. This commit fixes the Rust bindings generated for lifting and lowering of lists to account for lists of zero-length. It is undefined behavior to call `std::alloc::alloc` with a zero-sized layout. Additionally, as the behavior of the `canonical_abi_realloc` implemented in `wit-bindgen` is to return a non-null dummy pointer for a zero-sized alloc, the bindings were calling `std::alloc::dealloc` with a pointer that was not allocated with `std::alloc::alloc` (also undefined behavior). To fix this, the bindings now account for zero-length lists and skip allocation and deallocation where appropriate. * Update `list` runtime tests to pass empty lists and strings.
1 parent b40adda commit f7f95ba

File tree

8 files changed

+162
-38
lines changed

8 files changed

+162
-38
lines changed

crates/gen-rust-wasm/src/lib.rs

Lines changed: 36 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -750,12 +750,16 @@ impl FunctionBindgen<'_> {
750750

751751
fn emit_cleanup(&mut self) {
752752
for (ptr, layout) in mem::take(&mut self.cleanup) {
753-
self.push_str(&format!("std::alloc::dealloc({}, {});\n", ptr, layout));
753+
self.push_str(&format!(
754+
"if {layout}.size() != 0 {{\nstd::alloc::dealloc({ptr}, {layout});\n}}\n"
755+
));
754756
}
755757
if self.needs_cleanup_list {
756758
self.push_str(
757-
"for (ptr, layout) in cleanup_list {
758-
std::alloc::dealloc(ptr, layout);
759+
"for (ptr, layout) in cleanup_list {\n
760+
if layout.size() != 0 {\n
761+
std::alloc::dealloc(ptr, layout);\n
762+
}\n
759763
}\n",
760764
);
761765
}
@@ -1365,37 +1369,34 @@ impl Bindgen for FunctionBindgen<'_> {
13651369
Instruction::ListLower { element, realloc } => {
13661370
let body = self.blocks.pop().unwrap();
13671371
let tmp = self.tmp();
1368-
let vec = format!("vec{}", tmp);
1369-
let result = format!("result{}", tmp);
1370-
let layout = format!("layout{}", tmp);
1371-
let len = format!("len{}", tmp);
1372-
self.push_str(&format!("let {} = {};\n", vec, operands[0]));
1373-
self.push_str(&format!("let {} = {}.len() as i32;\n", len, vec));
1374-
let size = self.gen.sizes.size(element);
1375-
let align = self.gen.sizes.align(element);
1372+
let vec = format!("vec{tmp}");
1373+
let result = format!("result{tmp}");
1374+
let layout = format!("layout{tmp}");
1375+
let len = format!("len{tmp}");
13761376
self.push_str(&format!(
1377-
"let {} = core::alloc::Layout::from_size_align_unchecked({}.len() * {}, {});\n",
1378-
layout, vec, size, align,
1377+
"let {vec} = {operand0};\n",
1378+
operand0 = operands[0]
13791379
));
1380+
self.push_str(&format!("let {len} = {vec}.len() as i32;\n"));
1381+
let size = self.gen.sizes.size(element);
1382+
let align = self.gen.sizes.align(element);
13801383
self.push_str(&format!(
1381-
"let {} = std::alloc::alloc({});\n",
1382-
result, layout,
1384+
"let {layout} = core::alloc::Layout::from_size_align_unchecked({vec}.len() * {size}, {align});\n",
13831385
));
13841386
self.push_str(&format!(
1385-
"if {}.is_null() {{ std::alloc::handle_alloc_error({}); }}\n",
1386-
result, layout,
1387+
"let {result} = if {layout}.size() != 0\n{{\nlet ptr = std::alloc::alloc({layout});\n",
13871388
));
13881389
self.push_str(&format!(
1389-
"for (i, e) in {}.into_iter().enumerate() {{\n",
1390-
vec
1390+
"if ptr.is_null()\n{{\nstd::alloc::handle_alloc_error({layout});\n}}\nptr\n}}",
13911391
));
1392+
self.push_str(&format!("else {{\nstd::ptr::null_mut()\n}};\n",));
1393+
self.push_str(&format!("for (i, e) in {vec}.into_iter().enumerate() {{\n",));
13921394
self.push_str(&format!(
1393-
"let base = {} as i32 + (i as i32) * {};\n",
1394-
result, size,
1395+
"let base = {result} as i32 + (i as i32) * {size};\n",
13951396
));
13961397
self.push_str(&body);
13971398
self.push_str("}\n");
1398-
results.push(format!("{} as i32", result));
1399+
results.push(format!("{result} as i32"));
13991400
results.push(len);
14001401

14011402
if realloc.is_none() {
@@ -1414,14 +1415,19 @@ impl Bindgen for FunctionBindgen<'_> {
14141415
let tmp = self.tmp();
14151416
let size = self.gen.sizes.size(element);
14161417
let align = self.gen.sizes.align(element);
1417-
let len = format!("len{}", tmp);
1418-
let base = format!("base{}", tmp);
1419-
let result = format!("result{}", tmp);
1420-
self.push_str(&format!("let {} = {};\n", base, operands[0]));
1421-
self.push_str(&format!("let {} = {};\n", len, operands[1],));
1418+
let len = format!("len{tmp}");
1419+
let base = format!("base{tmp}");
1420+
let result = format!("result{tmp}");
1421+
self.push_str(&format!(
1422+
"let {base} = {operand0};\n",
1423+
operand0 = operands[0]
1424+
));
1425+
self.push_str(&format!(
1426+
"let {len} = {operand1};\n",
1427+
operand1 = operands[1]
1428+
));
14221429
self.push_str(&format!(
1423-
"let mut {} = Vec::with_capacity({} as usize);\n",
1424-
result, len,
1430+
"let mut {result} = Vec::with_capacity({len} as usize);\n",
14251431
));
14261432

14271433
self.push_str("for i in 0..");
@@ -1439,14 +1445,7 @@ impl Bindgen for FunctionBindgen<'_> {
14391445
self.push_str("}\n");
14401446
results.push(result);
14411447
self.push_str(&format!(
1442-
"std::alloc::dealloc(
1443-
{} as *mut _,
1444-
std::alloc::Layout::from_size_align_unchecked(
1445-
({} as usize) * {},
1446-
{},
1447-
),
1448-
);\n",
1449-
base, len, size, align
1448+
"if {len} != 0 {{\nstd::alloc::dealloc({base} as *mut _, std::alloc::Layout::from_size_align_unchecked(({len} as usize) * {size}, {align}));\n}}\n",
14501449
));
14511450
}
14521451

tests/runtime/lists/exports.wit

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
test-imports: func()
22
allocated-bytes: func() -> u32
33

4+
empty-list-param: func(a: list<u8>)
5+
empty-string-param: func(a: string)
6+
empty-list-result: func() -> list<u8>
7+
empty-string-result: func() -> string
8+
49
list-param: func(a: list<u8>)
510
list-param2: func(a: string)
611
list-param3: func(a: list<string>)

tests/runtime/lists/host.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,18 @@
77
import wasmtime
88

99
class MyImports:
10+
def empty_list_param(self, a: bytes) -> None:
11+
assert(a == b'')
12+
13+
def empty_string_param(self, a: str) -> None:
14+
assert(a == '')
15+
16+
def empty_list_result(self) -> bytes:
17+
return b''
18+
19+
def empty_string_result(self) -> str:
20+
return ''
21+
1022
def list_param(self, a: bytes) -> None:
1123
assert(a == b'\x01\x02\x03\x04')
1224

@@ -89,6 +101,10 @@ def run(wasm_file: str) -> None:
89101

90102
allocated_bytes = wasm.allocated_bytes(store)
91103
wasm.test_imports(store)
104+
wasm.empty_list_param(store, b'')
105+
wasm.empty_string_param(store, '')
106+
assert(wasm.empty_list_result(store) == b'')
107+
assert(wasm.empty_string_result(store) == '')
92108
wasm.list_param(store, b'\x01\x02\x03\x04')
93109
wasm.list_param2(store, "foo")
94110
wasm.list_param3(store, ["foo", "bar", "baz"])

tests/runtime/lists/host.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,22 @@ use wit_bindgen_wasmtime::Le;
99
pub struct MyImports;
1010

1111
impl Imports for MyImports {
12+
fn empty_list_param(&mut self, a: &[u8]) {
13+
assert_eq!(a, []);
14+
}
15+
16+
fn empty_string_param(&mut self, a: &str) {
17+
assert_eq!(a, "");
18+
}
19+
20+
fn empty_list_result(&mut self) -> Vec<u8> {
21+
Vec::new()
22+
}
23+
24+
fn empty_string_result(&mut self) -> String {
25+
String::new()
26+
}
27+
1228
fn list_param(&mut self, list: &[u8]) {
1329
assert_eq!(list, [1, 2, 3, 4]);
1430
}
@@ -139,6 +155,10 @@ fn run(wasm: &str) -> Result<()> {
139155

140156
let bytes = exports.allocated_bytes(&mut store)?;
141157
exports.test_imports(&mut store)?;
158+
exports.empty_list_param(&mut store, &[])?;
159+
exports.empty_string_param(&mut store, "")?;
160+
assert_eq!(exports.empty_list_result(&mut store)?, []);
161+
assert_eq!(exports.empty_string_result(&mut store)?, "");
142162
exports.list_param(&mut store, &[1, 2, 3, 4])?;
143163
exports.list_param2(&mut store, "foo")?;
144164
exports.list_param3(&mut store, &["foo", "bar", "baz"])?;

tests/runtime/lists/host.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,16 @@ import * as assert from 'assert';
88
async function run() {
99
const importObj = {};
1010
const imports: Imports = {
11+
emptyListParam(a) {
12+
assert.deepStrictEqual(Array.from(a), []);
13+
},
14+
emptyStringParam(a) {
15+
assert.strictEqual(a, '');
16+
},
17+
emptyListResult() {
18+
return new Uint8Array([]);
19+
},
20+
emptyStringResult() { return ''; },
1121
listParam(a) {
1222
assert.deepStrictEqual(Array.from(a), [1, 2, 3, 4]);
1323
},
@@ -113,14 +123,18 @@ async function run() {
113123

114124
const bytes = wasm.allocatedBytes();
115125
wasm.testImports();
126+
wasm.emptyListParam(new Uint8Array([]));
127+
wasm.emptyStringParam('');
116128
wasm.listParam(new Uint8Array([1, 2, 3, 4]));
117129
wasm.listParam2("foo");
118130
wasm.listParam3(["foo", "bar", "baz"]);
119131
wasm.listParam4([["foo", "bar"], ["baz"]]);
132+
assert.deepStrictEqual(Array.from(wasm.emptyListResult()), []);
133+
assert.deepStrictEqual(wasm.emptyStringResult(), "");
120134
assert.deepStrictEqual(Array.from(wasm.listResult()), [1, 2, 3, 4, 5]);
121135
assert.deepStrictEqual(wasm.listResult2(), "hello!");
122136
assert.deepStrictEqual(wasm.listResult3(), ["hello,", "world!"]);
123-
137+
124138
const buffer = new ArrayBuffer(8);
125139
(new Uint8Array(buffer)).set(new Uint8Array([1, 2, 3, 4]), 2);
126140
// Create a view of the four bytes in the middle of the buffer

tests/runtime/lists/imports.wit

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ flags flag64 {
1616
b56, b57, b58, b59, b60, b61, b62, b63,
1717
}
1818

19+
empty-list-param: func(a: list<u8>)
20+
empty-string-param: func(a: string)
21+
empty-list-result: func() -> list<u8>
22+
empty-string-result: func() -> string
23+
1924
list-param: func(a: list<u8>)
2025
list-param2: func(a: string)
2126
list-param3: func(a: list<string>)

tests/runtime/lists/wasm.c

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,32 @@ uint32_t exports_allocated_bytes(void) {
3535
}
3636

3737
void exports_test_imports() {
38+
{
39+
uint8_t list[] = {};
40+
imports_list_u8_t a;
41+
a.ptr = list;
42+
a.len = 0;
43+
imports_empty_list_param(&a);
44+
}
45+
46+
{
47+
imports_string_t a;
48+
imports_string_set(&a, "");
49+
imports_empty_string_param(&a);
50+
}
51+
52+
{
53+
imports_list_u8_t a;
54+
imports_empty_list_result(&a);
55+
assert(a.len == 0);
56+
}
57+
58+
{
59+
imports_string_t a;
60+
imports_empty_string_result(&a);
61+
assert(a.len == 0);
62+
}
63+
3864
{
3965
uint8_t list[] = {1, 2, 3, 4};
4066
imports_list_u8_t a;
@@ -201,6 +227,24 @@ void exports_test_imports() {
201227
}
202228
}
203229

230+
void exports_empty_list_param(exports_list_u8_t *a) {
231+
assert(a->len == 0);
232+
}
233+
234+
void exports_empty_string_param(exports_string_t *a) {
235+
assert(a->len == 0);
236+
}
237+
238+
void exports_empty_list_result(exports_list_u8_t *ret0) {
239+
ret0->ptr = 0;
240+
ret0->len = 0;
241+
}
242+
243+
void exports_empty_string_result(exports_string_t *ret0) {
244+
ret0->ptr = 0;
245+
ret0->len = 0;
246+
}
247+
204248
void exports_list_param(exports_list_u8_t *a) {
205249
assert(a->len == 4);
206250
assert(a->ptr[0] == 1);

tests/runtime/lists/wasm.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@ impl exports::Exports for Exports {
1717

1818
let _guard = test_rust_wasm::guard();
1919

20+
empty_list_param(&[]);
21+
empty_string_param("");
22+
assert!(empty_list_result().is_empty());
23+
assert!(empty_string_result().is_empty());
24+
2025
list_param(&[1, 2, 3, 4]);
2126
list_param2("foo");
2227
list_param3(&["foo", "bar", "baz"]);
@@ -125,6 +130,22 @@ impl exports::Exports for Exports {
125130
);
126131
}
127132

133+
fn empty_list_param(a: Vec<u8>) {
134+
assert!(a.is_empty());
135+
}
136+
137+
fn empty_string_param(a: String) {
138+
assert!(a.is_empty());
139+
}
140+
141+
fn empty_list_result() -> Vec<u8> {
142+
Vec::new()
143+
}
144+
145+
fn empty_string_result() -> String {
146+
String::new()
147+
}
148+
128149
fn list_param(list: Vec<u8>) {
129150
assert_eq!(list, [1, 2, 3, 4]);
130151
}

0 commit comments

Comments
 (0)