[runtime] Merge VisitExternalReference and VisitExternalPointer

... into VisitExternalPointer(HeapObject, ExternalPointerSlot,
                              ExternalPointerTag).

Drive-by: introduce ExternalPointerSlot - a slot containing an
ExternalPointer_t value.

This cleanup is a prerequisite for inlining Foreign object fields into
field's holder objects.

Bug: v8:12949
Change-Id: Ifd74ed285796b0952d7d06de82b56c63fd1f7f3e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3695361Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81064}

src/diagnostics/objects-printer.cc

@@ -16,6 +16,7 @@
 #include "src/interpreter/bytecodes.h"
 #include "src/objects/all-objects-inl.h"
 #include "src/objects/code-kind.h"
+#include "src/objects/instance-type.h"
 #include "src/regexp/regexp.h"
 #include "src/snapshot/embedded/embedded-data.h"
 #include "src/utils/ostreams.h"
@@ -624,6 +625,12 @@ void JSObject::JSObjectPrint(std::ostream& os) {
   JSObjectPrintBody(os, *this);
 }
 
+void JSExternalObject::JSExternalObjectPrint(std::ostream& os) {
+  JSObjectPrintHeader(os, *this, nullptr);
+  os << "\n - external value: " << value();
+  JSObjectPrintBody(os, *this);
+}
+
 void JSGeneratorObject::JSGeneratorObjectPrint(std::ostream& os) {
   JSObjectPrintHeader(os, *this, "JSGeneratorObject");
   os << "\n - function: " << Brief(function());

src/heap/mark-compact.cc

@@ -1604,9 +1604,10 @@ class RecordMigratedSlotVisitor : public ObjectVisitorWithCageBases {
 
   // Entries that are skipped for recording.
   inline void VisitExternalReference(Code host, RelocInfo* rinfo) final {}
-  inline void VisitExternalReference(Foreign host, Address* p) final {}
   inline void VisitRuntimeEntry(Code host, RelocInfo* rinfo) final {}
   inline void VisitInternalReference(Code host, RelocInfo* rinfo) final {}
+  inline void VisitExternalPointer(HeapObject host, ExternalPointerSlot slot,
+                                   ExternalPointerTag tag) final {}
 
   virtual void MarkArrayBufferExtensionPromoted(HeapObject object) {}
 

src/heap/marking-visitor.h

@@ -209,10 +209,10 @@ class MarkingVisitorBase : public HeapVisitor<int, ConcreteVisitor> {
     // reconstructed after GC.
   }
 
-  V8_INLINE void VisitExternalPointer(HeapObject host,
-                                      ExternalPointer_t ptr) final {
+  V8_INLINE void VisitExternalPointer(HeapObject host, ExternalPointerSlot slot,
+                                      ExternalPointerTag tag) final {
 #ifdef V8_SANDBOXED_EXTERNAL_POINTERS
-    uint32_t index = ptr >> kExternalPointerIndexShift;
+    uint32_t index = slot.load_raw() >> kExternalPointerIndexShift;
     external_pointer_table_->Mark(index);
 #endif  // V8_SANDBOXED_EXTERNAL_POINTERS
   }

src/objects/heap-object.h

@@ -168,7 +168,7 @@ class HeapObject : public Object {
   inline ObjectSlot RawField(int byte_offset) const;
   inline MaybeObjectSlot RawMaybeWeakField(int byte_offset) const;
   inline CodeObjectSlot RawCodeField(int byte_offset) const;
-  inline ExternalPointer_t RawExternalPointerField(int byte_offset) const;
+  inline ExternalPointerSlot RawExternalPointerField(int byte_offset) const;
 
   DECL_CAST(HeapObject)
 

src/objects/js-objects.h

@@ -910,6 +910,8 @@ class JSExternalObject
 
   static constexpr int kEndOfTaggedFieldsOffset = JSObject::kHeaderSize;
 
+  DECL_PRINTER(JSExternalObject)
+
   class BodyDescriptor;
 
  private:

src/objects/objects-body-descriptors-inl.h

@@ -98,8 +98,10 @@ void BodyDescriptorBase::IterateJSObjectBodyImpl(Map map, HeapObject obj,
          offset += kEmbedderDataSlotSize) {
       IteratePointer(obj, offset + EmbedderDataSlot::kTaggedPayloadOffset, v);
       v->VisitExternalPointer(
-          obj, obj.RawExternalPointerField(
-                   offset + EmbedderDataSlot::kExternalPointerOffset));
+          obj,
+          obj.RawExternalPointerField(offset +
+                                      EmbedderDataSlot::kExternalPointerOffset),
+          kEmbedderDataSlotPayloadTag);
     }
     // Proceed processing inobject properties.
     start_offset = inobject_fields_start_offset;
@@ -445,7 +447,8 @@ class JSExternalObject::BodyDescriptor final : public BodyDescriptorBase {
   static inline void IterateBody(Map map, HeapObject obj, int object_size,
                                  ObjectVisitor* v) {
     IteratePointers(obj, kPropertiesOrHashOffset, kEndOfTaggedFieldsOffset, v);
-    v->VisitExternalPointer(obj, obj.RawExternalPointerField(kValueOffset));
+    v->VisitExternalPointer(obj, obj.RawExternalPointerField(kValueOffset),
+                            kExternalObjectValueTag);
   }
 
   static inline int SizeOf(Map map, HeapObject object) {
@@ -691,11 +694,9 @@ class Foreign::BodyDescriptor final : public BodyDescriptorBase {
   template <typename ObjectVisitor>
   static inline void IterateBody(Map map, HeapObject obj, int object_size,
                                  ObjectVisitor* v) {
-    v->VisitExternalReference(
-        Foreign::cast(obj), reinterpret_cast<Address*>(
-                                obj.RawField(kForeignAddressOffset).address()));
     v->VisitExternalPointer(obj,
-                            obj.RawExternalPointerField(kForeignAddressOffset));
+                            obj.RawExternalPointerField(kForeignAddressOffset),
+                            kForeignForeignAddressTag);
   }
 
   static inline int SizeOf(Map map, HeapObject object) { return kSize; }
@@ -847,10 +848,12 @@ class ExternalOneByteString::BodyDescriptor final : public BodyDescriptorBase {
                                  ObjectVisitor* v) {
     ExternalString string = ExternalString::cast(obj);
     v->VisitExternalPointer(obj,
-                            string.RawExternalPointerField(kResourceOffset));
+                            string.RawExternalPointerField(kResourceOffset),
+                            kExternalStringResourceTag);
     if (string.is_uncached()) return;
-    v->VisitExternalPointer(
-        obj, string.RawExternalPointerField(kResourceDataOffset));
+    v->VisitExternalPointer(obj,
+                            string.RawExternalPointerField(kResourceDataOffset),
+                            kExternalStringResourceDataTag);
   }
 
   static inline int SizeOf(Map map, HeapObject object) { return kSize; }
@@ -865,10 +868,12 @@ class ExternalTwoByteString::BodyDescriptor final : public BodyDescriptorBase {
                                  ObjectVisitor* v) {
     ExternalString string = ExternalString::cast(obj);
     v->VisitExternalPointer(obj,
-                            string.RawExternalPointerField(kResourceOffset));
+                            string.RawExternalPointerField(kResourceOffset),
+                            kExternalStringResourceTag);
     if (string.is_uncached()) return;
-    v->VisitExternalPointer(
-        obj, string.RawExternalPointerField(kResourceDataOffset));
+    v->VisitExternalPointer(obj,
+                            string.RawExternalPointerField(kResourceDataOffset),
+                            kExternalStringResourceDataTag);
   }
 
   static inline int SizeOf(Map map, HeapObject object) { return kSize; }
@@ -992,7 +997,8 @@ class NativeContext::BodyDescriptor final : public BodyDescriptorBase {
     IterateCustomWeakPointers(obj, NativeContext::kStartOfWeakFieldsOffset,
                               NativeContext::kEndOfWeakFieldsOffset, v);
     v->VisitExternalPointer(obj,
-                            obj.RawExternalPointerField(kMicrotaskQueueOffset));
+                            obj.RawExternalPointerField(kMicrotaskQueueOffset),
+                            kNativeContextMicrotaskQueueTag);
   }
 
   static inline int SizeOf(Map map, HeapObject object) {
@@ -1019,7 +1025,8 @@ class CodeDataContainer::BodyDescriptor final : public BodyDescriptorBase {
     if (V8_EXTERNAL_CODE_SPACE_BOOL) {
       v->VisitCodePointer(obj, obj.RawCodeField(kCodeOffset));
       v->VisitExternalPointer(
-          obj, obj.RawExternalPointerField(kCodeEntryPointOffset));
+          obj, obj.RawExternalPointerField(kCodeEntryPointOffset),
+          kCodeEntryPointTag);
     }
   }
 
@@ -1055,8 +1062,10 @@ class EmbedderDataArray::BodyDescriptor final : public BodyDescriptorBase {
          offset < object_size; offset += kEmbedderDataSlotSize) {
       IteratePointer(obj, offset + EmbedderDataSlot::kTaggedPayloadOffset, v);
       v->VisitExternalPointer(
-          obj, obj.RawExternalPointerField(
-                   offset + EmbedderDataSlot::kExternalPointerOffset));
+          obj,
+          obj.RawExternalPointerField(offset +
+                                      EmbedderDataSlot::kExternalPointerOffset),
+          kEmbedderDataSlotPayloadTag);
     }
 
 #else

src/objects/objects-inl.h

@@ -700,8 +700,8 @@ CodeObjectSlot HeapObject::RawCodeField(int byte_offset) const {
   return CodeObjectSlot(field_address(byte_offset));
 }
 
-ExternalPointer_t HeapObject::RawExternalPointerField(int byte_offset) const {
-  return ReadRawExternalPointerField(field_address(byte_offset));
+ExternalPointerSlot HeapObject::RawExternalPointerField(int byte_offset) const {
+  return ExternalPointerSlot(field_address(byte_offset));
 }
 
 MapWord MapWord::FromMap(const Map map) {

src/objects/slots-inl.h

@@ -14,6 +14,7 @@
 #include "src/objects/maybe-object.h"
 #include "src/objects/objects.h"
 #include "src/objects/slots.h"
+#include "src/sandbox/external-pointer-inl.h"
 #include "src/utils/memcopy.h"
 
 namespace v8 {
@@ -153,6 +154,25 @@ void FullHeapObjectSlot::StoreHeapObject(HeapObject value) const {
   *location() = value.ptr();
 }
 
+ExternalPointer_t ExternalPointerSlot::load_raw() const {
+  return ReadRawExternalPointerField(address());
+}
+
+void ExternalPointerSlot::store_raw(ExternalPointer_t value) const {
+  WriteRawExternalPointerField(address(), value);
+}
+
+Address ExternalPointerSlot::load(const Isolate* isolate,
+                                  ExternalPointerTag tag) {
+  ExternalPointer_t encoded_value = load_raw();
+  return DecodeExternalPointer(isolate, encoded_value, tag);
+}
+
+void ExternalPointerSlot::store(Isolate* isolate, Address value,
+                                ExternalPointerTag tag) {
+  WriteExternalPointerField(address(), isolate, value, tag);
+}
+
 //
 // Utils.
 //

src/objects/slots.h

@@ -278,6 +278,27 @@ class OffHeapFullObjectSlot : public FullObjectSlot {
   inline Object Relaxed_Load() const = delete;
 };
 
+// An ExternalPointerSlot instance describes a kExternalPointerSize-sized field
+// ("slot") holding a pointer to objects located outside the V8 heap and V8
+// sandbox (think: ExternalPointer_t).
+// It's basically an ExternalPointer_t* but abstracting away the fact that the
+// pointer might not be kExternalPointerSize-aligned in certain configurations.
+// Its address() is the address of the slot.
+class ExternalPointerSlot
+    : public SlotBase<ExternalPointerSlot, ExternalPointer_t,
+                      kTaggedSize /* slot alignment */> {
+ public:
+  ExternalPointerSlot() : SlotBase(kNullAddress) {}
+  explicit ExternalPointerSlot(Address ptr) : SlotBase(ptr) {}
+
+  inline ExternalPointer_t load_raw() const;
+  inline void store_raw(ExternalPointer_t value) const;
+
+  inline Address load(const Isolate* isolate, ExternalPointerTag tag);
+
+  inline void store(Isolate* isolate, Address value, ExternalPointerTag tag);
+};
+
 }  // namespace internal
 }  // namespace v8
 

src/objects/visitors.h

@@ -168,8 +168,9 @@ class ObjectVisitor {
   // Visits an external reference embedded into a code object.
   virtual void VisitExternalReference(Code host, RelocInfo* rinfo) {}
 
-  // Visits an external reference.
-  virtual void VisitExternalReference(Foreign host, Address* p) {}
+  // Visits an external pointer.
+  virtual void VisitExternalPointer(HeapObject host, ExternalPointerSlot slot,
+                                    ExternalPointerTag tag) {}
 
   // Visits an (encoded) internal reference.
   virtual void VisitInternalReference(Code host, RelocInfo* rinfo) {}
@@ -182,10 +183,6 @@ class ObjectVisitor {
 
   // Visits the object's map pointer, decoding as necessary
   virtual void VisitMapPointer(HeapObject host) { UNREACHABLE(); }
-
-  // Visits an external pointer. This is currently only guaranteed to be called
-  // when the sandbox is enabled.
-  virtual void VisitExternalPointer(HeapObject host, ExternalPointer_t ptr) {}
 };
 
 // Helper version of ObjectVisitor that also takes care of caching base values

src/sandbox/external-pointer-inl.h

@@ -62,6 +62,18 @@ V8_INLINE ExternalPointer_t ReadRawExternalPointerField(Address field_address) {
   }
 }
 
+V8_INLINE void WriteRawExternalPointerField(Address field_address,
+                                            ExternalPointer_t value) {
+  // Pointer compression causes types larger than kTaggedSize to be unaligned.
+  constexpr bool v8_pointer_compression_unaligned =
+      kExternalPointerSize > kTaggedSize;
+  if (v8_pointer_compression_unaligned) {
+    base::WriteUnalignedValue<ExternalPointer_t>(field_address, value);
+  } else {
+    base::Memory<ExternalPointer_t>(field_address) = value;
+  }
+}
+
 V8_INLINE Address ReadExternalPointerField(Address field_address,
                                            const Isolate* isolate,
                                            ExternalPointerTag tag) {

src/sandbox/external-pointer.h

@@ -33,6 +33,10 @@ V8_INLINE void InitExternalPointerField(Address field_address, Isolate* isolate,
 // Reads and returns a raw external pointer value.
 V8_INLINE ExternalPointer_t ReadRawExternalPointerField(Address field_address);
 
+// Writes a raw external pointer value at given address.
+V8_INLINE void WriteRawExternalPointerField(Address field_address,
+                                            ExternalPointer_t value);
+
 // Reads external pointer for the field, and decodes it if the sandbox is
 // enabled.
 V8_INLINE Address ReadExternalPointerField(Address field_address,

src/snapshot/embedded/embedded-data.cc

@@ -11,7 +11,6 @@
 #include "src/snapshot/embedded/embedded-data-inl.h"
 #include "src/snapshot/snapshot-utils.h"
 #include "src/snapshot/snapshot.h"
-#include "v8-internal.h"
 
 namespace v8 {
 namespace internal {

src/snapshot/serializer.cc

@@ -12,6 +12,7 @@
 #include "src/heap/read-only-heap.h"
 #include "src/interpreter/interpreter.h"
 #include "src/objects/code.h"
+#include "src/objects/instance-type.h"
 #include "src/objects/js-array-buffer-inl.h"
 #include "src/objects/js-array-inl.h"
 #include "src/objects/map.h"
@@ -1005,14 +1006,6 @@ void Serializer::ObjectSerializer::OutputExternalReference(
   }
 }
 
-void Serializer::ObjectSerializer::VisitExternalReference(Foreign host,
-                                                          Address* p) {
-  // "Sandboxify" external reference.
-  OutputExternalReference(host.foreign_address(), kSystemPointerSize, true,
-                          kForeignForeignAddressTag);
-  bytes_processed_so_far_ += kExternalPointerSize;
-}
-
 class Serializer::ObjectSerializer::RelocInfoObjectPreSerializer {
  public:
   explicit RelocInfoObjectPreSerializer(Serializer* serializer)
@@ -1067,31 +1060,6 @@ void Serializer::ObjectSerializer::VisitExternalReference(Code host,
                           kExternalPointerNullTag);
 }
 
-void Serializer::ObjectSerializer::VisitExternalPointer(HeapObject host,
-                                                        ExternalPointer_t ptr) {
-  // TODO(v8:12700) handle other external references here as well. This should
-  // allow removing some of the other Visit* methods, should unify the sandbox
-  // vs no-sandbox implementation, and should allow removing various
-  // XYZForSerialization methods throughout the codebase.
-  if (host.IsJSExternalObject()) {
-#ifdef V8_SANDBOXED_EXTERNAL_POINTERS
-    // TODO(saelo) maybe add a helper method for this conversion if also needed
-    // in other places? This might require a ExternalPointerTable::Get variant
-    // that drops the pointer tag completely.
-    uint32_t index = ptr >> kExternalPointerIndexShift;
-    Address value =
-        isolate()->external_pointer_table().Get(index, kExternalObjectValueTag);
-#else
-    Address value = ptr;
-#endif
-    // TODO(v8:12700) should we specify here whether we expect the references to
-    // be internal or external (or either)?
-    OutputExternalReference(value, kSystemPointerSize, true,
-                            kExternalObjectValueTag);
-    bytes_processed_so_far_ += kExternalPointerSize;
-  }
-}
-
 void Serializer::ObjectSerializer::VisitInternalReference(Code host,
                                                           RelocInfo* rinfo) {
   Address entry = Handle<Code>::cast(object_)->entry();
@@ -1106,6 +1074,37 @@ void Serializer::ObjectSerializer::VisitInternalReference(Code host,
   sink_->PutInt(target_offset, "internal ref value");
 }
 
+void Serializer::ObjectSerializer::VisitExternalPointer(
+    HeapObject host, ExternalPointerSlot slot, ExternalPointerTag tag) {
+  PtrComprCageBase cage_base(isolate());
+  InstanceType instance_type = object_->map(cage_base).instance_type();
+  if (InstanceTypeChecker::IsForeign(instance_type) ||
+      InstanceTypeChecker::IsJSExternalObject(instance_type)) {
+    Address value = slot.load(isolate(), tag);
+    OutputExternalReference(value, kSystemPointerSize, true, tag);
+    bytes_processed_so_far_ += kExternalPointerSize;
+
+  } else {
+    // Serialization of external references in other objects is handled
+    // elsewhere or not supported.
+    DCHECK(
+        // See ObjectSerializer::SerializeJSTypedArray().
+        InstanceTypeChecker::IsJSTypedArray(instance_type) ||
+        // See ObjectSerializer::SerializeJSArrayBuffer().
+        InstanceTypeChecker::IsJSArrayBuffer(instance_type) ||
+        // See ObjectSerializer::SerializeExternalString().
+        InstanceTypeChecker::IsExternalString(instance_type) ||
+        // See ObjectSerializer::SanitizeNativeContextScope.
+        InstanceTypeChecker::IsNativeContext(instance_type) ||
+        // See ContextSerializer::SerializeJSObjectWithEmbedderFields().
+        (InstanceTypeChecker::IsJSObject(instance_type) &&
+         JSObject::cast(host).GetEmbedderFieldCount() > 0) ||
+        // See ObjectSerializer::OutputRawData().
+        (V8_EXTERNAL_CODE_SPACE_BOOL &&
+         InstanceTypeChecker::IsCodeDataContainer(instance_type)));
+  }
+}
+
 void Serializer::ObjectSerializer::VisitRuntimeEntry(Code host,
                                                      RelocInfo* rinfo) {
   // We no longer serialize code that contains runtime entries.

src/snapshot/serializer.h

@@ -447,14 +447,15 @@ class Serializer::ObjectSerializer : public ObjectVisitor {
                      MaybeObjectSlot end) override;
   void VisitCodePointer(HeapObject host, CodeObjectSlot slot) override;
   void VisitEmbeddedPointer(Code host, RelocInfo* target) override;
-  void VisitExternalReference(Foreign host, Address* p) override;
   void VisitExternalReference(Code host, RelocInfo* rinfo) override;
-  void VisitExternalPointer(HeapObject host, ExternalPointer_t ptr) override;
   void VisitInternalReference(Code host, RelocInfo* rinfo) override;
   void VisitCodeTarget(Code host, RelocInfo* target) override;
   void VisitRuntimeEntry(Code host, RelocInfo* reloc) override;
   void VisitOffHeapTarget(Code host, RelocInfo* target) override;
 
+  void VisitExternalPointer(HeapObject host, ExternalPointerSlot slot,
+                            ExternalPointerTag tag) override;
+
   Isolate* isolate() { return isolate_; }
 
  private: