From 17933c5fb6c297969ec425bf9b2758e62bee30c6 Mon Sep 17 00:00:00 2001 From: Yi LIU Date: Thu, 12 Feb 2026 20:04:15 +0800 Subject: [PATCH] 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, 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`). 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). --- crates/core/src/abi.rs | 13 +++++- crates/rust/tests/codegen.rs | 38 ++++++++++++++++++ tests/runtime/fixed-length-lists/alloc.rs | 30 ++++++++++++++ tests/runtime/fixed-length-lists/runner.rs | 46 ++++++++++++++++++++++ tests/runtime/fixed-length-lists/test.cpp | 14 +++++++ tests/runtime/fixed-length-lists/test.rs | 16 ++++++++ tests/runtime/fixed-length-lists/test.wit | 8 ++++ 7 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 tests/runtime/fixed-length-lists/alloc.rs diff --git a/crates/core/src/abi.rs b/crates/core/src/abi.rs index 7d625f222..a4fd33daf 100644 --- a/crates/core/src/abi.rs +++ b/crates/core/src/abi.rs @@ -2404,7 +2404,13 @@ impl<'a, B: Bindgen> Generator<'a, B> { TypeDefKind::Resource => unreachable!(), TypeDefKind::Unknown => unreachable!(), - TypeDefKind::FixedLengthList(..) => todo!(), + TypeDefKind::FixedLengthList(element, size) => { + self.flat_for_each_record_type( + ty, + iter::repeat_n(element, *size as usize), + |me, ty| me.deallocate(ty, what), + ); + } TypeDefKind::Map(..) => todo!(), }, } @@ -2526,7 +2532,10 @@ impl<'a, B: Bindgen> Generator<'a, B> { TypeDefKind::Future(_) => unreachable!(), TypeDefKind::Stream(_) => unreachable!(), TypeDefKind::Unknown => unreachable!(), - TypeDefKind::FixedLengthList(_, _) => {} + TypeDefKind::FixedLengthList(element, size) => { + let tys: Vec = iter::repeat_n(*element, *size as usize).collect(); + self.deallocate_indirect_fields(&tys, addr, offset, what); + } TypeDefKind::Map(..) => todo!(), }, } diff --git a/crates/rust/tests/codegen.rs b/crates/rust/tests/codegen.rs index 44ddf31d8..a34133757 100644 --- a/crates/rust/tests/codegen.rs +++ b/crates/rust/tests/codegen.rs @@ -142,6 +142,44 @@ mod newtyped_list { } } } + +#[test] +fn fixed_length_list_deallocation() { + let wit = r#" + package test:fll-dealloc; + + world test { + export return-string-array: func() -> list; + export accept-string-array: func(a: list); + } + "#; + + let mut resolve = wit_bindgen_core::wit_parser::Resolve::default(); + let pkg = resolve.push_str("test.wit", wit).unwrap(); + let world = resolve.select_world(&[pkg], None).unwrap(); + + let opts = wit_bindgen_rust::Opts { + generate_all: true, + ..Default::default() + }; + + let mut generator = opts.build(); + let mut files = wit_bindgen_core::Files::default(); + use wit_bindgen_core::WorldGenerator; + generator.generate(&mut resolve, world, &mut files).unwrap(); + + let (_name, contents) = files.iter().next().unwrap(); + let code = String::from_utf8_lossy(contents); + + // Verify post-return deallocation functions are generated for + // fixed-length lists containing heap-allocated elements (strings) + assert!( + code.contains("cabi_post_"), + "expected cabi_post_ deallocation function for fixed-length list \ + with string elements, but none was generated" + ); +} + #[allow(unused, reason = "testing codegen, not functionality")] mod retyped_list { use std::ops::Deref; diff --git a/tests/runtime/fixed-length-lists/alloc.rs b/tests/runtime/fixed-length-lists/alloc.rs new file mode 100644 index 000000000..0bb2559da --- /dev/null +++ b/tests/runtime/fixed-length-lists/alloc.rs @@ -0,0 +1,30 @@ +use std::alloc::{GlobalAlloc, Layout, System}; +use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; + +#[global_allocator] +static ALLOC: A = A; + +static ALLOC_AMT: AtomicUsize = AtomicUsize::new(0); + +struct A; + +unsafe impl GlobalAlloc for A { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + let ptr = System.alloc(layout); + if !ptr.is_null() { + ALLOC_AMT.fetch_add(layout.size(), SeqCst); + } + return ptr; + } + + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + // Poison all deallocations to try to catch any use-after-free in the + // bindings as early as possible. + std::ptr::write_bytes(ptr, 0xde, layout.size()); + ALLOC_AMT.fetch_sub(layout.size(), SeqCst); + System.dealloc(ptr, layout) + } +} +pub fn get() -> usize { + ALLOC_AMT.load(SeqCst) +} diff --git a/tests/runtime/fixed-length-lists/runner.rs b/tests/runtime/fixed-length-lists/runner.rs index 3e59cef12..eb6face0b 100644 --- a/tests/runtime/fixed-length-lists/runner.rs +++ b/tests/runtime/fixed-length-lists/runner.rs @@ -4,6 +4,29 @@ include!(env!("BINDINGS")); use test::fixed_length_lists::to_test::*; +mod alloc; + +struct Guard { + me_before: usize, + remote_before: u32, +} + +impl Guard { + fn new() -> Guard { + Guard { + me_before: alloc::get(), + remote_before: allocated_bytes(), + } + } +} + +impl Drop for Guard { + fn drop(&mut self) { + assert_eq!(self.me_before, alloc::get()); + assert_eq!(self.remote_before, allocated_bytes()); + } +} + struct Component; export!(Component); @@ -70,5 +93,28 @@ impl Guest for Component { assert_eq!(result[0].l, [1, -1]); assert_eq!(result[1].l, [2, -2]); } + + // Test deallocation of heap-allocated elements in fixed-length lists. + // Strings require heap allocation, so passing and returning them through + // fixed-length lists exercises the deallocate/deallocate_indirect paths. + { + let _guard = Guard::new(); + string_list_param([ + "hello".to_string(), + "world".to_string(), + "!".to_string(), + ]); + } + { + let _guard = Guard::new(); + let result = string_list_result(); + assert_eq!(result, ["foo", "bar"]); + } + { + let _guard = Guard::new(); + let result = + string_list_roundtrip(["hello".to_string(), "world".to_string()]); + assert_eq!(result, ["hello", "world"]); + } } } diff --git a/tests/runtime/fixed-length-lists/test.cpp b/tests/runtime/fixed-length-lists/test.cpp index 4c9bb515e..96a1712df 100644 --- a/tests/runtime/fixed-length-lists/test.cpp +++ b/tests/runtime/fixed-length-lists/test.cpp @@ -49,3 +49,17 @@ std::array to_test::NightmareOnCpp(std::array a) { return a; } +void to_test::StringListParam(std::array a) { + assert(a[0].get_view() == "hello"); + assert(a[1].get_view() == "world"); + assert(a[2].get_view() == "!"); +} +std::array to_test::StringListResult() { + return {wit::string::from_view("foo"), wit::string::from_view("bar")}; +} +std::array to_test::StringListRoundtrip(std::array a) { + return a; +} +uint32_t to_test::AllocatedBytes() { + return 0; +} diff --git a/tests/runtime/fixed-length-lists/test.rs b/tests/runtime/fixed-length-lists/test.rs index 566ef7d6d..3a319b7e6 100644 --- a/tests/runtime/fixed-length-lists/test.rs +++ b/tests/runtime/fixed-length-lists/test.rs @@ -4,9 +4,14 @@ struct Component; export!(Component); +mod alloc; + use crate::exports::test::fixed_length_lists::to_test::Nested; impl exports::test::fixed_length_lists::to_test::Guest for Component { + fn allocated_bytes() -> u32 { + alloc::get().try_into().unwrap() + } fn list_param(a: [u32; 4]) { assert_eq!(a, [1, 2, 3, 4]); } @@ -40,4 +45,15 @@ impl exports::test::fixed_length_lists::to_test::Guest for Component { fn nightmare_on_cpp(a: [Nested; 2]) -> [Nested; 2] { a } + fn string_list_param(a: [String; 3]) { + assert_eq!(a[0], "hello"); + assert_eq!(a[1], "world"); + assert_eq!(a[2], "!"); + } + fn string_list_result() -> [String; 2] { + ["foo".to_string(), "bar".to_string()] + } + fn string_list_roundtrip(a: [String; 2]) -> [String; 2] { + a + } } diff --git a/tests/runtime/fixed-length-lists/test.wit b/tests/runtime/fixed-length-lists/test.wit index 892d9296f..767c8aefe 100644 --- a/tests/runtime/fixed-length-lists/test.wit +++ b/tests/runtime/fixed-length-lists/test.wit @@ -20,6 +20,14 @@ interface to-test { } nightmare-on-cpp: func(a: list) -> list; + + /// Functions that test deallocation of heap-allocated elements + /// within fixed-length lists (e.g. strings require deallocation). + string-list-param: func(a: list); + string-list-result: func() -> list; + string-list-roundtrip: func(a: list) -> list; + + allocated-bytes: func() -> u32; } world test {