Bug 1106982 - Stop doing script/pc lookup in GetNonexistentProperty if extra warnings are disabled. r=jorendorff

js/src/jit-test/tests/basic/bug1106982.js

@@ -0,0 +1,13 @@
+var x = "wrong";
+var t = {x: "x"};
+var p = new Proxy(t, {
+    has(t, id) {
+        var found = id in t;
+        delete t[id];
+        return found;
+    },
+    get(t, id) { return t[id]; }
+});
+with (p)
+    x += " x";
+assertEq(t.x, "undefined x");

js/src/jit/BaselineIC.cpp

@@ -6823,8 +6823,13 @@ ComputeGetPropResult(JSContext *cx, BaselineFrame *frame, JSOp op, HandlePropert
             return false;
 
         RootedId id(cx, NameToId(name));
-        if (!GetProperty(cx, obj, obj, id, res))
-            return false;
+        if (op == JSOP_GETXPROP) {
+            if (!GetPropertyForNameLookup(cx, obj, id, res))
+                return false;
+        } else {
+            if (!GetProperty(cx, obj, obj, id, res))
+                return false;
+        }
 
 #if JS_HAS_NO_SUCH_METHOD
         // Handle objects with __noSuchMethod__.

js/src/jscntxtinlines.h

@@ -460,8 +460,11 @@ JSContext::currentScript(jsbytecode **ppc,
     if (act->isJit()) {
         JSScript *script = nullptr;
         js::jit::GetPcScript(const_cast<JSContext *>(this), &script, ppc);
-        if (!allowCrossCompartment && script->compartment() != compartment())
+        if (!allowCrossCompartment && script->compartment() != compartment()) {
+            if (ppc)
+                *ppc = nullptr;
             return nullptr;
+        }
         return script;
     }
 

js/src/vm/Interpreter.cpp

@@ -2411,11 +2411,9 @@ CASE(JSOP_THIS)
 END_CASE(JSOP_THIS)
 
 CASE(JSOP_GETPROP)
-CASE(JSOP_GETXPROP)
 CASE(JSOP_LENGTH)
 CASE(JSOP_CALLPROP)
 {
-
     MutableHandleValue lval = REGS.stackHandleAt(-1);
     if (!GetPropertyOperation(cx, REGS.fp(), script, REGS.pc, lval, lval))
         goto error;
@@ -2425,6 +2423,21 @@ CASE(JSOP_CALLPROP)
 }
 END_CASE(JSOP_GETPROP)
 
+CASE(JSOP_GETXPROP)
+{
+    RootedObject &obj = rootObject0;
+    obj = &REGS.sp[-1].toObject();
+    RootedId &id = rootId0;
+    id = NameToId(script->getName(REGS.pc));
+    MutableHandleValue rval = REGS.stackHandleAt(-1);
+    if (!GetPropertyForNameLookup(cx, obj, id, rval))
+        goto error;
+
+    TypeScript::Monitor(cx, script, REGS.pc, rval);
+    assertSameCompartmentDebugOnly(cx, rval);
+}
+END_CASE(JSOP_GETXPROP)
+
 CASE(JSOP_SETINTRINSIC)
 {
     HandleValue value = REGS.stackHandleAt(-1);

js/src/vm/NativeObject.cpp

@@ -1733,6 +1733,8 @@ Detecting(JSContext *cx, JSScript *script, jsbytecode *pc)
     return false;
 }
 
+enum IsNameLookup { NotNameLookup = false, NameLookup = true };
+
 /*
  * Finish getting the property `receiver[id]` after looking at every object on
  * the prototype chain and not finding any such property.
@@ -1752,7 +1754,7 @@ Detecting(JSContext *cx, JSScript *script, jsbytecode *pc)
  */
 static bool
 GetNonexistentProperty(JSContext *cx, HandleNativeObject obj, HandleId id,
-                       HandleObject receiver, MutableHandleValue vp)
+                       HandleObject receiver, IsNameLookup nameLookup, MutableHandleValue vp)
 {
     vp.setUndefined();
 
@@ -1768,12 +1770,7 @@ GetNonexistentProperty(JSContext *cx, HandleNativeObject obj, HandleId id,
     }
 
     // If we are doing a name lookup, this is a ReferenceError.
-    jsbytecode *pc = nullptr;
-    RootedScript script(cx, cx->currentScript(&pc));
-    if (!pc)
-        return true;
-    JSOp op = (JSOp) *pc;
-    if (op == JSOP_GETXPROP) {
+    if (nameLookup) {
         JSAutoByteString printable;
         if (js_ValueToPrintable(cx, IdToValue(id), &printable))
             js_ReportIsNotDefined(cx, printable.ptr());
@@ -1785,11 +1782,19 @@ GetNonexistentProperty(JSContext *cx, HandleNativeObject obj, HandleId id,
     //
     // Don't warn if extra warnings not enabled or for random getprop
     // operations.
-    if (!cx->compartment()->options().extraWarnings(cx) || (op != JSOP_GETPROP && op != JSOP_GETELEM))
+    if (!cx->compartment()->options().extraWarnings(cx))
+        return true;
+
+    jsbytecode *pc;
+    RootedScript script(cx, cx->currentScript(&pc));
+    if (!script)
+        return true;
+
+    if (*pc != JSOP_GETPROP && *pc != JSOP_GETELEM)
         return true;
 
     // Don't warn repeatedly for the same script.
-    if (!script || script->warnedAboutUndefinedProp())
+    if (script->warnedAboutUndefinedProp())
         return true;
 
     // Don't warn in self-hosted code (where the further presence of
@@ -1803,7 +1808,7 @@ GetNonexistentProperty(JSContext *cx, HandleNativeObject obj, HandleId id,
         return true;
 
     // Do not warn about tests like (obj[prop] == undefined).
-    pc += js_CodeSpec[op].length;
+    pc += js_CodeSpec[*pc].length;
     if (Detecting(cx, script, pc))
         return true;
 
@@ -1819,7 +1824,7 @@ GetNonexistentProperty(JSContext *cx, HandleNativeObject obj, HandleId id,
 /* The NoGC version of GetNonexistentProperty, present only to make types line up. */
 bool
 GetNonexistentProperty(JSContext *cx, NativeObject *obj, jsid id, JSObject *receiver,
-                       FakeMutableHandle<Value> vp)
+                       IsNameLookup nameLookup, FakeMutableHandle<Value> vp)
 {
     return false;
 }
@@ -1846,6 +1851,7 @@ NativeGetPropertyInline(JSContext *cx,
                         typename MaybeRooted<NativeObject*, allowGC>::HandleType obj,
                         typename MaybeRooted<JSObject*, allowGC>::HandleType receiver,
                         typename MaybeRooted<jsid, allowGC>::HandleType id,
+                        IsNameLookup nameLookup,
                         typename MaybeRooted<Value, allowGC>::MutableHandleType vp)
 {
     typename MaybeRooted<NativeObject*, allowGC>::RootType pobj(cx, obj);
@@ -1882,7 +1888,7 @@ NativeGetPropertyInline(JSContext *cx,
         // Step 4.c. The spec algorithm simply returns undefined if proto is
         // null, but see the comment on GetNonexistentProperty.
         if (!proto)
-            return GetNonexistentProperty(cx, obj, id, receiver, vp);
+            return GetNonexistentProperty(cx, obj, id, receiver, nameLookup, vp);
 
         // Step 4.d. If the prototype is also native, this step is a
         // recursive tail call, and we don't need to go through all the
@@ -1900,16 +1906,23 @@ bool
 js::NativeGetProperty(JSContext *cx, HandleNativeObject obj, HandleObject receiver, HandleId id,
                       MutableHandleValue vp)
 {
-    return NativeGetPropertyInline<CanGC>(cx, obj, receiver, id, vp);
+    return NativeGetPropertyInline<CanGC>(cx, obj, receiver, id, NotNameLookup, vp);
 }
 
 bool
 js::NativeGetPropertyNoGC(JSContext *cx, NativeObject *obj, JSObject *receiver, jsid id, Value *vp)
 {
     AutoAssertNoException noexc(cx);
-    return NativeGetPropertyInline<NoGC>(cx, obj, receiver, id, vp);
+    return NativeGetPropertyInline<NoGC>(cx, obj, receiver, id, NotNameLookup, vp);
 }
 
+bool
+js::GetPropertyForNameLookup(JSContext *cx, HandleObject obj, HandleId id, MutableHandleValue vp)
+{
+    if (GetPropertyOp op = obj->getOps()->getProperty)
+        return op(cx, obj, obj, id, vp);
+    return NativeGetPropertyInline<CanGC>(cx, obj.as<NativeObject>(), obj, id, NameLookup, vp);
+}
 
 /*** [[Set]] *************************************************************************************/
 

js/src/vm/NativeObject.h

@@ -1367,6 +1367,9 @@ HasDataProperty(JSContext *cx, NativeObject *obj, PropertyName *name, Value *vp)
     return HasDataProperty(cx, obj, NameToId(name), vp);
 }
 
+extern bool
+GetPropertyForNameLookup(JSContext *cx, HandleObject obj, HandleId id, MutableHandleValue vp);
+
 } /* namespace js */
 
 template <>