[liftoff] Fix cached memory start in debugging code

The cached memory start was not preserved across stack checks in debug
code. This only manifests if the stack check is actually executed, hence
it's tricky to reproduce.

R=ahaas@chromium.org

Bug: chromium:1222648
Change-Id: I8d678305022e3521bd457ad49ebed30d81b05231
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2987824
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75388}

src/wasm/baseline/liftoff-compiler.cc

@@ -670,7 +670,15 @@ class LiftoffCompiler {
             ? LiftoffAssembler::CacheState::SpillLocation::kStackSlots
             : LiftoffAssembler::CacheState::SpillLocation::kTopOfStack);
     if (V8_UNLIKELY(for_debugging_)) {
+      // When debugging, we do not just push all registers to the stack, but we
+      // spill them to their proper stack locations such that we can inspect
+      // them.
+      // The only exception is the cached memory start, which we just push
+      // before the stack check and pop afterwards.
       regs_to_save = {};
+      if (__ cache_state()->cached_mem_start != no_reg) {
+        regs_to_save.set(__ cache_state()->cached_mem_start);
+      }
       spilled_regs = GetSpilledRegistersForInspection();
     }
     out_of_line_code_.push_back(OutOfLineCode::StackCheck(
@@ -875,12 +883,13 @@ class LiftoffCompiler {
       return;
     }
 
-    // We cannot both push and spill registers.
-    DCHECK(ool->regs_to_save.is_empty() || ool->spilled_registers == nullptr);
     if (!ool->regs_to_save.is_empty()) {
       __ PushRegisters(ool->regs_to_save);
-    } else if (V8_UNLIKELY(ool->spilled_registers != nullptr)) {
+    }
+    if (V8_UNLIKELY(ool->spilled_registers != nullptr)) {
       for (auto& entry : ool->spilled_registers->entries) {
+        // We should not push and spill the same register.
+        DCHECK(!ool->regs_to_save.has(entry.reg));
         __ Spill(entry.offset, entry.reg, entry.kind);
       }
     }

test/mjsunit/wasm/liftoff-debug.js

+// Copyright 2021 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax --expose-gc --liftoff-only
+// Force all functions (the first 8, technically) to generate debug code.
+// Flags: --wasm-debug-mask-for-testing=255
+
+d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
+
+(function testGCInLoopStackCheck() {
+  print(arguments.callee.name);
+  const builder = new WasmModuleBuilder();
+  builder.addMemory(1, 1);
+
+  const imp_index = builder.addImport('q', 'triggerStackCheck', kSig_v_v);
+
+  const kIndex = 0;
+  const kValue = 11;
+
+  // This is a regression test for https://crbug.com/1222648:
+  // Add a memory instruction before the loop, to get the memory start cached.
+  // Then add a memory instruction inside the loop to make use of the cached
+  // memory start.
+  const main =
+      builder.addFunction('main', kSig_i_v)
+          .addBody([
+            kExprCallFunction, imp_index,     // schedule stack check
+            kExprI32Const,     kIndex,        // i32.const kIndex
+            kExprI32Const,     kValue,        // i32.const kValue
+            kExprI32StoreMem,  0,         0,  // i32.store align=0 offset=0
+            kExprLoop,         kWasmVoid,     // loop
+            kExprI32Const,     kIndex,        // i32.const kIndex
+            kExprI32LoadMem,   0,         0,  // i32.load align=0 offset=0
+            kExprReturn,                      // return
+            kExprEnd,                         // end loop
+            kExprUnreachable,                 // unreachable
+          ])
+          .exportFunc();
+
+  const instance = builder.instantiate(
+      {q: {triggerStackCheck: () => %ScheduleGCInStackCheck()}});
+
+  assertEquals(kValue, instance.exports.main());
+})();

tools/testrunner/local/variants.py

@@ -60,7 +60,8 @@ INCOMPATIBLE_FLAGS_PER_VARIANT = {
   "stress_concurrent_inlining": ["--single-threaded", "--predictable"],
   "stress": ["--always-opt", "--no-always-opt",
              "--max-inlined-bytecode-size=*",
-             "--max-inlined-bytecode-size-cumulative=*", "--stress-inline"],
+             "--max-inlined-bytecode-size-cumulative=*", "--stress-inline",
+             "--liftoff-only"],
   "sparkplug": ["--jitless"],
   "always_sparkplug": ["--jitless"],
   "code_serializer": ["--cache=after-execute", "--cache=full-code-cache",