[wasm] [interpreter] Fix stack transfer to loop labels

When branching to a loop header, we were trying to copy over {arity}
values from the value stack. This is correct for block labels, but not
for loops. When branching back to a loop header, no values need to be
transferred.

R=ahaas@chromium.org
BUG=chromium:715454

Change-Id: I90d806de63d039abf8dcac1abec057860c8f69ca
Reviewed-on: https://chromium-review.googlesource.com/488146
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44949}

src/wasm/wasm-interpreter.cc

@@ -814,16 +814,20 @@ class SideTable : public ZoneObject {
       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: {
-          bool loop = opcode == kExprLoop;
+        case kExprBlock: {
           BlockTypeOperand<false> operand(&i, i.pc());
-          TRACE("control @%u: %s, arity %d\n", i.pc_offset(),
-                loop ? "Loop" : "Block", operand.arity);
+          TRACE("control @%u: Block, arity %d\n", i.pc_offset(), operand.arity);
           CLabel* label =
               CLabel::New(&control_transfer_zone, stack_height, operand.arity);
           control_stack.push_back({i.pc(), label, nullptr});
-          if (loop) label->Bind(i.pc());
+          break;
+        }
+        case kExprLoop: {
+          TRACE("control @%u: Loop\n", i.pc_offset());
+          // Arity is always 0 for loop labels.
+          CLabel* label = CLabel::New(&control_transfer_zone, stack_height, 0);
+          control_stack.push_back({i.pc(), label, nullptr});
+          label->Bind(i.pc());
           break;
         }
         case kExprIf: {

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

@@ -2970,3 +2970,34 @@ WASM_EXEC_TEST(Int32RemS_dead) {
   CHECK_TRAP(r.Call(-1001, 0));
   CHECK_TRAP(r.Call(kMin, 0));
 }
+
+WASM_EXEC_TEST(BrToLoopWithValue) {
+  WasmRunner<int32_t, int32_t, int32_t> r(execution_mode);
+  // Subtracts <1> times 3 from <0> and returns the result.
+  BUILD(r,
+        // loop i32
+        kExprLoop, kLocalI32,
+        // decrement <0> by 3.
+        WASM_SET_LOCAL(0, WASM_I32_SUB(WASM_GET_LOCAL(0), WASM_I32V_1(3))),
+        // decrement <1> by 1.
+        WASM_SET_LOCAL(1, WASM_I32_SUB(WASM_GET_LOCAL(1), WASM_ONE)),
+        // load return value <0>, br_if will drop if if the branch is taken.
+        WASM_GET_LOCAL(0),
+        // continue loop if <1> is != 0.
+        WASM_BR_IF(0, WASM_GET_LOCAL(1)),
+        // end of loop, value loaded above is the return value.
+        kExprEnd);
+  CHECK_EQ(12, r.Call(27, 5));
+}
+
+WASM_EXEC_TEST(BrToLoopWithoutValue) {
+  // This was broken in the interpreter, see http://crbug.com/715454
+  WasmRunner<int32_t, int32_t> r(execution_mode);
+  BUILD(
+      r, kExprLoop, kLocalI32,                                       // loop i32
+      WASM_SET_LOCAL(0, WASM_I32_SUB(WASM_GET_LOCAL(0), WASM_ONE)),  // dec <0>
+      WASM_BR_IF(0, WASM_GET_LOCAL(0)),  // br_if <0> != 0
+      kExprUnreachable,                  // unreachable
+      kExprEnd);                         // end
+  CHECK_TRAP32(r.Call(2));
+}