diff --git a/circle.yml b/circle.yml index 6764d5d52..837a9741a 100644 --- a/circle.yml +++ b/circle.yml @@ -252,8 +252,8 @@ jobs: - benchmark: min_time: "0.01" - spectest: - expected_passed: 5136 - expected_failed: 296 + expected_passed: 5138 + expected_failed: 294 expected_skipped: 6381 sanitizers-macos: @@ -270,8 +270,8 @@ jobs: - benchmark: min_time: "0.01" - spectest: - expected_passed: 5136 - expected_failed: 296 + expected_passed: 5138 + expected_failed: 294 expected_skipped: 6381 benchmark: @@ -381,8 +381,8 @@ jobs: expected_failed: 9 expected_skipped: 7323 - spectest: - expected_passed: 5136 - expected_failed: 296 + expected_passed: 5138 + expected_failed: 294 expected_skipped: 6381 workflows: diff --git a/lib/fizzy/execute.cpp b/lib/fizzy/execute.cpp index cee4d7556..5bce6aca9 100644 --- a/lib/fizzy/execute.cpp +++ b/lib/fizzy/execute.cpp @@ -495,14 +495,9 @@ std::unique_ptr instantiate(Module module, globals.reserve(module.globalsec.size()); for (auto const& global : module.globalsec) { - // Wasm spec section 3.3.7 constrains initialization by another global to const imports only - // https://webassembly.github.io/spec/core/valid/instructions.html#expressions - if (global.expression.kind == ConstantExpression::Kind::GlobalGet && - global.expression.value.global_index >= imported_globals.size()) - { - throw instantiate_error{ - "global can be initialized by another const global only if it's imported"}; - } + // Constraint to use global.get only with imported globals is checked at validation. + assert(global.expression.kind != ConstantExpression::Kind::GlobalGet || + global.expression.value.global_index < imported_globals.size()); const auto value = eval_constant_expression( global.expression, imported_globals, module.globalsec, globals); diff --git a/lib/fizzy/parser.cpp b/lib/fizzy/parser.cpp index 84b0a5ff0..86e8f9667 100644 --- a/lib/fizzy/parser.cpp +++ b/lib/fizzy/parser.cpp @@ -12,6 +12,22 @@ namespace fizzy { +namespace +{ +void validate_constant_expression(const ConstantExpression& const_expr, const Module& module) +{ + if (const_expr.kind == ConstantExpression::Kind::Constant) + return; + + const auto global_idx = const_expr.value.global_index; + if (global_idx >= module.get_global_count()) + throw validation_error{"invalid global index in constant expression"}; + + if (module.is_global_mutable(global_idx)) + throw validation_error{"constant expression can use global.get only for const globals"}; +} +} // namespace + template parser_result parse(const uint8_t* pos, const uint8_t* end); @@ -533,11 +549,26 @@ Module parse(bytes_view input) if (!module.elementsec.empty() && !module.has_table()) throw validation_error{"element section encountered without a table section"}; + const auto total_global_count = module.get_global_count(); + for (const auto& global : module.globalsec) + { + validate_constant_expression(global.expression, module); + + // Wasm spec section 3.3.7 constrains initialization by another global to const imports only + // https://webassembly.github.io/spec/core/valid/instructions.html#expressions + if (global.expression.kind == ConstantExpression::Kind::GlobalGet && + global.expression.value.global_index >= module.imported_global_types.size()) + { + throw validation_error{ + "global can be initialized by another const global only if it's imported"}; + } + } + + if (module.funcsec.size() != code_binaries.size()) throw parser_error{"malformed binary: number of function and code entries must match"}; const auto total_func_count = module.get_function_count(); - const auto total_global_count = module.get_global_count(); // Validate exports. std::unordered_set export_names; diff --git a/test/unittests/instantiate_test.cpp b/test/unittests/instantiate_test.cpp index 85f4a9b02..f2e3dd4b3 100644 --- a/test/unittests/instantiate_test.cpp +++ b/test/unittests/instantiate_test.cpp @@ -857,31 +857,6 @@ TEST(instantiate, globals_initialized_from_imported) ASSERT_EQ(instance->globals.size(), 1); EXPECT_EQ(instance->globals[0], 42); - - // initializing from mutable global is not allowed - /* wat2wasm --no-check - (global (import "mod" "g1") (mut i32)) - (global (mut i32) (global.get 0)) - */ - const auto bin_invalid1 = - from_hex("0061736d01000000020b01036d6f64026731037f010606017f0123000b"); - const auto module_invalid1 = parse(bin_invalid1); - - ExternalGlobal g_mutable{&global_value, true}; - - EXPECT_THROW_MESSAGE(instantiate(module_invalid1, {}, {}, {}, {g_mutable}), instantiate_error, - "constant expression can use global_get only for const globals"); - - // initializing from non-imported global is not allowed - /* wat2wasm --no-check - (global i32 (i32.const 42)) - (global (mut i32) (global.get 0)) - */ - const auto bin_invalid2 = from_hex("0061736d01000000060b027f00412a0b7f0123000b"); - const auto module_invalid2 = parse(bin_invalid2); - - EXPECT_THROW_MESSAGE(instantiate(module_invalid2, {}, {}), instantiate_error, - "global can be initialized by another const global only if it's imported"); } TEST(instantiate, start_unreachable) diff --git a/test/unittests/parser_test.cpp b/test/unittests/parser_test.cpp index b702cf64d..26b2cde83 100644 --- a/test/unittests/parser_test.cpp +++ b/test/unittests/parser_test.cpp @@ -594,12 +594,17 @@ TEST(parser, global_single_mutable_const_inited) EXPECT_EQ(module.globalsec[0].expression.value.constant, 0x10); } -TEST(parser, global_single_const_global_inited) +TEST(parser, global_multi_global_inited) { - const auto section_contents = bytes{0x01, 0x7f, 0x00, uint8_t(Instr::global_get), 0x01, 0x0b}; - const auto bin = bytes{wasm_prefix} + make_section(6, section_contents); - + /* wat2wasm + (global (import "m" "g1") i32) + (global (import "m" "g2") i32) + (global i32 (global.get 1)) + */ + const auto bin = + from_hex("0061736d01000000021102016d026731037f00016d026732037f000606017f0023010b"); const auto module = parse(bin); + ASSERT_EQ(module.globalsec.size(), 1); EXPECT_FALSE(module.globalsec[0].type.is_mutable); EXPECT_EQ(module.globalsec[0].type.value_type, ValType::i32); diff --git a/test/unittests/validation_test.cpp b/test/unittests/validation_test.cpp index c261b9a93..ebd6635ec 100644 --- a/test/unittests/validation_test.cpp +++ b/test/unittests/validation_test.cpp @@ -24,6 +24,36 @@ TEST(validation, imported_function_unknown_type) parse(wasm), validation_error, "invalid type index of an imported function"); } +TEST(validation, globals_invalid_initializer) +{ + // initializing from non-existing global + /* wat2wasm --no-check + (global i32 (global.get 1)) + */ + const auto bin_invalid1 = from_hex("0061736d010000000606017f0023010b"); + EXPECT_THROW_MESSAGE( + parse(bin_invalid1), validation_error, "invalid global index in constant expression"); + + // initializing from mutable global is not allowed + /* wat2wasm --no-check + (global (import "mod" "g1") (mut i32)) + (global (mut i32) (global.get 0)) + */ + const auto bin_invalid2 = + from_hex("0061736d01000000020b01036d6f64026731037f010606017f0123000b"); + EXPECT_THROW_MESSAGE(parse(bin_invalid2), validation_error, + "constant expression can use global.get only for const globals"); + + // initializing from non-imported global is not allowed + /* wat2wasm --no-check + (global i32 (i32.const 42)) + (global (mut i32) (global.get 0)) + */ + const auto bin_invalid3 = from_hex("0061736d01000000060b027f00412a0b7f0123000b"); + EXPECT_THROW_MESSAGE(parse(bin_invalid3), validation_error, + "global can be initialized by another const global only if it's imported"); +} + TEST(validation, import_memories_multiple) { const auto section_contents =