Merge pull request #368 from wasmx/validate-br-table

Validate arity of labels in br_table

circle.yml

@@ -252,8 +252,8 @@ jobs:
       - benchmark:
           min_time: "0.01"
       - spectest:
-          expected_passed: 4901
-          expected_failed: 531
+          expected_passed: 4903
+          expected_failed: 529
           expected_skipped: 6381
 
   sanitizers-macos:
@@ -270,8 +270,8 @@ jobs:
       - benchmark:
           min_time: "0.01"
       - spectest:
-          expected_passed: 4901
-          expected_failed: 531
+          expected_passed: 4903
+          expected_failed: 529
           expected_skipped: 6381
 
   benchmark:
@@ -381,8 +381,8 @@ jobs:
           expected_failed: 8
           expected_skipped: 7323
       - spectest:
-          expected_passed: 4901
-          expected_failed: 531
+          expected_passed: 4903
+          expected_failed: 529
           expected_skipped: 6381
 
 workflows:

lib/fizzy/parser_expr.cpp

@@ -119,16 +119,21 @@ void validate_result_count(const ControlFrame& frame)
     //     throw validation_error{"too many results"};
 }
 
+inline uint8_t get_branch_arity(const ControlFrame& frame) noexcept
+{
+    // 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 : frame.arity;
+}
+
 void push_branch_immediates(const ControlFrame& frame, bytes& immediates)
 {
     // Push frame start location as br immediates - these are final if frame is loop,
     // but for block/if/else these are just placeholders, to be filled at end instruction.
     push(immediates, static_cast<uint32_t>(frame.code_offset));
     push(immediates, static_cast<uint32_t>(frame.immediates_offset));
     push(immediates, static_cast<uint32_t>(frame.parent_stack_height));
-    // Always pushing 0 as loop's arity, 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.
-    push(immediates, frame.instruction == Instr::loop ? uint8_t{0} : frame.arity);
+    push(immediates, get_branch_arity(frame));
 }
 
 }  // namespace
@@ -481,19 +486,23 @@ parser_result<Code> parse_expr(
             if (default_label_idx >= control_stack.size())
                 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);
+
             // Remember immediates offset for all br items to fill them at end instruction.
             push(code.immediates, static_cast<uint32_t>(label_indices.size()));
             for (const auto idx : label_indices)
             {
                 auto& branch_frame = control_stack[idx];
-                branch_frame.br_immediate_offsets.push_back(code.immediates.size());
 
+                if (get_branch_arity(branch_frame) != default_branch_arity)
+                    throw validation_error{"br_table labels have inconsistent types"};
+
+                branch_frame.br_immediate_offsets.push_back(code.immediates.size());
                 push_branch_immediates(branch_frame, code.immediates);
             }
 
-            auto& default_branch_frame = control_stack[default_label_idx];
             default_branch_frame.br_immediate_offsets.push_back(code.immediates.size());
-
             push_branch_immediates(default_branch_frame, code.immediates);
 
             frame.unreachable = true;

test/unittests/execute_control_test.cpp

@@ -934,3 +934,68 @@ TEST(execute_control, if_br_from_branch)
         EXPECT_THAT(execute(module, 0, {param}), Result(expected_results[param]));
     }
 }
+
+TEST(execute_control, br_table_arity)
+{
+    /* wat2wasm
+    (func  (param $x i32) (result i32)
+      (block $a (result i32)
+        (block $b (result i32)
+          i32.const 1
+          local.get $x
+          br_table $a $b
+        )
+        drop
+        i32.const 2
+      )
+    )
+    */
+    const auto wasm = from_hex(
+        "0061736d0100000001060160017f017f030201000a15011300027f027f410120000e0101000b1a41020b0b");
+
+    auto instance = parse(wasm);
+    EXPECT_THAT(execute(instance, 0, {0}), Result(1));
+    EXPECT_THAT(execute(instance, 0, {1}), Result(2));
+
+    /* wat2wasm
+    (func  (param $x i32) (result i32)
+      (block $a
+        (loop $b (result i32)
+          local.get $x
+          i32.const 1
+          i32.sub
+          local.tee $x
+          br_table $a $b
+        )
+        drop
+      )
+      i32.const 2
+    )
+    */
+    const auto wasm2 = from_hex(
+        "0061736d0100000001060160017f017f030201000a180116000240037f200041016b22000e0101000b1a0b4102"
+        "0b");
+
+    EXPECT_THAT(execute(parse(wasm2), 0, {2}), Result(2));
+
+    /* wat2wasm
+    (func  (param $x i32) (result i32)
+      (loop $a (result i32)
+        (block $b
+          local.get $x
+          i32.const 1
+          i32.add
+          local.tee $x
+          br_table $a $b
+        )
+        i32.const 2
+        return
+      )
+    )
+    */
+    const auto wasm3 = from_hex(
+        "0061736d0100000001060160017f017f030201000a18011600037f0240200041016a22000e0101000b41020f0b"
+        "0b");
+
+    EXPECT_THAT(execute(parse(wasm3), 0, {uint32_t(-1)}), Result(2));
+}

test/unittests/validation_test.cpp

@@ -196,6 +196,74 @@ TEST(validation, br_table_default_invalid_label_index)
     EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "invalid label index");
 }
 
+TEST(validation, br_table_invalid_arity)
+{
+    /* wat2wasm --no-check
+    (func  (param $x i32) (result i32)
+      (block $a (result i32)
+        (block $b
+          local.get $x
+          br_table $a $b
+        )
+        i32.const 2
+      )
+    )
+    */
+    const auto wasm = from_hex(
+        "0061736d0100000001060160017f017f030201000a12011000027f024020000e0101000b41020b0b");
+
+    EXPECT_THROW_MESSAGE(parse(wasm), validation_error, "br_table labels have inconsistent types");
+
+    /* wat2wasm --no-check
+    (func  (param $x i32)
+      (block $a
+        (block $b (result i32)
+          i32.const 1
+          local.get $x
+          br_table $b $a
+        )
+        drop
+      )
+    )
+    */
+    const auto wasm2 = from_hex(
+        "0061736d0100000001050160017f00030201000a130111000240027f410120000e0100010b1a0b0b");
+
+    EXPECT_THROW_MESSAGE(parse(wasm2), validation_error, "br_table labels have inconsistent types");
+
+    /* wat2wasm --no-check
+    (func (param $x i32) (result i32)
+      (loop $a (result i32)
+        (block $b (result i32)
+          i32.const 1
+          local.get $x
+          br_table $b $a
+        )
+      )
+    )
+    */
+    const auto wasm3 = from_hex(
+        "0061736d0100000001060160017f017f030201000a12011000037f027f410120000e0100010b0b0b");
+
+    EXPECT_THROW_MESSAGE(parse(wasm3), validation_error, "br_table labels have inconsistent types");
+
+    /* wat2wasm --no-check
+    (func (param $x i32) (result i32)
+      (block $a (result i32)
+        (loop $b (result i32)
+          i32.const 1
+          local.get $x
+          br_table $b $a
+        )
+      )
+    )
+    */
+    const auto wasm4 = from_hex(
+        "0061736d0100000001060160017f017f030201000a12011000027f037f410120000e0100010b0b0b");
+
+    EXPECT_THROW_MESSAGE(parse(wasm4), validation_error, "br_table labels have inconsistent types");
+}
+
 TEST(validation, call_unknown_function)
 {
     /* wat2wasm --no-check