Merge pull request #427 from wasmx/validate-br-table-label-types

Validate br_table label types

Author: axic

Diff for: circle.yml

@@ -270,8 +270,8 @@ jobs:
       - benchmark:
           min_time: "0.01"
       - spectest:
-          expected_passed: 5282
-          expected_failed: 150
+          expected_passed: 5283
+          expected_failed: 149
           expected_skipped: 6381
 
   sanitizers-macos:
@@ -288,8 +288,8 @@ jobs:
       - benchmark:
           min_time: "0.01"
       - spectest:
-          expected_passed: 5282
-          expected_failed: 150
+          expected_passed: 5283
+          expected_failed: 149
           expected_skipped: 6381
 
   benchmark:
@@ -400,8 +400,8 @@ jobs:
           expected_failed: 9
           expected_skipped: 7323
       - spectest:
-          expected_passed: 5282
-          expected_failed: 150
+          expected_passed: 5283
+          expected_failed: 149
           expected_skipped: 6381
       - collect_coverage_data
 

Diff for: lib/fizzy/parser_expr.cpp

@@ -131,13 +131,16 @@ void validate_result_count(const ControlFrame& frame, const Stack<ValType>& oper
         throw validation_error{"missing result"};
 }
 
-inline uint8_t get_branch_arity(const ControlFrame& frame) noexcept
+inline std::optional<ValType> get_branch_frame_type(const ControlFrame& frame) noexcept
 {
-    const uint8_t arity = frame.type.has_value() ? 1 : 0;
-
     // For loops arity is considered always 0, because br executed in loop jumps to the top,
     // resetting frame stack to 0, so it should not keep top stack value even if loop has a result.
-    return frame.instruction == Instr::loop ? 0 : arity;
+    return frame.instruction == Instr::loop ? std::nullopt : frame.type;
+}
+
+inline uint8_t get_branch_arity(const ControlFrame& frame) noexcept
+{
+    return get_branch_frame_type(frame).has_value() ? 1 : 0;
 }
 
 inline void validate_branch_stack_height(const ControlFrame& current_frame,
@@ -579,7 +582,7 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f
                 throw validation_error{"invalid label index"};
 
             auto& default_branch_frame = control_stack[default_label_idx];
-            const auto default_branch_arity = get_branch_arity(default_branch_frame);
+            const auto default_branch_type = get_branch_frame_type(default_branch_frame);
 
             validate_branch_stack_height(frame, default_branch_frame, operand_stack);
 
@@ -589,7 +592,7 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, FuncIdx f
             {
                 auto& branch_frame = control_stack[idx];
 
-                if (get_branch_arity(branch_frame) != default_branch_arity)
+                if (get_branch_frame_type(branch_frame) != default_branch_type)
                     throw validation_error{"br_table labels have inconsistent types"};
 
                 branch_frame.br_immediate_offsets.push_back(code.immediates.size());

Diff for: test/unittests/validation_test.cpp

@@ -384,7 +384,7 @@ TEST(validation, br_table_default_invalid_label_index)
     EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "invalid label index");
 }
 
-TEST(validation, br_table_invalid_arity)
+TEST(validation, br_table_invalid_type)
 {
     /* wat2wasm --no-check
     (func  (param $x i32) (result i32)
@@ -450,6 +450,22 @@ TEST(validation, br_table_invalid_arity)
         "0061736d0100000001060160017f017f030201000a12011000027f037f410120000e0100010b0b0b");
 
     EXPECT_THROW_MESSAGE(parse(wasm4), validation_error, "br_table labels have inconsistent types");
+
+    /* wat2wasm --no-check
+    (func (param $x i32)
+      (block $a (result i32)
+        (block $b (result i64)
+          i32.const 1
+          local.get $x
+          br_table $a $b
+        )
+      )
+      drop
+    )
+    */
+    const auto wasm5 = from_hex(
+        "0061736d0100000001050160017f00030201000a13011100027f027e410120000e0101000b0b1a0b");
+    EXPECT_THROW_MESSAGE(parse(wasm5), validation_error, "br_table labels have inconsistent types");
 }
 
 TEST(validation, call_unknown_function)