From c28b01f9d603a8ca18424c62e1bd66a239c4965f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 9 Apr 2020 11:53:15 +0200 Subject: [PATCH 1/9] Allocate space for locals together with stack This move memory management of args and locals to OperandStack where space for all can be allocated in single go. --- lib/fizzy/execute.cpp | 14 ++++-------- lib/fizzy/stack.hpp | 41 +++++++++++++++++++++++++---------- test/unittests/span_test.cpp | 2 +- test/unittests/stack_test.cpp | 14 ++++++------ 4 files changed, 42 insertions(+), 29 deletions(-) diff --git a/lib/fizzy/execute.cpp b/lib/fizzy/execute.cpp index d89c45fe7..67560ad85 100644 --- a/lib/fizzy/execute.cpp +++ b/lib/fizzy/execute.cpp @@ -477,10 +477,7 @@ ExecutionResult execute( const auto& code = instance.module.codesec[code_idx]; auto* const memory = instance.memory.get(); - std::vector locals(args.size() + code.local_count); - std::copy_n(args.begin(), args.size(), locals.begin()); - - OperandStack stack(static_cast(code.max_stack_height)); + OperandStack stack(args, code.local_count, static_cast(code.max_stack_height)); const Instr* pc = code.instructions.data(); const uint8_t* immediates = code.immediates.data(); @@ -614,22 +611,19 @@ ExecutionResult execute( case Instr::local_get: { const auto idx = read(immediates); - assert(idx <= locals.size()); - stack.push(locals[idx]); + stack.push(stack.local(idx)); break; } case Instr::local_set: { const auto idx = read(immediates); - assert(idx <= locals.size()); - locals[idx] = stack.pop(); + stack.local(idx) = stack.pop(); break; } case Instr::local_tee: { const auto idx = read(immediates); - assert(idx <= locals.size()); - locals[idx] = stack.top(); + stack.local(idx) = stack.top(); break; } case Instr::global_get: diff --git a/lib/fizzy/stack.hpp b/lib/fizzy/stack.hpp index f021ac5dc..7175eb331 100644 --- a/lib/fizzy/stack.hpp +++ b/lib/fizzy/stack.hpp @@ -4,6 +4,7 @@ #pragma once +#include "span.hpp" #include "value.hpp" #include #include @@ -61,6 +62,10 @@ class OperandStack /// in the constructor after the m_storage. Value* m_top; + Value* m_bottom; + + Value* m_locals; + /// The pre-allocated internal storage. Value m_small_storage[small_storage_size]; @@ -69,29 +74,43 @@ class OperandStack Value* bottom() noexcept { return m_large_storage ? m_large_storage.get() : m_small_storage; } - const Value* bottom() const noexcept - { - return m_large_storage ? m_large_storage.get() : m_small_storage; - } - public: /// Default constructor. /// /// Based on @p max_stack_height decides if to use small pre-allocated storage or allocate /// large storage. /// Sets the top item pointer to below the stack bottom. - explicit OperandStack(size_t max_stack_height) + OperandStack(span args, size_t num_locals, size_t max_stack_height) { - if (max_stack_height > small_storage_size) - m_large_storage = std::make_unique(max_stack_height); - m_top = bottom() - 1; + const auto num_args = args.size(); + const auto storage_size_required = num_args + num_locals + max_stack_height; + + Value* storage; + if (storage_size_required <= small_storage_size) + { + storage = &m_small_storage[0]; + } + else + { + m_large_storage = std::make_unique(storage_size_required); + storage = &m_large_storage[0]; + } + + m_locals = storage; + m_bottom = m_locals + num_args + num_locals; + m_top = m_bottom - 1; + + std::copy(std::begin(args), std::end(args), m_locals); + std::fill_n(m_locals + num_args, num_locals, 0); } OperandStack(const OperandStack&) = delete; OperandStack& operator=(const OperandStack&) = delete; + Value& local(size_t index) noexcept { return m_locals[index]; } + /// The current number of items on the stack (aka stack height). - size_t size() const noexcept { return static_cast(m_top + 1 - bottom()); } + size_t size() const noexcept { return static_cast(m_top + 1 - m_bottom); } /// Returns the reference to the top item. /// Requires non-empty stack. @@ -139,7 +158,7 @@ class OperandStack } /// Returns iterator to the bottom of the stack. - const Value* rbegin() const noexcept { return bottom(); } + const Value* rbegin() const noexcept { return m_bottom; } /// Returns end iterator counting from the bottom of the stack. const Value* rend() const noexcept { return m_top + 1; } diff --git a/test/unittests/span_test.cpp b/test/unittests/span_test.cpp index 11ccc9ace..a76dd03ea 100644 --- a/test/unittests/span_test.cpp +++ b/test/unittests/span_test.cpp @@ -50,7 +50,7 @@ TEST(span, array) TEST(span, stack) { - OperandStack stack(4); + OperandStack stack({}, 0, 4); stack.push(10); stack.push(11); stack.push(12); diff --git a/test/unittests/stack_test.cpp b/test/unittests/stack_test.cpp index 5ac226bf5..728b46df0 100644 --- a/test/unittests/stack_test.cpp +++ b/test/unittests/stack_test.cpp @@ -115,7 +115,7 @@ TEST(stack, struct_item) TEST(operand_stack, construct) { - OperandStack stack(0); + OperandStack stack({}, 0, 0); EXPECT_EQ(stack.size(), 0); stack.shrink(0); EXPECT_EQ(stack.size(), 0); @@ -123,7 +123,7 @@ TEST(operand_stack, construct) TEST(operand_stack, top) { - OperandStack stack(1); + OperandStack stack({}, 0, 1); EXPECT_EQ(stack.size(), 0); stack.push(1); @@ -147,7 +147,7 @@ TEST(operand_stack, top) TEST(operand_stack, small) { - OperandStack stack(3); + OperandStack stack({}, 0, 3); EXPECT_EQ(stack.size(), 0); stack.push(1); @@ -176,7 +176,7 @@ TEST(operand_stack, small) TEST(operand_stack, large) { constexpr auto max_height = 33; - OperandStack stack(max_height); + OperandStack stack({}, 0, max_height); for (unsigned i = 0; i < max_height; ++i) stack.push(i); @@ -190,7 +190,7 @@ TEST(operand_stack, large) TEST(operand_stack, shrink) { constexpr auto max_height = 60; - OperandStack stack(max_height); + OperandStack stack({}, 0, max_height); for (unsigned i = 0; i < max_height; ++i) stack.push(i); @@ -206,7 +206,7 @@ TEST(operand_stack, shrink) TEST(operand_stack, rbegin_rend) { - OperandStack stack(3); + OperandStack stack({}, 0, 3); EXPECT_EQ(stack.rbegin(), stack.rend()); stack.push(1); @@ -219,7 +219,7 @@ TEST(operand_stack, rbegin_rend) TEST(operand_stack, to_vector) { - OperandStack stack(3); + OperandStack stack({}, 0, 3); EXPECT_THAT(std::vector(stack.rbegin(), stack.rend()), IsEmpty()); stack.push(1); From f3bcaff8e29f0d2378ef3317b5296378d2204ea8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 10 Sep 2020 17:40:00 +0200 Subject: [PATCH 2/9] Fix OperandStack --- lib/fizzy/stack.hpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/fizzy/stack.hpp b/lib/fizzy/stack.hpp index 7175eb331..0db944c03 100644 --- a/lib/fizzy/stack.hpp +++ b/lib/fizzy/stack.hpp @@ -72,8 +72,6 @@ class OperandStack /// The unbounded storage for items. std::unique_ptr m_large_storage; - Value* bottom() noexcept { return m_large_storage ? m_large_storage.get() : m_small_storage; } - public: /// Default constructor. /// @@ -154,7 +152,7 @@ class OperandStack { assert(new_size <= size()); // For new_size == 0, the m_top will point below the storage. - m_top = bottom() + new_size - 1; + m_top = m_bottom + new_size - 1; } /// Returns iterator to the bottom of the stack. From 30ef64ddedcfd5705ae5c06575aeb7d5875fb18a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 10 Sep 2020 17:47:15 +0200 Subject: [PATCH 3/9] Stack external storage --- lib/fizzy/execute.cpp | 5 ++++- lib/fizzy/stack.hpp | 12 +++--------- test/unittests/span_test.cpp | 2 +- test/unittests/stack_test.cpp | 14 +++++++------- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/lib/fizzy/execute.cpp b/lib/fizzy/execute.cpp index 67560ad85..d47f25128 100644 --- a/lib/fizzy/execute.cpp +++ b/lib/fizzy/execute.cpp @@ -477,7 +477,10 @@ ExecutionResult execute( const auto& code = instance.module.codesec[code_idx]; auto* const memory = instance.memory.get(); - OperandStack stack(args, code.local_count, static_cast(code.max_stack_height)); + constexpr auto stack_space_size = 128 / sizeof(Value); + Value stack_space[stack_space_size]; + OperandStack stack(args, code.local_count, static_cast(code.max_stack_height), + stack_space, std::size(stack_space)); const Instr* pc = code.instructions.data(); const uint8_t* immediates = code.immediates.data(); diff --git a/lib/fizzy/stack.hpp b/lib/fizzy/stack.hpp index 0db944c03..76aee7d20 100644 --- a/lib/fizzy/stack.hpp +++ b/lib/fizzy/stack.hpp @@ -52,9 +52,6 @@ class Stack class OperandStack { - /// The size of the pre-allocated internal storage: 128 bytes. - static constexpr auto small_storage_size = 128 / sizeof(Value); - /// The pointer to the top item, or below the stack bottom if stack is empty. /// /// This pointer always alias m_storage, but it is kept as the first field @@ -66,9 +63,6 @@ class OperandStack Value* m_locals; - /// The pre-allocated internal storage. - Value m_small_storage[small_storage_size]; - /// The unbounded storage for items. std::unique_ptr m_large_storage; @@ -78,15 +72,15 @@ class OperandStack /// Based on @p max_stack_height decides if to use small pre-allocated storage or allocate /// large storage. /// Sets the top item pointer to below the stack bottom. - OperandStack(span args, size_t num_locals, size_t max_stack_height) + OperandStack(span args, size_t num_locals, size_t max_stack_height, Value* external_storage, size_t external_storage_size) { const auto num_args = args.size(); const auto storage_size_required = num_args + num_locals + max_stack_height; Value* storage; - if (storage_size_required <= small_storage_size) + if (storage_size_required <= external_storage_size) { - storage = &m_small_storage[0]; + storage = external_storage; } else { diff --git a/test/unittests/span_test.cpp b/test/unittests/span_test.cpp index a76dd03ea..7399594b2 100644 --- a/test/unittests/span_test.cpp +++ b/test/unittests/span_test.cpp @@ -50,7 +50,7 @@ TEST(span, array) TEST(span, stack) { - OperandStack stack({}, 0, 4); + OperandStack stack({}, 0, 4, nullptr, 0); stack.push(10); stack.push(11); stack.push(12); diff --git a/test/unittests/stack_test.cpp b/test/unittests/stack_test.cpp index 728b46df0..decca8bb1 100644 --- a/test/unittests/stack_test.cpp +++ b/test/unittests/stack_test.cpp @@ -115,7 +115,7 @@ TEST(stack, struct_item) TEST(operand_stack, construct) { - OperandStack stack({}, 0, 0); + OperandStack stack({}, 0, 0, nullptr, 0); EXPECT_EQ(stack.size(), 0); stack.shrink(0); EXPECT_EQ(stack.size(), 0); @@ -123,7 +123,7 @@ TEST(operand_stack, construct) TEST(operand_stack, top) { - OperandStack stack({}, 0, 1); + OperandStack stack({}, 0, 1, nullptr, 0); EXPECT_EQ(stack.size(), 0); stack.push(1); @@ -147,7 +147,7 @@ TEST(operand_stack, top) TEST(operand_stack, small) { - OperandStack stack({}, 0, 3); + OperandStack stack({}, 0, 3, nullptr, 0); EXPECT_EQ(stack.size(), 0); stack.push(1); @@ -176,7 +176,7 @@ TEST(operand_stack, small) TEST(operand_stack, large) { constexpr auto max_height = 33; - OperandStack stack({}, 0, max_height); + OperandStack stack({}, 0, max_height, nullptr, 0); for (unsigned i = 0; i < max_height; ++i) stack.push(i); @@ -190,7 +190,7 @@ TEST(operand_stack, large) TEST(operand_stack, shrink) { constexpr auto max_height = 60; - OperandStack stack({}, 0, max_height); + OperandStack stack({}, 0, max_height, nullptr, 0); for (unsigned i = 0; i < max_height; ++i) stack.push(i); @@ -206,7 +206,7 @@ TEST(operand_stack, shrink) TEST(operand_stack, rbegin_rend) { - OperandStack stack({}, 0, 3); + OperandStack stack({}, 0, 3, nullptr, 0); EXPECT_EQ(stack.rbegin(), stack.rend()); stack.push(1); @@ -219,7 +219,7 @@ TEST(operand_stack, rbegin_rend) TEST(operand_stack, to_vector) { - OperandStack stack({}, 0, 3); + OperandStack stack({}, 0, 3, nullptr, 0); EXPECT_THAT(std::vector(stack.rbegin(), stack.rend()), IsEmpty()); stack.push(1); From 522873e179c18395a6c7f1627ce5457f018b182c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 10 Sep 2020 09:52:01 +0200 Subject: [PATCH 4/9] 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 f485a646b..303340160 100644 --- a/lib/fizzy/execute.hpp +++ b/lib/fizzy/execute.hpp @@ -36,9 +36,16 @@ struct ExecutionResult constexpr ExecutionResult Void{true}; constexpr ExecutionResult Trap{false}; -// Execute a function on an instance. +/// Execute a function on an instance. ExecutionResult execute( - Instance& instance, FuncIdx func_idx, span args, int depth = 0) noexcept; + Instance& instance, FuncIdx func_idx, span args, int depth) noexcept; + +/// Execute a function on an instance with implicit depth 0. +inline ExecutionResult execute( + Instance& instance, FuncIdx func_idx, span args) noexcept +{ + return execute(instance, func_idx, args, 0); +} inline ExecutionResult execute( Instance& instance, FuncIdx func_idx, std::initializer_list args) noexcept From 5f08ee277a7959252b9dfd5763ce66dfc6b89b5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 10 Sep 2020 10:31:52 +0200 Subject: [PATCH 5/9] Introduce ThreadContext --- lib/fizzy/execute.cpp | 29 +++++++++++---------- lib/fizzy/execute.hpp | 7 ++--- lib/fizzy/instantiate.cpp | 13 +++++++--- lib/fizzy/instantiate.hpp | 14 ++++++++-- test/unittests/api_test.cpp | 17 ++++++++----- test/unittests/execute_call_test.cpp | 34 +++++++++++++++---------- test/unittests/execute_control_test.cpp | 4 ++- test/unittests/execute_test.cpp | 18 ++++++------- test/unittests/instantiate_test.cpp | 15 ++++++----- test/utils/fizzy_engine.cpp | 3 ++- 10 files changed, 93 insertions(+), 61 deletions(-) diff --git a/lib/fizzy/execute.cpp b/lib/fizzy/execute.cpp index d47f25128..859f19264 100644 --- a/lib/fizzy/execute.cpp +++ b/lib/fizzy/execute.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include namespace fizzy @@ -54,14 +55,14 @@ void branch(const Code& code, OperandStack& stack, const Instr*& pc, const uint8 template bool invoke_function(const FuncType& func_type, const F& func, Instance& instance, - OperandStack& stack, int depth) noexcept + OperandStack& stack, ThreadContext& thread_context) noexcept { const auto num_args = func_type.inputs.size(); assert(stack.size() >= num_args); span call_args{stack.rend() - num_args, num_args}; stack.drop(num_args); - const auto ret = func(instance, call_args, depth + 1); + const auto ret = func(instance, call_args, thread_context); // Bubble up traps if (ret.trapped) return false; @@ -78,12 +79,13 @@ bool invoke_function(const FuncType& func_type, const F& func, Instance& instanc } inline bool invoke_function(const FuncType& func_type, uint32_t func_idx, Instance& instance, - OperandStack& stack, int depth) noexcept + OperandStack& stack, ThreadContext& thread_context) noexcept { - const auto func = [func_idx](Instance& _instance, span args, int _depth) noexcept { - return execute(_instance, func_idx, args, _depth); + const auto func = [func_idx](Instance& _instance, span args, + ThreadContext& _thread_context) noexcept { + return execute(_instance, func_idx, args, _thread_context); }; - return invoke_function(func_type, func, instance, stack, depth); + return invoke_function(func_type, func, instance, stack, thread_context); } template @@ -461,15 +463,16 @@ __attribute__((no_sanitize("float-cast-overflow"))) inline constexpr float demot } // namespace -ExecutionResult execute( - Instance& instance, FuncIdx func_idx, span args, int depth) noexcept +ExecutionResult execute(Instance& instance, FuncIdx func_idx, span args, + ThreadContext& thread_context) noexcept { - assert(depth >= 0); - if (depth > CallStackLimit) + if (thread_context.depth > CallStackLimit) return Trap; + std::lock_guard guard{thread_context}; + 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, thread_context); const auto code_idx = func_idx - instance.imported_functions.size(); assert(code_idx < instance.module.codesec.size()); @@ -565,7 +568,7 @@ ExecutionResult execute( const auto called_func_idx = read(immediates); const auto& func_type = instance.module.get_function_type(called_func_idx); - if (!invoke_function(func_type, called_func_idx, instance, stack, depth)) + if (!invoke_function(func_type, called_func_idx, instance, stack, thread_context)) goto trap; break; } @@ -590,7 +593,7 @@ ExecutionResult execute( if (expected_type != actual_type) goto trap; - if (!invoke_function(actual_type, called_func->function, instance, stack, depth)) + if (!invoke_function(actual_type, called_func->function, instance, stack, thread_context)) goto trap; break; } diff --git a/lib/fizzy/execute.hpp b/lib/fizzy/execute.hpp index 303340160..3148499c2 100644 --- a/lib/fizzy/execute.hpp +++ b/lib/fizzy/execute.hpp @@ -37,14 +37,15 @@ constexpr ExecutionResult Void{true}; constexpr ExecutionResult Trap{false}; /// Execute a function on an instance. -ExecutionResult execute( - Instance& instance, FuncIdx func_idx, span args, int depth) noexcept; +ExecutionResult execute(Instance& instance, FuncIdx func_idx, span args, + ThreadContext& thread_context) noexcept; /// Execute a function on an instance with implicit depth 0. inline ExecutionResult execute( Instance& instance, FuncIdx func_idx, span args) noexcept { - return execute(instance, func_idx, args, 0); + ThreadContext thread_context; + return execute(instance, func_idx, args, thread_context); } inline ExecutionResult execute( diff --git a/lib/fizzy/instantiate.cpp b/lib/fizzy/instantiate.cpp index 4353803db..b2012c223 100644 --- a/lib/fizzy/instantiate.cpp +++ b/lib/fizzy/instantiate.cpp @@ -328,7 +328,9 @@ std::unique_ptr instantiate(Module module, for (const auto idx : instance->module.elementsec[i].init) { auto func = [idx, &instance_ref = *instance](fizzy::Instance&, span args, - int depth) { return execute(instance_ref, idx, args, depth); }; + ThreadContext& thread_context) { + return execute(instance_ref, idx, args, thread_context); + }; *it_table++ = ExternalFunction{std::move(func), instance->module.get_function_type(idx)}; @@ -361,7 +363,9 @@ std::unique_ptr instantiate(Module module, auto& table_function = (*it_table)->function; table_function = [shared_instance, func = std::move(table_function)]( fizzy::Instance& _instance, span args, - int depth) { return func(_instance, args, depth); }; + ThreadContext& thread_context) { + return func(_instance, args, thread_context); + }; ++it_table; } } @@ -432,8 +436,9 @@ std::optional find_exported_function(Instance& instance, std:: return std::nullopt; const auto idx = *opt_index; - auto func = [idx, &instance](fizzy::Instance&, span args, int depth) { - return execute(instance, idx, args, depth); + auto func = [idx, &instance]( + fizzy::Instance&, span args, ThreadContext& thread_context) { + return execute(instance, idx, args, thread_context); }; return ExternalFunction{std::move(func), instance.module.get_function_type(idx)}; diff --git a/lib/fizzy/instantiate.hpp b/lib/fizzy/instantiate.hpp index 0998e2a97..062d0a587 100644 --- a/lib/fizzy/instantiate.hpp +++ b/lib/fizzy/instantiate.hpp @@ -22,9 +22,19 @@ namespace fizzy struct ExecutionResult; struct Instance; +class ThreadContext +{ +public: + int depth = 0; + + void lock() noexcept { ++depth; } + + void unlock() noexcept { --depth; } +}; + struct ExternalFunction { - std::function, int depth)> function; + std::function, ThreadContext&)> function; FuncType type; }; @@ -102,7 +112,7 @@ struct ImportedFunction std::string name; std::vector inputs; std::optional output; - std::function, int depth)> function; + std::function, ThreadContext&)> function; }; // Create vector of ExternalFunctions ready to be passed to instantiate. diff --git a/test/unittests/api_test.cpp b/test/unittests/api_test.cpp index 5a913ed54..84bddd4cd 100644 --- a/test/unittests/api_test.cpp +++ b/test/unittests/api_test.cpp @@ -16,10 +16,10 @@ namespace { auto function_returning_value(Value value) noexcept { - return [value](Instance&, span, int) { return value; }; + return [value](Instance&, span, ThreadContext&) { return value; }; } -ExecutionResult function_returning_void(Instance&, span, int) noexcept +ExecutionResult function_returning_void(Instance&, span, ThreadContext&) noexcept { return Void; } @@ -243,7 +243,8 @@ TEST(api, find_exported_function) auto opt_function = find_exported_function(*instance, "foo"); ASSERT_TRUE(opt_function); - EXPECT_THAT(opt_function->function(*instance, {}, 0), Result(42)); + ThreadContext thread_context; + EXPECT_THAT(opt_function->function(*instance, {}, thread_context), Result(42)); EXPECT_TRUE(opt_function->type.inputs.empty()); ASSERT_EQ(opt_function->type.outputs.size(), 1); EXPECT_EQ(opt_function->type.outputs[0], ValType::i32); @@ -262,7 +263,7 @@ TEST(api, find_exported_function) "0061736d010000000105016000017f021001087370656374657374036261720000040401700000050401010102" "0606017f0041000b07170403666f6f000001670300037461620100036d656d0200"); - auto bar = [](Instance&, span, int) { return Value{42}; }; + auto bar = [](Instance&, span, ThreadContext&) { return Value{42}; }; const auto bar_type = FuncType{{}, {ValType::i32}}; auto instance_reexported_function = @@ -270,7 +271,7 @@ TEST(api, find_exported_function) opt_function = find_exported_function(*instance_reexported_function, "foo"); ASSERT_TRUE(opt_function); - EXPECT_THAT(opt_function->function(*instance, {}, 0), Result(42)); + EXPECT_THAT(opt_function->function(*instance, {}, thread_context), Result(42)); EXPECT_TRUE(opt_function->type.inputs.empty()); ASSERT_EQ(opt_function->type.outputs.size(), 1); EXPECT_EQ(opt_function->type.outputs[0], ValType::i32); @@ -411,8 +412,10 @@ TEST(api, find_exported_table) ASSERT_TRUE(opt_table); EXPECT_EQ(opt_table->table, instance->table.get()); EXPECT_EQ(opt_table->table->size(), 2); - EXPECT_THAT((*opt_table->table)[0]->function(*instance, {}, 0), Result(2)); - EXPECT_THAT((*opt_table->table)[1]->function(*instance, {}, 0), Result(1)); + + ThreadContext thread_context; + EXPECT_THAT((*opt_table->table)[0]->function(*instance, {}, thread_context), Result(2)); + EXPECT_THAT((*opt_table->table)[1]->function(*instance, {}, thread_context), Result(1)); EXPECT_EQ(opt_table->limits.min, 2); ASSERT_TRUE(opt_table->limits.max.has_value()); EXPECT_EQ(opt_table->limits.max, 20); diff --git a/test/unittests/execute_call_test.cpp b/test/unittests/execute_call_test.cpp index a338459df..dc7b88600 100644 --- a/test/unittests/execute_call_test.cpp +++ b/test/unittests/execute_call_test.cpp @@ -147,11 +147,11 @@ TEST(execute_call, call_indirect_imported_table) const Module module = parse(bin); - auto f1 = [](Instance&, span, int) { return Value{1}; }; - auto f2 = [](Instance&, span, int) { return Value{2}; }; - auto f3 = [](Instance&, span, int) { return Value{3}; }; - auto f4 = [](Instance&, span, int) { return Value{4}; }; - auto f5 = [](Instance&, span, int) { return Trap; }; + auto f1 = [](Instance&, span, ThreadContext&) { return Value{1}; }; + auto f2 = [](Instance&, span, ThreadContext&) { return Value{2}; }; + auto f3 = [](Instance&, span, ThreadContext&) { return Value{3}; }; + auto f4 = [](Instance&, span, ThreadContext&) { return Value{4}; }; + auto f5 = [](Instance&, span, ThreadContext&) { return Trap; }; auto out_i32 = FuncType{{}, {ValType::i32}}; auto out_i64 = FuncType{{}, {ValType::i64}}; @@ -218,7 +218,9 @@ TEST(execute_call, imported_function_call) const auto module = parse(wasm); - constexpr auto host_foo = [](Instance&, span, int) { return Value{42}; }; + constexpr auto host_foo = [](Instance&, span, ThreadContext&) { + return Value{42}; + }; const auto host_foo_type = module.typesec[0]; auto instance = instantiate(module, {{host_foo, host_foo_type}}); @@ -243,7 +245,7 @@ TEST(execute_call, imported_function_call_with_arguments) const auto module = parse(wasm); - auto host_foo = [](Instance&, span args, int) { + auto host_foo = [](Instance&, span args, ThreadContext&) { return Value{as_uint32(args[0]) * 2}; }; const auto host_foo_type = module.typesec[0]; @@ -287,11 +289,11 @@ TEST(execute_call, imported_functions_call_indirect) ASSERT_EQ(module.importsec.size(), 2); ASSERT_EQ(module.codesec.size(), 2); - constexpr auto sqr = [](Instance&, span args, int) { + constexpr auto sqr = [](Instance&, span args, ThreadContext&) { const auto x = as_uint32(args[0]); return Value{uint64_t{x} * uint64_t{x}}; }; - constexpr auto isqrt = [](Instance&, span args, int) { + constexpr auto isqrt = [](Instance&, span args, ThreadContext&) { const auto x = as_uint32(args[0]); return Value{(11 + uint64_t{x} / 11) / 2}; }; @@ -337,7 +339,8 @@ 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&, span args, int) -> ExecutionResult { + auto sub = [&instance1, func_idx]( + Instance&, span args, ThreadContext&) -> ExecutionResult { return fizzy::execute(*instance1, *func_idx, args); }; @@ -476,8 +479,10 @@ TEST(execute_call, call_max_depth) const auto module = parse(bin); auto instance = instantiate(module); - EXPECT_THAT(execute(*instance, 0, {}, 2048), Result(42)); - EXPECT_THAT(execute(*instance, 1, {}, 2048), Traps()); + ThreadContext thread_context; + thread_context.depth = 2048; + EXPECT_THAT(execute(*instance, 0, {}, thread_context), Result(42)); + EXPECT_THAT(execute(*instance, 1, {}, thread_context), Traps()); } // A regression test for incorrect number of arguments passed to a call. @@ -515,8 +520,9 @@ TEST(execute_call, call_imported_infinite_recursion) "0061736d010000000105016000017f020b01036d6f6403666f6f0000030201000a0601040010000b"); const auto module = parse(wasm); - auto host_foo = [](Instance& instance, span, int depth) -> ExecutionResult { - return execute(instance, 0, {}, depth + 1); + auto host_foo = [](Instance& instance, span, + ThreadContext& thread_context) -> ExecutionResult { + return execute(instance, 0, {}, thread_context); }; const auto host_foo_type = module.typesec[0]; diff --git a/test/unittests/execute_control_test.cpp b/test/unittests/execute_control_test.cpp index 7993f1730..4e24bf7be 100644 --- a/test/unittests/execute_control_test.cpp +++ b/test/unittests/execute_control_test.cpp @@ -665,7 +665,9 @@ TEST(execute_control, br_1_out_of_function_and_imported_function) "0a0d010b00034041010c010b41000b"); constexpr auto fake_imported_function = [](Instance&, span, - int) noexcept -> ExecutionResult { return Void; }; + ThreadContext&) noexcept -> ExecutionResult { + return Void; + }; const auto module = parse(bin); auto instance = instantiate(module, {{fake_imported_function, module.typesec[0]}}); diff --git a/test/unittests/execute_test.cpp b/test/unittests/execute_test.cpp index fef35dd18..4025a1cc3 100644 --- a/test/unittests/execute_test.cpp +++ b/test/unittests/execute_test.cpp @@ -776,7 +776,7 @@ TEST(execute, imported_function) const auto module = parse(wasm); ASSERT_EQ(module.typesec.size(), 1); - constexpr auto host_foo = [](Instance&, span args, int) { + constexpr auto host_foo = [](Instance&, span args, ThreadContext&) { return Value{as_uint32(args[0]) + as_uint32(args[1])}; }; @@ -796,10 +796,10 @@ TEST(execute, imported_two_functions) const auto module = parse(wasm); ASSERT_EQ(module.typesec.size(), 1); - constexpr auto host_foo1 = [](Instance&, span args, int) { + constexpr auto host_foo1 = [](Instance&, span args, ThreadContext&) { return Value{as_uint32(args[0]) + as_uint32(args[1])}; }; - constexpr auto host_foo2 = [](Instance&, span args, int) { + constexpr auto host_foo2 = [](Instance&, span args, ThreadContext&) { return Value{as_uint32(args[0]) * as_uint32(args[1])}; }; @@ -823,10 +823,10 @@ TEST(execute, imported_functions_and_regular_one) "0061736d0100000001070160027f7f017f021702036d6f6404666f6f310000036d6f6404666f6f320000030201" "000a0901070041aa80a8010b"); - constexpr auto host_foo1 = [](Instance&, span args, int) { + constexpr auto host_foo1 = [](Instance&, span args, ThreadContext&) { return Value{as_uint32(args[0]) + as_uint32(args[1])}; }; - constexpr auto host_foo2 = [](Instance&, span args, int) { + constexpr auto host_foo2 = [](Instance&, span args, ThreadContext&) { return Value{as_uint32(args[0]) * as_uint32(args[0])}; }; @@ -838,7 +838,7 @@ TEST(execute, imported_functions_and_regular_one) EXPECT_THAT(execute(*instance, 1, {20}), Result(400)); // check correct number of arguments is passed to host - constexpr auto count_args = [](Instance&, span args, int) { + constexpr auto count_args = [](Instance&, span args, ThreadContext&) { return Value{args.size()}; }; @@ -863,10 +863,10 @@ TEST(execute, imported_two_functions_different_type) "0061736d01000000010c0260027f7f017f60017e017e021702036d6f6404666f6f310000036d6f6404666f6f32" "0001030201010a0901070042aa80a8010b"); - constexpr auto host_foo1 = [](Instance&, span args, int) { + constexpr auto host_foo1 = [](Instance&, span args, ThreadContext&) { return Value{as_uint32(args[0]) + as_uint32(args[1])}; }; - constexpr auto host_foo2 = [](Instance&, span args, int) { + constexpr auto host_foo2 = [](Instance&, span args, ThreadContext&) { return Value{args[0].i64 * args[0].i64}; }; @@ -887,7 +887,7 @@ TEST(execute, imported_function_traps) */ const auto wasm = from_hex("0061736d0100000001070160027f7f017f020b01036d6f6403666f6f0000"); - constexpr auto host_foo = [](Instance&, span, int) -> ExecutionResult { + constexpr auto host_foo = [](Instance&, span, ThreadContext&) -> ExecutionResult { return Trap; }; diff --git a/test/unittests/instantiate_test.cpp b/test/unittests/instantiate_test.cpp index a31366f07..b7b01e216 100644 --- a/test/unittests/instantiate_test.cpp +++ b/test/unittests/instantiate_test.cpp @@ -17,7 +17,8 @@ namespace uint32_t call_table_func(Instance& instance, size_t idx) { const auto& elem = (*instance.table)[idx]; - const auto res = elem->function(instance, {}, 0); + ThreadContext thread_context; + const auto res = elem->function(instance, {}, thread_context); EXPECT_TRUE(res.has_value); return as_uint32(res.value); } @@ -31,7 +32,7 @@ TEST(instantiate, imported_functions) const auto bin = from_hex("0061736d0100000001060160017f017f020b01036d6f6403666f6f0000"); const auto module = parse(bin); - auto host_foo = [](Instance&, span, int) { return Trap; }; + auto host_foo = [](Instance&, span, ThreadContext&) { return Trap; }; auto instance = instantiate(module, {{host_foo, module.typesec[0]}}); ASSERT_EQ(instance->imported_functions.size(), 1); @@ -52,8 +53,8 @@ TEST(instantiate, imported_functions_multiple) "0061736d0100000001090260017f017f600000021702036d6f6404666f6f310000036d6f6404666f6f320001"); const auto module = parse(bin); - auto host_foo1 = [](Instance&, span, int) { return Trap; }; - auto host_foo2 = [](Instance&, span, int) { return Trap; }; + auto host_foo1 = [](Instance&, span, ThreadContext&) { return Trap; }; + auto host_foo2 = [](Instance&, span, ThreadContext&) { return Trap; }; auto instance = instantiate(module, {{host_foo1, module.typesec[0]}, {host_foo2, module.typesec[1]}}); @@ -88,7 +89,7 @@ TEST(instantiate, imported_function_wrong_type) const auto bin = from_hex("0061736d0100000001060160017f017f020b01036d6f6403666f6f0000"); const auto module = parse(bin); - auto host_foo = [](Instance&, span, int) { return Trap; }; + auto host_foo = [](Instance&, span, ThreadContext&) { return Trap; }; const auto host_foo_type = FuncType{{}, {}}; EXPECT_THROW_MESSAGE(instantiate(module, {{host_foo, host_foo_type}}), instantiate_error, @@ -654,7 +655,7 @@ TEST(instantiate, element_section_fills_imported_table) "0061736d010000000105016000017f020b01016d037461620170000403050400000000090f020041010b020001" "0041020b0202030a1504040041010b040041020b040041030b040041040b"); - auto f0 = [](Instance&, span, int) { return Value{0}; }; + auto f0 = [](Instance&, span, ThreadContext&) { return Value{0}; }; table_elements table(4); table[0] = ExternalFunction{f0, FuncType{{}, {ValType::i32}}}; @@ -682,7 +683,7 @@ TEST(instantiate, element_section_out_of_bounds_doesnt_change_imported_table) "0b0200000a0601040041010b"); Module module = parse(bin); - auto f0 = [](Instance&, span, int) { return Value{0}; }; + auto f0 = [](Instance&, span, ThreadContext&) { return Value{0}; }; table_elements table(3); table[0] = ExternalFunction{f0, FuncType{{}, {ValType::i32}}}; diff --git a/test/utils/fizzy_engine.cpp b/test/utils/fizzy_engine.cpp index 5d3abdcf4..11d93f52a 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, fizzy::span args, int) +fizzy::ExecutionResult env_adler32( + fizzy::Instance& instance, fizzy::span args, ThreadContext&) { assert(instance.memory != nullptr); const auto ret = fizzy::test::adler32( From 02bbfb2e45d3f3b43a51aca965cf730616d931c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 10 Sep 2020 10:48:13 +0200 Subject: [PATCH 6/9] Add stack space to ThreadContext --- lib/fizzy/instantiate.hpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/fizzy/instantiate.hpp b/lib/fizzy/instantiate.hpp index 062d0a587..df758a1b6 100644 --- a/lib/fizzy/instantiate.hpp +++ b/lib/fizzy/instantiate.hpp @@ -25,6 +25,17 @@ struct Instance; class ThreadContext { public: + static constexpr size_t N = 32; + + Value stack_space[N]; + + Value* free_space = stack_space; + + size_t space_left() const noexcept + { + return static_cast((stack_space + N) - free_space); + } + int depth = 0; void lock() noexcept { ++depth; } From ecda82a13040c8c5e74c05158c96eb3964218b48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 10 Sep 2020 12:31:50 +0200 Subject: [PATCH 7/9] Add better ThreadContextGuard --- lib/fizzy/execute.cpp | 25 ++++++++++++++++++++++--- test/unittests/execute_call_test.cpp | 1 + 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/lib/fizzy/execute.cpp b/lib/fizzy/execute.cpp index 859f19264..660703de6 100644 --- a/lib/fizzy/execute.cpp +++ b/lib/fizzy/execute.cpp @@ -10,7 +10,6 @@ #include #include #include -#include #include namespace fizzy @@ -461,6 +460,25 @@ __attribute__((no_sanitize("float-cast-overflow"))) inline constexpr float demot return static_cast(value); } +class ThreadContextGuard +{ + ThreadContext& m_thread_context; + Value* const m_free_space; + +public: + explicit ThreadContextGuard(ThreadContext& thread_context) noexcept + : m_thread_context{thread_context}, m_free_space{m_thread_context.free_space} + { + m_thread_context.lock(); + } + + ~ThreadContextGuard() noexcept + { + m_thread_context.unlock(); + m_thread_context.free_space = m_free_space; + } +}; + } // namespace ExecutionResult execute(Instance& instance, FuncIdx func_idx, span args, @@ -469,7 +487,7 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, span if (thread_context.depth > CallStackLimit) return Trap; - std::lock_guard guard{thread_context}; + ThreadContextGuard guard{thread_context}; if (func_idx < instance.imported_functions.size()) return instance.imported_functions[func_idx].function(instance, args, thread_context); @@ -593,7 +611,8 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, span if (expected_type != actual_type) goto trap; - if (!invoke_function(actual_type, called_func->function, instance, stack, thread_context)) + if (!invoke_function( + actual_type, called_func->function, instance, stack, thread_context)) goto trap; break; } diff --git a/test/unittests/execute_call_test.cpp b/test/unittests/execute_call_test.cpp index dc7b88600..d781d8d00 100644 --- a/test/unittests/execute_call_test.cpp +++ b/test/unittests/execute_call_test.cpp @@ -529,6 +529,7 @@ TEST(execute_call, call_imported_infinite_recursion) auto instance = instantiate(module, {{host_foo, host_foo_type}}); EXPECT_THAT(execute(*instance, 0, {}), Traps()); + EXPECT_THAT(execute(*instance, 1, {}), Traps()); } TEST(execute_call, call_indirect_imported_table_infinite_recursion) From 9dfe017d6150dea09d31712f134632ab510dc86f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 10 Sep 2020 18:17:43 +0200 Subject: [PATCH 8/9] Use thread stack stace --- lib/fizzy/execute.cpp | 4 +--- lib/fizzy/stack.hpp | 3 ++- test/unittests/span_test.cpp | 3 ++- test/unittests/stack_test.cpp | 27 ++++++++++++++++++++------- 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/lib/fizzy/execute.cpp b/lib/fizzy/execute.cpp index 660703de6..520a8311c 100644 --- a/lib/fizzy/execute.cpp +++ b/lib/fizzy/execute.cpp @@ -498,10 +498,8 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, span const auto& code = instance.module.codesec[code_idx]; auto* const memory = instance.memory.get(); - constexpr auto stack_space_size = 128 / sizeof(Value); - Value stack_space[stack_space_size]; OperandStack stack(args, code.local_count, static_cast(code.max_stack_height), - stack_space, std::size(stack_space)); + thread_context.free_space, thread_context.space_left()); const Instr* pc = code.instructions.data(); const uint8_t* immediates = code.immediates.data(); diff --git a/lib/fizzy/stack.hpp b/lib/fizzy/stack.hpp index 76aee7d20..519996ac6 100644 --- a/lib/fizzy/stack.hpp +++ b/lib/fizzy/stack.hpp @@ -72,7 +72,7 @@ class OperandStack /// Based on @p max_stack_height decides if to use small pre-allocated storage or allocate /// large storage. /// Sets the top item pointer to below the stack bottom. - OperandStack(span args, size_t num_locals, size_t max_stack_height, Value* external_storage, size_t external_storage_size) + OperandStack(span args, size_t num_locals, size_t max_stack_height, Value*& external_storage, size_t external_storage_size) { const auto num_args = args.size(); const auto storage_size_required = num_args + num_locals + max_stack_height; @@ -81,6 +81,7 @@ class OperandStack if (storage_size_required <= external_storage_size) { storage = external_storage; + external_storage += storage_size_required; } else { diff --git a/test/unittests/span_test.cpp b/test/unittests/span_test.cpp index 7399594b2..3b3380baf 100644 --- a/test/unittests/span_test.cpp +++ b/test/unittests/span_test.cpp @@ -50,7 +50,8 @@ TEST(span, array) TEST(span, stack) { - OperandStack stack({}, 0, 4, nullptr, 0); + Value* external_storage = nullptr; + OperandStack stack({}, 0, 4, external_storage, 0); stack.push(10); stack.push(11); stack.push(12); diff --git a/test/unittests/stack_test.cpp b/test/unittests/stack_test.cpp index decca8bb1..640369845 100644 --- a/test/unittests/stack_test.cpp +++ b/test/unittests/stack_test.cpp @@ -115,7 +115,8 @@ TEST(stack, struct_item) TEST(operand_stack, construct) { - OperandStack stack({}, 0, 0, nullptr, 0); + fizzy::Value* external_storage = nullptr; + OperandStack stack({}, 0, 0, external_storage, 0); EXPECT_EQ(stack.size(), 0); stack.shrink(0); EXPECT_EQ(stack.size(), 0); @@ -123,7 +124,9 @@ TEST(operand_stack, construct) TEST(operand_stack, top) { - OperandStack stack({}, 0, 1, nullptr, 0); + fizzy::Value* external_storage = nullptr; + + OperandStack stack({}, 0, 1, external_storage, 0); EXPECT_EQ(stack.size(), 0); stack.push(1); @@ -147,7 +150,9 @@ TEST(operand_stack, top) TEST(operand_stack, small) { - OperandStack stack({}, 0, 3, nullptr, 0); + fizzy::Value* external_storage = nullptr; + + OperandStack stack({}, 0, 3, external_storage, 0); EXPECT_EQ(stack.size(), 0); stack.push(1); @@ -175,8 +180,10 @@ TEST(operand_stack, small) TEST(operand_stack, large) { + fizzy::Value* external_storage = nullptr; + constexpr auto max_height = 33; - OperandStack stack({}, 0, max_height, nullptr, 0); + OperandStack stack({}, 0, max_height, external_storage, 0); for (unsigned i = 0; i < max_height; ++i) stack.push(i); @@ -189,8 +196,10 @@ TEST(operand_stack, large) TEST(operand_stack, shrink) { + fizzy::Value* external_storage = nullptr; + constexpr auto max_height = 60; - OperandStack stack({}, 0, max_height, nullptr, 0); + OperandStack stack({}, 0, max_height, external_storage, 0); for (unsigned i = 0; i < max_height; ++i) stack.push(i); @@ -206,7 +215,9 @@ TEST(operand_stack, shrink) TEST(operand_stack, rbegin_rend) { - OperandStack stack({}, 0, 3, nullptr, 0); + fizzy::Value* external_storage = nullptr; + + OperandStack stack({}, 0, 3, external_storage, 0); EXPECT_EQ(stack.rbegin(), stack.rend()); stack.push(1); @@ -219,7 +230,9 @@ TEST(operand_stack, rbegin_rend) TEST(operand_stack, to_vector) { - OperandStack stack({}, 0, 3, nullptr, 0); + fizzy::Value* external_storage = nullptr; + + OperandStack stack({}, 0, 3, external_storage, 0); EXPECT_THAT(std::vector(stack.rbegin(), stack.rend()), IsEmpty()); stack.push(1); From 0e7bc270d61e3f8e9f21652f7a238625662258bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 10 Sep 2020 18:27:18 +0200 Subject: [PATCH 9/9] Increase thread stack space size --- lib/fizzy/instantiate.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fizzy/instantiate.hpp b/lib/fizzy/instantiate.hpp index df758a1b6..a431bb142 100644 --- a/lib/fizzy/instantiate.hpp +++ b/lib/fizzy/instantiate.hpp @@ -25,7 +25,7 @@ struct Instance; class ThreadContext { public: - static constexpr size_t N = 32; + static constexpr size_t N = 512; Value stack_space[N];