[wasm] Decoder should narrow unreachable types on the stack.

R=rossberg@chromium.org

Bug: v8:6651
Change-Id: Iaa9217cacded9bdd3f0a35775275e79c231c272a
Reviewed-on: https://chromium-review.googlesource.com/642969Reviewed-by: Andreas Rossberg <rossberg@chromium.org>
Commit-Queue: Ben Titzer <titzer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47709}

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

@@ -1118,9 +1118,9 @@ class WasmFullDecoder : public WasmDecoder<validate> {
     return &stack_[stack_.size() - depth - 1];
   }
 
-  inline const Value& GetMergeValueFromStack(Control* c, size_t i) {
+  inline Value& GetMergeValueFromStack(Control* c, size_t i) {
     DCHECK_GT(c->merge.arity, i);
-    DCHECK_GE(stack_.size(), c->merge.arity);
+    DCHECK_GE(stack_.size(), c->stack_depth + c->merge.arity);
     return stack_[stack_.size() - c->merge.arity + i];
   }
 
@@ -2037,26 +2037,6 @@ class WasmFullDecoder : public WasmDecoder<validate> {
 
   int startrel(const byte* ptr) { return static_cast<int>(ptr - this->start_); }
 
-  bool TypeCheckBreak(unsigned depth) {
-    DCHECK(validate);  // Only call this for validation.
-    Control* c = control_at(depth);
-    if (c->is_loop()) {
-      // This is the inner loop block, which does not have a value.
-      return true;
-    }
-    size_t expected = control_.back().stack_depth + c->merge.arity;
-    if (stack_.size() < expected && !control_.back().unreachable) {
-      this->errorf(
-          this->pc_,
-          "expected at least %u values on the stack for br to @%d, found %d",
-          c->merge.arity, startrel(c->pc),
-          static_cast<int>(stack_.size() - c->stack_depth));
-      return false;
-    }
-
-    return TypeCheckMergeValues(c);
-  }
-
   void FallThruTo(Control* c) {
     DCHECK_EQ(c, &control_.back());
     if (!TypeCheckFallThru(c)) return;
@@ -2066,17 +2046,22 @@ class WasmFullDecoder : public WasmDecoder<validate> {
   }
 
   bool TypeCheckMergeValues(Control* c) {
-    // Typecheck the values left on the stack.
-    size_t avail = stack_.size() - c->stack_depth;
-    size_t start = avail >= c->merge.arity ? 0 : c->merge.arity - avail;
-    for (size_t i = start; i < c->merge.arity; ++i) {
+    DCHECK_GE(stack_.size(), c->stack_depth + c->merge.arity);
+    // Typecheck the topmost {c->merge.arity} values on the stack.
+    for (size_t i = 0; i < c->merge.arity; ++i) {
       auto& val = GetMergeValueFromStack(c, i);
       auto& old = c->merge[i];
-      if (val.type != old.type && val.type != kWasmVar) {
-        this->errorf(
-            this->pc_, "type error in merge[%zu] (expected %s, got %s)", i,
-            WasmOpcodes::TypeName(old.type), WasmOpcodes::TypeName(val.type));
-        return false;
+      if (val.type != old.type) {
+        if (val.type == kWasmVar) {
+          // if {val.type} is polymorphic, which results from unreachable, make
+          // it more specific by using the merge value's expected type.
+          val.type = old.type;
+        } else {
+          this->errorf(
+              this->pc_, "type error in merge[%zu] (expected %s, got %s)", i,
+              WasmOpcodes::TypeName(old.type), WasmOpcodes::TypeName(val.type));
+          return false;
+        }
       }
     }
 
@@ -2086,10 +2071,10 @@ class WasmFullDecoder : public WasmDecoder<validate> {
   bool TypeCheckFallThru(Control* c) {
     DCHECK_EQ(c, &control_.back());
     if (!validate) return true;
-    // Fallthru must match arity exactly.
-    size_t expected = c->stack_depth + c->merge.arity;
-    if (stack_.size() != expected &&
-        (stack_.size() > expected || !c->unreachable)) {
+    size_t expected = c->merge.arity;
+    size_t actual = stack_.size() - c->stack_depth;
+    // Fallthrus must match the arity of the control exactly.
+    if (!InsertUnreachablesIfNecessary(expected, actual) || actual > expected) {
       this->errorf(this->pc_,
                    "expected %u elements on the stack for fallthru to @%d",
                    c->merge.arity, startrel(c->pc));
@@ -2099,6 +2084,42 @@ class WasmFullDecoder : public WasmDecoder<validate> {
     return TypeCheckMergeValues(c);
   }
 
+  bool TypeCheckBreak(unsigned depth) {
+    DCHECK(validate);  // Only call this for validation.
+    Control* c = control_at(depth);
+    if (c->is_loop()) {
+      // This is the inner loop block, which does not have a value.
+      return true;
+    }
+    // Breaks must have at least the number of values expected; can have more.
+    size_t expected = c->merge.arity;
+    size_t actual = stack_.size() - control_.back().stack_depth;
+    if (!InsertUnreachablesIfNecessary(expected, actual)) {
+      this->errorf(this->pc_, "expected %u elements on the stack for br to @%d",
+                   c->merge.arity, startrel(c->pc));
+      return false;
+    }
+    return TypeCheckMergeValues(c);
+  }
+
+  inline bool InsertUnreachablesIfNecessary(size_t expected, size_t actual) {
+    if (actual < expected) {
+      if (control_.back().unreachable) {
+        // A slow path. When the actual number of values on the stack is less
+        // than the expected number of values and the current control is
+        // unreachable, insert unreachable values below the actual values.
+        // This simplifies {TypeCheckMergeValues}.
+        auto pos = stack_.begin() + (stack_.size() - actual);
+        stack_.insert(pos, (expected - actual), Value::Unreachable(this->pc_));
+        return true;
+      } else {
+        // There aren't enough values on the stack.
+        return false;
+      }
+    }
+    return true;  // enough actual values are there.
+  }
+
   virtual void onFirstError() {
     this->end_ = this->pc_;  // Terminate decoding loop.
     TRACE(" !%s\n", this->error_msg_.c_str());

test/mjsunit/wasm/unreachable-validation.js

@@ -128,3 +128,5 @@ run(I, "U (block (iblock 0 0 brt01) drop)", [unr, ...block, ...iblock, ...zero,
 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]);
+
+run(I, "(iblock (block (U brif 1))", [...iblock, ...block, unr, kExprBrIf, 0, end, end, kExprDrop]);

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

@@ -1846,6 +1846,14 @@ TEST_F(FunctionBodyDecoderTest, BreakIfBinop_fail) {
       WASM_BLOCK_I(WASM_F32_ABS(WASM_BRV_IF(0, WASM_F32(0.0f), WASM_ZERO))));
 }
 
+TEST_F(FunctionBodyDecoderTest, BreakIfUnrNarrow) {
+  EXPECT_FAILURE_S(
+      sigs.f_ff(),
+      WASM_BLOCK_I(WASM_BRV_IF(0, WASM_UNREACHABLE, WASM_UNREACHABLE),
+                   WASM_RETURN0),
+      WASM_F32(0.0));
+}
+
 TEST_F(FunctionBodyDecoderTest, BreakNesting1) {
   for (int i = 0; i < 5; i++) {
     // (block[2] (loop[2] (if (get p) break[N]) (set p 1)) p)