From 4d1f17e4d6b01b7877abf89aa13b10f501ecd087 Mon Sep 17 00:00:00 2001 From: Leszek Swirski Date: Wed, 6 May 2020 17:45:50 +0200 Subject: [PATCH] [sandbox] Access ExternalString ResourceData via bottlenecks Bug: v8:10391 Change-Id: I4e86394c53d02eab797c2daad2ccfde6acb83bf0 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2151350 Commit-Queue: Leszek Swirski Reviewed-by: Jakob Gruber Reviewed-by: Igor Sheludko Reviewed-by: Nico Hartmann Cr-Commit-Position: refs/heads/master@{#67619} --- src/builtins/builtins-string-gen.cc | 16 +++++------ src/builtins/builtins-string-gen.h | 2 +- src/codegen/code-stub-assembler.cc | 15 +++++----- src/compiler/access-builder.cc | 3 +- src/objects/string-inl.h | 32 ++++++++++++--------- src/objects/string.h | 6 ++-- src/objects/string.tq | 2 +- src/snapshot/serializer.cc | 2 +- tools/debug_helper/get-object-properties.cc | 14 +++++++-- 9 files changed, 53 insertions(+), 39 deletions(-) diff --git a/src/builtins/builtins-string-gen.cc b/src/builtins/builtins-string-gen.cc index e2d1635274..689ced18f5 100644 --- a/src/builtins/builtins-string-gen.cc +++ b/src/builtins/builtins-string-gen.cc @@ -20,10 +20,10 @@ namespace internal { using Node = compiler::Node; -TNode StringBuiltinsAssembler::DirectStringData( +TNode StringBuiltinsAssembler::DirectStringData( TNode string, TNode string_instance_type) { // Compute the effective offset of the first character. - TVARIABLE(IntPtrT, var_data); + TVARIABLE(RawPtrT, var_data); Label if_sequential(this), if_external(this), if_join(this); Branch(Word32Equal(Word32And(string_instance_type, Int32Constant(kStringRepresentationMask)), @@ -32,9 +32,9 @@ TNode StringBuiltinsAssembler::DirectStringData( BIND(&if_sequential); { - var_data = IntPtrAdd( - IntPtrConstant(SeqOneByteString::kHeaderSize - kHeapObjectTag), - BitcastTaggedToWord(string)); + var_data = RawPtrAdd( + ReinterpretCast(BitcastTaggedToWord(string)), + IntPtrConstant(SeqOneByteString::kHeaderSize - kHeapObjectTag)); Goto(&if_join); } @@ -47,7 +47,7 @@ TNode StringBuiltinsAssembler::DirectStringData( Int32Constant(kUncachedExternalStringMask)), Int32Constant(kUncachedExternalStringTag))); var_data = - LoadObjectField(string, ExternalString::kResourceDataOffset); + DecodeExternalPointer(LoadExternalStringResourceData(CAST(string))); Goto(&if_join); } @@ -254,8 +254,8 @@ void StringBuiltinsAssembler::StringEqual_Loop( CSA_ASSERT(this, WordEqual(LoadStringLengthAsWord(rhs), length)); // Compute the effective offset of the first character. - TNode lhs_data = DirectStringData(lhs, lhs_instance_type); - TNode rhs_data = DirectStringData(rhs, rhs_instance_type); + TNode lhs_data = DirectStringData(lhs, lhs_instance_type); + TNode rhs_data = DirectStringData(rhs, rhs_instance_type); // Loop over the {lhs} and {rhs} strings to see if they are equal. TVARIABLE(IntPtrT, var_offset, IntPtrConstant(0)); diff --git a/src/builtins/builtins-string-gen.h b/src/builtins/builtins-string-gen.h index 93b2086dd7..2b4dadbbb0 100644 --- a/src/builtins/builtins-string-gen.h +++ b/src/builtins/builtins-string-gen.h @@ -67,7 +67,7 @@ class StringBuiltinsAssembler : public CodeStubAssembler { TNode rhs_instance_type, MachineType rhs_type, TNode length, Label* if_equal, Label* if_not_equal); - TNode DirectStringData(TNode string, + TNode DirectStringData(TNode string, TNode string_instance_type); void DispatchOnStringEncodings(const TNode lhs_instance_type, diff --git a/src/codegen/code-stub-assembler.cc b/src/codegen/code-stub-assembler.cc index 6426a7d131..6713a04bfe 100644 --- a/src/codegen/code-stub-assembler.cc +++ b/src/codegen/code-stub-assembler.cc @@ -6881,12 +6881,13 @@ TNode ToDirectStringAssembler::TryToSequential( { STATIC_ASSERT(SeqOneByteString::kHeaderSize == SeqTwoByteString::kHeaderSize); - TNode result = BitcastTaggedToWord(var_string_.value()); + TNode result = + ReinterpretCast(BitcastTaggedToWord(var_string_.value())); if (ptr_kind == PTR_TO_DATA) { - result = IntPtrAdd(result, IntPtrConstant(SeqOneByteString::kHeaderSize - + result = RawPtrAdd(result, IntPtrConstant(SeqOneByteString::kHeaderSize - kHeapObjectTag)); } - var_result = ReinterpretCast(result); + var_result = result; Goto(&out); } @@ -6896,13 +6897,13 @@ TNode ToDirectStringAssembler::TryToSequential( if_bailout); TNode string = var_string_.value(); - TNode result = - LoadObjectField(string, ExternalString::kResourceDataOffset); + TNode result = + DecodeExternalPointer(LoadExternalStringResourceData(CAST(string))); if (ptr_kind == PTR_TO_STRING) { - result = IntPtrSub(result, IntPtrConstant(SeqOneByteString::kHeaderSize - + result = RawPtrSub(result, IntPtrConstant(SeqOneByteString::kHeaderSize - kHeapObjectTag)); } - var_result = ReinterpretCast(result); + var_result = result; Goto(&out); } diff --git a/src/compiler/access-builder.cc b/src/compiler/access-builder.cc index 5b7acc1b1b..e5b146a918 100644 --- a/src/compiler/access-builder.cc +++ b/src/compiler/access-builder.cc @@ -691,7 +691,8 @@ FieldAccess AccessBuilder::ForExternalStringResourceData() { ExternalString::kResourceDataOffset, Handle(), MaybeHandle(), - Type::ExternalPointer(), + V8_HEAP_SANDBOX_BOOL ? Type::SandboxedExternalPointer() + : Type::ExternalPointer(), MachineType::Pointer(), kNoWriteBarrier}; return access; diff --git a/src/objects/string-inl.h b/src/objects/string-inl.h index 85d4a86d4d..78f859b78c 100644 --- a/src/objects/string-inl.h +++ b/src/objects/string-inl.h @@ -5,9 +5,9 @@ #ifndef V8_OBJECTS_STRING_INL_H_ #define V8_OBJECTS_STRING_INL_H_ +#include "src/common/external-pointer-inl.h" #include "src/common/external-pointer.h" -#include "src/objects/string.h" - +#include "src/common/globals.h" #include "src/handles/handles-inl.h" #include "src/heap/factory.h" #include "src/numbers/conversions-inl.h" @@ -15,6 +15,7 @@ #include "src/objects/name-inl.h" #include "src/objects/smi-inl.h" #include "src/objects/string-table-inl.h" +#include "src/objects/string.h" #include "src/strings/string-hasher-inl.h" // Has to be the last include (doesn't have include guards): @@ -613,9 +614,9 @@ void ExternalString::set_address_as_resource(Isolate* isolate, EncodeExternalPointer(isolate, address); WriteField(kResourceOffset, encoded_address); if (IsExternalOneByteString()) { - ExternalOneByteString::cast(*this).update_data_cache(); + ExternalOneByteString::cast(*this).update_data_cache(isolate); } else { - ExternalTwoByteString::cast(*this).update_data_cache(); + ExternalTwoByteString::cast(*this).update_data_cache(isolate); } } @@ -625,10 +626,11 @@ uint32_t ExternalString::resource_as_uint32() { return static_cast(encoded_address); } -void ExternalString::set_uint32_as_resource(uint32_t value) { +void ExternalString::set_uint32_as_resource(Isolate* isolate, uint32_t value) { WriteField(kResourceOffset, value); if (is_uncached()) return; - WriteField
(kResourceDataOffset, kNullAddress); + WriteField(kResourceDataOffset, + EncodeExternalPointer(isolate, kNullAddress)); } void ExternalString::DisposeResource(Isolate* isolate) { @@ -655,10 +657,11 @@ DEF_GETTER(ExternalOneByteString, resource, DecodeExternalPointer(isolate, encoded_address)); } -void ExternalOneByteString::update_data_cache() { +void ExternalOneByteString::update_data_cache(Isolate* isolate) { if (is_uncached()) return; - WriteField
(kResourceDataOffset, - reinterpret_cast
(resource()->data())); + const ExternalPointer_t encoded_resource_data = EncodeExternalPointer( + isolate, reinterpret_cast
(resource()->data())); + WriteField(kResourceDataOffset, encoded_resource_data); } void ExternalOneByteString::SetResource( @@ -675,7 +678,7 @@ void ExternalOneByteString::set_resource( const ExternalPointer_t encoded_address = EncodeExternalPointer(isolate, reinterpret_cast
(resource)); WriteField(kResourceOffset, encoded_address); - if (resource != nullptr) update_data_cache(); + if (resource != nullptr) update_data_cache(isolate); } const uint8_t* ExternalOneByteString::GetChars() { @@ -695,10 +698,11 @@ DEF_GETTER(ExternalTwoByteString, resource, DecodeExternalPointer(isolate, encoded_address)); } -void ExternalTwoByteString::update_data_cache() { +void ExternalTwoByteString::update_data_cache(Isolate* isolate) { if (is_uncached()) return; - WriteField
(kResourceDataOffset, - reinterpret_cast
(resource()->data())); + const ExternalPointer_t encoded_resource_data = EncodeExternalPointer( + isolate, reinterpret_cast
(resource()->data())); + WriteField(kResourceDataOffset, encoded_resource_data); } void ExternalTwoByteString::SetResource( @@ -715,7 +719,7 @@ void ExternalTwoByteString::set_resource( const ExternalPointer_t encoded_address = EncodeExternalPointer(isolate, reinterpret_cast
(resource)); WriteField(kResourceOffset, encoded_address); - if (resource != nullptr) update_data_cache(); + if (resource != nullptr) update_data_cache(isolate); } const uint16_t* ExternalTwoByteString::GetChars() { return resource()->data(); } diff --git a/src/objects/string.h b/src/objects/string.h index ae2e21ef6a..2163e11d1f 100644 --- a/src/objects/string.h +++ b/src/objects/string.h @@ -719,7 +719,7 @@ class ExternalString : public String { DECL_GETTER(resource_as_address, Address) inline void set_address_as_resource(Isolate* isolate, Address address); inline uint32_t resource_as_uint32(); - inline void set_uint32_as_resource(uint32_t value); + inline void set_uint32_as_resource(Isolate* isolate, uint32_t value); // Disposes string's resource object if it has not already been disposed. inline void DisposeResource(Isolate* isolate); @@ -751,7 +751,7 @@ class ExternalOneByteString : public ExternalString { // The cached pointer is always valid, as the external character array does = // not move during lifetime. Deserialization is the only exception, after // which the pointer cache has to be refreshed. - inline void update_data_cache(); + inline void update_data_cache(Isolate* isolate); inline const uint8_t* GetChars(); @@ -792,7 +792,7 @@ class ExternalTwoByteString : public ExternalString { // The cached pointer is always valid, as the external character array does = // not move during lifetime. Deserialization is the only exception, after // which the pointer cache has to be refreshed. - inline void update_data_cache(); + inline void update_data_cache(Isolate* isolate); inline const uint16_t* GetChars(); diff --git a/src/objects/string.tq b/src/objects/string.tq index 6b5dcc6201..7d3f250964 100644 --- a/src/objects/string.tq +++ b/src/objects/string.tq @@ -20,7 +20,7 @@ extern class ConsString extends String { @generateBodyDescriptor extern class ExternalString extends String { resource: ExternalPointer; - resource_data: RawPtr; + resource_data: ExternalPointer; } extern class ExternalOneByteString extends ExternalString {} diff --git a/src/snapshot/serializer.cc b/src/snapshot/serializer.cc index 88bab91299..82f80b82d8 100644 --- a/src/snapshot/serializer.cc +++ b/src/snapshot/serializer.cc @@ -413,7 +413,7 @@ void Serializer::ObjectSerializer::SerializeExternalString() { if (serializer_->external_reference_encoder_.TryEncode(resource).To( &reference)) { DCHECK(reference.is_from_api()); - string.set_uint32_as_resource(reference.index()); + string.set_uint32_as_resource(serializer_->isolate(), reference.index()); SerializeObject(); string.set_address_as_resource(serializer_->isolate(), resource); } else { diff --git a/tools/debug_helper/get-object-properties.cc b/tools/debug_helper/get-object-properties.cc index 9146dd4633..0e8fbf02a6 100644 --- a/tools/debug_helper/get-object-properties.cc +++ b/tools/debug_helper/get-object-properties.cc @@ -7,7 +7,8 @@ #include "debug-helper-internal.h" #include "heap-constants.h" #include "include/v8-internal.h" -#include "src/common/ptr-compr-inl.h" +#include "src/common/external-pointer.h" +#include "src/execution/isolate-utils.h" #include "src/objects/string-inl.h" #include "src/strings/unicode-inl.h" #include "torque-generated/class-debug-readers-tq.h" @@ -323,8 +324,15 @@ class ReadStringVisitor : public TqObjectVisitor { // require knowledge of the embedder. For now, we only read cached external // strings. if (IsExternalStringCached(object)) { - uintptr_t data_address = reinterpret_cast( - GetOrFinish(object->GetResourceDataValue(accessor_))); + ExternalPointer_t resource_data = + GetOrFinish(object->GetResourceDataValue(accessor_)); +#ifdef V8_COMPRESS_POINTERS + uintptr_t data_address = static_cast(DecodeExternalPointer( + Isolate::FromRoot(GetIsolateRoot(heap_addresses_.any_heap_pointer)), + resource_data)); +#else + uintptr_t data_address = reinterpret_cast(resource_data); +#endif // V8_COMPRESS_POINTERS if (done_) return; ReadStringCharacters(object, data_address); } else {