From c6d174f9976608e8101a84265df7bcbbe846ef64 Mon Sep 17 00:00:00 2001 From: Andrei Maiboroda Date: Mon, 8 Jun 2020 17:24:06 +0200 Subject: [PATCH 1/2] Validate that constant expression has exactly one instruction --- lib/fizzy/parser.cpp | 98 +++++++++++++++--------------- test/unittests/parser_test.cpp | 16 +---- test/unittests/validation_test.cpp | 13 ++++ 3 files changed, 64 insertions(+), 63 deletions(-) diff --git a/lib/fizzy/parser.cpp b/lib/fizzy/parser.cpp index c967d37fc..55bed40de 100644 --- a/lib/fizzy/parser.cpp +++ b/lib/fizzy/parser.cpp @@ -123,60 +123,62 @@ inline parser_result parse_constant_expression( { ConstantExpression result; - Instr instr; - do + uint8_t opcode; + std::tie(opcode, pos) = parse_byte(pos, end); + + const auto instr = static_cast(opcode); + switch (instr) { - uint8_t opcode; - std::tie(opcode, pos) = parse_byte(pos, end); + default: + throw validation_error{ + "unexpected instruction in the constant expression: " + std::to_string(*(pos - 1))}; - instr = static_cast(opcode); - switch (instr) - { - default: - throw validation_error{ - "unexpected instruction in the constant expression: " + std::to_string(*(pos - 1))}; + case Instr::end: + throw validation_error{"constant expression is empty"}; - case Instr::end: - break; + case Instr::global_get: + { + result.kind = ConstantExpression::Kind::GlobalGet; + std::tie(result.value.global_index, pos) = leb128u_decode(pos, end); + break; + } - case Instr::global_get: - { - result.kind = ConstantExpression::Kind::GlobalGet; - std::tie(result.value.global_index, pos) = leb128u_decode(pos, end); - break; - } + case Instr::i32_const: + { + result.kind = ConstantExpression::Kind::Constant; + int32_t value; + std::tie(value, pos) = leb128s_decode(pos, end); + result.value.constant = static_cast(value); + break; + } - case Instr::i32_const: - { - result.kind = ConstantExpression::Kind::Constant; - int32_t value; - std::tie(value, pos) = leb128s_decode(pos, end); - result.value.constant = static_cast(value); - break; - } + case Instr::i64_const: + { + result.kind = ConstantExpression::Kind::Constant; + int64_t value; + std::tie(value, pos) = leb128s_decode(pos, end); + result.value.constant = static_cast(value); + break; + } + case Instr::f32_const: + // TODO: support this once floating points are implemented + result.kind = ConstantExpression::Kind::Constant; + result.value.constant = 0; + pos = skip(4, pos, end); + break; + case Instr::f64_const: + // TODO: support this once floating points are implemented + result.kind = ConstantExpression::Kind::Constant; + result.value.constant = 0; + pos = skip(8, pos, end); + break; + } - case Instr::i64_const: - { - result.kind = ConstantExpression::Kind::Constant; - int64_t value; - std::tie(value, pos) = leb128s_decode(pos, end); - result.value.constant = static_cast(value); - break; - } - case Instr::f32_const: - // TODO: support this once floating points are implemented - result.kind = ConstantExpression::Kind::Constant; - result.value.constant = 0; - pos = skip(4, pos, end); - break; - case Instr::f64_const: - // TODO: support this once floating points are implemented - result.kind = ConstantExpression::Kind::Constant; - result.value.constant = 0; - pos = skip(8, pos, end); - break; - } - } while (instr != Instr::end); + uint8_t end_opcode; + std::tie(end_opcode, pos) = parse_byte(pos, end); + + if (static_cast(end_opcode) != Instr::end) + throw validation_error{"constant expression has multiple instructions"}; return {result, pos}; } diff --git a/test/unittests/parser_test.cpp b/test/unittests/parser_test.cpp index 9006c57ed..93ec08300 100644 --- a/test/unittests/parser_test.cpp +++ b/test/unittests/parser_test.cpp @@ -612,20 +612,6 @@ TEST(parser, global_multi_global_inited) EXPECT_EQ(module.globalsec[0].expression.value.global_index, 0x01); } -TEST(parser, global_single_multi_instructions_inited) -{ - const auto section_contents = bytes{ - 0x01, 0x7f, 0x01, uint8_t(Instr::i32_const), 0x10, uint8_t(Instr::i64_const), 0x7f, 0x0b}; - const auto bin = bytes{wasm_prefix} + make_section(6, section_contents); - - const auto module = parse(bin); - ASSERT_EQ(module.globalsec.size(), 1); - EXPECT_TRUE(module.globalsec[0].type.is_mutable); - EXPECT_EQ(module.globalsec[0].type.value_type, ValType::i32); - EXPECT_EQ(module.globalsec[0].expression.kind, ConstantExpression::Kind::Constant); - EXPECT_EQ(module.globalsec[0].expression.value.constant, uint64_t(-1)); -} - TEST(parser, global_multi_const_inited) { const auto section_contents = @@ -916,7 +902,7 @@ TEST(parser, element_section_invalid_initializer) TEST(parser, element_section_no_table_section) { const auto wasm = - bytes{wasm_prefix} + make_section(9, make_vec({"000b"_bytes + make_vec({"00"_bytes})})); + bytes{wasm_prefix} + make_section(9, make_vec({"0041000b"_bytes + make_vec({"00"_bytes})})); EXPECT_THROW_MESSAGE( parse(wasm), validation_error, "element section encountered without a table section"); } diff --git a/test/unittests/validation_test.cpp b/test/unittests/validation_test.cpp index aca96e2ae..cd88e7687 100644 --- a/test/unittests/validation_test.cpp +++ b/test/unittests/validation_test.cpp @@ -52,6 +52,19 @@ TEST(validation, globals_invalid_initializer) 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"); + + /* wat2wasm --no-check + (global (mut i32) (i32.const 16) (i32.const -1)) + */ + const auto bin_multi = from_hex("0061736d010000000608017f014110417f0b"); + EXPECT_THROW_MESSAGE( + parse(bin_multi), validation_error, "constant expression has multiple instructions"); + + /* wat2wasm --no-check + (global (mut i64)) + */ + const auto bin_no_instr = from_hex("0061736d010000000604017e010b"); + EXPECT_THROW_MESSAGE(parse(bin_no_instr), validation_error, "constant expression is empty"); } TEST(validation, import_memories_multiple) From c189718a21e10d092903468fa47954f9dbb10390 Mon Sep 17 00:00:00 2001 From: Andrei Maiboroda Date: Fri, 26 Jun 2020 18:30:06 +0200 Subject: [PATCH 2/2] ci: update spectest expectations --- circle.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/circle.yml b/circle.yml index 837a9741a..072078f56 100644 --- a/circle.yml +++ b/circle.yml @@ -252,8 +252,8 @@ jobs: - benchmark: min_time: "0.01" - spectest: - expected_passed: 5138 - expected_failed: 294 + expected_passed: 5140 + expected_failed: 292 expected_skipped: 6381 sanitizers-macos: @@ -270,8 +270,8 @@ jobs: - benchmark: min_time: "0.01" - spectest: - expected_passed: 5138 - expected_failed: 294 + expected_passed: 5140 + expected_failed: 292 expected_skipped: 6381 benchmark: @@ -381,8 +381,8 @@ jobs: expected_failed: 9 expected_skipped: 7323 - spectest: - expected_passed: 5138 - expected_failed: 294 + expected_passed: 5140 + expected_failed: 292 expected_skipped: 6381 workflows: