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 <ulan@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Hai Dang <dhai@google.com>
Cr-Commit-Position: refs/heads/master@{#56315}

src/bootstrapper.cc

@@ -2141,8 +2141,10 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> 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 ---

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,

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());

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 {

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)  \

src/heap/factory.cc

@@ -1308,7 +1308,7 @@ Handle<ExternalOneByteString> Factory::NewNativeSourceString(
 }
 
 Handle<JSStringIterator> Factory::NewJSStringIterator(Handle<String> string) {
-  Handle<Map> map(isolate()->native_context()->string_iterator_map(),
+  Handle<Map> map(isolate()->native_context()->initial_string_iterator_map(),
                   isolate());
   Handle<String> flat_string = String::Flatten(isolate(), string);
   Handle<JSStringIterator> iterator =

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<Cell> string_length_overflow_cell = factory->NewCell(
       handle(Smi::FromInt(Isolate::kProtectorValid), isolate()));
   set_string_length_protector(*string_length_overflow_cell);

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
 

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());

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();

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;

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)   \

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) {

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)                          \

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]);

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]);

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]);

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());

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());

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());

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());