[wasm] [interpreter] Ignore stack effects after unreachable

During computation of the side table, ignore stack effects of
instructions following any unconditional jump in the same block
(|unreachable|, |br|, |br_table| or |return| jump out of the block).
Without this fix, the current stack height might underflow, or we compute an
unnecessarily large max_stack_height_. Note that those instruction will
never get executed anyway.
Hence, we don't need to store any side table information for such
unreachable code.

R=rossberg@chromium.org
BUG=chromium:716936, chromium:715990

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

src/wasm/wasm-interpreter.cc

@@ -744,6 +744,9 @@ class SideTable : public ZoneObject {
       // Arity (number of values on the stack) when exiting this control
       // structure via |end|.
       uint32_t exit_arity;
+      // Track whether this block was already left, i.e. all further
+      // instructions are unreachable.
+      bool unreachable = false;
 
       Control(const byte* pc, CLabel* end_label, CLabel* else_label,
               uint32_t exit_arity)
@@ -772,19 +775,32 @@ class SideTable : public ZoneObject {
     CLabel* func_label =
         CLabel::New(&control_transfer_zone, stack_height, func_arity);
     control_stack.emplace_back(code->orig_start, func_label, func_arity);
+    auto control_parent = [&]() -> Control& {
+      DCHECK_LE(2, control_stack.size());
+      return control_stack[control_stack.size() - 2];
+    };
+    auto copy_unreachable = [&] {
+      control_stack.back().unreachable = control_parent().unreachable;
+    };
     for (BytecodeIterator i(code->orig_start, code->orig_end, &code->locals);
          i.has_next(); i.next()) {
       WasmOpcode opcode = i.current();
-      auto stack_effect =
-          StackEffect(module, code->function->sig, i.pc(), i.end());
-      TRACE("@%u: control %s (sp %d - %d + %d)\n", i.pc_offset(),
-            WasmOpcodes::OpcodeName(opcode), stack_height, stack_effect.first,
-            stack_effect.second);
-      DCHECK_GE(stack_height, stack_effect.first);
-      DCHECK_GE(kMaxUInt32, static_cast<uint64_t>(stack_height) -
-                                stack_effect.first + stack_effect.second);
-      stack_height = stack_height - stack_effect.first + stack_effect.second;
-      if (stack_height > max_stack_height_) max_stack_height_ = stack_height;
+      bool unreachable = control_stack.back().unreachable;
+      if (unreachable) {
+        TRACE("@%u: %s (is unreachable)\n", i.pc_offset(),
+              WasmOpcodes::OpcodeName(opcode));
+      } else {
+        auto stack_effect =
+            StackEffect(module, code->function->sig, i.pc(), i.end());
+        TRACE("@%u: %s (sp %d - %d + %d)\n", i.pc_offset(),
+              WasmOpcodes::OpcodeName(opcode), stack_height, stack_effect.first,
+              stack_effect.second);
+        DCHECK_GE(stack_height, stack_effect.first);
+        DCHECK_GE(kMaxUInt32, static_cast<uint64_t>(stack_height) -
+                                  stack_effect.first + stack_effect.second);
+        stack_height = stack_height - stack_effect.first + stack_effect.second;
+        if (stack_height > max_stack_height_) max_stack_height_ = stack_height;
+      }
       switch (opcode) {
         case kExprBlock:
         case kExprLoop: {
@@ -795,6 +811,7 @@ class SideTable : public ZoneObject {
           CLabel* label = CLabel::New(&control_transfer_zone, stack_height,
                                       is_loop ? 0 : operand.arity);
           control_stack.emplace_back(i.pc(), label, operand.arity);
+          copy_unreachable();
           if (is_loop) label->Bind(i.pc());
           break;
         }
@@ -807,13 +824,17 @@ class SideTable : public ZoneObject {
               CLabel::New(&control_transfer_zone, stack_height, 0);
           control_stack.emplace_back(i.pc(), end_label, else_label,
                                      operand.arity);
-          else_label->Ref(i.pc(), stack_height);
+          copy_unreachable();
+          if (!unreachable) else_label->Ref(i.pc(), stack_height);
           break;
         }
         case kExprElse: {
           Control* c = &control_stack.back();
+          copy_unreachable();
           TRACE("control @%u: Else\n", i.pc_offset());
-          c->end_label->Ref(i.pc(), stack_height);
+          if (!control_parent().unreachable) {
+            c->end_label->Ref(i.pc(), stack_height);
+          }
           DCHECK_NOT_NULL(c->else_label);
           c->else_label->Bind(i.pc() + 1);
           c->else_label->Finish(&map_, code->orig_start);
@@ -841,14 +862,14 @@ class SideTable : public ZoneObject {
           BreakDepthOperand<false> operand(&i, i.pc());
           TRACE("control @%u: Br[depth=%u]\n", i.pc_offset(), operand.depth);
           Control* c = &control_stack[control_stack.size() - operand.depth - 1];
-          c->end_label->Ref(i.pc(), stack_height);
+          if (!unreachable) c->end_label->Ref(i.pc(), stack_height);
           break;
         }
         case kExprBrIf: {
           BreakDepthOperand<false> operand(&i, i.pc());
           TRACE("control @%u: BrIf[depth=%u]\n", i.pc_offset(), operand.depth);
           Control* c = &control_stack[control_stack.size() - operand.depth - 1];
-          c->end_label->Ref(i.pc(), stack_height);
+          if (!unreachable) c->end_label->Ref(i.pc(), stack_height);
           break;
         }
         case kExprBrTable: {
@@ -856,17 +877,21 @@ class SideTable : public ZoneObject {
           BranchTableIterator<false> iterator(&i, operand);
           TRACE("control @%u: BrTable[count=%u]\n", i.pc_offset(),
                 operand.table_count);
-          while (iterator.has_next()) {
-            uint32_t j = iterator.cur_index();
-            uint32_t target = iterator.next();
-            Control* c = &control_stack[control_stack.size() - target - 1];
-            c->end_label->Ref(i.pc() + j, stack_height);
+          if (!unreachable) {
+            while (iterator.has_next()) {
+              uint32_t j = iterator.cur_index();
+              uint32_t target = iterator.next();
+              Control* c = &control_stack[control_stack.size() - target - 1];
+              c->end_label->Ref(i.pc() + j, stack_height);
+            }
           }
           break;
         }
-        default: {
+        default:
           break;
-        }
+      }
+      if (WasmOpcodes::IsUnconditionalJump(opcode)) {
+        control_stack.back().unreachable = true;
       }
     }
     DCHECK_EQ(0, control_stack.size());

src/wasm/wasm-opcodes.cc

@@ -286,6 +286,7 @@ bool WasmOpcodes::IsPrefixOpcode(WasmOpcode opcode) {
       return false;
   }
 }
+
 bool WasmOpcodes::IsControlOpcode(WasmOpcode opcode) {
   switch (opcode) {
 #define CHECK_OPCODE(name, opcode, _) \
@@ -298,6 +299,18 @@ bool WasmOpcodes::IsControlOpcode(WasmOpcode opcode) {
   }
 }
 
+bool WasmOpcodes::IsUnconditionalJump(WasmOpcode opcode) {
+  switch (opcode) {
+    case kExprUnreachable:
+    case kExprBr:
+    case kExprBrTable:
+    case kExprReturn:
+      return true;
+    default:
+      return false;
+  }
+}
+
 std::ostream& operator<<(std::ostream& os, const FunctionSig& sig) {
   if (sig.return_count() == 0) os << "v";
   for (auto ret : sig.returns()) {

src/wasm/wasm-opcodes.h

@@ -598,6 +598,9 @@ class V8_EXPORT_PRIVATE WasmOpcodes {
   static FunctionSig* AtomicSignature(WasmOpcode opcode);
   static bool IsPrefixOpcode(WasmOpcode opcode);
   static bool IsControlOpcode(WasmOpcode opcode);
+  // Check whether the given opcode always jumps, i.e. all instructions after
+  // this one in the current block are dead. Returns false for |end|.
+  static bool IsUnconditionalJump(WasmOpcode opcode);
 
   static int TrapReasonToMessageId(TrapReason reason);
   static const char* TrapReasonMessage(TrapReason reason);

test/cctest/wasm/test-run-wasm.cc

@@ -3007,3 +3007,46 @@ WASM_EXEC_TEST(LoopsWithValues) {
   BUILD(r, WASM_LOOP_I(WASM_LOOP_I(WASM_ONE), WASM_ONE, kExprI32Add));
   CHECK_EQ(2, r.Call());
 }
+
+WASM_EXEC_TEST(InvalidStackAfterUnreachable) {
+  WasmRunner<int32_t> r(execution_mode);
+  BUILD(r, kExprUnreachable, kExprI32Add);
+  CHECK_TRAP32(r.Call());
+}
+
+WASM_EXEC_TEST(InvalidStackAfterBr) {
+  WasmRunner<int32_t> r(execution_mode);
+  BUILD(r, WASM_BRV(0, WASM_I32V_1(27)), kExprI32Add);
+  CHECK_EQ(27, r.Call());
+}
+
+WASM_EXEC_TEST(InvalidStackAfterReturn) {
+  WasmRunner<int32_t> r(execution_mode);
+  BUILD(r, WASM_RETURN1(WASM_I32V_1(17)), kExprI32Add);
+  CHECK_EQ(17, r.Call());
+}
+
+WASM_EXEC_TEST(BranchOverUnreachableCode) {
+  WasmRunner<int32_t> r(execution_mode);
+  BUILD(r,
+        // Start a block which breaks in the middle (hence unreachable code
+        // afterwards) and continue execution after this block.
+        WASM_BLOCK_I(WASM_BRV(0, WASM_I32V_1(17)), kExprI32Add),
+        // Add one to the 17 returned from the block.
+        WASM_ONE, kExprI32Add);
+  CHECK_EQ(18, r.Call());
+}
+
+WASM_EXEC_TEST(BlockInsideUnreachable) {
+  WasmRunner<int32_t> r(execution_mode);
+  BUILD(r, WASM_RETURN1(WASM_I32V_1(17)), WASM_BLOCK(WASM_BR(0)));
+  CHECK_EQ(17, r.Call());
+}
+
+WASM_EXEC_TEST(IfInsideUnreachable) {
+  WasmRunner<int32_t> r(execution_mode);
+  BUILD(
+      r, WASM_RETURN1(WASM_I32V_1(17)),
+      WASM_IF_ELSE_I(WASM_ONE, WASM_BRV(0, WASM_ONE), WASM_RETURN1(WASM_ONE)));
+  CHECK_EQ(17, r.Call());
+}

test/unittests/wasm/control-transfer-unittest.cc

@@ -489,14 +489,41 @@ TEST_F(ControlTransferTest, BiggerSpDiffs) {
       0,              // @7
       kExprI32Const,  // @8
       0,              // @9
-      kExprBr,        // @10
+      kExprI32Const,  // @10
       0,              // @11
-      kExprBr,        // @12
-      1,              // @13
-      kExprEnd,       // @14
-      kExprEnd        // @15
+      kExprBrIf,      // @12
+      0,              // @13
+      kExprBr,        // @14
+      1,              // @15
+      kExprEnd,       // @16
+      kExprEnd        // @17
   };
-  CheckTransfers(code, {{10, 5, 2, 0}, {12, 4, 3, 1}});
+  CheckTransfers(code, {{12, 5, 2, 0}, {14, 4, 3, 1}});
+}
+
+TEST_F(ControlTransferTest, NoInfoForUnreachableCode) {
+  byte code[] = {
+      kExprBlock,        // @0
+      kLocalVoid,        // @1
+      kExprBr,           // @2
+      0,                 // @3
+      kExprBr,           // @4 -- no control transfer entry!
+      1,                 // @5
+      kExprEnd,          // @6
+      kExprBlock,        // @7
+      kLocalVoid,        // @8
+      kExprUnreachable,  // @9
+      kExprI32Const,     // @10
+      0,                 // @11
+      kExprIf,           // @12 -- no control transfer entry!
+      kLocalVoid,        // @13
+      kExprBr,           // @14 -- no control transfer entry!
+      0,                 // @15
+      kExprElse,         // @16 -- no control transfer entry!
+      kExprEnd,          // @17
+      kExprEnd           // @18
+  };
+  CheckTransfers(code, {{2, 5, 0, 0}});
 }
 
 }  // namespace wasm