From 3f2e5de43d7b2154f9227ba822e3012fa9cc2296 Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Thu, 11 Nov 2010 10:06:56 -0800 Subject: [PATCH] Bug 586482 - arguments.callee.caller not equal to proto-delegated joined function object method (r=igor). --- js/src/jsfun.cpp | 101 ++++++++++-------- js/src/jsobj.cpp | 1 + js/src/tests/js1_8_5/regress/jstests.list | 1 + .../tests/js1_8_5/regress/regress-586482.js | 25 +++++ 4 files changed, 81 insertions(+), 47 deletions(-) create mode 100644 js/src/tests/js1_8_5/regress/regress-586482.js diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index b7b15a52ff3..8ce27c3e42d 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -1406,60 +1406,67 @@ 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())); + 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. + * No point worrying about anything above a non-native object + * on the |this| object's prototype chain, as a non-native is + * responsible for handling its entire prototype chain. */ - 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()) + break; - if (IsFunctionObject(v, &clone) && - GET_FUNCTION_PRIVATE(cx, clone) == fun && - clone->hasMethodObj(*thisp)) { - JS_ASSERT(clone != &funobj); - *vp = v; - calleeValue().setObject(*clone); + const Shape *shape = thisp->nativeLookup(ATOM_TO_JSID(fun->methodAtom())); + if (shape) { + if (thisp->hasMethodBarrier()) { + /* + * 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); - return true; - } + 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 control flow reaches here, 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. + * There are not any properties left referring to fun. + */ + JSObject *newfunobj = CloneFunctionObject(cx, fun, fun->getParent()); + if (!newfunobj) + return false; + newfunobj->setMethodObj(*thisp); + calleeValue().setObject(*newfunobj); + } + return true; + } + } while ((thisp = thisp->getProto()) != NULL); } } diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index f9a9179a7ae..33f9e81d236 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -4872,6 +4872,7 @@ js_LookupPropertyWithFlagsInline(JSContext *cx, JSObject *obj, jsid id, uintN fl if (!proto->isNative()) { if (!proto->lookupProperty(cx, id, objp, propp)) return -1; + JS_ASSERT_IF(*propp, !(*objp)->isNative()); return protoIndex + 1; } diff --git a/js/src/tests/js1_8_5/regress/jstests.list b/js/src/tests/js1_8_5/regress/jstests.list index 0eb1e88ed76..f93c2e17375 100644 --- a/js/src/tests/js1_8_5/regress/jstests.list +++ b/js/src/tests/js1_8_5/regress/jstests.list @@ -31,6 +31,7 @@ script regress-577648-1.js script regress-577648-2.js script regress-583429.js script regress-584355.js +script regress-586482.js script regress-588339.js script regress-yarr-regexp.js script regress-592217.js diff --git a/js/src/tests/js1_8_5/regress/regress-586482.js b/js/src/tests/js1_8_5/regress/regress-586482.js new file mode 100644 index 00000000000..c236c74d440 --- /dev/null +++ b/js/src/tests/js1_8_5/regress/regress-586482.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");