Merge pull request #407 from wasmx/validate-types-block

Validate block result types
wasmx · Jul 20, 2020 · 0a83cab · 0a83cab
2 parents e7a052a + 98ae70c
commit 0a83cab
Show file tree

Hide file tree

Showing 4 changed files with 167 additions and 39 deletions.
diff --git a/circle.yml b/circle.yml
@@ -270,8 +270,8 @@ jobs:
       - benchmark:
           min_time: "0.01"
       - spectest:
-          expected_passed: 5283
-          expected_failed: 149
+          expected_passed: 5347
+          expected_failed: 85
           expected_skipped: 6381
 
   sanitizers-macos:
@@ -288,8 +288,8 @@ jobs:
       - benchmark:
           min_time: "0.01"
       - spectest:
-          expected_passed: 5283
-          expected_failed: 149
+          expected_passed: 5347
+          expected_failed: 85
           expected_skipped: 6381
 
   benchmark:
@@ -400,8 +400,8 @@ jobs:
           expected_failed: 9
           expected_skipped: 7323
       - spectest:
-          expected_passed: 5283
-          expected_failed: 149
+          expected_passed: 5347
+          expected_failed: 85
           expected_skipped: 6381
       - collect_coverage_data
 

diff --git a/lib/fizzy/parser_expr.cpp b/lib/fizzy/parser_expr.cpp
@@ -112,7 +112,22 @@ void update_operand_stack(const ControlFrame& frame, Stack<ValType>& operand_sta
         operand_stack.push(output_type);
 }
 
-void validate_result_count(const ControlFrame& frame, const Stack<ValType>& operand_stack)
+inline void drop_operand(
+    const ControlFrame& frame, Stack<ValType>& operand_stack, std::optional<ValType> expected_type)
+{
+    if (!frame.unreachable &&
+        static_cast<int>(operand_stack.size()) < frame.parent_stack_height + 1)
+        throw validation_error{"stack underflow"};
+
+    if (frame.unreachable && static_cast<int>(operand_stack.size()) == frame.parent_stack_height)
+        return;
+
+    const auto actual_type = operand_stack.pop();
+    if (expected_type.has_value() && actual_type != *expected_type)
+        throw validation_error{"type mismatch"};
+}
+
+void update_result_stack(const ControlFrame& frame, Stack<ValType>& operand_stack)
 {
     const auto frame_stack_height = static_cast<int>(operand_stack.size());
 
@@ -124,11 +139,8 @@ void validate_result_count(const ControlFrame& frame, const Stack<ValType>& oper
     if (frame_stack_height > frame.parent_stack_height + arity)
         throw validation_error{"too many results"};
 
-    if (frame.unreachable)
-        return;
-
-    if (frame_stack_height < frame.parent_stack_height + arity)
-        throw validation_error{"missing result"};
+    if (arity != 0)
+        drop_operand(frame, operand_stack, frame.type);
 }
 
 inline std::optional<ValType> get_branch_frame_type(const ControlFrame& frame) noexcept
@@ -179,21 +191,6 @@ inline void mark_frame_unreachable(ControlFrame& frame, Stack<ValType>& operand_
     operand_stack.shrink(static_cast<size_t>(frame.parent_stack_height));
 }
 
-inline void drop_operand(
-    const ControlFrame& frame, Stack<ValType>& operand_stack, std::optional<ValType> expected_type)
-{
-    if (!frame.unreachable &&
-        static_cast<int>(operand_stack.size()) < frame.parent_stack_height + 1)
-        throw validation_error{"stack underflow"};
-
-    if (frame.unreachable && static_cast<int>(operand_stack.size()) == frame.parent_stack_height)
-        return;
-
-    const auto actual_type = operand_stack.pop();
-    if (expected_type.has_value() && actual_type != *expected_type)
-        throw validation_error{"type mismatch"};
-}
-
 inline void push_operand(Stack<ValType>& operand_stack, ValType type)
 {
     operand_stack.push(type);
@@ -469,13 +466,12 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f
             if (frame.instruction != Instr::if_)
                 throw parser_error{"unexpected else instruction (if instruction missing)"};
 
-            validate_result_count(frame, operand_stack);  // else is the end of if.
+            update_result_stack(frame, operand_stack);  // else is the end of if.
 
-            // Reset frame after if. The if result type validation not implemented yet.
+            // Reset frame after if.
             frame.unreachable = false;
             const auto if_imm_offset = frame.immediates_offset;
             frame.immediates_offset = code.immediates.size();
-            operand_stack.shrink(static_cast<size_t>(frame.parent_stack_height));
 
             // Placeholders for immediate values, filled at the matching end instructions.
             push(code.immediates, uint32_t{0});  // Diff to the end instruction.
@@ -496,7 +492,7 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f
 
         case Instr::end:
         {
-            validate_result_count(frame, operand_stack);
+            update_result_stack(frame, operand_stack);
 
             if (frame.instruction != Instr::loop)  // If end of block/if/else instruction.
             {

diff --git a/test/unittests/validation_stack_test.cpp b/test/unittests/validation_stack_test.cpp
@@ -95,7 +95,7 @@ TEST(validation_stack, func_missing_result)
     )
     */
     const auto wasm = from_hex("0061736d010000000105016000017f030201000a040102000b");
-    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "missing result");
+    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "stack underflow");
 }
 
 TEST(validation_stack, func_too_many_results)
@@ -157,7 +157,7 @@ TEST(validation_stack, block_missing_result)
     )
     */
     const auto wasm = from_hex("0061736d01000000010401600000030201000a08010600027f0b1a0b");
-    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "missing result");
+    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "stack underflow");
 }
 
 TEST(validation_stack, block_with_result_stack_underflow)
@@ -228,7 +228,7 @@ TEST(validation_stack, loop_missing_result)
     )
     */
     const auto wasm = from_hex("0061736d01000000010401600000030201000a08010600037f0b1a0b");
-    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "missing result");
+    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "stack underflow");
 }
 
 TEST(validation_stack, loop_with_result_stack_underflow)
@@ -781,7 +781,7 @@ TEST(validation_stack, if_missing_result)
     )
     */
     const auto wasm = from_hex("0061736d01000000010401600000030201000a0a0108004100047f0b1a0b");
-    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "missing result");
+    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "stack underflow");
 }
 
 TEST(validation_stack, if_missing_result_v2)
@@ -799,7 +799,7 @@ TEST(validation_stack, if_missing_result_v2)
     )
     */
     const auto wasm = from_hex("0061736d01000000010401600000030201000a0c010a004100047f05010b1a0b");
-    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "missing result");
+    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "stack underflow");
 }
 
 TEST(validation_stack, if_missing_result_v3)
@@ -819,7 +819,7 @@ TEST(validation_stack, if_missing_result_v3)
     */
     const auto wasm =
         from_hex("0061736d01000000010401600000030201000a0d010b004100047f0541020b1a0b");
-    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "missing result");
+    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "stack underflow");
 }
 
 TEST(validation_stack, else_missing_result)
@@ -839,7 +839,7 @@ TEST(validation_stack, else_missing_result)
     */
     const auto wasm =
         from_hex("0061736d01000000010401600000030201000a0e010c004100047f410105010b1a0b");
-    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "missing result");
+    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "stack underflow");
 }
 
 TEST(validation_stack, else_missing_result_v2)
@@ -861,7 +861,7 @@ TEST(validation_stack, else_missing_result_v2)
     */
     const auto wasm =
         from_hex("0061736d01000000010401600000030201000a10010e004100047f41010541021a0b1a0b");
-    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "missing result");
+    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "stack underflow");
 }
 
 TEST(validation_stack, else_stack_underflow)

diff --git a/test/unittests/validation_stack_type_test.cpp b/test/unittests/validation_stack_type_test.cpp
@@ -422,3 +422,135 @@ TEST(validation_stack_type, unreachable_global)
         from_hex("0061736d01000000010401600000030201000606017f0141000b0a0901070000420024000b");
     EXPECT_THROW_MESSAGE(parse(wasm_set_invalid), validation_error, "type mismatch");
 }
+
+TEST(validation_stack_type, block_type_mismatch)
+{
+    /* wat2wasm --no-check
+    (func (result i32)
+      i64.const 0
+    )
+    */
+    const auto wasm = from_hex("0061736d010000000105016000017f030201000a0601040042000b");
+    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "type mismatch");
+}
+
+TEST(validation_stack_type, unreachable_end)
+{
+    /* wat2wasm
+    (func (result i32)
+      unreachable
+    )
+    */
+    const auto wasm = from_hex("0061736d010000000105016000017f030201000a05010300000b");
+    EXPECT_NO_THROW(parse(wasm));
+
+    /* wat2wasm
+    (func (result i32)
+      unreachable
+      i32.const 0
+    )
+    */
+    const auto wasm_match = from_hex("0061736d010000000105016000017f030201000a070105000041000b");
+    EXPECT_NO_THROW(parse(wasm_match));
+
+    /* wat2wasm --no-check
+    (func (result i32)
+      unreachable
+      i64.const 0
+    )
+    */
+    const auto wasm_mismatch = from_hex("0061736d010000000105016000017f030201000a070105000042000b");
+    EXPECT_THROW_MESSAGE(parse(wasm_mismatch), validation_error, "type mismatch");
+}
+
+TEST(validation_stack_type, if_type_mismatch)
+{
+    /* wat2wasm --no-check
+    (func (result i32)
+      (i32.const 0)
+      (if (result i32)
+        (then (i64.const 0))
+      )
+    )
+    */
+    const auto wasm = from_hex("0061736d010000000105016000017f030201000a0b0109004100047f42000b0b");
+    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "type mismatch");
+
+    /* wat2wasm --no-check
+    (func (result i32)
+      (i32.const 0)
+      (if (result i32)
+        (then (i32.const 0))
+        (else (i64.const 0))
+      )
+    )
+    */
+    const auto wasm_else =
+        from_hex("0061736d010000000105016000017f030201000a0e010c004100047f41000542000b0b");
+    EXPECT_THROW_MESSAGE(parse(wasm_else), validation_error, "type mismatch");
+}
+
+TEST(validation_stack_type, if_unreachable)
+{
+    /* wat2wasm
+    (func (result i32)
+      (i32.const 0)
+      (if (result i32)
+        (then
+          unreachable
+          i32.const 0
+        )
+        (else (i32.const 0))
+      )
+    )
+    */
+    const auto wasm_then_unreachable =
+        from_hex("0061736d010000000105016000017f030201000a0f010d004100047f0041000541000b0b");
+    EXPECT_NO_THROW(parse(wasm_then_unreachable));
+
+    /* wat2wasm
+    (func (result i32)
+      (i32.const 0)
+      (if (result i32)
+        (then (i32.const 0))
+        (else
+          unreachable
+          i32.const 0
+        )
+      )
+    )
+    */
+    const auto wasm_else_unreachable =
+        from_hex("0061736d010000000105016000017f030201000a0f010d004100047f4100050041000b0b");
+    EXPECT_NO_THROW(parse(wasm_else_unreachable));
+
+    /* wat2wasm --no-check
+    (func (result i32)
+      (i32.const 0)
+      (if (result i32)
+        (then
+          unreachable
+          i64.const 0)
+        (else (i32.const 0))
+      )
+    )
+    */
+    const auto wasm_then_mismatch =
+        from_hex("0061736d010000000105016000017f030201000a0f010d004100047f0042000541000b0b");
+    EXPECT_THROW_MESSAGE(parse(wasm_then_mismatch), validation_error, "type mismatch");
+
+    /* wat2wasm --no-check
+    (func (result i32)
+      (i32.const 0)
+      (if (result i32)
+        (then (i32.const 0))
+        (else
+          unreachable
+          i64.const 0)
+      )
+    )
+    */
+    const auto wasm_else_mismatch =
+        from_hex("0061736d010000000105016000017f030201000a0f010d004100047f4100050042000b0b");
+    EXPECT_THROW_MESSAGE(parse(wasm_else_mismatch), validation_error, "type mismatch");
+}