Backed out changeset 4b4511cf734e (bug 1492920) for landing on both autoland and inbound.
authorMihai Alexandru Michis <malexandru@mozilla.com>
Thu, 22 Aug 2019 22:37:58 +0300
changeset 553321 2992e31e7b7e838d5baf9501839ddaa2fbe9135f
parent 553320 4b4511cf734e381d12dc99f22efc962cd8ba7d3c
child 553322 8947c1aea0811b9ba7b1bc426114b2a64f07fd48
push id2165
push userffxbld-merge
push dateMon, 14 Oct 2019 16:30:58 +0000
treeherdermozilla-release@0eae18af659f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1492920
milestone70.0a1
backs out4b4511cf734e381d12dc99f22efc962cd8ba7d3c
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 4b4511cf734e (bug 1492920) for landing on both autoland and inbound.
js/src/jit-test/tests/basic/bug1492920.js
js/src/jit/CacheIR.cpp
js/src/vm/JSFunction.cpp
js/src/vm/JSFunction.h
deleted file mode 100644
--- a/js/src/jit-test/tests/basic/bug1492920.js
+++ /dev/null
@@ -1,41 +0,0 @@
-var N = 20;
-var k = 15;  // k < N
-
-/* test 1: insertion of new blank object in ctor.__proto__ chain */
-
-function C() {}
-C.__proto__ = Object.create(Function.prototype);
-
-for (var i = 0; i < N; i++) {
-	var o = new C();
-	assertEq(o instanceof C, true);
-}
-
-/* test 2: overriding of @@hasInstance on the proto chain, partway
- * through execution (should trigger a guard) */
-
-function D() {}
-
-for (var i = 0; i < N; i++) {
-    var o = new D();
-    if (i == k) {
-        D.__proto__ = {[Symbol.hasInstance]() { return false; }};
-    }
-    assertEq(o instanceof D, i < k);
-}
-
-/* test 3: overriding of @@hasInstance on an intermediate object in the proto
- * chain */
-
-function E() {}
-
-E.__proto__ = Object.create(Object.create(Object.create(Function.prototype)));
-var intermediateProto = E.__proto__.__proto__;
-
-for (var i = 0; i < N; i++) {
-  var o = new E;
-  if (i == k) {
-    intermediateProto.__proto__ = {[Symbol.hasInstance]() { return false; }};
-  }
-  assertEq(o instanceof E, i < k);
-}
--- a/js/src/jit/CacheIR.cpp
+++ b/js/src/jit/CacheIR.cpp
@@ -4411,45 +4411,32 @@ AttachDecision InstanceOfIRGenerator::tr
 
   HandleFunction fun = rhsObj_.as<JSFunction>();
 
   if (fun->isBoundFunction()) {
     trackAttached(IRGenerator::NotAttached);
     return AttachDecision::NoAction;
   }
 
-  // Look up the @@hasInstance property, and check that Function.__proto__ is
-  // the property holder, and that no object further down the prototype chain
-  // (including this function) has shadowed it; together with the fact that
-  // Function.__proto__[@@hasInstance] is immutable, this ensures that the
-  // hasInstance hook will not change without the need to guard on the actual
-  // property value.
-  PropertyResult hasInstanceProp;
-  JSObject* hasInstanceHolder = nullptr;
-  jsid hasInstanceID = SYMBOL_TO_JSID(cx_->wellKnownSymbols().hasInstance);
-  if (!LookupPropertyPure(cx_, fun, hasInstanceID, &hasInstanceHolder,
-                          &hasInstanceProp) ||
-      !hasInstanceProp.isFound() || hasInstanceProp.isNonNativeProperty()) {
+  // If the user has supplied their own @@hasInstance method we shouldn't
+  // clobber it.
+  if (!js::FunctionHasDefaultHasInstance(fun, cx_->wellKnownSymbols())) {
+    trackAttached(IRGenerator::NotAttached);
+    return AttachDecision::NoAction;
+  }
+
+  // Refuse to optimize any function whose [[Prototype]] isn't
+  // Function.prototype.
+  if (!fun->hasStaticPrototype() || fun->hasUncacheableProto()) {
     trackAttached(IRGenerator::NotAttached);
     return AttachDecision::NoAction;
   }
 
   Value funProto = cx_->global()->getPrototype(JSProto_Function);
-  if (hasInstanceHolder != &funProto.toObject()) {
-    trackAttached(IRGenerator::NotAttached);
-    return AttachDecision::NoAction;
-  }
-
-  // If the above succeeded, then these should be true about @@hasInstance,
-  // because the property on Function.__proto__ is an immutable data property:
-  MOZ_ASSERT(hasInstanceProp.shape()->isDataProperty());
-  MOZ_ASSERT(!hasInstanceProp.shape()->configurable());
-  MOZ_ASSERT(!hasInstanceProp.shape()->writable());
-
-  if (!IsCacheableProtoChain(fun, hasInstanceHolder)) {
+  if (!funProto.isObject() || fun->staticPrototype() != &funProto.toObject()) {
     trackAttached(IRGenerator::NotAttached);
     return AttachDecision::NoAction;
   }
 
   // Ensure that the function's prototype slot is the same.
   Shape* shape = fun->lookupPure(cx_->names().prototype);
   if (!shape || !shape->isDataProperty()) {
     trackAttached(IRGenerator::NotAttached);
@@ -4468,25 +4455,16 @@ AttachDecision InstanceOfIRGenerator::tr
 
   // Abstract Objects
   ValOperandId lhs(writer.setInputOperandId(0));
   ValOperandId rhs(writer.setInputOperandId(1));
 
   ObjOperandId rhsId = writer.guardToObject(rhs);
   writer.guardShape(rhsId, fun->lastProperty());
 
-  // Ensure that the shapes up the prototype chain for the RHS remain the same
-  // so that @@hasInstance is not shadowed by some intermediate prototype
-  // object.
-  if (hasInstanceHolder != fun) {
-    GeneratePrototypeGuards(writer, fun, hasInstanceHolder, rhsId);
-    ObjOperandId holderId = writer.loadObject(hasInstanceHolder);
-    TestMatchingHolder(writer, hasInstanceHolder, holderId);
-  }
-
   // Load prototypeObject into the cache -- consumed twice in the IC
   ObjOperandId protoId = writer.loadObject(prototypeObject);
   // Ensure that rhs[slot] == prototypeObject.
   writer.guardFunctionPrototype(rhsId, slot, protoId);
 
   // Needn't guard LHS is object, because the actual stub can handle that
   // and correctly return false.
   writer.loadInstanceOfObjectResult(lhs, protoId, slot);
--- a/js/src/vm/JSFunction.cpp
+++ b/js/src/vm/JSFunction.cpp
@@ -1021,24 +1021,16 @@ bool js::FunctionHasDefaultHasInstance(J
   Shape* shape = fun->lookupPure(id);
   if (shape) {
     if (!shape->isDataProperty()) {
       return false;
     }
     const Value hasInstance = fun->as<NativeObject>().getSlot(shape->slot());
     return IsNativeFunction(hasInstance, fun_symbolHasInstance);
   }
-
-#ifdef DEBUG
-  // The proto must be Function.__proto__. It has an immutable @@hasInstance
-  // property. (Callers should check this first.)
-  Value funProto = fun->global().getPrototype(JSProto_Function);
-  MOZ_ASSERT(fun->staticPrototype() == &funProto.toObject());
-#endif
-
   return true;
 }
 
 bool js::fun_toString(JSContext* cx, unsigned argc, Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
   MOZ_ASSERT(IsFunctionObject(args.calleev()));
 
   RootedObject obj(cx, ToObject(cx, args.thisv()));
--- a/js/src/vm/JSFunction.h
+++ b/js/src/vm/JSFunction.h
@@ -1092,19 +1092,16 @@ extern JSFunction* DefineFunction(
     JSContext* cx, HandleObject obj, HandleId id, JSNative native,
     unsigned nargs, unsigned flags,
     gc::AllocKind allocKind = gc::AllocKind::FUNCTION);
 
 extern bool fun_toString(JSContext* cx, unsigned argc, Value* vp);
 
 struct WellKnownSymbols;
 
-// Assumes that fun.__proto__ === Function.__proto__, i.e., does not check for
-// the case where a function with a non-default __proto__ has an overridden
-// @@hasInstance handler. Will assert if not.
 extern bool FunctionHasDefaultHasInstance(JSFunction* fun,
                                           const WellKnownSymbols& symbols);
 
 extern void ThrowTypeErrorBehavior(JSContext* cx);
 
 /*
  * Function extended with reserved slots for use by various kinds of functions.
  * Most functions do not have these extensions, but enough do that efficient