From 7040cac9dbbfa6ce2a5304f6ce7afeefc70d1e2b Mon Sep 17 00:00:00 2001 From: Brian Hackett Date: Tue, 16 Jul 2013 18:54:47 -0600 Subject: [PATCH] Bug 894669 - Add analysis for finding variables unnecessarily entrained by inner functions, r=luke. --- js/src/shell/js.cpp | 16 ++++ js/src/vm/ScopeObject.cpp | 172 ++++++++++++++++++++++++++++++++++++++ js/src/vm/ScopeObject.h | 5 ++ 3 files changed, 193 insertions(+) diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 3018137125b9..73ec489d74e1 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -172,6 +172,7 @@ static bool compileOnly = false; static bool fuzzingSafe = false; #ifdef DEBUG +static bool dumpEntrainedVariables = false; static bool OOM_printAllocationCount = false; #endif @@ -428,6 +429,12 @@ RunFile(JSContext *cx, Handle obj, const char *filename, FILE *file, RootedScript script(cx); script = JS::Compile(cx, obj, options, file); + +#ifdef DEBUG + if (dumpEntrainedVariables) + AnalyzeEntrainedVariables(cx, script); +#endif + JS_SetOptions(cx, oldopts); JS_ASSERT_IF(!script, gGotError); if (script && !compileOnly) { @@ -5069,6 +5076,11 @@ ProcessArgs(JSContext *cx, JSObject *obj_, OptionParser *op) #endif /* JS_ION */ +#ifdef DEBUG + if (op->getBoolOption("dump-entrained-variables")) + dumpEntrainedVariables = true; +#endif + /* |scriptArgs| gets bound on the global before any code is run. */ if (!BindScriptArgs(cx, obj, op)) return EXIT_FAILURE; @@ -5302,6 +5314,10 @@ main(int argc, char **argv, char **envp) "to test JIT codegen (no-op on platforms other than x86).") || !op.addBoolOption('\0', "fuzzing-safe", "Don't expose functions that aren't safe for " "fuzzers to call") +#ifdef DEBUG + || !op.addBoolOption('\0', "dump-entrained-variables", "Print variables which are " + "unnecessarily entrained by inner functions") +#endif #ifdef JSGC_GENERATIONAL || !op.addBoolOption('\0', "no-ggc", "Disable Generational GC") #endif diff --git a/js/src/vm/ScopeObject.cpp b/js/src/vm/ScopeObject.cpp index a292fd38e29a..25deafbcf98c 100644 --- a/js/src/vm/ScopeObject.cpp +++ b/js/src/vm/ScopeObject.cpp @@ -2192,3 +2192,175 @@ js::GetDebugScopeForFrame(JSContext *cx, AbstractFramePtr frame) ScopeIter si(frame, cx); return GetDebugScope(cx, si); } + +#ifdef DEBUG + +typedef HashSet PropertyNameSet; + +static bool +RemoveReferencedNames(JSContext *cx, HandleScript script, PropertyNameSet &remainingNames) +{ + // Remove from remainingNames --- the closure variables in some outer + // script --- any free variables in this script. This analysis isn't perfect: + // + // - It will not account for free variables in an inner script which are + // actually accessing some name in an intermediate script between the + // inner and outer scripts. This can cause remainingNames to be an + // underapproximation. + // + // - It will not account for new names introduced via eval. This can cause + // remainingNames to be an overapproximation. This would be easy to fix + // but is nice to have as the eval will probably not access these + // these names and putting eval in an inner script is bad news if you + // care about entraining variables unnecessarily. + + for (jsbytecode *pc = script->code; + pc != script->code + script->length; + pc += GetBytecodeLength(pc)) + { + PropertyName *name; + + switch (JSOp(*pc)) { + case JSOP_NAME: + case JSOP_CALLNAME: + case JSOP_SETNAME: + name = script->getName(pc); + break; + + case JSOP_GETALIASEDVAR: + case JSOP_CALLALIASEDVAR: + case JSOP_SETALIASEDVAR: + name = ScopeCoordinateName(cx, script, pc); + break; + + default: + name = NULL; + break; + } + + if (name) + remainingNames.remove(name); + } + + if (script->hasObjects()) { + ObjectArray *objects = script->objects(); + for (size_t i = 0; i < objects->length; i++) { + JSObject *obj = objects->vector[i]; + if (obj->is() && obj->as().isInterpreted()) { + JSFunction *fun = &obj->as(); + RootedScript innerScript(cx, fun->getOrCreateScript(cx)); + if (!innerScript) + return false; + + if (!RemoveReferencedNames(cx, innerScript, remainingNames)) + return false; + } + } + } + + return true; +} + +static bool +AnalyzeEntrainedVariablesInScript(JSContext *cx, HandleScript script, HandleScript innerScript) +{ + PropertyNameSet remainingNames(cx); + if (!remainingNames.init()) + return false; + + for (BindingIter bi(script); bi; bi++) { + if (bi->aliased()) { + PropertyNameSet::AddPtr p = remainingNames.lookupForAdd(bi->name()); + if (!p && !remainingNames.add(p, bi->name())) + return false; + } + } + + if (!RemoveReferencedNames(cx, innerScript, remainingNames)) + return false; + + if (!remainingNames.empty()) { + Sprinter buf(cx); + if (!buf.init()) + return false; + + buf.printf("Script "); + + if (JSAtom *name = script->function()->displayAtom()) { + buf.putString(name); + buf.printf(" "); + } + + buf.printf("(%s:%d) has variables entrained by ", script->filename(), script->lineno); + + if (JSAtom *name = innerScript->function()->displayAtom()) { + buf.putString(name); + buf.printf(" "); + } + + buf.printf("(%s:%d) ::", innerScript->filename(), innerScript->lineno); + + for (PropertyNameSet::Range r = remainingNames.all(); !r.empty(); r.popFront()) { + buf.printf(" "); + buf.putString(r.front()); + } + + printf("%s\n", buf.string()); + } + + if (innerScript->hasObjects()) { + ObjectArray *objects = innerScript->objects(); + for (size_t i = 0; i < objects->length; i++) { + JSObject *obj = objects->vector[i]; + if (obj->is() && obj->as().isInterpreted()) { + JSFunction *fun = &obj->as(); + RootedScript innerInnerScript(cx, fun->nonLazyScript()); + if (!AnalyzeEntrainedVariablesInScript(cx, script, innerInnerScript)) + return false; + } + } + } + + return true; +} + +// Look for local variables in script or any other script inner to it, which are +// part of the script's call object and are unnecessarily entrained by their own +// inner scripts which do not refer to those variables. An example is: +// +// function foo() { +// var a, b; +// function bar() { return a; } +// function baz() { return b; } +// } +// +// |bar| unnecessarily entrains |b|, and |baz| unnecessarily entrains |a|. +bool +js::AnalyzeEntrainedVariables(JSContext *cx, HandleScript script) +{ + if (!script->hasObjects()) + return true; + + ObjectArray *objects = script->objects(); + for (size_t i = 0; i < objects->length; i++) { + JSObject *obj = objects->vector[i]; + if (obj->is() && obj->as().isInterpreted()) { + JSFunction *fun = &obj->as(); + RootedScript innerScript(cx, fun->getOrCreateScript(cx)); + if (!innerScript) + return false; + + if (script->function() && script->function()->isHeavyweight()) { + if (!AnalyzeEntrainedVariablesInScript(cx, script, innerScript)) + return false; + } + + if (!AnalyzeEntrainedVariables(cx, innerScript)) + return false; + } + } + + return true; +} + +#endif diff --git a/js/src/vm/ScopeObject.h b/js/src/vm/ScopeObject.h index 5a395de1d598..aa75d5f4501a 100644 --- a/js/src/vm/ScopeObject.h +++ b/js/src/vm/ScopeObject.h @@ -747,6 +747,11 @@ StaticBlockObject::enclosingBlock() const return obj && obj->is() ? &obj->as() : NULL; } +#ifdef DEBUG +bool +AnalyzeEntrainedVariables(JSContext *cx, HandleScript script); +#endif + } // namespace js #endif /* vm_ScopeObject_h */