[sandbox] Detect double-initialization of external pointer fields

This CL adds lightweight checking to the ExternalPointerTable GC
algorithm to detect double initialization of external pointer fields.
These are forbidden as they interfere with the table compaction
algorithm.

Bug: v8:10391
Change-Id: Id69fdcce883aa86f8e2c456a0fe7a1f011719464
Cq-Include-Trybots: luci.v8.try:v8_linux64_heap_sandbox_dbg_ng,v8_linux_arm64_sim_heap_sandbox_dbg_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3858228Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Samuel Groß <saelo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82777}

src/sandbox/external-pointer-table-inl.h

@@ -158,6 +158,12 @@ void ExternalPointerTable::Mark(ExternalPointerHandle handle,
       // No need for an atomic store as the entry will only be accessed during
       // sweeping.
       store(index, make_evacuation_entry(handle_location));
+#ifdef DEBUG
+      // Mark the handle as visited in debug builds to detect double
+      // initialization of external pointer fields.
+      auto handle_ptr = reinterpret_cast<base::Atomic32*>(handle_location);
+      base::Relaxed_Store(handle_ptr, handle | kVisitedHandleMarker);
+#endif  // DEBUG
     } else {
       // In this case, the application has allocated a sufficiently large
       // number of entries from the freelist so that new entries would now be

src/sandbox/external-pointer-table.cc

@@ -164,6 +164,14 @@ uint32_t ExternalPointerTable::SweepAndCompact(Isolate* isolate) {
       ExternalPointerHandle old_handle = *handle_location;
       ExternalPointerHandle new_handle = index_to_handle(i);
 
+      // For the compaction algorithm to work optimally, double initialization
+      // of entries is forbidden, see below. This DCHECK can detect double
+      // initialization of external pointer fields in debug builds by checking
+      // that the old_handle was visited during marking.
+      // There's no need to clear the marking bit from the handle as the handle
+      // will be replaced by a new, unmarked handle.
+      DCHECK(HandleWasVisitedDuringMarking(old_handle));
+
       // The following DCHECKs assert that the compaction algorithm works
       // correctly: it always moves an entry from the evacuation area to the
       // front of the table. One reason this invariant can be broken is if an

src/sandbox/external-pointer-table.h

@@ -218,6 +218,15 @@ class V8_EXPORT_PRIVATE ExternalPointerTable {
   // attempts as `should_be_evacuated` (see above) will always be false.
   static constexpr uint32_t kCompactionAbortedMarker = 0xf0000000;
 
+  // In debug builds during GC marking, this value is ORed into
+  // ExternalPointerHandles whose entries are marked for evacuation. During
+  // sweeping, the Handles for evacuated entries are checked to have this
+  // marker value. This allows detecting re-initialized entries, which are
+  // problematic for table compaction. This is only possible for entries marked
+  // for evacuation as the location of the Handle is only known for those.
+  static constexpr uint32_t kVisitedHandleMarker = 0x1;
+  static_assert(kExternalPointerIndexShift >= 1);
+
   // Outcome of external pointer table compaction to use for the
   // ExternalPointerTableCompactionOutcome histogram.
   enum class TableCompactionOutcome {
@@ -285,7 +294,8 @@ class V8_EXPORT_PRIVATE ExternalPointerTable {
 
   inline uint32_t handle_to_index(ExternalPointerHandle handle) const {
     uint32_t index = handle >> kExternalPointerIndexShift;
-    DCHECK_EQ(handle, index << kExternalPointerIndexShift);
+    DCHECK_EQ(handle & ~kVisitedHandleMarker,
+              index << kExternalPointerIndexShift);
     DCHECK_LT(index, capacity());
     return index;
   }
@@ -296,6 +306,12 @@ class V8_EXPORT_PRIVATE ExternalPointerTable {
     return handle;
   }
 
+#ifdef DEBUG
+  inline bool HandleWasVisitedDuringMarking(ExternalPointerHandle handle) {
+    return (handle & kVisitedHandleMarker) == kVisitedHandleMarker;
+  }
+#endif  // DEBUG
+
   // Computes the address of the specified entry.
   inline Address entry_address(uint32_t index) const {
     return buffer_ + index * sizeof(Address);