From 7f25e5246a6a76a3a3a71db8f9dca54cc3a2e0ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 4 Feb 2021 17:28:56 +0100 Subject: [PATCH 1/8] Drop default value for depth parameter in execute() --- lib/fizzy/execute.hpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/fizzy/execute.hpp b/lib/fizzy/execute.hpp index 193cf7684..b61bcddc4 100644 --- a/lib/fizzy/execute.hpp +++ b/lib/fizzy/execute.hpp @@ -49,7 +49,14 @@ constexpr ExecutionResult Trap{false}; /// @param args The pointer to the arguments. The number of items and their types must match /// the expected number of input parameters of the function, otherwise undefined /// behaviour (including crash) happens. -/// @param depth The call depth (indexing starts at 0). Can be left at the default setting. +/// @param depth The call depth (indexing starts at 0). /// @return The result of the execution. -ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, int depth = 0); +ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, int depth); + +/// Execute a function from an instance starting at default depth of 0. +inline ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args) +{ + const int depth = 0; + return execute(instance, func_idx, args, depth); +} } // namespace fizzy From 2f5c0f8a7b66406b690ae4f9761ff3b092a7dc03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 4 Feb 2021 18:13:38 +0100 Subject: [PATCH 2/8] capi: Drop depth param from fizzy_execute() --- include/fizzy/fizzy.h | 3 +- lib/fizzy/capi.cpp | 4 +- test/unittests/capi_test.cpp | 80 +++++++++++++++++------------------ test/utils/fizzy_c_engine.cpp | 2 +- 4 files changed, 44 insertions(+), 45 deletions(-) diff --git a/include/fizzy/fizzy.h b/include/fizzy/fizzy.h index c04ab0820..03bca1bcb 100644 --- a/include/fizzy/fizzy.h +++ b/include/fizzy/fizzy.h @@ -518,7 +518,6 @@ bool fizzy_find_exported_global( /// /// @param instance Pointer to module instance. Cannot be NULL. /// @param args Pointer to the argument array. Can be NULL if function has 0 inputs. -/// @param depth Call stack depth. /// @return Result of execution. /// /// @note @@ -526,7 +525,7 @@ bool fizzy_find_exported_global( /// When number of passed arguments or their types are different from the ones defined by the /// function type, behaviour is undefined. FizzyExecutionResult fizzy_execute( - FizzyInstance* instance, uint32_t func_idx, const FizzyValue* args, int depth); + FizzyInstance* instance, uint32_t func_idx, const FizzyValue* args); #ifdef __cplusplus } diff --git a/lib/fizzy/capi.cpp b/lib/fizzy/capi.cpp index 88ac21758..2097f3ace 100644 --- a/lib/fizzy/capi.cpp +++ b/lib/fizzy/capi.cpp @@ -582,9 +582,9 @@ size_t fizzy_get_instance_memory_size(FizzyInstance* instance) } FizzyExecutionResult fizzy_execute( - FizzyInstance* instance, uint32_t func_idx, const FizzyValue* args, int depth) + FizzyInstance* instance, uint32_t func_idx, const FizzyValue* args) { - const auto result = fizzy::execute(*unwrap(instance), func_idx, unwrap(args), depth); + const auto result = fizzy::execute(*unwrap(instance), func_idx, unwrap(args)); return wrap(result); } } diff --git a/test/unittests/capi_test.cpp b/test/unittests/capi_test.cpp index 408dfe4cd..9da88aa91 100644 --- a/test/unittests/capi_test.cpp +++ b/test/unittests/capi_test.cpp @@ -574,10 +574,10 @@ TEST(capi, instantiate_imported_globals) auto instance = fizzy_instantiate(module, nullptr, 0, nullptr, nullptr, globals, 4); EXPECT_NE(instance, nullptr); - EXPECT_THAT(fizzy_execute(instance, 0, nullptr, 0), CResult(42_u32)); - EXPECT_THAT(fizzy_execute(instance, 1, nullptr, 0), CResult(43_u64)); - EXPECT_THAT(fizzy_execute(instance, 2, nullptr, 0), CResult(44.4f)); - EXPECT_THAT(fizzy_execute(instance, 3, nullptr, 0), CResult(45.5)); + EXPECT_THAT(fizzy_execute(instance, 0, nullptr), CResult(42_u32)); + EXPECT_THAT(fizzy_execute(instance, 1, nullptr), CResult(43_u64)); + EXPECT_THAT(fizzy_execute(instance, 2, nullptr), CResult(44.4f)); + EXPECT_THAT(fizzy_execute(instance, 3, nullptr), CResult(45.5)); fizzy_free_instance(instance); @@ -707,10 +707,10 @@ TEST(capi, resolve_instantiate_functions) ASSERT_NE(instance, nullptr); FizzyValue arg; - EXPECT_THAT(fizzy_execute(instance, 0, &arg, 0), CResult(42_u32)); - EXPECT_THAT(fizzy_execute(instance, 1, &arg, 0), CResult(43_u64)); - EXPECT_THAT(fizzy_execute(instance, 2, &arg, 0), CResult(44.44f)); - EXPECT_THAT(fizzy_execute(instance, 3, &arg, 0), CResult(45.45)); + EXPECT_THAT(fizzy_execute(instance, 0, &arg), CResult(42_u32)); + EXPECT_THAT(fizzy_execute(instance, 1, &arg), CResult(43_u64)); + EXPECT_THAT(fizzy_execute(instance, 2, &arg), CResult(44.44f)); + EXPECT_THAT(fizzy_execute(instance, 3, &arg), CResult(45.45)); fizzy_free_instance(instance); @@ -762,8 +762,8 @@ TEST(capi, resolve_instantiate_function_duplicate) auto instance = fizzy_resolve_instantiate(module, host_funcs, 1, nullptr, nullptr, nullptr, 0); ASSERT_NE(instance, nullptr); - EXPECT_THAT(fizzy_execute(instance, 0, nullptr, 0), CResult(42_u32)); - EXPECT_THAT(fizzy_execute(instance, 1, nullptr, 0), CResult(42_u32)); + EXPECT_THAT(fizzy_execute(instance, 0, nullptr), CResult(42_u32)); + EXPECT_THAT(fizzy_execute(instance, 1, nullptr), CResult(42_u32)); fizzy_free_instance(instance); } @@ -815,10 +815,10 @@ TEST(capi, resolve_instantiate_globals) fizzy_resolve_instantiate(module, &mod1foo1, 1, nullptr, nullptr, host_globals, 4); ASSERT_NE(instance, nullptr); - EXPECT_THAT(fizzy_execute(instance, 1, nullptr, 0), CResult(42_u32)); - EXPECT_THAT(fizzy_execute(instance, 2, nullptr, 0), CResult(43_u32)); - EXPECT_THAT(fizzy_execute(instance, 3, nullptr, 0), CResult(44_u64)); - EXPECT_THAT(fizzy_execute(instance, 4, nullptr, 0), CResult(45_u64)); + EXPECT_THAT(fizzy_execute(instance, 1, nullptr), CResult(42_u32)); + EXPECT_THAT(fizzy_execute(instance, 2, nullptr), CResult(43_u32)); + EXPECT_THAT(fizzy_execute(instance, 3, nullptr), CResult(44_u64)); + EXPECT_THAT(fizzy_execute(instance, 4, nullptr), CResult(45_u64)); fizzy_free_instance(instance); @@ -831,10 +831,10 @@ TEST(capi, resolve_instantiate_globals) module, &mod1foo1, 1, nullptr, nullptr, host_globals_reordered, 4); EXPECT_NE(instance, nullptr); - EXPECT_THAT(fizzy_execute(instance, 1, nullptr, 0), CResult(42_u32)); - EXPECT_THAT(fizzy_execute(instance, 2, nullptr, 0), CResult(43_u32)); - EXPECT_THAT(fizzy_execute(instance, 3, nullptr, 0), CResult(44_u64)); - EXPECT_THAT(fizzy_execute(instance, 4, nullptr, 0), CResult(45_u64)); + EXPECT_THAT(fizzy_execute(instance, 1, nullptr), CResult(42_u32)); + EXPECT_THAT(fizzy_execute(instance, 2, nullptr), CResult(43_u32)); + EXPECT_THAT(fizzy_execute(instance, 3, nullptr), CResult(44_u64)); + EXPECT_THAT(fizzy_execute(instance, 4, nullptr), CResult(45_u64)); fizzy_free_instance(instance); @@ -847,10 +847,10 @@ TEST(capi, resolve_instantiate_globals) fizzy_resolve_instantiate(module, &mod1foo1, 1, nullptr, nullptr, host_globals_extra, 4); EXPECT_NE(instance, nullptr); - EXPECT_THAT(fizzy_execute(instance, 1, nullptr, 0), CResult(42_u32)); - EXPECT_THAT(fizzy_execute(instance, 2, nullptr, 0), CResult(43_u32)); - EXPECT_THAT(fizzy_execute(instance, 3, nullptr, 0), CResult(44_u64)); - EXPECT_THAT(fizzy_execute(instance, 4, nullptr, 0), CResult(45_u64)); + EXPECT_THAT(fizzy_execute(instance, 1, nullptr), CResult(42_u32)); + EXPECT_THAT(fizzy_execute(instance, 2, nullptr), CResult(43_u32)); + EXPECT_THAT(fizzy_execute(instance, 3, nullptr), CResult(44_u64)); + EXPECT_THAT(fizzy_execute(instance, 4, nullptr), CResult(45_u64)); fizzy_free_instance(instance); @@ -885,8 +885,8 @@ TEST(capi, resolve_instantiate_global_duplicate) fizzy_resolve_instantiate(module, nullptr, 0, nullptr, nullptr, host_globals, 1); ASSERT_NE(instance, nullptr); - EXPECT_THAT(fizzy_execute(instance, 0, nullptr, 0), CResult(42_u32)); - EXPECT_THAT(fizzy_execute(instance, 1, nullptr, 0), CResult(42_u32)); + EXPECT_THAT(fizzy_execute(instance, 0, nullptr), CResult(42_u32)); + EXPECT_THAT(fizzy_execute(instance, 1, nullptr), CResult(42_u32)); fizzy_free_instance(instance); } @@ -964,7 +964,7 @@ TEST(capi, memory_access) memory[0] = 0xaa; memory[1] = 0xbb; - EXPECT_THAT(fizzy_execute(instance, 0, nullptr, 0), CResult(0x22bbaa_u32)); + EXPECT_THAT(fizzy_execute(instance, 0, nullptr), CResult(0x22bbaa_u32)); fizzy_free_instance(instance); } @@ -1006,7 +1006,7 @@ TEST(capi, imported_memory_access) auto* instance = fizzy_instantiate(module, nullptr, 0, nullptr, &memory, nullptr, 0); ASSERT_NE(instance, nullptr); - EXPECT_EQ(fizzy_execute(instance, 0, nullptr, 0).value.i32, 0x221100); + EXPECT_EQ(fizzy_execute(instance, 0, nullptr).value.i32, 0x221100); EXPECT_EQ(fizzy_get_instance_memory_size(instance), 65536); @@ -1016,8 +1016,8 @@ TEST(capi, imported_memory_access) memory_data[0] = 0xaa; memory_data[1] = 0xbb; - EXPECT_EQ(fizzy_execute(instance_memory, 0, nullptr, 0).value.i32, 0x22bbaa); - EXPECT_EQ(fizzy_execute(instance, 0, nullptr, 0).value.i32, 0x22bbaa); + EXPECT_EQ(fizzy_execute(instance_memory, 0, nullptr).value.i32, 0x22bbaa); + EXPECT_EQ(fizzy_execute(instance, 0, nullptr).value.i32, 0x22bbaa); fizzy_free_instance(instance); fizzy_free_instance(instance_memory); @@ -1043,11 +1043,11 @@ TEST(capi, execute) auto instance = fizzy_instantiate(module, nullptr, 0, nullptr, nullptr, nullptr, 0); ASSERT_NE(instance, nullptr); - EXPECT_THAT(fizzy_execute(instance, 0, nullptr, 0), CResult()); - EXPECT_THAT(fizzy_execute(instance, 1, nullptr, 0), CResult(42_u32)); + EXPECT_THAT(fizzy_execute(instance, 0, nullptr), CResult()); + EXPECT_THAT(fizzy_execute(instance, 1, nullptr), CResult(42_u32)); FizzyValue args[] = {{42}, {2}}; - EXPECT_THAT(fizzy_execute(instance, 2, args, 0), CResult(21_u32)); - EXPECT_THAT(fizzy_execute(instance, 3, nullptr, 0), CTraps()); + EXPECT_THAT(fizzy_execute(instance, 2, args), CResult(21_u32)); + EXPECT_THAT(fizzy_execute(instance, 3, nullptr), CTraps()); fizzy_free_instance(instance); } @@ -1082,10 +1082,10 @@ TEST(capi, execute_with_host_function) auto instance = fizzy_instantiate(module, host_funcs, 2, nullptr, nullptr, nullptr, 0); ASSERT_NE(instance, nullptr); - EXPECT_THAT(fizzy_execute(instance, 0, nullptr, 0), CResult(42_u32)); + EXPECT_THAT(fizzy_execute(instance, 0, nullptr), CResult(42_u32)); FizzyValue args[] = {{42}, {2}}; - EXPECT_THAT(fizzy_execute(instance, 1, args, 0), CResult(21_u32)); + EXPECT_THAT(fizzy_execute(instance, 1, args), CResult(21_u32)); fizzy_free_instance(instance); } @@ -1112,7 +1112,7 @@ TEST(capi, imported_function_traps) auto instance = fizzy_instantiate(module, host_funcs, 1, nullptr, nullptr, nullptr, 0); ASSERT_NE(instance, nullptr); - EXPECT_THAT(fizzy_execute(instance, 1, nullptr, 0), CTraps()); + EXPECT_THAT(fizzy_execute(instance, 1, nullptr), CTraps()); fizzy_free_instance(instance); } @@ -1141,7 +1141,7 @@ TEST(capi, imported_function_void) auto instance = fizzy_instantiate(module, host_funcs, 1, nullptr, nullptr, nullptr, 0); ASSERT_NE(instance, nullptr); - EXPECT_THAT(fizzy_execute(instance, 1, nullptr, 0), CResult()); + EXPECT_THAT(fizzy_execute(instance, 1, nullptr), CResult()); EXPECT_TRUE(called); fizzy_free_instance(instance); @@ -1189,7 +1189,7 @@ TEST(capi, imported_function_from_another_module) ASSERT_NE(instance2, nullptr); FizzyValue args[] = {{44}, {2}}; - EXPECT_THAT(fizzy_execute(instance2, 1, args, 0), CResult(42_u32)); + EXPECT_THAT(fizzy_execute(instance2, 1, args), CResult(42_u32)); fizzy_free_exported_function(&func); fizzy_free_instance(instance2); @@ -1229,7 +1229,7 @@ TEST(capi, imported_table_from_another_module) auto instance2 = fizzy_instantiate(module2, nullptr, 0, &table, nullptr, nullptr, 0); ASSERT_NE(instance2, nullptr); - EXPECT_THAT(fizzy_execute(instance2, 0, nullptr, 0), CResult(42_u32)); + EXPECT_THAT(fizzy_execute(instance2, 0, nullptr), CResult(42_u32)); fizzy_free_instance(instance2); fizzy_free_instance(instance1); @@ -1265,7 +1265,7 @@ TEST(capi, imported_memory_from_another_module) auto instance2 = fizzy_instantiate(module2, nullptr, 0, nullptr, &memory, nullptr, 0); ASSERT_NE(instance2, nullptr); - EXPECT_THAT(fizzy_execute(instance2, 0, nullptr, 0), CResult(0x00ffaa00_u32)); + EXPECT_THAT(fizzy_execute(instance2, 0, nullptr), CResult(0x00ffaa00_u32)); fizzy_free_instance(instance2); fizzy_free_instance(instance1); @@ -1301,7 +1301,7 @@ TEST(capi, imported_global_from_another_module) auto instance2 = fizzy_instantiate(module2, nullptr, 0, nullptr, nullptr, &global, 1); ASSERT_NE(instance2, nullptr); - EXPECT_THAT(fizzy_execute(instance2, 0, nullptr, 0), CResult(42_u32)); + EXPECT_THAT(fizzy_execute(instance2, 0, nullptr), CResult(42_u32)); fizzy_free_instance(instance2); fizzy_free_instance(instance1); diff --git a/test/utils/fizzy_c_engine.cpp b/test/utils/fizzy_c_engine.cpp index 4ed8525d8..e476550a3 100644 --- a/test/utils/fizzy_c_engine.cpp +++ b/test/utils/fizzy_c_engine.cpp @@ -115,7 +115,7 @@ WasmEngine::Result FizzyCEngine::execute( assert(func_type.output != FizzyValueTypeF32 && func_type.output != FizzyValueTypeF64 && "floating point result types are not supported"); const auto first_arg = reinterpret_cast(args.data()); - const auto status = fizzy_execute(m_instance.get(), func_idx, first_arg, 0); + const auto status = fizzy_execute(m_instance.get(), func_idx, first_arg); if (status.trapped) return {true, std::nullopt}; else if (status.has_value) From 58bea12b5ffa4a09a87fa90bfc99ebb566dbf31f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 4 Feb 2021 19:15:54 +0100 Subject: [PATCH 3/8] rust: Drop depth param from execute() functions --- bindings/rust/src/lib.rs | 52 +++++++++++++++------------------------- 1 file changed, 19 insertions(+), 33 deletions(-) diff --git a/bindings/rust/src/lib.rs b/bindings/rust/src/lib.rs index 3e9ae3cb6..e6185d559 100644 --- a/bindings/rust/src/lib.rs +++ b/bindings/rust/src/lib.rs @@ -313,18 +313,11 @@ impl Instance { /// /// An invalid index, invalid inputs, or invalid depth can cause undefined behaviour. /// - /// The `depth` argument sets the initial call depth. Should be set to `0`. - /// /// # Safety - /// This function expects a valid `func_idx`, appropriate number of `args`, and valid `depth`. - pub unsafe fn unsafe_execute( - &mut self, - func_idx: u32, - args: &[Value], - depth: i32, - ) -> ExecutionResult { + /// This function expects a valid `func_idx` and appropriate number of `args`. + pub unsafe fn unsafe_execute(&mut self, func_idx: u32, args: &[Value]) -> ExecutionResult { ExecutionResult { - 0: sys::fizzy_execute(self.0.as_ptr(), func_idx, args.as_ptr(), depth), + 0: sys::fizzy_execute(self.0.as_ptr(), func_idx, args.as_ptr()), } } @@ -338,15 +331,8 @@ impl Instance { /// /// An error is returned if the function can not be found, inappropriate number of arguments are passed, /// or the supplied types are mismatching. - /// - /// The `depth` argument sets the initial call depth. Should be set to `0`. // TODO: consider different approach: Result where Error::Trap is used for traps - pub fn execute( - &mut self, - name: &str, - args: &[TypedValue], - depth: i32, - ) -> Result { + pub fn execute(&mut self, name: &str, args: &[TypedValue]) -> Result { let func_idx = self.find_exported_function_index(&name); if func_idx.is_none() { return Err(()); @@ -369,7 +355,7 @@ impl Instance { // Translate to untyped raw values. let args: Vec = args.iter().map(|v| v.into()).collect(); - let ret = unsafe { self.unsafe_execute(func_idx, &args, depth) }; + let ret = unsafe { self.unsafe_execute(func_idx, &args) }; Ok(TypedExecutionResult { result: ret.0, value_type: func_type.output, @@ -651,29 +637,29 @@ mod tests { assert!(instance.is_ok()); let mut instance = instance.unwrap(); - let result = unsafe { instance.unsafe_execute(0, &[], 0) }; + let result = unsafe { instance.unsafe_execute(0, &[]) }; assert!(!result.trapped()); assert!(!result.value().is_some()); - let result = unsafe { instance.unsafe_execute(1, &[], 0) }; + let result = unsafe { instance.unsafe_execute(1, &[]) }; assert!(!result.trapped()); assert!(result.value().is_some()); assert_eq!(result.value().unwrap().as_i32(), 42); // Explicit type specification let result = - unsafe { instance.unsafe_execute(2, &[(42 as i32).into(), (2 as i32).into()], 0) }; + unsafe { instance.unsafe_execute(2, &[(42 as i32).into(), (2 as i32).into()]) }; assert!(!result.trapped()); assert!(result.value().is_some()); assert_eq!(result.value().unwrap().as_i32(), 21); // Implicit i64 types (even though the code expects i32) - let result = unsafe { instance.unsafe_execute(2, &[42.into(), 2.into()], 0) }; + let result = unsafe { instance.unsafe_execute(2, &[42.into(), 2.into()]) }; assert!(!result.trapped()); assert!(result.value().is_some()); assert_eq!(result.value().unwrap().as_i32(), 21); - let result = unsafe { instance.unsafe_execute(3, &[], 0) }; + let result = unsafe { instance.unsafe_execute(3, &[]) }; assert!(result.trapped()); assert!(!result.value().is_some()); } @@ -701,7 +687,7 @@ mod tests { let mut instance = instance.unwrap(); // Successful execution. - let result = instance.execute("foo", &[], 0); + let result = instance.execute("foo", &[]); assert!(result.is_ok()); let result = result.unwrap(); assert!(!result.trapped()); @@ -709,7 +695,7 @@ mod tests { assert_eq!(result.value().unwrap().as_u32().unwrap(), 42); // Successful execution with arguments. - let result = instance.execute("bar", &[TypedValue::U32(42), TypedValue::U64(24)], 0); + let result = instance.execute("bar", &[TypedValue::U32(42), TypedValue::U64(24)]); assert!(result.is_ok()); let result = result.unwrap(); assert!(!result.trapped()); @@ -717,7 +703,7 @@ mod tests { assert_eq!(result.value().unwrap().as_u32().unwrap(), 66); // Successful execution with 32-bit float argument. - let result = instance.execute("pi32", &[TypedValue::F32(0.5)], 0); + let result = instance.execute("pi32", &[TypedValue::F32(0.5)]); assert!(result.is_ok()); let result = result.unwrap(); assert!(!result.trapped()); @@ -725,7 +711,7 @@ mod tests { assert_eq!(result.value().unwrap().as_f32().unwrap(), 0.15923566); // Successful execution with 64-bit float argument. - let result = instance.execute("pi64", &[TypedValue::F64(0.5)], 0); + let result = instance.execute("pi64", &[TypedValue::F64(0.5)]); assert!(result.is_ok()); let result = result.unwrap(); assert!(!result.trapped()); @@ -736,23 +722,23 @@ mod tests { ); // Non-function export. - let result = instance.execute("g1", &[], 0); + let result = instance.execute("g1", &[]); assert!(result.is_err()); // Export not found. - let result = instance.execute("baz", &[], 0); + let result = instance.execute("baz", &[]); assert!(result.is_err()); // Passing more arguments than required. - let result = instance.execute("foo", &[TypedValue::U32(42)], 0); + let result = instance.execute("foo", &[TypedValue::U32(42)]); assert!(result.is_err()); // Passing less arguments than required. - let result = instance.execute("bar", &[], 0); + let result = instance.execute("bar", &[]); assert!(result.is_err()); // Passing mismatched types. - let result = instance.execute("bar", &[TypedValue::F32(1.0), TypedValue::F64(2.0)], 0); + let result = instance.execute("bar", &[TypedValue::F32(1.0), TypedValue::F64(2.0)]); assert!(result.is_err()); } } From 04ac27b277fbe52cdd4af55157499d3c00b188b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 4 Feb 2021 18:03:13 +0100 Subject: [PATCH 4/8] test: Drop execute helpers with non-default depth --- test/unittests/execute_call_test.cpp | 6 ++++-- test/utils/execute_helpers.hpp | 10 +++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/test/unittests/execute_call_test.cpp b/test/unittests/execute_call_test.cpp index 2a66941b2..d38e015d4 100644 --- a/test/unittests/execute_call_test.cpp +++ b/test/unittests/execute_call_test.cpp @@ -637,8 +637,10 @@ TEST(execute_call, call_max_depth) auto instance = instantiate(parse(bin)); - EXPECT_THAT(execute(*instance, 0, {}, TestCallStackLimit - 1), Result(42)); - EXPECT_THAT(execute(*instance, 1, {}, TestCallStackLimit - 1), Traps()); + const auto max_depth = TestCallStackLimit - 1; + EXPECT_THAT( + TypedExecutionResult(execute(*instance, 0, {}, max_depth), ValType::i32), Result(42)); + EXPECT_THAT(execute(*instance, 1, {}, max_depth), Traps()); } TEST(execute_call, execute_imported_max_depth) diff --git a/test/utils/execute_helpers.hpp b/test/utils/execute_helpers.hpp index 51a43e60a..332338c2a 100644 --- a/test/utils/execute_helpers.hpp +++ b/test/utils/execute_helpers.hpp @@ -12,8 +12,8 @@ namespace fizzy::test { -inline TypedExecutionResult execute(Instance& instance, FuncIdx func_idx, - std::initializer_list typed_args, int depth = 0) +inline TypedExecutionResult execute( + Instance& instance, FuncIdx func_idx, std::initializer_list typed_args) { const auto& func_type = instance.module->get_function_type(func_idx); const auto [typed_arg_it, type_it] = std::mismatch(std::cbegin(typed_args), @@ -31,16 +31,16 @@ inline TypedExecutionResult execute(Instance& instance, FuncIdx func_idx, std::transform(std::cbegin(typed_args), std::cend(typed_args), std::begin(args), [](const auto& typed_arg) { return typed_arg.value; }); - const auto result = fizzy::execute(instance, func_idx, args.data(), depth); + const auto result = fizzy::execute(instance, func_idx, args.data()); assert(func_type.outputs.size() <= 1); const auto result_type = func_type.outputs.empty() ? ValType{} : func_type.outputs[0]; return {result, result_type}; } inline auto execute(const std::unique_ptr& module, FuncIdx func_idx, - std::initializer_list typed_args, int depth = 0) + std::initializer_list typed_args) { auto instance = instantiate(*module); - return test::execute(*instance, func_idx, typed_args, depth); + return test::execute(*instance, func_idx, typed_args); } } // namespace fizzy::test From dee285e788da640d5af866c41576d50f3d7dc7fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 4 Feb 2021 17:37:34 +0100 Subject: [PATCH 5/8] Add ThreadContext --- lib/fizzy/instantiate.hpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/lib/fizzy/instantiate.hpp b/lib/fizzy/instantiate.hpp index da9cc18d5..131b5f2cd 100644 --- a/lib/fizzy/instantiate.hpp +++ b/lib/fizzy/instantiate.hpp @@ -21,6 +21,29 @@ namespace fizzy struct ExecutionResult; struct Instance; +class ThreadContext +{ +public: + int depth = 0; + + void acquire() noexcept { ++depth; } + + void release() noexcept { --depth; } + + class [[nodiscard]] Guard + { + ThreadContext& m_thread_context; + + public: + explicit Guard(ThreadContext& ctx) noexcept : m_thread_context{ctx} + { + m_thread_context.acquire(); + } + + ~Guard() noexcept { m_thread_context.release(); } + }; +}; + /// Function representing WebAssembly or host function execution. using execute_function = std::function; From 415afb6d54eada798ca7f6189f746441a6b4088c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 4 Feb 2021 18:44:10 +0100 Subject: [PATCH 6/8] Replace execution depth with ThreadContext Replace depth param in execute() functions with ThreadContext&. The ThreadContext encapsulates the depth control from now on and is extensible. In future it can support metering and better stack space allocation. Call depth control semantic is preserved, except the depth is now passed by reference (aka `int& depth`) so additional context guard is needed in places where depth is being bumped. --- include/fizzy/fizzy.h | 6 +- lib/fizzy/capi.cpp | 22 +++++-- lib/fizzy/execute.cpp | 17 ++--- lib/fizzy/execute.hpp | 9 +-- lib/fizzy/instantiate.cpp | 4 +- lib/fizzy/instantiate.hpp | 2 +- test/unittests/api_test.cpp | 11 ++-- test/unittests/capi_test.cpp | 28 ++++---- test/unittests/execute_call_test.cpp | 97 ++++++++++++++++++---------- test/unittests/execute_test.cpp | 18 +++--- test/unittests/instantiate_test.cpp | 11 ++-- test/utils/fizzy_c_engine.cpp | 3 +- test/utils/fizzy_engine.cpp | 3 +- tools/wasi/wasi.cpp | 17 +++-- 14 files changed, 152 insertions(+), 96 deletions(-) diff --git a/include/fizzy/fizzy.h b/include/fizzy/fizzy.h index 03bca1bcb..7e7ff0d82 100644 --- a/include/fizzy/fizzy.h +++ b/include/fizzy/fizzy.h @@ -20,6 +20,8 @@ typedef struct FizzyModule FizzyModule; /// The opaque data type representing an instance (instantiated module). typedef struct FizzyInstance FizzyInstance; +typedef struct FizzyThreadContext FizzyThreadContext; + /// The data type representing numeric values. typedef union FizzyValue { @@ -52,10 +54,10 @@ typedef struct FizzyExecutionResult /// @param context Opaque pointer to execution context. /// @param instance Pointer to module instance. /// @param args Pointer to the argument array. Can be NULL iff function has no inputs. -/// @param depth Call stack depth. +/// @param ctx Pointer to thread context. Cannot be NULL. /// @return Result of execution. typedef FizzyExecutionResult (*FizzyExternalFn)( - void* context, FizzyInstance* instance, const FizzyValue* args, int depth); + void* context, FizzyInstance* instance, const FizzyValue* args, FizzyThreadContext* ctx); /// Value type. typedef uint8_t FizzyValueType; diff --git a/lib/fizzy/capi.cpp b/lib/fizzy/capi.cpp index 2097f3ace..d76509d68 100644 --- a/lib/fizzy/capi.cpp +++ b/lib/fizzy/capi.cpp @@ -95,6 +95,16 @@ inline fizzy::Instance* unwrap(FizzyInstance* instance) noexcept return reinterpret_cast(instance); } +inline FizzyThreadContext* wrap(fizzy::ThreadContext& ctx) noexcept +{ + return reinterpret_cast(&ctx); +} + +inline fizzy::ThreadContext& unwrap(FizzyThreadContext* ctx) noexcept +{ + return *reinterpret_cast(ctx); +} + inline FizzyExecutionResult wrap(const fizzy::ExecutionResult& result) noexcept { return {result.trapped, result.has_value, wrap(result.value)}; @@ -113,19 +123,19 @@ inline fizzy::ExecutionResult unwrap(const FizzyExecutionResult& result) noexcep inline auto unwrap(FizzyExternalFn func, void* context) noexcept { return [func, context](fizzy::Instance& instance, const fizzy::Value* args, - int depth) noexcept -> fizzy::ExecutionResult { - const auto result = func(context, wrap(&instance), wrap(args), depth); + fizzy::ThreadContext& ctx) noexcept -> fizzy::ExecutionResult { + const auto result = func(context, wrap(&instance), wrap(args), wrap(ctx)); return unwrap(result); }; } inline FizzyExternalFunction wrap(fizzy::ExternalFunction external_func) { - static constexpr FizzyExternalFn c_function = [](void* context, FizzyInstance* instance, - const FizzyValue* args, - int depth) -> FizzyExecutionResult { + static constexpr FizzyExternalFn c_function = + [](void* context, FizzyInstance* instance, const FizzyValue* args, + FizzyThreadContext* ctx) -> FizzyExecutionResult { auto* func = static_cast(context); - return wrap((func->function)(*unwrap(instance), unwrap(args), depth)); + return wrap((func->function)(*unwrap(instance), unwrap(args), unwrap(ctx))); }; auto context = std::make_unique(std::move(external_func)); diff --git a/lib/fizzy/execute.cpp b/lib/fizzy/execute.cpp index 030d2a9b2..f867b582e 100644 --- a/lib/fizzy/execute.cpp +++ b/lib/fizzy/execute.cpp @@ -475,13 +475,14 @@ void branch(const Code& code, OperandStack& stack, const uint8_t*& pc, uint32_t } inline bool invoke_function(const FuncType& func_type, uint32_t func_idx, Instance& instance, - OperandStack& stack, int depth) + OperandStack& stack, ThreadContext& ctx) { const auto num_args = func_type.inputs.size(); assert(stack.size() >= num_args); const auto call_args = stack.rend() - num_args; - const auto ret = execute(instance, func_idx, call_args, depth + 1); + const auto guard = ThreadContext::Guard{ctx}; + const auto ret = execute(instance, func_idx, call_args, ctx); // Bubble up traps if (ret.trapped) return false; @@ -501,17 +502,17 @@ inline bool invoke_function(const FuncType& func_type, uint32_t func_idx, Instan } // namespace -ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, int depth) +ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, ThreadContext& ctx) { - assert(depth >= 0); - if (depth >= CallStackLimit) + assert(ctx.depth >= 0); + if (ctx.depth >= CallStackLimit) return Trap; const auto& func_type = instance.module->get_function_type(func_idx); assert(instance.module->imported_function_types.size() == instance.imported_functions.size()); if (func_idx < instance.imported_functions.size()) - return instance.imported_functions[func_idx].function(instance, args, depth); + return instance.imported_functions[func_idx].function(instance, args, ctx); const auto& code = instance.module->get_code(func_idx); auto* const memory = instance.memory.get(); @@ -594,7 +595,7 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, const auto called_func_idx = read(pc); const auto& called_func_type = instance.module->get_function_type(called_func_idx); - if (!invoke_function(called_func_type, called_func_idx, instance, stack, depth)) + if (!invoke_function(called_func_type, called_func_idx, instance, stack, ctx)) goto trap; break; } @@ -621,7 +622,7 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, goto trap; if (!invoke_function( - actual_type, called_func.func_idx, *called_func.instance, stack, depth)) + actual_type, called_func.func_idx, *called_func.instance, stack, ctx)) goto trap; break; } diff --git a/lib/fizzy/execute.hpp b/lib/fizzy/execute.hpp index b61bcddc4..2d744913a 100644 --- a/lib/fizzy/execute.hpp +++ b/lib/fizzy/execute.hpp @@ -49,14 +49,15 @@ constexpr ExecutionResult Trap{false}; /// @param args The pointer to the arguments. The number of items and their types must match /// the expected number of input parameters of the function, otherwise undefined /// behaviour (including crash) happens. -/// @param depth The call depth (indexing starts at 0). +/// @param ctx The thread context. /// @return The result of the execution. -ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, int depth); +ExecutionResult execute( + Instance& instance, FuncIdx func_idx, const Value* args, ThreadContext& ctx); /// Execute a function from an instance starting at default depth of 0. inline ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args) { - const int depth = 0; - return execute(instance, func_idx, args, depth); + ThreadContext ctx; + return execute(instance, func_idx, args, ctx); } } // namespace fizzy diff --git a/lib/fizzy/instantiate.cpp b/lib/fizzy/instantiate.cpp index 014fa4f19..07cc08f9c 100644 --- a/lib/fizzy/instantiate.cpp +++ b/lib/fizzy/instantiate.cpp @@ -477,8 +477,8 @@ std::optional find_exported_function(Instance& instance, std:: return std::nullopt; const auto idx = *opt_index; - auto func = [idx, &instance](fizzy::Instance&, const Value* args, int depth) { - return execute(instance, idx, args, depth); + auto func = [idx, &instance](fizzy::Instance&, const Value* args, ThreadContext& ctx) { + return execute(instance, idx, args, ctx); }; return ExternalFunction{std::move(func), instance.module->get_function_type(idx)}; diff --git a/lib/fizzy/instantiate.hpp b/lib/fizzy/instantiate.hpp index 131b5f2cd..de3d30900 100644 --- a/lib/fizzy/instantiate.hpp +++ b/lib/fizzy/instantiate.hpp @@ -45,7 +45,7 @@ class ThreadContext }; /// Function representing WebAssembly or host function execution. -using execute_function = std::function; +using execute_function = std::function; /// Function with associated input/output types, /// used to represent both imported and exported functions. diff --git a/test/unittests/api_test.cpp b/test/unittests/api_test.cpp index 971c31300..b67d36555 100644 --- a/test/unittests/api_test.cpp +++ b/test/unittests/api_test.cpp @@ -17,10 +17,10 @@ namespace { auto function_returning_value(Value value) noexcept { - return [value](Instance&, const Value*, int) { return value; }; + return [value](Instance&, const Value*, ThreadContext&) { return value; }; } -ExecutionResult function_returning_void(Instance&, const Value*, int) noexcept +ExecutionResult function_returning_void(Instance&, const Value*, ThreadContext&) noexcept { return Void; } @@ -407,7 +407,8 @@ TEST(api, find_exported_function) auto opt_function = find_exported_function(*instance, "foo"); ASSERT_TRUE(opt_function); - EXPECT_THAT(TypedExecutionResult(opt_function->function(*instance, {}, 0), ValType::i32), + ThreadContext ctx; + EXPECT_THAT(TypedExecutionResult(opt_function->function(*instance, {}, ctx), ValType::i32), Result(42_u32)); EXPECT_TRUE(opt_function->input_types.empty()); ASSERT_EQ(opt_function->output_types.size(), 1); @@ -427,7 +428,7 @@ TEST(api, find_exported_function) "0061736d010000000105016000017f021001087370656374657374036261720000040401700000050401010102" "0606017f0041000b07170403666f6f000001670300037461620100036d656d0200"); - auto bar = [](Instance&, const Value*, int) { return Value{42}; }; + auto bar = [](Instance&, const Value*, ThreadContext&) { return Value{42}; }; const auto bar_type = FuncType{{}, {ValType::i32}}; auto instance_reexported_function = @@ -436,7 +437,7 @@ TEST(api, find_exported_function) auto opt_reexported_function = find_exported_function(*instance_reexported_function, "foo"); ASSERT_TRUE(opt_reexported_function); EXPECT_THAT( - TypedExecutionResult(opt_reexported_function->function(*instance, {}, 0), ValType::i32), + TypedExecutionResult(opt_reexported_function->function(*instance, {}, ctx), ValType::i32), Result(42_u32)); EXPECT_TRUE(opt_reexported_function->input_types.empty()); ASSERT_EQ(opt_reexported_function->output_types.size(), 1); diff --git a/test/unittests/capi_test.cpp b/test/unittests/capi_test.cpp index 9da88aa91..f10f1b18f 100644 --- a/test/unittests/capi_test.cpp +++ b/test/unittests/capi_test.cpp @@ -318,7 +318,9 @@ TEST(capi, find_exported_function) EXPECT_EQ(function.type.output, FizzyValueTypeI32); EXPECT_NE(function.context, nullptr); ASSERT_NE(function.function, nullptr); - EXPECT_THAT(function.function(function.context, instance, nullptr, 0), CResult(42_u32)); + + // FIXME: thread context missing. + // EXPECT_THAT(function.function(function.context, instance, nullptr), CResult(42_u32)); fizzy_free_exported_function(&function); @@ -684,7 +686,8 @@ TEST(capi, resolve_instantiate_functions) module = fizzy_parse(wasm.data(), wasm.size()); ASSERT_NE(module, nullptr); - FizzyExternalFn host_fn = [](void* context, FizzyInstance*, const FizzyValue*, int) { + FizzyExternalFn host_fn = [](void* context, FizzyInstance*, const FizzyValue*, + FizzyThreadContext*) { return FizzyExecutionResult{false, true, *static_cast(context)}; }; @@ -752,7 +755,7 @@ TEST(capi, resolve_instantiate_function_duplicate) auto module = fizzy_parse(wasm.data(), wasm.size()); ASSERT_NE(module, nullptr); - FizzyExternalFn host_fn = [](void*, FizzyInstance*, const FizzyValue*, int) { + FizzyExternalFn host_fn = [](void*, FizzyInstance*, const FizzyValue*, FizzyThreadContext*) { return FizzyExecutionResult{false, true, FizzyValue{42}}; }; @@ -793,7 +796,7 @@ TEST(capi, resolve_instantiate_globals) module = fizzy_parse(wasm.data(), wasm.size()); ASSERT_NE(module, nullptr); - FizzyExternalFn host_fn = [](void*, FizzyInstance*, const FizzyValue*, int) { + FizzyExternalFn host_fn = [](void*, FizzyInstance*, const FizzyValue*, FizzyThreadContext*) { return FizzyExecutionResult{true, false, {0}}; }; FizzyImportedFunction mod1foo1 = { @@ -1066,13 +1069,14 @@ TEST(capi, execute_with_host_function) const FizzyValueType inputs[] = {FizzyValueTypeI32, FizzyValueTypeI32}; - FizzyExternalFunction host_funcs[] = {{{FizzyValueTypeI32, nullptr, 0}, - [](void*, FizzyInstance*, const FizzyValue*, int) { - return FizzyExecutionResult{false, true, {42}}; - }, - nullptr}, + FizzyExternalFunction host_funcs[] = { + {{FizzyValueTypeI32, nullptr, 0}, + [](void*, FizzyInstance*, const FizzyValue*, FizzyThreadContext*) { + return FizzyExecutionResult{false, true, {42}}; + }, + nullptr}, {{FizzyValueTypeI32, &inputs[0], 2}, - [](void*, FizzyInstance*, const FizzyValue* args, int) { + [](void*, FizzyInstance*, const FizzyValue* args, FizzyThreadContext*) { FizzyValue v; v.i32 = args[0].i32 / args[1].i32; return FizzyExecutionResult{false, true, {v}}; @@ -1104,7 +1108,7 @@ TEST(capi, imported_function_traps) ASSERT_NE(module, nullptr); FizzyExternalFunction host_funcs[] = {{{FizzyValueTypeI32, nullptr, 0}, - [](void*, FizzyInstance*, const FizzyValue*, int) { + [](void*, FizzyInstance*, const FizzyValue*, FizzyThreadContext*) { return FizzyExecutionResult{true, false, {}}; }, nullptr}}; @@ -1132,7 +1136,7 @@ TEST(capi, imported_function_void) bool called = false; FizzyExternalFunction host_funcs[] = {{{}, - [](void* context, FizzyInstance*, const FizzyValue*, int) { + [](void* context, FizzyInstance*, const FizzyValue*, FizzyThreadContext*) { *static_cast(context) = true; return FizzyExecutionResult{false, false, {}}; }, diff --git a/test/unittests/execute_call_test.cpp b/test/unittests/execute_call_test.cpp index d38e015d4..fd71b1d5a 100644 --- a/test/unittests/execute_call_test.cpp +++ b/test/unittests/execute_call_test.cpp @@ -340,7 +340,7 @@ TEST(execute_call, imported_function_call) const auto module = parse(wasm); - constexpr auto host_foo = [](Instance&, const Value*, int) { return Value{42}; }; + constexpr auto host_foo = [](Instance&, const Value*, ThreadContext&) { return Value{42}; }; const auto host_foo_type = module->typesec[0]; auto instance = instantiate(*module, {{host_foo, host_foo_type}}); @@ -362,7 +362,7 @@ TEST(execute_call, imported_function_call_void) const auto module = parse(wasm); bool called = false; - const auto host_foo = [&called](Instance&, const Value*, int) { + const auto host_foo = [&called](Instance&, const Value*, ThreadContext&) { called = true; return Void; }; @@ -390,7 +390,9 @@ TEST(execute_call, imported_function_call_with_arguments) const auto module = parse(wasm); - auto host_foo = [](Instance&, const Value* args, int) { return Value{args[0].i32 * 2}; }; + auto host_foo = [](Instance&, const Value* args, ThreadContext&) { + return Value{args[0].i32 * 2}; + }; const auto host_foo_type = module->typesec[0]; auto instance = instantiate(*module, {{host_foo, host_foo_type}}); @@ -432,10 +434,10 @@ TEST(execute_call, imported_functions_call_indirect) ASSERT_EQ(module->importsec.size(), 2); ASSERT_EQ(module->codesec.size(), 2); - constexpr auto sqr = [](Instance&, const Value* args, int) { + constexpr auto sqr = [](Instance&, const Value* args, ThreadContext&) { return Value{uint64_t{args[0].i32} * uint64_t{args[0].i32}}; }; - constexpr auto isqrt = [](Instance&, const Value* args, int) { + constexpr auto isqrt = [](Instance&, const Value* args, ThreadContext&) { return Value{(11 + uint64_t{args[0].i32} / 11) / 2}; }; @@ -479,8 +481,9 @@ TEST(execute_call, imported_function_from_another_module) const auto func_idx = fizzy::find_exported_function(*module1, "sub"); ASSERT_TRUE(func_idx.has_value()); - auto sub = [&instance1, func_idx](Instance&, const Value* args, int depth) -> ExecutionResult { - return fizzy::execute(*instance1, *func_idx, args, depth); + auto sub = [&instance1, func_idx](Instance&, const Value* args, ThreadContext& ctx) { + // ThreadContext is bumped here. + return fizzy::execute(*instance1, *func_idx, args, ctx); }; auto instance2 = instantiate(parse(bin2), {{sub, module1->typesec[0]}}); @@ -615,8 +618,8 @@ TEST(execute_call, call_initial_depth) const auto wasm = from_hex("0061736d01000000010401600000020b01036d6f6403666f6f0000"); auto module = parse(wasm); - auto host_foo = [](Instance& /*instance*/, const Value*, int depth) -> ExecutionResult { - EXPECT_EQ(depth, 0); + auto host_foo = [](Instance&, const Value*, ThreadContext& ctx) { + EXPECT_EQ(ctx.depth, 0); return Void; }; const auto host_foo_type = module->typesec[0]; @@ -637,10 +640,12 @@ TEST(execute_call, call_max_depth) auto instance = instantiate(parse(bin)); - const auto max_depth = TestCallStackLimit - 1; - EXPECT_THAT( - TypedExecutionResult(execute(*instance, 0, {}, max_depth), ValType::i32), Result(42)); - EXPECT_THAT(execute(*instance, 1, {}, max_depth), Traps()); + ThreadContext ctx; + ctx.depth = TestCallStackLimit - 1; + EXPECT_THAT(TypedExecutionResult(execute(*instance, 0, {}, ctx), ValType::i32), Result(42)); + EXPECT_EQ(ctx.depth, TestCallStackLimit - 1); + EXPECT_THAT(execute(*instance, 1, {}, ctx), Traps()); + EXPECT_EQ(ctx.depth, TestCallStackLimit - 1); } TEST(execute_call, execute_imported_max_depth) @@ -654,18 +659,26 @@ TEST(execute_call, execute_imported_max_depth) from_hex("0061736d01000000010401600000020b01036d6f6403666f6f0000030201000a040102000b"); auto module = parse(wasm); - auto host_foo = [](Instance& /*instance*/, const Value*, int depth) -> ExecutionResult { - EXPECT_LE(depth, TestCallStackLimit - 1); + auto host_foo = [](Instance&, const Value*, ThreadContext& ctx) { + EXPECT_LE(ctx.depth, TestCallStackLimit - 1); return Void; }; const auto host_foo_type = module->typesec[0]; auto instance = instantiate(std::move(module), {{host_foo, host_foo_type}}); - EXPECT_THAT(execute(*instance, 0, {}, TestCallStackLimit - 1), Result()); - EXPECT_THAT(execute(*instance, 1, {}, TestCallStackLimit - 1), Result()); - EXPECT_THAT(execute(*instance, 0, {}, TestCallStackLimit), Traps()); - EXPECT_THAT(execute(*instance, 1, {}, TestCallStackLimit), Traps()); + ThreadContext ctx; + ctx.depth = TestCallStackLimit - 1; + EXPECT_THAT(execute(*instance, 0, {}, ctx), Result()); + EXPECT_EQ(ctx.depth, TestCallStackLimit - 1); + EXPECT_THAT(execute(*instance, 1, {}, ctx), Result()); + EXPECT_EQ(ctx.depth, TestCallStackLimit - 1); + + ctx.depth = TestCallStackLimit; + EXPECT_THAT(execute(*instance, 0, {}, ctx), Traps()); + EXPECT_EQ(ctx.depth, TestCallStackLimit); + EXPECT_THAT(execute(*instance, 1, {}, ctx), Traps()); + EXPECT_EQ(ctx.depth, TestCallStackLimit); } TEST(execute_call, imported_function_from_another_module_max_depth) @@ -696,14 +709,19 @@ TEST(execute_call, imported_function_from_another_module_max_depth) const auto func_idx = fizzy::find_exported_function(*instance1->module, "f"); ASSERT_TRUE(func_idx.has_value()); - auto sub = [&instance1, func_idx](Instance&, const Value* args, int depth) -> ExecutionResult { - return fizzy::execute(*instance1, *func_idx, args, depth + 1); + auto sub = [&instance1, func_idx](Instance&, const Value* args, ThreadContext& ctx) { + const auto guard = ThreadContext::Guard{ctx}; + return fizzy::execute(*instance1, *func_idx, args, ctx); }; auto instance2 = instantiate(std::move(module2), {{sub, instance1->module->typesec[0]}}); - EXPECT_THAT(execute(*instance2, 2, {}, TestCallStackLimit - 1 - 1), Traps()); - EXPECT_THAT(execute(*instance2, 3, {}, TestCallStackLimit - 1 - 1), Result()); + ThreadContext ctx; + ctx.depth = TestCallStackLimit - 1 - 1; + EXPECT_THAT(execute(*instance2, 2, {}, ctx), Traps()); + EXPECT_EQ(ctx.depth, TestCallStackLimit - 1 - 1); + EXPECT_THAT(execute(*instance2, 3, {}, ctx), Result()); + EXPECT_EQ(ctx.depth, TestCallStackLimit - 1 - 1); } TEST(execute_call, count_calls_to_imported_function) @@ -772,10 +790,11 @@ TEST(execute_call, call_imported_infinite_recursion) const auto module = parse(wasm); int counter = 0; - auto host_foo = [&counter](Instance& instance, const Value* args, int depth) { - EXPECT_LE(depth, TestCallStackLimit - 1); + auto host_foo = [&counter](Instance& instance, const Value* args, ThreadContext& ctx) { + EXPECT_LE(ctx.depth, TestCallStackLimit - 1); ++counter; - return execute(instance, 0, args, depth + 1); + const auto guard = ThreadContext::Guard{ctx}; + return execute(instance, 0, args, ctx); }; const auto host_foo_type = module->typesec[0]; @@ -803,11 +822,13 @@ TEST(execute_call, call_imported_interleaved_infinite_recursion) const auto module = parse(wasm); int counter = 0; - auto host_foo = [&counter](Instance& instance, const Value* args, int depth) { + auto host_foo = [&counter](Instance& instance, const Value* args, ThreadContext& ctx) { // Function $f will increase depth. This means each iteration goes 2 steps deeper. - EXPECT_LT(depth, CallStackLimit); + EXPECT_LT(ctx.depth, CallStackLimit); ++counter; - return execute(instance, 1, args, depth + 1); + + const auto guard = ThreadContext::Guard{ctx}; + return execute(instance, 1, args, ctx); }; const auto host_foo_type = module->typesec[0]; @@ -832,10 +853,13 @@ TEST(execute_call, call_imported_max_depth_recursion) const auto wasm = from_hex("0061736d010000000105016000017f020b01036d6f6403666f6f0000"); const auto module = parse(wasm); - auto host_foo = [](Instance& instance, const Value* args, int depth) -> ExecutionResult { - if (depth == TestCallStackLimit - 1) + auto host_foo = [](Instance& instance, const Value* args, + ThreadContext& ctx) -> ExecutionResult { + if (ctx.depth == TestCallStackLimit - 1) return Value{uint32_t{1}}; // Terminate recursion on the max depth. - return execute(instance, 0, args, depth + 1); + + const auto guard = ThreadContext::Guard{ctx}; + return execute(instance, 0, args, ctx); }; const auto host_foo_type = module->typesec[0]; @@ -856,11 +880,14 @@ TEST(execute_call, call_via_imported_max_depth_recursion) "0061736d010000000105016000017f020b01036d6f6403666f6f0000030201000a0601040010000b"); const auto module = parse(wasm); - auto host_foo = [](Instance& instance, const Value* args, int depth) -> ExecutionResult { + auto host_foo = [](Instance& instance, const Value* args, + ThreadContext& ctx) -> ExecutionResult { // Function $f will increase depth. This means each iteration goes 2 steps deeper. - if (depth == TestCallStackLimit - 1) + if (ctx.depth == TestCallStackLimit - 1) return Value{uint32_t{1}}; // Terminate recursion on the max depth. - return execute(instance, 1, args, depth + 1); + + const auto guard = ThreadContext::Guard{ctx}; + return execute(instance, 1, args, ctx); }; const auto host_foo_type = module->typesec[0]; diff --git a/test/unittests/execute_test.cpp b/test/unittests/execute_test.cpp index 8d54f7ceb..7b953c804 100644 --- a/test/unittests/execute_test.cpp +++ b/test/unittests/execute_test.cpp @@ -810,7 +810,7 @@ TEST(execute, imported_function) const auto module = parse(wasm); ASSERT_EQ(module->typesec.size(), 1); - constexpr auto host_foo = [](Instance&, const Value* args, int) { + constexpr auto host_foo = [](Instance&, const Value* args, ThreadContext&) { return Value{args[0].i32 + args[1].i32}; }; @@ -830,10 +830,10 @@ TEST(execute, imported_two_functions) const auto module = parse(wasm); ASSERT_EQ(module->typesec.size(), 1); - constexpr auto host_foo1 = [](Instance&, const Value* args, int) { + constexpr auto host_foo1 = [](Instance&, const Value* args, ThreadContext&) { return Value{args[0].i32 + args[1].i32}; }; - constexpr auto host_foo2 = [](Instance&, const Value* args, int) { + constexpr auto host_foo2 = [](Instance&, const Value* args, ThreadContext&) { return Value{args[0].i32 * args[1].i32}; }; @@ -857,10 +857,10 @@ TEST(execute, imported_functions_and_regular_one) "0061736d0100000001070160027f7f017f021702036d6f6404666f6f310000036d6f6404666f6f320000030201" "000a0901070041aa80a8010b"); - constexpr auto host_foo1 = [](Instance&, const Value* args, int) { + constexpr auto host_foo1 = [](Instance&, const Value* args, ThreadContext&) { return Value{args[0].i32 + args[1].i32}; }; - constexpr auto host_foo2 = [](Instance&, const Value* args, int) { + constexpr auto host_foo2 = [](Instance&, const Value* args, ThreadContext&) { return Value{args[0].i32 * args[1].i32}; }; @@ -887,10 +887,10 @@ TEST(execute, imported_two_functions_different_type) "0061736d01000000010c0260027f7f017f60017e017e021702036d6f6404666f6f310000036d6f6404666f6f32" "0001030201010a0901070042aa80a8010b"); - constexpr auto host_foo1 = [](Instance&, const Value* args, int) { + constexpr auto host_foo1 = [](Instance&, const Value* args, ThreadContext&) { return Value{args[0].i32 + args[1].i32}; }; - constexpr auto host_foo2 = [](Instance&, const Value* args, int) { + constexpr auto host_foo2 = [](Instance&, const Value* args, ThreadContext&) { return Value{args[0].i64 * args[0].i64}; }; @@ -911,7 +911,9 @@ TEST(execute, imported_function_traps) */ const auto wasm = from_hex("0061736d0100000001070160027f7f017f020b01036d6f6403666f6f0000"); - constexpr auto host_foo = [](Instance&, const Value*, int) -> ExecutionResult { return Trap; }; + constexpr auto host_foo = [](Instance&, const Value*, ThreadContext&) -> ExecutionResult { + return Trap; + }; const auto module = parse(wasm); auto instance = instantiate(*module, {{host_foo, module->typesec[0]}}); diff --git a/test/unittests/instantiate_test.cpp b/test/unittests/instantiate_test.cpp index 08decd382..aa2528f2a 100644 --- a/test/unittests/instantiate_test.cpp +++ b/test/unittests/instantiate_test.cpp @@ -15,12 +15,12 @@ using namespace fizzy::test; namespace { -ExecutionResult host_fn_1(Instance&, const Value*, int) noexcept +ExecutionResult host_fn_1(Instance&, const Value*, ThreadContext&) noexcept { return Trap; } -ExecutionResult host_fn_2(Instance&, const Value*, int) noexcept +ExecutionResult host_fn_2(Instance&, const Value*, ThreadContext&) noexcept { return Trap; } @@ -28,7 +28,7 @@ ExecutionResult host_fn_2(Instance&, const Value*, int) noexcept uint32_t call_table_func(Instance& instance, size_t idx) { const auto& elem = (*instance.table)[idx]; - const auto res = execute(*elem.instance, elem.func_idx, {}, 0); + const auto res = execute(*elem.instance, elem.func_idx, {}); EXPECT_TRUE(res.has_value); return res.value.i32; } @@ -37,8 +37,9 @@ uint32_t call_table_func(Instance& instance, size_t idx) TEST(instantiate, check_test_host_functions) { Instance instance{{}, {nullptr, nullptr}, {}, {}, {nullptr, nullptr}, {}, {}, {}, {}}; - EXPECT_THAT(host_fn_1(instance, nullptr, 0), Traps()); - EXPECT_THAT(host_fn_2(instance, nullptr, 0), Traps()); + ThreadContext ctx; + EXPECT_THAT(host_fn_1(instance, nullptr, ctx), Traps()); + EXPECT_THAT(host_fn_2(instance, nullptr, ctx), Traps()); } TEST(instantiate, imported_functions) diff --git a/test/utils/fizzy_c_engine.cpp b/test/utils/fizzy_c_engine.cpp index e476550a3..6997376af 100644 --- a/test/utils/fizzy_c_engine.cpp +++ b/test/utils/fizzy_c_engine.cpp @@ -28,7 +28,8 @@ class FizzyCEngine final : public WasmEngine namespace { -FizzyExecutionResult env_adler32(void*, FizzyInstance* instance, const FizzyValue* args, int) +FizzyExecutionResult env_adler32( + void*, FizzyInstance* instance, const FizzyValue* args, FizzyThreadContext*) { auto* memory = fizzy_get_instance_memory_data(instance); assert(memory != nullptr); diff --git a/test/utils/fizzy_engine.cpp b/test/utils/fizzy_engine.cpp index 9055f19be..8c898ad79 100644 --- a/test/utils/fizzy_engine.cpp +++ b/test/utils/fizzy_engine.cpp @@ -53,7 +53,8 @@ FuncType translate_signature(std::string_view signature) return func_type; } -fizzy::ExecutionResult env_adler32(fizzy::Instance& instance, const fizzy::Value* args, int) +fizzy::ExecutionResult env_adler32( + fizzy::Instance& instance, const fizzy::Value* args, ThreadContext&) { assert(instance.memory != nullptr); const auto ret = fizzy::test::adler32( diff --git a/tools/wasi/wasi.cpp b/tools/wasi/wasi.cpp index b7e2606e7..85421719a 100644 --- a/tools/wasi/wasi.cpp +++ b/tools/wasi/wasi.cpp @@ -18,19 +18,22 @@ namespace // and we are a single-run tool. This may change in the future and should reevaluate. uvwasi_t state; -fizzy::ExecutionResult wasi_return_enosys(fizzy::Instance&, const fizzy::Value*, int) +fizzy::ExecutionResult wasi_return_enosys( + fizzy::Instance&, const fizzy::Value*, fizzy::ThreadContext&) { return fizzy::Value{uint32_t{UVWASI_ENOSYS}}; } -fizzy::ExecutionResult wasi_proc_exit(fizzy::Instance&, const fizzy::Value* args, int) +fizzy::ExecutionResult wasi_proc_exit( + fizzy::Instance&, const fizzy::Value* args, fizzy::ThreadContext&) { uvwasi_proc_exit(&state, static_cast(args[0].as())); // Should not reach this. return fizzy::Trap; } -fizzy::ExecutionResult wasi_fd_write(fizzy::Instance& instance, const fizzy::Value* args, int) +fizzy::ExecutionResult wasi_fd_write( + fizzy::Instance& instance, const fizzy::Value* args, fizzy::ThreadContext&) { const auto fd = args[0].as(); const auto iov_ptr = args[1].as(); @@ -51,7 +54,8 @@ fizzy::ExecutionResult wasi_fd_write(fizzy::Instance& instance, const fizzy::Val return fizzy::Value{uint32_t{ret}}; } -fizzy::ExecutionResult wasi_fd_read(fizzy::Instance& instance, const fizzy::Value* args, int) +fizzy::ExecutionResult wasi_fd_read( + fizzy::Instance& instance, const fizzy::Value* args, fizzy::ThreadContext&) { const auto fd = args[0].as(); const auto iov_ptr = args[1].as(); @@ -72,7 +76,8 @@ fizzy::ExecutionResult wasi_fd_read(fizzy::Instance& instance, const fizzy::Valu return fizzy::Value{uint32_t{ret}}; } -fizzy::ExecutionResult wasi_fd_prestat_get(fizzy::Instance& instance, const fizzy::Value* args, int) +fizzy::ExecutionResult wasi_fd_prestat_get( + fizzy::Instance& instance, const fizzy::Value* args, fizzy::ThreadContext&) { const auto fd = args[0].as(); const auto prestat_ptr = args[1].as(); @@ -86,7 +91,7 @@ fizzy::ExecutionResult wasi_fd_prestat_get(fizzy::Instance& instance, const fizz } fizzy::ExecutionResult wasi_environ_sizes_get( - fizzy::Instance& instance, const fizzy::Value* args, int) + fizzy::Instance& instance, const fizzy::Value* args, fizzy::ThreadContext&) { const auto environc = args[0].as(); const auto environ_buf_size = args[1].as(); From e0a6dfd5024bf7a94a867ce16b6f65666900c8a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Mon, 8 Feb 2021 09:35:35 +0100 Subject: [PATCH 7/8] Alternative implementation of depth bump in ThreadContext --- lib/fizzy/execute.cpp | 2 +- lib/fizzy/instantiate.hpp | 24 +++++++++++------------- test/unittests/execute_call_test.cpp | 10 +++++----- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/lib/fizzy/execute.cpp b/lib/fizzy/execute.cpp index f867b582e..9871277cf 100644 --- a/lib/fizzy/execute.cpp +++ b/lib/fizzy/execute.cpp @@ -481,7 +481,7 @@ inline bool invoke_function(const FuncType& func_type, uint32_t func_idx, Instan assert(stack.size() >= num_args); const auto call_args = stack.rend() - num_args; - const auto guard = ThreadContext::Guard{ctx}; + const auto guard = ctx.bump_call_depth(); const auto ret = execute(instance, func_idx, call_args, ctx); // Bubble up traps if (ret.trapped) diff --git a/lib/fizzy/instantiate.hpp b/lib/fizzy/instantiate.hpp index de3d30900..4ee86747e 100644 --- a/lib/fizzy/instantiate.hpp +++ b/lib/fizzy/instantiate.hpp @@ -23,25 +23,23 @@ struct Instance; class ThreadContext { -public: - int depth = 0; - - void acquire() noexcept { ++depth; } - - void release() noexcept { --depth; } - class [[nodiscard]] Guard { ThreadContext& m_thread_context; public: - explicit Guard(ThreadContext& ctx) noexcept : m_thread_context{ctx} - { - m_thread_context.acquire(); - } - - ~Guard() noexcept { m_thread_context.release(); } + explicit Guard(ThreadContext& ctx) noexcept : m_thread_context{ctx} {} + ~Guard() noexcept { --m_thread_context.depth; } }; + +public: + int depth = 0; + + Guard bump_call_depth() noexcept + { + ++depth; + return Guard{*this}; + } }; /// Function representing WebAssembly or host function execution. diff --git a/test/unittests/execute_call_test.cpp b/test/unittests/execute_call_test.cpp index fd71b1d5a..b77d5c6cf 100644 --- a/test/unittests/execute_call_test.cpp +++ b/test/unittests/execute_call_test.cpp @@ -710,7 +710,7 @@ TEST(execute_call, imported_function_from_another_module_max_depth) ASSERT_TRUE(func_idx.has_value()); auto sub = [&instance1, func_idx](Instance&, const Value* args, ThreadContext& ctx) { - const auto guard = ThreadContext::Guard{ctx}; + const auto guard = ctx.bump_call_depth(); return fizzy::execute(*instance1, *func_idx, args, ctx); }; @@ -793,7 +793,7 @@ TEST(execute_call, call_imported_infinite_recursion) auto host_foo = [&counter](Instance& instance, const Value* args, ThreadContext& ctx) { EXPECT_LE(ctx.depth, TestCallStackLimit - 1); ++counter; - const auto guard = ThreadContext::Guard{ctx}; + const auto guard = ctx.bump_call_depth(); return execute(instance, 0, args, ctx); }; const auto host_foo_type = module->typesec[0]; @@ -827,7 +827,7 @@ TEST(execute_call, call_imported_interleaved_infinite_recursion) EXPECT_LT(ctx.depth, CallStackLimit); ++counter; - const auto guard = ThreadContext::Guard{ctx}; + const auto guard = ctx.bump_call_depth(); return execute(instance, 1, args, ctx); }; const auto host_foo_type = module->typesec[0]; @@ -858,7 +858,7 @@ TEST(execute_call, call_imported_max_depth_recursion) if (ctx.depth == TestCallStackLimit - 1) return Value{uint32_t{1}}; // Terminate recursion on the max depth. - const auto guard = ThreadContext::Guard{ctx}; + const auto guard = ctx.bump_call_depth(); return execute(instance, 0, args, ctx); }; const auto host_foo_type = module->typesec[0]; @@ -886,7 +886,7 @@ TEST(execute_call, call_via_imported_max_depth_recursion) if (ctx.depth == TestCallStackLimit - 1) return Value{uint32_t{1}}; // Terminate recursion on the max depth. - const auto guard = ThreadContext::Guard{ctx}; + const auto guard = ctx.bump_call_depth(); return execute(instance, 1, args, ctx); }; const auto host_foo_type = module->typesec[0]; From 62cfb871ba2d19c44ea87729b9806a05b5bac9af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Mon, 8 Feb 2021 09:36:56 +0100 Subject: [PATCH 8/8] Move ThreadContext to execute.hpp --- lib/fizzy/execute.hpp | 22 ++++++++++++++++++++++ lib/fizzy/instantiate.hpp | 22 +--------------------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/lib/fizzy/execute.hpp b/lib/fizzy/execute.hpp index 2d744913a..6278f5914 100644 --- a/lib/fizzy/execute.hpp +++ b/lib/fizzy/execute.hpp @@ -41,6 +41,28 @@ constexpr ExecutionResult Void{true}; /// Shortcut for execution that resulted in a trap. constexpr ExecutionResult Trap{false}; + +class ThreadContext +{ + class [[nodiscard]] Guard + { + ThreadContext& m_thread_context; + + public: + explicit Guard(ThreadContext& ctx) noexcept : m_thread_context{ctx} {} + ~Guard() noexcept { --m_thread_context.depth; } + }; + +public: + int depth = 0; + + Guard bump_call_depth() noexcept + { + ++depth; + return Guard{*this}; + } +}; + /// Execute a function from an instance. /// /// @param instance The instance. diff --git a/lib/fizzy/instantiate.hpp b/lib/fizzy/instantiate.hpp index 4ee86747e..2a8fcc38a 100644 --- a/lib/fizzy/instantiate.hpp +++ b/lib/fizzy/instantiate.hpp @@ -20,27 +20,7 @@ namespace fizzy { struct ExecutionResult; struct Instance; - -class ThreadContext -{ - class [[nodiscard]] Guard - { - ThreadContext& m_thread_context; - - public: - explicit Guard(ThreadContext& ctx) noexcept : m_thread_context{ctx} {} - ~Guard() noexcept { --m_thread_context.depth; } - }; - -public: - int depth = 0; - - Guard bump_call_depth() noexcept - { - ++depth; - return Guard{*this}; - } -}; +class ThreadContext; /// Function representing WebAssembly or host function execution. using execute_function = std::function;