[es2015] Remove the @@hasInstance protector cell.

We cannot skip the @@hasInstance lookup in instanceof depending on a
global protector cell, as the lookup of the property is observable
via proxies or accessors. So remove the global protector and properly
implement CSA::InstanceOf via GetPropertyStub, with an appropriate
fast-path for Function.prototype[@@hasInstance] where we call the
builtin code object directly if the function matches, skipping all
the checks from the call sequence, and also avoid the redundant
ToBoolean conversion on the result.

R=yangguo@chromium.org
TBR=ulan@chromium.org
BUG=v8:5958

Review-Url: https://codereview.chromium.org/2684033012
Cr-Commit-Position: refs/heads/master@{#43137}

src/builtins/builtins.h

@@ -641,8 +641,11 @@ class Isolate;
   CPP(ObjectSeal)                                                              \
   CPP(ObjectValues)                                                            \
                                                                                \
-  TFS(InstanceOf, BUILTIN, kNoExtraICState, Compare, 1)                        \
+  /* instanceof */                                                             \
   TFS(OrdinaryHasInstance, BUILTIN, kNoExtraICState, Compare, 1)               \
+  TFS(InstanceOf, BUILTIN, kNoExtraICState, Compare, 1)                        \
+                                                                               \
+  /* for-in */                                                                 \
   TFS(ForInFilter, BUILTIN, kNoExtraICState, ForInFilter, 1)                   \
   TFS(ForInNext, BUILTIN, kNoExtraICState, ForInNext, 1)                       \
   TFS(ForInPrepare, BUILTIN, kNoExtraICState, ForInPrepare, 3)                 \

src/code-stub-assembler.cc

@@ -2911,6 +2911,10 @@ Node* CodeStubAssembler::IsCallableMap(Node* map) {
       Int32Constant(0));
 }
 
+Node* CodeStubAssembler::IsCallable(Node* object) {
+  return IsCallableMap(LoadMap(object));
+}
+
 Node* CodeStubAssembler::IsConstructorMap(Node* map) {
   CSA_ASSERT(this, IsMap(map));
   return Word32NotEqual(
@@ -7962,33 +7966,90 @@ Node* CodeStubAssembler::GetSuperConstructor(Node* active_function,
 
 Node* CodeStubAssembler::InstanceOf(Node* object, Node* callable,
                                     Node* context) {
-  Label return_runtime(this, Label::kDeferred), end(this);
-  Variable result(this, MachineRepresentation::kTagged);
+  Variable var_result(this, MachineRepresentation::kTagged);
+  Label if_notcallable(this, Label::kDeferred),
+      if_notreceiver(this, Label::kDeferred), if_otherhandler(this),
+      if_nohandler(this, Label::kDeferred), return_true(this),
+      return_false(this), return_result(this, &var_result);
+
+  // Ensure that the {callable} is actually a JSReceiver.
+  GotoIf(TaggedIsSmi(callable), &if_notreceiver);
+  GotoUnless(IsJSReceiver(callable), &if_notreceiver);
+
+  // Load the @@hasInstance property from {callable}.
+  Node* inst_of_handler = CallStub(CodeFactory::GetProperty(isolate()), context,
+                                   callable, HasInstanceSymbolConstant());
+
+  // Optimize for the likely case where {inst_of_handler} is the builtin
+  // Function.prototype[@@hasInstance] method, and emit a direct call in
+  // that case without any additional checking.
+  Node* native_context = LoadNativeContext(context);
+  Node* function_has_instance =
+      LoadContextElement(native_context, Context::FUNCTION_HAS_INSTANCE_INDEX);
+  GotoUnless(WordEqual(inst_of_handler, function_has_instance),
+             &if_otherhandler);
+  {
+    // Call to Function.prototype[@@hasInstance] directly.
+    Callable builtin(isolate()->builtins()->FunctionPrototypeHasInstance(),
+                     CallTrampolineDescriptor(isolate()));
+    Node* result = CallJS(builtin, context, inst_of_handler, callable, object);
+    var_result.Bind(result);
+    Goto(&return_result);
+  }
 
-  // Check if no one installed @@hasInstance somewhere.
-  GotoUnless(
-      WordEqual(LoadObjectField(LoadRoot(Heap::kHasInstanceProtectorRootIndex),
-                                PropertyCell::kValueOffset),
-                SmiConstant(Smi::FromInt(Isolate::kProtectorValid))),
-      &return_runtime);
+  Bind(&if_otherhandler);
+  {
+    // Check if there's actually an {inst_of_handler}.
+    GotoIf(IsNull(inst_of_handler), &if_nohandler);
+    GotoIf(IsUndefined(inst_of_handler), &if_nohandler);
 
-  // Check if {callable} is a valid receiver.
-  GotoIf(TaggedIsSmi(callable), &return_runtime);
-  GotoUnless(IsCallableMap(LoadMap(callable)), &return_runtime);
+    // Call the {inst_of_handler} for {callable} and {object}.
+    Node* result = CallJS(
+        CodeFactory::Call(isolate(), ConvertReceiverMode::kNotNullOrUndefined),
+        context, inst_of_handler, callable, object);
 
-  // Use the inline OrdinaryHasInstance directly.
-  result.Bind(OrdinaryHasInstance(context, callable, object));
-  Goto(&end);
+    // Convert the {result} to a Boolean.
+    BranchIfToBooleanIsTrue(result, &return_true, &return_false);
+  }
 
-  // TODO(bmeurer): Use GetPropertyStub here once available.
-  Bind(&return_runtime);
+  Bind(&if_nohandler);
   {
-    result.Bind(CallRuntime(Runtime::kInstanceOf, context, object, callable));
-    Goto(&end);
+    // Ensure that the {callable} is actually Callable.
+    GotoUnless(IsCallable(callable), &if_notcallable);
+
+    // Use the OrdinaryHasInstance algorithm.
+    Node* result = CallStub(CodeFactory::OrdinaryHasInstance(isolate()),
+                            context, callable, object);
+    var_result.Bind(result);
+    Goto(&return_result);
   }
 
-  Bind(&end);
-  return result.value();
+  Bind(&if_notcallable);
+  {
+    Node* result =
+        CallRuntime(Runtime::kThrowNonCallableInInstanceOfCheck, context);
+    var_result.Bind(result);
+    Goto(&return_result);
+  }
+
+  Bind(&if_notreceiver);
+  {
+    Node* result =
+        CallRuntime(Runtime::kThrowNonObjectInInstanceOfCheck, context);
+    var_result.Bind(result);
+    Goto(&return_result);
+  }
+
+  Bind(&return_true);
+  var_result.Bind(TrueConstant());
+  Goto(&return_result);
+
+  Bind(&return_false);
+  var_result.Bind(FalseConstant());
+  Goto(&return_result);
+
+  Bind(&return_result);
+  return var_result.value();
 }
 
 Node* CodeStubAssembler::NumberInc(Node* value) {

src/code-stub-assembler.h

@@ -33,6 +33,7 @@ enum class PrimitiveType { kBoolean, kNumber, kString, kSymbol };
   V(FixedCOWArrayMap, FixedCOWArrayMap)               \
   V(FixedDoubleArrayMap, FixedDoubleArrayMap)         \
   V(FunctionTemplateInfoMap, FunctionTemplateInfoMap) \
+  V(has_instance_symbol, HasInstanceSymbol)           \
   V(HeapNumberMap, HeapNumberMap)                     \
   V(NoClosuresCellMap, NoClosuresCellMap)             \
   V(OneClosureCellMap, OneClosureCellMap)             \
@@ -698,6 +699,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   Node* IsJSReceiver(Node* object);
   Node* IsMap(Node* object);
   Node* IsCallableMap(Node* map);
+  Node* IsCallable(Node* object);
   Node* IsBoolean(Node* object);
   Node* IsName(Node* object);
   Node* IsSymbol(Node* object);

src/full-codegen/arm/full-codegen-arm.cc

@@ -2658,6 +2658,7 @@ void FullCodeGenerator::VisitCompareOperation(CompareOperation* expr) {
       SetExpressionPosition(expr);
       PopOperand(r1);
       __ Call(isolate()->builtins()->InstanceOf(), RelocInfo::CODE_TARGET);
+      RestoreContext();
       PrepareForBailoutBeforeSplit(expr, false, NULL, NULL);
       __ CompareRoot(r0, Heap::kTrueValueRootIndex);
       Split(eq, if_true, if_false, fall_through);

src/full-codegen/arm64/full-codegen-arm64.cc

@@ -2634,6 +2634,7 @@ void FullCodeGenerator::VisitCompareOperation(CompareOperation* expr) {
       SetExpressionPosition(expr);
       PopOperand(x1);
       __ Call(isolate()->builtins()->InstanceOf(), RelocInfo::CODE_TARGET);
+      RestoreContext();
       PrepareForBailoutBeforeSplit(expr, false, NULL, NULL);
       __ CompareRoot(x0, Heap::kTrueValueRootIndex);
       Split(eq, if_true, if_false, fall_through);

src/full-codegen/ia32/full-codegen-ia32.cc

@@ -2592,6 +2592,7 @@ void FullCodeGenerator::VisitCompareOperation(CompareOperation* expr) {
       SetExpressionPosition(expr);
       PopOperand(edx);
       __ Call(isolate()->builtins()->InstanceOf(), RelocInfo::CODE_TARGET);
+      RestoreContext();
       PrepareForBailoutBeforeSplit(expr, false, NULL, NULL);
       __ cmp(eax, isolate()->factory()->true_value());
       Split(equal, if_true, if_false, fall_through);

src/full-codegen/mips/full-codegen-mips.cc

@@ -2685,6 +2685,7 @@ void FullCodeGenerator::VisitCompareOperation(CompareOperation* expr) {
       __ mov(a0, result_register());
       PopOperand(a1);
       __ Call(isolate()->builtins()->InstanceOf(), RelocInfo::CODE_TARGET);
+      RestoreContext();
       PrepareForBailoutBeforeSplit(expr, false, NULL, NULL);
       __ LoadRoot(at, Heap::kTrueValueRootIndex);
       Split(eq, v0, Operand(at), if_true, if_false, fall_through);

src/full-codegen/mips64/full-codegen-mips64.cc

@@ -2686,6 +2686,7 @@ void FullCodeGenerator::VisitCompareOperation(CompareOperation* expr) {
       __ mov(a0, result_register());
       PopOperand(a1);
       __ Call(isolate()->builtins()->InstanceOf(), RelocInfo::CODE_TARGET);
+      RestoreContext();
       PrepareForBailoutBeforeSplit(expr, false, NULL, NULL);
       __ LoadRoot(a4, Heap::kTrueValueRootIndex);
       Split(eq, v0, Operand(a4), if_true, if_false, fall_through);

src/full-codegen/ppc/full-codegen-ppc.cc

@@ -2671,6 +2671,7 @@ void FullCodeGenerator::VisitCompareOperation(CompareOperation* expr) {
       SetExpressionPosition(expr);
       PopOperand(r4);
       __ Call(isolate()->builtins()->InstanceOf(), RelocInfo::CODE_TARGET);
+      RestoreContext();
       PrepareForBailoutBeforeSplit(expr, false, NULL, NULL);
       __ CompareRoot(r3, Heap::kTrueValueRootIndex);
       Split(eq, if_true, if_false, fall_through);

src/full-codegen/s390/full-codegen-s390.cc

@@ -2611,6 +2611,7 @@ void FullCodeGenerator::VisitCompareOperation(CompareOperation* expr) {
       SetExpressionPosition(expr);
       PopOperand(r3);
       __ Call(isolate()->builtins()->InstanceOf(), RelocInfo::CODE_TARGET);
+      RestoreContext();
       PrepareForBailoutBeforeSplit(expr, false, NULL, NULL);
       __ CompareRoot(r2, Heap::kTrueValueRootIndex);
       Split(eq, if_true, if_false, fall_through);

src/full-codegen/x64/full-codegen-x64.cc

@@ -2576,6 +2576,7 @@ void FullCodeGenerator::VisitCompareOperation(CompareOperation* expr) {
       SetExpressionPosition(expr);
       PopOperand(rdx);
       __ Call(isolate()->builtins()->InstanceOf(), RelocInfo::CODE_TARGET);
+      RestoreContext();
       PrepareForBailoutBeforeSplit(expr, false, NULL, NULL);
       __ CompareRoot(rax, Heap::kTrueValueRootIndex);
       Split(equal, if_true, if_false, fall_through);

src/full-codegen/x87/full-codegen-x87.cc

@@ -2582,6 +2582,7 @@ void FullCodeGenerator::VisitCompareOperation(CompareOperation* expr) {
       SetExpressionPosition(expr);
       PopOperand(edx);
       __ Call(isolate()->builtins()->InstanceOf(), RelocInfo::CODE_TARGET);
+      RestoreContext();
       PrepareForBailoutBeforeSplit(expr, false, NULL, NULL);
       __ cmp(eax, isolate()->factory()->true_value());
       Split(equal, if_true, if_false, fall_through);

src/heap/heap.cc

@@ -2807,10 +2807,6 @@ void Heap::CreateInitialObjects() {
   cell->set_value(the_hole_value());
   set_empty_property_cell(*cell);
 
-  cell = factory->NewPropertyCell();
-  cell->set_value(Smi::FromInt(Isolate::kProtectorValid));
-  set_has_instance_protector(*cell);
-
   cell = factory->NewPropertyCell();
   cell->set_value(Smi::FromInt(Isolate::kProtectorValid));
   set_array_iterator_protector(*cell);

src/heap/heap.h

@@ -167,7 +167,6 @@ using v8::MemoryPressureLevel;
   /* Protectors */                                                             \
   V(PropertyCell, array_protector, ArrayProtector)                             \
   V(Cell, is_concat_spreadable_protector, IsConcatSpreadableProtector)         \
-  V(PropertyCell, has_instance_protector, HasInstanceProtector)                \
   V(Cell, species_protector, SpeciesProtector)                                 \
   V(PropertyCell, string_length_protector, StringLengthProtector)              \
   V(Cell, fast_array_iteration_protector, FastArrayIterationProtector)         \

src/isolate-inl.h

@@ -133,11 +133,6 @@ bool Isolate::IsArraySpeciesLookupChainIntact() {
          Smi::cast(species_cell->value())->value() == kProtectorValid;
 }
 
-bool Isolate::IsHasInstanceLookupChainIntact() {
-  PropertyCell* has_instance_cell = heap()->has_instance_protector();
-  return has_instance_cell->value() == Smi::FromInt(kProtectorValid);
-}
-
 bool Isolate::IsStringLengthOverflowIntact() {
   PropertyCell* string_length_cell = heap()->string_length_protector();
   return string_length_cell->value() == Smi::FromInt(kProtectorValid);

src/isolate.cc

@@ -3073,15 +3073,6 @@ void Isolate::UpdateArrayProtectorOnSetElement(Handle<JSObject> object) {
       handle(Smi::FromInt(kProtectorInvalid), this));
 }
 
-void Isolate::InvalidateHasInstanceProtector() {
-  DCHECK(factory()->has_instance_protector()->value()->IsSmi());
-  DCHECK(IsHasInstanceLookupChainIntact());
-  PropertyCell::SetValueWithInvalidation(
-      factory()->has_instance_protector(),
-      handle(Smi::FromInt(kProtectorInvalid), this));
-  DCHECK(!IsHasInstanceLookupChainIntact());
-}
-
 void Isolate::InvalidateIsConcatSpreadableProtector() {
   DCHECK(factory()->is_concat_spreadable_protector()->value()->IsSmi());
   DCHECK(IsIsConcatSpreadableLookupChainIntact());

src/isolate.h

@@ -996,7 +996,6 @@ class Isolate {
 
   bool IsFastArrayConstructorPrototypeChainIntact();
   inline bool IsArraySpeciesLookupChainIntact();
-  inline bool IsHasInstanceLookupChainIntact();
   bool IsIsConcatSpreadableLookupChainIntact();
   bool IsIsConcatSpreadableLookupChainIntact(JSReceiver* receiver);
   inline bool IsStringLengthOverflowIntact();
@@ -1023,7 +1022,6 @@ class Isolate {
     UpdateArrayProtectorOnSetElement(object);
   }
   void InvalidateArraySpeciesProtector();
-  void InvalidateHasInstanceProtector();
   void InvalidateIsConcatSpreadableProtector();
   void InvalidateStringLengthOverflowProtector();
   void InvalidateArrayIteratorProtector();

src/lookup.cc

@@ -191,9 +191,6 @@ void LookupIterator::InternalUpdateProtector() {
   } else if (*name_ == heap()->is_concat_spreadable_symbol()) {
     if (!isolate_->IsIsConcatSpreadableLookupChainIntact()) return;
     isolate_->InvalidateIsConcatSpreadableProtector();
-  } else if (*name_ == heap()->has_instance_symbol()) {
-    if (!isolate_->IsHasInstanceLookupChainIntact()) return;
-    isolate_->InvalidateHasInstanceProtector();
   } else if (*name_ == heap()->iterator_symbol()) {
     if (!isolate_->IsArrayIteratorLookupChainIntact()) return;
     if (holder_->IsJSArray()) {

src/runtime/runtime-internal.cc

@@ -225,6 +225,18 @@ RUNTIME_FUNCTION(Runtime_ThrowSymbolIteratorInvalid) {
       isolate, NewTypeError(MessageTemplate::kSymbolIteratorInvalid));
 }
 
+RUNTIME_FUNCTION(Runtime_ThrowNonCallableInInstanceOfCheck) {
+  HandleScope scope(isolate);
+  THROW_NEW_ERROR_RETURN_FAILURE(
+      isolate, NewTypeError(MessageTemplate::kNonCallableInInstanceOfCheck));
+}
+
+RUNTIME_FUNCTION(Runtime_ThrowNonObjectInInstanceOfCheck) {
+  HandleScope scope(isolate);
+  THROW_NEW_ERROR_RETURN_FAILURE(
+      isolate, NewTypeError(MessageTemplate::kNonObjectInInstanceOfCheck));
+}
+
 RUNTIME_FUNCTION(Runtime_ThrowNotGeneric) {
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());

src/runtime/runtime.h

@@ -319,6 +319,8 @@ namespace internal {
   F(ThrowInvalidStringLength, 0, 1)                 \
   F(ThrowIteratorResultNotAnObject, 1, 1)           \
   F(ThrowSymbolIteratorInvalid, 0, 1)               \
+  F(ThrowNonCallableInInstanceOfCheck, 0, 1)        \
+  F(ThrowNonObjectInInstanceOfCheck, 0, 1)          \
   F(ThrowNotGeneric, 1, 1)                          \
   F(ThrowReferenceError, 1, 1)                      \
   F(ThrowStackOverflow, 0, 1)                       \

test/mjsunit/regress/regress-v8-5958.js

+// Copyright 2017 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
+
+class A {}
+class B {}
+
+A.__proto__ = new Proxy(A.__proto__, {
+  get: function (target, property, receiver) {
+    if (property === Symbol.hasInstance) throw new B;
+  }
+});
+
+assertThrows(() => (new A) instanceof A, B);