diff --git a/include/v8-context.h b/include/v8-context.h index 0fb760bfbf..227a86f23b 100644 --- a/include/v8-context.h +++ b/include/v8-context.h @@ -368,17 +368,17 @@ Local Context::GetEmbedderData(int index) { } void* Context::GetAlignedPointerFromEmbedderData(int index) { -#ifndef V8_ENABLE_CHECKS - using A = internal::Address; using I = internal::Internals; + static_assert(I::kEmbedderDataSlotSize == internal::kApiSystemPointerSize, + "Enable fast path with sandboxed external pointers enabled " + "once embedder data slots are 32 bits large"); +#if !defined(V8_ENABLE_CHECKS) && !defined(V8_SANDBOXED_EXTERNAL_POINTERS) + using A = internal::Address; A ctx = *reinterpret_cast(this); A embedder_data = I::ReadTaggedPointerField(ctx, I::kNativeContextEmbedderDataOffset); int value_offset = I::kEmbedderDataArrayHeaderSize + (I::kEmbedderDataSlotSize * index); -#ifdef V8_SANDBOXED_EXTERNAL_POINTERS - value_offset += I::kEmbedderDataSlotRawPayloadOffset; -#endif internal::Isolate* isolate = I::GetIsolateForSandbox(ctx); return reinterpret_cast( I::ReadExternalPointerField(isolate, embedder_data, value_offset, diff --git a/include/v8-internal.h b/include/v8-internal.h index 4d3840454d..abfafb49f6 100644 --- a/include/v8-internal.h +++ b/include/v8-internal.h @@ -256,6 +256,26 @@ constexpr bool kAllowBackingStoresOutsideSandbox = false; constexpr bool kAllowBackingStoresOutsideSandbox = true; #endif // V8_SANDBOXED_POINTERS +// The size of the virtual memory reservation for an external pointer table. +// This determines the maximum number of entries in a table. Using a maximum +// size allows omitting bounds checks on table accesses if the indices are +// guaranteed (e.g. through shifting) to be below the maximum index. This +// value must be a power of two. +static const size_t kExternalPointerTableReservationSize = 128 * MB; + +// The maximum number of entries in an external pointer table. +static const size_t kMaxSandboxedExternalPointers = + kExternalPointerTableReservationSize / kApiSystemPointerSize; + +// The external pointer table indices stored in HeapObjects as external +// pointers are shifted to the left by this amount to guarantee that they are +// smaller than the maximum table size. +static const uint32_t kExternalPointerIndexShift = 8; +static_assert((1 << (32 - kExternalPointerIndexShift)) == + kMaxSandboxedExternalPointers, + "kExternalPointerTableReservationSize and " + "kExternalPointerIndexShift don't match"); + #endif // V8_SANDBOX_IS_AVAILABLE // If sandboxed external pointers are enabled, these tag values will be ORed @@ -288,17 +308,6 @@ enum ExternalPointerTag : uint64_t { constexpr uint64_t kExternalPointerTagMask = 0xffff000000000000; -// The size of the virtual memory reservation for an external pointer table. -// This determines the maximum number of entries in a table. Using a maximum -// size allows omitting bounds checks on table accesses if the indices are -// guaranteed (e.g. through shifting) to be below the maximum index. This -// value must be a power of two. -static const size_t kExternalPointerTableReservationSize = 128 * MB; - -// The maximum number of entries in an external pointer table. -static const size_t kMaxSandboxedExternalPointers = - kExternalPointerTableReservationSize / kApiSystemPointerSize; - // Converts encoded external pointer to address. V8_EXPORT Address DecodeExternalPointerImpl(const Isolate* isolate, ExternalPointer_t pointer, diff --git a/include/v8-object.h b/include/v8-object.h index b07042e490..4a38350258 100644 --- a/include/v8-object.h +++ b/include/v8-object.h @@ -735,18 +735,18 @@ Local Object::GetInternalField(int index) { } void* Object::GetAlignedPointerFromInternalField(int index) { -#ifndef V8_ENABLE_CHECKS - using A = internal::Address; using I = internal::Internals; + static_assert(I::kEmbedderDataSlotSize == internal::kApiSystemPointerSize, + "Enable fast path with sandboxed external pointers enabled " + "once embedder data slots are 32 bits large"); +#if !defined(V8_ENABLE_CHECKS) && !defined(V8_SANDBOXED_EXTERNAL_POINTERS) + using A = internal::Address; A obj = *reinterpret_cast(this); // Fast path: If the object is a plain JSObject, which is the common case, we // know where to find the internal fields and can return the value directly. auto instance_type = I::GetInstanceType(obj); if (v8::internal::CanHaveInternalField(instance_type)) { int offset = I::kJSObjectHeaderSize + (I::kEmbedderDataSlotSize * index); -#ifdef V8_SANDBOXED_EXTERNAL_POINTERS - offset += I::kEmbedderDataSlotRawPayloadOffset; -#endif internal::Isolate* isolate = I::GetIsolateForSandbox(obj); A value = I::ReadExternalPointerField( isolate, obj, offset, internal::kEmbedderDataSlotPayloadTag); diff --git a/src/codegen/arm64/macro-assembler-arm64.cc b/src/codegen/arm64/macro-assembler-arm64.cc index 2cfb5730c5..ee13836872 100644 --- a/src/codegen/arm64/macro-assembler-arm64.cc +++ b/src/codegen/arm64/macro-assembler-arm64.cc @@ -3109,6 +3109,7 @@ void TurboAssembler::LoadExternalPointerField(Register destination, DCHECK(!AreAliased(destination, isolate_root)); ASM_CODE_COMMENT(this); #ifdef V8_SANDBOXED_EXTERNAL_POINTERS + DCHECK_NE(kExternalPointerNullTag, tag); UseScratchRegisterScope temps(this); Register external_table = temps.AcquireX(); if (isolate_root == no_reg) { @@ -3120,11 +3121,13 @@ void TurboAssembler::LoadExternalPointerField(Register destination, IsolateData::external_pointer_table_offset() + Internals::kExternalPointerTableBufferOffset)); Ldr(destination.W(), field_operand); - Ldr(destination, - MemOperand(external_table, destination, LSL, kSystemPointerSizeLog2)); - if (tag != 0) { - And(destination, destination, Immediate(~tag)); - } + // MemOperand doesn't support LSR currently (only LSL), so here we do the + // offset computation separately first. + STATIC_ASSERT(kExternalPointerIndexShift > kSystemPointerSizeLog2); + int shift_amount = kExternalPointerIndexShift - kSystemPointerSizeLog2; + Mov(destination, Operand(destination, LSR, shift_amount)); + Ldr(destination, MemOperand(external_table, destination)); + And(destination, destination, Immediate(~tag)); #else Ldr(destination, field_operand); #endif // V8_SANDBOXED_EXTERNAL_POINTERS diff --git a/src/codegen/code-stub-assembler.cc b/src/codegen/code-stub-assembler.cc index b6c2357689..6d0324d715 100644 --- a/src/codegen/code-stub-assembler.cc +++ b/src/codegen/code-stub-assembler.cc @@ -1584,17 +1584,22 @@ TNode CodeStubAssembler::EmptyBackingStoreBufferConstant() { #endif // V8_SANDBOXED_POINTERS } -TNode CodeStubAssembler::ChangeUint32ToExternalPointer( - TNode value) { +#ifdef V8_SANDBOXED_EXTERNAL_POINTERS +TNode CodeStubAssembler::ChangeIndexToExternalPointer( + TNode index) { DCHECK_EQ(kExternalPointerSize, kUInt32Size); - return ReinterpretCast(value); + TNode shifted_index = + Word32Shl(index, Uint32Constant(kExternalPointerIndexShift)); + return ReinterpretCast(shifted_index); } -TNode CodeStubAssembler::ChangeExternalPointerToUint32( - TNode value) { +TNode CodeStubAssembler::ChangeExternalPointerToIndex( + TNode external_pointer) { DCHECK_EQ(kExternalPointerSize, kUInt32Size); - return ReinterpretCast(value); + TNode shifted_index = ReinterpretCast(external_pointer); + return Word32Shr(shifted_index, Uint32Constant(kExternalPointerIndexShift)); } +#endif // V8_SANDBOXED_EXTERNAL_POINTERS void CodeStubAssembler::InitializeExternalPointerField(TNode object, TNode offset) { @@ -1656,9 +1661,8 @@ void CodeStubAssembler::InitializeExternalPointerField(TNode object, // real value). TODO(saelo) initialize the entry with zero here and switch // callers to a version that initializes the entry with a given pointer. - TNode encoded = - ChangeUint32ToExternalPointer(index.value()); - StoreObjectFieldNoWriteBarrier(object, offset, encoded); + TNode pointer = ChangeIndexToExternalPointer(index.value()); + StoreObjectFieldNoWriteBarrier(object, offset, pointer); #endif } @@ -1674,7 +1678,9 @@ TNode CodeStubAssembler::LoadExternalPointerFromObject( TNode encoded = LoadObjectField(object, offset); - TNode index = ChangeExternalPointerToUint32(encoded); + TNode index = ChangeExternalPointerToIndex(encoded); + // TODO(v8:10391): consider updating ElementOffsetFromIndex to generate code + // that does one shift right instead of two shifts (right and then left). TNode table_offset = ElementOffsetFromIndex( ChangeUint32ToWord(index), SYSTEM_POINTER_ELEMENTS, 0); @@ -1701,8 +1707,9 @@ void CodeStubAssembler::StoreExternalPointerToObject( TNode encoded = LoadObjectField(object, offset); - TNode index = ChangeExternalPointerToUint32(encoded); - // TODO(v8:10391, saelo): bounds check if table is not caged + TNode index = ChangeExternalPointerToIndex(encoded); + // TODO(v8:10391): consider updating ElementOffsetFromIndex to generate code + // that does one shift right instead of two shifts (right and then left). TNode table_offset = ElementOffsetFromIndex( ChangeUint32ToWord(index), SYSTEM_POINTER_ELEMENTS, 0); diff --git a/src/codegen/code-stub-assembler.h b/src/codegen/code-stub-assembler.h index 986495cba4..bde0f836a5 100644 --- a/src/codegen/code-stub-assembler.h +++ b/src/codegen/code-stub-assembler.h @@ -1093,8 +1093,10 @@ class V8_EXPORT_PRIVATE CodeStubAssembler // ExternalPointerT-related functionality. // - TNode ChangeUint32ToExternalPointer(TNode value); - TNode ChangeExternalPointerToUint32(TNode value); +#ifdef V8_SANDBOXED_EXTERNAL_POINTERS + TNode ChangeIndexToExternalPointer(TNode index); + TNode ChangeExternalPointerToIndex(TNode pointer); +#endif // V8_SANDBOXED_EXTERNAL_POINTERS // Initialize an external pointer field in an object. void InitializeExternalPointerField(TNode object, int offset) { diff --git a/src/codegen/x64/macro-assembler-x64.cc b/src/codegen/x64/macro-assembler-x64.cc index f6a31b3379..68fc024dd3 100644 --- a/src/codegen/x64/macro-assembler-x64.cc +++ b/src/codegen/x64/macro-assembler-x64.cc @@ -418,6 +418,7 @@ void TurboAssembler::LoadExternalPointerField( Register scratch, IsolateRootLocation isolateRootLocation) { DCHECK(!AreAliased(destination, scratch)); #ifdef V8_SANDBOXED_EXTERNAL_POINTERS + DCHECK_NE(kExternalPointerNullTag, tag); DCHECK(!field_operand.AddressUsesRegister(scratch)); if (isolateRootLocation == IsolateRootLocation::kInRootRegister) { DCHECK(root_array_available_); @@ -431,11 +432,10 @@ void TurboAssembler::LoadExternalPointerField( Internals::kExternalPointerTableBufferOffset)); } movl(destination, field_operand); + shrq(destination, Immediate(kExternalPointerIndexShift)); movq(destination, Operand(scratch, destination, times_8, 0)); - if (tag != 0) { - movq(scratch, Immediate64(~tag)); - andq(destination, scratch); - } + movq(scratch, Immediate64(~tag)); + andq(destination, scratch); #else movq(destination, field_operand); #endif // V8_SANDBOXED_EXTERNAL_POINTERS diff --git a/src/compiler/memory-lowering.cc b/src/compiler/memory-lowering.cc index 99e4515c5a..6b5eb17b33 100644 --- a/src/compiler/memory-lowering.cc +++ b/src/compiler/memory-lowering.cc @@ -412,6 +412,7 @@ Node* MemoryLowering::DecodeExternalPointer( DCHECK(V8_SANDBOXED_EXTERNAL_POINTERS_BOOL); DCHECK(node->opcode() == IrOpcode::kLoad); DCHECK_EQ(kExternalPointerSize, kUInt32Size); + DCHECK_NE(kExternalPointerNullTag, external_pointer_tag); Node* effect = NodeProperties::GetEffectInput(node); Node* control = NodeProperties::GetControlInput(node); __ InitializeEffectControl(effect, control); @@ -419,7 +420,11 @@ Node* MemoryLowering::DecodeExternalPointer( // Clone the load node and put it here. // TODO(turbofan): consider adding GraphAssembler::Clone() suitable for // cloning nodes from arbitrary locaions in effect/control chains. - Node* index = __ AddNode(graph()->CloneNode(node)); + STATIC_ASSERT(kExternalPointerIndexShift > kSystemPointerSizeLog2); + Node* shifted_index = __ AddNode(graph()->CloneNode(node)); + Node* shift_amount = + __ Int32Constant(kExternalPointerIndexShift - kSystemPointerSizeLog2); + Node* offset = __ Word32Shr(shifted_index, shift_amount); // Uncomment this to generate a breakpoint for debugging purposes. // __ DebugBreak(); @@ -436,13 +441,10 @@ Node* MemoryLowering::DecodeExternalPointer( ExternalReference::external_pointer_table_address(isolate())); Node* table = __ Load(MachineType::Pointer(), table_address, Internals::kExternalPointerTableBufferOffset); - Node* offset = __ Int32Mul(index, __ Int32Constant(sizeof(Address))); Node* decoded_ptr = __ Load(MachineType::Pointer(), table, __ ChangeUint32ToUint64(offset)); - if (external_pointer_tag != 0) { - Node* tag = __ IntPtrConstant(~external_pointer_tag); - decoded_ptr = __ WordAnd(decoded_ptr, tag); - } + Node* tag = __ IntPtrConstant(~external_pointer_tag); + decoded_ptr = __ WordAnd(decoded_ptr, tag); return decoded_ptr; #else return node; diff --git a/src/compiler/wasm-compiler.cc b/src/compiler/wasm-compiler.cc index 69a35eb7ee..9991b8b062 100644 --- a/src/compiler/wasm-compiler.cc +++ b/src/compiler/wasm-compiler.cc @@ -3224,13 +3224,15 @@ Node* WasmGraphBuilder::BuildLoadExternalPointerFromObject( #ifdef V8_SANDBOXED_EXTERNAL_POINTERS Node* external_pointer = gasm_->LoadFromObject( MachineType::Uint32(), object, wasm::ObjectAccess::ToTagged(offset)); + STATIC_ASSERT(kExternalPointerIndexShift > kSystemPointerSizeLog2); + Node* shift_amount = + gasm_->Int32Constant(kExternalPointerIndexShift - kSystemPointerSizeLog2); + Node* scaled_index = gasm_->Word32Shr(external_pointer, shift_amount); Node* isolate_root = BuildLoadIsolateRoot(); Node* table = gasm_->LoadFromObject(MachineType::Pointer(), isolate_root, IsolateData::external_pointer_table_offset() + Internals::kExternalPointerTableBufferOffset); - Node* scaled_index = gasm_->Int32Mul( - external_pointer, gasm_->Int32Constant(kSystemPointerSize)); Node* decoded_ptr = gasm_->Load(MachineType::Pointer(), table, scaled_index); return gasm_->WordAnd(decoded_ptr, gasm_->IntPtrConstant(~tag)); #else diff --git a/src/heap/marking-visitor.h b/src/heap/marking-visitor.h index dfb0a0d613..dd02b19448 100644 --- a/src/heap/marking-visitor.h +++ b/src/heap/marking-visitor.h @@ -198,7 +198,8 @@ class MarkingVisitorBase : public HeapVisitor { V8_INLINE void VisitExternalPointer(HeapObject host, ExternalPointer_t ptr) final { #ifdef V8_SANDBOXED_EXTERNAL_POINTERS - external_pointer_table_->Mark(static_cast(ptr)); + uint32_t index = ptr >> kExternalPointerIndexShift; + external_pointer_table_->Mark(index); #endif // V8_SANDBOXED_EXTERNAL_POINTERS } diff --git a/src/logging/counters-definitions.h b/src/logging/counters-definitions.h index 29212a8428..a3f7ea1308 100644 --- a/src/logging/counters-definitions.h +++ b/src/logging/counters-definitions.h @@ -111,10 +111,17 @@ namespace internal { 3) \ /* number of times a cache event is triggered for a wasm module */ \ HR(wasm_cache_count, V8.WasmCacheCount, 0, 100, 101) \ - /* Number of in-use external pointers in the external pointer table */ \ - /* Counted after sweeping the table at the end of mark-compact GC */ \ - HR(sandboxed_external_pointers_count, V8.SandboxedExternalPointersCount, 0, \ + SANDBOXED_HISTOGRAM_LIST(HR) + +#ifdef V8_SANDBOX_IS_AVAILABLE +#define SANDBOXED_HISTOGRAM_LIST(HR) \ + /* Number of in-use external pointers in the external pointer table */ \ + /* Counted after sweeping the table at the end of mark-compact GC */ \ + HR(sandboxed_external_pointers_count, V8.SandboxedExternalPointersCount, 0, \ kMaxSandboxedExternalPointers, 101) +#else +#define SANDBOXED_HISTOGRAM_LIST(HR) +#endif // V8_SANDBOX_IS_AVAILABLE #define NESTED_TIMED_HISTOGRAM_LIST(HT) \ /* Timer histograms, not thread safe: HT(name, caption, max, unit) */ \ diff --git a/src/sandbox/external-pointer-inl.h b/src/sandbox/external-pointer-inl.h index f3842542a2..61a6cb1a4b 100644 --- a/src/sandbox/external-pointer-inl.h +++ b/src/sandbox/external-pointer-inl.h @@ -18,7 +18,8 @@ V8_INLINE Address DecodeExternalPointer(const Isolate* isolate, ExternalPointerTag tag) { #ifdef V8_SANDBOXED_EXTERNAL_POINTERS STATIC_ASSERT(kExternalPointerSize == kInt32Size); - return isolate->external_pointer_table().Get(encoded_pointer, tag); + uint32_t index = encoded_pointer >> kExternalPointerIndexShift; + return isolate->external_pointer_table().Get(index, tag); #else STATIC_ASSERT(kExternalPointerSize == kSystemPointerSize); return encoded_pointer; @@ -35,6 +36,7 @@ V8_INLINE void InitExternalPointerField(Address field_address, Isolate* isolate, #ifdef V8_SANDBOXED_EXTERNAL_POINTERS ExternalPointer_t index = isolate->external_pointer_table().Allocate(); isolate->external_pointer_table().Set(index, value, tag); + index <<= kExternalPointerIndexShift; base::Memory(field_address) = index; #else // Pointer compression causes types larger than kTaggedSize to be unaligned. @@ -72,6 +74,7 @@ V8_INLINE void WriteExternalPointerField(Address field_address, ExternalPointerTag tag) { #ifdef V8_SANDBOXED_EXTERNAL_POINTERS ExternalPointer_t index = base::Memory(field_address); + index >>= kExternalPointerIndexShift; isolate->external_pointer_table().Set(index, value, tag); #else // Pointer compression causes types larger than kTaggedSize to be unaligned.