diff --git a/lib/fizzy/execute.cpp b/lib/fizzy/execute.cpp index 5b35a7e89..c162dc436 100644 --- a/lib/fizzy/execute.cpp +++ b/lib/fizzy/execute.cpp @@ -528,10 +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; case Instr::if_: { if (stack.pop().as() != 0) @@ -553,14 +549,12 @@ 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: - 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..879392ae4 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: @@ -462,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: @@ -472,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_: @@ -529,12 +531,10 @@ 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); + // 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_) { @@ -558,10 +558,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: diff --git a/test/unittests/parser_expr_test.cpp b/test/unittests/parser_expr_test.cpp index 0c235215e..dc1104ab0 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)); 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::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::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::drop, Instr::end)); EXPECT_EQ(code4.max_stack_height, 1); } @@ -64,18 +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::nop, Instr::nop, Instr::block, 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::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::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::drop, Instr::end)); } TEST(parser_expr, instr_block_input_buffer_overflow) @@ -94,8 +93,8 @@ 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, - /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::end)); + ElementsAre(Instr::br, /*arity:*/ 0, 0, 0, 0, /*code_offset:*/ 0, 0, 0, 0, + /*stack_drop:*/ 0, 0, 0, 0, Instr::end)); /* wat2wasm (func @@ -109,9 +108,8 @@ 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, - /*code_offset:*/ 5, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::end, Instr::drop, - Instr::end)); + 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::drop, Instr::end)); /* wat2wasm (func @@ -127,9 +125,8 @@ 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, - /*code_offset:*/ 0, 0, 0, 0, /*stack_drop:*/ 1, 0, 0, 0, Instr::end, Instr::drop, - Instr::end)); + 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::drop, Instr::end)); } TEST(parser_expr, loop_return) @@ -140,10 +137,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:*/ 13, 0, 0, 0, + /*stack_drop:*/ 0, 0, 0, 0, Instr::end)); } TEST(parser_expr, block_br) @@ -162,12 +158,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::nop, 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, - 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:*/ 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 @@ -182,9 +177,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, - Instr::end)); + ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 0, 0, 0, 0, + /*code_offset:*/ 18, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::drop, Instr::end)); /* wat2wasm (func @@ -200,9 +194,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, - Instr::end)); + ElementsAre(Instr::i32_const, 0, 0, 0, 0, Instr::br, /*arity:*/ 1, 0, 0, 0, + /*code_offset:*/ 18, 0, 0, 0, /*stack_drop:*/ 0, 0, 0, 0, Instr::drop, Instr::end)); } TEST(parser_expr, block_return) @@ -214,8 +207,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:*/ 13, 0, 0, 0, + /*stack_drop:*/ 0, 0, 0, 0, Instr::end)); } TEST(parser_expr, if_br) @@ -230,10 +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 @@ -249,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) @@ -286,27 +278,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:*/ 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:*/ 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); } @@ -330,11 +321,11 @@ 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:*/ 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:*/ 47, 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); } @@ -517,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 @@ -528,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)); +} 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)