Bug 1109924. Remove pointless DOM proxy handler guards that are covered by the shape guards we have already done. r=jandem
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 19 Feb 2015 14:35:15 -0500
changeset 257015 a286c01c617c1eb8a0c45e61d49b718995da19d2
parent 257014 06d87431a1aea07bfa4e8e0df47bddc6e9504215
child 257016 387ce2b80bd3169d660413ae194bf22b7b600757
push id4610
push userjlund@mozilla.com
push dateMon, 30 Mar 2015 18:32:55 +0000
treeherdermozilla-beta@4df54044d9ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1109924
milestone38.0a1
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
Bug 1109924. Remove pointless DOM proxy handler guards that are covered by the shape guards we have already done. r=jandem
js/src/jit/BaselineIC.cpp
js/src/jit/BaselineIC.h
js/src/jit/IonCaches.cpp
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -3114,39 +3114,29 @@ static void GetFixedOrDynamicSlotOffset(
 
 static JSObject *
 GetDOMProxyProto(JSObject *obj)
 {
     MOZ_ASSERT(IsCacheableDOMProxy(obj));
     return obj->getTaggedProto().toObjectOrNull();
 }
 
+// Callers are expected to have already guarded on the shape of the
+// object, which guarantees the object is a DOM proxy.
 static void
-GenerateDOMProxyChecks(JSContext *cx, MacroAssembler &masm, Register object,
-                       Address checkProxyHandlerAddr,
-                       Address *checkExpandoShapeAddr,
-                       Address *expandoAndGenerationAddr,
-                       Address *generationAddr,
-                       Register scratch,
-                       GeneralRegisterSet &domProxyRegSet,
-                       Label *checkFailed)
-{
-    // Guard the following:
-    //      1. The object is a DOMProxy.
-    //      2. The object does not have expando properties, or has an expando
-    //          which is known to not have the desired property.
-    Address handlerAddr(object, ProxyObject::offsetOfHandler());
-
-    // Check that object is a DOMProxy.
-    masm.loadPtr(checkProxyHandlerAddr, scratch);
-    masm.branchPtr(Assembler::NotEqual, handlerAddr, scratch, checkFailed);
-
-    // At this point, if not checking for an expando object, just return.
-    if (!checkExpandoShapeAddr)
-        return;
+CheckDOMProxyExpandoDoesNotShadow(JSContext *cx, MacroAssembler &masm, Register object,
+                                  const Address &checkExpandoShapeAddr,
+                                  Address *expandoAndGenerationAddr,
+                                  Address *generationAddr,
+                                  Register scratch,
+                                  GeneralRegisterSet &domProxyRegSet,
+                                  Label *checkFailed)
+{
+    // Guard that the object does not have expando properties, or has an expando
+    // which is known to not have the desired property.
 
     // For the remaining code, we need to reserve some registers to load a value.
     // This is ugly, but unavoidable.
     ValueOperand tempVal = domProxyRegSet.takeAnyValue();
     masm.pushValue(tempVal);
 
     Label failDOMProxyCheck;
     Label domProxyOk;
@@ -3173,17 +3163,17 @@ GenerateDOMProxyChecks(JSContext *cx, Ma
 
     // If the incoming object does not have an expando object then we're sure we're not
     // shadowing.
     masm.branchTestUndefined(Assembler::Equal, tempVal, &domProxyOk);
 
     // The reference object used to generate this check may not have had an
     // expando object at all, in which case the presence of a non-undefined
     // expando value in the incoming object is automatically a failure.
-    masm.loadPtr(*checkExpandoShapeAddr, scratch);
+    masm.loadPtr(checkExpandoShapeAddr, scratch);
     masm.branchPtr(Assembler::Equal, scratch, ImmPtr(nullptr), &failDOMProxyCheck);
 
     // Otherwise, ensure that the incoming object has an object for its expando value and that
     // the shape matches.
     masm.branchTestObject(Assembler::NotEqual, tempVal, &failDOMProxyCheck);
     Register objReg = masm.extractObject(tempVal, tempVal.scratchReg());
     masm.branchTestObjShape(Assembler::Equal, objReg, scratch, &domProxyOk);
 
@@ -7563,27 +7553,26 @@ ICGetPropCallDOMProxyNativeCompiler::gen
 
     // Unbox.
     Register objReg = masm.extractObject(R0, ExtractTemp0);
 
     // Shape guard.
     masm.loadPtr(Address(BaselineStubReg, ICGetProp_CallDOMProxyNative::offsetOfShape()), scratch);
     masm.branchTestObjShape(Assembler::NotEqual, objReg, scratch, &failure);
 
-    // Guard for ListObject.
+    // Guard that our expando object hasn't started shadowing this property.
     {
         GeneralRegisterSet domProxyRegSet(GeneralRegisterSet::All());
         domProxyRegSet.take(BaselineStubReg);
         domProxyRegSet.take(objReg);
         domProxyRegSet.take(scratch);
         Address expandoShapeAddr(BaselineStubReg, ICGetProp_CallDOMProxyNative::offsetOfExpandoShape());
-        GenerateDOMProxyChecks(
+        CheckDOMProxyExpandoDoesNotShadow(
                 cx, masm, objReg,
-                Address(BaselineStubReg, ICGetProp_CallDOMProxyNative::offsetOfProxyHandler()),
-                &expandoShapeAddr, expandoAndGenerationAddr, generationAddr,
+                expandoShapeAddr, expandoAndGenerationAddr, generationAddr,
                 scratch,
                 domProxyRegSet,
                 &failure);
     }
 
     Register holderReg = regs.takeAny();
     masm.loadPtr(Address(BaselineStubReg, ICGetProp_CallDOMProxyNative::offsetOfHolder()),
                  holderReg);
@@ -7655,22 +7644,22 @@ ICGetPropCallDOMProxyNativeCompiler::get
         generation = expandoAndGeneration->generation;
     }
 
     if (expandoVal.isObject())
         expandoShape = expandoVal.toObject().lastProperty();
 
     if (kind == ICStub::GetProp_CallDOMProxyNative) {
         return ICStub::New<ICGetProp_CallDOMProxyNative>(
-            space, getStubCode(), firstMonitorStub_, shape, proxy_->handler(),
+            space, getStubCode(), firstMonitorStub_, shape,
             expandoShape, holder_, holderShape, getter_, pcOffset_);
     }
 
     return ICStub::New<ICGetProp_CallDOMProxyWithGenerationNative>(
-        space, getStubCode(), firstMonitorStub_, shape, proxy_->handler(),
+        space, getStubCode(), firstMonitorStub_, shape,
         expandoAndGeneration, generation, expandoShape, holder_, holderShape, getter_,
         pcOffset_);
 }
 
 ICStub *
 ICGetProp_DOMProxyShadowed::Compiler::getStub(ICStubSpace *space)
 {
     RootedShape shape(cx, proxy_->lastProperty());
@@ -7705,32 +7694,18 @@ ICGetProp_DOMProxyShadowed::Compiler::ge
 
     // Unbox.
     Register objReg = masm.extractObject(R0, ExtractTemp0);
 
     // Shape guard.
     masm.loadPtr(Address(BaselineStubReg, ICGetProp_DOMProxyShadowed::offsetOfShape()), scratch);
     masm.branchTestObjShape(Assembler::NotEqual, objReg, scratch, &failure);
 
-    // Guard for ListObject.
-    {
-        GeneralRegisterSet domProxyRegSet(GeneralRegisterSet::All());
-        domProxyRegSet.take(BaselineStubReg);
-        domProxyRegSet.take(objReg);
-        domProxyRegSet.take(scratch);
-        GenerateDOMProxyChecks(
-                cx, masm, objReg,
-                Address(BaselineStubReg, ICGetProp_DOMProxyShadowed::offsetOfProxyHandler()),
-                /*expandoShapeAddr=*/nullptr,
-                /*expandoAndGenerationAddr=*/nullptr,
-                /*generationAddr=*/nullptr,
-                scratch,
-                domProxyRegSet,
-                &failure);
-    }
+    // No need to do any more guards; it's safe to call ProxyGet even
+    // if we've since stopped shadowing.
 
     // Call ProxyGet(JSContext *cx, HandleObject proxy, HandlePropertyName name, MutableHandleValue vp);
 
     // Push a stub frame so that we can perform a non-tail call.
     enterStubFrame(masm, scratch);
 
     // Push property name and proxy object.
     masm.loadPtr(Address(BaselineStubReg, ICGetProp_DOMProxyShadowed::offsetOfName()), scratch);
@@ -12113,26 +12088,24 @@ ICCall_ScriptedFunCall::Clone(ICStubSpac
                               ICCall_ScriptedFunCall &other)
 {
     return New<ICCall_ScriptedFunCall>(space, other.jitCode(), firstMonitorStub, other.pcOffset_);
 }
 
 ICGetPropCallDOMProxyNativeStub::ICGetPropCallDOMProxyNativeStub(Kind kind, JitCode *stubCode,
                                                                  ICStub *firstMonitorStub,
                                                                  Shape *shape,
-                                                                 const BaseProxyHandler *proxyHandler,
                                                                  Shape *expandoShape,
                                                                  JSObject *holder,
                                                                  Shape *holderShape,
                                                                  JSFunction *getter,
                                                                  uint32_t pcOffset)
 : ICGetPropCallGetter(kind, stubCode, firstMonitorStub, holder, holderShape,
                       getter, pcOffset),
     shape_(shape),
-    proxyHandler_(proxyHandler),
     expandoShape_(expandoShape)
 { }
 
 ICGetPropCallDOMProxyNativeCompiler::ICGetPropCallDOMProxyNativeCompiler(JSContext *cx,
                                                                          ICStub::Kind kind,
                                                                          ICStub *firstMonitorStub,
                                                                          Handle<ProxyObject*> proxy,
                                                                          HandleObject holder,
@@ -12150,28 +12123,28 @@ ICGetPropCallDOMProxyNativeCompiler::ICG
     MOZ_ASSERT(proxy_->handler()->family() == GetDOMProxyHandlerFamily());
 }
 
 /* static */ ICGetProp_CallDOMProxyNative *
 ICGetProp_CallDOMProxyNative::Clone(ICStubSpace *space, ICStub *firstMonitorStub,
                                     ICGetProp_CallDOMProxyNative &other)
 {
     return New<ICGetProp_CallDOMProxyNative>(space, other.jitCode(), firstMonitorStub,
-                                             other.shape_, other.proxyHandler_, other.expandoShape_,
+                                             other.shape_, other.expandoShape_,
                                              other.holder_, other.holderShape_, other.getter_,
                                              other.pcOffset_);
 }
 
 /* static */ ICGetProp_CallDOMProxyWithGenerationNative *
 ICGetProp_CallDOMProxyWithGenerationNative::Clone(ICStubSpace *space,
                                                   ICStub *firstMonitorStub,
                                                   ICGetProp_CallDOMProxyWithGenerationNative &other)
 {
     return New<ICGetProp_CallDOMProxyWithGenerationNative>(space, other.jitCode(), firstMonitorStub,
-                                                           other.shape_, other.proxyHandler_,
+                                                           other.shape_,
                                                            other.expandoAndGeneration_,
                                                            other.generation_,
                                                            other.expandoShape_, other.holder_,
                                                            other.holderShape_, other.getter_,
                                                            other.pcOffset_);
 }
 
 ICGetProp_DOMProxyShadowed::ICGetProp_DOMProxyShadowed(JitCode *stubCode,
--- a/js/src/jit/BaselineIC.h
+++ b/js/src/jit/BaselineIC.h
@@ -4472,58 +4472,52 @@ class ICGetProp_CallNativePrototype : pu
         }
     };
 };
 
 class ICGetPropCallDOMProxyNativeStub : public ICGetPropCallGetter
 {
   friend class ICStubSpace;
   protected:
-    // Shape of the DOMProxy
+    // Shape of the DOMProxy.  This enforces its class and hence its proxy handler.
     HeapPtrShape shape_;
 
-    // Proxy handler to check against.
-    const BaseProxyHandler *proxyHandler_;
-
     // Object shape of expected expando object. (nullptr if no expando object should be there)
     HeapPtrShape expandoShape_;
 
     ICGetPropCallDOMProxyNativeStub(ICStub::Kind kind, JitCode *stubCode,
                                     ICStub *firstMonitorStub, Shape *shape,
-                                    const BaseProxyHandler *proxyHandler, Shape *expandoShape,
+                                    Shape *expandoShape,
                                     JSObject *holder, Shape *holderShape,
                                     JSFunction *getter, uint32_t pcOffset);
 
   public:
     HeapPtrShape &shape() {
         return shape_;
     }
     HeapPtrShape &expandoShape() {
         return expandoShape_;
     }
     static size_t offsetOfShape() {
         return offsetof(ICGetPropCallDOMProxyNativeStub, shape_);
     }
-    static size_t offsetOfProxyHandler() {
-        return offsetof(ICGetPropCallDOMProxyNativeStub, proxyHandler_);
-    }
     static size_t offsetOfExpandoShape() {
         return offsetof(ICGetPropCallDOMProxyNativeStub, expandoShape_);
     }
 };
 
 class ICGetProp_CallDOMProxyNative : public ICGetPropCallDOMProxyNativeStub
 {
     friend class ICStubSpace;
     ICGetProp_CallDOMProxyNative(JitCode *stubCode, ICStub *firstMonitorStub, Shape *shape,
-                                 const BaseProxyHandler *proxyHandler, Shape *expandoShape,
+                                 Shape *expandoShape,
                                  JSObject *holder, Shape *holderShape,
                                  JSFunction *getter, uint32_t pcOffset)
       : ICGetPropCallDOMProxyNativeStub(ICStub::GetProp_CallDOMProxyNative, stubCode,
-                                        firstMonitorStub, shape, proxyHandler, expandoShape,
+                                        firstMonitorStub, shape, expandoShape,
                                         holder, holderShape, getter, pcOffset)
     {}
 
   public:
     static ICGetProp_CallDOMProxyNative *Clone(ICStubSpace *space,
                                                ICStub *firstMonitorStub,
                                                ICGetProp_CallDOMProxyNative &other);
 };
@@ -4531,23 +4525,23 @@ class ICGetProp_CallDOMProxyNative : pub
 class ICGetProp_CallDOMProxyWithGenerationNative : public ICGetPropCallDOMProxyNativeStub
 {
   protected:
     ExpandoAndGeneration *expandoAndGeneration_;
     uint32_t generation_;
 
   public:
     ICGetProp_CallDOMProxyWithGenerationNative(JitCode *stubCode, ICStub *firstMonitorStub,
-                                               Shape *shape, const BaseProxyHandler *proxyHandler,
+                                               Shape *shape,
                                                ExpandoAndGeneration *expandoAndGeneration,
                                                uint32_t generation, Shape *expandoShape,
                                                JSObject *holder, Shape *holderShape,
                                                JSFunction *getter, uint32_t pcOffset)
       : ICGetPropCallDOMProxyNativeStub(ICStub::GetProp_CallDOMProxyWithGenerationNative,
-                                        stubCode, firstMonitorStub, shape, proxyHandler,
+                                        stubCode, firstMonitorStub, shape,
                                         expandoShape, holder, holderShape, getter, pcOffset),
         expandoAndGeneration_(expandoAndGeneration),
         generation_(generation)
     {
     }
 
     static ICGetProp_CallDOMProxyWithGenerationNative *
     Clone(ICStubSpace *space, ICStub *firstMonitorStub,
--- a/js/src/jit/IonCaches.cpp
+++ b/js/src/jit/IonCaches.cpp
@@ -696,35 +696,26 @@ EmitLoadSlot(MacroAssembler &masm, Nativ
     } else {
         masm.loadPtr(Address(holderReg, NativeObject::offsetOfSlots()), scratchReg);
 
         Address addr(scratchReg, holder->dynamicSlotIndex(shape->slot()) * sizeof(Value));
         masm.loadTypedOrValue(addr, output);
     }
 }
 
+// Callers are expected to have already guarded on the shape of the
+// object, which guarantees the object is a DOM proxy.
 static void
-GenerateDOMProxyChecks(JSContext *cx, MacroAssembler &masm, JSObject *obj,
-                       PropertyName *name, Register object, Label *stubFailure,
-                       bool skipExpandoCheck = false)
+CheckDOMProxyExpandoDoesNotShadow(JSContext *cx, MacroAssembler &masm, JSObject *obj,
+                                  PropertyName *name, Register object, Label *stubFailure)
 {
     MOZ_ASSERT(IsCacheableDOMProxy(obj));
 
-    // Guard the following:
-    //      1. The object is a DOMProxy.
-    //      2. The object does not have expando properties, or has an expando
-    //          which is known to not have the desired property.
-    Address handlerAddr(object, ProxyObject::offsetOfHandler());
-
-    // Check that object is a DOMProxy.
-    masm.branchPtr(Assembler::NotEqual, handlerAddr,
-                   ImmPtr(obj->as<ProxyObject>().handler()), stubFailure);
-
-    if (skipExpandoCheck)
-        return;
+    // Guard that the object does not have expando properties, or has an expando
+    // which is known to not have the desired property.
 
     // For the remaining code, we need to reserve some registers to load a value.
     // This is ugly, but unvaoidable.
     RegisterSet domProxyRegSet(RegisterSet::All());
     domProxyRegSet.take(AnyRegister(object));
     ValueOperand tempVal = domProxyRegSet.takeValueOperand();
     masm.pushValue(tempVal);
 
@@ -1545,19 +1536,19 @@ GetPropertyIC::tryAttachDOMProxyShadowed
     RepatchStubAppender attacher(*this);
 
     // Guard on the shape of the object.
     attacher.branchNextStubOrLabel(masm, Assembler::NotEqual,
                                    Address(object(), JSObject::offsetOfShape()),
                                    ImmGCPtr(obj->lastProperty()),
                                    &failures);
 
-    // Make sure object is a DOMProxy
-    GenerateDOMProxyChecks(cx, masm, obj, name(), object(), &failures,
-                           /*skipExpandoCheck=*/true);
+    // No need for more guards: we know this is a DOM proxy, since the shape
+    // guard enforces a given JSClass, so just go ahead and emit the call to
+    // ProxyGet.
 
     if (!EmitCallProxyGet(cx, masm, attacher, name(), liveRegs_, object(), output(),
                           pc(), returnAddr))
     {
         return false;
     }
 
     // Success.
@@ -1614,18 +1605,18 @@ GetPropertyIC::tryAttachDOMProxyUnshadow
     RepatchStubAppender attacher(*this);
 
     // Guard on the shape of the object.
     attacher.branchNextStubOrLabel(masm, Assembler::NotEqual,
                                    Address(object(), JSObject::offsetOfShape()),
                                    ImmGCPtr(obj->lastProperty()),
                                    &failures);
 
-    // Make sure object is a DOMProxy proxy
-    GenerateDOMProxyChecks(cx, masm, obj, name, object(), &failures);
+    // Guard that our expando object hasn't started shadowing this property.
+    CheckDOMProxyExpandoDoesNotShadow(cx, masm, obj, name, object(), &failures);
 
     if (holder) {
         // Found the property on the prototype chain. Treat it like a native
         // getprop.
         Register scratchReg = output().valueReg().scratchReg();
         GeneratePrototypeGuards(cx, ion, masm, obj, holder, object(), scratchReg, &failures);
 
         // Rename scratch for clarity.
@@ -2233,19 +2224,19 @@ SetPropertyIC::attachDOMProxyShadowed(JS
     MacroAssembler masm(cx, ion, outerScript, profilerLeavePc_);
     RepatchStubAppender attacher(*this);
 
     // Guard on the shape of the object.
     masm.branchPtr(Assembler::NotEqual,
                    Address(object(), JSObject::offsetOfShape()),
                    ImmGCPtr(obj->lastProperty()), &failures);
 
-    // Make sure object is a DOMProxy
-    GenerateDOMProxyChecks(cx, masm, obj, name(), object(), &failures,
-                           /*skipExpandoCheck=*/true);
+    // No need for more guards: we know this is a DOM proxy, since the shape
+    // guard enforces a given JSClass, so just go ahead and emit the call to
+    // ProxySet.
 
     RootedId propId(cx, AtomToId(name()));
     if (!EmitCallProxySet(cx, masm, attacher, propId, liveRegs_, object(),
                           value(), returnAddr, strict()))
     {
         return false;
     }
 
@@ -2507,18 +2498,18 @@ SetPropertyIC::attachDOMProxyUnshadowed(
     MacroAssembler masm(cx, ion, outerScript, profilerLeavePc_);
     RepatchStubAppender attacher(*this);
 
     // Guard on the shape of the object.
     masm.branchPtr(Assembler::NotEqual,
                    Address(object(), JSObject::offsetOfShape()),
                    ImmGCPtr(obj->lastProperty()), &failures);
 
-    // Make sure object is a DOMProxy
-    GenerateDOMProxyChecks(cx, masm, obj, name(), object(), &failures);
+    // Guard that our expando object hasn't started shadowing this property.
+    CheckDOMProxyExpandoDoesNotShadow(cx, masm, obj, name(), object(), &failures);
 
     RootedPropertyName propName(cx, name());
     RootedObject holder(cx);
     RootedShape shape(cx);
     bool isSetter;
     if (!IsCacheableDOMProxyUnshadowedSetterCall(cx, obj, propName, &holder,
                                                  &shape, &isSetter))
     {