Bug 561359 - Change JSOP_LAMBDA to apply the method optimization deterministically. In particular, it no longer depends on whether enclosing Blocks have been reified. This prevents incorrect behavior and assertions when a JSOP_LAMBDA, JSOP_INITMETHOD pair apply the method optimization once, populating the property cache, but later the same JSOP_LAMBDA instruction does not (under the old code) apply the optimization. With this patch, if JSOP_LAMBDA pushes the uncloned function once, it always will. r=brendan.

This commit is contained in:
Jason Orendorff 2011-03-14 15:54:34 -05:00
parent f93c39ea90
commit efd9982808
10 changed files with 171 additions and 112 deletions

View File

@ -218,8 +218,7 @@ JSCompartment::wrap(JSContext *cx, Value *vp)
/*
* Wrappers should really be parented to the wrapped parent of the wrapped
* object, but in that case a wrapped global object would have a NULL
* parent without being a proper global object (JSCLASS_IS_GLOBAL). Instead
,
* parent without being a proper global object (JSCLASS_IS_GLOBAL). Instead,
* we parent all wrappers to the global object in their home compartment.
* This loses us some transparency, and is generally very cheesy.
*/

View File

@ -5588,86 +5588,84 @@ BEGIN_CASE(JSOP_LAMBDA)
/* do-while(0) so we can break instead of using a goto. */
do {
JSObject *parent;
if (FUN_NULL_CLOSURE(fun)) {
if (fun->joinable()) {
parent = &regs.fp->scopeChain();
if (obj->getParent() == parent) {
jsbytecode *pc2 = AdvanceOverBlockchainOp(regs.pc + JSOP_LAMBDA_LENGTH);
JSOp op2 = JSOp(*pc2);
jsbytecode *pc2 = AdvanceOverBlockchainOp(regs.pc + JSOP_LAMBDA_LENGTH);
JSOp op2 = JSOp(*pc2);
/*
* Optimize var obj = {method: function () { ... }, ...},
* this.method = function () { ... }; and other significant
* single-use-of-null-closure bytecode sequences.
*
* WARNING: code in TraceRecorder::record_JSOP_LAMBDA must
* match the optimization cases in the following code that
* break from the outer do-while(0).
*/
if (op2 == JSOP_INITMETHOD) {
/*
* Optimize var obj = {method: function () { ... }, ...},
* this.method = function () { ... }; and other significant
* single-use-of-null-closure bytecode sequences.
*
* WARNING: code in TraceRecorder::record_JSOP_LAMBDA must
* match the optimization cases in the following code that
* break from the outer do-while(0).
*/
if (op2 == JSOP_INITMETHOD) {
#ifdef DEBUG
const Value &lref = regs.sp[-1];
JS_ASSERT(lref.isObject());
JSObject *obj2 = &lref.toObject();
JS_ASSERT(obj2->getClass() == &js_ObjectClass);
const Value &lref = regs.sp[-1];
JS_ASSERT(lref.isObject());
JSObject *obj2 = &lref.toObject();
JS_ASSERT(obj2->getClass() == &js_ObjectClass);
#endif
fun->setMethodAtom(script->getAtom(GET_FULL_INDEX(pc2 - regs.pc)));
JS_FUNCTION_METER(cx, joinedinitmethod);
break;
}
if (op2 == JSOP_SETMETHOD) {
#ifdef DEBUG
op2 = JSOp(pc2[JSOP_SETMETHOD_LENGTH]);
JS_ASSERT(op2 == JSOP_POP || op2 == JSOP_POPV);
#endif
const Value &lref = regs.sp[-1];
if (lref.isObject() && lref.toObject().canHaveMethodBarrier()) {
fun->setMethodAtom(script->getAtom(GET_FULL_INDEX(pc2 - regs.pc)));
JS_FUNCTION_METER(cx, joinedinitmethod);
JS_FUNCTION_METER(cx, joinedsetmethod);
break;
}
} else if (fun->joinable()) {
if (op2 == JSOP_CALL) {
/*
* Array.prototype.sort and String.prototype.replace are
* optimized as if they are special form. We know that they
* won't leak the joined function object in obj, therefore
* we don't need to clone that compiler- created function
* object for identity/mutation reasons.
*/
int iargc = GET_ARGC(pc2);
if (op2 == JSOP_SETMETHOD) {
#ifdef DEBUG
op2 = JSOp(pc2[JSOP_SETMETHOD_LENGTH]);
JS_ASSERT(op2 == JSOP_POP || op2 == JSOP_POPV);
#endif
const Value &lref = regs.sp[-1];
if (lref.isObject() && lref.toObject().canHaveMethodBarrier()) {
fun->setMethodAtom(script->getAtom(GET_FULL_INDEX(pc2 - regs.pc)));
JS_FUNCTION_METER(cx, joinedsetmethod);
break;
}
} else if (fun->joinable()) {
if (op2 == JSOP_CALL) {
/*
* Array.prototype.sort and String.prototype.replace are
* optimized as if they are special form. We know that they
* won't leak the joined function object in obj, therefore
* we don't need to clone that compiler- created function
* object for identity/mutation reasons.
*/
int iargc = GET_ARGC(pc2);
/*
* Note that we have not yet pushed obj as the final argument,
* so regs.sp[1 - (iargc + 2)], and not regs.sp[-(iargc + 2)],
* is the callee for this JSOP_CALL.
*/
const Value &cref = regs.sp[1 - (iargc + 2)];
JSObject *callee;
/*
* Note that we have not yet pushed obj as the final argument,
* so regs.sp[1 - (iargc + 2)], and not regs.sp[-(iargc + 2)],
* is the callee for this JSOP_CALL.
*/
const Value &cref = regs.sp[1 - (iargc + 2)];
JSObject *callee;
if (IsFunctionObject(cref, &callee)) {
JSFunction *calleeFun = GET_FUNCTION_PRIVATE(cx, callee);
if (Native native = calleeFun->maybeNative()) {
if (iargc == 1 && native == array_sort) {
JS_FUNCTION_METER(cx, joinedsort);
break;
}
if (iargc == 2 && native == str_replace) {
JS_FUNCTION_METER(cx, joinedreplace);
break;
}
if (IsFunctionObject(cref, &callee)) {
JSFunction *calleeFun = GET_FUNCTION_PRIVATE(cx, callee);
if (Native native = calleeFun->maybeNative()) {
if (iargc == 1 && native == array_sort) {
JS_FUNCTION_METER(cx, joinedsort);
break;
}
if (iargc == 2 && native == str_replace) {
JS_FUNCTION_METER(cx, joinedreplace);
break;
}
}
} else if (op2 == JSOP_NULL) {
pc2 += JSOP_NULL_LENGTH;
op2 = JSOp(*pc2);
}
} else if (op2 == JSOP_NULL) {
pc2 += JSOP_NULL_LENGTH;
op2 = JSOp(*pc2);
if (op2 == JSOP_CALL && GET_ARGC(pc2) == 0) {
JS_FUNCTION_METER(cx, joinedmodulepat);
break;
}
if (op2 == JSOP_CALL && GET_ARGC(pc2) == 0) {
JS_FUNCTION_METER(cx, joinedmodulepat);
break;
}
}
}

View File

@ -313,7 +313,9 @@ bool
JSFunctionBox::joinable() const
{
return FUN_NULL_CLOSURE((JSFunction *) object) &&
!(tcflags & (TCF_FUN_USES_ARGUMENTS | TCF_FUN_USES_OWN_NAME));
(tcflags & (TCF_FUN_USES_ARGUMENTS |
TCF_FUN_USES_OWN_NAME |
TCF_COMPILE_N_GO)) == TCF_COMPILE_N_GO;
}
bool
@ -4732,8 +4734,6 @@ CloneParseTree(JSParseNode *opn, JSTreeContext *tc)
#endif /* JS_HAS_DESTRUCTURING */
extern const char js_with_statement_str[];
static JSParseNode *
ContainsStmt(JSParseNode *pn, TokenKind tt)
{

View File

@ -15556,7 +15556,7 @@ TraceRecorder::record_JSOP_LAMBDA()
* JSOP_INITMETHOD logic governing the early ARECORD_CONTINUE returns below
* must agree with the corresponding break-from-do-while(0) logic there.
*/
if (FUN_NULL_CLOSURE(fun) && FUN_OBJECT(fun)->getParent() == &cx->fp()->scopeChain()) {
if (fun->joinable()) {
jsbytecode *pc2 = AdvanceOverBlockchainOp(cx->regs->pc + JSOP_LAMBDA_LENGTH);
JSOp op2 = JSOp(*pc2);

View File

@ -1419,60 +1419,68 @@ stubs::RegExp(VMFrame &f, JSObject *regex)
JSObject * JS_FASTCALL
stubs::LambdaForInit(VMFrame &f, JSFunction *fun)
{
JS_ASSERT(fun->joinable());
JS_ASSERT(FUN_NULL_CLOSURE(fun));
JS_ASSERT(f.fp()->script()->compileAndGo);
JSObject *obj = FUN_OBJECT(fun);
if (FUN_NULL_CLOSURE(fun) && obj->getParent() == &f.fp()->scopeChain()) {
JS_ASSERT(obj->getParent() != NULL);
fun->setMethodAtom(f.fp()->script()->getAtom(GET_SLOTNO(f.regs.pc)));
return obj;
}
JSObject * JS_FASTCALL
stubs::LambdaForSet(VMFrame &f, JSFunction *fun)
{
JS_ASSERT(fun->joinable());
JS_ASSERT(FUN_NULL_CLOSURE(fun));
JS_ASSERT(f.fp()->script()->compileAndGo);
JSObject *obj = FUN_OBJECT(fun);
JS_ASSERT(obj->getParent() != NULL);
const Value &lref = f.regs.sp[-1];
if (lref.isObject() && lref.toObject().canHaveMethodBarrier()) {
fun->setMethodAtom(f.fp()->script()->getAtom(GET_SLOTNO(f.regs.pc)));
return obj;
}
return Lambda(f, fun);
}
JSObject * JS_FASTCALL
stubs::LambdaForSet(VMFrame &f, JSFunction *fun)
{
JSObject *obj = FUN_OBJECT(fun);
if (FUN_NULL_CLOSURE(fun) && obj->getParent() == &f.fp()->scopeChain()) {
const Value &lref = f.regs.sp[-1];
if (lref.isObject() && lref.toObject().canHaveMethodBarrier()) {
fun->setMethodAtom(f.fp()->script()->getAtom(GET_SLOTNO(f.regs.pc)));
return obj;
}
}
return Lambda(f, fun);
}
JSObject * JS_FASTCALL
stubs::LambdaJoinableForCall(VMFrame &f, JSFunction *fun)
{
JS_ASSERT(fun->joinable());
JS_ASSERT(FUN_NULL_CLOSURE(fun));
JS_ASSERT(f.fp()->script()->compileAndGo);
JSObject *obj = FUN_OBJECT(fun);
if (FUN_NULL_CLOSURE(fun) && obj->getParent() == &f.fp()->scopeChain()) {
/*
* Array.prototype.sort and String.prototype.replace are
* optimized as if they are special form. We know that they
* won't leak the joined function object in obj, therefore
* we don't need to clone that compiler- created function
* object for identity/mutation reasons.
*/
int iargc = GET_ARGC(f.regs.pc);
JS_ASSERT(obj->getParent() != NULL);
/*
* Note that we have not yet pushed obj as the final argument,
* so regs.sp[1 - (iargc + 2)], and not regs.sp[-(iargc + 2)],
* is the callee for this JSOP_CALL.
*/
const Value &cref = f.regs.sp[1 - (iargc + 2)];
JSObject *callee;
/*
* Array.prototype.sort and String.prototype.replace are
* optimized as if they are special form. We know that they
* won't leak the joined function object in obj, therefore
* we don't need to clone that compiler- created function
* object for identity/mutation reasons.
*/
int iargc = GET_ARGC(f.regs.pc);
if (IsFunctionObject(cref, &callee)) {
JSFunction *calleeFun = callee->getFunctionPrivate();
Native native = calleeFun->maybeNative();
/*
* Note that we have not yet pushed obj as the final argument,
* so regs.sp[1 - (iargc + 2)], and not regs.sp[-(iargc + 2)],
* is the callee for this JSOP_CALL.
*/
const Value &cref = f.regs.sp[1 - (iargc + 2)];
JSObject *callee;
if (native) {
if (iargc == 1 && native == array_sort)
return obj;
if (iargc == 2 && native == str_replace)
return obj;
}
if (IsFunctionObject(cref, &callee)) {
JSFunction *calleeFun = callee->getFunctionPrivate();
Native native = calleeFun->maybeNative();
if (native) {
if (iargc == 1 && native == array_sort)
return obj;
if (iargc == 2 && native == str_replace)
return obj;
}
}
return Lambda(f, fun);

View File

@ -21,6 +21,10 @@ script regress-559438.js
script regress-560101.js
script regress-560998-1.js
script regress-560998-2.js
script regress-561359-1.js
script regress-561359-2.js
script regress-561359-3.js
script regress-561359-4.js
script regress-563210.js
script regress-563221.js
script regress-566549.js

View File

@ -0,0 +1,13 @@
// Any copyright is dedicated to the Public Domain.
// http://creativecommons.org/licenses/publicdomain/
// Contributor: Gary Kwong <gary@rumblingedge.com>
for (let z = 0; z < 2; z++) {
with({
x: function() {}
}) {
for (l in [x]) {}
}
}
reportCompare(0, 0, 'ok');

View File

@ -0,0 +1,14 @@
// Any copyright is dedicated to the Public Domain.
// http://creativecommons.org/licenses/publicdomain/
// Contributor: Jason Orendorff <jorendorff@mozilla.com>
function f(s) {
var obj = {m: function () { return a; }};
eval(s);
return obj;
}
var obj = f("var a = 'right';");
var a = 'wrong';
assertEq(obj.m(), 'right');
reportCompare(0, 0, 'ok');

View File

@ -0,0 +1,13 @@
// Any copyright is dedicated to the Public Domain.
// http://creativecommons.org/licenses/publicdomain/
// Contributor: Jason Orendorff <jorendorff@mozilla.com>
function f(s) {
with (s)
return {m: function () { return a; }};
}
var obj = f({a: 'right'});
var a = 'wrong';
assertEq(obj.m(), 'right');
reportCompare(0, 0, 'ok');

View File

@ -0,0 +1,10 @@
// Any copyright is dedicated to the Public Domain.
// http://creativecommons.org/licenses/publicdomain/
// Contributor: Jason Orendorff <jorendorff@mozilla.com>
var x;
for (let i = 0; i < 2; i++)
x = {m: function () {}, n: function () { i++; }};
x.m;
reportCompare(0, 0, 'ok');