From 4a3d24cd677fe4fbdc643c3ad3d100dcdf2d3fa8 Mon Sep 17 00:00:00 2001
From: Olivier Flückiger <olivf@chromium.org>
Date: Thu, 16 Jul 2026 12:00:07 +0000
Subject: [PATCH] [sandbox] Re-Introduc CAS loops in JSDispatchEntry

Remove the invariant that the JSDispatchEntry WB needs to be invoked
on every object, to fix the mark bit race.

Additionally, this CL:
- Checks !is_marking() in SetCodeAndEntrypointPointer to have a fast
  normal release store when no marker is running.
- Bottlenecks Code updates and WB in the JSDispatchTable.
- Updates the WB verification accordingly.

Bug: 525686865, 525689847

Change-Id: I40d00400e952bc0e0d9114e5942f3da4e0320e65
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8064734
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Samuel Groß <saelo@chromium.org>
Auto-Submit: Olivier Flückiger <olivf@chromium.org>
Commit-Queue: Samuel Groß <saelo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#108711}
---

diff --git a/src/heap/factory.cc b/src/heap/factory.cc
index 66368b0..c461222 100644
--- a/src/heap/factory.cc
+++ b/src/heap/factory.cc
@@ -5472,28 +5472,20 @@
       JSDispatchTable& jdt = isolate_->js_dispatch_table();
       Tagged<Code> old_code = jdt.GetCode(dispatch_handle);
 
-      // A write barrier is needed when settings code, because the update can
-      // race with marking which could leave the dispatch slot unmarked.
-      // TODO(olivf): This should be fixed by using a more traditional WB
-      // for dispatch handles (i.e. have a marking queue with dispatch handles
-      // instead of marking through the handle).
-      constexpr WriteBarrierMode mode_if_setting_code =
-          WriteBarrierMode::UPDATE_WRITE_BARRIER;
-
       // TODO(olivf): We should go through the cases where this is still
       // needed and maybe find some alternative to initialize it correctly
       // from the beginning.
       if (old_code->is_builtin()) {
-        jdt.SetCodeNoWriteBarrier(dispatch_handle, *code);
-        function->set_dispatch_handle(dispatch_handle, mode_if_setting_code);
+        jdt.SetCodeNoWriteBarrier(dispatch_handle, *code, isolate);
+        function->set_dispatch_handle(dispatch_handle, mode);
       } else {
         // On a transition of a feedback cell from one closure to many, make
         // sure that the code on the feedback cell isn't native context
         // specialized, and if it was, eagerly re-optimize.
         if (cell_transition == FeedbackCell::kOneToMany &&
             old_code->is_context_specialized()) {
-          jdt.SetCodeNoWriteBarrier(dispatch_handle, *code);
-          function->set_dispatch_handle(dispatch_handle, mode_if_setting_code);
+          jdt.SetCodeNoWriteBarrier(dispatch_handle, *code, isolate);
+          function->set_dispatch_handle(dispatch_handle, mode);
           DCHECK(old_code->kind() == CodeKind::MAGLEV ||
                  old_code->kind() == CodeKind::TURBOFAN_JS);
           if (!old_code->marked_for_deoptimization()) {
diff --git a/src/heap/heap-write-barrier-inl.h b/src/heap/heap-write-barrier-inl.h
index 32bce78..564a3b0 100644
--- a/src/heap/heap-write-barrier-inl.h
+++ b/src/heap/heap-write-barrier-inl.h
@@ -295,7 +295,7 @@
                                        WriteBarrierMode mode) {
 #if V8_VERIFY_WRITE_BARRIERS
   if (v8_flags.verify_write_barriers) {
-    CHECK(WriteBarrier::VerifyDispatchHandleMarkingState(host, handle, mode));
+    WriteBarrier::VerifyDispatchHandleWriteBarrier(host, handle, mode);
   }
 #endif  // V8_VERIFY_WRITE_BARRIERS
   if (mode == SKIP_WRITE_BARRIER) {
diff --git a/src/heap/heap-write-barrier.cc b/src/heap/heap-write-barrier.cc
index 479bf55..378b75b 100644
--- a/src/heap/heap-write-barrier.cc
+++ b/src/heap/heap-write-barrier.cc
@@ -191,7 +191,6 @@
   // Mark both the table entry and its content.
   Isolate* isolate = Isolate::Current();
   JSDispatchTable& jdt = isolate->js_dispatch_table();
-  static_assert(JSDispatchTable::kWriteBarrierSetsEntryMarkBit);
 #ifdef DEBUG
   Heap* heap = isolate->heap();
   jdt.VerifyEntry(handle, heap->js_dispatch_table_space(),
@@ -574,42 +573,36 @@
 #if V8_VERIFY_WRITE_BARRIERS
 
 // static
-bool WriteBarrier::VerifyDispatchHandleMarkingState(Tagged<HeapObject> host,
+void WriteBarrier::VerifyDispatchHandleWriteBarrier(Tagged<HeapObject> host,
                                                     JSDispatchHandle handle,
                                                     WriteBarrierMode mode) {
   JSDispatchTable& jdt = Isolate::Current()->js_dispatch_table();
   Tagged<Code> value = jdt.GetCode(handle);
 
   if (mode == SKIP_WRITE_BARRIER) {
+    // Builtins do not need write barriers, neither do their JSDispatch handles
+    // as the handles don't move and are immortal. This additional check is
+    // needed since in some configurations and cctests the builtins Code objects
+    // are not in R/O space.
     if (value->is_builtin()) {
-      // Builtins are immortal and immovable, so no write barrier needed.
-      BasePage* page = BasePage::FromHeapObject(value);
-      DCHECK(page->never_evacuate());
-      return true;
+      static_assert(!JSDispatchTable::kSupportsCompaction);
+      return;
     }
 
-    if (WriteBarrier::IsRequired(host, value)) {
-      return false;
+    // First check the barrier for the value
+    VerifySkipWriteBarrier(host, value, mode);
+
+    // Now check the barrier for the handle itself
+    if (jdt.InReadOnlySegment(handle)) {
+      return;
     }
+    if (IsMostRecentYoungAllocation(host->address())) {
+      return;
+    }
+    UNREACHABLE();
+  } else {
+    DCHECK_EQ(mode, UPDATE_WRITE_BARRIER);
   }
-
-  if (CurrentMarkingBarrier(host)->is_not_major()) return true;
-
-  // Ensure we don't have a black -> white -> black edge. This could happen when
-  // skipping a write barrier while concurrently the dispatch entry is marked
-  // from another JSFunction.
-  if (ReadOnlyHeap::Contains(host) ||
-      (IsMarking(host) && mode != SKIP_WRITE_BARRIER) ||
-      !CurrentMarkingBarrier(host)->IsMarked(host)) {
-    return true;
-  }
-  if (jdt.IsMarked(handle)) {
-    return true;
-  }
-  if (ReadOnlyHeap::Contains(value)) {
-    return true;
-  }
-  return !CurrentMarkingBarrier(host)->IsMarked(value);
 }
 
 #endif  // V8_VERIFY_WRITE_BARRIERS
diff --git a/src/heap/heap-write-barrier.h b/src/heap/heap-write-barrier.h
index 4439a05..bc075fc 100644
--- a/src/heap/heap-write-barrier.h
+++ b/src/heap/heap-write-barrier.h
@@ -154,7 +154,7 @@
   template <typename T>
   static inline bool IsRequired(const HeapObject* host, T value);
 
-  static bool VerifyDispatchHandleMarkingState(Tagged<HeapObject> host,
+  static void VerifyDispatchHandleWriteBarrier(Tagged<HeapObject> host,
                                                JSDispatchHandle value,
                                                WriteBarrierMode mode);
 #endif  // V8_VERIFY_WRITE_BARRIERS
diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc
index 6881d42..5fa5816 100644
--- a/src/heap/mark-compact.cc
+++ b/src/heap/mark-compact.cc
@@ -3295,8 +3295,8 @@
                              code->kind() == CodeKind::TURBOFAN_JS ||
                              code->is_interpreter_trampoline_builtin());
                       entry.SetCodeAndEntrypointPointer(
-                          compile_lazy.ptr(),
-                          compile_lazy->instruction_start());
+                          compile_lazy.ptr(), compile_lazy->instruction_start(),
+                          isolate);
                     }
                   });
       })
@@ -5998,7 +5998,8 @@
 #undef CASE
             return code->instruction_start();
           })();
-          jdt.SetCodeAndEntrypointNoWriteBarrier(handle, code, new_entrypoint);
+          jdt.SetCodeAndEntrypointNoWriteBarrier(handle, code, new_entrypoint,
+                                                 heap_->isolate());
           CHECK_IMPLIES(jdt.IsTieringRequested(handle),
                         old_entrypoint == new_entrypoint);
         }
diff --git a/src/heap/read-only-promotion.cc b/src/heap/read-only-promotion.cc
index 254ed31..de6cad2 100644
--- a/src/heap/read-only-promotion.cc
+++ b/src/heap/read-only-promotion.cc
@@ -514,7 +514,8 @@
           Tagged<HeapObject> new_code = it->second;
           CHECK(IsCode(new_code));
           // TODO(saelo): is it worth logging something in this case?
-          jdt.SetCodeNoWriteBarrier(handle, TrustedCast<Code>(new_code));
+          jdt.SetCodeNoWriteBarrier(handle, TrustedCast<Code>(new_code),
+                                    isolate);
         });
     // Note the we should technically also update the entries in the