From 2306999c9cca1cd3f57de6eb43a174d74984b7ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Thu, 8 Oct 2020 15:38:11 +0200 Subject: [PATCH 1/3] Fix using Write::write without checking the return value Use write_all instead which garantues that the whole buffer was written. --- cli/build/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/build/main.rs b/cli/build/main.rs index 9ed2d90..612553e 100644 --- a/cli/build/main.rs +++ b/cli/build/main.rs @@ -230,7 +230,7 @@ mod tests { let wasm_path = target_path.join("example_wasm.wasm"); let mut f = fs::File::create(wasm_path).expect("create fail failed"); - f.write(b"\0asm").expect("write file failed"); + f.write_all(b"\0asm").expect("write file failed"); } let path = tmp_dir.path().to_string_lossy(); From 5e3b06de0582c71ce2fdf5a07e56c256feed4d12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Tue, 13 Oct 2020 09:39:57 +0200 Subject: [PATCH 2/3] Fix Instruction::CallIndirect stack height metering The stack height metering for functions containing CallIndirect was wrong. The code did not take into consideration that is pops one value from the stack. The effect was that the stack height of functions using this instruction was higher than its real height. --- src/stack_height/max_height.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/stack_height/max_height.rs b/src/stack_height/max_height.rs index 2f7cde1..d74928d 100644 --- a/src/stack_height/max_height.rs +++ b/src/stack_height/max_height.rs @@ -288,6 +288,9 @@ pub(crate) fn compute(func_idx: u32, module: &elements::Module) -> Result Date: Tue, 13 Oct 2020 10:42:36 +0200 Subject: [PATCH 3/3] Don't generate duplicate thunks Previously, functions that appear in multiple places (exported, start function, table) would generate a thunk for each place they appear in. Those additional thunks are identical and only only one of them would be referenced. Main offender are tables with redundant entries. This commit eliminates those duplicate thunks without adding any additional overhead. --- src/stack_height/thunk.rs | 20 +++-------------- tests/expectations/stack-height/start.wat | 20 ++--------------- tests/expectations/stack-height/table.wat | 26 ++++------------------- 3 files changed, 9 insertions(+), 57 deletions(-) diff --git a/src/stack_height/thunk.rs b/src/stack_height/thunk.rs index e3edb30..2bad3fe 100644 --- a/src/stack_height/thunk.rs +++ b/src/stack_height/thunk.rs @@ -13,7 +13,6 @@ struct Thunk { signature: FunctionType, // Index in function space of this thunk. idx: Option, - original_func_idx: u32, callee_stack_cost: u32, } @@ -23,9 +22,6 @@ pub(crate) fn generate_thunks( ) -> Result { // First, we need to collect all function indices that should be replaced by thunks - // Function indices which needs to generate thunks. - let mut need_thunks: Vec = Vec::new(); - let mut replacement_map: Map = { let exports = module .export_section() @@ -57,12 +53,10 @@ pub(crate) fn generate_thunks( // Don't generate a thunk if stack_cost of a callee is zero. if callee_stack_cost != 0 { - need_thunks.push(func_idx); replacement_map.insert(func_idx, Thunk { signature: resolve_func_type(func_idx, &module)?.clone(), idx: None, callee_stack_cost, - original_func_idx: func_idx, }); } } @@ -76,17 +70,9 @@ pub(crate) fn generate_thunks( let mut next_func_idx = module.functions_space() as u32; let mut mbuilder = builder::from_module(module); - for func_idx in need_thunks { - let mut thunk = replacement_map - .get_mut(&func_idx) - .expect( - "`func_idx` should come from `need_thunks`; - `need_thunks` is populated with the same items that in `replacement_map`; - qed" - ); - + for (func_idx, thunk) in replacement_map.iter_mut() { let instrumented_call = instrument_call!( - thunk.original_func_idx as u32, + *func_idx, thunk.callee_stack_cost as i32, ctx.stack_height_global_idx(), ctx.stack_limit() @@ -113,7 +99,7 @@ pub(crate) fn generate_thunks( // Signature of the thunk should match the original function signature. .signature() .with_params(thunk.signature.params().to_vec()) - .with_return_type(thunk.signature.return_type().clone()) + .with_return_type(thunk.signature.return_type()) .build() .body() .with_instructions(elements::Instructions::new( diff --git a/tests/expectations/stack-height/start.wat b/tests/expectations/stack-height/start.wat index aa38a2e..7a3e905 100644 --- a/tests/expectations/stack-height/start.wat +++ b/tests/expectations/stack-height/start.wat @@ -22,23 +22,7 @@ i32.const 1 i32.sub global.set 0) - (func (;4;) (type 1) - global.get 0 - i32.const 1 - i32.add - global.set 0 - global.get 0 - i32.const 1024 - i32.gt_u - if ;; label = @1 - unreachable - end - call 1 - global.get 0 - i32.const 1 - i32.sub - global.set 0) (global (;0;) (mut i32) (i32.const 0)) - (export "exported_start" (func 4)) + (export "exported_start" (func 3)) (export "call" (func 2)) - (start 4)) + (start 3)) diff --git a/tests/expectations/stack-height/table.wat b/tests/expectations/stack-height/table.wat index 710debc..3b05d18 100644 --- a/tests/expectations/stack-height/table.wat +++ b/tests/expectations/stack-height/table.wat @@ -26,25 +26,7 @@ local.get 0 local.get 1 i32.add) - (func (;3;) (type 2) (param i32 i32) (result i32) - local.get 0 - local.get 1 - global.get 0 - i32.const 2 - i32.add - global.set 0 - global.get 0 - i32.const 1024 - i32.gt_u - if ;; label = @1 - unreachable - end - call 2 - global.get 0 - i32.const 2 - i32.sub - global.set 0) - (func (;4;) (type 1) (param i32) + (func (;3;) (type 1) (param i32) local.get 0 global.get 0 i32.const 2 @@ -61,7 +43,7 @@ i32.const 2 i32.sub global.set 0) - (func (;5;) (type 2) (param i32 i32) (result i32) + (func (;4;) (type 2) (param i32 i32) (result i32) local.get 0 local.get 1 global.get 0 @@ -81,5 +63,5 @@ global.set 0) (table (;0;) 10 funcref) (global (;0;) (mut i32) (i32.const 0)) - (export "i32.add" (func 5)) - (elem (;0;) (i32.const 0) func 0 4 5)) + (export "i32.add" (func 4)) + (elem (;0;) (i32.const 0) func 0 3 4))