- Revision
- 91883
- Author
- [email protected]
- Date
- 2011-07-27 16:48:56 -0700 (Wed, 27 Jul 2011)
Log Message
https://bugs.webkit.org/show_bug.cgi?id=65294
DFG JIT - may speculate based on wrong arguments.
Reviewed by Oliver Hunt
In the case of a DFG compiled function calling to and compiling a second function that
also compiles through the DFG JIT (i.e. compilation triggered with DFGOperations.cpp),
we call compileFor passing the caller functions exec state, rather than the callee's.
This may lead to mis-optimization, since the DFG compiler will example the exec state's
arguments on the assumption that these will be passed to the callee - it is wanting the
callee exec state, not the caller's exec state.
Fixing this for all cases of compilation is tricksy, due to the way the numeric sort
function is compiled, & the structure of the calls in the Interpreter::execute methods.
Only fix for compilation from the JIT, in other calls don't speculate based on arguments
for now.
* dfg/DFGOperations.cpp:
* runtime/Executable.cpp:
(JSC::tryDFGCompile):
(JSC::tryDFGCompileFunction):
(JSC::FunctionExecutable::compileForCallInternal):
* runtime/Executable.h:
(JSC::FunctionExecutable::compileForCall):
(JSC::FunctionExecutable::compileFor):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (91882 => 91883)
--- trunk/Source/_javascript_Core/ChangeLog 2011-07-27 23:40:36 UTC (rev 91882)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-07-27 23:48:56 UTC (rev 91883)
@@ -1,3 +1,31 @@
+2011-07-27 Gavin Barraclough <[email protected]>
+
+ https://bugs.webkit.org/show_bug.cgi?id=65294
+ DFG JIT - may speculate based on wrong arguments.
+
+ Reviewed by Oliver Hunt
+
+ In the case of a DFG compiled function calling to and compiling a second function that
+ also compiles through the DFG JIT (i.e. compilation triggered with DFGOperations.cpp),
+ we call compileFor passing the caller functions exec state, rather than the callee's.
+ This may lead to mis-optimization, since the DFG compiler will example the exec state's
+ arguments on the assumption that these will be passed to the callee - it is wanting the
+ callee exec state, not the caller's exec state.
+
+ Fixing this for all cases of compilation is tricksy, due to the way the numeric sort
+ function is compiled, & the structure of the calls in the Interpreter::execute methods.
+ Only fix for compilation from the JIT, in other calls don't speculate based on arguments
+ for now.
+
+ * dfg/DFGOperations.cpp:
+ * runtime/Executable.cpp:
+ (JSC::tryDFGCompile):
+ (JSC::tryDFGCompileFunction):
+ (JSC::FunctionExecutable::compileForCallInternal):
+ * runtime/Executable.h:
+ (JSC::FunctionExecutable::compileForCall):
+ (JSC::FunctionExecutable::compileFor):
+
2011-07-27 Oliver Hunt <[email protected]>
Handle callback oriented JSONP
Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.cpp (91882 => 91883)
--- trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2011-07-27 23:40:36 UTC (rev 91882)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2011-07-27 23:48:56 UTC (rev 91883)
@@ -529,8 +529,9 @@
if (executable->isHostFunction())
codePtr = executable->generatedJITCodeFor(kind).addressForCall();
else {
+ execCallee->setScopeChain(callee->scope());
FunctionExecutable* functionExecutable = static_cast<FunctionExecutable*>(executable);
- JSObject* error = functionExecutable->compileFor(exec, callee->scope(), kind);
+ JSObject* error = functionExecutable->compileFor(execCallee, callee->scope(), kind);
if (error) {
globalData->exception = createStackOverflowError(exec);
return 0;
@@ -540,7 +541,6 @@
codePtr = functionExecutable->generatedJITCodeFor(kind).addressForCall();
else
codePtr = functionExecutable->generatedJITCodeWithArityCheckFor(kind);
- execCallee->setScopeChain(callee->scope());
}
CallLinkInfo& callLinkInfo = exec->codeBlock()->getCallLinkInfo(returnAddress);
if (!callLinkInfo.seenOnce())
@@ -574,16 +574,16 @@
return handleHostCall(execCallee, calleeAsValue, kind);
JSFunction* function = asFunction(calleeAsFunctionCell);
+ execCallee->setScopeChain(function->scopeUnchecked());
ExecutableBase* executable = function->executable();
if (UNLIKELY(!executable->hasJITCodeFor(kind))) {
FunctionExecutable* functionExecutable = static_cast<FunctionExecutable*>(executable);
- JSObject* error = functionExecutable->compileFor(exec, function->scope(), kind);
+ JSObject* error = functionExecutable->compileFor(execCallee, function->scope(), kind);
if (error) {
exec->globalData().exception = error;
return 0;
}
}
- execCallee->setScopeChain(function->scopeUnchecked());
return executable->generatedJITCodeWithArityCheckFor(kind).executableAddress();
}
Modified: trunk/Source/_javascript_Core/runtime/Executable.cpp (91882 => 91883)
--- trunk/Source/_javascript_Core/runtime/Executable.cpp 2011-07-27 23:40:36 UTC (rev 91882)
+++ trunk/Source/_javascript_Core/runtime/Executable.cpp 2011-07-27 23:48:56 UTC (rev 91883)
@@ -57,14 +57,12 @@
if (!parse(dfg, globalData, codeBlock))
return false;
- dfg.predictArgumentTypes(exec);
-
DFG::JITCompiler dataFlowJIT(globalData, dfg, codeBlock);
dataFlowJIT.compile(jitCode);
return true;
}
-static bool tryDFGCompileFunction(ExecState* exec, CodeBlock* codeBlock, JITCode& jitCode, MacroAssemblerCodePtr& jitCodeWithArityCheck)
+static bool tryDFGCompileFunction(ExecState* exec, ExecState* calleeArgsExec, CodeBlock* codeBlock, JITCode& jitCode, MacroAssemblerCodePtr& jitCodeWithArityCheck)
{
#if ENABLE(DFG_JIT_RESTRICTIONS)
// FIXME: No flow control yet supported, don't bother scanning the bytecode if there are any jump targets.
@@ -77,7 +75,8 @@
if (!parse(dfg, globalData, codeBlock))
return false;
- dfg.predictArgumentTypes(exec);
+ if (calleeArgsExec)
+ dfg.predictArgumentTypes(calleeArgsExec);
DFG::JITCompiler dataFlowJIT(globalData, dfg, codeBlock);
dataFlowJIT.compileFunction(jitCode, jitCodeWithArityCheck);
@@ -85,7 +84,7 @@
}
#else
static bool tryDFGCompile(ExecState*, CodeBlock*, JITCode&) { return false; }
-static bool tryDFGCompileFunction(ExecState*, CodeBlock*, JITCode&, MacroAssemblerCodePtr&) { return false; }
+static bool tryDFGCompileFunction(ExecState*, ExecState*, CodeBlock*, JITCode&, MacroAssemblerCodePtr&) { return false; }
#endif
class ExecutableFinalizer : public WeakHandleOwner {
@@ -317,7 +316,7 @@
m_programCodeBlock->visitAggregate(visitor);
}
-JSObject* FunctionExecutable::compileForCallInternal(ExecState* exec, ScopeChainNode* scopeChainNode)
+JSObject* FunctionExecutable::compileForCallInternal(ExecState* exec, ScopeChainNode* scopeChainNode, ExecState* calleeArgsExec)
{
JSObject* exception = 0;
JSGlobalData* globalData = scopeChainNode->globalData;
@@ -351,7 +350,7 @@
#if ENABLE(JIT)
if (exec->globalData().canUseJIT()) {
- bool dfgCompiled = tryDFGCompileFunction(exec, m_codeBlockForCall.get(), m_jitCodeForCall, m_jitCodeForCallWithArityCheck);
+ bool dfgCompiled = tryDFGCompileFunction(exec, calleeArgsExec, m_codeBlockForCall.get(), m_jitCodeForCall, m_jitCodeForCallWithArityCheck);
if (!dfgCompiled)
m_jitCodeForCall = JIT::compile(scopeChainNode->globalData, m_codeBlockForCall.get(), &m_jitCodeForCallWithArityCheck);
Modified: trunk/Source/_javascript_Core/runtime/Executable.h (91882 => 91883)
--- trunk/Source/_javascript_Core/runtime/Executable.h 2011-07-27 23:40:36 UTC (rev 91882)
+++ trunk/Source/_javascript_Core/runtime/Executable.h 2011-07-27 23:48:56 UTC (rev 91883)
@@ -405,12 +405,12 @@
return *m_codeBlockForConstruct;
}
- JSObject* compileForCall(ExecState* exec, ScopeChainNode* scopeChainNode)
+ JSObject* compileForCall(ExecState* exec, ScopeChainNode* scopeChainNode, ExecState* calleeArgsExec = 0)
{
ASSERT(exec->globalData().dynamicGlobalObject);
JSObject* error = 0;
if (!m_codeBlockForCall)
- error = compileForCallInternal(exec, scopeChainNode);
+ error = compileForCallInternal(exec, scopeChainNode, calleeArgsExec);
ASSERT(!error == !!m_codeBlockForCall);
return error;
}
@@ -449,8 +449,14 @@
JSObject* compileFor(ExecState* exec, ScopeChainNode* scopeChainNode, CodeSpecializationKind kind)
{
+ // compileFor should only be called with a callframe set up to call this function,
+ // since we will make speculative optimizations based on the arguments.
+ ASSERT(exec->callee());
+ ASSERT(exec->callee()->inherits(&JSFunction::s_info));
+ ASSERT(asFunction(exec->callee())->jsExecutable() == this);
+
if (kind == CodeForCall)
- return compileForCall(exec, scopeChainNode);
+ return compileForCall(exec, scopeChainNode, exec);
ASSERT(kind == CodeForConstruct);
return compileForConstruct(exec, scopeChainNode);
}
@@ -491,7 +497,7 @@
FunctionExecutable(JSGlobalData&, const Identifier& name, const SourceCode&, bool forceUsesArguments, FunctionParameters*, bool, int firstLine, int lastLine);
FunctionExecutable(ExecState*, const Identifier& name, const SourceCode&, bool forceUsesArguments, FunctionParameters*, bool, int firstLine, int lastLine);
- JSObject* compileForCallInternal(ExecState*, ScopeChainNode*);
+ JSObject* compileForCallInternal(ExecState*, ScopeChainNode*, ExecState* calleeArgsExec);
JSObject* compileForConstructInternal(ExecState*, ScopeChainNode*);
static const unsigned StructureFlags = OverridesVisitChildren | ScriptExecutable::StructureFlags;
