[wasm] Detect active stacks more reliably

Add an enum in the jump buffer to represent the state of the
stack: active, suspended, or retired. Update the state on stack switch
and check that they are consistent.

The previous method relied on comparing the current stack pointer with
the bounds of the stack, which was not reliable because the upper bound
of the native stack is not known precisely.

R=clemensb@chromium.org

Bug: v8:13236
Change-Id: If1880aa3efd5a9dc03c3c52ac5315d369d886a50
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3870925
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83017}

src/builtins/x64/builtins-x64.cc

@@ -2869,6 +2869,21 @@ void RestoreAfterBuiltinCall(MacroAssembler* masm, Register function_data,
   __ popq(current_param);
 }
 
+// Check that the stack was in the old state (if generated code assertions are
+// enabled), and switch to the new state.
+void SwitchStackState(MacroAssembler* masm, Register jmpbuf,
+                      wasm::JumpBuffer::StackState old_state,
+                      wasm::JumpBuffer::StackState new_state) {
+  if (FLAG_debug_code) {
+    __ cmpl(MemOperand(jmpbuf, wasm::kJmpBufStateOffset), Immediate(old_state));
+    Label ok;
+    __ j(equal, &ok, Label::kNear);
+    __ Trap();
+    __ bind(&ok);
+  }
+  __ movl(MemOperand(jmpbuf, wasm::kJmpBufStateOffset), Immediate(new_state));
+}
+
 void FillJumpBuffer(MacroAssembler* masm, Register jmpbuf, Label* pc) {
   __ movq(MemOperand(jmpbuf, wasm::kJmpBufSpOffset), rsp);
   __ movq(MemOperand(jmpbuf, wasm::kJmpBufFpOffset), rbp);
@@ -2877,11 +2892,15 @@ void FillJumpBuffer(MacroAssembler* masm, Register jmpbuf, Label* pc) {
   __ movq(MemOperand(jmpbuf, wasm::kJmpBufStackLimitOffset), kScratchRegister);
   __ leaq(kScratchRegister, MemOperand(pc, 0));
   __ movq(MemOperand(jmpbuf, wasm::kJmpBufPcOffset), kScratchRegister);
+  SwitchStackState(masm, jmpbuf, wasm::JumpBuffer::Active,
+                   wasm::JumpBuffer::Inactive);
 }
 
 void LoadJumpBuffer(MacroAssembler* masm, Register jmpbuf, bool load_pc) {
   __ movq(rsp, MemOperand(jmpbuf, wasm::kJmpBufSpOffset));
   __ movq(rbp, MemOperand(jmpbuf, wasm::kJmpBufFpOffset));
+  SwitchStackState(masm, jmpbuf, wasm::JumpBuffer::Inactive,
+                   wasm::JumpBuffer::Active);
   if (load_pc) {
     __ jmp(MemOperand(jmpbuf, wasm::kJmpBufPcOffset));
   }
@@ -2935,14 +2954,15 @@ void ReloadParentContinuation(MacroAssembler* masm, Register wasm_instance,
   Register active_continuation = tmp1;
   __ LoadRoot(active_continuation, RootIndex::kActiveContinuation);
 
-  // Set a null pointer in the jump buffer's SP slot to indicate to the stack
-  // frame iterator that this stack is empty.
+  // We don't need to save the full register state since we are switching out of
+  // this stack for the last time. Mark the stack as retired.
   Register jmpbuf = kScratchRegister;
   __ LoadExternalPointerField(
       jmpbuf,
       FieldOperand(active_continuation, WasmContinuationObject::kJmpbufOffset),
       kWasmContinuationJmpbufTag, tmp2);
-  __ movq(Operand(jmpbuf, wasm::kJmpBufSpOffset), Immediate(kNullAddress));
+  SwitchStackState(masm, jmpbuf, wasm::JumpBuffer::Active,
+                   wasm::JumpBuffer::Retired);
 
   Register parent = tmp2;
   __ LoadAnyTaggedField(

src/execution/frames.cc

@@ -166,9 +166,7 @@ void StackFrameIterator::Reset(ThreadLocalTop* top) {
 
 #if V8_ENABLE_WEBASSEMBLY
 void StackFrameIterator::Reset(ThreadLocalTop* top, wasm::StackMemory* stack) {
-  if (stack->jmpbuf()->sp == kNullAddress) {
-    // A null SP indicates that the computation associated with this stack has
-    // returned, leaving the stack segment empty.
+  if (stack->jmpbuf()->state == wasm::JumpBuffer::Retired) {
     return;
   }
   StackFrame::State state;

src/execution/isolate.cc

@@ -1969,12 +1969,15 @@ Object Isolate::UnwindAndFindHandler() {
       // switch.
       auto stack = Managed<wasm::StackMemory>::cast(current_stack.stack());
       // Mark this stack as empty.
-      stack.get()->jmpbuf()->sp = 0x0;
+      DCHECK_EQ(stack.get()->jmpbuf()->state, wasm::JumpBuffer::Active);
+      stack.get()->jmpbuf()->state = wasm::JumpBuffer::Retired;
       HeapObject parent = current_stack.parent();
       DCHECK(!parent.IsUndefined());
       current_stack = WasmContinuationObject::cast(parent);
       wasm::StackMemory* parent_stack =
           Managed<wasm::StackMemory>::cast(current_stack.stack()).get().get();
+      DCHECK_EQ(parent_stack->jmpbuf()->state, wasm::JumpBuffer::Inactive);
+      parent_stack->jmpbuf()->state = wasm::JumpBuffer::Active;
       iter.Reset(thread_local_top(), parent_stack);
 
       // Update the continuation and suspender state.
@@ -4452,7 +4455,7 @@ bool Isolate::Init(SnapshotData* startup_snapshot_data,
     }
     HandleScope scope(this);
     Handle<WasmContinuationObject> continuation = WasmContinuationObject::New(
-        this, std::move(stack), AllocationType::kOld);
+        this, std::move(stack), wasm::JumpBuffer::Active, AllocationType::kOld);
     heap()
         ->roots_table()
         .slot(RootIndex::kActiveContinuation)

src/runtime/runtime-wasm.cc

@@ -812,7 +812,7 @@ RUNTIME_FUNCTION(Runtime_WasmAllocateSuspender) {
                            isolate->root(RootIndex::kActiveContinuation)),
                        isolate);
   Handle<WasmContinuationObject> target =
-      WasmContinuationObject::New(isolate, parent);
+      WasmContinuationObject::New(isolate, wasm::JumpBuffer::Inactive, parent);
   auto target_stack =
       Managed<wasm::StackMemory>::cast(target->stack()).get().get();
   isolate->wasm_stacks()->Add(target_stack);

src/wasm/stacks.h

@@ -22,13 +22,15 @@ struct JumpBuffer {
   Address fp;
   Address pc;
   void* stack_limit;
-  // TODO(thibaudm/fgm): Add general-purpose registers.
+  enum StackState : int32_t { Active, Inactive, Retired };
+  StackState state;
 };
 
 constexpr int kJmpBufSpOffset = offsetof(JumpBuffer, sp);
 constexpr int kJmpBufFpOffset = offsetof(JumpBuffer, fp);
 constexpr int kJmpBufPcOffset = offsetof(JumpBuffer, pc);
 constexpr int kJmpBufStackLimitOffset = offsetof(JumpBuffer, stack_limit);
+constexpr int kJmpBufStateOffset = offsetof(JumpBuffer, state);
 
 class StackMemory {
  public:
@@ -72,10 +74,7 @@ class StackMemory {
 
   // Track external memory usage for Managed<StackMemory> objects.
   size_t owned_size() { return sizeof(StackMemory) + (owned_ ? size_ : 0); }
-  bool IsActive() {
-    byte* sp = reinterpret_cast<byte*>(GetCurrentStackPosition());
-    return limit_ < sp && sp <= limit_ + size_;
-  }
+  bool IsActive() { return jmpbuf_.state == JumpBuffer::Active; }
 
  private:
 #ifdef DEBUG

src/wasm/wasm-objects.cc

@@ -1861,10 +1861,12 @@ void DecodeI64ExceptionValue(Handle<FixedArray> encoded_values,
 // static
 Handle<WasmContinuationObject> WasmContinuationObject::New(
     Isolate* isolate, std::unique_ptr<wasm::StackMemory> stack,
-    Handle<HeapObject> parent, AllocationType allocation_type) {
+    wasm::JumpBuffer::StackState state, Handle<HeapObject> parent,
+    AllocationType allocation_type) {
   stack->jmpbuf()->stack_limit = stack->jslimit();
   stack->jmpbuf()->sp = stack->base();
   stack->jmpbuf()->fp = kNullAddress;
+  stack->jmpbuf()->state = state;
   wasm::JumpBuffer* jmpbuf = stack->jmpbuf();
   size_t external_size = stack->owned_size();
   Handle<Foreign> managed_stack = Managed<wasm::StackMemory>::FromUniquePtr(
@@ -1879,18 +1881,19 @@ Handle<WasmContinuationObject> WasmContinuationObject::New(
 // static
 Handle<WasmContinuationObject> WasmContinuationObject::New(
     Isolate* isolate, std::unique_ptr<wasm::StackMemory> stack,
-    AllocationType allocation_type) {
+    wasm::JumpBuffer::StackState state, AllocationType allocation_type) {
   auto parent = ReadOnlyRoots(isolate).undefined_value();
-  return New(isolate, std::move(stack), handle(parent, isolate),
+  return New(isolate, std::move(stack), state, handle(parent, isolate),
              allocation_type);
 }
 
 // static
 Handle<WasmContinuationObject> WasmContinuationObject::New(
-    Isolate* isolate, Handle<WasmContinuationObject> parent) {
+    Isolate* isolate, wasm::JumpBuffer::StackState state,
+    Handle<WasmContinuationObject> parent) {
   auto stack =
       std::unique_ptr<wasm::StackMemory>(wasm::StackMemory::New(isolate));
-  return New(isolate, std::move(stack), parent);
+  return New(isolate, std::move(stack), state, parent);
 }
 
 // static

src/wasm/wasm-objects.h

@@ -1033,9 +1033,11 @@ class WasmContinuationObject
  public:
   static Handle<WasmContinuationObject> New(
       Isolate* isolate, std::unique_ptr<wasm::StackMemory> stack,
+      wasm::JumpBuffer::StackState state,
       AllocationType allocation_type = AllocationType::kYoung);
   static Handle<WasmContinuationObject> New(
-      Isolate* isolate, Handle<WasmContinuationObject> parent);
+      Isolate* isolate, wasm::JumpBuffer::StackState state,
+      Handle<WasmContinuationObject> parent);
 
   DECL_EXTERNAL_POINTER_ACCESSORS(jmpbuf, Address)
 
@@ -1046,7 +1048,7 @@ class WasmContinuationObject
  private:
   static Handle<WasmContinuationObject> New(
       Isolate* isolate, std::unique_ptr<wasm::StackMemory> stack,
-      Handle<HeapObject> parent,
+      wasm::JumpBuffer::StackState state, Handle<HeapObject> parent,
       AllocationType allocation_type = AllocationType::kYoung);
 
   TQ_OBJECT_CONSTRUCTORS(WasmContinuationObject)