From 6cb0a014de47e897e1e83ba15aae2096a76be054 Mon Sep 17 00:00:00 2001 From: Hai Dang Date: Mon, 1 Oct 2018 12:57:19 +0200 Subject: [PATCH] Add string iterator protector. The protector is useful for follow-up optimizations on string iterator. Tests are also added. Change-Id: I416037c742628c4d4d3b878d0df727a9ae7162f7 Reviewed-on: https://chromium-review.googlesource.com/1251122 Reviewed-by: Ulan Degenbaev Reviewed-by: Sigurd Schneider Reviewed-by: Benedikt Meurer Reviewed-by: Georg Neis Commit-Queue: Hai Dang Cr-Commit-Position: refs/heads/master@{#56315} --- src/bootstrapper.cc | 4 +++- src/builtins/builtins-string-gen.cc | 4 ++-- src/compiler/js-create-lowering.cc | 3 ++- src/compiler/js-heap-broker.h | 2 +- src/contexts.h | 4 +++- src/heap/factory.cc | 2 +- src/heap/setup-heap-internal.cc | 4 ++++ src/isolate-inl.h | 5 +++++ src/isolate.cc | 9 +++++++++ src/isolate.h | 13 ++++++++++++ src/lookup.cc | 23 +++++++++++++++++---- src/roots.h | 1 + src/runtime/runtime-test.cc | 7 +++++++ src/runtime/runtime.h | 1 + test/mjsunit/es6/string-iterator2.js | 24 ++++++++++++++++++++++ test/mjsunit/es6/string-iterator3.js | 20 +++++++++++++++++++ test/mjsunit/es6/string-iterator4.js | 30 ++++++++++++++++++++++++++++ test/mjsunit/es6/string-iterator5.js | 15 ++++++++++++++ test/mjsunit/es6/string-iterator6.js | 11 ++++++++++ test/mjsunit/es6/string-iterator7.js | 13 ++++++++++++ test/mjsunit/es6/string-iterator8.js | 14 +++++++++++++ 21 files changed, 198 insertions(+), 11 deletions(-) create mode 100644 test/mjsunit/es6/string-iterator2.js create mode 100644 test/mjsunit/es6/string-iterator3.js create mode 100644 test/mjsunit/es6/string-iterator4.js create mode 100644 test/mjsunit/es6/string-iterator5.js create mode 100644 test/mjsunit/es6/string-iterator6.js create mode 100644 test/mjsunit/es6/string-iterator7.js create mode 100644 test/mjsunit/es6/string-iterator8.js diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index 74676a3ecd..f9b22d96c4 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -2141,8 +2141,10 @@ void Genesis::InitializeGlobal(Handle global_object, JS_STRING_ITERATOR_TYPE, JSStringIterator::kSize, 0, string_iterator_prototype, Builtins::kIllegal); string_iterator_function->shared()->set_native(false); - native_context()->set_string_iterator_map( + native_context()->set_initial_string_iterator_map( string_iterator_function->initial_map()); + native_context()->set_initial_string_iterator_prototype( + *string_iterator_prototype); } { // --- S y m b o l --- diff --git a/src/builtins/builtins-string-gen.cc b/src/builtins/builtins-string-gen.cc index 1ee7a4f760..a6e51d7483 100644 --- a/src/builtins/builtins-string-gen.cc +++ b/src/builtins/builtins-string-gen.cc @@ -2359,8 +2359,8 @@ TF_BUILTIN(StringPrototypeIterator, CodeStubAssembler) { ToThisString(context, receiver, "String.prototype[Symbol.iterator]"); Node* native_context = LoadNativeContext(context); - Node* map = - LoadContextElement(native_context, Context::STRING_ITERATOR_MAP_INDEX); + Node* map = LoadContextElement(native_context, + Context::INITIAL_STRING_ITERATOR_MAP_INDEX); Node* iterator = Allocate(JSStringIterator::kSize); StoreMapNoWriteBarrier(iterator, map); StoreObjectFieldRoot(iterator, JSValue::kPropertiesOrHashOffset, diff --git a/src/compiler/js-create-lowering.cc b/src/compiler/js-create-lowering.cc index c5801e4b5c..e8abae7c14 100644 --- a/src/compiler/js-create-lowering.cc +++ b/src/compiler/js-create-lowering.cc @@ -977,7 +977,8 @@ Reduction JSCreateLowering::ReduceJSCreateStringIterator(Node* node) { Node* string = NodeProperties::GetValueInput(node, 0); Node* effect = NodeProperties::GetEffectInput(node); - Node* map = jsgraph()->Constant(native_context().string_iterator_map()); + Node* map = + jsgraph()->Constant(native_context().initial_string_iterator_map()); // Allocate new iterator and attach the iterator to this string. AllocationBuilder a(jsgraph(), effect, graph()->start()); a.Allocate(JSStringIterator::kSize, NOT_TENURED, Type::OtherObject()); diff --git a/src/compiler/js-heap-broker.h b/src/compiler/js-heap-broker.h index 6ecab316cf..88c47b3c45 100644 --- a/src/compiler/js-heap-broker.h +++ b/src/compiler/js-heap-broker.h @@ -257,7 +257,7 @@ class ContextRef : public HeapObjectRef { V(Map, sloppy_arguments_map) \ V(Map, slow_object_with_null_prototype_map) \ V(Map, strict_arguments_map) \ - V(Map, string_iterator_map) \ + V(Map, initial_string_iterator_map) \ V(ScriptContextTable, script_context_table) class NativeContextRef : public ContextRef { diff --git a/src/contexts.h b/src/contexts.h index 071fda12c5..f33c8b16bc 100644 --- a/src/contexts.h +++ b/src/contexts.h @@ -194,6 +194,9 @@ enum ContextLookupFlags { V(INITIAL_MAP_PROTOTYPE_MAP_INDEX, Map, initial_map_prototype_map) \ V(INITIAL_OBJECT_PROTOTYPE_INDEX, JSObject, initial_object_prototype) \ V(INITIAL_SET_PROTOTYPE_MAP_INDEX, Map, initial_set_prototype_map) \ + V(INITIAL_STRING_ITERATOR_MAP_INDEX, Map, initial_string_iterator_map) \ + V(INITIAL_STRING_ITERATOR_PROTOTYPE_INDEX, JSObject, \ + initial_string_iterator_prototype) \ V(INITIAL_STRING_PROTOTYPE_INDEX, JSObject, initial_string_prototype) \ V(INITIAL_WEAKMAP_PROTOTYPE_MAP_INDEX, Map, initial_weakmap_prototype_map) \ V(INITIAL_WEAKSET_PROTOTYPE_MAP_INDEX, Map, initial_weakset_prototype_map) \ @@ -330,7 +333,6 @@ enum ContextLookupFlags { V(CLASS_FUNCTION_MAP_INDEX, Map, class_function_map) \ V(STRING_FUNCTION_INDEX, JSFunction, string_function) \ V(STRING_FUNCTION_PROTOTYPE_MAP_INDEX, Map, string_function_prototype_map) \ - V(STRING_ITERATOR_MAP_INDEX, Map, string_iterator_map) \ V(SYMBOL_FUNCTION_INDEX, JSFunction, symbol_function) \ V(NATIVE_FUNCTION_MAP_INDEX, Map, native_function_map) \ V(WASM_EXCEPTION_CONSTRUCTOR_INDEX, JSFunction, wasm_exception_constructor) \ diff --git a/src/heap/factory.cc b/src/heap/factory.cc index 1c773d6479..f4fc54beda 100644 --- a/src/heap/factory.cc +++ b/src/heap/factory.cc @@ -1308,7 +1308,7 @@ Handle Factory::NewNativeSourceString( } Handle Factory::NewJSStringIterator(Handle string) { - Handle map(isolate()->native_context()->string_iterator_map(), + Handle map(isolate()->native_context()->initial_string_iterator_map(), isolate()); Handle flat_string = String::Flatten(isolate(), string); Handle iterator = diff --git a/src/heap/setup-heap-internal.cc b/src/heap/setup-heap-internal.cc index a001d71315..09438c0e95 100644 --- a/src/heap/setup-heap-internal.cc +++ b/src/heap/setup-heap-internal.cc @@ -854,6 +854,10 @@ void Heap::CreateInitialObjects() { cell->set_value(Smi::FromInt(Isolate::kProtectorValid)); set_promise_species_protector(*cell); + cell = factory->NewPropertyCell(factory->empty_string()); + cell->set_value(Smi::FromInt(Isolate::kProtectorValid)); + set_string_iterator_protector(*cell); + Handle string_length_overflow_cell = factory->NewCell( handle(Smi::FromInt(Isolate::kProtectorValid), isolate())); set_string_length_protector(*string_length_overflow_cell); diff --git a/src/isolate-inl.h b/src/isolate-inl.h index c1b353b732..fe5157cabc 100644 --- a/src/isolate-inl.h +++ b/src/isolate-inl.h @@ -179,6 +179,11 @@ bool Isolate::IsArrayIteratorLookupChainIntact() { return array_iterator_cell->value() == Smi::FromInt(kProtectorValid); } +bool Isolate::IsStringIteratorLookupChainIntact() { + PropertyCell* string_iterator_cell = heap()->string_iterator_protector(); + return string_iterator_cell->value() == Smi::FromInt(kProtectorValid); +} + } // namespace internal } // namespace v8 diff --git a/src/isolate.cc b/src/isolate.cc index 407c7ffc92..7cb67f9bfd 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -3562,6 +3562,15 @@ void Isolate::InvalidateArrayIteratorProtector() { DCHECK(!IsArrayIteratorLookupChainIntact()); } +void Isolate::InvalidateStringIteratorProtector() { + DCHECK(factory()->string_iterator_protector()->value()->IsSmi()); + DCHECK(IsStringIteratorLookupChainIntact()); + PropertyCell::SetValueWithInvalidation( + this, factory()->string_iterator_protector(), + handle(Smi::FromInt(kProtectorInvalid), this)); + DCHECK(!IsStringIteratorLookupChainIntact()); +} + void Isolate::InvalidateArrayBufferNeuteringProtector() { DCHECK(factory()->array_buffer_neutering_protector()->value()->IsSmi()); DCHECK(IsArrayBufferNeuteringIntact()); diff --git a/src/isolate.h b/src/isolate.h index 4e3a374310..065c06607d 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -1227,6 +1227,18 @@ class Isolate : private HiddenFactory { inline bool IsStringLengthOverflowIntact(); inline bool IsArrayIteratorLookupChainIntact(); + // The StringIteratorProtector protects the original string iterating behavior + // for primitive strings. As long as the StringIteratorProtector is valid, + // iterating over a primitive string is guaranteed to be unobservable from + // user code and can thus be cut short. More specifically, the protector gets + // invalidated as soon as either String.prototype[Symbol.iterator] or + // String.prototype[Symbol.iterator]().next is modified. This guarantee does + // not apply to string objects (as opposed to primitives), since they could + // define their own Symbol.iterator. + // String.prototype itself does not need to be protected, since it is + // non-configurable and non-writable. + inline bool IsStringIteratorLookupChainIntact(); + // Make sure we do check for neutered array buffers. inline bool IsArrayBufferNeuteringIntact(); @@ -1267,6 +1279,7 @@ class Isolate : private HiddenFactory { void InvalidateIsConcatSpreadableProtector(); void InvalidateStringLengthOverflowProtector(); void InvalidateArrayIteratorProtector(); + void InvalidateStringIteratorProtector(); void InvalidateArrayBufferNeuteringProtector(); V8_EXPORT_PRIVATE void InvalidatePromiseHookProtector(); void InvalidatePromiseResolveProtector(); diff --git a/src/lookup.cc b/src/lookup.cc index 3cdf623bbe..c6cc06eeae 100644 --- a/src/lookup.cc +++ b/src/lookup.cc @@ -323,12 +323,19 @@ void LookupIterator::InternalUpdateProtector() { } } } else if (*name_ == roots.next_string()) { - if (!isolate_->IsArrayIteratorLookupChainIntact()) return; - // Setting the next property of %ArrayIteratorPrototype% also needs to - // invalidate the array iterator protector. if (isolate_->IsInAnyContext( *holder_, Context::INITIAL_ARRAY_ITERATOR_PROTOTYPE_INDEX)) { + // Setting the next property of %ArrayIteratorPrototype% also needs to + // invalidate the array iterator protector. + if (!isolate_->IsArrayIteratorLookupChainIntact()) return; isolate_->InvalidateArrayIteratorProtector(); + } else if (isolate_->IsInAnyContext( + *receiver_, + Context::INITIAL_STRING_ITERATOR_PROTOTYPE_INDEX)) { + // Setting the next property of %StringIteratorPrototype% invalidates the + // string iterator protector. + if (!isolate_->IsStringIteratorLookupChainIntact()) return; + isolate_->InvalidateStringIteratorProtector(); } } else if (*name_ == roots.species_symbol()) { if (!isolate_->IsArraySpeciesLookupChainIntact() && @@ -354,9 +361,17 @@ void LookupIterator::InternalUpdateProtector() { if (!isolate_->IsIsConcatSpreadableLookupChainIntact()) return; isolate_->InvalidateIsConcatSpreadableProtector(); } else if (*name_ == roots.iterator_symbol()) { - if (!isolate_->IsArrayIteratorLookupChainIntact()) return; if (holder_->IsJSArray()) { + if (!isolate_->IsArrayIteratorLookupChainIntact()) return; isolate_->InvalidateArrayIteratorProtector(); + } else if (isolate_->IsInAnyContext( + *receiver_, Context::INITIAL_STRING_PROTOTYPE_INDEX)) { + // Setting the Symbol.iterator property of String.prototype invalidates + // the string iterator protector. Symbol.iterator can also be set on a + // String wrapper, but not on a primitive string. We only support + // protector for primitive strings. + if (!isolate_->IsStringIteratorLookupChainIntact()) return; + isolate_->InvalidateStringIteratorProtector(); } } else if (*name_ == roots.resolve_string()) { if (!isolate_->IsPromiseResolveLookupChainIntact()) return; diff --git a/src/roots.h b/src/roots.h index b40feab4b4..633e41fc40 100644 --- a/src/roots.h +++ b/src/roots.h @@ -234,6 +234,7 @@ class Symbol; V(PropertyCell, promise_hook_protector, PromiseHookProtector) \ V(Cell, promise_resolve_protector, PromiseResolveProtector) \ V(PropertyCell, promise_then_protector, PromiseThenProtector) \ + V(PropertyCell, string_iterator_protector, StringIteratorProtector) \ /* Caches */ \ V(FixedArray, number_string_cache, NumberStringCache) \ V(FixedArray, single_character_string_cache, SingleCharacterStringCache) \ diff --git a/src/runtime/runtime-test.cc b/src/runtime/runtime-test.cc index f00d8ba9bf..bcc36e9d87 100644 --- a/src/runtime/runtime-test.cc +++ b/src/runtime/runtime-test.cc @@ -935,6 +935,13 @@ RUNTIME_FUNCTION(Runtime_PromiseSpeciesProtector) { isolate->IsPromiseSpeciesLookupChainIntact()); } +RUNTIME_FUNCTION(Runtime_StringIteratorProtector) { + SealHandleScope shs(isolate); + DCHECK_EQ(0, args.length()); + return isolate->heap()->ToBoolean( + isolate->IsStringIteratorLookupChainIntact()); +} + // Take a compiled wasm module and serialize it into an array buffer, which is // then returned. RUNTIME_FUNCTION(Runtime_SerializeWasmModule) { diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index 80fc323510..842a925d37 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -500,6 +500,7 @@ namespace internal { F(ArraySpeciesProtector, 0, 1) \ F(TypedArraySpeciesProtector, 0, 1) \ F(PromiseSpeciesProtector, 0, 1) \ + F(StringIteratorProtector, 0, 1) \ F(SystemBreak, 0, 1) \ F(TraceEnter, 0, 1) \ F(TraceExit, 1, 1) \ diff --git a/test/mjsunit/es6/string-iterator2.js b/test/mjsunit/es6/string-iterator2.js new file mode 100644 index 0000000000..2c23a4aa76 --- /dev/null +++ b/test/mjsunit/es6/string-iterator2.js @@ -0,0 +1,24 @@ +// Copyright 2018 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --no-stress-opt + +// Tests for primitive strings. + +var str = 'ott'; +assertEquals(['o', 't', 't'], [...str]); +assertTrue(%StringIteratorProtector()); + +str[Symbol.iterator] = {}; +// Symbol.iterator can't be set on primitive strings, so it shouldn't invalidate +// the protector. +assertTrue(%StringIteratorProtector()); + +// This changes the String Iterator prototype. No more tests should be run after +// this in the same instance. +var iterator = str[Symbol.iterator](); +iterator.__proto__.next = () => ({value : undefined, done : true}); + +assertFalse(%StringIteratorProtector()); +assertEquals([], [...str]); diff --git a/test/mjsunit/es6/string-iterator3.js b/test/mjsunit/es6/string-iterator3.js new file mode 100644 index 0000000000..1b0e0273e5 --- /dev/null +++ b/test/mjsunit/es6/string-iterator3.js @@ -0,0 +1,20 @@ +// Copyright 2018 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --no-stress-opt + +// Tests for primitive strings. + +var str = 'ott'; +assertTrue(%StringIteratorProtector()); +assertEquals(['o', 't', 't'], [...str]); + +// This changes the String prototype. No more tests should be run after this in +// the same instance. +str.__proto__[Symbol.iterator] = + function() { + return {next : () => ({value : undefined, done : true})}; + }; +assertFalse(%StringIteratorProtector()); +assertEquals([], [...str]); diff --git a/test/mjsunit/es6/string-iterator4.js b/test/mjsunit/es6/string-iterator4.js new file mode 100644 index 0000000000..48c6521d3b --- /dev/null +++ b/test/mjsunit/es6/string-iterator4.js @@ -0,0 +1,30 @@ +// Copyright 2018 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --no-stress-opt + +// Tests for wrapped strings. + +var str = new String('ott'); +assertTrue(%StringIteratorProtector()); +assertEquals(['o', 't', 't'], [...str]); + +function iterator_fn() { + return {next : () => ({value : undefined, done : true})}; +}; + +str[Symbol.iterator] = iterator_fn; +// This shouldn't invalidate the protector, because it doesn't support String +// objects. +assertTrue(%StringIteratorProtector()); +assertEquals([], [...str]); + + +var str2 = new String('ott'); +assertEquals(['o', 't', 't'], [...str2]); +// This changes the String prototype. No more tests should be run after this in +// the same instance. +str2.__proto__[Symbol.iterator] = iterator_fn; +assertFalse(%StringIteratorProtector()); +assertEquals([], [...str2]); diff --git a/test/mjsunit/es6/string-iterator5.js b/test/mjsunit/es6/string-iterator5.js new file mode 100644 index 0000000000..ec9754a4bd --- /dev/null +++ b/test/mjsunit/es6/string-iterator5.js @@ -0,0 +1,15 @@ +// Copyright 2018 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax + +// Tests for primitive strings. + +var iterator = 'ott'[Symbol.iterator](); + +// These modifications shouldn't invalidate the String iterator protector. +iterator.__proto__.fonts = {}; +assertTrue(%StringIteratorProtector()); +iterator.__proto__[0] = 0; +assertTrue(%StringIteratorProtector()); diff --git a/test/mjsunit/es6/string-iterator6.js b/test/mjsunit/es6/string-iterator6.js new file mode 100644 index 0000000000..d1cd1f31eb --- /dev/null +++ b/test/mjsunit/es6/string-iterator6.js @@ -0,0 +1,11 @@ +// Copyright 2018 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --no-stress-opt + +assertTrue(%StringIteratorProtector()); + +delete 'ott'.__proto__[Symbol.iterator]; + +assertFalse(%StringIteratorProtector()); diff --git a/test/mjsunit/es6/string-iterator7.js b/test/mjsunit/es6/string-iterator7.js new file mode 100644 index 0000000000..387c6e81fc --- /dev/null +++ b/test/mjsunit/es6/string-iterator7.js @@ -0,0 +1,13 @@ +// Copyright 2018 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax + +assertTrue(%StringIteratorProtector()); + +const p = ""[Symbol.iterator]().__proto__; +let x = Object.create(p); +x.next = 42; + +assertTrue(%StringIteratorProtector()); diff --git a/test/mjsunit/es6/string-iterator8.js b/test/mjsunit/es6/string-iterator8.js new file mode 100644 index 0000000000..dbd4b7c46a --- /dev/null +++ b/test/mjsunit/es6/string-iterator8.js @@ -0,0 +1,14 @@ +// Copyright 2018 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax + +assertTrue(%StringIteratorProtector()); + +var proto = String.prototype; + +String.prototype = {}; + +assertEquals(proto, String.prototype); +assertTrue(%StringIteratorProtector());