Skip to content

Commit

Permalink
refactor: represent table elements as instance pointer + function index
Browse files Browse the repository at this point in the history
  • Loading branch information
gumb0 committed Oct 21, 2020
1 parent c179d1b commit 98a8d93
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 58 deletions.
21 changes: 7 additions & 14 deletions lib/fizzy/execute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -481,15 +481,14 @@ void branch(const Code& code, OperandStack& stack, const Instr*& pc, const uint8
stack.drop(stack_drop);
}

template <class F>
inline bool invoke_function(
const FuncType& func_type, const F& func, Instance& instance, OperandStack& stack, int depth)
inline bool invoke_function(const FuncType& func_type, uint32_t func_idx, Instance& instance,
OperandStack& stack, int depth)
{
const auto num_args = func_type.inputs.size();
assert(stack.size() >= num_args);
const auto call_args = stack.rend() - num_args;

const auto ret = func(instance, call_args, depth + 1);
const auto ret = execute(instance, func_idx, call_args, depth + 1);
// Bubble up traps
if (ret.trapped)
return false;
Expand All @@ -507,14 +506,6 @@ inline bool invoke_function(
return true;
}

inline bool invoke_function(const FuncType& func_type, uint32_t func_idx, Instance& instance,
OperandStack& stack, int depth)
{
const auto func = [func_idx](Instance& _instance, const Value* args, int _depth) {
return execute(_instance, func_idx, args, _depth);
};
return invoke_function(func_type, func, instance, stack, depth);
}
} // namespace

ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, int depth)
Expand Down Expand Up @@ -638,12 +629,14 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args,
goto trap;

// check actual type against expected type
const auto& actual_type = called_func->type;
const auto& actual_type =
called_func->instance->module->get_function_type(called_func->func_idx);
const auto& expected_type = instance.module->typesec[expected_type_idx];
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->func_idx, *called_func->instance, stack, depth))
goto trap;
break;
}
Expand Down
14 changes: 3 additions & 11 deletions lib/fizzy/instantiate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -330,11 +330,7 @@ std::unique_ptr<Instance> instantiate(std::unique_ptr<const Module> module,
auto it_table = instance->table->begin() + elementsec_offsets[i];
for (const auto idx : instance->module->elementsec[i].init)
{
auto func = [idx, &instance_ref = *instance](fizzy::Instance&, const Value* args,
int depth) { return execute(instance_ref, idx, args, depth); };

*it_table++ =
ExternalFunction{std::move(func), instance->module->get_function_type(idx)};
*it_table++ = {instance.get(), idx, {}};
}
}

Expand All @@ -360,12 +356,8 @@ std::unique_ptr<Instance> instantiate(std::unique_ptr<const Module> module,
auto it_table = shared_instance->table->begin() + elementsec_offsets[i];
for ([[maybe_unused]] auto _ : shared_instance->module->elementsec[i].init)
{
// Wrap the function with the lambda capturing shared instance
auto& table_function = (*it_table)->function;
table_function = [shared_instance, func = std::move(table_function)](
fizzy::Instance& _instance, const Value* args,
int depth) { return func(_instance, args, depth); };
++it_table;
// Capture shared instance in table element.
(*it_table++)->shared_instance = shared_instance;
}
}
}
Expand Down
13 changes: 12 additions & 1 deletion lib/fizzy/instantiate.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,18 @@ struct ExternalFunction
FuncType type;
};

using table_elements = std::vector<std::optional<ExternalFunction>>;
// Table element, which references a function in any instance.
struct TableElement
{
Instance* instance;
FuncIdx func_idx;
// This pointer is empty most of the time and is used only to keep instance alive in one edge
// case, when start function traps, but instantiate has already modified some elements of a
// shared (imported) table.
std::shared_ptr<Instance> shared_instance;
};

using table_elements = std::vector<std::optional<TableElement>>;
using table_ptr = std::unique_ptr<table_elements, void (*)(table_elements*)>;

struct ExternalTable
Expand Down
8 changes: 6 additions & 2 deletions test/unittests/api_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -412,8 +412,12 @@ 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));
ASSERT_TRUE((*opt_table->table)[0].has_value());
EXPECT_EQ((*opt_table->table)[0]->instance, instance.get());
EXPECT_EQ((*opt_table->table)[0]->func_idx, 1);
ASSERT_TRUE((*opt_table->table)[1].has_value());
EXPECT_EQ((*opt_table->table)[1]->instance, instance.get());
EXPECT_EQ((*opt_table->table)[1]->func_idx, 0);
EXPECT_EQ(opt_table->limits.min, 2);
ASSERT_TRUE(opt_table->limits.max.has_value());
EXPECT_EQ(opt_table->limits.max, 20);
Expand Down
33 changes: 20 additions & 13 deletions test/unittests/execute_call_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,25 @@ TEST(execute_call, call_indirect_with_argument)

TEST(execute_call, call_indirect_imported_table)
{
/* wat2wasm
(module
(table 5 20 anyfunc)
(export "t" (table 0))
(elem (i32.const 0) $f3 $f2 $f1 $f4 $f5)
(func $f1 (result i32) (i32.const 1))
(func $f2 (result i32) (i32.const 2))
(func $f3 (result i32) (i32.const 3))
(func $f4 (result i64) (i64.const 4))
(func $f5 (result i32) unreachable)
)
*/
const auto bin_exported_table = from_hex(
"0061736d010000000109026000017f6000017e03060500000001000405017001051407050101740100090b0100"
"41000b0502010003040a1905040041010b040041020b040041030b040042040b0300000b");
auto instance_exported_table = instantiate(parse(bin_exported_table));
auto table = find_exported_table(*instance_exported_table, "t");
ASSERT_TRUE(table.has_value());

/* wat2wasm
(module
(type $out_i32 (func (result i32)))
Expand All @@ -171,19 +190,7 @@ TEST(execute_call, call_indirect_imported_table)
"0061736d01000000010a026000017f60017f017f020a01016d01740170010514030201010a0901070020001100"
"000b");

auto f1 = [](Instance&, const Value*, int) { return Value{1}; };
auto f2 = [](Instance&, const Value*, int) { return Value{2}; };
auto f3 = [](Instance&, const Value*, int) { return Value{3}; };
auto f4 = [](Instance&, const Value*, int) { return Value{4}; };
auto f5 = [](Instance&, const Value*, int) { return Trap; };

auto out_i32 = FuncType{{}, {ValType::i32}};
auto out_i64 = FuncType{{}, {ValType::i64}};

table_elements table{
{{f3, out_i32}}, {{f2, out_i32}}, {{f1, out_i32}}, {{f4, out_i64}}, {{f5, out_i32}}};

auto instance = instantiate(parse(bin), {}, {{&table, {5, 20}}});
auto instance = instantiate(parse(bin), {}, {*table});

for (const auto param : {0u, 1u, 2u})
{
Expand Down
57 changes: 40 additions & 17 deletions test/unittests/instantiate_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace
uint32_t call_table_func(Instance& instance, size_t idx)
{
const auto& elem = (*instance.table)[idx];
const auto res = elem->function(instance, {}, 0);
const auto res = execute(*elem->instance, elem->func_idx, {}, 0);
EXPECT_TRUE(res.has_value);
return as_uint32(res.value);
}
Expand Down Expand Up @@ -622,6 +622,21 @@ TEST(instantiate, element_section_offset_too_large)

TEST(instantiate, element_section_fills_imported_table)
{
/* wat2wasm
(module
(table 4 anyfunc)
(export "t" (table 0))
(elem (i32.const 0) $f0)
(func $f0 (result i32) (i32.const 0))
)
*/
const auto bin_exported_table = from_hex(
"0061736d010000000105016000017f03020100040401700004070501017401000907010041000b01000a060104"
"0041000b");
auto instance_exported_table = instantiate(parse(bin_exported_table));
auto table = find_exported_table(*instance_exported_table, "t");
ASSERT_TRUE(table.has_value());

/* wat2wasm
(module
(table (import "m" "tab") 4 funcref)
Expand All @@ -637,11 +652,7 @@ TEST(instantiate, element_section_fills_imported_table)
"0061736d010000000105016000017f020b01016d037461620170000403050400000000090f020041010b020001"
"0041020b0202030a1504040041010b040041020b040041030b040041040b");

auto f0 = [](Instance&, const Value*, int) { return Value{0}; };

table_elements table(4);
table[0] = ExternalFunction{f0, FuncType{{}, {ValType::i32}}};
auto instance = instantiate(parse(bin), {}, {{&table, {4, std::nullopt}}});
auto instance = instantiate(parse(bin), {}, {*table});

ASSERT_EQ(instance->table->size(), 4);
EXPECT_EQ(call_table_func(*instance, 0), 0);
Expand All @@ -652,6 +663,21 @@ TEST(instantiate, element_section_fills_imported_table)

TEST(instantiate, element_section_out_of_bounds_doesnt_change_imported_table)
{
/* wat2wasm
(module
(table 3 anyfunc)
(export "t" (table 0))
(elem (i32.const 0) $f0)
(func $f0 (result i32) (i32.const 0))
)
*/
const auto bin_exported_table = from_hex(
"0061736d010000000105016000017f03020100040401700003070501017401000907010041000b01000a060104"
"0041000b");
auto instance_exported_table = instantiate(parse(bin_exported_table));
auto table = find_exported_table(*instance_exported_table, "t");
ASSERT_TRUE(table.has_value());

/* wat2wasm
(module
(table (import "m" "tab") 3 funcref)
Expand All @@ -664,18 +690,15 @@ TEST(instantiate, element_section_out_of_bounds_doesnt_change_imported_table)
"0061736d010000000105016000017f020b01016d037461620170000303020100090f020041000b020000004102"
"0b0200000a0601040041010b");

auto f0 = [](Instance&, const Value*, int) { return Value{0}; };

table_elements table(3);
table[0] = ExternalFunction{f0, FuncType{{}, {ValType::i32}}};

EXPECT_THROW_MESSAGE(instantiate(parse(bin), {}, {{&table, {3, std::nullopt}}}),
instantiate_error, "element segment is out of table bounds");
EXPECT_THROW_MESSAGE(instantiate(parse(bin), {}, {*table}), instantiate_error,
"element segment is out of table bounds");

ASSERT_EQ(table.size(), 3);
EXPECT_EQ(*table[0]->function.target<decltype(f0)>(), f0);
EXPECT_FALSE(table[1].has_value());
EXPECT_FALSE(table[2].has_value());
const auto& table_elements = *table->table;
ASSERT_EQ(table_elements.size(), 3);
EXPECT_EQ(table_elements[0]->instance, instance_exported_table.get());
EXPECT_EQ(table_elements[0]->func_idx, 0);
EXPECT_FALSE(table_elements[1].has_value());
EXPECT_FALSE(table_elements[2].has_value());
}

TEST(instantiate, data_section)
Expand Down

0 comments on commit 98a8d93

Please sign in to comment.