[wasm] Remove redundant validation

And add some tests for (seemingly) previously uncovered cases.
The additional check for unreachable code is not needed any more, since
polymorphic stack values get assigned a specific type on their first
use or validation anyway. Hence the first entry in the br_table will
assign specific types to all polymorphic stack values, and type checking
will fail if later entries do not match.

R=rossberg@chromium.org
CC=titzer@chromium.org

Change-Id: I1d0f91f927a2aa5186f874112e91ebffa1f1b3a7
Reviewed-on: https://chromium-review.googlesource.com/675405Reviewed-by: Andreas Rossberg <rossberg@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48109}

src/wasm/function-body-decoder-impl.h

@@ -1396,7 +1396,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
             BranchTableIterator<validate> iterator(this, operand);
             if (!this->Validate(this->pc_, operand, control_.size())) break;
             auto key = Pop(0, kWasmI32);
-            MergeValues* merge = nullptr;
+            uint32_t br_arity = 0;
             while (iterator.has_next()) {
               const uint32_t i = iterator.cur_index();
               const byte* pos = iterator.pc();
@@ -1406,27 +1406,15 @@ class WasmFullDecoder : public WasmDecoder<validate> {
                 break;
               }
               // Check that label types match up.
-              static MergeValues loop_dummy = {0, {nullptr}};
               Control* c = control_at(target);
-              MergeValues* current = c->is_loop() ? &loop_dummy : &c->merge;
+              uint32_t arity = c->is_loop() ? 0 : c->merge.arity;
               if (i == 0) {
-                merge = current;
-              } else if (!VALIDATE(merge->arity == current->arity)) {
+                br_arity = arity;
+              } else if (!VALIDATE(br_arity == arity)) {
                 this->errorf(pos,
                              "inconsistent arity in br_table target %d"
                              " (previous was %u, this one %u)",
-                             i, merge->arity, current->arity);
-              } else if (control_at(0)->unreachable) {
-                for (uint32_t j = 0; j < merge->arity; ++j) {
-                  if (!VALIDATE((*merge)[j].type == (*current)[j].type)) {
-                    this->errorf(pos,
-                                 "type error in br_table target %d operand %d"
-                                 " (previous expected %s, this one %s)",
-                                 i, j, WasmOpcodes::TypeName((*merge)[j].type),
-                                 WasmOpcodes::TypeName((*current)[j].type));
-                    break;
-                  }
-                }
+                             i, br_arity, arity);
               }
               if (!VALIDATE(TypeCheckBreak(target))) break;
             }

test/unittests/wasm/function-body-decoder-unittest.cc

@@ -2165,6 +2165,69 @@ TEST_F(FunctionBodyDecoderTest, BrTable_invalid_br2) {
   }
 }
 
+TEST_F(FunctionBodyDecoderTest, BrTable_arity_mismatch1) {
+  EXPECT_FAILURE(
+      v_v,
+      WASM_BLOCK(WASM_BLOCK_I(
+          WASM_ONE, WASM_BR_TABLE(WASM_ONE, 1, BR_TARGET(0), BR_TARGET(1)))));
+}
+
+TEST_F(FunctionBodyDecoderTest, BrTable_arity_mismatch2) {
+  EXPECT_FAILURE(
+      v_v,
+      WASM_BLOCK_I(WASM_BLOCK(
+          WASM_ONE, WASM_BR_TABLE(WASM_ONE, 1, BR_TARGET(0), BR_TARGET(1)))));
+}
+
+TEST_F(FunctionBodyDecoderTest, BrTable_arity_mismatch_loop1) {
+  EXPECT_FAILURE(
+      v_v,
+      WASM_LOOP(WASM_BLOCK_I(
+          WASM_ONE, WASM_BR_TABLE(WASM_ONE, 1, BR_TARGET(0), BR_TARGET(1)))));
+}
+
+TEST_F(FunctionBodyDecoderTest, BrTable_arity_mismatch_loop2) {
+  EXPECT_FAILURE(
+      v_v,
+      WASM_BLOCK_I(WASM_LOOP(
+          WASM_ONE, WASM_BR_TABLE(WASM_ONE, 1, BR_TARGET(0), BR_TARGET(1)))));
+}
+
+TEST_F(FunctionBodyDecoderTest, BrTable_loop_block) {
+  EXPECT_VERIFIES(
+      v_v,
+      WASM_LOOP(WASM_BLOCK(
+          WASM_ONE, WASM_BR_TABLE(WASM_ONE, 1, BR_TARGET(0), BR_TARGET(1)))));
+}
+
+TEST_F(FunctionBodyDecoderTest, BrTable_block_loop) {
+  EXPECT_VERIFIES(
+      v_v,
+      WASM_LOOP(WASM_BLOCK(
+          WASM_ONE, WASM_BR_TABLE(WASM_ONE, 1, BR_TARGET(0), BR_TARGET(1)))));
+}
+
+TEST_F(FunctionBodyDecoderTest, BrTable_type_mismatch1) {
+  EXPECT_FAILURE(
+      v_v,
+      WASM_BLOCK_I(WASM_BLOCK_F(
+          WASM_ONE, WASM_BR_TABLE(WASM_ONE, 1, BR_TARGET(0), BR_TARGET(1)))));
+}
+
+TEST_F(FunctionBodyDecoderTest, BrTable_type_mismatch2) {
+  EXPECT_FAILURE(
+      v_v,
+      WASM_BLOCK_F(WASM_BLOCK_I(
+          WASM_ONE, WASM_BR_TABLE(WASM_ONE, 1, BR_TARGET(0), BR_TARGET(1)))));
+}
+
+TEST_F(FunctionBodyDecoderTest, BrTable_type_mismatch_unreachable) {
+  EXPECT_FAILURE(v_v,
+                 WASM_BLOCK_F(WASM_BLOCK_I(
+                     WASM_UNREACHABLE,
+                     WASM_BR_TABLE(WASM_ONE, 1, BR_TARGET(0), BR_TARGET(1)))));
+}
+
 TEST_F(FunctionBodyDecoderTest, BrUnreachable1) {
   EXPECT_VERIFIES(v_i, WASM_GET_LOCAL(0), kExprBrTable, 0, BR_TARGET(0));
 }