[sandbox] Avoid double-initialization of external pointer fields

This is a reland of commit a31e8f24

Remove the checking logic, which will be addressed in a separate CL.

Original change's description:
> [sandbox] Forbid double-initialization of ExternalPointerSlots
>
> Double initialization may cause the ExternalPointerTable compaction
> algorithm to behave non-optimally: Consider the case of an Entry E1 that
> is owned by a HeapObject O and is marked for evacuation during GC
> marking. In that case, a new entry E2 is allocated for it, and during
> sweeping, E1 will be evacuated into E2 and the Handle in O updated to
> point to E2. However, if a new entry E3 for O is allocated before
> sweeping, then during sweeping E3 (instead of E1) will be moved into E2.
> This may then violate the invariant that the compaction algorithms
> always evacuates an entry out of the evacuation area.
>
> This CL therefore forbids double initializaiton of external pointer
> slots and adds DCHECKs to attempt to catch these in debug builds.
>
> Bug: v8:10391
> Change-Id: I128dc930e8b3f863dab18ba648f34d68d8cb276b
> 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/+/3856563
> Reviewed-by: Igor Sheludko <ishell@chromium.org>
> Commit-Queue: Samuel Groß <saelo@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82729}

Bug: v8:10391
Change-Id: I6cef79f4adc340fdcdc291ad0f0c2210f5bf48cd
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/+/3857423Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Samuel Groß <saelo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82744}

src/objects/embedder-data-slot-inl.h

@@ -115,8 +115,11 @@ bool EmbedderDataSlot::store_aligned_pointer(Isolate* isolate, void* ptr) {
   if (!HAS_SMI_TAG(value)) return false;
 #ifdef V8_SANDBOXED_EXTERNAL_POINTERS
   DCHECK_EQ(0, value & kExternalPointerTagMask);
-  // This also mark the entry as alive until the next GC.
-  InitExternalPointerField<kEmbedderDataSlotPayloadTag>(
+  // When the sandbox is enabled, the external pointer handles in
+  // EmbedderDataSlots are lazily initialized: initially they contain the null
+  // external pointer handle (see EmbedderDataSlot::Initialize), and only once
+  // an external pointer is stored in them are they properly initialized.
+  WriteLazilyInitializedExternalPointerField<kEmbedderDataSlotPayloadTag>(
       address() + kExternalPointerOffset, isolate, value);
   ObjectSlot(address() + kTaggedPayloadOffset).Relaxed_Store(Smi::zero());
   return true;

src/sandbox/external-pointer-inl.h

@@ -80,6 +80,30 @@ V8_INLINE void WriteExternalPointerField(Address field_address,
   WriteMaybeUnalignedValue<Address>(field_address, value);
 }
 
+template <ExternalPointerTag tag>
+V8_INLINE void WriteLazilyInitializedExternalPointerField(Address field_address,
+                                                          Isolate* isolate,
+                                                          Address value) {
+#ifdef V8_ENABLE_SANDBOX
+  if (IsSandboxedExternalPointerType(tag)) {
+    // See comment above for why this uses a Relaxed_Load and Release_Store.
+    ExternalPointerTable& table = GetExternalPointerTable<tag>(isolate);
+    auto location = reinterpret_cast<ExternalPointerHandle*>(field_address);
+    ExternalPointerHandle handle = base::AsAtomic32::Relaxed_Load(location);
+    if (handle == kNullExternalPointerHandle) {
+      // Field has not been initialized yet.
+      ExternalPointerHandle handle =
+          table.AllocateAndInitializeEntry(value, tag);
+      base::AsAtomic32::Release_Store(location, handle);
+    } else {
+      table.Set(handle, value, tag);
+    }
+    return;
+  }
+#endif  // V8_ENABLE_SANDBOX
+  WriteMaybeUnalignedValue<Address>(field_address, value);
+}
+
 }  // namespace internal
 }  // namespace v8
 

src/sandbox/external-pointer-table.cc

@@ -163,6 +163,13 @@ uint32_t ExternalPointerTable::SweepAndCompact(Isolate* isolate) {
 
       ExternalPointerHandle old_handle = *handle_location;
       ExternalPointerHandle new_handle = index_to_handle(i);
+
+      // 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
+      // external pointer slot is re-initialized, in which case the old_handle
+      // may now also point before the evacuation area. For that reason,
+      // re-initialization of external pointer slots is forbidden.
       DCHECK_GE(handle_to_index(old_handle), first_block_of_evacuation_area);
       DCHECK_LT(handle_to_index(new_handle), first_block_of_evacuation_area);
 

src/sandbox/external-pointer-table.h

@@ -397,7 +397,7 @@ class V8_EXPORT_PRIVATE ExternalPointerTable {
   }
 
   static bool is_evacuation_entry(Address entry) {
-    return (entry & kEvacuationEntryTag) == kEvacuationEntryTag;
+    return (entry & kExternalPointerTagMask) == kEvacuationEntryTag;
   }
 
   static Address extract_handle_location_from_evacuation_entry(Address entry) {

src/sandbox/external-pointer.h

@@ -34,6 +34,17 @@ template <ExternalPointerTag tag>
 V8_INLINE void WriteExternalPointerField(Address field_address,
                                          Isolate* isolate, Address value);
 
+// Writes and possibly initialized a lazily initialized external pointer field.
+// When the sandbox is enabled, a lazily initialized external pointer field
+// initially contains the kNullExternalPointerHandle and will only be properly
+// initialized (i.e. allocate an entry in the external pointer table) once a
+// value is written into it for the first time.
+// If the sandbox is disabled, this is equivalent to WriteExternalPointerField.
+template <ExternalPointerTag tag>
+V8_INLINE void WriteLazilyInitializedExternalPointerField(Address field_address,
+                                                          Isolate* isolate,
+                                                          Address value);
+
 }  // namespace internal
 }  // namespace v8