Skip to content

Pass arguments as const Value* #552

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 2, 2020
Merged

Pass arguments as const Value* #552

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f3e3f8c
Decouple OperandStack from span<const Value>
chfast Sep 23, 2020
7b9ef70
Use function type to get the number of args
chfast Sep 23, 2020
b903ef9
test: Explicitly convert single 0 arg to Value
chfast Oct 1, 2020
4816192
Pass arguments as const Value*
chfast Sep 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions lib/fizzy/execute.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,29 +454,30 @@ inline bool invoke_function(const FuncType& func_type, uint32_t func_idx, Instan
OperandStack& stack, int depth) noexcept
{
const auto func = [func_idx](Instance& _instance, span<const Value> args, int _depth) noexcept {
return execute(_instance, func_idx, args, _depth);
return execute(_instance, func_idx, args.data(), _depth);
};
return invoke_function(func_type, func, instance, stack, depth);
}
} // namespace

ExecutionResult execute(
Instance& instance, FuncIdx func_idx, span<const Value> args, int depth) noexcept
ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, int depth) noexcept
{
assert(depth >= 0);
if (depth > CallStackLimit)
return Trap;

assert(args.size() == instance.module.get_function_type(func_idx).inputs.size());
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, func_type.inputs.size()}, depth);

const auto& code = instance.module.get_code(func_idx);
auto* const memory = instance.memory.get();

OperandStack stack(args, code.local_count, static_cast<size_t>(code.max_stack_height));
OperandStack stack(args, func_type.inputs.size(), code.local_count,
static_cast<size_t>(code.max_stack_height));

const Instr* pc = code.instructions.data();
const uint8_t* immediates = code.immediates.data();
Expand Down
5 changes: 3 additions & 2 deletions lib/fizzy/execute.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,12 @@ constexpr ExecutionResult Trap{false};

// Execute a function on an instance.
ExecutionResult execute(
Instance& instance, FuncIdx func_idx, span<const Value> args, int depth = 0) noexcept;
Instance& instance, FuncIdx func_idx, const Value* args, int depth = 0) noexcept;

inline ExecutionResult execute(
Instance& instance, FuncIdx func_idx, std::initializer_list<Value> args) noexcept
{
return execute(instance, func_idx, span<const Value>{args});
assert(args.size() == instance.module.get_function_type(func_idx).inputs.size());
return execute(instance, func_idx, args.begin());
}
} // namespace fizzy
4 changes: 2 additions & 2 deletions lib/fizzy/instantiate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ std::unique_ptr<Instance> instantiate(Module module,
for (const auto idx : instance->module.elementsec[i].init)
{
auto func = [idx, &instance_ref = *instance](fizzy::Instance&, span<const Value> args,
int depth) { return execute(instance_ref, idx, args, depth); };
int depth) { return execute(instance_ref, idx, args.data(), depth); };

*it_table++ =
ExternalFunction{std::move(func), instance->module.get_function_type(idx)};
Expand Down Expand Up @@ -433,7 +433,7 @@ std::optional<ExternalFunction> find_exported_function(Instance& instance, std::

const auto idx = *opt_index;
auto func = [idx, &instance](fizzy::Instance&, span<const Value> args, int depth) {
return execute(instance, idx, args, depth);
return execute(instance, idx, args.data(), depth);
};

return ExternalFunction{std::move(func), instance.module.get_function_type(idx)};
Expand Down
10 changes: 5 additions & 5 deletions lib/fizzy/stack.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

#pragma once

#include "cxx20/span.hpp"
#include "value.hpp"
#include <algorithm>
#include <cassert>
#include <cstdint>
#include <memory>
Expand Down Expand Up @@ -96,9 +96,9 @@ class OperandStack
/// after the arguments.
/// @param max_stack_height The maximum operand stack height in the function. This excludes
/// args and num_local_variables.
OperandStack(span<const Value> args, size_t num_local_variables, size_t max_stack_height)
OperandStack(
const Value* args, size_t num_args, size_t num_local_variables, size_t max_stack_height)
{
const auto num_args = args.size();
const auto storage_size_required = num_args + num_local_variables + max_stack_height;

if (storage_size_required <= small_storage_size)
Expand All @@ -114,8 +114,8 @@ class OperandStack
m_bottom = m_locals + num_args + num_local_variables;
m_top = m_bottom - 1;

std::copy(std::begin(args), std::end(args), m_locals);
std::fill_n(m_locals + num_args, num_local_variables, 0);
const auto local_variables = std::copy_n(args, num_args, m_locals);
std::fill_n(local_variables, num_local_variables, 0);
}

OperandStack(const OperandStack&) = delete;
Expand Down
3 changes: 2 additions & 1 deletion test/spectests/spectests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,8 @@ class test_runner
args.push_back(*arg_value);
}

return fizzy::execute(*instance, *func_idx, args);
assert(args.size() == instance->module.get_function_type(*func_idx).inputs.size());
return fizzy::execute(*instance, *func_idx, args.data());
}

bool check_integer_result(fizzy::Value actual_value, const json& expected)
Expand Down
3 changes: 1 addition & 2 deletions test/testfloat/testfloat.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,7 @@ bool check(const FunctionDescription& func, fizzy::Instance& instance, const uin
for (size_t i = 0; i < func.num_arguments; ++i)
args[i] = make_value(func.param_types[i], inputs[i]);

const auto r = fizzy::execute(
instance, func.idx, fizzy::span<const fizzy::Value>(args, func.num_arguments));
const auto r = fizzy::execute(instance, func.idx, args);

if (func.options == Options::TrapIsInvalidOperation)
{
Expand Down
12 changes: 6 additions & 6 deletions test/unittests/api_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ TEST(api, resolve_imported_functions)
module, external_functions, {}, {}, std::vector<ExternalGlobal>(external_globals));

EXPECT_THAT(execute(*instance, 0, {}), Result(0));
EXPECT_THAT(execute(*instance, 1, {0}), Result(1));
EXPECT_THAT(execute(*instance, 2, {0}), Result(2));
EXPECT_THAT(execute(*instance, 1, {Value{0}}), Result(1));
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When passing single 0, clang thinks it should go as nullptr. This is a workaround.

EXPECT_THAT(execute(*instance, 2, {Value{0}}), Result(2));
EXPECT_THAT(execute(*instance, 3, {0, 0}), Result());


Expand All @@ -119,8 +119,8 @@ TEST(api, resolve_imported_functions)
std::vector<ExternalGlobal>(external_globals));

EXPECT_THAT(execute(*instance_reordered, 0, {}), Result(0));
EXPECT_THAT(execute(*instance_reordered, 1, {0}), Result(1));
EXPECT_THAT(execute(*instance_reordered, 2, {0}), Result(2));
EXPECT_THAT(execute(*instance_reordered, 1, {Value{0}}), Result(1));
EXPECT_THAT(execute(*instance_reordered, 2, {Value{0}}), Result(2));
EXPECT_THAT(execute(*instance_reordered, 3, {0, 0}), Result());


Expand All @@ -141,8 +141,8 @@ TEST(api, resolve_imported_functions)
module, external_functions_extra, {}, {}, std::vector<ExternalGlobal>(external_globals));

EXPECT_THAT(execute(*instance_extra, 0, {}), Result(0));
EXPECT_THAT(execute(*instance_extra, 1, {0}), Result(1));
EXPECT_THAT(execute(*instance_extra, 2, {0}), Result(2));
EXPECT_THAT(execute(*instance_extra, 1, {Value{0}}), Result(1));
EXPECT_THAT(execute(*instance_extra, 2, {Value{0}}), Result(2));
EXPECT_THAT(execute(*instance_extra, 3, {0, 0}), Result());


Expand Down
2 changes: 1 addition & 1 deletion test/unittests/execute_call_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ TEST(execute_call, imported_function_from_another_module)
ASSERT_TRUE(func_idx.has_value());

auto sub = [&instance1, func_idx](Instance&, span<const Value> args, int) -> ExecutionResult {
return fizzy::execute(*instance1, *func_idx, args);
return fizzy::execute(*instance1, *func_idx, args.data());
};

auto instance2 = instantiate(module2, {{sub, module1.typesec[0]}});
Expand Down
8 changes: 4 additions & 4 deletions test/unittests/execute_floating_point_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1893,7 +1893,7 @@ TEST(execute_floating_point, f32_load_overflow)
auto instance = instantiate(parse(wasm));

// Offset is 0x7fffffff + 0 => 0x7fffffff
EXPECT_THAT(execute(*instance, 0, {0}), Traps());
EXPECT_THAT(execute(*instance, 0, {Value{0}}), Traps());
// Offset is 0x7fffffff + 0x80000000 => 0xffffffff
EXPECT_THAT(execute(*instance, 0, {0x80000000}), Traps());
// Offset is 0x7fffffff + 0x80000001 => 0x100000000
Expand Down Expand Up @@ -1966,7 +1966,7 @@ TEST(execute_floating_point, f64_load_overflow)
auto instance = instantiate(parse(wasm));

// Offset is 0x7fffffff + 0 => 0x7fffffff
EXPECT_THAT(execute(*instance, 0, {0}), Traps());
EXPECT_THAT(execute(*instance, 0, {Value{0}}), Traps());
// Offset is 0x7fffffff + 0x80000000 => 0xffffffff
EXPECT_THAT(execute(*instance, 0, {0x80000000}), Traps());
// Offset is 0x7fffffff + 0x80000001 => 0x100000000
Expand Down Expand Up @@ -2043,7 +2043,7 @@ TEST(execute_floating_point, f32_store_overflow)
auto instance = instantiate(parse(wasm));

// Offset is 0x7fffffff + 0 => 0x7fffffff
EXPECT_THAT(execute(*instance, 0, {0}), Traps());
EXPECT_THAT(execute(*instance, 0, {Value{0}}), Traps());
// Offset is 0x7fffffff + 0x80000000 => 0xffffffff
EXPECT_THAT(execute(*instance, 0, {0x80000000}), Traps());
// Offset is 0x7fffffff + 0x80000001 => 0x100000000
Expand Down Expand Up @@ -2120,7 +2120,7 @@ TEST(execute_floating_point, f64_store_overflow)
auto instance = instantiate(parse(wasm));

// Offset is 0x7fffffff + 0 => 0x7fffffff
EXPECT_THAT(execute(*instance, 0, {0}), Traps());
EXPECT_THAT(execute(*instance, 0, {Value{0}}), Traps());
// Offset is 0x7fffffff + 0x80000000 => 0xffffffff
EXPECT_THAT(execute(*instance, 0, {0x80000000}), Traps());
// Offset is 0x7fffffff + 0x80000001 => 0x100000000
Expand Down
10 changes: 5 additions & 5 deletions test/unittests/execute_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ TEST(execute, i32_load_overflow)
auto instance = instantiate(parse(wasm));

// Offset is 0x7fffffff + 0 => 0x7fffffff
EXPECT_THAT(execute(*instance, 0, {0}), Traps());
EXPECT_THAT(execute(*instance, 0, {Value{0}}), Traps());
// Offset is 0x7fffffff + 0x80000000 => 0xffffffff
EXPECT_THAT(execute(*instance, 0, {0x80000000}), Traps());
// Offset is 0x7fffffff + 0x80000001 => 0x100000000
Expand All @@ -362,7 +362,7 @@ TEST(execute, i64_load_overflow)
auto instance = instantiate(parse(wasm));

// Offset is 0x7fffffff + 0 => 0x7fffffff
EXPECT_THAT(execute(*instance, 0, {0}), Traps());
EXPECT_THAT(execute(*instance, 0, {Value{0}}), Traps());
// Offset is 0x7fffffff + 0x80000000 => 0xffffffff
EXPECT_THAT(execute(*instance, 0, {0x80000000}), Traps());
// Offset is 0x7fffffff + 0x80000001 => 0x100000000
Expand Down Expand Up @@ -486,7 +486,7 @@ TEST(execute, i32_store_overflow)
auto instance = instantiate(parse(wasm));

// Offset is 0x7fffffff + 0 => 0x7fffffff
EXPECT_THAT(execute(*instance, 0, {0}), Traps());
EXPECT_THAT(execute(*instance, 0, {Value{0}}), Traps());
// Offset is 0x7fffffff + 0x80000000 => 0xffffffff
EXPECT_THAT(execute(*instance, 0, {0x80000000}), Traps());
// Offset is 0x7fffffff + 0x80000001 => 0x100000000
Expand All @@ -510,7 +510,7 @@ TEST(execute, i64_store_overflow)
auto instance = instantiate(parse(wasm));

// Offset is 0x7fffffff + 0 => 0x7fffffff
EXPECT_THAT(execute(*instance, 0, {0}), Traps());
EXPECT_THAT(execute(*instance, 0, {Value{0}}), Traps());
// Offset is 0x7fffffff + 0x80000000 => 0xffffffff
EXPECT_THAT(execute(*instance, 0, {0x80000000}), Traps());
// Offset is 0x7fffffff + 0x80000001 => 0x100000000
Expand Down Expand Up @@ -980,7 +980,7 @@ TEST(execute, reuse_args)

const std::vector<Value> args{20, 3};
const auto expected = args[0].i64 % (args[0].i64 / args[1].i64);
EXPECT_THAT(execute(*instance, 0, args), Result(expected));
EXPECT_THAT(execute(*instance, 0, args.data()), Result(expected));
EXPECT_THAT(args[0].i64, 20);
EXPECT_THAT(args[1].i64, 3);

Expand Down
2 changes: 1 addition & 1 deletion test/unittests/span_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ TEST(span, array)

TEST(span, stack)
{
OperandStack stack({}, 0, 4);
OperandStack stack(nullptr, 0, 0, 4);
stack.push(10);
stack.push(11);
stack.push(12);
Expand Down
18 changes: 9 additions & 9 deletions test/unittests/stack_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,13 @@ TEST(stack, struct_item)

TEST(operand_stack, construct)
{
OperandStack stack({}, 0, 0);
OperandStack stack(nullptr, 0, 0, 0);
EXPECT_EQ(stack.size(), 0);
}

TEST(operand_stack, top)
{
OperandStack stack({}, 0, 1);
OperandStack stack(nullptr, 0, 0, 1);
EXPECT_EQ(stack.size(), 0);

stack.push(1);
Expand Down Expand Up @@ -158,7 +158,7 @@ TEST(operand_stack, top)

TEST(operand_stack, small)
{
OperandStack stack({}, 0, 3);
OperandStack stack(nullptr, 0, 0, 3);
ASSERT_LT(address_diff(&stack, stack.rbegin()), 100) << "not allocated on the system stack";

EXPECT_EQ(stack.size(), 0);
Expand Down Expand Up @@ -189,7 +189,7 @@ TEST(operand_stack, small)
TEST(operand_stack, small_with_locals)
{
const fizzy::Value args[] = {0xa1, 0xa2};
OperandStack stack(args, 3, 1);
OperandStack stack(args, std::size(args), 3, 1);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is example where std::size() is better than sizeof.

ASSERT_LT(address_diff(&stack, stack.rbegin()), 100) << "not allocated on the system stack";

EXPECT_EQ(stack.size(), 0);
Expand Down Expand Up @@ -229,7 +229,7 @@ TEST(operand_stack, small_with_locals)
TEST(operand_stack, large)
{
constexpr auto max_height = 33;
OperandStack stack({}, 0, max_height);
OperandStack stack(nullptr, 0, 0, max_height);
ASSERT_GT(address_diff(&stack, stack.rbegin()), 100) << "not allocated on the heap";

for (unsigned i = 0; i < max_height; ++i)
Expand All @@ -247,7 +247,7 @@ TEST(operand_stack, large_with_locals)
constexpr auto max_height = 33;
constexpr auto num_locals = 5;
constexpr auto num_args = std::size(args);
OperandStack stack(args, num_locals, max_height);
OperandStack stack(args, num_args, num_locals, max_height);
ASSERT_GT(address_diff(&stack, stack.rbegin()), 100) << "not allocated on the heap";

for (unsigned i = 0; i < max_height; ++i)
Expand Down Expand Up @@ -279,7 +279,7 @@ TEST(operand_stack, large_with_locals)

TEST(operand_stack, rbegin_rend)
{
OperandStack stack({}, 0, 3);
OperandStack stack(nullptr, 0, 0, 3);
EXPECT_EQ(stack.rbegin(), stack.rend());

stack.push(1);
Expand All @@ -293,7 +293,7 @@ TEST(operand_stack, rbegin_rend)
TEST(operand_stack, rbegin_rend_locals)
{
const fizzy::Value args[] = {0xa1};
OperandStack stack(args, 4, 2);
OperandStack stack(args, std::size(args), 4, 2);
EXPECT_EQ(stack.rbegin(), stack.rend());

stack.push(1);
Expand All @@ -313,7 +313,7 @@ TEST(operand_stack, rbegin_rend_locals)

TEST(operand_stack, to_vector)
{
OperandStack stack({}, 0, 3);
OperandStack stack(nullptr, 0, 0, 3);
EXPECT_THAT(std::vector(stack.rbegin(), stack.rend()), IsEmpty());

stack.push(1);
Expand Down
5 changes: 3 additions & 2 deletions test/utils/fizzy_engine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,9 @@ WasmEngine::Result FizzyEngine::execute(
{
static_assert(sizeof(uint64_t) == sizeof(Value));
const auto first_arg = reinterpret_cast<const Value*>(args.data());
const auto status = fizzy::execute(
*m_instance, static_cast<uint32_t>(func_ref), span<const Value>(first_arg, args.size()));
assert(args.size() ==
m_instance->module.get_function_type(static_cast<uint32_t>(func_ref)).inputs.size());
const auto status = fizzy::execute(*m_instance, static_cast<uint32_t>(func_ref), first_arg);
if (status.trapped)
return {true, std::nullopt};
else if (status.has_value)
Expand Down