From 8dbcb884d9d23eeb5e00dc58f545c131fcee967f Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Mon, 5 Oct 2009 16:55:21 -0700 Subject: [PATCH] Fix constructor method (foo.bar/foo[baz] initialized from a lambda) invocation to go through the method read barrier (518103, r=jorendorff). --- js/src/jsemit.cpp | 28 +++++++++++++++---- js/src/jsfun.cpp | 15 +++++++--- js/src/jsfun.h | 17 +++++++++-- js/src/tests/e4x/Regress/regress-355478.js | 2 +- js/src/tests/ecma_3/FunExpr/jstests.list | 1 + js/src/tests/ecma_3/FunExpr/regress-518103.js | 27 ++++++++++++++++++ 6 files changed, 77 insertions(+), 13 deletions(-) create mode 100644 js/src/tests/ecma_3/FunExpr/regress-518103.js diff --git a/js/src/jsemit.cpp b/js/src/jsemit.cpp index 38f850742c9..59c9f68c9c5 100644 --- a/js/src/jsemit.cpp +++ b/js/src/jsemit.cpp @@ -6269,26 +6269,39 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) case TOK_NEW: case TOK_LP: { + bool callop = (PN_TYPE(pn) == TOK_LP); uintN oldflags; /* - * Emit function call or operator new (constructor call) code. + * Emit callable invocation or operator new (constructor call) code. * First, emit code for the left operand to evaluate the callable or * constructable object expression. + * + * For operator new applied to other expressions than E4X ones, we emit + * JSOP_GETPROP instead of JSOP_CALLPROP, etc. This is necessary to + * interpose the lambda-initialized method read barrier -- see the code + * in jsops.cpp for JSOP_LAMBDA followed by JSOP_{SET,INIT}PROP. + * + * Then (or in a call case that has no explicit reference-base object) + * we emit JSOP_NULL as a placeholder local GC root to hold the |this| + * parameter: in the operator new case, the newborn instance; in the + * base-less call case, a cookie meaning "use the global object as the + * |this| value" (or in ES5 strict mode, "use undefined", so we should + * use JSOP_PUSH instead of JSOP_NULL -- see bug 514570). */ pn2 = pn->pn_head; switch (pn2->pn_type) { case TOK_NAME: - if (!EmitNameOp(cx, cg, pn2, JS_TRUE)) + if (!EmitNameOp(cx, cg, pn2, callop)) return JS_FALSE; break; case TOK_DOT: - if (!EmitPropOp(cx, pn2, PN_OP(pn2), cg, JS_TRUE)) + if (!EmitPropOp(cx, pn2, PN_OP(pn2), cg, callop)) return JS_FALSE; break; case TOK_LB: JS_ASSERT(pn2->pn_op == JSOP_GETELEM); - if (!EmitElemOp(cx, pn2, JSOP_CALLELEM, cg)) + if (!EmitElemOp(cx, pn2, callop ? JSOP_CALLELEM : JSOP_GETELEM, cg)) return JS_FALSE; break; case TOK_UNARYOP: @@ -6296,6 +6309,7 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) if (pn2->pn_op == JSOP_XMLNAME) { if (!EmitXMLName(cx, pn2, JSOP_CALLXMLNAME, cg)) return JS_FALSE; + callop = true; /* suppress JSOP_NULL after */ break; } #endif @@ -6307,9 +6321,11 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) */ if (!js_EmitTree(cx, cg, pn2)) return JS_FALSE; - if (js_Emit1(cx, cg, JSOP_NULL) < 0) - return JS_FALSE; + callop = false; /* trigger JSOP_NULL after */ + break; } + if (!callop && js_Emit1(cx, cg, JSOP_NULL) < 0) + return JS_FALSE; /* Remember start of callable-object bytecode for decompilation hint. */ off = top; diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index 4042a21a05b..0019b617e3f 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -1451,10 +1451,14 @@ fun_resolve(JSContext *cx, JSObject *obj, jsval id, uintN flags, fun = GET_FUNCTION_PRIVATE(cx, obj); /* - * No need to reflect fun.prototype in 'fun.prototype = ... '. + * No need to reflect fun.prototype in 'fun.prototype = ... '. Assert that + * fun is not a compiler-created function object, which must never leak to + * script or embedding code and then be mutated. */ - if (flags & JSRESOLVE_ASSIGNING) + if (flags & JSRESOLVE_ASSIGNING) { + JS_ASSERT(!js_InternalFunctionObject(obj)); return JS_TRUE; + } /* * Ok, check whether id is 'prototype' and bootstrap the function object's @@ -1462,7 +1466,7 @@ fun_resolve(JSContext *cx, JSObject *obj, jsval id, uintN flags, */ atom = cx->runtime->atomState.classPrototypeAtom; if (id == ATOM_KEY(atom)) { - JSObject *proto; + JS_ASSERT(!js_InternalFunctionObject(obj)); /* * Beware of the wacky case of a user function named Object -- trying @@ -1475,7 +1479,8 @@ fun_resolve(JSContext *cx, JSObject *obj, jsval id, uintN flags, * Make the prototype object to have the same parent as the function * object itself. */ - proto = js_NewObject(cx, &js_ObjectClass, NULL, OBJ_GET_PARENT(cx, obj)); + JSObject *proto = + js_NewObject(cx, &js_ObjectClass, NULL, OBJ_GET_PARENT(cx, obj)); if (!proto) return JS_FALSE; @@ -1498,6 +1503,8 @@ fun_resolve(JSContext *cx, JSObject *obj, jsval id, uintN flags, atom = OFFSET_TO_ATOM(cx->runtime, lfp->atomOffset); if (id == ATOM_KEY(atom)) { + JS_ASSERT(!js_InternalFunctionObject(obj)); + if (!js_DefineNativeProperty(cx, obj, ATOM_TO_JSID(atom), JSVAL_VOID, fun_getProperty, JS_PropertyStub, diff --git a/js/src/jsfun.h b/js/src/jsfun.h index 8a0ef503f23..d2918fcad32 100644 --- a/js/src/jsfun.h +++ b/js/src/jsfun.h @@ -160,8 +160,8 @@ struct JSFunction : public JSObject { } u; JSAtom *atom; /* name for diagnostics and decompiling */ - bool optimizedClosure() { return FUN_KIND(this) > JSFUN_INTERPRETED; } - bool needsWrapper() { return FUN_NULL_CLOSURE(this) && u.i.skipmin != 0; } + bool optimizedClosure() const { return FUN_KIND(this) > JSFUN_INTERPRETED; } + bool needsWrapper() const { return FUN_NULL_CLOSURE(this) && u.i.skipmin != 0; } uintN countVars() const { JS_ASSERT(FUN_INTERPRETED(this)); @@ -226,6 +226,19 @@ extern JS_FRIEND_DATA(JSClass) js_FunctionClass; (JS_ASSERT(HAS_FUNCTION_CLASS(funobj)), \ (JSFunction *) (funobj)->getPrivate()) +/* + * Return true if this is a compiler-created internal function accessed by + * its own object. Such a function object must not be accessible to script + * or embedding code. + */ +inline bool +js_InternalFunctionObject(JSObject *funobj) +{ + JS_ASSERT(HAS_FUNCTION_CLASS(funobj)); + JSFunction *fun = (JSFunction *) funobj->getPrivate(); + return funobj == fun && (fun->flags & JSFUN_LAMBDA) && !funobj->getParent(); +} + struct js_ArgsPrivateNative; inline js_ArgsPrivateNative * diff --git a/js/src/tests/e4x/Regress/regress-355478.js b/js/src/tests/e4x/Regress/regress-355478.js index 79afe636e9b..272667d262a 100644 --- a/js/src/tests/e4x/Regress/regress-355478.js +++ b/js/src/tests/e4x/Regress/regress-355478.js @@ -46,7 +46,7 @@ var expect = ''; printBugNumber(BUGNUMBER); START(summary); -expect = 'TypeError: XML.prototype.hasOwnProperty called on incompatible Object'; +expect = 'TypeError: .hasOwnProperty is not a constructor'; actual = ''; try diff --git a/js/src/tests/ecma_3/FunExpr/jstests.list b/js/src/tests/ecma_3/FunExpr/jstests.list index 2d02ded689f..e41c3a446d8 100644 --- a/js/src/tests/ecma_3/FunExpr/jstests.list +++ b/js/src/tests/ecma_3/FunExpr/jstests.list @@ -2,3 +2,4 @@ url-prefix ../../jsreftest.html?test=ecma_3/FunExpr/ script fe-001-n.js script fe-001.js script fe-002.js +script regress-518103.js diff --git a/js/src/tests/ecma_3/FunExpr/regress-518103.js b/js/src/tests/ecma_3/FunExpr/regress-518103.js new file mode 100644 index 00000000000..b9949674d93 --- /dev/null +++ b/js/src/tests/ecma_3/FunExpr/regress-518103.js @@ -0,0 +1,27 @@ +/* + * Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/licenses/publicdomain/ + */ + +var BUGNUMBER = 518103; +var summary = 'lambda constructor "method" vs. instanceof'; +var actual; +var expect; + +printBugNumber(BUGNUMBER); +printStatus(summary); + +var Y = {widget: {}}; + +Y.widget.DataSource = function () {}; +Y.widget.DS_JSArray = function (A) { this.data = A; }; +Y.widget.DS_JSArray.prototype = new Y.widget.DataSource(); + +var J = new Y.widget.DS_JSArray( [ ] ); + +actual = J instanceof Y.widget.DataSource; +expect = true; + +reportCompare(actual, expect, summary); + +printStatus("All tests passed!");