Merge pull request #142 from Peefy/fix-wasm-c-str-mem-alloc

fix: wasm c str memory alloc on the end \0 char
kcl-lang · Sep 10, 2024 · ff53e25 · ff53e25
2 parents cb0cf3b + 008b6cb
commit ff53e25
Show file tree

Hide file tree

Showing 7 changed files with 56 additions and 33 deletions.
diff --git a/.github/workflows/wasm-test.yaml b/.github/workflows/wasm-test.yaml
@@ -57,6 +57,12 @@ jobs:
 
       - name: Go example e2e tests
         run: cd examples/go && go mod tidy && go run main.go
+
+      - name: Node.js example e2e tests
+        run: cd examples/node && npm install && npm run build
+
+      - name: Browser example e2e tests
+        run: cd examples/browser && npm install && npm run build
 
       - name: Publish Dry Run
         if: "startsWith(github.ref, 'refs/tags/') && contains(github.ref, '-')"

diff --git a/wasm/examples/browser/package.json b/wasm/examples/browser/package.json
@@ -15,7 +15,8 @@
   ],
   "dependencies": {
     "@kcl-lang/wasm-lib": "file:../..",
-    "@wasmer/wasi": "^1.2.2"
+    "@wasmer/wasi": "^1.2.2",
+    "buffer": "^6.0.3"
   },
   "devDependencies": {
     "copy-webpack-plugin": "^8.1.1",

diff --git a/wasm/examples/go/pkg/module/utils.go b/wasm/examples/go/pkg/module/utils.go
@@ -10,13 +10,16 @@ func copyStringToWasmMemory(
 ) (int32, int32, error) {
 	bytes := []byte(str)
 	length := len(bytes)
-	ptr, err := malloc.Call(store, int32(length))
+	// C str '\0'
+	ptr, err := malloc.Call(store, int32(length)+1)
 	if err != nil {
 		return 0, 0, err
 	}
 	data := memory.UnsafeData(store)
 	idx := ptr.(int32)
 	copy(data[idx:(int(idx)+length)], bytes)
+	// C str '\0'
+	data[int(idx)+length] = 0
 	return idx, int32(length), nil
 }
 

diff --git a/wasm/examples/rust/src/lib.rs b/wasm/examples/rust/src/lib.rs
@@ -65,7 +65,8 @@ impl KCLModule {
         let free = instance.get_typed_func::<(i32, i32), ()>(&mut store, "kcl_free")?;
         let run = instance.get_typed_func::<(i32, i32), i32>(&mut store, "kcl_run")?;
         let fmt = instance.get_typed_func::<i32, i32>(&mut store, "kcl_fmt")?;
-        let runtime_err = instance.get_typed_func::<(i32, i32), i32>(&mut store, "kcl_runtime_err")?;
+        let runtime_err =
+            instance.get_typed_func::<(i32, i32), i32>(&mut store, "kcl_runtime_err")?;
         Ok(KCLModule {
             instance,
             store,
@@ -85,25 +86,35 @@ impl KCLModule {
         let (source_ptr, source_len) =
             copy_string_to_wasm_memory(&mut self.store, &self.malloc, self.memory, &opts.source)?;
         let runtime_err_len = 1024;
-        let (runtime_err_ptr, _) = malloc_bytes_from_wasm_memory(&mut self.store, &self.malloc, runtime_err_len)?;
+        let (runtime_err_ptr, _) =
+            malloc_bytes_from_wasm_memory(&mut self.store, &self.malloc, runtime_err_len)?;
         let result_str = match self.run.call(&mut self.store, (filename_ptr, source_ptr)) {
             Ok(result_ptr) => {
                 let (result_str, result_len) =
                     copy_cstr_from_wasm_memory(&mut self.store, self.memory, result_ptr as usize)?;
                 free_memory(&mut self.store, &self.free, result_ptr, result_len)?;
                 result_str
-            },
+            }
             Err(err) => {
-                self.runtime_err.call(&mut self.store, (runtime_err_ptr, runtime_err_len))?;
-                let (runtime_err_str, runtime_err_len) =
-                    copy_cstr_from_wasm_memory(&mut self.store, self.memory, runtime_err_ptr as usize)?;
-                free_memory(&mut self.store, &self.free, runtime_err_ptr, runtime_err_len)?;
+                self.runtime_err
+                    .call(&mut self.store, (runtime_err_ptr, runtime_err_len))?;
+                let (runtime_err_str, runtime_err_len) = copy_cstr_from_wasm_memory(
+                    &mut self.store,
+                    self.memory,
+                    runtime_err_ptr as usize,
+                )?;
+                free_memory(
+                    &mut self.store,
+                    &self.free,
+                    runtime_err_ptr,
+                    runtime_err_len,
+                )?;
                 if runtime_err_str.is_empty() {
-                    return Err(err)
+                    return Err(err);
                 } else {
                     runtime_err_str
                 }
-            },
+            }
         };
         free_memory(&mut self.store, &self.free, filename_ptr, filename_len)?;
         free_memory(&mut self.store, &self.free, source_ptr, source_len)?;
@@ -133,21 +144,25 @@ fn copy_string_to_wasm_memory<T>(
     let bytes = string.as_bytes();
     let length = bytes.len();
 
-    let ptr = malloc.call(&mut *store, length as i32)?;
+    // C str '\0'
+    let ptr = malloc.call(&mut *store, length as i32 + 1)?;
 
     let data = memory.data_mut(&mut *store);
     data[ptr as usize..(ptr as usize + length as usize)].copy_from_slice(bytes);
+    // C str '\0'
+    data[ptr as usize + length] = 0;
 
-    Ok((ptr, length as usize))
+    Ok((ptr, length as usize + 1))
 }
 
 fn malloc_bytes_from_wasm_memory<T>(
     store: &mut Store<T>,
     malloc: &TypedFunc<i32, i32>,
     length: i32,
 ) -> Result<(i32, usize)> {
-    let ptr = malloc.call(&mut *store, length)?;
-    Ok((ptr, length as usize))
+    // C str '\0'
+    let ptr = malloc.call(&mut *store, length + 1)?;
+    Ok((ptr, length as usize + 1))
 }
 
 fn copy_cstr_from_wasm_memory<T>(

diff --git a/wasm/examples/rust/src/tests.rs b/wasm/examples/rust/src/tests.rs
@@ -2,7 +2,6 @@ use crate::{FmtOptions, KCLModule, RunOptions};
 use anyhow::Result;
 
 const WASM_PATH: &str = "../../kcl.wasm";
-const BENCH_COUNT: usize = 20;
 const SOURCES: &[&str] = &[
     r#"apiVersion = "apps/v1"
 kind = "Deployment"
@@ -205,23 +204,25 @@ fn test_run() -> Result<()> {
         source: "a = 1".to_string(),
     };
     let mut module = KCLModule::from_path(WASM_PATH)?;
-    for _ in 0..BENCH_COUNT {
-        let result = module.run(&opts)?;
-        println!("{}", result);
-    }
+    let result = module.run(&opts)?;
+    println!("{}", result);
     Ok(())
 }
 
 #[test]
 fn test_run_examples() -> Result<()> {
+    // Singleton WASM instance
+    let mut module = KCLModule::from_path(WASM_PATH)?;
     for source in SOURCES {
         let opts = RunOptions {
             filename: "test.k".to_string(),
             source: source.to_string(),
         };
-        let mut module = KCLModule::from_path(WASM_PATH)?;
         let result = module.run(&opts)?;
-        assert!(!result.starts_with("ERROR:"), "source: {source}. result: {result}");
+        assert!(
+            !result.starts_with("ERROR:"),
+            "source: {source}. result: {result}"
+        );
     }
     Ok(())
 }
@@ -268,9 +269,7 @@ fn test_fmt() -> Result<()> {
         source: "a = 1".to_string(),
     };
     let mut module = KCLModule::from_path(WASM_PATH)?;
-    for _ in 0..BENCH_COUNT {
-        let result = module.fmt(&opts)?;
-        println!("{}", result);
-    }
+    let result = module.fmt(&opts)?;
+    println!("{}", result);
     Ok(())
 }
diff --git a/wasm/src/index.ts b/wasm/src/index.ts
@@ -180,7 +180,7 @@ function copyStringToWasmMemory(
   const buffer = new Uint8Array(
     exports.memory.buffer,
     pointer,
-    encodedString.length
+    encodedString.length + 1
   );
   buffer.set(encodedString);
   buffer[encodedString.length] = 0; // Null-terminate the string

diff --git a/wasm/tests/run.test.ts b/wasm/tests/run.test.ts
@@ -271,13 +271,12 @@ nginx = Nginx {
 ];
 
 test("run example tests", async () => {
+  const inst = await load();
   SNIPPETS.forEach((snippet) => {
-    load().then((inst) => {
-      const result = invokeKCLRun(inst, {
-        filename: "test.k",
-        source: snippet.value,
-      });
-      expect(result).not.toContain("ERROR:");
+    const result = invokeKCLRun(inst, {
+      filename: "test.k",
+      source: snippet.value,
     });
+    expect(result).not.toContain("ERROR:");
   });
 });