From 433db1c7eefa49ec5c79cc280bab058be67d5068 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Tue, 20 Oct 2020 17:02:18 +0200 Subject: [PATCH 1/7] Skip nop instructions --- lib/fizzy/execute.cpp | 3 +-- lib/fizzy/parser_expr.cpp | 2 ++ test/unittests/parser_expr_test.cpp | 7 +++---- test/unittests/parser_test.cpp | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/fizzy/execute.cpp b/lib/fizzy/execute.cpp index 5b35a7e89..6fccb95d1 100644 --- a/lib/fizzy/execute.cpp +++ b/lib/fizzy/execute.cpp @@ -528,7 +528,6 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, { case Instr::unreachable: goto trap; - case Instr::nop: case Instr::block: case Instr::loop: break; @@ -560,7 +559,7 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, } case Instr::br: case Instr::br_if: - case Instr::return_: + case Instr::return_: // TODO: Replace return with br { const auto arity = read(pc); diff --git a/lib/fizzy/parser_expr.cpp b/lib/fizzy/parser_expr.cpp index f75f2b4e9..ed3b734d0 100644 --- a/lib/fizzy/parser_expr.cpp +++ b/lib/fizzy/parser_expr.cpp @@ -329,6 +329,8 @@ parser_result parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f } case Instr::nop: + continue; + case Instr::i32_eqz: case Instr::i32_eq: case Instr::i32_ne: diff --git a/test/unittests/parser_expr_test.cpp b/test/unittests/parser_expr_test.cpp index 0c235215e..76d8266e1 100644 --- a/test/unittests/parser_expr_test.cpp +++ b/test/unittests/parser_expr_test.cpp @@ -64,8 +64,7 @@ TEST(parser_expr, instr_block) const auto empty = "010102400b0b"_bytes; const auto [code1, pos1] = parse_expr(empty); - EXPECT_THAT(code1.instructions, - ElementsAre(Instr::nop, Instr::nop, Instr::block, Instr::end, Instr::end)); + EXPECT_THAT(code1.instructions, ElementsAre(Instr::block, Instr::end, Instr::end)); const auto block_i64 = "027e42000b1a0b"_bytes; const auto [code2, pos2] = parse_expr(block_i64); @@ -163,9 +162,9 @@ TEST(parser_expr, block_br) const auto code_bin = "010240410a21010c00410b21010b20011a0b"_bytes; const auto [code, pos] = parse_expr(code_bin, 0, {{2, ValType::i32}}); EXPECT_THAT( - code.instructions, ElementsAre(Instr::nop, Instr::block, Instr::i32_const, 0x0a, 0, 0, 0, + code.instructions, ElementsAre(Instr::block, Instr::i32_const, 0x0a, 0, 0, 0, Instr::local_set, 1, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0, - /*code_offset:*/ 36, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + /*code_offset:*/ 35, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::i32_const, 0x0b, 0, 0, 0, Instr::local_set, 1, 0, 0, 0, Instr::end, Instr::local_get, 1, 0, 0, 0, Instr::drop, Instr::end)); EXPECT_EQ(code.max_stack_height, 1); diff --git a/test/unittests/parser_test.cpp b/test/unittests/parser_test.cpp index 9cdc97d28..759112ae7 100644 --- a/test/unittests/parser_test.cpp +++ b/test/unittests/parser_test.cpp @@ -1088,7 +1088,7 @@ TEST(parser, code_section_with_basic_instructions) EXPECT_EQ(module->codesec[0].local_count, 4); EXPECT_THAT(module->codesec[0].instructions, ElementsAre(Instr::local_get, 1, 0, 0, 0, Instr::i32_const, 2, 0, 0, 0, Instr::i32_add, - Instr::local_set, 3, 0, 0, 0, Instr::nop, Instr::unreachable, Instr::end)); + Instr::local_set, 3, 0, 0, 0, Instr::unreachable, Instr::end)); } TEST(parser, code_section_with_memory_size) From 7a3936ef8307b8d4a28d7f1d2081a88c6a48cf29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Tue, 20 Oct 2020 16:57:57 +0200 Subject: [PATCH 2/7] Skip block and loop instructions --- lib/fizzy/execute.cpp | 3 - lib/fizzy/parser_expr.cpp | 4 +- test/unittests/parser_expr_test.cpp | 110 ++++++++++++++-------------- 3 files changed, 55 insertions(+), 62 deletions(-) diff --git a/lib/fizzy/execute.cpp b/lib/fizzy/execute.cpp index 6fccb95d1..089013ca3 100644 --- a/lib/fizzy/execute.cpp +++ b/lib/fizzy/execute.cpp @@ -528,9 +528,6 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, { case Instr::unreachable: goto trap; - case Instr::block: - case Instr::loop: - break; case Instr::if_: { if (stack.pop().as() != 0) diff --git a/lib/fizzy/parser_expr.cpp b/lib/fizzy/parser_expr.cpp index ed3b734d0..1e55aa5bc 100644 --- a/lib/fizzy/parser_expr.cpp +++ b/lib/fizzy/parser_expr.cpp @@ -464,7 +464,7 @@ parser_result parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f // Push label with immediates offset after arity. control_stack.emplace(Instr::block, block_type, static_cast(operand_stack.size()), code.instructions.size()); - break; + continue; } case Instr::loop: @@ -474,7 +474,7 @@ parser_result parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f control_stack.emplace(Instr::loop, loop_type, static_cast(operand_stack.size()), code.instructions.size()); - break; + continue; } case Instr::if_: diff --git a/test/unittests/parser_expr_test.cpp b/test/unittests/parser_expr_test.cpp index 76d8266e1..aae59e2fa 100644 --- a/test/unittests/parser_expr_test.cpp +++ b/test/unittests/parser_expr_test.cpp @@ -28,25 +28,25 @@ TEST(parser_expr, instr_loop) { const auto loop_void = "03400b0b"_bytes; const auto [code1, pos1] = parse_expr(loop_void); - EXPECT_THAT(code1.instructions, ElementsAre(Instr::loop, Instr::end, Instr::end)); + EXPECT_THAT(code1.instructions, ElementsAre(Instr::end, Instr::end)); EXPECT_EQ(code1.max_stack_height, 0); const auto loop_i32 = "037f41000b1a0b"_bytes; const auto [code2, pos2] = parse_expr(loop_i32); - EXPECT_THAT(code2.instructions, ElementsAre(Instr::loop, Instr::i32_const, 0, 0, 0, 0, - Instr::end, Instr::drop, Instr::end)); + EXPECT_THAT(code2.instructions, + ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::end, Instr::drop, Instr::end)); EXPECT_EQ(code2.max_stack_height, 1); const auto loop_f32 = "037d43000000000b1a0b"_bytes; const auto [code3, pos3] = parse_expr(loop_f32); - EXPECT_THAT(code3.instructions, ElementsAre(Instr::loop, Instr::f32_const, 0, 0, 0, 0, - Instr::end, Instr::drop, Instr::end)); + EXPECT_THAT(code3.instructions, + ElementsAre(Instr::f32_const, 0, 0, 0, 0, Instr::end, Instr::drop, Instr::end)); EXPECT_EQ(code3.max_stack_height, 1); const auto loop_f64 = "037c4400000000000000000b1a0b"_bytes; const auto [code4, pos4] = parse_expr(loop_f64); - EXPECT_THAT(code4.instructions, ElementsAre(Instr::loop, Instr::f64_const, 0, 0, 0, 0, 0, 0, 0, - 0, Instr::end, Instr::drop, Instr::end)); + EXPECT_THAT(code4.instructions, + ElementsAre(Instr::f64_const, 0, 0, 0, 0, 0, 0, 0, 0, Instr::end, Instr::drop, Instr::end)); EXPECT_EQ(code4.max_stack_height, 1); } @@ -64,17 +64,17 @@ TEST(parser_expr, instr_block) const auto empty = "010102400b0b"_bytes; const auto [code1, pos1] = parse_expr(empty); - EXPECT_THAT(code1.instructions, ElementsAre(Instr::block, Instr::end, Instr::end)); + EXPECT_THAT(code1.instructions, ElementsAre(Instr::end, Instr::end)); const auto block_i64 = "027e42000b1a0b"_bytes; const auto [code2, pos2] = parse_expr(block_i64); - EXPECT_THAT(code2.instructions, ElementsAre(Instr::block, Instr::i64_const, 0, 0, 0, 0, 0, 0, 0, - 0, Instr::end, Instr::drop, Instr::end)); + EXPECT_THAT(code2.instructions, + ElementsAre(Instr::i64_const, 0, 0, 0, 0, 0, 0, 0, 0, Instr::end, Instr::drop, Instr::end)); const auto block_f64 = "027c4400000000000000000b1a0b"_bytes; const auto [code3, pos3] = parse_expr(block_f64); - EXPECT_THAT(code3.instructions, ElementsAre(Instr::block, Instr::f64_const, 0, 0, 0, 0, 0, 0, 0, - 0, Instr::end, Instr::drop, Instr::end)); + EXPECT_THAT(code3.instructions, + ElementsAre(Instr::f64_const, 0, 0, 0, 0, 0, 0, 0, 0, Instr::end, Instr::drop, Instr::end)); } TEST(parser_expr, instr_block_input_buffer_overflow) @@ -93,7 +93,7 @@ TEST(parser_expr, loop_br) const auto module = parse(wasm); EXPECT_THAT(module->codesec[0].instructions, - ElementsAre(Instr::loop, Instr::br, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 0, 0, 0, 0, + ElementsAre(Instr::br, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 0, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::end)); /* wat2wasm @@ -108,7 +108,7 @@ TEST(parser_expr, loop_br) const auto module_parent_stack = parse(wasm_parent_stack); EXPECT_THAT(module_parent_stack->codesec[0].instructions, - ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::loop, Instr::br, /*arity:*/ 0, 0, 0, 0, + ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 5, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::drop, Instr::end)); @@ -126,7 +126,7 @@ TEST(parser_expr, loop_br) const auto module_arity = parse(wasm_arity); EXPECT_THAT(module_arity->codesec[0].instructions, - ElementsAre(Instr::loop, Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0, + ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 0, 0, 0, 0, /*stack_drop:*/ 1, 0, 0, 0, Instr::end, Instr::drop, Instr::end)); } @@ -139,10 +139,9 @@ TEST(parser_expr, loop_return) const auto wasm = from_hex("0061736d01000000010401600000030201000a0801060003400f0b0b"); const auto module = parse(wasm); - EXPECT_THAT( - module->codesec[0].instructions, ElementsAre(Instr::loop, Instr::return_, - /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 15, 0, 0, 0, - /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::end)); + EXPECT_THAT(module->codesec[0].instructions, + ElementsAre(Instr::return_, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 14, 0, 0, 0, + /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::end)); } TEST(parser_expr, block_br) @@ -161,12 +160,11 @@ TEST(parser_expr, block_br) const auto code_bin = "010240410a21010c00410b21010b20011a0b"_bytes; const auto [code, pos] = parse_expr(code_bin, 0, {{2, ValType::i32}}); - EXPECT_THAT( - code.instructions, ElementsAre(Instr::block, Instr::i32_const, 0x0a, 0, 0, 0, - Instr::local_set, 1, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0, - /*code_offset:*/ 35, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, - Instr::i32_const, 0x0b, 0, 0, 0, Instr::local_set, 1, 0, 0, 0, - Instr::end, Instr::local_get, 1, 0, 0, 0, Instr::drop, Instr::end)); + EXPECT_THAT(code.instructions, + ElementsAre(Instr::i32_const, 0x0a, 0, 0, 0, Instr::local_set, 1, 0, 0, 0, Instr::br, + /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 34, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + Instr::i32_const, 0x0b, 0, 0, 0, Instr::local_set, 1, 0, 0, 0, Instr::end, + Instr::local_get, 1, 0, 0, 0, Instr::drop, Instr::end)); EXPECT_EQ(code.max_stack_height, 1); /* wat2wasm @@ -181,8 +179,8 @@ TEST(parser_expr, block_br) const auto module_parent_stack = parse(wasm_parent_stack); EXPECT_THAT(module_parent_stack->codesec[0].instructions, - ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::block, Instr::br, /*arity:*/ 0, 0, 0, 0, - /*code_offset:*/ 20, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::drop, + ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0, + /*code_offset:*/ 19, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::drop, Instr::end)); /* wat2wasm @@ -199,8 +197,8 @@ TEST(parser_expr, block_br) const auto module_arity = parse(wasm_arity); EXPECT_THAT(module_arity->codesec[0].instructions, - ElementsAre(Instr::block, Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 1, 0, 0, 0, - /*code_offset:*/ 20, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::drop, + ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 1, 0, 0, 0, + /*code_offset:*/ 19, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::drop, Instr::end)); } @@ -213,8 +211,8 @@ TEST(parser_expr, block_return) const auto module = parse(wasm); EXPECT_THAT(module->codesec[0].instructions, - ElementsAre(Instr::block, Instr::return_, /*arity:*/ 0, 0, 0, 0, - /*code_offset:*/ 15, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::end)); + ElementsAre(Instr::return_, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 14, 0, 0, 0, + /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::end)); } TEST(parser_expr, if_br) @@ -230,9 +228,8 @@ TEST(parser_expr, if_br) EXPECT_THAT(module->codesec[0].instructions, ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::if_, /*else_offset:*/ 24, 0, 0, 0, - Instr::br, /*arity:*/ 0, 0, 0, 0, - /*code_offset:*/ 24, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, - /*24:*/ Instr::end)); + Instr::br, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 24, 0, 0, 0, + /*stack_drop:*/ 0, 0, 0, 0, Instr::end, /*24:*/ Instr::end)); /* wat2wasm (func @@ -285,27 +282,26 @@ TEST(parser_expr, instr_br_table) const auto& code = module->codesec[0]; EXPECT_THAT(code.instructions, - ElementsAre(Instr::block, Instr::block, Instr::block, Instr::block, Instr::block, - Instr::local_get, 0, 0, 0, 0, Instr::br_table, + ElementsAre(Instr::local_get, 0, 0, 0, 0, Instr::br_table, /*label_count:*/ 4, 0, 0, 0, /*arity:*/ 0, 0, 0, 0, - /*code_offset:*/ 135, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, - /*code_offset:*/ 116, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, - /*code_offset:*/ 97, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, - /*code_offset:*/ 78, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, - /*code_offset:*/ 154, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, - - /*59:*/ Instr::i32_const, 0x41, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, - /*code_offset:*/ 159, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, - /*78:*/ Instr::i32_const, 0x42, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, - /*code_offset:*/ 159, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, - /*97:*/ Instr::i32_const, 0x43, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, - /*code_offset:*/ 159, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, - /*116:*/ Instr::i32_const, 0x44, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, - /*code_offset:*/ 159, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, - /*135:*/ Instr::i32_const, 0x45, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, - /*code_offset:*/ 159, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, - /*154:*/ Instr::i32_const, 0x46, 0, 0, 0, - /*159:*/ Instr::end)); + /*code_offset:*/ 130, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + /*code_offset:*/ 111, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + /*code_offset:*/ 92, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + /*code_offset:*/ 73, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + /*code_offset:*/ 149, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + + /*54:*/ Instr::i32_const, 0x41, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, + /*code_offset:*/ 154, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, + /*73:*/ Instr::i32_const, 0x42, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, + /*code_offset:*/ 154, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, + /*92:*/ Instr::i32_const, 0x43, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, + /*code_offset:*/ 154, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, + /*111:*/ Instr::i32_const, 0x44, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, + /*code_offset:*/ 154, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, + /*130:*/ Instr::i32_const, 0x45, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, + /*code_offset:*/ 154, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, + /*149:*/ Instr::i32_const, 0x46, 0, 0, 0, + /*154:*/ Instr::end)); EXPECT_EQ(code.max_stack_height, 1); } @@ -329,10 +325,10 @@ TEST(parser_expr, instr_br_table_empty_vector) const auto& code = module->codesec[0]; EXPECT_THAT(code.instructions, - ElementsAre(Instr::block, Instr::local_get, 0, 0, 0, 0, Instr::br_table, - /*label_count:*/ 0, 0, 0, 0, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 42, 0, 0, 0, + ElementsAre(Instr::local_get, 0, 0, 0, 0, Instr::br_table, + /*label_count:*/ 0, 0, 0, 0, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 41, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::i32_const, 0x63, 0, 0, 0, Instr::return_, - /*arity:*/ 1, 0, 0, 0, /*code_offset:*/ 47, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + /*arity:*/ 1, 0, 0, 0, /*code_offset:*/ 46, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::i32_const, 0x64, 0, 0, 0, Instr::end)); EXPECT_EQ(code.max_stack_height, 1); From b15a01fd192a8584a21e6a25d3332cd1c2a1f68d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Tue, 20 Oct 2020 19:24:34 +0200 Subject: [PATCH 3/7] Do not omit end instructions --- lib/fizzy/parser_expr.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/fizzy/parser_expr.cpp b/lib/fizzy/parser_expr.cpp index 1e55aa5bc..f95531583 100644 --- a/lib/fizzy/parser_expr.cpp +++ b/lib/fizzy/parser_expr.cpp @@ -531,12 +531,8 @@ parser_result parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f if (frame.instruction != Instr::loop) // If end of block/if/else instruction. { - // In case it's an outermost implicit function block, - // we want br to jump to the final end of the function. - // Otherwise jump to the next instruction after block's end. - const auto target_pc = control_stack.size() == 1 ? - static_cast(code.instructions.size()) : - static_cast(code.instructions.size() + 1); + // Jump on the matching end instruction. + const auto target_pc = static_cast(code.instructions.size()); if (frame.instruction == Instr::if_ || frame.instruction == Instr::else_) { From d733acee7dc83db67cea68a0d39781129972bb4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Tue, 20 Oct 2020 19:24:53 +0200 Subject: [PATCH 4/7] Skip end instructions --- lib/fizzy/parser_expr.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/fizzy/parser_expr.cpp b/lib/fizzy/parser_expr.cpp index f95531583..5747ad3fb 100644 --- a/lib/fizzy/parser_expr.cpp +++ b/lib/fizzy/parser_expr.cpp @@ -556,10 +556,14 @@ parser_result parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f control_stack.pop(); // Pop the current frame. if (control_stack.empty()) + { continue_parsing = false; - else if (frame_type.has_value()) + break; + } + + if (frame_type.has_value()) push_operand(operand_stack, *frame_type); - break; + continue; } case Instr::br: From 51d25805348100435d8bf3e9de0417841aeb401d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Tue, 20 Oct 2020 19:25:59 +0200 Subject: [PATCH 5/7] Simplify end implementation --- lib/fizzy/execute.cpp | 6 ++---- lib/fizzy/parser_expr.cpp | 4 +++- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/fizzy/execute.cpp b/lib/fizzy/execute.cpp index 089013ca3..c162dc436 100644 --- a/lib/fizzy/execute.cpp +++ b/lib/fizzy/execute.cpp @@ -549,10 +549,8 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, } case Instr::end: { - // End execution if it's a final end instruction. - if (pc == &code.instructions[code.instructions.size()]) - goto end; - break; + assert(pc == &code.instructions[code.instructions.size()]); + goto end; } case Instr::br: case Instr::br_if: diff --git a/lib/fizzy/parser_expr.cpp b/lib/fizzy/parser_expr.cpp index 5747ad3fb..879392ae4 100644 --- a/lib/fizzy/parser_expr.cpp +++ b/lib/fizzy/parser_expr.cpp @@ -531,7 +531,9 @@ parser_result parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f if (frame.instruction != Instr::loop) // If end of block/if/else instruction. { - // Jump on the matching end instruction. + // The position of the "end": + // for the outermost implicit function block this is the function's end instruction, + // otherwise this is the next instruction after the block's end. const auto target_pc = static_cast(code.instructions.size()); if (frame.instruction == Instr::if_ || frame.instruction == Instr::else_) From da8c473c6c77b1c323acc95ba904082024ac5417 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 12 Nov 2020 11:04:26 +0100 Subject: [PATCH 6/7] test: Update end-related tests --- test/unittests/parser_expr_test.cpp | 96 ++++++++++++++--------------- 1 file changed, 46 insertions(+), 50 deletions(-) diff --git a/test/unittests/parser_expr_test.cpp b/test/unittests/parser_expr_test.cpp index aae59e2fa..318603692 100644 --- a/test/unittests/parser_expr_test.cpp +++ b/test/unittests/parser_expr_test.cpp @@ -28,25 +28,25 @@ TEST(parser_expr, instr_loop) { const auto loop_void = "03400b0b"_bytes; const auto [code1, pos1] = parse_expr(loop_void); - EXPECT_THAT(code1.instructions, ElementsAre(Instr::end, Instr::end)); + EXPECT_THAT(code1.instructions, ElementsAre(Instr::end)); EXPECT_EQ(code1.max_stack_height, 0); const auto loop_i32 = "037f41000b1a0b"_bytes; const auto [code2, pos2] = parse_expr(loop_i32); - EXPECT_THAT(code2.instructions, - ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::end, Instr::drop, Instr::end)); + EXPECT_THAT( + code2.instructions, ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::drop, Instr::end)); EXPECT_EQ(code2.max_stack_height, 1); const auto loop_f32 = "037d43000000000b1a0b"_bytes; const auto [code3, pos3] = parse_expr(loop_f32); - EXPECT_THAT(code3.instructions, - ElementsAre(Instr::f32_const, 0, 0, 0, 0, Instr::end, Instr::drop, Instr::end)); + EXPECT_THAT( + code3.instructions, ElementsAre(Instr::f32_const, 0, 0, 0, 0, Instr::drop, Instr::end)); EXPECT_EQ(code3.max_stack_height, 1); const auto loop_f64 = "037c4400000000000000000b1a0b"_bytes; const auto [code4, pos4] = parse_expr(loop_f64); EXPECT_THAT(code4.instructions, - ElementsAre(Instr::f64_const, 0, 0, 0, 0, 0, 0, 0, 0, Instr::end, Instr::drop, Instr::end)); + ElementsAre(Instr::f64_const, 0, 0, 0, 0, 0, 0, 0, 0, Instr::drop, Instr::end)); EXPECT_EQ(code4.max_stack_height, 1); } @@ -64,17 +64,17 @@ TEST(parser_expr, instr_block) const auto empty = "010102400b0b"_bytes; const auto [code1, pos1] = parse_expr(empty); - EXPECT_THAT(code1.instructions, ElementsAre(Instr::end, Instr::end)); + EXPECT_THAT(code1.instructions, ElementsAre(Instr::end)); const auto block_i64 = "027e42000b1a0b"_bytes; const auto [code2, pos2] = parse_expr(block_i64); EXPECT_THAT(code2.instructions, - ElementsAre(Instr::i64_const, 0, 0, 0, 0, 0, 0, 0, 0, Instr::end, Instr::drop, Instr::end)); + ElementsAre(Instr::i64_const, 0, 0, 0, 0, 0, 0, 0, 0, Instr::drop, Instr::end)); const auto block_f64 = "027c4400000000000000000b1a0b"_bytes; const auto [code3, pos3] = parse_expr(block_f64); EXPECT_THAT(code3.instructions, - ElementsAre(Instr::f64_const, 0, 0, 0, 0, 0, 0, 0, 0, Instr::end, Instr::drop, Instr::end)); + ElementsAre(Instr::f64_const, 0, 0, 0, 0, 0, 0, 0, 0, Instr::drop, Instr::end)); } TEST(parser_expr, instr_block_input_buffer_overflow) @@ -94,7 +94,7 @@ TEST(parser_expr, loop_br) EXPECT_THAT(module->codesec[0].instructions, ElementsAre(Instr::br, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 0, 0, 0, 0, - /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::end)); + /*stack_drop:*/ 0, 0, 0, 0, Instr::end)); /* wat2wasm (func @@ -109,8 +109,7 @@ TEST(parser_expr, loop_br) EXPECT_THAT(module_parent_stack->codesec[0].instructions, ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0, - /*code_offset:*/ 5, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::drop, - Instr::end)); + /*code_offset:*/ 5, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::drop, Instr::end)); /* wat2wasm (func @@ -127,8 +126,7 @@ TEST(parser_expr, loop_br) EXPECT_THAT(module_arity->codesec[0].instructions, ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0, - /*code_offset:*/ 0, 0, 0, 0, /*stack_drop:*/ 1, 0, 0, 0, Instr::end, Instr::drop, - Instr::end)); + /*code_offset:*/ 0, 0, 0, 0, /*stack_drop:*/ 1, 0, 0, 0, Instr::drop, Instr::end)); } TEST(parser_expr, loop_return) @@ -140,8 +138,8 @@ TEST(parser_expr, loop_return) const auto module = parse(wasm); EXPECT_THAT(module->codesec[0].instructions, - ElementsAre(Instr::return_, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 14, 0, 0, 0, - /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::end)); + ElementsAre(Instr::return_, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 13, 0, 0, 0, + /*stack_drop:*/ 0, 0, 0, 0, Instr::end)); } TEST(parser_expr, block_br) @@ -162,9 +160,9 @@ TEST(parser_expr, block_br) const auto [code, pos] = parse_expr(code_bin, 0, {{2, ValType::i32}}); EXPECT_THAT(code.instructions, ElementsAre(Instr::i32_const, 0x0a, 0, 0, 0, Instr::local_set, 1, 0, 0, 0, Instr::br, - /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 34, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, - Instr::i32_const, 0x0b, 0, 0, 0, Instr::local_set, 1, 0, 0, 0, Instr::end, - Instr::local_get, 1, 0, 0, 0, Instr::drop, Instr::end)); + /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 33, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + Instr::i32_const, 0x0b, 0, 0, 0, Instr::local_set, 1, 0, 0, 0, Instr::local_get, 1, 0, + 0, 0, Instr::drop, Instr::end)); EXPECT_EQ(code.max_stack_height, 1); /* wat2wasm @@ -180,8 +178,7 @@ TEST(parser_expr, block_br) EXPECT_THAT(module_parent_stack->codesec[0].instructions, ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0, - /*code_offset:*/ 19, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::drop, - Instr::end)); + /*code_offset:*/ 18, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::drop, Instr::end)); /* wat2wasm (func @@ -198,8 +195,7 @@ TEST(parser_expr, block_br) EXPECT_THAT(module_arity->codesec[0].instructions, ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 1, 0, 0, 0, - /*code_offset:*/ 19, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::drop, - Instr::end)); + /*code_offset:*/ 18, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::drop, Instr::end)); } TEST(parser_expr, block_return) @@ -211,8 +207,8 @@ TEST(parser_expr, block_return) const auto module = parse(wasm); EXPECT_THAT(module->codesec[0].instructions, - ElementsAre(Instr::return_, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 14, 0, 0, 0, - /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::end)); + ElementsAre(Instr::return_, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 13, 0, 0, 0, + /*stack_drop:*/ 0, 0, 0, 0, Instr::end)); } TEST(parser_expr, if_br) @@ -227,9 +223,9 @@ TEST(parser_expr, if_br) const auto module = parse(wasm); EXPECT_THAT(module->codesec[0].instructions, - ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::if_, /*else_offset:*/ 24, 0, 0, 0, - Instr::br, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 24, 0, 0, 0, - /*stack_drop:*/ 0, 0, 0, 0, Instr::end, /*24:*/ Instr::end)); + ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::if_, /*else_offset:*/ 23, 0, 0, 0, + Instr::br, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 23, 0, 0, 0, + /*stack_drop:*/ 0, 0, 0, 0, /*23:*/ Instr::end)); /* wat2wasm (func @@ -245,9 +241,9 @@ TEST(parser_expr, if_br) EXPECT_THAT(module_parent_stack->codesec[0].instructions, ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::i32_const, 0, 0, 0, 0, Instr::if_, - /*else_offset:*/ 29, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0, - /*code_offset:*/ 29, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, - /*29:*/ Instr::end, Instr::drop, Instr::end)); + /*else_offset:*/ 28, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0, + /*code_offset:*/ 28, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + /*28:*/ Instr::drop, Instr::end)); } TEST(parser_expr, instr_br_table) @@ -284,24 +280,24 @@ TEST(parser_expr, instr_br_table) EXPECT_THAT(code.instructions, ElementsAre(Instr::local_get, 0, 0, 0, 0, Instr::br_table, /*label_count:*/ 4, 0, 0, 0, /*arity:*/ 0, 0, 0, 0, - /*code_offset:*/ 130, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, - /*code_offset:*/ 111, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, - /*code_offset:*/ 92, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, - /*code_offset:*/ 73, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, - /*code_offset:*/ 149, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + /*code_offset:*/ 126, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + /*code_offset:*/ 108, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + /*code_offset:*/ 90, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + /*code_offset:*/ 72, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + /*code_offset:*/ 144, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, /*54:*/ Instr::i32_const, 0x41, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, - /*code_offset:*/ 154, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, - /*73:*/ Instr::i32_const, 0x42, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, - /*code_offset:*/ 154, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, - /*92:*/ Instr::i32_const, 0x43, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, - /*code_offset:*/ 154, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, - /*111:*/ Instr::i32_const, 0x44, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, - /*code_offset:*/ 154, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, - /*130:*/ Instr::i32_const, 0x45, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, - /*code_offset:*/ 154, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, - /*149:*/ Instr::i32_const, 0x46, 0, 0, 0, - /*154:*/ Instr::end)); + /*code_offset:*/ 149, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + /*72:*/ Instr::i32_const, 0x42, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, + /*code_offset:*/ 149, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + /*90:*/ Instr::i32_const, 0x43, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, + /*code_offset:*/ 149, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + /*108:*/ Instr::i32_const, 0x44, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, + /*code_offset:*/ 149, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + /*126:*/ Instr::i32_const, 0x45, 0, 0, 0, Instr::return_, /*arity:*/ 1, 0, 0, 0, + /*code_offset:*/ 149, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + /*144:*/ Instr::i32_const, 0x46, 0, 0, 0, + /*149:*/ Instr::end)); EXPECT_EQ(code.max_stack_height, 1); } @@ -326,10 +322,10 @@ TEST(parser_expr, instr_br_table_empty_vector) EXPECT_THAT(code.instructions, ElementsAre(Instr::local_get, 0, 0, 0, 0, Instr::br_table, - /*label_count:*/ 0, 0, 0, 0, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 41, 0, 0, 0, + /*label_count:*/ 0, 0, 0, 0, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 40, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::i32_const, 0x63, 0, 0, 0, Instr::return_, - /*arity:*/ 1, 0, 0, 0, /*code_offset:*/ 46, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, - Instr::end, Instr::i32_const, 0x64, 0, 0, 0, Instr::end)); + /*arity:*/ 1, 0, 0, 0, /*code_offset:*/ 45, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, + Instr::i32_const, 0x64, 0, 0, 0, Instr::end)); EXPECT_EQ(code.max_stack_height, 1); } From 2bdbbc556153bcd07f398ed6aae0d1cd8d4dca83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 12 Nov 2020 22:06:59 +0100 Subject: [PATCH 7/7] test: Add a test where only final end is produces in parser --- test/unittests/parser_expr_test.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/unittests/parser_expr_test.cpp b/test/unittests/parser_expr_test.cpp index 318603692..dc1104ab0 100644 --- a/test/unittests/parser_expr_test.cpp +++ b/test/unittests/parser_expr_test.cpp @@ -508,6 +508,7 @@ TEST(parser_expr, call_1arg_1result) EXPECT_EQ(module->codesec[0].max_stack_height, 1); EXPECT_EQ(module->codesec[1].max_stack_height, 1); } + TEST(parser_expr, call_nonexisting_typeidx) { // This creates a wasm module where code[0] has a call instruction calling function[1] which @@ -519,3 +520,28 @@ TEST(parser_expr, call_nonexisting_typeidx) EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "invalid function type index"); } + +TEST(parser_expr, nop_like_instructions_are_skipped) +{ + /* wat2wasm + (func + nop + (block + nop + (loop + nop + (block nop) + nop + ) + nop + ) + nop + ) + */ + const auto wasm = from_hex( + "0061736d01000000010401600000030201000a14011200010240010340010240010b010b010b010b"); + + const auto module = parse(wasm); + ASSERT_EQ(module->codesec.size(), 1); + EXPECT_THAT(module->codesec[0].instructions, ElementsAre(Instr::end)); +}