chore: optimize block stack, remove EndFunc (#12)

Signed-off-by: Henry Gressmann <[email protected]>

Author: explodingcamera

.github/workflows/test.yaml

@@ -47,7 +47,7 @@ jobs:
         run: cargo +stable build --workspace
 
       - name: Run tests (stable)
-        run: cargo +stable test --workspace
+        run: cargo +stable test --workspace && cargo +stable run --example wasm-rust all
 
       - name: Run MVP testsuite
         run: cargo +stable test-mvp
@@ -75,7 +75,7 @@ jobs:
         run: cargo +nightly build --workspace --no-default-features
 
       - name: Run tests (nightly, no default features)
-        run: cargo +nightly test --workspace --no-default-features
+        run: cargo +nightly test --workspace --no-default-features && cargo +nightly run --example wasm-rust all
 
       - name: Run MVP testsuite (nightly)
         run: cargo +nightly test-mvp

CHANGELOG.md

@@ -9,16 +9,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- `Imports` and `Module` are now cloneable (#9)
+
 ### Changed
 
-- Improved documentation and added more tests
-- Tests can now be run on more targets
-- Nightly version has been updated to fix broken builds in some cases
-- Enhance support for scripted language bindings by making Imports and Module cloneable
-- Add `aarch64-apple-darwin` and `armv7-unknown-linux-gnueabihf` targets to CI
+- Improved documentation and added more tests (735c7cb636edfd4704460c94a9c7d65e5bf4df48)
+- Tests can now be run on more targets (#11)
+- Nightly version has been updated to fix broken builds in some cases (#12)
+- Add `aarch64-apple-darwin` and `armv7-unknown-linux-gnueabihf` targets to CI (#12)
 
 ### Removed
 
+- Removed the `EndFunc` instruction, as it was already covered by the `Return` instruction\
+  This also fixes a weird bug that only occurred on certain nightly versions of Rust
+
 ## [0.5.0] - 2024-03-01
 
 **All Commits**: https://github.com/explodingcamera/tinywasm/compare/v0.4.0...v0.5.0

crates/cli/src/bin.rs

@@ -4,7 +4,7 @@ use argh::FromArgs;
 use args::WasmArg;
 use color_eyre::eyre::Result;
 use log::{debug, info};
-use tinywasm::{self, types::WasmValue, Module};
+use tinywasm::{types::WasmValue, Module};
 
 use crate::args::to_wasm_args;
 mod args;

crates/parser/src/visit.rs

@@ -392,7 +392,7 @@ impl<'a> wasmparser::VisitOperator<'a> for FunctionBuilder {
 
     fn visit_end(&mut self) -> Self::Output {
         let Some(label_pointer) = self.label_ptrs.pop() else {
-            return self.visit(Instruction::EndFunc);
+            return self.visit(Instruction::Return);
         };
 
         let current_instr_ptr = self.instructions.len();

crates/tinywasm/Cargo.toml

@@ -13,7 +13,7 @@ name="tinywasm"
 path="src/lib.rs"
 
 [dependencies]
-log={version="0.4", optional=true}
+_log={version="0.4", optional=true, package="log"}
 tinywasm-parser={version="0.5.0", path="../parser", default-features=false, optional=true}
 tinywasm-types={version="0.5.0", path="../types", default-features=false}
 libm={version="0.2", default-features=false}
@@ -29,7 +29,7 @@ pretty_env_logger="0.5"
 
 [features]
 default=["std", "parser", "logging", "archive"]
-logging=["log", "tinywasm-parser?/logging", "tinywasm-types/logging"]
+logging=["_log", "tinywasm-parser?/logging", "tinywasm-types/logging"]
 std=["tinywasm-parser?/std", "tinywasm-types/std"]
 parser=["tinywasm-parser"]
 unsafe=["tinywasm-types/unsafe"]

crates/tinywasm/src/imports.rs

@@ -191,6 +191,7 @@ impl From<&Import> for ExternName {
 ///
 /// ## Example
 /// ```rust
+/// # use _log as log;
 /// # fn main() -> tinywasm::Result<()> {
 /// use tinywasm::{Imports, Extern};
 /// use tinywasm::types::{ValType, TableType, MemoryType, WasmValue};

crates/tinywasm/src/lib.rs

@@ -79,7 +79,7 @@ extern crate alloc;
 // log for logging (optional).
 #[cfg(feature = "logging")]
 #[allow(clippy::single_component_path_imports)]
-use log;
+use _log as log;
 
 // noop fallback if logging is disabled.
 #[cfg(not(feature = "logging"))]

crates/tinywasm/src/runtime/interpreter/mod.rs

@@ -80,7 +80,7 @@ enum ExecResult {
 fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &ModuleInstance) -> Result<ExecResult> {
     let instrs = &cf.func_instance.0.instructions;
 
-    if unlikely(cf.instr_ptr >= instrs.len() || instrs.is_empty()) {
+    if unlikely(cf.instr_ptr as usize >= instrs.len() || instrs.is_empty()) {
         log::error!("instr_ptr out of bounds: {} >= {}", cf.instr_ptr, instrs.len());
         return Err(Error::Other(format!("instr_ptr out of bounds: {} >= {}", cf.instr_ptr, instrs.len())));
     }
@@ -128,7 +128,7 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M
             };
 
             let params = stack.values.pop_n_rev(wasm_func.ty.params.len())?;
-            let call_frame = CallFrame::new(wasm_func, func_inst.owner, params, stack.blocks.len());
+            let call_frame = CallFrame::new(wasm_func, func_inst.owner, params, stack.blocks.len() as u32);
 
             // push the call frame
             cf.instr_ptr += 1; // skip the call instruction
@@ -181,7 +181,7 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M
             }
 
             let params = stack.values.pop_n_rev(wasm_func.ty.params.len())?;
-            let call_frame = CallFrame::new(wasm_func, func_inst.owner, params, stack.blocks.len());
+            let call_frame = CallFrame::new(wasm_func, func_inst.owner, params, stack.blocks.len() as u32);
 
             // push the call frame
             cf.instr_ptr += 1; // skip the call instruction
@@ -198,8 +198,8 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M
                 cf.enter_block(
                     BlockFrame::new(
                         cf.instr_ptr,
-                        cf.instr_ptr + *end_offset as usize,
-                        stack.values.len(),
+                        cf.instr_ptr + *end_offset,
+                        stack.values.len() as u32,
                         BlockType::If,
                         &args.unpack(),
                         module,
@@ -213,26 +213,26 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M
             // falsy value is on the top of the stack
             if *else_offset != 0 {
                 let label = BlockFrame::new(
-                    cf.instr_ptr + *else_offset as usize,
-                    cf.instr_ptr + *end_offset as usize,
-                    stack.values.len(),
+                    cf.instr_ptr + *else_offset,
+                    cf.instr_ptr + *end_offset,
+                    stack.values.len() as u32,
                     BlockType::Else,
                     &args.unpack(),
                     module,
                 );
-                cf.instr_ptr += *else_offset as usize;
+                cf.instr_ptr += *else_offset;
                 cf.enter_block(label, &mut stack.values, &mut stack.blocks);
             } else {
-                cf.instr_ptr += *end_offset as usize;
+                cf.instr_ptr += *end_offset;
             }
         }
 
         Loop(args, end_offset) => {
             cf.enter_block(
                 BlockFrame::new(
                     cf.instr_ptr,
-                    cf.instr_ptr + *end_offset as usize,
-                    stack.values.len(),
+                    cf.instr_ptr + *end_offset,
+                    stack.values.len() as u32,
                     BlockType::Loop,
                     args,
                     module,
@@ -246,8 +246,8 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M
             cf.enter_block(
                 BlockFrame::new(
                     cf.instr_ptr,
-                    cf.instr_ptr + *end_offset as usize,
-                    stack.values.len(),
+                    cf.instr_ptr + *end_offset,
+                    stack.values.len() as u32,
                     BlockType::Block,
                     args,
                     module,
@@ -258,7 +258,9 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M
         }
 
         BrTable(default, len) => {
-            let instr = cf.instructions()[cf.instr_ptr + 1..cf.instr_ptr + 1 + *len as usize]
+            let start = cf.instr_ptr + 1;
+            let end = cf.instr_ptr + 1 + *len;
+            let instr = cf.instructions()[start as usize..end as usize]
                 .iter()
                 .map(|i| match i {
                     BrLabel(l) => Ok(*l),
@@ -294,24 +296,13 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M
             false => return Ok(ExecResult::Call),
         },
 
-        EndFunc => {
-            if unlikely(stack.blocks.len() != cf.block_ptr) {
-                panic!("endfunc: block frames not empty, this should have been validated by the parser");
-            }
-
-            match stack.call_stack.is_empty() {
-                true => return Ok(ExecResult::Return),
-                false => return Ok(ExecResult::Call),
-            }
-        }
-
         // We're essentially using else as a EndBlockFrame instruction for if blocks
         Else(end_offset) => {
             let block =
                 stack.blocks.pop().expect("else: no label to end, this should have been validated by the parser");
 
-            stack.values.truncate_keep(block.stack_ptr, block.results);
-            cf.instr_ptr += *end_offset as usize;
+            stack.values.truncate_keep(block.stack_ptr, block.results as u32);
+            cf.instr_ptr += *end_offset;
         }
 
         // remove the label from the label stack
@@ -321,7 +312,7 @@ fn exec_one(cf: &mut CallFrame, stack: &mut Stack, store: &mut Store, module: &M
                 .pop()
                 .expect("end blockframe: no label to end, this should have been validated by the parser");
 
-            stack.values.truncate_keep(block.stack_ptr, block.results);
+            stack.values.truncate_keep(block.stack_ptr, block.results as u32);
         }
 
         LocalGet(local_index) => stack.values.push(cf.get_local(*local_index as usize)),

crates/tinywasm/src/runtime/stack/block_stack.rs

@@ -18,15 +18,15 @@ impl BlockStack {
 
     #[inline]
     /// get the label at the given index, where 0 is the top of the stack
-    pub(crate) fn get_relative_to(&self, index: usize, offset: usize) -> Option<&BlockFrame> {
-        let len = self.0.len() - offset;
+    pub(crate) fn get_relative_to(&self, index: u32, offset: u32) -> Option<&BlockFrame> {
+        let len = (self.0.len() as u32) - offset;
 
         // the vast majority of wasm functions don't use break to return
         if unlikely(index >= len) {
             return None;
         }
 
-        Some(&self.0[self.0.len() - index - 1])
+        Some(&self.0[self.0.len() - index as usize - 1])
     }
 
     #[inline]
@@ -36,31 +36,32 @@ impl BlockStack {
 
     /// keep the top `len` blocks and discard the rest
     #[inline]
-    pub(crate) fn truncate(&mut self, len: usize) {
-        self.0.truncate(len);
+    pub(crate) fn truncate(&mut self, len: u32) {
+        self.0.truncate(len as usize);
     }
 }
 
 #[derive(Debug, Clone)]
 pub(crate) struct BlockFrame {
     // position of the instruction pointer when the block was entered
-    pub(crate) instr_ptr: usize,
+    pub(crate) instr_ptr: u32,
     // position of the end instruction of the block
-    pub(crate) end_instr_ptr: usize,
+    pub(crate) end_instr_ptr: u32,
 
     // position of the stack pointer when the block was entered
-    pub(crate) stack_ptr: usize,
-    pub(crate) results: usize,
-    pub(crate) params: usize,
+    pub(crate) stack_ptr: u32,
+
+    pub(crate) results: u8,
+    pub(crate) params: u8,
     pub(crate) ty: BlockType,
 }
 
 impl BlockFrame {
     #[inline]
     pub(crate) fn new(
-        instr_ptr: usize,
-        end_instr_ptr: usize,
-        stack_ptr: usize,
+        instr_ptr: u32,
+        end_instr_ptr: u32,
+        stack_ptr: u32,
         ty: BlockType,
         args: &BlockArgs,
         module: &ModuleInstance,
@@ -70,7 +71,7 @@ impl BlockFrame {
             BlockArgs::Type(_) => (0, 1),
             BlockArgs::FuncType(t) => {
                 let ty = module.func_ty(*t);
-                (ty.params.len(), ty.results.len())
+                (ty.params.len() as u8, ty.results.len() as u8)
             }
         };
 

crates/tinywasm/src/runtime/stack/call_stack.rs

@@ -48,8 +48,8 @@ impl CallStack {
 
 #[derive(Debug, Clone)]
 pub(crate) struct CallFrame {
-    pub(crate) instr_ptr: usize,
-    pub(crate) block_ptr: usize,
+    pub(crate) instr_ptr: u32,
+    pub(crate) block_ptr: u32,
     pub(crate) func_instance: (Rc<WasmFunction>, ModuleInstanceAddr),
     pub(crate) locals: Box<[RawWasmValue]>,
 }
@@ -63,7 +63,9 @@ impl CallFrame {
         blocks: &mut super::BlockStack,
     ) {
         if block_frame.params > 0 {
-            values.extend_from_within((block_frame.stack_ptr - block_frame.params)..block_frame.stack_ptr);
+            let start = (block_frame.stack_ptr - block_frame.params as u32) as usize;
+            let end = block_frame.stack_ptr as usize;
+            values.extend_from_within(start..end);
         }
 
         blocks.push(block_frame);
@@ -77,7 +79,7 @@ impl CallFrame {
         values: &mut super::ValueStack,
         blocks: &mut super::BlockStack,
     ) -> Option<()> {
-        let break_to = blocks.get_relative_to(break_to_relative as usize, self.block_ptr)?;
+        let break_to = blocks.get_relative_to(break_to_relative, self.block_ptr)?;
 
         // instr_ptr points to the label instruction, but the next step
         // will increment it by 1 since we're changing the "current" instr_ptr
@@ -92,7 +94,7 @@ impl CallFrame {
                 // check if we're breaking to the loop
                 if break_to_relative != 0 {
                     // we also want to trim the label stack to the loop (but not including the loop)
-                    blocks.truncate(blocks.len() - break_to_relative as usize);
+                    blocks.truncate(blocks.len() as u32 - break_to_relative);
                     return Some(());
                 }
             }
@@ -106,7 +108,7 @@ impl CallFrame {
                 self.instr_ptr = break_to.end_instr_ptr;
 
                 // we also want to trim the label stack, including the block
-                blocks.truncate(blocks.len() - (break_to_relative as usize + 1));
+                blocks.truncate(blocks.len() as u32 - (break_to_relative + 1));
             }
         }
 
@@ -118,8 +120,8 @@ impl CallFrame {
     pub(crate) fn new(
         wasm_func_inst: Rc<WasmFunction>,
         owner: ModuleInstanceAddr,
-        params: impl Iterator<Item = RawWasmValue> + ExactSizeIterator,
-        block_ptr: usize,
+        params: impl ExactSizeIterator<Item = RawWasmValue>,
+        block_ptr: u32,
     ) -> Self {
         let locals = {
             let local_types = &wasm_func_inst.locals;
@@ -150,6 +152,6 @@ impl CallFrame {
 
     #[inline(always)]
     pub(crate) fn current_instruction(&self) -> &Instruction {
-        &self.func_instance.0.instructions[self.instr_ptr]
+        &self.func_instance.0.instructions[self.instr_ptr as usize]
     }
 }