Skip to content
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

Make hard memory limit configurable #515

Merged
merged 4 commits into from
Sep 4, 2020
+212 −29
Merged

Make hard memory limit configurable #515

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
75e629a
Configure hard memory limit in instantiate
gumb0 Sep 3, 2020
7ac48ee
test: Set hard memory limit to 4 Gb in spectests runner
gumb0 Sep 2, 2020
a8cb9b8
ci: update spectest expectations
gumb0 Sep 2, 2020
696a9f2
test: custom memory hard limit
gumb0 Sep 3, 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
4 changes: 2 additions & 2 deletions circle.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,10 @@ commands:
default: false
expected_passed:
type: integer
default: 18899
default: 18900
expected_failed:
type: integer
default: 3
default: 2
expected_skipped:
type: integer
default: 477
Expand Down
38 changes: 22 additions & 16 deletions lib/fizzy/execute.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// SPDX-License-Identifier: Apache-2.0

#include "execute.hpp"
#include "limits.hpp"
#include "module.hpp"
#include "stack.hpp"
#include "trunc_boundaries.hpp"
Expand Down Expand Up @@ -172,7 +171,7 @@ std::tuple<table_ptr, Limits> allocate_table(
}

std::tuple<bytes_ptr, Limits> allocate_memory(const std::vector<Memory>& module_memories,
const std::vector<ExternalMemory>& imported_memories)
const std::vector<ExternalMemory>& imported_memories, uint32_t memory_pages_limit)
{
static const auto bytes_delete = [](bytes* b) noexcept { delete b; };
static const auto null_delete = [](bytes*) noexcept {};
Expand All @@ -185,11 +184,11 @@ std::tuple<bytes_ptr, Limits> allocate_memory(const std::vector<Memory>& module_
const auto memory_max = module_memories[0].limits.max;

// TODO: better error handling
if ((memory_min > MemoryPagesLimit) ||
(memory_max.has_value() && *memory_max > MemoryPagesLimit))
if ((memory_min > memory_pages_limit) ||
(memory_max.has_value() && *memory_max > memory_pages_limit))
{
throw instantiate_error{"cannot exceed hard memory limit of " +
std::to_string(MemoryPagesLimit * PageSize) + " bytes"};
std::to_string(memory_pages_limit * PageSize) + " bytes"};
}

// NOTE: fill it with zeroes
Expand All @@ -202,11 +201,11 @@ std::tuple<bytes_ptr, Limits> allocate_memory(const std::vector<Memory>& module_
const auto memory_max = imported_memories[0].limits.max;

// TODO: better error handling
if ((memory_min > MemoryPagesLimit) ||
(memory_max.has_value() && *memory_max > MemoryPagesLimit))
if ((memory_min > memory_pages_limit) ||
(memory_max.has_value() && *memory_max > memory_pages_limit))
{
throw instantiate_error{"imported memory limits cannot exceed hard memory limit of " +
std::to_string(MemoryPagesLimit * PageSize) + " bytes"};
std::to_string(memory_pages_limit * PageSize) + " bytes"};
}

bytes_ptr memory{imported_memories[0].data, null_delete};
Expand Down Expand Up @@ -688,7 +687,8 @@ std::optional<uint32_t> find_export(const Module& module, ExternalKind kind, std

std::unique_ptr<Instance> instantiate(Module module,
std::vector<ExternalFunction> imported_functions, std::vector<ExternalTable> imported_tables,
std::vector<ExternalMemory> imported_memories, std::vector<ExternalGlobal> imported_globals)
std::vector<ExternalMemory> imported_memories, std::vector<ExternalGlobal> imported_globals,
uint32_t memory_pages_limit /*= DefaultMemoryPagesLimit*/)
{
assert(module.funcsec.size() == module.codesec.size());

Expand All @@ -712,7 +712,16 @@ std::unique_ptr<Instance> instantiate(Module module,

auto [table, table_limits] = allocate_table(module.tablesec, imported_tables);

auto [memory, memory_limits] = allocate_memory(module.memorysec, imported_memories);
auto [memory, memory_limits] =
allocate_memory(module.memorysec, imported_memories, memory_pages_limit);
// In case upper limit for local/imported memory is defined,
// we adjust the hard memory limit, to ensure memory.grow will fail when exceeding it.
// Note: allocate_memory ensures memory's max limit is always below memory_pages_limit.
if (memory_limits.max.has_value())
Copy link
Member

Choose a reason for hiding this comment

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

Would add something like:

Suggested change
if (memory_limits.max.has_value())
// If imported memory is used, we need to adjust the memory limit.
// Note: allocate_memory ensures this limit is always below memory_pages_limit.
if (memory_limits.max.has_value())

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added the comment, but it applies to non-imported memory, too.

Copy link
Member

Choose a reason for hiding this comment

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

Ah right, it is the memory section setting.

{
assert(*memory_limits.max <= memory_pages_limit);
memory_pages_limit = *memory_limits.max;
}

// Before starting to fill memory and table,
// check that data and element segments are within bounds.
Expand Down Expand Up @@ -756,8 +765,8 @@ std::unique_ptr<Instance> instantiate(Module module,
// We need to create instance before filling table,
// because table functions will capture the pointer to instance.
auto instance = std::make_unique<Instance>(std::move(module), std::move(memory), memory_limits,
std::move(table), table_limits, std::move(globals), std::move(imported_functions),
std::move(imported_globals));
memory_pages_limit, std::move(table), table_limits, std::move(globals),
std::move(imported_functions), std::move(imported_globals));

// Fill the table based on elements segment
for (size_t i = 0; i < instance->module.elementsec.size(); ++i)
Expand Down Expand Up @@ -1240,10 +1249,7 @@ ExecutionResult execute(
uint32_t ret = static_cast<uint32_t>(cur_pages);
try
{
const size_t memory_max_pages =
(instance.memory_limits.max.has_value() ? *instance.memory_limits.max :
MemoryPagesLimit);
if (new_pages > memory_max_pages)
if (new_pages > instance.memory_pages_limit)
throw std::bad_alloc();
memory->resize(new_pages * PageSize);
}
Expand Down
11 changes: 8 additions & 3 deletions lib/fizzy/execute.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#pragma once

#include "exceptions.hpp"
#include "limits.hpp"
#include "module.hpp"
#include "span.hpp"
#include "types.hpp"
Expand Down Expand Up @@ -73,6 +74,8 @@ struct Instance
// For these cases unique_ptr would either have a normal deleter or noop deleter respectively
bytes_ptr memory = {nullptr, [](bytes*) {}};
Limits memory_limits;
// Hard limit for memory growth in pages, checked when memory is defined as unbounded in module
uint32_t memory_pages_limit = 0;
// Table is either allocated and owned by the instance or imported and owned externally.
// For these cases unique_ptr would either have a normal deleter or noop deleter respectively.
table_ptr table = {nullptr, [](table_elements*) {}};
Expand All @@ -81,13 +84,14 @@ struct Instance
std::vector<ExternalFunction> imported_functions;
std::vector<ExternalGlobal> imported_globals;

Instance(Module _module, bytes_ptr _memory, Limits _memory_limits, table_ptr _table,
Limits _table_limits, std::vector<Value> _globals,
Instance(Module _module, bytes_ptr _memory, Limits _memory_limits, uint32_t _memory_pages_limit,
table_ptr _table, Limits _table_limits, std::vector<Value> _globals,
std::vector<ExternalFunction> _imported_functions,
std::vector<ExternalGlobal> _imported_globals)
: module(std::move(_module)),
memory(std::move(_memory)),
memory_limits(_memory_limits),
memory_pages_limit(_memory_pages_limit),
table(std::move(_table)),
table_limits(_table_limits),
globals(std::move(_globals)),
Expand All @@ -101,7 +105,8 @@ std::unique_ptr<Instance> instantiate(Module module,
std::vector<ExternalFunction> imported_functions = {},
std::vector<ExternalTable> imported_tables = {},
std::vector<ExternalMemory> imported_memories = {},
std::vector<ExternalGlobal> imported_globals = {});
std::vector<ExternalGlobal> imported_globals = {},
uint32_t memory_pages_limit = DefaultMemoryPagesLimit);

// Execute a function on an instance.
ExecutionResult execute(
Expand Down
10 changes: 6 additions & 4 deletions lib/fizzy/limits.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,20 @@

#pragma once

#include <cstdint>

namespace fizzy
{
// The page size as defined by the WebAssembly 1.0 specification.
constexpr unsigned PageSize = 65536;
constexpr uint32_t PageSize = 65536;

// The maximum memory page limit as defined by the specification.
// It is only possible to address 4 GB (32-bit) of memory.
constexpr unsigned MemoryPagesValidationLimit = (4 * 1024 * 1024 * 1024ULL) / PageSize;
constexpr uint32_t MemoryPagesValidationLimit = (4 * 1024 * 1024 * 1024ULL) / PageSize;
static_assert(MemoryPagesValidationLimit == 65536);

// Set hard limit of 256MB of memory.
constexpr unsigned MemoryPagesLimit = (256 * 1024 * 1024ULL) / PageSize;
// The default hard limit of the memory size (256MB) as number of pages.
constexpr uint32_t DefaultMemoryPagesLimit = (256 * 1024 * 1024ULL) / PageSize;

// Call depth limit is set to default limit in wabt.
// https://github.com/WebAssembly/wabt/blob/ae2140ddc6969ef53599fe2fab81818de65db875/src/interp/interp.h#L1007
Expand Down
8 changes: 5 additions & 3 deletions test/spectests/spectests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ namespace
{
constexpr auto JsonExtension = ".json";
constexpr auto UnnamedModule = "_unnamed";
constexpr unsigned TestMemoryPagesLimit = (4 * 1024 * 1024 * 1024ULL) / fizzy::PageSize; // 4 Gb

// spectest module details:
// https://github.com/WebAssembly/spec/issues/1111
Expand Down Expand Up @@ -126,9 +127,10 @@ class test_runner
continue;
}

m_instances[name] = fizzy::instantiate(std::move(module),
std::move(imports.functions), std::move(imports.tables),
std::move(imports.memories), std::move(imports.globals));
m_instances[name] =
fizzy::instantiate(std::move(module), std::move(imports.functions),
std::move(imports.tables), std::move(imports.memories),
std::move(imports.globals), TestMemoryPagesLimit);
Copy link
Member

Choose a reason for hiding this comment

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

I'll actually override the default in #329 too.


m_last_module_name = name;
}
Expand Down
109 changes: 109 additions & 0 deletions test/unittests/execute_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,115 @@ TEST(execute, memory_grow)
EXPECT_THAT(execute(module, 0, {0xffffffe}), Result(-1));
}

TEST(execute, memory_grow_custom_hard_limit)
{
constexpr std::pair<uint32_t, uint32_t> test_cases[]{
{0, 1},
{1, 1},
{15, 1},
{16, -1},
{0xffffffff, -1},
};

/* wat2wasm
(memory 1)
(func (param i32) (result i32)
get_local 0
memory.grow
)
*/
const auto wasm =
from_hex("0061736d0100000001060160017f017f0302010005030100010a08010600200040000b");
const auto module = parse(wasm);

for (const auto& test_case : test_cases)
{
const auto instance = instantiate(module, {}, {}, {}, {}, 16);
EXPECT_THAT(execute(*instance, 0, {test_case.first}), Result(test_case.second));
}

/* wat2wasm
(memory 1 16)
(func (param i32) (result i32)
get_local 0
memory.grow
)
*/
const auto wasm_max_limit =
from_hex("0061736d0100000001060160017f017f030201000504010101100a08010600200040000b");
const auto module_max_limit = parse(wasm_max_limit);

for (const auto& test_case : test_cases)
{
const auto instance = instantiate(module_max_limit, {}, {}, {}, {}, 32);
EXPECT_THAT(execute(*instance, 0, {test_case.first}), Result(test_case.second));
}

/* wat2wasm
(memory (import "mod" "mem") 1)
(func (param i32) (result i32)
get_local 0
memory.grow
)
*/
const auto wasm_imported = from_hex(
"0061736d0100000001060160017f017f020c01036d6f64036d656d020001030201000a08010600200040000b");
const auto module_imported = parse(wasm_imported);

for (const auto& test_case : test_cases)
{
bytes memory(PageSize, 0);
const auto instance =
instantiate(module_imported, {}, {}, {{&memory, {1, std::nullopt}}}, {}, 16);
EXPECT_THAT(execute(*instance, 0, {test_case.first}), Result(test_case.second));

bytes memory_max_limit(PageSize, 0);
const auto instance_max_limit =
instantiate(module_imported, {}, {}, {{&memory_max_limit, {1, 16}}}, {}, 32);
EXPECT_THAT(execute(*instance_max_limit, 0, {test_case.first}), Result(test_case.second));
}

/* wat2wasm
(memory (import "mod" "mem") 1 16)
(func (param i32) (result i32)
get_local 0
memory.grow
)
*/
const auto wasm_imported_max_limit = from_hex(
"0061736d0100000001060160017f017f020d01036d6f64036d656d02010110030201000a08010600200040000"
"b");
const auto module_imported_max_limit = parse(wasm_imported_max_limit);

for (const auto& test_case : test_cases)
{
bytes memory(PageSize, 0);
const auto instance =
instantiate(module_imported_max_limit, {}, {}, {{&memory, {1, 16}}}, {}, 32);
EXPECT_THAT(execute(*instance, 0, {test_case.first}), Result(test_case.second));
}

/* wat2wasm
(memory (import "mod" "mem") 1 32)
(func (param i32) (result i32)
get_local 0
memory.grow
)
*/
const auto wasm_imported_max_limit_narrowing = from_hex(
"0061736d0100000001060160017f017f020d01036d6f64036d656d02010120030201000a08010600200040000"
"b");
const auto module_imported_max_limit_narrowing = parse(wasm_imported_max_limit_narrowing);

for (const auto& test_case : test_cases)
{
bytes memory(PageSize, 0);
const auto instance =
instantiate(module_imported_max_limit_narrowing, {}, {}, {{&memory, {1, 16}}}, {}, 32);
EXPECT_THAT(execute(*instance, 0, {test_case.first}), Result(test_case.second));
}
}

TEST(execute, start_section)
{
// In this test the start function (index 1) writes a i32 value to the memory
Expand Down
61 changes: 60 additions & 1 deletion test/unittests/instantiate_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ TEST(instantiate, imported_memory_invalid)
const auto bin_without_max = from_hex("0061736d01000000020a01036d6f64016d020001");
const auto module_without_max = parse(bin_without_max);
EXPECT_THROW_MESSAGE(
instantiate(module_without_max, {}, {}, {{&memory, {1, MemoryPagesLimit + 1}}}),
instantiate(module_without_max, {}, {}, {{&memory, {1, DefaultMemoryPagesLimit + 1}}}),
instantiate_error,
"imported memory limits cannot exceed hard memory limit of 268435456 bytes");
}
Expand Down Expand Up @@ -490,6 +490,65 @@ TEST(instantiate, memory_single_large_maximum)
"cannot exceed hard memory limit of 268435456 bytes");
}

TEST(instantiate, memory_single_custom_hard_limit)
{
/* wat2wasm
(memory 2 4)
*/
const auto bin = from_hex("0061736d01000000050401010204");
const auto module = parse(bin);

EXPECT_THROW_MESSAGE(instantiate(module, {}, {}, {}, {}, 1), instantiate_error,
"cannot exceed hard memory limit of 65536 bytes");
EXPECT_THROW_MESSAGE(instantiate(module, {}, {}, {}, {}, 3), instantiate_error,
"cannot exceed hard memory limit of 196608 bytes");

EXPECT_NO_THROW(instantiate(module, {}, {}, {}, {}, 4));
EXPECT_NO_THROW(instantiate(module, {}, {}, {}, {}, 8));
}

TEST(instantiate, imported_memory_custom_hard_limit)
{
/* wat2wasm
(memory (import "mod" "mem") 2)
*/
const auto bin = from_hex("0061736d01000000020c01036d6f64036d656d020002");
const auto module = parse(bin);

bytes memory(PageSize * 3, 0);

EXPECT_THROW_MESSAGE(instantiate(module, {}, {}, {{&memory, {3, 4}}}, {}, 1), instantiate_error,
"imported memory limits cannot exceed hard memory limit of 65536 bytes");
EXPECT_THROW_MESSAGE(instantiate(module, {}, {}, {{&memory, {3, 4}}}, {}, 2), instantiate_error,
"imported memory limits cannot exceed hard memory limit of 131072 bytes");
EXPECT_THROW_MESSAGE(instantiate(module, {}, {}, {{&memory, {3, 4}}}, {}, 3), instantiate_error,
"imported memory limits cannot exceed hard memory limit of 196608 bytes");

EXPECT_NO_THROW(instantiate(module, {}, {}, {{&memory, {3, std::nullopt}}}, {}, 3));
EXPECT_NO_THROW(instantiate(module, {}, {}, {{&memory, {3, std::nullopt}}}, {}, 4));
EXPECT_NO_THROW(instantiate(module, {}, {}, {{&memory, {3, 4}}}, {}, 4));
EXPECT_NO_THROW(instantiate(module, {}, {}, {{&memory, {3, 4}}}, {}, 8));

/* wat2wasm
(memory (import "mod" "mem") 2 6)
*/
const auto bin_max_limit = from_hex("0061736d01000000020d01036d6f64036d656d02010206");
const auto module_max_limit = parse(bin_max_limit);

EXPECT_THROW_MESSAGE(instantiate(module_max_limit, {}, {}, {{&memory, {3, 4}}}, {}, 1),
instantiate_error, "imported memory limits cannot exceed hard memory limit of 65536 bytes");
EXPECT_THROW_MESSAGE(instantiate(module_max_limit, {}, {}, {{&memory, {3, 4}}}, {}, 2),
instantiate_error,
"imported memory limits cannot exceed hard memory limit of 131072 bytes");
EXPECT_THROW_MESSAGE(instantiate(module_max_limit, {}, {}, {{&memory, {3, 4}}}, {}, 3),
instantiate_error,
"imported memory limits cannot exceed hard memory limit of 196608 bytes");

EXPECT_NO_THROW(instantiate(module_max_limit, {}, {}, {{&memory, {3, 4}}}, {}, 4));
EXPECT_NO_THROW(instantiate(module_max_limit, {}, {}, {{&memory, {3, 6}}}, {}, 6));
EXPECT_NO_THROW(instantiate(module_max_limit, {}, {}, {{&memory, {3, 6}}}, {}, 8));
}

TEST(instantiate, element_section)
{
/* wat2wasm
Expand Down