chore: improve documentation, tests

explodingcamera · explodingcamera · commit 525208ec8e86 · 2024-01-10T01:11:55.000+01:00
Signed-off-by: Henry Gressmann &lt;mail@henrygressmann.de&gt;
diff --git a/.cargo/config.toml b/.cargo/config.toml
@@ -2,6 +2,6 @@
 version-dev="workspaces version --no-git-commit --force tinywasm*"
 dev="run -- -l debug run"
 
-test-mvp="test --package tinywasm --test test-mvp"
-test-wast="test --package tinywasm --test test-wast --"
-generate-charts="test --package tinywasm --test generate-charts"
+test-mvp="test --package tinywasm --test test-mvp --release -- --enable "
+test-wast="test --package tinywasm --test test-wast -- --enable "
+generate-charts="test --package tinywasm --test generate-charts -- --enable "
diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml
@@ -20,10 +20,10 @@ jobs:
           rustup default stable
 
       - name: Build (stable)
-        run: cargo +stable build --verbose
+        run: cargo +stable build --all
 
       - name: Run tests (stable)
-        run: cargo +stable test --verbose
+        run: cargo +stable test --all
 
   test-no-std:
     name: Test without default features on nightly Rust
@@ -37,7 +37,7 @@ jobs:
           rustup default nightly
 
       - name: Build (nightly, no default features)
-        run: cargo +nightly build --verbose --no-default-features
+        run: cargo +nightly build --all --no-default-features
 
       - name: Run tests (nightly, no default features)
-        run: cargo +nightly test --verbose --no-default-features
+        run: cargo +nightly test --all --no-default-features
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml
@@ -19,7 +19,7 @@ argh="0.1"
 color-eyre={version="0.6", default-features=false}
 log="0.4"
 pretty_env_logger="0.5"
-wast={version="69.0", optional=true}
+wast={version="70.0", optional=true}
 
 [features]
 default=["wat"]
diff --git a/crates/parser/Cargo.toml b/crates/parser/Cargo.toml
@@ -9,7 +9,6 @@ repository.workspace=true
 
 [dependencies]
 # fork of wasmparser with no_std support, see https://github.com/bytecodealliance/wasmtime/issues/3495
-# TODO: create dependency free parser
 wasmparser={version="0.100", package="wasmparser-nostd", default-features=false}
 log={version="0.4", optional=true}
 tinywasm-types={version="0.2.0-alpha.0", path="../types"}
diff --git a/crates/parser/src/lib.rs b/crates/parser/src/lib.rs
@@ -10,6 +10,7 @@ extern crate alloc;
 #[allow(clippy::single_component_path_imports)]
 use log;
 
+// noop fallback if logging is disabled.
 #[cfg(not(feature = "logging"))]
 mod log {
     macro_rules! debug    ( ($($tt:tt)*) => {{}} );
diff --git a/crates/tinywasm/Cargo.toml b/crates/tinywasm/Cargo.toml
@@ -19,7 +19,7 @@ tinywasm-types={version="0.2.0-alpha.0", path="../types", default-features=false
 
 [dev-dependencies]
 wasm-testsuite={path="../wasm-testsuite"}
-wast={version="69.0"}
+wast={version="70.0"}
 owo-colors={version="4.0"}
 eyre={version="0.6"}
 serde_json={version="1.0"}
diff --git a/crates/tinywasm/src/func.rs b/crates/tinywasm/src/func.rs
@@ -19,7 +19,7 @@ pub struct FuncHandle {
 }
 
 impl FuncHandle {
-    /// Call a function
+    /// Call a function (Invocation)
     ///
     /// See <https://webassembly.github.io/spec/core/exec/modules.html#invocation>
     pub fn call(&self, store: &mut Store, params: &[WasmValue]) -> Result<Vec<WasmValue>> {
@@ -66,8 +66,14 @@ impl FuncHandle {
 
         // Once the function returns:
         let result_m = func_ty.results.len();
+
+        // 1. Assert: m values are on the top of the stack (Ensured by validation)
+        debug_assert!(stack.values.len() >= result_m);
+
+        // 2. Pop m values from the stack
         let res = stack.values.last_n(result_m)?;
 
+        // The values are returned as the results of the invocation.
         Ok(res
             .iter()
             .zip(func_ty.results.iter())
diff --git a/crates/tinywasm/src/instance.rs b/crates/tinywasm/src/instance.rs
@@ -42,16 +42,30 @@ impl ModuleInstance {
     }
 
     /// Instantiate the module in the given store
+    ///
+    /// See <https://webassembly.github.io/spec/core/exec/modules.html#exec-instantiation>
     pub fn instantiate(store: &mut Store, module: Module, imports: Option<Imports>) -> Result<Self> {
+        // This doesn't completely follow the steps in the spec, but the end result is the same
+        // Constant expressions are evaluated directly where they are used, so we
+        // don't need to create a auxiliary frame etc.
+
         let idx = store.next_module_instance_idx();
         let imports = imports.unwrap_or_default();
+
+        // TODO: doesn't link other modules yet
         let linked_imports = imports.link(store, &module)?;
+        let global_addrs = store.add_globals(module.data.globals.into(), &module.data.imports, &linked_imports, idx)?;
 
+        // TODO: imported functions missing
         let func_addrs = store.add_funcs(module.data.funcs.into(), idx);
+
         let table_addrs = store.add_tables(module.data.table_types.into(), idx);
         let mem_addrs = store.add_mems(module.data.memory_types.into(), idx)?;
-        let global_addrs = store.add_globals(module.data.globals.into(), &module.data.imports, &linked_imports, idx)?;
+
+        // TODO: active/declared elems need to be initialized
         let elem_addrs = store.add_elems(module.data.elements.into(), idx)?;
+
+        // TODO: active data segments need to be initialized
         let data_addrs = store.add_datas(module.data.data.into(), idx);
 
         let instance = ModuleInstanceInner {
@@ -70,8 +84,10 @@ impl ModuleInstance {
             imports: module.data.imports,
             exports: crate::ExportInstance(module.data.exports),
         };
+
         let instance = ModuleInstance::new(instance);
         store.add_instance(instance.clone())?;
+
         Ok(instance)
     }
 
@@ -176,13 +192,18 @@ impl ModuleInstance {
             }
         };
 
-        let func_addr = self.0.func_addrs[func_index as usize];
-        let func = store.get_func(func_addr as usize)?;
+        let func_addr = self
+            .0
+            .func_addrs
+            .get(func_index as usize)
+            .expect("No func addr for start func, this is a bug");
+
+        let func = store.get_func(*func_addr as usize)?;
         let ty = self.0.types[func.ty_addr() as usize].clone();
 
         Ok(Some(FuncHandle {
             module: self.clone(),
-            addr: func_addr,
+            addr: *func_addr,
             ty,
             name: None,
         }))
diff --git a/crates/tinywasm/src/lib.rs b/crates/tinywasm/src/lib.rs
@@ -43,7 +43,7 @@
 //! // Get a typed handle to the exported "add" function
 //! // Alternatively, you can use `instance.get_func` to get an untyped handle
 //! // that takes and returns WasmValue types
-//! let func = instance.get_typed_func::<(i32, i32), (i32,)>(&mut store, "add")?;
+//! let func = instance.typed_func::<(i32, i32), (i32,)>(&mut store, "add")?;
 //! let res = func.call(&mut store, (1, 2))?;
 //!
 //! assert_eq!(res, (3,));
@@ -60,7 +60,7 @@
 //! a custom allocator. This removes support for parsing from files and streams,
 //! but otherwise the API is the same.
 //!
-//! Additionally, if you want proper error types, you must use a `nightly` compiler.
+//! Additionally, if you want proper error types, you must use a `nightly` compiler to have the error trait in core.
 
 mod std;
 extern crate alloc;
@@ -70,6 +70,7 @@ extern crate alloc;
 #[allow(clippy::single_component_path_imports)]
 use log;
 
+// noop fallback if logging is disabled.
 #[cfg(not(feature = "logging"))]
 pub(crate) mod log {
     macro_rules! debug    ( ($($tt:tt)*) => {{}} );
diff --git a/crates/tinywasm/src/module.rs b/crates/tinywasm/src/module.rs
@@ -55,9 +55,7 @@ impl Module {
     /// See <https://webassembly.github.io/spec/core/exec/modules.html#exec-instantiation>
     pub fn instantiate(self, store: &mut Store, imports: Option<Imports>) -> Result<ModuleInstance> {
         let instance = ModuleInstance::instantiate(store, self, imports)?;
-
-        // TODO: this currently panics if theres no start fn
-        // let _ = instance.start(store)?;
+        let _ = instance.start(store)?;
         Ok(instance)
     }
 }
diff --git a/crates/tinywasm/src/store.rs b/crates/tinywasm/src/store.rs
@@ -227,19 +227,28 @@ impl Store {
         let elem_count = self.data.elems.len();
         let mut elem_addrs = Vec::with_capacity(elem_count);
         for (i, elem) in elems.into_iter().enumerate() {
-            self.data.elems.push(ElemInstance::new(elem.kind, idx));
-            elem_addrs.push((i + elem_count) as Addr);
+            match elem.kind {
+                // doesn't need to be initialized, can be initialized lazily using the `table.init` instruction
+                ElementKind::Passive => {}
+
+                // this one is active, so we need to initialize it (essentially a `table.init` instruction)
+                ElementKind::Active { .. } => {
+                    // a. Let n be the length of the vector elem[i].init
+                    // b. Execute the instruction sequence einstrs
+                    // c. Execute the instruction i32.const 0
+                    // d. Execute the instruction i32.const n
+                    // e. Execute the instruction table.init tableidx i
+                    // f. Execute the instruction elm.drop i
+                }
 
-            // match elem.kind {
-            //     ElementKind::Active { table, offset } => {
-            //         // let table = self.data.tables[table as usize];
+                // this one is not available to the runtime but needs to be initialized to declare references
+                ElementKind::Declared => {
+                    // a. Execute the instruction elm.drop i
+                }
+            }
 
-            //         // let offset = self.eval_const(&offset)?;
-            //         // let offset = offset.raw_value() as usize;
-            //         // let offset = offset + elem_addrs[i] as usize;
-            //         // let offset = offset as Addr;
-            //     }
-            // }
+            self.data.elems.push(ElemInstance::new(elem.kind, idx));
+            elem_addrs.push((i + elem_count) as Addr);
         }
 
         Ok(elem_addrs)
@@ -252,6 +261,20 @@ impl Store {
         for (i, data) in datas.into_iter().enumerate() {
             self.data.datas.push(DataInstance::new(data.data.to_vec(), idx));
             data_addrs.push((i + data_count) as Addr);
+
+            use tinywasm_types::DataKind::*;
+            match data.kind {
+                Active { .. } => {
+                    // a. Assert: memidx == 0
+                    // b. Let n be the length of the vector
+                    // c. Execute the instruction sequence
+                    // d. Execute the instruction
+                    // e. Execute the instruction
+                    // f. Execute the instruction
+                    // g. Execute the instruction
+                }
+                Passive => {}
+            }
         }
         data_addrs
     }
diff --git a/crates/tinywasm/tests/generate-charts.rs b/crates/tinywasm/tests/generate-charts.rs
@@ -6,11 +6,18 @@ fn main() -> Result<()> {
 }
 
 fn generate_charts() -> Result<()> {
+    let args = std::env::args().collect::<Vec<_>>();
+    if args.len() < 2 || args[1] != "--enable" {
+        return Ok(());
+    }
+
     // Create a line chart
     charts::create_progress_chart(
         std::path::Path::new("./tests/generated/mvp.csv"),
         std::path::Path::new("./tests/generated/progress-mvp.svg"),
     )?;
 
+    println!("created progress chart: ./tests/generated/progress-mvp.svg");
+
     Ok(())
 }
diff --git a/crates/tinywasm/tests/generated/mvp.csv b/crates/tinywasm/tests/generated/mvp.csv
diff --git a/crates/tinywasm/tests/generated/progress-mvp.svg b/crates/tinywasm/tests/generated/progress-mvp.svg
diff --git a/crates/tinywasm/tests/test-mvp.rs b/crates/tinywasm/tests/test-mvp.rs
diff --git a/crates/tinywasm/tests/test-wast.rs b/crates/tinywasm/tests/test-wast.rs
diff --git a/crates/tinywasm/tests/testsuite/run.rs b/crates/tinywasm/tests/testsuite/run.rs
