Skip to content

Commit 17933c5

Browse files
committed
fix: implement deallocation for fixed-length lists with heap elements
Fixed two bugs in the core ABI for FixedLengthList types: 1. `deallocate` 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` 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 it using `deallocate_indirect_fields`. Added both a codegen test (verifying `cabi_post_` functions are generated) and a runtime test with allocation tracking (verifying no memory leaks when passing/returning fixed-length lists of strings).
1 parent fd57889 commit 17933c5

File tree

7 files changed

+163
-2
lines changed

7 files changed

+163
-2
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/tests/codegen.rs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,44 @@ mod newtyped_list {
142142
}
143143
}
144144
}
145+
146+
#[test]
147+
fn fixed_length_list_deallocation() {
148+
let wit = r#"
149+
package test:fll-dealloc;
150+
151+
world test {
152+
export return-string-array: func() -> list<string, 3>;
153+
export accept-string-array: func(a: list<string, 3>);
154+
}
155+
"#;
156+
157+
let mut resolve = wit_bindgen_core::wit_parser::Resolve::default();
158+
let pkg = resolve.push_str("test.wit", wit).unwrap();
159+
let world = resolve.select_world(&[pkg], None).unwrap();
160+
161+
let opts = wit_bindgen_rust::Opts {
162+
generate_all: true,
163+
..Default::default()
164+
};
165+
166+
let mut generator = opts.build();
167+
let mut files = wit_bindgen_core::Files::default();
168+
use wit_bindgen_core::WorldGenerator;
169+
generator.generate(&mut resolve, world, &mut files).unwrap();
170+
171+
let (_name, contents) = files.iter().next().unwrap();
172+
let code = String::from_utf8_lossy(contents);
173+
174+
// Verify post-return deallocation functions are generated for
175+
// fixed-length lists containing heap-allocated elements (strings)
176+
assert!(
177+
code.contains("cabi_post_"),
178+
"expected cabi_post_ deallocation function for fixed-length list \
179+
with string elements, but none was generated"
180+
);
181+
}
182+
145183
#[allow(unused, reason = "testing codegen, not functionality")]
146184
mod retyped_list {
147185
use std::ops::Deref;
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: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,29 @@ 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);
@@ -70,5 +93,28 @@ impl Guest for Component {
7093
assert_eq!(result[0].l, [1, -1]);
7194
assert_eq!(result[1].l, [2, -2]);
7295
}
96+
97+
// Test deallocation of heap-allocated elements in fixed-length lists.
98+
// Strings require heap allocation, so passing and returning them through
99+
// fixed-length lists exercises the deallocate/deallocate_indirect paths.
100+
{
101+
let _guard = Guard::new();
102+
string_list_param([
103+
"hello".to_string(),
104+
"world".to_string(),
105+
"!".to_string(),
106+
]);
107+
}
108+
{
109+
let _guard = Guard::new();
110+
let result = string_list_result();
111+
assert_eq!(result, ["foo", "bar"]);
112+
}
113+
{
114+
let _guard = Guard::new();
115+
let result =
116+
string_list_roundtrip(["hello".to_string(), "world".to_string()]);
117+
assert_eq!(result, ["hello", "world"]);
118+
}
73119
}
74120
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,17 @@ 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() == "hello");
54+
assert(a[1].get_view() == "world");
55+
assert(a[2].get_view() == "!");
56+
}
57+
std::array<wit::string, 2> to_test::StringListResult() {
58+
return {wit::string::from_view("foo"), wit::string::from_view("bar")};
59+
}
60+
std::array<wit::string, 2> to_test::StringListRoundtrip(std::array<wit::string, 2> a) {
61+
return a;
62+
}
63+
uint32_t to_test::AllocatedBytes() {
64+
return 0;
65+
}

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

Lines changed: 16 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,15 @@ 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[0], "hello");
50+
assert_eq!(a[1], "world");
51+
assert_eq!(a[2], "!");
52+
}
53+
fn string_list_result() -> [String; 2] {
54+
["foo".to_string(), "bar".to_string()]
55+
}
56+
fn string_list_roundtrip(a: [String; 2]) -> [String; 2] {
57+
a
58+
}
4359
}

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

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

2222
nightmare-on-cpp: func(a: list<nested, 2>) -> list<nested, 2>;
23+
24+
/// Functions that test deallocation of heap-allocated elements
25+
/// within fixed-length lists (e.g. strings require deallocation).
26+
string-list-param: func(a: list<string, 3>);
27+
string-list-result: func() -> list<string, 2>;
28+
string-list-roundtrip: func(a: list<string, 2>) -> list<string, 2>;
29+
30+
allocated-bytes: func() -> u32;
2331
}
2432

2533
world test {

0 commit comments

Comments
 (0)