MoonBit fixes (#1553)

* Fix use-after-free issue in moonbit

* Make the moonbit test runner work with the latest compiler

* Make existing tests pass including fixed-length list

* fmt

Author: vigoo

crates/moonbit/src/lib.rs

@@ -2369,6 +2369,22 @@ impl Bindgen for FunctionBindgen<'_, '_> {
             }
 
             Instruction::Return { amt, .. } => {
+                // Bind return operands to locals BEFORE cleanup to avoid
+                // use-after-free when operands contain inline loads from
+                // return_area or other freed memory.
+                let return_locals: Vec<String> = if *amt > 0 {
+                    operands
+                        .iter()
+                        .map(|op| {
+                            let local = self.locals.tmp("ret");
+                            uwriteln!(self.src, "let {local} = {op}");
+                            local
+                        })
+                        .collect()
+                } else {
+                    Vec::new()
+                };
+
                 for clean in &self.cleanup {
                     let address = &clean.address;
                     self.r#gen.ffi_imports.insert(ffi::FREE);
@@ -2377,19 +2393,14 @@ impl Bindgen for FunctionBindgen<'_, '_> {
 
                 if self.needs_cleanup_list {
                     self.r#gen.ffi_imports.insert(ffi::FREE);
-                    uwrite!(
-                        self.src,
-                        "
-                        cleanup_list.each(mbt_ffi_free)
-                        ",
-                    );
+                    uwriteln!(self.src, "cleanup_list.each(mbt_ffi_free)",);
                 }
 
                 match *amt {
                     0 => (),
-                    1 => uwriteln!(self.src, "return {}", operands[0]),
+                    1 => uwriteln!(self.src, "return {}", return_locals[0]),
                     _ => {
-                        let results = operands.join(", ");
+                        let results = return_locals.join(", ");
                         uwriteln!(self.src, "return ({results})");
                     }
                 }
@@ -2708,10 +2719,94 @@ impl Bindgen for FunctionBindgen<'_, '_> {
             Instruction::ErrorContextLower { .. }
             | Instruction::ErrorContextLift { .. }
             | Instruction::DropHandle { .. } => todo!(),
-            Instruction::FixedLengthListLift { .. } => todo!(),
-            Instruction::FixedLengthListLower { .. } => todo!(),
-            Instruction::FixedLengthListLowerToMemory { .. } => todo!(),
-            Instruction::FixedLengthListLiftFromMemory { .. } => todo!(),
+            Instruction::FixedLengthListLift {
+                element: _,
+                size,
+                id: _,
+            } => {
+                let array = self.locals.tmp("array");
+                let mut elements = String::new();
+                for a in operands.drain(0..(*size as usize)) {
+                    elements.push_str(&a);
+                    elements.push_str(", ");
+                }
+                uwriteln!(self.src, "let {array} : FixedArray[_] = [{elements}]");
+                results.push(array);
+            }
+            Instruction::FixedLengthListLower {
+                element: _,
+                size,
+                id: _,
+            } => {
+                for i in 0..(*size as usize) {
+                    results.push(format!("({})[{i}]", operands[0]));
+                }
+            }
+            Instruction::FixedLengthListLowerToMemory {
+                element,
+                size: _,
+                id: _,
+            } => {
+                let Block {
+                    body,
+                    results: block_results,
+                } = self.blocks.pop().unwrap();
+                assert!(block_results.is_empty());
+
+                let vec = operands[0].clone();
+                let target = operands[1].clone();
+                let size = self.r#gen.r#gen.sizes.size(element).size_wasm32();
+                let index = self.locals.tmp("index");
+
+                uwrite!(
+                    self.src,
+                    "
+                    for {index} = 0; {index} < ({vec}).length(); {index} = {index} + 1 {{
+                        let iter_elem = ({vec})[{index}]
+                        let iter_base = ({target}) + ({index} * {size})
+                        {body}
+                    }}
+                    ",
+                );
+            }
+            Instruction::FixedLengthListLiftFromMemory {
+                element,
+                size: fll_size,
+                id: _,
+            } => {
+                let Block {
+                    body,
+                    results: block_results,
+                } = self.blocks.pop().unwrap();
+                let address = &operands[0];
+                let array = self.locals.tmp("array");
+                let ty = self
+                    .r#gen
+                    .r#gen
+                    .pkg_resolver
+                    .type_name(self.r#gen.name, element);
+                let elem_size = self.r#gen.r#gen.sizes.size(element).size_wasm32();
+                let index = self.locals.tmp("index");
+
+                let result = match &block_results[..] {
+                    [result] => result,
+                    _ => todo!("result count == {}", block_results.len()),
+                };
+
+                uwrite!(
+                    self.src,
+                    "
+                    let {array} : Array[{ty}] = []
+                    for {index} = 0; {index} < {fll_size}; {index} = {index} + 1 {{
+                        let iter_base = ({address}) + ({index} * {elem_size})
+                        {body}
+                        {array}.push({result})
+                    }}
+                    ",
+                );
+
+                results.push(format!("FixedArray::from_array({array}[:])"));
+            }
         }
     }
 
@@ -2747,11 +2842,10 @@ impl Bindgen for FunctionBindgen<'_, '_> {
 
         if !self.cleanup.is_empty() {
             self.needs_cleanup_list = true;
-            self.r#gen.ffi_imports.insert(ffi::FREE);
 
             for cleanup in &self.cleanup {
                 let address = &cleanup.address;
-                uwriteln!(self.src, "mbt_ffi_free({address})",);
+                uwriteln!(self.src, "cleanup_list.push({address})",);
             }
         }
 

crates/moonbit/src/pkg.rs

@@ -203,6 +203,9 @@ impl PkgResolver {
                         }
                         _ => format!("Array[{}]", self.type_name(this, &ty)),
                     },
+                    TypeDefKind::FixedLengthList(ty, _size) => {
+                        format!("FixedArray[{}]", self.type_name(this, &ty))
+                    }
                     TypeDefKind::Tuple(tuple) => {
                         format!(
                             "({})",

crates/test/src/moonbit.rs

@@ -60,11 +60,12 @@ impl LanguageMethods for MoonBit {
         }
 
         // Compile the MoonBit bindings to a wasm file
+        let manifest = compile.bindings_dir.join("moon.mod.json");
         let mut cmd = Command::new("moon");
         cmd.arg("build")
             .arg("--no-strip") // for debugging
-            .arg("-C")
-            .arg(compile.bindings_dir);
+            .arg("--manifest-path")
+            .arg(&manifest);
         runner.run_command(&mut cmd)?;
         // Build the component
         let artifact = compile
@@ -93,27 +94,28 @@ impl LanguageMethods for MoonBit {
 
     fn should_fail_verify(
         &self,
-        _name: &str,
+        name: &str,
         config: &crate::config::WitConfig,
         _args: &[String],
     ) -> bool {
-        config.async_
+        // async-resource-func actually works, but most other async tests
+        // fail during codegen or verification
+        config.async_ && name != "async-resource-func.wit"
     }
 
     fn verify(&self, runner: &Runner, verify: &crate::Verify) -> anyhow::Result<()> {
+        let manifest = verify.bindings_dir.join("moon.mod.json");
         let mut cmd = Command::new("moon");
         cmd.arg("check")
             .arg("--warn-list")
             .arg("-28")
             // .arg("--deny-warn")
-            .arg("--source-dir")
-            .arg(verify.bindings_dir);
+            .arg("--manifest-path")
+            .arg(&manifest);
 
         runner.run_command(&mut cmd)?;
         let mut cmd = Command::new("moon");
-        cmd.arg("build")
-            .arg("--source-dir")
-            .arg(verify.bindings_dir);
+        cmd.arg("build").arg("--manifest-path").arg(&manifest);
 
         runner.run_command(&mut cmd)?;
         Ok(())

tests/runtime/list-in-variant/runner.mbt

@@ -0,0 +1,36 @@
+//@ [lang]
+//@ path = 'gen/world/runner/stub.mbt'
+//@ pkg_config = """{ "import": ["test/list-in-variant/interface/test_/list_in_variant/toTest"] }"""
+
+///|
+pub fn run() -> Unit {
+  // list-in-option
+  let r1 = @toTest.list_in_option(Some(["hello", "world"]))
+  guard r1 == "hello,world"
+  let r2 = @toTest.list_in_option(None)
+  guard r2 == "none"
+
+  // list-in-variant
+  let r3 = @toTest.list_in_variant(@toTest.PayloadOrEmpty::WithData(["foo", "bar", "baz"]))
+  guard r3 == "foo,bar,baz"
+  let r4 = @toTest.list_in_variant(@toTest.PayloadOrEmpty::Empty)
+  guard r4 == "empty"
+
+  // list-in-result
+  let r5 = @toTest.list_in_result(Ok(["a", "b", "c"]))
+  guard r5 == "a,b,c"
+  let r6 = @toTest.list_in_result(Err("oops"))
+  guard r6 == "err:oops"
+
+  // list-in-option-with-return (Bug 1 + Bug 2)
+  let s1 = @toTest.list_in_option_with_return(Some(["hello", "world"]))
+  guard s1.count == 2U
+  guard s1.label == "hello,world"
+  let s2 = @toTest.list_in_option_with_return(None)
+  guard s2.count == 0U
+  guard s2.label == "none"
+
+  // top-level-list (contrast)
+  let r7 = @toTest.top_level_list(["x", "y", "z"])
+  guard r7 == "x,y,z"
+}

tests/runtime/list-in-variant/runner.rs

@@ -0,0 +1,39 @@
+include!(env!("BINDINGS"));
+
+use crate::test::list_in_variant::to_test::*;
+
+struct Component;
+
+export!(Component);
+
+impl Guest for Component {
+    fn run() {
+        // list-in-option (Bug 1: list freed inside match arm before FFI call)
+        let hw: Vec<String> = ["hello", "world"].into_iter().map(Into::into).collect();
+        assert_eq!(list_in_option(Some(&hw)), "hello,world");
+        assert_eq!(list_in_option(None), "none");
+
+        // list-in-variant (Bug 1: same pattern with variant)
+        let fbb = PayloadOrEmpty::WithData(vec!["foo".into(), "bar".into(), "baz".into()]);
+        assert_eq!(list_in_variant(&fbb), "foo,bar,baz");
+        assert_eq!(list_in_variant(&PayloadOrEmpty::Empty), "empty");
+
+        // list-in-result (Bug 1: same pattern with result)
+        let abc: Vec<String> = ["a", "b", "c"].into_iter().map(Into::into).collect();
+        assert_eq!(list_in_result(Ok(&abc)), "a,b,c");
+        assert_eq!(list_in_result(Err("oops")), "err:oops");
+
+        // list-in-option-with-return (Bug 1 + Bug 2: freed list + return_area read-after-free)
+        let hw2: Vec<String> = ["hello", "world"].into_iter().map(Into::into).collect();
+        let s = list_in_option_with_return(Some(&hw2));
+        assert_eq!(s.count, 2);
+        assert_eq!(s.label, "hello,world");
+        let s = list_in_option_with_return(None);
+        assert_eq!(s.count, 0);
+        assert_eq!(s.label, "none");
+
+        // top-level-list (NOT affected — contrast case)
+        let xyz: Vec<String> = ["x", "y", "z"].into_iter().map(Into::into).collect();
+        assert_eq!(top_level_list(&xyz), "x,y,z");
+    }
+}

tests/runtime/list-in-variant/test.rs

@@ -0,0 +1,47 @@
+include!(env!("BINDINGS"));
+
+use crate::exports::test::list_in_variant::to_test::*;
+
+struct Component;
+
+export!(Component);
+
+impl exports::test::list_in_variant::to_test::Guest for Component {
+    fn list_in_option(data: Option<Vec<String>>) -> String {
+        match data {
+            Some(list) => list.join(","),
+            None => "none".to_string(),
+        }
+    }
+
+    fn list_in_variant(data: PayloadOrEmpty) -> String {
+        match data {
+            PayloadOrEmpty::WithData(list) => list.join(","),
+            PayloadOrEmpty::Empty => "empty".to_string(),
+        }
+    }
+
+    fn list_in_result(data: Result<Vec<String>, String>) -> String {
+        match data {
+            Ok(list) => list.join(","),
+            Err(e) => format!("err:{}", e),
+        }
+    }
+
+    fn list_in_option_with_return(data: Option<Vec<String>>) -> Summary {
+        match data {
+            Some(list) => Summary {
+                count: list.len() as u32,
+                label: list.join(","),
+            },
+            None => Summary {
+                count: 0,
+                label: "none".to_string(),
+            },
+        }
+    }
+
+    fn top_level_list(items: Vec<String>) -> String {
+        items.join(",")
+    }
+}

tests/runtime/list-in-variant/test.wit

@@ -0,0 +1,31 @@
+package test:list-in-variant;
+
+interface to-test {
+  list-in-option: func(data: option<list<string>>) -> string;
+
+  variant payload-or-empty {
+    empty,
+    with-data(list<string>),
+  }
+  list-in-variant: func(data: payload-or-empty) -> string;
+
+  list-in-result: func(data: result<list<string>, string>) -> string;
+
+  record summary {
+    count: u32,
+    label: string,
+  }
+  list-in-option-with-return: func(data: option<list<string>>) -> summary;
+
+  top-level-list: func(items: list<string>) -> string;
+}
+
+world test {
+  export to-test;
+}
+
+world runner {
+  import to-test;
+
+  export run: func();
+}