Skip to content

Commit d9baa75

Browse files
committed
fix: implement deallocation for fixed-length lists with heap elements
Fixed three bugs in the handling of FixedLengthList types: 1. `deallocate` in abi.rs was `todo!()`, causing a panic when flat deallocation was needed (e.g. `list<own<resource>, 3>`). Now uses `flat_for_each_record_type` to iterate element types. 2. `deallocate_indirect` in abi.rs was a silent no-op `=> {}`, leaking heap-allocated elements (e.g. strings in `list<string, 3>`). Now iterates each element's memory offset and deallocates using `deallocate_indirect_fields`. 3. `FixedLengthListLower` in the Rust code generator used array indexing (`a[0]`, `a[1]`, ...) to extract elements, which fails for non-Copy types like String. Changed to use array destructuring (`let [e0, e1, ...] = a;`) to properly move elements out. Added runtime tests with `list<string, 3>` functions (param, result, roundtrip) and allocation tracking via Guard pattern to verify no memory leaks when passing/returning fixed-length lists of strings.
1 parent d61d370 commit d9baa75

File tree

7 files changed

+143
-10
lines changed

7 files changed

+143
-10
lines changed

crates/core/src/abi.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2404,7 +2404,13 @@ impl<'a, B: Bindgen> Generator<'a, B> {
24042404
TypeDefKind::Resource => unreachable!(),
24052405
TypeDefKind::Unknown => unreachable!(),
24062406

2407-
TypeDefKind::FixedLengthList(..) => todo!(),
2407+
TypeDefKind::FixedLengthList(element, size) => {
2408+
self.flat_for_each_record_type(
2409+
ty,
2410+
iter::repeat_n(element, *size as usize),
2411+
|me, ty| me.deallocate(ty, what),
2412+
);
2413+
}
24082414
TypeDefKind::Map(..) => todo!(),
24092415
},
24102416
}
@@ -2526,7 +2532,10 @@ impl<'a, B: Bindgen> Generator<'a, B> {
25262532
TypeDefKind::Future(_) => unreachable!(),
25272533
TypeDefKind::Stream(_) => unreachable!(),
25282534
TypeDefKind::Unknown => unreachable!(),
2529-
TypeDefKind::FixedLengthList(_, _) => {}
2535+
TypeDefKind::FixedLengthList(element, size) => {
2536+
let tys: Vec<Type> = iter::repeat_n(*element, *size as usize).collect();
2537+
self.deallocate_indirect_fields(&tys, addr, offset, what);
2538+
}
25302539
TypeDefKind::Map(..) => todo!(),
25312540
},
25322541
}

crates/rust/src/bindgen.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,9 +1224,12 @@ impl Bindgen for FunctionBindgen<'_, '_> {
12241224
size,
12251225
id: _,
12261226
} => {
1227-
for i in 0..(*size as usize) {
1228-
results.push(format!("{}[{i}]", operands[0]));
1229-
}
1227+
let tmp = self.tmp();
1228+
let names: Vec<String> = (0..(*size as usize))
1229+
.map(|i| format!("e{tmp}_{i}"))
1230+
.collect();
1231+
self.push_str(&format!("let [{}] = {};\n", names.join(", "), operands[0]));
1232+
results.extend(names);
12301233
}
12311234
Instruction::FixedLengthListLiftFromMemory {
12321235
element,
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
use std::alloc::{GlobalAlloc, Layout, System};
2+
use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
3+
4+
#[global_allocator]
5+
static ALLOC: A = A;
6+
7+
static ALLOC_AMT: AtomicUsize = AtomicUsize::new(0);
8+
9+
struct A;
10+
11+
unsafe impl GlobalAlloc for A {
12+
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
13+
let ptr = System.alloc(layout);
14+
if !ptr.is_null() {
15+
ALLOC_AMT.fetch_add(layout.size(), SeqCst);
16+
}
17+
return ptr;
18+
}
19+
20+
unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
21+
// Poison all deallocations to try to catch any use-after-free in the
22+
// bindings as early as possible.
23+
std::ptr::write_bytes(ptr, 0xde, layout.size());
24+
ALLOC_AMT.fetch_sub(layout.size(), SeqCst);
25+
System.dealloc(ptr, layout)
26+
}
27+
}
28+
pub fn get() -> usize {
29+
ALLOC_AMT.load(SeqCst)
30+
}

tests/runtime/fixed-length-lists/runner.rs

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,30 +4,66 @@ include!(env!("BINDINGS"));
44

55
use test::fixed_length_lists::to_test::*;
66

7+
mod alloc;
8+
9+
struct Guard {
10+
me_before: usize,
11+
remote_before: u32,
12+
}
13+
14+
impl Guard {
15+
fn new() -> Guard {
16+
Guard {
17+
me_before: alloc::get(),
18+
remote_before: allocated_bytes(),
19+
}
20+
}
21+
}
22+
23+
impl Drop for Guard {
24+
fn drop(&mut self) {
25+
assert_eq!(self.me_before, alloc::get());
26+
assert_eq!(self.remote_before, allocated_bytes());
27+
}
28+
}
29+
730
struct Component;
831

932
export!(Component);
1033

1134
impl Guest for Component {
1235
fn run() {
13-
list_param([1, 2, 3, 4]);
14-
list_param2([[1, 2], [3, 4]]);
15-
list_param3([
16-
-1, 2, -3, 4, -5, 6, -7, 8, -9, 10, -11, 12, -13, 14, -15, 16, -17, 18, -19, 20,
17-
]);
1836
{
37+
let _guard = Guard::new();
38+
list_param([1, 2, 3, 4]);
39+
}
40+
{
41+
let _guard = Guard::new();
42+
list_param2([[1, 2], [3, 4]]);
43+
}
44+
{
45+
let _guard = Guard::new();
46+
list_param3([
47+
-1, 2, -3, 4, -5, 6, -7, 8, -9, 10, -11, 12, -13, 14, -15, 16, -17, 18, -19, 20,
48+
]);
49+
}
50+
{
51+
let _guard = Guard::new();
1952
let result = list_result();
2053
assert_eq!(result, [b'0', b'1', b'A', b'B', b'a', b'b', 128, 255]);
2154
}
2255
{
56+
let _guard = Guard::new();
2357
let result = list_minmax16([0, 1024, 32768, 65535], [1, 2048, -32767, -2]);
2458
assert_eq!(result, ([0, 1024, 32768, 65535], [1, 2048, -32767, -2]));
2559
}
2660
{
61+
let _guard = Guard::new();
2762
let result = list_minmax_float([2.0, -42.0], [0.25, -0.125]);
2863
assert_eq!(result, ([2.0, -42.0], [0.25, -0.125]));
2964
}
3065
{
66+
let _guard = Guard::new();
3167
let result =
3268
list_roundtrip([b'a', b'b', b'c', b'd', 0, 1, 2, 3, b'A', b'B', b'Y', b'Z']);
3369
assert_eq!(
@@ -36,13 +72,15 @@ impl Guest for Component {
3672
);
3773
}
3874
{
75+
let _guard = Guard::new();
3976
let result = nested_roundtrip([[1, 5], [42, 1_000_000]], [[-1, 3], [-2_000_000, 4711]]);
4077
assert_eq!(
4178
result,
4279
([[1, 5], [42, 1_000_000]], [[-1, 3], [-2_000_000, 4711]])
4380
);
4481
}
4582
{
83+
let _guard = Guard::new();
4684
let result = large_roundtrip(
4785
[[1, 5], [42, 1_000_000]],
4886
[
@@ -66,9 +104,24 @@ impl Guest for Component {
66104
);
67105
}
68106
{
107+
let _guard = Guard::new();
69108
let result = nightmare_on_cpp([Nested { l: [1, -1] }, Nested { l: [2, -2] }]);
70109
assert_eq!(result[0].l, [1, -1]);
71110
assert_eq!(result[1].l, [2, -2]);
72111
}
112+
{
113+
let _guard = Guard::new();
114+
string_list_param(["foo".to_owned(), "bar".to_owned(), "baz".to_owned()]);
115+
}
116+
{
117+
let _guard = Guard::new();
118+
let result = string_list_result();
119+
assert_eq!(result, ["foo", "bar", "baz"]);
120+
}
121+
{
122+
let _guard = Guard::new();
123+
let result = string_list_roundtrip(["foo".to_owned(), "bar".to_owned(), "baz".to_owned()]);
124+
assert_eq!(result, ["foo", "bar", "baz"]);
125+
}
73126
}
74127
}

tests/runtime/fixed-length-lists/test.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,21 @@ std::array<to_test::Nested, 2>
4949
to_test::NightmareOnCpp(std::array<to_test::Nested, 2> a) {
5050
return a;
5151
}
52+
void to_test::StringListParam(std::array<wit::string, 3> a) {
53+
assert(a[0].get_view() == "foo");
54+
assert(a[1].get_view() == "bar");
55+
assert(a[2].get_view() == "baz");
56+
}
57+
std::array<wit::string, 3> to_test::StringListResult() {
58+
return std::array<wit::string, 3>{
59+
wit::string::from_view("foo"),
60+
wit::string::from_view("bar"),
61+
wit::string::from_view("baz"),
62+
};
63+
}
64+
std::array<wit::string, 3> to_test::StringListRoundtrip(std::array<wit::string, 3> a) {
65+
return a;
66+
}
67+
uint32_t to_test::AllocatedBytes() {
68+
return 0;
69+
}

tests/runtime/fixed-length-lists/test.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,14 @@ struct Component;
44

55
export!(Component);
66

7+
mod alloc;
8+
79
use crate::exports::test::fixed_length_lists::to_test::Nested;
810

911
impl exports::test::fixed_length_lists::to_test::Guest for Component {
12+
fn allocated_bytes() -> u32 {
13+
alloc::get().try_into().unwrap()
14+
}
1015
fn list_param(a: [u32; 4]) {
1116
assert_eq!(a, [1, 2, 3, 4]);
1217
}
@@ -40,4 +45,13 @@ impl exports::test::fixed_length_lists::to_test::Guest for Component {
4045
fn nightmare_on_cpp(a: [Nested; 2]) -> [Nested; 2] {
4146
a
4247
}
48+
fn string_list_param(a: [String; 3]) {
49+
assert_eq!(a, ["foo", "bar", "baz"]);
50+
}
51+
fn string_list_result() -> [String; 3] {
52+
["foo".to_owned(), "bar".to_owned(), "baz".to_owned()]
53+
}
54+
fn string_list_roundtrip(a: [String; 3]) -> [String; 3] {
55+
a
56+
}
4357
}

tests/runtime/fixed-length-lists/test.wit

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@ interface to-test {
2020
}
2121

2222
nightmare-on-cpp: func(a: list<nested, 2>) -> list<nested, 2>;
23+
24+
string-list-param: func(a: list<string, 3>);
25+
string-list-result: func() -> list<string, 3>;
26+
string-list-roundtrip: func(a: list<string, 3>) -> list<string, 3>;
27+
28+
allocated-bytes: func() -> u32;
2329
}
2430

2531
world test {

0 commit comments

Comments
 (0)