Fix joined method leak via arguments.callee.caller (586482, r=igor).

js/src/jsfun.cpp

@@ -1439,60 +1439,78 @@ JSStackFrame::getValidCalleeObject(JSContext *cx, Value *vp)
 
         if (&fun->compiledFunObj() == &funobj && fun->methodAtom()) {
             JSObject *thisp = &thisv.toObject();
-            JS_ASSERT(thisp->canHaveMethodBarrier());
-
-            if (thisp->hasMethodBarrier()) {
-                const Shape *shape = thisp->nativeLookup(ATOM_TO_JSID(fun->methodAtom()));
+            JSObject *first_barriered_thisp = NULL;
 
+            do {
                 /*
-                 * The method property might have been deleted while the method
-                 * barrier flag stuck, so we must lookup and test here.
-                 *
-                 * Two cases follow: the method barrier was not crossed yet, so
-                 * we cross it here; the method barrier *was* crossed, in which
-                 * case we must fetch and validate the cloned (unjoined) funobj
-                 * in the method property's slot.
-                 *
-                 * In either case we must allow for the method property to have
-                 * been replaced, or its value to have been overwritten.
+                 * While a non-native object is responsible for handling its
+                 * entire prototype chain, notable non-natives including dense
+                 * and typed arrays have native prototypes, so keep going.
                  */
-                if (shape) {
-                    if (shape->isMethod() && &shape->methodObject() == &funobj) {
-                        if (!thisp->methodReadBarrier(cx, *shape, vp))
-                            return false;
-                        calleeValue().setObject(vp->toObject());
-                        return true;
-                    }
-                    if (shape->hasSlot()) {
-                        Value v = thisp->getSlot(shape->slot);
-                        JSObject *clone;
+                if (!thisp->isNative())
+                    continue;
 
-                        if (IsFunctionObject(v, &clone) &&
-                            GET_FUNCTION_PRIVATE(cx, clone) == fun &&
-                            clone->hasMethodObj(*thisp)) {
-                            JS_ASSERT(clone != &funobj);
-                            *vp = v;
-                            calleeValue().setObject(*clone);
+                if (thisp->hasMethodBarrier()) {
+                    const Shape *shape = thisp->nativeLookup(ATOM_TO_JSID(fun->methodAtom()));
+                    if (shape) {
+                        /*
+                         * Two cases follow: the method barrier was not crossed
+                         * yet, so we cross it here; the method barrier *was*
+                         * crossed but after the call, in which case we fetch
+                         * and validate the cloned (unjoined) funobj from the
+                         * method property's slot.
+                         *
+                         * In either case we must allow for the method property
+                         * to have been replaced, or its value overwritten.
+                         */
+                        if (shape->isMethod() && &shape->methodObject() == &funobj) {
+                            if (!thisp->methodReadBarrier(cx, *shape, vp))
+                                return false;
+                            calleeValue().setObject(vp->toObject());
                             return true;
                         }
-                    }
-                }
 
-                /*
-                 * If control flows here, we can't find an already-existing
-                 * clone (or force to exist a fresh clone) created via thisp's
-                 * method read barrier, so we must clone fun and store it in
-                 * fp's callee to avoid re-cloning upon repeated foo.caller
-                 * access. It seems that there are no longer any properties
-                 * referring to fun.
-                 */
-                JSObject *newfunobj = CloneFunctionObject(cx, fun, fun->getParent());
-                if (!newfunobj)
-                    return false;
-                newfunobj->setMethodObj(*thisp);
-                calleeValue().setObject(*newfunobj);
+                        if (shape->hasSlot()) {
+                            Value v = thisp->getSlot(shape->slot);
+                            JSObject *clone;
+
+                            if (IsFunctionObject(v, &clone) &&
+                                GET_FUNCTION_PRIVATE(cx, clone) == fun &&
+                                clone->hasMethodObj(*thisp)) {
+                                JS_ASSERT(clone != &funobj);
+                                *vp = v;
+                                calleeValue().setObject(*clone);
+                                return true;
+                            }
+                        }
+                    }
+
+                    if (!first_barriered_thisp)
+                        first_barriered_thisp = thisp;
+                }
+            } while ((thisp = thisp->getProto()) != NULL);
+
+            if (!first_barriered_thisp)
                 return true;
-            }
+
+            /*
+             * At this point, we couldn't find an already-existing clone (or
+             * force to exist a fresh clone) created via thisp's method read
+             * barrier, so we must clone fun and store it in fp's callee to
+             * avoid re-cloning upon repeated foo.caller access.
+             *
+             * This must mean the code in js_DeleteProperty could not find this
+             * stack frame on the stack when the method was deleted. We've lost
+             * track of the method, so we associate it with the first barriered
+             * object found starting from thisp on the prototype chain.
+             */
+            JSObject *newfunobj = CloneFunctionObject(cx, fun, fun->getParent());
+            if (!newfunobj)
+                return false;
+            newfunobj->setMethodObj(*first_barriered_thisp);
+            calleeValue().setObject(*newfunobj);
+            vp->setObject(*newfunobj);
+            return true;
         }
     }
 

js/src/jsobj.cpp

@@ -4882,6 +4882,21 @@ js_LookupPropertyWithFlagsInline(JSContext *cx, JSObject *obj, jsid id, uintN fl
         if (!proto->isNative()) {
             if (!proto->lookupProperty(cx, id, objp, propp))
                 return -1;
+#ifdef DEBUG
+            /*
+             * Non-native objects must have either non-native lookup results,
+             * or else native results from the non-native's prototype chain.
+             *
+             * See JSStackFrame::getValidCalleeObject, where we depend on this
+             * fact to force a prototype-delegated joined method accessed via
+             * arguments.callee through the delegating |this| object's method
+             * read barrier.
+             */
+            if (*propp && (*objp)->isNative()) {
+                while ((proto = proto->getProto()) != *objp)
+                    JS_ASSERT(proto);
+            }
+#endif
             return protoIndex + 1;
         }
 
@@ -5816,9 +5831,15 @@ js_DeleteProperty(JSContext *cx, JSObject *obj, jsid id, Value *rval, JSBool str
                     for (JSStackFrame *fp = cx->maybefp(); fp; fp = fp->prev()) {
                         if (fp->isFunctionFrame() &&
                             &fp->callee() == &fun->compiledFunObj() &&
-                            fp->thisValue().isObject() &&
-                            &fp->thisValue().toObject() == obj) {
-                            fp->calleeValue().setObject(*funobj);
+                            fp->thisValue().isObject())
+                        {
+                            JSObject *tmp = &fp->thisValue().toObject();
+                            do {
+                                if (tmp == obj) {
+                                    fp->calleeValue().setObject(*funobj);
+                                    break;
+                                }
+                            } while ((tmp = tmp->getProto()) != NULL);
                         }
                     }
                 }

js/src/jsobjinlines.h

@@ -562,8 +562,8 @@ inline bool
 JSObject::hasMethodObj(const JSObject& obj) const
 {
     return JSSLOT_FUN_METHOD_OBJ < numSlots() &&
-        getSlot(JSSLOT_FUN_METHOD_OBJ).isObject() &&
-        &getSlot(JSSLOT_FUN_METHOD_OBJ).toObject() == &obj;
+           getSlot(JSSLOT_FUN_METHOD_OBJ).isObject() &&
+           &getSlot(JSSLOT_FUN_METHOD_OBJ).toObject() == &obj;
 }
 
 inline void

js/src/tests/js1_8_5/regress/jstests.list

@@ -31,6 +31,11 @@ script regress-577648-1.js
 script regress-577648-2.js
 script regress-583429.js
 script regress-584355.js
+script regress-586482-1.js
+script regress-586482-2.js
+script regress-586482-3.js
+script regress-586482-4.js
+script regress-586482-5.js
 script regress-588339.js
 script regress-yarr-regexp.js
 script regress-592217.js

js/src/tests/js1_8_5/regress/regress-586482-1.js

@@ -0,0 +1,25 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+var expect = true;
+var actual;
+
+var checkCaller = function(me) {
+    var caller = arguments.callee.caller;
+    var callerIsMethod = (caller === me['doThing']);
+    actual = callerIsMethod;
+};
+
+var MyObj = function() {
+};
+
+MyObj.prototype.doThing = function() {
+    checkCaller(this);
+};
+
+(new MyObj()).doThing();
+
+reportCompare(expect, actual, "ok");

js/src/tests/js1_8_5/regress/regress-586482-2.js

@@ -0,0 +1,22 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+var expect = true;
+var actual;
+
+var checkCaller = function(me) {
+    var caller = arguments.callee.caller;
+    var callerIsMethod = (caller === me['doThing']);
+    actual = callerIsMethod;
+};
+
+Object.prototype.doThing = function() {
+    checkCaller(this);
+};
+
+["dense"].doThing();
+
+reportCompare(expect, actual, "ok");

js/src/tests/js1_8_5/regress/regress-586482-3.js

@@ -0,0 +1,27 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+var expect = true;
+var actual;
+
+var checkCaller = function(me) {
+    var f = me['doThing'];
+    delete MyObj.prototype['doThing'];
+    var caller = arguments.callee.caller;
+    var callerIsMethod = (f === caller);
+    actual = callerIsMethod;
+};
+
+var MyObj = function() {
+};
+
+MyObj.prototype.doThing = function() {
+    checkCaller(this);
+};
+
+(new MyObj()).doThing();
+
+reportCompare(expect, actual, "ok");

js/src/tests/js1_8_5/regress/regress-586482-4.js

@@ -0,0 +1,25 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+var expect, actual;
+
+var obj = {
+    f: function() {
+        expect = this.g;
+        actual = arguments.callee.caller;
+        print("Ok");
+    }
+};
+
+var obj2 = { __proto__: obj, g: function() { this.f(); }};
+
+var obj3 = { __proto__: obj2, h: function() { this.g(); }};
+
+var obj4 = { __proto__: obj3 }
+
+obj4.h();
+
+reportCompare(expect, actual, "ok");

js/src/tests/js1_8_5/regress/regress-586482-5.js

@@ -0,0 +1,18 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+function check() {
+    obj2.__proto__ = null;
+    return arguments.callee.caller;
+}
+
+var obj = { f: function() { check(); }};
+
+var obj2 = { __proto__: obj };
+
+obj2.f();
+
+reportCompare(0, 0, "ok");