[wasm] Inspect right control frames for unreachable flag

We were looking at the unreachable flag or stack_depth of the target frame
instead of the current one in a couple of places (most notably BreakTo).
This change fixes these bugs and makes us pass the latest spec tests for
br_table validation. Also need to ensure that br_table targets have consistent
types, which is not implied if the stack is polymorphic.

R=titzer@chromium.org
BUG=

Review-Url: https://codereview.chromium.org/2696813002
Cr-Commit-Position: refs/heads/master@{#43250}

src/wasm/function-body-decoder.cc

@@ -89,9 +89,9 @@ struct MergeValues {
     Value first;
   } vals;  // Either multiple values or a single value.
 
-  Value& first() {
-    DCHECK_GT(arity, 0);
-    return arity == 1 ? vals.first : vals.array[0];
+  Value& operator[](size_t i) {
+    DCHECK_GT(arity, i);
+    return arity == 1 ? vals.first : vals.array[i];
   }
 };
 
@@ -960,6 +960,7 @@ class WasmFullDecoder : public WasmDecoder {
 
                 SsaEnv* copy = Steal(break_env);
                 ssa_env_ = copy;
+                MergeValues* merge = nullptr;
                 while (ok() && iterator.has_next()) {
                   uint32_t i = iterator.cur_index();
                   const byte* pos = iterator.pc();
@@ -973,6 +974,26 @@ class WasmFullDecoder : public WasmDecoder {
                                           ? BUILD(IfDefault, sw)
                                           : BUILD(IfValue, i, sw);
                   BreakTo(target);
+
+                  // Check that label types match up.
+                  Control* c = &control_[control_.size() - target - 1];
+                  if (i == 0) {
+                    merge = &c->merge;
+                  } else if (merge->arity != c->merge.arity) {
+                    error(pos, pos, "inconsistent arity in br_table target %d"
+                          " (previous was %u, this one %u)",
+                          i, merge->arity, c->merge.arity);
+                  } else if (control_.back().unreachable) {
+                    for (uint32_t j = 0; ok() && j < merge->arity; ++j) {
+                      if ((*merge)[j].type != c->merge[j].type) {
+                        error(pos, 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(c->merge[j].type));
+                      }
+                    }
+                  }
                 }
                 if (failed()) break;
               } else {
@@ -1507,6 +1528,7 @@ class WasmFullDecoder : public WasmDecoder {
   }
 
   void PushEndValues(Control* c) {
+    DCHECK_EQ(c, &control_.back());
     stack_.resize(c->stack_depth);
     if (c->merge.arity == 1) {
       stack_.push_back(c->merge.vals.first);
@@ -1568,8 +1590,8 @@ class WasmFullDecoder : public WasmDecoder {
       Goto(ssa_env_, c->end_env);
     } else {
       // Merge the value(s) into the end of the block.
-      size_t expected = c->stack_depth + c->merge.arity;
-      if (!c->unreachable && stack_.size() < expected) {
+      size_t expected = control_.back().stack_depth + c->merge.arity;
+      if (stack_.size() < expected && !control_.back().unreachable) {
         error(
             pc_, pc_,
             "expected at least %u values on the stack for br to @%d, found %d",
@@ -1582,10 +1604,11 @@ class WasmFullDecoder : public WasmDecoder {
   }
 
   void FallThruTo(Control* c) {
+    DCHECK_EQ(c, &control_.back());
     // Merge the value(s) into the end of the block.
     size_t expected = c->stack_depth + c->merge.arity;
     if (stack_.size() == expected ||
-        (c->unreachable && stack_.size() < expected)) {
+        (stack_.size() < expected && c->unreachable)) {
       MergeValuesInto(c);
       c->unreachable = false;
       return;
@@ -1599,10 +1622,11 @@ class WasmFullDecoder : public WasmDecoder {
   }
 
   void TypeCheckFallThru(Control* c) {
+    DCHECK_EQ(c, &control_.back());
     // Fallthru must match arity exactly.
     int arity = static_cast<int>(c->merge.arity);
     if (c->stack_depth + arity < stack_.size() ||
-        (!c->unreachable && c->stack_depth + arity != stack_.size())) {
+        (c->stack_depth + arity != stack_.size() && !c->unreachable)) {
       error(pc_, pc_, "expected %d elements on the stack for fallthru to @%d",
             arity, startrel(c->pc));
       return;
@@ -1612,8 +1636,7 @@ class WasmFullDecoder : public WasmDecoder {
     for (size_t i = avail >= c->merge.arity ? 0 : c->merge.arity - avail;
          i < c->merge.arity; i++) {
       Value& val = GetMergeValueFromStack(c, i);
-      Value& old =
-          c->merge.arity == 1 ? c->merge.vals.first : c->merge.vals.array[i];
+      Value& old = c->merge[i];
       if (val.type != old.type) {
         error(pc_, pc_, "type error in merge[%zu] (expected %s, got %s)", i,
               WasmOpcodes::TypeName(old.type), WasmOpcodes::TypeName(val.type));
@@ -1628,12 +1651,11 @@ class WasmFullDecoder : public WasmDecoder {
     bool reachable = ssa_env_->go();
     Goto(ssa_env_, target);
 
-    size_t avail = stack_.size() - c->stack_depth;
+    size_t avail = stack_.size() - control_.back().stack_depth;
     for (size_t i = avail >= c->merge.arity ? 0 : c->merge.arity - avail;
          i < c->merge.arity; i++) {
       Value& val = GetMergeValueFromStack(c, i);
-      Value& old =
-          c->merge.arity == 1 ? c->merge.vals.first : c->merge.vals.array[i];
+      Value& old = c->merge[i];
       if (val.type != old.type && val.type != kWasmVar) {
         error(pc_, pc_, "type error in merge[%zu] (expected %s, got %s)", i,
               WasmOpcodes::TypeName(old.type), WasmOpcodes::TypeName(val.type));

test/mjsunit/wasm/function-names.js

@@ -10,7 +10,7 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
 var builder = new WasmModuleBuilder();
 
 var last_func_index = builder.addFunction("exec_unreachable", kSig_v_v)
-  .addBody([kExprUnreachable])
+  .addBody([kExprUnreachable]).index
 
 var illegal_func_name = [0xff];
 var func_names = [ "☠", illegal_func_name, "some math: (½)² = ¼", "" ];

test/mjsunit/wasm/globals.js

@@ -13,7 +13,7 @@ function TestImported(type, val, expected) {
   var sig = makeSig([], [type]);
   var g = builder.addImportedGlobal("uuu", "foo", type);
   builder.addFunction("main", sig)
-    .addBody([kExprGetGlobal, g.index])
+    .addBody([kExprGetGlobal, g])
     .exportAs("main");
   builder.addGlobal(kWasmI32);  // pad
 

test/mjsunit/wasm/unreachable-validation.js

@@ -40,7 +40,10 @@ let iadd = kExprI32Add;
 let unr = kExprUnreachable;
 let ret = kExprReturn;
 let br0 = [kExprBr, 0];
-let brt = [kExprBrTable, 0, 0];
+let br1 = [kExprBr, 1];
+let brt0 = [kExprBrTable, 0, 0];
+let brt1 = [kExprBrTable, 0, 1];
+let brt01 = [kExprBrTable, 1, 0, 1];
 let f32 = [kExprF32Const, 0, 0, 0, 0];
 let zero = [kExprI32Const, 0];
 let if_else_empty = [kExprIf, kWasmStmt, kExprElse, kExprEnd];
@@ -49,6 +52,10 @@ let if_else_unr = [kExprIf, kWasmStmt, kExprUnreachable, kExprElse, kExprUnreach
 let block_unr = [kExprBlock, kWasmStmt, kExprUnreachable, kExprEnd];
 let loop_unr = [kExprLoop, kWasmStmt, kExprUnreachable, kExprEnd];
 let block_block_unr = [kExprBlock, kWasmStmt, kExprBlock, kWasmStmt, kExprUnreachable, kExprEnd, kExprEnd];
+let block = [kExprBlock, kWasmStmt]
+let iblock = [kExprBlock, kWasmI32]
+let fblock = [kExprBlock, kWasmF32]
+let end = kExprEnd;
 let drop = kExprDrop;
 
 run(V, "U", [unr]);
@@ -68,16 +75,16 @@ run(V, "(if 0 U U)", [...zero, ...if_else_unr]);
 run(V, 'U nop', [unr, nop]);
 run(V, 'U iadd drop', [unr, iadd, drop]);
 run(V, 'br0 iadd drop', [...br0, iadd, drop]);
-run(V, '0 brt iadd drop', [...zero, ...brt, iadd, drop]);
+run(V, '0 brt0 iadd drop', [...zero, ...brt0, iadd, drop]);
 run(V, 'ret iadd drop', [ret, iadd, drop]);
 
 run(V, 'U 0 0 iadd drop', [unr, ...zero, ...zero, iadd, drop]);
 run(V, 'br0 0 0 iadd drop', [...br0, ...zero, ...zero, iadd, drop]);
-run(V, '0 brt 0 0 iadd drop', [...zero, ...brt, ...zero, ...zero, iadd, drop]);
+run(V, '0 brt0 0 0 iadd drop', [...zero, ...brt0, ...zero, ...zero, iadd, drop]);
 run(V, 'ret 0 0 iadd drop', [ret, ...zero, ...zero, iadd, drop]);
 
 run(I, 'br0 iadd', [...br0, iadd]);
-run(I, '0 brt iadd', [...zero, ...brt, iadd]);
+run(I, '0 brt0 iadd', [...zero, ...brt0, iadd]);
 run(I, 'ret iadd', [ret, iadd]);
 run(I, '0 0 br0 iadd', [...zero, ...zero, ...br0, iadd]);
 run(I, '0 0 ret iadd', [...zero, ...zero, ret, iadd]);
@@ -111,3 +118,13 @@ run(V, '0f U 0 iadd drop', [...f32, unr, ...zero, iadd, drop]);
 run(I, "0 U 0f iadd drop", [...zero, unr, ...zero, ...f32, iadd, drop]);
 run(I, "0f (block U) 0 iadd drop", [...f32, ...block_unr, ...zero, iadd, drop]);
 run(I, "0 (block U) 0f iadd drop", [...zero, ...block_unr, ...f32, iadd, drop]);
+
+run(I, "(iblock 0 (block br1)) drop", [...iblock, ...zero, ...block, ...br1, end, end, drop]);
+run(I, "(iblock 0 (block 0 brt1)) drop", [...iblock, ...zero, ...block, ...zero, ...brt1, end, end, drop]);
+run(I, "(block (iblock 0 0 brt01) drop)", [...block, ...iblock, ...zero, ...zero, ...brt01, end, drop, end]);
+run(I, "U (iblock 0 (block br1)) drop", [unr, ...iblock, ...zero, ...block, ...br1, end, end, drop]);
+run(I, "U (iblock 0 (block 0 brt1)) drop", [unr, ...iblock, ...zero, ...block, ...zero, ...brt1, end, end, drop]);
+run(I, "U (block (iblock 0 0 brt01) drop)", [unr, ...block, ...iblock, ...zero, ...zero, ...brt01, end, drop, end]);
+run(V, "(iblock (iblock U 0 brt01)) drop", [...iblock, ...iblock, unr, ...zero, ...brt01, end, end, drop]);
+run(I, "(block (fblock U 0 brt01) drop)", [...iblock, ...fblock, unr, ...zero, ...brt01, end, drop, end]);
+run(I, "(iblock (fblock U 0 brt01) drop 0) drop", [...iblock, ...fblock, unr, ...zero, ...brt01, end, drop, ...zero, end, drop]);

test/mjsunit/wasm/wasm-module-builder.js

@@ -97,6 +97,9 @@ class WasmFunctionBuilder {
   }
 
   addBody(body) {
+    for (let b of body) {
+      if (typeof b != 'number') throw new Error("invalid body");
+    }
     this.body = body;
     // Automatically add the end for the function block to the body.
     body.push(kExprEnd);