[builtins] Widen the fast-path for Promise builtins.

This adds a new isolate wide Promise#then protector, which guards the
"then" lookup for all JSPromise instances whose [[Prototype]] is the
initial %PromisePrototype%. Thus arbitrary mutations to the
Promise.prototype (i.e. monkey-patching other methods or installing
new functions) no longer sent you down the slow-path. Use this protector
in Promise.prototype.catch and in Promise.resolve.

Drive-by-fix: Restructure the resolve logic a bit and avoid the
expensive and large SameValue check, which can be turned into a simple
reference equal, as the promise in there is known to be a JSPromise
anyways.

Bug: v8:7253
Change-Id: If68b12c6bc6ca9c4d10552ae84854ebc3b5774f9
Reviewed-on: https://chromium-review.googlesource.com/899302
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51085}

src/bootstrapper.cc

@@ -2420,10 +2420,6 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
     Handle<Map> prototype_map(prototype->map());
     Map::SetShouldBeFastPrototypeMap(prototype_map, true, isolate);
 
-    // Store the initial Promise.prototype map. This is used in fast-path
-    // checks. Do not alter the prototype after this point.
-    native_context()->set_promise_prototype_map(*prototype_map);
-
     {  // Internal: PromiseInternalConstructor
        // Also exposed as extrasUtils.createPromise.
       Handle<JSFunction> function =
@@ -4445,7 +4441,6 @@ void Genesis::InitializeGlobal_harmony_promise_finally() {
   // to prototype, so we update the saved map.
   Handle<Map> prototype_map(prototype->map());
   Map::SetShouldBeFastPrototypeMap(prototype_map, true, isolate());
-  native_context()->set_promise_prototype_map(*prototype_map);
 
   {
     Handle<SharedFunctionInfo> info = SimpleCreateSharedFunctionInfo(

src/builtins/builtins-promise-gen.h

@@ -127,16 +127,18 @@ class PromiseBuiltinsAssembler : public CodeStubAssembler {
 
   void InternalResolvePromise(Node* context, Node* promise, Node* result);
 
-  void BranchIfFastPath(Node* context, Node* promise, Label* if_isunmodified,
-                        Label* if_ismodified);
-
-  void BranchIfFastPath(Node* native_context, Node* promise_fun, Node* promise,
-                        Label* if_isunmodified, Label* if_ismodified);
-
   Node* CreatePromiseContext(Node* native_context, int slots);
   void PromiseFulfill(Node* context, Node* promise, Node* result,
                       v8::Promise::PromiseState status);
 
+  // We can skip the "then" lookup on {receiver_map} if it's [[Prototype]]
+  // is the (initial) Promise.prototype and the Promise#then() protector
+  // is intact, as that guards the lookup path for the "then" property
+  // on JSPromise instances which have the (initial) %PromisePrototype%.
+  void BranchIfPromiseThenLookupChainIntact(Node* native_context,
+                                            Node* receiver_map, Label* if_fast,
+                                            Label* if_slow);
+
   void BranchIfAccessCheckFailed(Node* context, Node* native_context,
                                  Node* promise_constructor, Node* executor,
                                  Label* if_noaccess);

src/code-stub-assembler.cc

@@ -4165,6 +4165,13 @@ Node* CodeStubAssembler::IsNoElementsProtectorCellInvalid() {
   return WordEqual(cell_value, invalid);
 }
 
+Node* CodeStubAssembler::IsPromiseThenProtectorCellInvalid() {
+  Node* invalid = SmiConstant(Isolate::kProtectorInvalid);
+  Node* cell = LoadRoot(Heap::kPromiseThenProtectorRootIndex);
+  Node* cell_value = LoadObjectField(cell, PropertyCell::kValueOffset);
+  return WordEqual(cell_value, invalid);
+}
+
 Node* CodeStubAssembler::IsSpeciesProtectorCellInvalid() {
   Node* invalid = SmiConstant(Isolate::kProtectorInvalid);
   Node* cell = LoadRoot(Heap::kSpeciesProtectorRootIndex);
@@ -7560,6 +7567,7 @@ void CodeStubAssembler::CheckForAssociatedProtector(Node* name,
          if_protector);
   GotoIf(WordEqual(name, LoadRoot(Heap::kis_concat_spreadable_symbolRootIndex)),
          if_protector);
+  GotoIf(WordEqual(name, LoadRoot(Heap::kthen_stringRootIndex)), if_protector);
   // Fall through if no case matched.
 }
 

src/code-stub-assembler.h

@@ -1143,7 +1143,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   Node* IsShortExternalStringInstanceType(Node* instance_type);
   Node* IsSpecialReceiverInstanceType(Node* instance_type);
   Node* IsSpecialReceiverMap(Node* map);
-  Node* IsSpeciesProtectorCellInvalid();
   Node* IsStringInstanceType(Node* instance_type);
   Node* IsString(Node* object);
   Node* IsSymbolInstanceType(Node* instance_type);
@@ -1156,6 +1155,9 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
     return IsSharedFunctionInfoMap(LoadMap(object));
   }
 
+  Node* IsPromiseThenProtectorCellInvalid();
+  Node* IsSpeciesProtectorCellInvalid();
+
   // True iff |object| is a Smi or a HeapNumber.
   Node* IsNumber(Node* object);
   // True iff |object| is a Smi or a HeapNumber or a BigInt.

src/compiler/js-call-reducer.cc

@@ -10,7 +10,6 @@
 #include "src/code-stubs.h"
 #include "src/compilation-dependencies.h"
 #include "src/compiler/access-builder.h"
-#include "src/compiler/access-info.h"
 #include "src/compiler/allocation-builder.h"
 #include "src/compiler/js-graph.h"
 #include "src/compiler/linkage.h"
@@ -3944,27 +3943,30 @@ Reduction JSCallReducer::ReducePromisePrototypeCatch(Node* node) {
     return NoChange();
   }
 
+  // Check that the Promise.then protector is intact. This protector guards
+  // that all JSPromise instances whose [[Prototype]] is the initial
+  // %PromisePrototype% yield the initial %PromisePrototype%.then method
+  // when looking up "then".
+  if (!isolate()->IsPromiseThenLookupChainIntact()) return NoChange();
+
   // Check if we know something about {receiver} already.
   ZoneHandleSet<Map> receiver_maps;
   NodeProperties::InferReceiverMapsResult result =
       NodeProperties::InferReceiverMaps(receiver, effect, &receiver_maps);
   if (result == NodeProperties::kNoReceiverMaps) return NoChange();
   DCHECK_NE(0, receiver_maps.size());
-  if (receiver_maps.size() != 1) return NoChange();
-  Handle<Map> receiver_map = receiver_maps[0];
-
-  // Lookup the "then" method on the {receiver_map}.
-  PropertyAccessInfo access_info;
-  AccessInfoFactory access_info_factory(dependencies(), native_context(),
-                                        graph()->zone());
-  if (!access_info_factory.ComputePropertyAccessInfo(
-          receiver_map, factory()->then_string(), AccessMode::kLoad,
-          &access_info) ||
-      !access_info.IsDataConstant()) {
-    return NoChange();
+
+  // Check whether all {receiver_maps} are JSPromise maps and
+  // have the initial Promise.prototype as their [[Prototype]].
+  for (Handle<Map> receiver_map : receiver_maps) {
+    if (!receiver_map->IsJSPromiseMap()) return NoChange();
+    if (receiver_map->prototype() != native_context()->promise_prototype()) {
+      return NoChange();
+    }
   }
-  dependencies()->AssumePrototypeMapsStable(receiver_map, access_info.holder());
-  Handle<Object> then = access_info.constant();
+
+  // Add a code dependency on the necessary protectors.
+  dependencies()->AssumePropertyCell(factory()->promise_then_protector());
 
   // If the {receiver_maps} aren't reliable, we need to repeat the
   // map check here, guarded by the CALL_IC.
@@ -3978,7 +3980,8 @@ Reduction JSCallReducer::ReducePromisePrototypeCatch(Node* node) {
   // Massage the {node} to call "then" instead by first removing all inputs
   // following the onRejected parameter, and then filling up the parameters
   // to two inputs from the left with undefined.
-  NodeProperties::ReplaceValueInput(node, jsgraph()->Constant(then), 0);
+  Node* target = jsgraph()->Constant(handle(native_context()->promise_then()));
+  NodeProperties::ReplaceValueInput(node, target, 0);
   NodeProperties::ReplaceEffectInput(node, effect);
   for (; arity > 1; --arity) node->RemoveInput(3);
   for (; arity < 2; ++arity) {

src/contexts.h

@@ -338,7 +338,6 @@ enum ContextLookupFlags {
   V(PROMISE_ALL_RESOLVE_ELEMENT_SHARED_FUN, SharedFunctionInfo,                \
     promise_all_resolve_element_shared_fun)                                    \
   V(PROMISE_PROTOTYPE_INDEX, JSObject, promise_prototype)                      \
-  V(PROMISE_PROTOTYPE_MAP_INDEX, Map, promise_prototype_map)                   \
   V(REGEXP_EXEC_FUNCTION_INDEX, JSFunction, regexp_exec_function)              \
   V(REGEXP_FUNCTION_INDEX, JSFunction, regexp_function)                        \
   V(REGEXP_LAST_MATCH_INFO_INDEX, RegExpMatchInfo, regexp_last_match_info)     \

src/heap/heap.h

@@ -215,6 +215,7 @@ using v8::MemoryPressureLevel;
   V(PropertyCell, array_buffer_neutering_protector,                            \
     ArrayBufferNeuteringProtector)                                             \
   V(PropertyCell, promise_hook_protector, PromiseHookProtector)                \
+  V(PropertyCell, promise_then_protector, PromiseThenProtector)                \
   /* Special numbers */                                                        \
   V(HeapNumber, nan_value, NanValue)                                           \
   V(HeapNumber, hole_nan_value, HoleNanValue)                                  \

src/heap/setup-heap-internal.cc

@@ -643,6 +643,10 @@ void Heap::CreateInitialObjects() {
   cell->set_value(Smi::FromInt(Isolate::kProtectorValid));
   set_promise_hook_protector(*cell);
 
+  cell = factory->NewPropertyCell(factory->empty_string());
+  cell->set_value(Smi::FromInt(Isolate::kProtectorValid));
+  set_promise_then_protector(*cell);
+
   set_serialized_objects(empty_fixed_array());
   set_serialized_global_proxy_sizes(empty_fixed_array());
 

src/isolate.cc

@@ -3482,6 +3482,13 @@ bool Isolate::IsPromiseHookProtectorIntact() {
   return is_promise_hook_protector_intact;
 }
 
+bool Isolate::IsPromiseThenLookupChainIntact() {
+  PropertyCell* promise_then_cell = heap()->promise_then_protector();
+  bool is_promise_then_protector_intact =
+      Smi::ToInt(promise_then_cell->value()) == kProtectorValid;
+  return is_promise_then_protector_intact;
+}
+
 void Isolate::UpdateNoElementsProtectorOnSetElement(Handle<JSObject> object) {
   DisallowHeapAllocation no_gc;
   if (!object->map()->is_prototype_map()) return;
@@ -3550,6 +3557,15 @@ void Isolate::InvalidatePromiseHookProtector() {
   DCHECK(!IsPromiseHookProtectorIntact());
 }
 
+void Isolate::InvalidatePromiseThenProtector() {
+  DCHECK(factory()->promise_then_protector()->value()->IsSmi());
+  DCHECK(IsPromiseThenLookupChainIntact());
+  PropertyCell::SetValueWithInvalidation(
+      factory()->promise_then_protector(),
+      handle(Smi::FromInt(kProtectorInvalid), this));
+  DCHECK(!IsPromiseThenLookupChainIntact());
+}
+
 bool Isolate::IsAnyInitialArrayPrototype(Handle<JSArray> array) {
   DisallowHeapAllocation no_gc;
   return IsInAnyContext(*array, Context::INITIAL_ARRAY_PROTOTYPE_INDEX);

src/isolate.h

@@ -1106,6 +1106,10 @@ class Isolate {
   // active.
   bool IsPromiseHookProtectorIntact();
 
+  // Make sure a lookup of "then" on any JSPromise whose [[Prototype]] is the
+  // initial %PromisePrototype% yields the initial method.
+  bool IsPromiseThenLookupChainIntact();
+
   // On intent to set an element in object, make sure that appropriate
   // notifications occur if the set is on the elements of the array or
   // object prototype. Also ensure that changes to prototype chain between
@@ -1127,6 +1131,7 @@ class Isolate {
   void InvalidateArrayIteratorProtector();
   void InvalidateArrayBufferNeuteringProtector();
   void InvalidatePromiseHookProtector();
+  void InvalidatePromiseThenProtector();
 
   // Returns true if array is the initial array prototype in any native context.
   bool IsAnyInitialArrayPrototype(Handle<JSArray> array);

src/lookup.cc

@@ -306,6 +306,14 @@ void LookupIterator::InternalUpdateProtector() {
     if (holder_->IsJSArray()) {
       isolate_->InvalidateArrayIteratorProtector();
     }
+  } else if (*name_ == heap()->then_string()) {
+    if (!isolate_->IsPromiseThenLookupChainIntact()) return;
+    // Setting the "then" property on any JSPromise instance or on the
+    // initial %PromisePrototype% invalidates the Promise#then protector.
+    if (holder_->IsJSPromise() ||
+        isolate_->IsInAnyContext(*holder_, Context::PROMISE_PROTOTYPE_INDEX)) {
+      isolate_->InvalidatePromiseThenProtector();
+    }
   }
 }
 

src/lookup.h

@@ -275,11 +275,12 @@ class V8_EXPORT_PRIVATE LookupIterator final BASE_EMBEDDED {
   inline void UpdateProtector() {
     if (IsElement()) return;
     // This list must be kept in sync with
-    // CodeStubAssembler::HasAssociatedProtector!
+    // CodeStubAssembler::CheckForAssociatedProtector!
     if (*name_ == heap()->is_concat_spreadable_symbol() ||
         *name_ == heap()->constructor_string() ||
         *name_ == heap()->species_symbol() ||
-        *name_ == heap()->iterator_symbol()) {
+        *name_ == heap()->iterator_symbol() ||
+        *name_ == heap()->then_string()) {
       InternalUpdateProtector();
     }
   }

test/cctest/heap/test-heap.cc

@@ -112,8 +112,6 @@ TEST(ContextMaps) {
                            Context::STRING_FUNCTION_INDEX);
   VerifyStoredPrototypeMap(isolate, Context::REGEXP_PROTOTYPE_MAP_INDEX,
                            Context::REGEXP_FUNCTION_INDEX);
-  VerifyStoredPrototypeMap(isolate, Context::PROMISE_PROTOTYPE_MAP_INDEX,
-                           Context::PROMISE_FUNCTION_INDEX);
 }
 
 TEST(InitialObjects) {

test/mjsunit/compiler/promise-prototype-catch-custom-then-1.js

+// 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
+
+function foo(p) { return p.catch(x => x); }
+
+const a = Promise.resolve(1);
+
+foo(a);
+foo(a);
+%OptimizeFunctionOnNextCall(foo);
+foo(a);
+
+let custom_then_called = false;
+a.__proto__.then = function() { custom_then_called = true; }
+foo(a);
+assertTrue(custom_then_called);

test/mjsunit/compiler/promise-prototype-catch-custom-then-2.js

+// 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
+
+function foo(p) { return p.catch(x => x); }
+
+const a = Promise.resolve(1);
+
+foo(a);
+foo(a);
+%OptimizeFunctionOnNextCall(foo);
+foo(a);
+
+let custom_then_called = false;
+a.then = function() { custom_then_called = true; }
+foo(a);
+assertTrue(custom_then_called);

test/mjsunit/compiler/promise-prototype-catch-subclass.js

+// 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
+
+let custom_then_called = false;
+
+function foo(p) {
+  custom_then_called = false;
+  p.catch(x => x);
+  return custom_then_called;
+}
+
+class MyPromise extends Promise {
+  then(onFulfilled, onRejected) {
+    custom_then_called = true;
+    return super.then(onFulfilled, onRejected);
+  }
+}
+
+const a = MyPromise.resolve(1);
+
+assertTrue(foo(a));
+assertTrue(foo(a));
+%OptimizeFunctionOnNextCall(foo);
+assertTrue(foo(a));

tools/v8heapconst.py

@@ -337,9 +337,9 @@ KNOWN_OBJECTS = {
   ("OLD_SPACE", 0x029d9): "FastArrayIterationProtector",
   ("OLD_SPACE", 0x029e9): "ArrayIteratorProtector",
   ("OLD_SPACE", 0x02a11): "ArrayBufferNeuteringProtector",
-  ("OLD_SPACE", 0x02a61): "InfinityValue",
-  ("OLD_SPACE", 0x02a71): "MinusZeroValue",
-  ("OLD_SPACE", 0x02a81): "MinusInfinityValue",
+  ("OLD_SPACE", 0x02a89): "InfinityValue",
+  ("OLD_SPACE", 0x02a99): "MinusZeroValue",
+  ("OLD_SPACE", 0x02aa9): "MinusInfinityValue",
 }
 
 # List of known V8 Frame Markers.