[sandbox] Implement ReadExternalPointerField in v8-internal.h

Previously it was implemented in api.cc, therefore requiring an additional
function call when accessing external pointer fields from embedder code with
the sandbox enabled. Now ReadExternalPointerField can be inlined.

Bug: v8:10391
Change-Id: Ia8cb2df148ac96f979fd3e22989b0ff6177abcec
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/+/3714245Reviewed-by: Igor Sheludko <ishell@chromium.org>
Auto-Submit: Samuel Groß <saelo@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81271}

include/v8-context.h

@@ -379,7 +379,7 @@ void* Context::GetAlignedPointerFromEmbedderData(int index) {
 #ifdef V8_SANDBOXED_EXTERNAL_POINTERS
   value_offset += I::kEmbedderDataSlotRawPayloadOffset;
 #endif
-  internal::Isolate* isolate = I::GetIsolateForSandbox(ctx);
+  Isolate* isolate = I::GetIsolateForSandbox(ctx);
   return reinterpret_cast<void*>(
       I::ReadExternalPointerField(isolate, embedder_data, value_offset,
                                   internal::kEmbedderDataSlotPayloadTag));

include/v8-internal.h

@@ -325,11 +325,6 @@ enum ExternalPointerTag : uint64_t {
 // clang-format on
 #undef MAKE_TAG
 
-// Converts encoded external pointer to address.
-V8_EXPORT Address DecodeExternalPointerImpl(const Isolate* isolate,
-                                            ExternalPointer_t pointer,
-                                            ExternalPointerTag tag);
-
 // {obj} must be the raw tagged pointer representation of a HeapObject
 // that's guaranteed to never be in ReadOnlySpace.
 V8_EXPORT internal::Isolate* IsolateFromNeverReadOnlySpaceObject(Address obj);
@@ -379,6 +374,17 @@ class Internals {
   static const int kBuiltinTier0EntryTableSize = 7 * kApiSystemPointerSize;
   static const int kBuiltinTier0TableSize = 7 * kApiSystemPointerSize;
 
+  // ExternalPointerTable layout guarantees.
+  static const int kExternalPointerTableBufferOffset = 0;
+  static const int kExternalPointerTableCapacityOffset =
+      kExternalPointerTableBufferOffset + kApiSystemPointerSize;
+  static const int kExternalPointerTableFreelistHeadOffset =
+      kExternalPointerTableCapacityOffset + kApiInt32Size;
+  static const int kExternalPointerTableMutexOffset =
+      kExternalPointerTableFreelistHeadOffset + kApiInt32Size;
+  static const int kExternalPointerTableSize =
+      kExternalPointerTableMutexOffset + kApiSystemPointerSize;
+
   // IsolateData layout guarantees.
   static const int kIsolateCageBaseOffset = 0;
   static const int kIsolateStackGuardOffset =
@@ -397,14 +403,15 @@ class Internals {
       kIsolateFastCCallCallerPcOffset + kApiSystemPointerSize;
   static const int kIsolateLongTaskStatsCounterOffset =
       kIsolateFastApiCallTargetOffset + kApiSystemPointerSize;
+#ifdef V8_SANDBOXED_EXTERNAL_POINTERS
+  static const int kIsolateExternalPointerTableOffset =
+      kIsolateLongTaskStatsCounterOffset + kApiSizetSize;
+  static const int kIsolateRootsOffset =
+      kIsolateExternalPointerTableOffset + kExternalPointerTableSize;
+#else
   static const int kIsolateRootsOffset =
       kIsolateLongTaskStatsCounterOffset + kApiSizetSize;
-
-  static const int kExternalPointerTableBufferOffset = 0;
-  static const int kExternalPointerTableCapacityOffset =
-      kExternalPointerTableBufferOffset + kApiSystemPointerSize;
-  static const int kExternalPointerTableFreelistHeadOffset =
-      kExternalPointerTableCapacityOffset + kApiInt32Size;
+#endif
 
   static const int kUndefinedValueRootIndex = 4;
   static const int kTheHoleValueRootIndex = 5;
@@ -554,6 +561,16 @@ class Internals {
     return reinterpret_cast<internal::Address*>(addr);
   }
 
+#ifdef V8_SANDBOXED_EXTERNAL_POINTERS
+  V8_INLINE static internal::Address* GetExternalPointerTableBase(
+      v8::Isolate* isolate) {
+    internal::Address addr = reinterpret_cast<internal::Address>(isolate) +
+                             kIsolateExternalPointerTableOffset +
+                             kExternalPointerTableBufferOffset;
+    return *reinterpret_cast<internal::Address**>(addr);
+  }
+#endif
+
   template <typename T>
   V8_INLINE static T ReadRawField(internal::Address heap_object_ptr,
                                   int offset) {
@@ -594,35 +611,27 @@ class Internals {
 #endif
   }
 
-  V8_INLINE static internal::Isolate* GetIsolateForSandbox(
-      internal::Address obj) {
+  V8_INLINE static v8::Isolate* GetIsolateForSandbox(internal::Address obj) {
 #ifdef V8_SANDBOXED_EXTERNAL_POINTERS
-    return internal::IsolateFromNeverReadOnlySpaceObject(obj);
+    return reinterpret_cast<v8::Isolate*>(
+        internal::IsolateFromNeverReadOnlySpaceObject(obj));
 #else
     // Not used in non-sandbox mode.
     return nullptr;
 #endif
   }
 
-  V8_INLINE static Address DecodeExternalPointer(
-      const Isolate* isolate, ExternalPointer_t encoded_pointer,
-      ExternalPointerTag tag) {
-#ifdef V8_SANDBOXED_EXTERNAL_POINTERS
-    return internal::DecodeExternalPointerImpl(isolate, encoded_pointer, tag);
-#else
-    return encoded_pointer;
-#endif
-  }
-
   V8_INLINE static internal::Address ReadExternalPointerField(
-      internal::Isolate* isolate, internal::Address heap_object_ptr, int offset,
+      v8::Isolate* isolate, internal::Address heap_object_ptr, int offset,
       ExternalPointerTag tag) {
 #ifdef V8_SANDBOXED_EXTERNAL_POINTERS
+    // See src/sandbox/external-pointer-table-inl.h. Logic duplicated here so it
+    // can be inlined in embedder code and doesn't require an additional call.
+    internal::Address* table = GetExternalPointerTableBase(isolate);
     internal::ExternalPointer_t encoded_value =
-        ReadRawField<uint32_t>(heap_object_ptr, offset);
-    // We currently have to treat zero as nullptr in embedder slots.
-    return encoded_value ? DecodeExternalPointer(isolate, encoded_value, tag)
-                         : 0;
+        ReadRawField<ExternalPointer_t>(heap_object_ptr, offset);
+    uint32_t index = encoded_value >> kExternalPointerIndexShift;
+    return table[index] & ~tag;
 #else
     return ReadRawField<Address>(heap_object_ptr, offset);
 #endif

include/v8-object.h

@@ -741,7 +741,7 @@ void* Object::GetAlignedPointerFromInternalField(int index) {
 #ifdef V8_SANDBOXED_EXTERNAL_POINTERS
     offset += I::kEmbedderDataSlotRawPayloadOffset;
 #endif
-    internal::Isolate* isolate = I::GetIsolateForSandbox(obj);
+    Isolate* isolate = I::GetIsolateForSandbox(obj);
     A value = I::ReadExternalPointerField(
         isolate, obj, offset, internal::kEmbedderDataSlotPayloadTag);
     return reinterpret_cast<void*>(value);

include/v8-primitive.h

@@ -785,7 +785,7 @@ String::ExternalStringResource* String::GetExternalStringResource() const {
 
   ExternalStringResource* result;
   if (I::IsExternalTwoByteString(I::GetInstanceType(obj))) {
-    internal::Isolate* isolate = I::GetIsolateForSandbox(obj);
+    Isolate* isolate = I::GetIsolateForSandbox(obj);
     A value =
         I::ReadExternalPointerField(isolate, obj, I::kStringResourceOffset,
                                     internal::kExternalStringResourceTag);
@@ -809,7 +809,7 @@ String::ExternalStringResourceBase* String::GetExternalStringResourceBase(
   ExternalStringResourceBase* resource;
   if (type == I::kExternalOneByteRepresentationTag ||
       type == I::kExternalTwoByteRepresentationTag) {
-    internal::Isolate* isolate = I::GetIsolateForSandbox(obj);
+    Isolate* isolate = I::GetIsolateForSandbox(obj);
     A value =
         I::ReadExternalPointerField(isolate, obj, I::kStringResourceOffset,
                                     internal::kExternalStringResourceTag);

src/api/api.cc

@@ -3856,12 +3856,6 @@ MaybeLocal<Uint32> Value::ToUint32(Local<Context> context) const {
   RETURN_ESCAPED(result);
 }
 
-i::Address i::DecodeExternalPointerImpl(const i::Isolate* i_isolate,
-                                        i::ExternalPointer_t encoded_pointer,
-                                        ExternalPointerTag tag) {
-  return i::DecodeExternalPointer(i_isolate, encoded_pointer, tag);
-}
-
 i::Isolate* i::IsolateFromNeverReadOnlySpaceObject(i::Address obj) {
   return i::GetIsolateFromWritableObject(i::HeapObject::cast(i::Object(obj)));
 }
@@ -5802,9 +5796,9 @@ String::ExternalStringResource* String::GetExternalStringResourceSlow() const {
   }
 
   if (i::StringShape(str).IsExternalTwoByte()) {
-    i::Isolate* i_isolate = i::Internals::GetIsolateForSandbox(str.ptr());
+    Isolate* isolate = i::Internals::GetIsolateForSandbox(str.ptr());
     i::Address value = i::Internals::ReadExternalPointerField(
-        i_isolate, str.ptr(), i::Internals::kStringResourceOffset,
+        isolate, str.ptr(), i::Internals::kStringResourceOffset,
         i::kExternalStringResourceTag);
     return reinterpret_cast<String::ExternalStringResource*>(value);
   }
@@ -5846,9 +5840,9 @@ String::ExternalStringResourceBase* String::GetExternalStringResourceBaseSlow(
       static_cast<Encoding>(type & i::Internals::kStringEncodingMask);
   if (i::StringShape(str).IsExternalOneByte() ||
       i::StringShape(str).IsExternalTwoByte()) {
-    i::Isolate* i_isolate = i::Internals::GetIsolateForSandbox(string);
+    Isolate* isolate = i::Internals::GetIsolateForSandbox(string);
     i::Address value = i::Internals::ReadExternalPointerField(
-        i_isolate, string, i::Internals::kStringResourceOffset,
+        isolate, string, i::Internals::kStringResourceOffset,
         i::kExternalStringResourceTag);
     resource = reinterpret_cast<ExternalStringResourceBase*>(value);
   }

src/execution/isolate-data.h

@@ -39,6 +39,7 @@ class Isolate;
   V(kFastCCallCallerPCOffset, kSystemPointerSize, fast_c_call_caller_pc)      \
   V(kFastApiCallTargetOffset, kSystemPointerSize, fast_api_call_target)       \
   V(kLongTaskStatsCounterOffset, kSizetSize, long_task_stats_counter)         \
+  ISOLATE_DATA_FIELDS_SANDBOXED_EXTERNAL_POINTERS(V)                          \
   /* Full tables (arbitrary size, potentially slower access). */              \
   V(kRootsTableOffset, RootsTable::kEntriesCount* kSystemPointerSize,         \
     roots_table)                                                              \
@@ -52,7 +53,6 @@ class Isolate;
   /* Linear allocation areas for the heap's new and old space */              \
   V(kNewAllocationInfo, LinearAllocationArea::kSize, new_allocation_info)     \
   V(kOldAllocationInfo, LinearAllocationArea::kSize, old_allocation_info)     \
-  ISOLATE_DATA_FIELDS_SANDBOXED_EXTERNAL_POINTERS(V)                          \
   V(kStackIsIterableOffset, kUInt8Size, stack_is_iterable)
 
 #ifdef V8_SANDBOXED_EXTERNAL_POINTERS
@@ -193,6 +193,11 @@ class IsolateData final {
   // long tasks.
   size_t long_task_stats_counter_ = 0;
 
+  // Table containing pointers to external objects.
+#ifdef V8_SANDBOXED_EXTERNAL_POINTERS
+  ExternalPointerTable external_pointer_table_;
+#endif
+
   RootsTable roots_table_;
   ExternalReferenceTable external_reference_table_;
 
@@ -209,11 +214,6 @@ class IsolateData final {
   LinearAllocationArea new_allocation_info_;
   LinearAllocationArea old_allocation_info_;
 
-  // Table containing pointers to external objects.
-#ifdef V8_SANDBOXED_EXTERNAL_POINTERS
-  ExternalPointerTable external_pointer_table_;
-#endif
-
   // Whether the SafeStackFrameIterator can successfully iterate the current
   // stack. Only valid values are 0 or 1.
   uint8_t stack_is_iterable_ = 1;

src/execution/isolate.cc

@@ -3348,6 +3348,21 @@ Isolate::Isolate(std::unique_ptr<i::IsolateAllocator> isolate_allocator,
 }
 
 void Isolate::CheckIsolateLayout() {
+#ifdef V8_SANDBOXED_EXTERNAL_POINTERS
+  CHECK_EQ(static_cast<int>(OFFSET_OF(ExternalPointerTable, buffer_)),
+           Internals::kExternalPointerTableBufferOffset);
+  CHECK_EQ(static_cast<int>(OFFSET_OF(ExternalPointerTable, capacity_)),
+           Internals::kExternalPointerTableCapacityOffset);
+  CHECK_EQ(static_cast<int>(OFFSET_OF(ExternalPointerTable, freelist_head_)),
+           Internals::kExternalPointerTableFreelistHeadOffset);
+  CHECK_EQ(static_cast<int>(OFFSET_OF(ExternalPointerTable, mutex_)),
+           Internals::kExternalPointerTableMutexOffset);
+  CHECK_EQ(static_cast<int>(sizeof(ExternalPointerTable)),
+           Internals::kExternalPointerTableSize);
+  CHECK_EQ(static_cast<int>(sizeof(ExternalPointerTable)),
+           ExternalPointerTable::kSize);
+#endif
+
   CHECK_EQ(OFFSET_OF(Isolate, isolate_data_), 0);
   CHECK_EQ(static_cast<int>(OFFSET_OF(Isolate, isolate_data_.embedder_data_)),
            Internals::kIsolateEmbedderDataOffset);
@@ -3364,6 +3379,11 @@ void Isolate::CheckIsolateLayout() {
            Internals::kIsolateLongTaskStatsCounterOffset);
   CHECK_EQ(static_cast<int>(OFFSET_OF(Isolate, isolate_data_.stack_guard_)),
            Internals::kIsolateStackGuardOffset);
+#ifdef V8_SANDBOXED_EXTERNAL_POINTERS
+  CHECK_EQ(static_cast<int>(
+               OFFSET_OF(Isolate, isolate_data_.external_pointer_table_)),
+           Internals::kIsolateExternalPointerTableOffset);
+#endif
   CHECK_EQ(static_cast<int>(OFFSET_OF(Isolate, isolate_data_.roots_table_)),
            Internals::kIsolateRootsOffset);
 
@@ -3372,15 +3392,6 @@ void Isolate::CheckIsolateLayout() {
                 Builtins::kBuiltinTier0Count * kSystemPointerSize);
   static_assert(Internals::kBuiltinTier0EntryTableSize ==
                 Builtins::kBuiltinTier0Count * kSystemPointerSize);
-
-#ifdef V8_SANDBOXED_EXTERNAL_POINTERS
-  CHECK_EQ(static_cast<int>(OFFSET_OF(ExternalPointerTable, buffer_)),
-           Internals::kExternalPointerTableBufferOffset);
-  CHECK_EQ(static_cast<int>(OFFSET_OF(ExternalPointerTable, capacity_)),
-           Internals::kExternalPointerTableCapacityOffset);
-  CHECK_EQ(static_cast<int>(OFFSET_OF(ExternalPointerTable, freelist_head_)),
-           Internals::kExternalPointerTableFreelistHeadOffset);
-#endif
 }
 
 void Isolate::ClearSerializerData() {