[wasm][jspi] Fix stack unwinding at stack boundary

thibaudmichaud · V8 LUCI CQ · commit 6508440f449d · 2024-09-03T10:50:31.000Z
When an exception is not caught by the current stack and is forwarded to the parent stack, treat it as a return stack-switch: retire the current stack and mark the parent as the new active stack. R=clemensb@chromium.org,ishell@chromium.org Fixed: 363051803 Change-Id: I700dfb1972cfdf4a35e4a0d5d0f5b53cb48b7a9c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5831369 Reviewed-by: Clemens Backes <clemensb@chromium.org> Commit-Queue: Thibaud Michaud <thibaudm@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Cr-Commit-Position: refs/heads/main@{#95925}
diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc
@@ -2232,10 +2232,16 @@ Tagged<Object> Isolate::UnwindAndFindHandler() {
         // We reached the base of the wasm stack. Follow the chain of
         // continuations to find the parent stack and reset the iterator.
         DCHECK(!continuation.is_null());
-        continuation = Cast<WasmContinuationObject>(continuation->parent());
         wasm::StackMemory* stack =
             reinterpret_cast<wasm::StackMemory*>(continuation->stack());
-        iter.Reset(thread_local_top(), stack);
+        RetireWasmStack(stack);
+        continuation = Cast<WasmContinuationObject>(continuation->parent());
+        wasm::StackMemory* parent =
+            reinterpret_cast<wasm::StackMemory*>(continuation->stack());
+        parent->jmpbuf()->state = wasm::JumpBuffer::Active;
+        roots_table().slot(RootIndex::kActiveContinuation).store(continuation);
+        SyncStackLimit();
+        iter.Reset(thread_local_top(), parent);
       }
     }
 #endif
@@ -3753,6 +3759,25 @@ void Isolate::UpdateCentralStackInfo() {
   }
 }
 
+void Isolate::RetireWasmStack(wasm::StackMemory* stack) {
+  stack->jmpbuf()->state = wasm::JumpBuffer::Retired;
+  size_t index = stack->index();
+  // We can only return from a stack that was still in the global list.
+  DCHECK_LT(index, wasm_stacks().size());
+  std::unique_ptr<wasm::StackMemory> stack_ptr =
+      std::move(wasm_stacks()[index]);
+  DCHECK_EQ(stack_ptr.get(), stack);
+  if (index != wasm_stacks().size() - 1) {
+    wasm_stacks()[index] = std::move(wasm_stacks().back());
+    wasm_stacks()[index]->set_index(index);
+  }
+  wasm_stacks().pop_back();
+  for (size_t i = 0; i < wasm_stacks().size(); ++i) {
+    SLOW_DCHECK(wasm_stacks()[i]->index() == i);
+  }
+  stack_pool().Add(std::move(stack_ptr));
+}
+
 wasm::WasmOrphanedGlobalHandle* Isolate::NewWasmOrphanedGlobalHandle() {
   return wasm::WasmEngine::NewOrphanedGlobalHandle(&wasm_orphaned_handle_);
 }
diff --git a/src/execution/isolate.h b/src/execution/isolate.h
@@ -2205,6 +2205,12 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   void UpdateCentralStackInfo();
 
   void SyncStackLimit();
+
+  // To be called when returning from {stack}, or when an exception crosses the
+  // stack boundary. This updates the {StackMemory} object and the global
+  // {wasm_stacks_} list. This does *not* update the ActiveContinuation root and
+  // the stack limit.
+  void RetireWasmStack(wasm::StackMemory* stack);
 #else
   bool IsOnCentralStack() { return true; }
 #endif
diff --git a/src/wasm/stacks.cc b/src/wasm/stacks.cc
@@ -113,10 +113,6 @@ Address StackMemory::Shrink() {
 void StackMemory::Reset() {
   active_segment_ = first_segment_;
   size_ = active_segment_->size_;
-#if DEBUG
-  constexpr uint8_t kZapValue = 0xab;
-  FillWith(kZapValue);
-#endif
 }
 
 std::unique_ptr<StackMemory> StackPool::GetOrAllocate() {
diff --git a/src/wasm/wasm-external-refs.cc b/src/wasm/wasm-external-refs.cc
@@ -970,19 +970,9 @@ void return_switch(Isolate* isolate, Address raw_continuation) {
 
   Tagged<WasmContinuationObject> continuation =
       Cast<WasmContinuationObject>(Tagged<Object>{raw_continuation});
-  size_t index = reinterpret_cast<StackMemory*>(continuation->stack())->index();
-  // We can only return from a stack that was still in the global list.
-  DCHECK_LT(index, isolate->wasm_stacks().size());
-  std::unique_ptr<StackMemory> stack = std::move(isolate->wasm_stacks()[index]);
-  if (index != isolate->wasm_stacks().size() - 1) {
-    isolate->wasm_stacks()[index] = std::move(isolate->wasm_stacks().back());
-    isolate->wasm_stacks()[index]->set_index(index);
-  }
-  isolate->wasm_stacks().pop_back();
-  for (size_t i = 0; i < isolate->wasm_stacks().size(); ++i) {
-    SLOW_DCHECK(isolate->wasm_stacks()[i]->index() == i);
-  }
-  isolate->stack_pool().Add(std::move(stack));
+  wasm::StackMemory* stack =
+      reinterpret_cast<StackMemory*>(continuation->stack());
+  isolate->RetireWasmStack(stack);
   isolate->SyncStackLimit();
 }
 
