[sandbox] Shrink ExternalPointer_t to 32 bits

When sandboxed external pointers are enabled, external pointers now only
require 32 bits of storage space in a HeapObject. This CL does not shrink
the size of EmbedderDataSlots, which will happen in a follow-up CL.

Bug: v8:10391
Change-Id: I3cf8b68c3b985cf806a45183717f50462a88c281
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/+/3359629Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Samuel Groß <saelo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78754}

include/v8-internal.h

@@ -179,7 +179,11 @@ using SandboxedPointer_t = Address;
 // ExternalPointers point to objects located outside the sandbox. When sandboxed
 // external pointers are enabled, these are stored in an external pointer table
 // and referenced from HeapObjects through indices.
+#ifdef V8_SANDBOXED_EXTERNAL_POINTERS
+using ExternalPointer_t = uint32_t;
+#else
 using ExternalPointer_t = Address;
+#endif
 
 #ifdef V8_SANDBOX_IS_AVAILABLE
 

src/codegen/arm64/macro-assembler-arm64.cc

@@ -3119,7 +3119,7 @@ void TurboAssembler::LoadExternalPointerField(Register destination,
       MemOperand(isolate_root,
                  IsolateData::external_pointer_table_offset() +
                      Internals::kExternalPointerTableBufferOffset));
-  Ldr(destination, field_operand);
+  Ldr(destination.W(), field_operand);
   Ldr(destination,
       MemOperand(external_table, destination, LSL, kSystemPointerSizeLog2));
   if (tag != 0) {

src/codegen/code-stub-assembler.cc

@@ -1347,10 +1347,10 @@ TNode<HeapObject> CodeStubAssembler::AllocateRawDoubleAligned(
                      top_address, limit_address);
 #elif defined(V8_HOST_ARCH_64_BIT)
 #ifdef V8_COMPRESS_POINTERS
-  // TODO(ishell, v8:8875): Consider using aligned allocations once the
-  // allocation alignment inconsistency is fixed. For now we keep using
-  // unaligned access since both x64 and arm64 architectures (where pointer
-  // compression is supported) allow unaligned access to doubles and full words.
+// TODO(ishell, v8:8875): Consider using aligned allocations once the
+// allocation alignment inconsistency is fixed. For now we keep using
+// unaligned access since both x64 and arm64 architectures (where pointer
+// compression is supported) allow unaligned access to doubles and full words.
 #endif  // V8_COMPRESS_POINTERS
   // Allocation on 64 bit machine is naturally double aligned
   return AllocateRaw(size_in_bytes, flags & ~AllocationFlag::kDoubleAlignment,
@@ -1586,14 +1586,14 @@ TNode<RawPtrT> CodeStubAssembler::EmptyBackingStoreBufferConstant() {
 
 TNode<ExternalPointerT> CodeStubAssembler::ChangeUint32ToExternalPointer(
     TNode<Uint32T> value) {
-  STATIC_ASSERT(kExternalPointerSize == kSystemPointerSize);
-  return ReinterpretCast<ExternalPointerT>(ChangeUint32ToWord(value));
+  DCHECK_EQ(kExternalPointerSize, kUInt32Size);
+  return ReinterpretCast<ExternalPointerT>(value);
 }
 
 TNode<Uint32T> CodeStubAssembler::ChangeExternalPointerToUint32(
     TNode<ExternalPointerT> value) {
-  STATIC_ASSERT(kExternalPointerSize == kSystemPointerSize);
-  return Unsigned(TruncateWordToInt32(ReinterpretCast<UintPtrT>(value)));
+  DCHECK_EQ(kExternalPointerSize, kUInt32Size);
+  return ReinterpretCast<Uint32T>(value);
 }
 
 void CodeStubAssembler::InitializeExternalPointerField(TNode<HeapObject> object,
@@ -1675,7 +1675,6 @@ TNode<RawPtrT> CodeStubAssembler::LoadExternalPointerFromObject(
   TNode<ExternalPointerT> encoded =
       LoadObjectField<ExternalPointerT>(object, offset);
   TNode<Word32T> index = ChangeExternalPointerToUint32(encoded);
-  // TODO(v8:10391, saelo): bounds check if table is not caged
   TNode<IntPtrT> table_offset = ElementOffsetFromIndex(
       ChangeUint32ToWord(index), SYSTEM_POINTER_ELEMENTS, 0);
 

src/codegen/tnode.h

@@ -84,11 +84,16 @@ struct UintPtrT : WordT {
   static constexpr MachineType kMachineType = MachineType::UintPtr();
 };
 
+// An index into the external pointer table.
+#ifdef V8_SANDBOXED_EXTERNAL_POINTERS
+struct ExternalPointerT : Uint32T {
+  static constexpr MachineType kMachineType = MachineType::Uint32();
+};
+#else
 struct ExternalPointerT : UntaggedT {
-  static const MachineRepresentation kMachineRepresentation =
-      MachineType::PointerRepresentation();
   static constexpr MachineType kMachineType = MachineType::Pointer();
 };
+#endif
 
 struct Float32T : UntaggedT {
   static const MachineRepresentation kMachineRepresentation =

src/common/globals.h

@@ -352,6 +352,11 @@ STATIC_ASSERT(kPointerSize == (1 << kPointerSizeLog2));
 // This type defines raw storage type for external (or off-V8 heap) pointers
 // stored on V8 heap.
 constexpr int kExternalPointerSize = sizeof(ExternalPointer_t);
+#ifdef V8_SANDBOXED_EXTERNAL_POINTERS
+STATIC_ASSERT(kExternalPointerSize == kTaggedSize);
+#else
+STATIC_ASSERT(kExternalPointerSize == kSystemPointerSize);
+#endif
 
 constexpr int kEmbedderDataSlotSize = kSystemPointerSize;
 

src/compiler/backend/instruction.cc

@@ -262,7 +262,7 @@ std::ostream& operator<<(std::ostream& os, const InstructionOperand& op) {
           os << "|c";
           break;
         case MachineRepresentation::kSandboxedPointer:
-          os << "|cg";
+          os << "|sb";
           break;
         case MachineRepresentation::kMapWord:
           UNREACHABLE();

src/compiler/memory-lowering.cc

@@ -411,6 +411,7 @@ Node* MemoryLowering::DecodeExternalPointer(
 #ifdef V8_SANDBOXED_EXTERNAL_POINTERS
   DCHECK(V8_SANDBOXED_EXTERNAL_POINTERS_BOOL);
   DCHECK(node->opcode() == IrOpcode::kLoad);
+  DCHECK_EQ(kExternalPointerSize, kUInt32Size);
   Node* effect = NodeProperties::GetEffectInput(node);
   Node* control = NodeProperties::GetControlInput(node);
   __ InitializeEffectControl(effect, control);
@@ -424,12 +425,17 @@ Node* MemoryLowering::DecodeExternalPointer(
   // __ DebugBreak();
 
   // Decode loaded external pointer.
-  STATIC_ASSERT(kExternalPointerSize == kSystemPointerSize);
-  Node* external_pointer_table_address = __ ExternalConstant(
+  //
+  // Here we access the external pointer table through an ExternalReference.
+  // Alternatively, we could also hardcode the address of the table since it is
+  // never reallocated. However, in that case we must be able to guarantee that
+  // the generated code is never executed under a different Isolate, as that
+  // would allow access to external objects from different Isolates. It also
+  // would break if the code is serialized/deserialized at some point.
+  Node* table_address = __ ExternalConstant(
       ExternalReference::external_pointer_table_address(isolate()));
-  Node* table = __ Load(MachineType::Pointer(), external_pointer_table_address,
+  Node* table = __ Load(MachineType::Pointer(), table_address,
                         Internals::kExternalPointerTableBufferOffset);
-  // TODO(v8:10391, saelo): bounds check if table is not caged
   Node* offset = __ Int32Mul(index, __ Int32Constant(sizeof(Address)));
   Node* decoded_ptr =
       __ Load(MachineType::Pointer(), table, __ ChangeUint32ToUint64(offset));
@@ -467,7 +473,7 @@ Reduction MemoryLowering::ReduceLoadField(Node* node) {
   MachineType type = access.machine_type;
   if (V8_SANDBOXED_EXTERNAL_POINTERS_BOOL &&
       access.type.Is(Type::SandboxedExternalPointer())) {
-    // External pointer table indices are 32bit numbers
+    // External pointer table indices are stored as 32-bit numbers
     type = MachineType::Uint32();
   }
 

src/compiler/wasm-compiler.cc

@@ -3224,19 +3224,24 @@ Node* WasmGraphBuilder::BuildIndirectCall(
   }
 }
 
-Node* WasmGraphBuilder::BuildUnsandboxExternalPointer(Node* external_pointer) {
+Node* WasmGraphBuilder::BuildLoadExternalPointerFromObject(Node* object,
+                                                           int offset) {
 #ifdef V8_SANDBOXED_EXTERNAL_POINTERS
+  Node* external_pointer = gasm_->LoadFromObject(
+      MachineType::Uint32(), object, wasm::ObjectAccess::ToTagged(offset));
   Node* isolate_root = BuildLoadIsolateRoot();
   Node* table =
       gasm_->LoadFromObject(MachineType::Pointer(), isolate_root,
                             IsolateData::external_pointer_table_offset() +
                                 Internals::kExternalPointerTableBufferOffset);
-  Node* offset = gasm_->Int32Mul(external_pointer, gasm_->Int32Constant(8));
-  Node* decoded_ptr = gasm_->Load(MachineType::Pointer(), table, offset);
+  Node* scaled_index = gasm_->Int32Mul(
+      external_pointer, gasm_->Int32Constant(kSystemPointerSize));
+  Node* decoded_ptr = gasm_->Load(MachineType::Pointer(), table, scaled_index);
   Node* tag = gasm_->IntPtrConstant(~kForeignForeignAddressTag);
   return gasm_->WordAnd(decoded_ptr, tag);
 #else
-  return external_pointer;
+  return gasm_->LoadFromObject(MachineType::Pointer(), object,
+                               wasm::ObjectAccess::ToTagged(offset));
 #endif
 }
 
@@ -3245,11 +3250,8 @@ Node* WasmGraphBuilder::BuildLoadCallTargetFromExportedFunctionData(
   Node* internal = gasm_->LoadFromObject(
       MachineType::TaggedPointer(), function,
       wasm::ObjectAccess::ToTagged(WasmExportedFunctionData::kInternalOffset));
-  Node* external_pointer =
-      gasm_->LoadFromObject(MachineType::Pointer(), internal,
-                            wasm::ObjectAccess::ToTagged(
-                                WasmInternalFunction::kForeignAddressOffset));
-  return BuildUnsandboxExternalPointer(external_pointer);
+  return BuildLoadExternalPointerFromObject(
+      internal, WasmInternalFunction::kForeignAddressOffset);
 }
 
 // TODO(9495): Support CAPI function refs.
@@ -3272,12 +3274,8 @@ Node* WasmGraphBuilder::BuildCallRef(const wasm::FunctionSig* real_sig,
       MachineType::TaggedPointer(), function,
       wasm::ObjectAccess::ToTagged(WasmInternalFunction::kRefOffset));
 
-  Node* external_target =
-      gasm_->LoadFromObject(MachineType::Pointer(), function,
-                            wasm::ObjectAccess::ToTagged(
-                                WasmInternalFunction::kForeignAddressOffset));
-
-  Node* target = BuildUnsandboxExternalPointer(external_target);
+  Node* target = BuildLoadExternalPointerFromObject(
+      function, WasmInternalFunction::kForeignAddressOffset);
   Node* is_null_target = gasm_->WordEqual(target, gasm_->IntPtrConstant(0));
   gasm_->GotoIfNot(is_null_target, &end_label, target);
   {
@@ -6761,11 +6759,8 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
         Node* internal = gasm_->LoadFromObject(
             MachineType::TaggedPointer(), function_data,
             wasm::ObjectAccess::ToTagged(WasmFunctionData::kInternalOffset));
-        Node* sandboxed_pointer = gasm_->LoadFromObject(
-            MachineType::Pointer(), internal,
-            wasm::ObjectAccess::ToTagged(
-                WasmInternalFunction::kForeignAddressOffset));
-        args[0] = BuildUnsandboxExternalPointer(sandboxed_pointer);
+        args[0] = BuildLoadExternalPointerFromObject(
+            internal, WasmInternalFunction::kForeignAddressOffset);
         Node* instance_node = gasm_->LoadFromObject(
             MachineType::TaggedPointer(), internal,
             wasm::ObjectAccess::ToTagged(WasmInternalFunction::kRefOffset));

src/compiler/wasm-compiler.h

@@ -776,7 +776,7 @@ class WasmGraphBuilder {
   Node* BuildMultiReturnFixedArrayFromIterable(const wasm::FunctionSig* sig,
                                                Node* iterable, Node* context);
 
-  Node* BuildUnsandboxExternalPointer(Node* external_pointer);
+  Node* BuildLoadExternalPointerFromObject(Node* object, int offset);
 
   Node* BuildLoadCallTargetFromExportedFunctionData(Node* function_data);
 

src/objects/js-array-buffer.tq

@@ -14,7 +14,8 @@ bitfield struct JSArrayBufferFlags extends uint32 {
 extern class JSArrayBuffer extends JSObject {
   byte_length: uintptr;
   max_byte_length: uintptr;
-  backing_store: ExternalPointer;
+  // A SandboxedPtr if the sandbox is enabled
+  backing_store: RawPtr;
   extension: RawPtr;
   bit_field: JSArrayBufferFlags;
   // Pads header size to be a multiple of kTaggedSize.
@@ -74,7 +75,8 @@ macro IsLengthTrackingJSArrayBufferView(array: JSArrayBufferView): bool {
 
 extern class JSTypedArray extends JSArrayBufferView {
   length: uintptr;
-  external_pointer: ExternalPointer;
+  // A SandboxedPtr if the sandbox is enabled
+  external_pointer: RawPtr;
   base_pointer: ByteArray|Smi;
 }
 
@@ -85,7 +87,8 @@ macro IsOnHeapTypedArray(array: JSTypedArray): bool {
 }
 
 extern class JSDataView extends JSArrayBufferView {
-  data_pointer: ExternalPointer;
+  // A SandboxedPtr if the sandbox is enabled
+  data_pointer: RawPtr;
 }
 
 @abstract

src/sandbox/external-pointer-inl.h

@@ -16,11 +16,11 @@ namespace internal {
 V8_INLINE Address DecodeExternalPointer(const Isolate* isolate,
                                         ExternalPointer_t encoded_pointer,
                                         ExternalPointerTag tag) {
-  STATIC_ASSERT(kExternalPointerSize == kSystemPointerSize);
 #ifdef V8_SANDBOXED_EXTERNAL_POINTERS
-  uint32_t index = static_cast<uint32_t>(encoded_pointer);
-  return isolate->external_pointer_table().Get(index, tag);
+  STATIC_ASSERT(kExternalPointerSize == kInt32Size);
+  return isolate->external_pointer_table().Get(encoded_pointer, tag);
 #else
+  STATIC_ASSERT(kExternalPointerSize == kSystemPointerSize);
   return encoded_pointer;
 #endif
 }
@@ -34,12 +34,8 @@ V8_INLINE void InitExternalPointerField(Address field_address, Isolate* isolate,
                                         Address value, ExternalPointerTag tag) {
 #ifdef V8_SANDBOXED_EXTERNAL_POINTERS
   ExternalPointer_t index = isolate->external_pointer_table().Allocate();
-  isolate->external_pointer_table().Set(static_cast<uint32_t>(index), value,
-                                        tag);
-  static_assert(kExternalPointerSize == kSystemPointerSize,
-                "Review the code below, once kExternalPointerSize is 4-byte "
-                "the address of the field will always be aligned");
-  base::WriteUnalignedValue<ExternalPointer_t>(field_address, index);
+  isolate->external_pointer_table().Set(index, value, tag);
+  base::Memory<ExternalPointer_t>(field_address) = index;
 #else
   // Pointer compression causes types larger than kTaggedSize to be unaligned.
   constexpr bool v8_pointer_compression_unaligned =
@@ -54,9 +50,6 @@ V8_INLINE void InitExternalPointerField(Address field_address, Isolate* isolate,
 }
 
 V8_INLINE ExternalPointer_t ReadRawExternalPointerField(Address field_address) {
-  static_assert(kExternalPointerSize == kSystemPointerSize,
-                "Review the code below, once kExternalPointerSize is 4-byte "
-                "the address of the field will always be aligned");
   // Pointer compression causes types larger than kTaggedSize to be unaligned.
   constexpr bool v8_pointer_compression_unaligned =
       kExternalPointerSize > kTaggedSize;
@@ -78,13 +71,8 @@ V8_INLINE void WriteExternalPointerField(Address field_address,
                                          Isolate* isolate, Address value,
                                          ExternalPointerTag tag) {
 #ifdef V8_SANDBOXED_EXTERNAL_POINTERS
-  static_assert(kExternalPointerSize == kSystemPointerSize,
-                "Review the code below, once kExternalPointerSize is 4-byte "
-                "the address of the field will always be aligned");
-  ExternalPointer_t index =
-      base::ReadUnalignedValue<ExternalPointer_t>(field_address);
-  isolate->external_pointer_table().Set(static_cast<uint32_t>(index), value,
-                                        tag);
+  ExternalPointer_t index = base::Memory<ExternalPointer_t>(field_address);
+  isolate->external_pointer_table().Set(index, value, tag);
 #else
   // Pointer compression causes types larger than kTaggedSize to be unaligned.
   constexpr bool v8_pointer_compression_unaligned =

src/snapshot/deserializer.cc

@@ -208,8 +208,8 @@ template <typename TSlot>
 int Deserializer<IsolateT>::WriteExternalPointer(TSlot dest, Address value,
                                                  ExternalPointerTag tag) {
   DCHECK(!next_reference_is_weak_);
+  DCHECK(IsAligned(kExternalPointerSize, TSlot::kSlotDataSize));
   InitExternalPointerField(dest.address(), main_thread_isolate(), value, tag);
-  STATIC_ASSERT(IsAligned(kExternalPointerSize, TSlot::kSlotDataSize));
   return (kExternalPointerSize / TSlot::kSlotDataSize);
 }
 

src/snapshot/serializer.cc

@@ -965,7 +965,7 @@ void Serializer::ObjectSerializer::OutputExternalReference(Address target,
 void Serializer::ObjectSerializer::VisitExternalReference(Foreign host,
                                                           Address* p) {
   // "Sandboxify" external reference.
-  OutputExternalReference(host.foreign_address(), kExternalPointerSize, true);
+  OutputExternalReference(host.foreign_address(), kSystemPointerSize, true);
   bytes_processed_so_far_ += kExternalPointerSize;
 }
 

test/cctest/test-api.cc

@@ -693,8 +693,9 @@ TEST(MakingExternalStringConditions) {
   CHECK(local_string->CanMakeExternal());
 
   // Tiny strings are not in-place externalizable when pointer compression is
-  // enabled.
-  CHECK_EQ(i::kTaggedSize == i::kSystemPointerSize,
+  // enabled, but they are if sandboxed external pointers are enabled.
+  CHECK_EQ(V8_SANDBOXED_EXTERNAL_POINTERS_BOOL ||
+               i::kTaggedSize == i::kSystemPointerSize,
            tiny_local_string->CanMakeExternal());
 }
 
@@ -723,8 +724,9 @@ TEST(MakingExternalOneByteStringConditions) {
   CHECK(local_string->CanMakeExternal());
 
   // Tiny strings are not in-place externalizable when pointer compression is
-  // enabled.
-  CHECK_EQ(i::kTaggedSize == i::kSystemPointerSize,
+  // enabled, but they are if sandboxed external pointers are enabled.
+  CHECK_EQ(V8_SANDBOXED_EXTERNAL_POINTERS_BOOL ||
+               i::kTaggedSize == i::kSystemPointerSize,
            tiny_local_string->CanMakeExternal());
 }
 

test/cctest/test-strings.cc

@@ -2136,15 +2136,19 @@ TEST(CheckCachedDataInternalExternalUncachedString) {
   // that we indeed cached it.
   Handle<ExternalOneByteString> external_string =
       Handle<ExternalOneByteString>::cast(string);
-  CHECK(external_string->is_uncached());
+  // If sandboxed external pointers are enabled, string objects will always be
+  // cacheable because they are smaller.
+  CHECK(V8_SANDBOXED_EXTERNAL_POINTERS_BOOL || external_string->is_uncached());
   CHECK(external_string->resource()->IsCacheable());
-  CHECK_NOT_NULL(external_string->resource()->cached_data());
-  CHECK_EQ(external_string->resource()->cached_data(),
-           external_string->resource()->data());
+  if (!V8_SANDBOXED_EXTERNAL_POINTERS_BOOL) {
+    CHECK_NOT_NULL(external_string->resource()->cached_data());
+    CHECK_EQ(external_string->resource()->cached_data(),
+             external_string->resource()->data());
+  }
 }
 
 // Show that we cache the data pointer for internal, external and uncached
-// strings with cacheable resources through MakeExternal. One byte version.
+// strings with cacheable resources through MakeExternal. Two byte version.
 TEST(CheckCachedDataInternalExternalUncachedStringTwoByte) {
   CcTest::InitializeVM();
   Factory* factory = CcTest::i_isolate()->factory();
@@ -2175,11 +2179,15 @@ TEST(CheckCachedDataInternalExternalUncachedStringTwoByte) {
   // that we indeed cached it.
   Handle<ExternalTwoByteString> external_string =
       Handle<ExternalTwoByteString>::cast(string);
-  CHECK(external_string->is_uncached());
+  // If sandboxed external pointers are enabled, string objects will always be
+  // cacheable because they are smaller.
+  CHECK(V8_SANDBOXED_EXTERNAL_POINTERS_BOOL || external_string->is_uncached());
   CHECK(external_string->resource()->IsCacheable());
-  CHECK_NOT_NULL(external_string->resource()->cached_data());
-  CHECK_EQ(external_string->resource()->cached_data(),
-           external_string->resource()->data());
+  if (!V8_SANDBOXED_EXTERNAL_POINTERS_BOOL) {
+    CHECK_NOT_NULL(external_string->resource()->cached_data());
+    CHECK_EQ(external_string->resource()->cached_data(),
+             external_string->resource()->data());
+  }
 }
 
 }  // namespace test_strings