Reland "[runtime] Fix protector invalidation"

verwaest · Commit Bot · commit 9c766330e06c · 2019-07-19T13:32:25.000Z
This is a reland of e55e0aa Original change's description: > [runtime] Fix protector invalidation > > Protectors trigger when special properties are modified or masked. Previously > we would check whether the property stored on the holder would invalidate the > protector. Stores to to the receiver rather than the holder, however, so this > CL changes holder for receiver, and adds additional checks that were missing. > > Bug: v8:9466 > Change-Id: I81bc3d73f91381da0d254e9eb79365ae2d25d998 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1708468 > Commit-Queue: Leszek Swirski <leszeks@chromium.org> > Reviewed-by: Leszek Swirski <leszeks@chromium.org> > Cr-Commit-Position: refs/heads/master@{#62805} Tbr: leszeks@chromium.org Bug: v8:9466 Change-Id: I693c73577ca9a35a271f509770cc1c87e5cc4b73 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1709420 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Leszek Swirski <leszeks@chromium.org> Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org> Cr-Commit-Position: refs/heads/master@{#62829}
diff --git a/src/objects/lookup.cc b/src/objects/lookup.cc
@@ -212,7 +212,7 @@ void LookupIterator::ReloadPropertyInformation() {
 
 namespace {
 
-bool IsTypedArrayFunctionInAnyContext(Isolate* isolate, JSReceiver holder) {
+bool IsTypedArrayFunctionInAnyContext(Isolate* isolate, HeapObject object) {
   static uint32_t context_slots[] = {
 #define TYPED_ARRAY_CONTEXT_SLOTS(Type, type, TYPE, ctype) \
   Context::TYPE##_ARRAY_FUN_INDEX,
@@ -221,17 +221,19 @@ bool IsTypedArrayFunctionInAnyContext(Isolate* isolate, JSReceiver holder) {
 #undef TYPED_ARRAY_CONTEXT_SLOTS
   };
 
-  if (!holder.IsJSFunction(isolate)) return false;
+  if (!object.IsJSFunction(isolate)) return false;
 
   return std::any_of(
       std::begin(context_slots), std::end(context_slots),
-      [=](uint32_t slot) { return isolate->IsInAnyContext(holder, slot); });
+      [=](uint32_t slot) { return isolate->IsInAnyContext(object, slot); });
 }
 
 }  // namespace
 
 void LookupIterator::InternalUpdateProtector() {
   if (isolate_->bootstrapper()->IsActive()) return;
+  if (!receiver_->IsHeapObject()) return;
+  Handle<HeapObject> receiver = Handle<HeapObject>::cast(receiver_);
 
   Handle<NativeContext> native_context = isolate_->native_context();
 
@@ -244,70 +246,74 @@ void LookupIterator::InternalUpdateProtector() {
       return;
     }
     // Setting the constructor property could change an instance's @@species
-    if (holder_->IsJSArray(isolate_)) {
+    if (receiver->IsJSArray(isolate_)) {
       if (!isolate_->IsArraySpeciesLookupChainIntact()) return;
       isolate_->CountUsage(
           v8::Isolate::UseCounterFeature::kArrayInstanceConstructorModified);
       isolate_->InvalidateArraySpeciesProtector();
       return;
-    } else if (holder_->IsJSPromise(isolate_)) {
+    } else if (receiver->IsJSPromise(isolate_)) {
       if (!isolate_->IsPromiseSpeciesLookupChainIntact()) return;
       isolate_->InvalidatePromiseSpeciesProtector();
       return;
-    } else if (holder_->IsJSRegExp(isolate_)) {
+    } else if (receiver->IsJSRegExp(isolate_)) {
       if (!isolate_->IsRegExpSpeciesLookupChainIntact(native_context)) return;
       isolate_->InvalidateRegExpSpeciesProtector(native_context);
       return;
-    } else if (holder_->IsJSTypedArray(isolate_)) {
+    } else if (receiver->IsJSTypedArray(isolate_)) {
       if (!isolate_->IsTypedArraySpeciesLookupChainIntact()) return;
       isolate_->InvalidateTypedArraySpeciesProtector();
       return;
     }
-    if (holder_->map(isolate_).is_prototype_map()) {
+    if (receiver->map(isolate_).is_prototype_map()) {
       DisallowHeapAllocation no_gc;
       // Setting the constructor of any prototype with the @@species protector
       // (of any realm) also needs to invalidate the protector.
-      // For typed arrays, we check a prototype of this holder since TypedArrays
-      // have different prototypes for each type, and their parent prototype is
-      // pointing the same TYPED_ARRAY_PROTOTYPE.
-      if (isolate_->IsInAnyContext(*holder_,
+      // For typed arrays, we check a prototype of this receiver since
+      // TypedArrays have different prototypes for each type, and their parent
+      // prototype is pointing the same TYPED_ARRAY_PROTOTYPE.
+      if (isolate_->IsInAnyContext(*receiver,
                                    Context::INITIAL_ARRAY_PROTOTYPE_INDEX)) {
         if (!isolate_->IsArraySpeciesLookupChainIntact()) return;
         isolate_->CountUsage(
             v8::Isolate::UseCounterFeature::kArrayPrototypeConstructorModified);
         isolate_->InvalidateArraySpeciesProtector();
-      } else if (isolate_->IsInAnyContext(*holder_,
+      } else if (isolate_->IsInAnyContext(*receiver,
                                           Context::PROMISE_PROTOTYPE_INDEX)) {
         if (!isolate_->IsPromiseSpeciesLookupChainIntact()) return;
         isolate_->InvalidatePromiseSpeciesProtector();
-      } else if (isolate_->IsInAnyContext(*holder_,
+      } else if (isolate_->IsInAnyContext(*receiver,
                                           Context::REGEXP_PROTOTYPE_INDEX)) {
         if (!isolate_->IsRegExpSpeciesLookupChainIntact(native_context)) return;
         isolate_->InvalidateRegExpSpeciesProtector(native_context);
       } else if (isolate_->IsInAnyContext(
-                     holder_->map(isolate_).prototype(isolate_),
+                     receiver->map(isolate_).prototype(isolate_),
                      Context::TYPED_ARRAY_PROTOTYPE_INDEX)) {
         if (!isolate_->IsTypedArraySpeciesLookupChainIntact()) return;
         isolate_->InvalidateTypedArraySpeciesProtector();
       }
     }
   } else if (*name_ == roots.next_string()) {
-    if (isolate_->IsInAnyContext(
-            *holder_, Context::INITIAL_ARRAY_ITERATOR_PROTOTYPE_INDEX)) {
+    if (receiver->IsJSArrayIterator() ||
+        isolate_->IsInAnyContext(
+            *receiver, 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(
-                   *holder_, Context::INITIAL_MAP_ITERATOR_PROTOTYPE_INDEX)) {
+    } else if (receiver->IsJSMapIterator() ||
+               isolate_->IsInAnyContext(
+                   *receiver, Context::INITIAL_MAP_ITERATOR_PROTOTYPE_INDEX)) {
       if (!isolate_->IsMapIteratorLookupChainIntact()) return;
       isolate_->InvalidateMapIteratorProtector();
-    } else if (isolate_->IsInAnyContext(
-                   *holder_, Context::INITIAL_SET_ITERATOR_PROTOTYPE_INDEX)) {
+    } else if (receiver->IsJSSetIterator() ||
+               isolate_->IsInAnyContext(
+                   *receiver, Context::INITIAL_SET_ITERATOR_PROTOTYPE_INDEX)) {
       if (!isolate_->IsSetIteratorLookupChainIntact()) return;
       isolate_->InvalidateSetIteratorProtector();
-    } else if (isolate_->IsInAnyContext(
-                   *receiver_,
+    } else if (receiver->IsJSStringIterator() ||
+               isolate_->IsInAnyContext(
+                   *receiver,
                    Context::INITIAL_STRING_ITERATOR_PROTOTYPE_INDEX)) {
       // Setting the next property of %StringIteratorPrototype% invalidates the
       // string iterator protector.
@@ -323,44 +329,54 @@ void LookupIterator::InternalUpdateProtector() {
     }
     // Setting the Symbol.species property of any Array, Promise or TypedArray
     // constructor invalidates the @@species protector
-    if (isolate_->IsInAnyContext(*holder_, Context::ARRAY_FUNCTION_INDEX)) {
+    if (isolate_->IsInAnyContext(*receiver, Context::ARRAY_FUNCTION_INDEX)) {
       if (!isolate_->IsArraySpeciesLookupChainIntact()) return;
       isolate_->CountUsage(
           v8::Isolate::UseCounterFeature::kArraySpeciesModified);
       isolate_->InvalidateArraySpeciesProtector();
-    } else if (isolate_->IsInAnyContext(*holder_,
+    } else if (isolate_->IsInAnyContext(*receiver,
                                         Context::PROMISE_FUNCTION_INDEX)) {
       if (!isolate_->IsPromiseSpeciesLookupChainIntact()) return;
       isolate_->InvalidatePromiseSpeciesProtector();
-    } else if (isolate_->IsInAnyContext(*holder_,
+    } else if (isolate_->IsInAnyContext(*receiver,
                                         Context::REGEXP_FUNCTION_INDEX)) {
       if (!isolate_->IsRegExpSpeciesLookupChainIntact(native_context)) return;
       isolate_->InvalidateRegExpSpeciesProtector(native_context);
-    } else if (IsTypedArrayFunctionInAnyContext(isolate_, *holder_)) {
+    } else if (IsTypedArrayFunctionInAnyContext(isolate_, *receiver)) {
       if (!isolate_->IsTypedArraySpeciesLookupChainIntact()) return;
       isolate_->InvalidateTypedArraySpeciesProtector();
     }
   } else if (*name_ == roots.is_concat_spreadable_symbol()) {
     if (!isolate_->IsIsConcatSpreadableLookupChainIntact()) return;
     isolate_->InvalidateIsConcatSpreadableProtector();
   } else if (*name_ == roots.iterator_symbol()) {
-    if (holder_->IsJSArray(isolate_)) {
+    if (receiver->IsJSArray(isolate_)) {
       if (!isolate_->IsArrayIteratorLookupChainIntact()) return;
       isolate_->InvalidateArrayIteratorProtector();
+    } else if (receiver->IsJSSet(isolate_) || receiver->IsJSSetIterator() ||
+               isolate_->IsInAnyContext(
+                   *receiver, Context::INITIAL_SET_ITERATOR_PROTOTYPE_INDEX) ||
+               isolate_->IsInAnyContext(*receiver,
+                                        Context::INITIAL_SET_PROTOTYPE_INDEX)) {
+      if (isolate_->IsSetIteratorLookupChainIntact()) {
+        isolate_->InvalidateSetIteratorProtector();
+      }
+    } else if (receiver->IsJSMapIterator() ||
+               isolate_->IsInAnyContext(
+                   *receiver, Context::INITIAL_MAP_ITERATOR_PROTOTYPE_INDEX)) {
+      if (isolate_->IsMapIteratorLookupChainIntact()) {
+        isolate_->InvalidateMapIteratorProtector();
+      }
     } else if (isolate_->IsInAnyContext(
-                   *holder_, Context::INITIAL_ITERATOR_PROTOTYPE_INDEX)) {
+                   *receiver, Context::INITIAL_ITERATOR_PROTOTYPE_INDEX)) {
       if (isolate_->IsMapIteratorLookupChainIntact()) {
         isolate_->InvalidateMapIteratorProtector();
       }
       if (isolate_->IsSetIteratorLookupChainIntact()) {
         isolate_->InvalidateSetIteratorProtector();
       }
-    } else if (isolate_->IsInAnyContext(*holder_,
-                                        Context::INITIAL_SET_PROTOTYPE_INDEX)) {
-      if (!isolate_->IsSetIteratorLookupChainIntact()) return;
-      isolate_->InvalidateSetIteratorProtector();
     } else if (isolate_->IsInAnyContext(
-                   *receiver_, Context::INITIAL_STRING_PROTOTYPE_INDEX)) {
+                   *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
@@ -372,7 +388,7 @@ void LookupIterator::InternalUpdateProtector() {
     if (!isolate_->IsPromiseResolveLookupChainIntact()) return;
     // Setting the "resolve" property on any %Promise% intrinsic object
     // invalidates the Promise.resolve protector.
-    if (isolate_->IsInAnyContext(*holder_, Context::PROMISE_FUNCTION_INDEX)) {
+    if (isolate_->IsInAnyContext(*receiver, Context::PROMISE_FUNCTION_INDEX)) {
       isolate_->InvalidatePromiseResolveProtector();
     }
   } else if (*name_ == roots.then_string()) {
@@ -384,10 +400,10 @@ void LookupIterator::InternalUpdateProtector() {
     // to guard the fast-path in AsyncGeneratorResolve, where we can skip
     // the ResolvePromise step and go directly to FulfillPromise if we
     // know that the Object.prototype doesn't contain a "then" method.
-    if (holder_->IsJSPromise(isolate_) ||
-        isolate_->IsInAnyContext(*holder_,
+    if (receiver->IsJSPromise(isolate_) ||
+        isolate_->IsInAnyContext(*receiver,
                                  Context::INITIAL_OBJECT_PROTOTYPE_INDEX) ||
-        isolate_->IsInAnyContext(*holder_, Context::PROMISE_PROTOTYPE_INDEX)) {
+        isolate_->IsInAnyContext(*receiver, Context::PROMISE_PROTOTYPE_INDEX)) {
       isolate_->InvalidatePromiseThenProtector();
     }
   }
diff --git a/test/mjsunit/es6/map-iterator-8.js b/test/mjsunit/es6/map-iterator-8.js
@@ -25,7 +25,7 @@ assertEquals([], [...map.keys()]);
 assertEquals([], [...map.values()]);
 assertEquals([], [...iterator]);
 
-assertFalse(%SetIteratorProtector());
+assertTrue(%SetIteratorProtector());
 assertEquals([1,2,3], [...set]);
 assertEquals([[1,1],[2,2],[3,3]], [...set.entries()]);
 assertEquals([1,2,3], [...set.keys()]);
diff --git a/test/mjsunit/es6/map-iterator-9.js b/test/mjsunit/es6/map-iterator-9.js
@@ -23,7 +23,7 @@ assertEquals([1,2,3], [...map.keys()]);
 assertEquals([2,3,4], [...map.values()]);
 assertEquals([], [...iterator]);
 
-assertFalse(%SetIteratorProtector());
+assertTrue(%SetIteratorProtector());
 assertEquals([1,2,3], [...set]);
 assertEquals([[1,1],[2,2],[3,3]], [...set.entries()]);
 assertEquals([1,2,3], [...set.keys()]);
diff --git a/test/mjsunit/es6/set-iterator-8.js b/test/mjsunit/es6/set-iterator-8.js
@@ -18,7 +18,7 @@ assertTrue(%SetIteratorProtector());
 var iterator = set.keys();
 iterator.__proto__[Symbol.iterator] = () => ({next: () => ({done: true})});
 
-assertFalse(%MapIteratorProtector());
+assertTrue(%MapIteratorProtector());
 assertEquals([[1,2], [2,3], [3,4]], [...map]);
 assertEquals([[1,2], [2,3], [3,4]], [...map.entries()]);
 assertEquals([1,2,3], [...map.keys()]);
diff --git a/test/mjsunit/es6/set-iterator-9.js b/test/mjsunit/es6/set-iterator-9.js
@@ -17,7 +17,7 @@ assertTrue(%SetIteratorProtector());
 var iterator = set.keys();
 iterator[Symbol.iterator] = () => ({next: () => ({done: true})});
 
-assertFalse(%MapIteratorProtector());
+assertTrue(%MapIteratorProtector());
 assertEquals([[1,2], [2,3], [3,4]], [...map]);
 assertEquals([[1,2], [2,3], [3,4]], [...map.entries()]);
 assertEquals([1,2,3], [...map.keys()]);
diff --git a/test/mjsunit/regress/regress-9466.js b/test/mjsunit/regress/regress-9466.js
@@ -0,0 +1,10 @@
+// Copyright 2019 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.
+
+const o = [];
+o.__proto__ = {};
+o.constructor = function() {};
+o.constructor[Symbol.species] = function f() {};
+o.__proto__ = Array.prototype;
+assertEquals(o.constructor[Symbol.species], o.concat([1,2,3]).constructor);
