From 9ade53ec36c8503d74a641bdf956300bd959a0d6 Mon Sep 17 00:00:00 2001 From: Brian Hackett Date: Wed, 18 Jul 2012 06:35:00 -0600 Subject: [PATCH] Use distinct types and scripts for generic inner wrapper functions, bug 638794. r=jandem --- js/src/jsfun.cpp | 19 ++++++++----- js/src/jsinfer.cpp | 15 ++++++++--- js/src/jsinferinlines.h | 60 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 11 deletions(-) diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index 048063435c9..2b5fe5a14ed 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -1268,7 +1268,7 @@ js_CloneFunctionObject(JSContext *cx, HandleFunction fun, HandleObject parent, clone->initializeExtended(); } - if (cx->compartment == fun->compartment()) { + if (cx->compartment == fun->compartment() && !types::UseNewTypeForClone(fun)) { /* * We can use the same type as the original function provided that (a) * its prototype is correct, and (b) its type is not a singleton. The @@ -1279,6 +1279,9 @@ js_CloneFunctionObject(JSContext *cx, HandleFunction fun, HandleObject parent, if (fun->getProto() == proto && !fun->hasSingletonType()) clone->setType(fun->type()); } else { + if (!clone->setSingletonType(cx)) + return NULL; + /* * Across compartments we have to clone the script for interpreted * functions. Cross-compartment cloning only happens via JSAPI @@ -1289,22 +1292,24 @@ js_CloneFunctionObject(JSContext *cx, HandleFunction fun, HandleObject parent, RootedScript script(cx, clone->script()); JS_ASSERT(script); JS_ASSERT(script->compartment() == fun->compartment()); - JS_ASSERT(script->compartment() != cx->compartment); - JS_ASSERT(!script->enclosingStaticScope()); + JS_ASSERT_IF(script->compartment() != cx->compartment, + !script->enclosingStaticScope() && !script->compileAndGo); + + RootedObject scope(cx, script->enclosingStaticScope()); clone->mutableScript().init(NULL); - JSScript *cscript = CloneScript(cx, NullPtr(), clone, script); + JSScript *cscript = CloneScript(cx, scope, clone, script); if (!cscript) return NULL; clone->setScript(cscript); cscript->setFunction(clone); - if (!clone->setTypeForScriptedFunction(cx)) - return NULL; + + GlobalObject *global = script->compileAndGo ? &script->global() : NULL; js_CallNewScriptHook(cx, clone->script(), clone); - Debugger::onNewScript(cx, clone->script(), NULL); + Debugger::onNewScript(cx, clone->script(), global); } } return clone; diff --git a/js/src/jsinfer.cpp b/js/src/jsinfer.cpp index b587ccb3576..ce3f221df96 100644 --- a/js/src/jsinfer.cpp +++ b/js/src/jsinfer.cpp @@ -3629,7 +3629,7 @@ ScriptAnalysis::analyzeTypesBytecode(JSContext *cx, unsigned offset, res = &pushed[0]; if (res) { - if (script->hasGlobal()) + if (script->hasGlobal() && !UseNewTypeForClone(obj->toFunction())) res->addType(cx, Type::ObjectType(obj)); else res->addType(cx, Type::UnknownType()); @@ -3926,12 +3926,14 @@ ScriptAnalysis::analyzeTypesBytecode(JSContext *cx, unsigned offset, pushed[0].addType(cx, Type::UnknownType()); break; - case JSOP_CALLEE: - if (script->hasGlobal()) - pushed[0].addType(cx, Type::ObjectType(script->function())); + case JSOP_CALLEE: { + JSFunction *fun = script->function(); + if (script->hasGlobal() && !UseNewTypeForClone(fun)) + pushed[0].addType(cx, Type::ObjectType(fun)); else pushed[0].addType(cx, Type::UnknownType()); break; + } default: /* Display fine-grained debug information first */ @@ -5040,6 +5042,11 @@ JSFunction::setTypeForScriptedFunction(JSContext *cx, bool singleton) if (singleton) { if (!setSingletonType(cx)) return false; + } else if (UseNewTypeForClone(this)) { + /* + * Leave the default unknown-properties type for the function, it + * should not be used by scripts or appear in type sets. + */ } else { RootedFunction self(cx, this); diff --git a/js/src/jsinferinlines.h b/js/src/jsinferinlines.h index 98c1b6ad6e9..b7bbd9293f2 100644 --- a/js/src/jsinferinlines.h +++ b/js/src/jsinferinlines.h @@ -489,6 +489,66 @@ UseNewTypeAtEntry(JSContext *cx, StackFrame *fp) UseNewType(cx, fp->prev()->script(), fp->prevpc()); } +inline bool +UseNewTypeForClone(JSFunction *fun) +{ + if (fun->hasSingletonType() || !fun->isInterpreted()) + return false; + + /* + * When a function is being used as a wrapper for another function, it + * improves precision greatly to distinguish between different instances of + * the wrapper; otherwise we will conflate much of the information about + * the wrapped functions. + * + * An important example is the Class.create function at the core of the + * Prototype.js library, which looks like: + * + * var Class = { + * create: function() { + * return function() { + * this.initialize.apply(this, arguments); + * } + * } + * }; + * + * Each instance of the innermost function will have a different wrapped + * initialize method. We capture this, along with similar cases, by looking + * for short scripts which use both .apply and arguments. For such scripts, + * whenever creating a new instance of the function we both give that + * instance a singleton type and clone the underlying script. + */ + + JSScript *script = fun->script(); + + if (script->length >= 50) + return false; + + if (script->hasConsts() || + script->hasObjects() || + script->hasRegexps() || + script->hasClosedArgs() || + script->hasClosedVars()) + { + return false; + } + + bool hasArguments = false; + bool hasApply = false; + + for (jsbytecode *pc = script->code; + pc != script->code + script->length; + pc += GetBytecodeLength(pc)) + { + if (*pc == JSOP_ARGUMENTS) + hasArguments = true; + if (*pc == JSOP_FUNAPPLY) + hasApply = true; + } + + return hasArguments && hasApply; +} + ///////////////////////////////////////////////////////////////////// // Script interface functions /////////////////////////////////////////////////////////////////////