- Revision
- 174359
- Author
- [email protected]
- Date
- 2014-10-06 12:29:27 -0700 (Mon, 06 Oct 2014)
Log Message
REGRESSION(r174226): [JSC] Crash when running the perf test Speedometer/Full.html
https://bugs.webkit.org/show_bug.cgi?id=137404
Reviewed by Michael Saboff.
Update the Arguments object to recognise that it must always have an
environment record if the referenced callee has one, and if such is not
present it should not try to extract one from the callframe, as that
path leads to madness.
Happily this makes some of the other code more sensible, and removes a
bunch of unnecessary and icky logic.
* interpreter/Interpreter.cpp:
(JSC::unwindCallFrame):
* jit/JITOperations.cpp:
* llint/LLIntSlowPaths.cpp:
(JSC::LLInt::LLINT_SLOW_PATH_DECL):
* runtime/Arguments.cpp:
(JSC::Arguments::tearOff):
(JSC::Arguments::didTearOffActivation): Deleted.
* runtime/Arguments.h:
(JSC::Arguments::argument):
(JSC::Arguments::finishCreation):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (174358 => 174359)
--- trunk/Source/_javascript_Core/ChangeLog 2014-10-06 19:20:19 UTC (rev 174358)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-10-06 19:29:27 UTC (rev 174359)
@@ -1,3 +1,30 @@
+2014-10-06 Oliver Hunt <[email protected]>
+
+ REGRESSION(r174226): [JSC] Crash when running the perf test Speedometer/Full.html
+ https://bugs.webkit.org/show_bug.cgi?id=137404
+
+ Reviewed by Michael Saboff.
+
+ Update the Arguments object to recognise that it must always have an
+ environment record if the referenced callee has one, and if such is not
+ present it should not try to extract one from the callframe, as that
+ path leads to madness.
+
+ Happily this makes some of the other code more sensible, and removes a
+ bunch of unnecessary and icky logic.
+
+ * interpreter/Interpreter.cpp:
+ (JSC::unwindCallFrame):
+ * jit/JITOperations.cpp:
+ * llint/LLIntSlowPaths.cpp:
+ (JSC::LLInt::LLINT_SLOW_PATH_DECL):
+ * runtime/Arguments.cpp:
+ (JSC::Arguments::tearOff):
+ (JSC::Arguments::didTearOffActivation): Deleted.
+ * runtime/Arguments.h:
+ (JSC::Arguments::argument):
+ (JSC::Arguments::finishCreation):
+
2014-10-04 Brian J. Burg <[email protected]>
Unreviewed, rolling out r174319.
Modified: trunk/Source/_javascript_Core/interpreter/Interpreter.cpp (174358 => 174359)
--- trunk/Source/_javascript_Core/interpreter/Interpreter.cpp 2014-10-06 19:20:19 UTC (rev 174358)
+++ trunk/Source/_javascript_Core/interpreter/Interpreter.cpp 2014-10-06 19:29:27 UTC (rev 174359)
@@ -448,7 +448,6 @@
ASSERT(!callFrame->hadException());
}
- JSValue lexicalEnvironment;
if (codeBlock->codeType() == FunctionCode && codeBlock->needsActivation()) {
#if ENABLE(DFG_JIT)
RELEASE_ASSERT(!visitor->isInlinedFrame());
@@ -457,10 +456,8 @@
if (codeBlock->codeType() == FunctionCode && codeBlock->usesArguments()) {
if (Arguments* arguments = visitor->existingArguments()) {
- if (lexicalEnvironment && lexicalEnvironment.isCell())
- arguments->didTearOffActivation(callFrame, jsCast<JSLexicalEnvironment*>(lexicalEnvironment));
#if ENABLE(DFG_JIT)
- else if (visitor->isInlinedFrame())
+ if (visitor->isInlinedFrame())
arguments->tearOff(callFrame, visitor->inlineCallFrame());
#endif
else
Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (174358 => 174359)
--- trunk/Source/_javascript_Core/jit/JITOperations.cpp 2014-10-06 19:20:19 UTC (rev 174358)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp 2014-10-06 19:29:27 UTC (rev 174359)
@@ -1590,13 +1590,9 @@
return JSValue::encode(result);
}
-void JIT_OPERATION operationTearOffArguments(ExecState* exec, JSCell* argumentsCell, JSCell* activationCell)
+void JIT_OPERATION operationTearOffArguments(ExecState* exec, JSCell* argumentsCell, JSCell*)
{
ASSERT(exec->codeBlock()->usesArguments());
- if (activationCell) {
- jsCast<Arguments*>(argumentsCell)->didTearOffActivation(exec, jsCast<JSLexicalEnvironment*>(activationCell));
- return;
- }
jsCast<Arguments*>(argumentsCell)->tearOff(exec);
}
Modified: trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp (174358 => 174359)
--- trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2014-10-06 19:20:19 UTC (rev 174358)
+++ trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2014-10-06 19:29:27 UTC (rev 174359)
@@ -1250,10 +1250,7 @@
LLINT_BEGIN();
ASSERT(exec->codeBlock()->usesArguments());
Arguments* arguments = jsCast<Arguments*>(exec->uncheckedR(VirtualRegister(pc[1].u.operand).offset()).jsValue());
- if (JSValue activationValue = LLINT_OP_C(2).jsValue())
- arguments->didTearOffActivation(exec, jsCast<JSLexicalEnvironment*>(activationValue));
- else
- arguments->tearOff(exec);
+ arguments->tearOff(exec);
LLINT_END();
}
Modified: trunk/Source/_javascript_Core/runtime/Arguments.cpp (174358 => 174359)
--- trunk/Source/_javascript_Core/runtime/Arguments.cpp 2014-10-06 19:20:19 UTC (rev 174358)
+++ trunk/Source/_javascript_Core/runtime/Arguments.cpp 2014-10-06 19:29:27 UTC (rev 174359)
@@ -372,6 +372,8 @@
void Arguments::tearOff(CallFrame* callFrame)
{
+ if (m_callee->jsExecutable()->needsActivation())
+ RELEASE_ASSERT(m_lexicalEnvironment);
if (isTornOff())
return;
@@ -391,21 +393,9 @@
}
}
-void Arguments::didTearOffActivation(ExecState* exec, JSLexicalEnvironment* lexicalEnvironment)
-{
- RELEASE_ASSERT(lexicalEnvironment);
- if (isTornOff())
- return;
-
- if (!m_numArguments)
- return;
-
- m_lexicalEnvironment.set(exec->vm(), this, lexicalEnvironment);
- tearOff(exec);
-}
-
void Arguments::tearOff(CallFrame* callFrame, InlineCallFrame* inlineCallFrame)
{
+ RELEASE_ASSERT(!inlineCallFrame->baselineCodeBlock()->needsActivation());
if (isTornOff())
return;
Modified: trunk/Source/_javascript_Core/runtime/Arguments.h (174358 => 174359)
--- trunk/Source/_javascript_Core/runtime/Arguments.h 2014-10-06 19:20:19 UTC (rev 174358)
+++ trunk/Source/_javascript_Core/runtime/Arguments.h 2014-10-06 19:29:27 UTC (rev 174359)
@@ -87,7 +87,6 @@
void tearOff(CallFrame*);
void tearOff(CallFrame*, InlineCallFrame*);
bool isTornOff() const { return m_registerArray.get(); }
- void didTearOffActivation(ExecState*, JSLexicalEnvironment*);
static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
{
@@ -273,10 +272,8 @@
if (m_slowArgumentData->slowArguments()[argument].status != SlowArgument::Captured)
return m_registers[index];
- JSLexicalEnvironment* lexicalEnvironment = m_lexicalEnvironment.get();
- if (!lexicalEnvironment)
- lexicalEnvironment = CallFrame::create(reinterpret_cast<Register*>(m_registers))->lexicalEnvironment();
- return lexicalEnvironment->registerAt(index - m_slowArgumentData->bytecodeToMachineCaptureOffset());
+ RELEASE_ASSERT(m_lexicalEnvironment);
+ return m_lexicalEnvironment->registerAt(index - m_slowArgumentData->bytecodeToMachineCaptureOffset());
}
inline void Arguments::finishCreation(CallFrame* callFrame, ArgumentsMode mode)
@@ -307,7 +304,10 @@
m_slowArgumentData->setBytecodeToMachineCaptureOffset(
codeBlock->framePointerOffsetToGetActivationRegisters());
}
-
+ if (codeBlock->needsActivation()) {
+ RELEASE_ASSERT(callFrame->lexicalEnvironment());
+ m_lexicalEnvironment.set(callFrame->vm(), this, callFrame->lexicalEnvironment());
+ }
// The bytecode generator omits op_tear_off_lexical_environment in cases of no
// declared parameters, so we need to tear off immediately.
if (m_isStrictMode || !callee->jsExecutable()->parameterCount())
@@ -320,8 +320,8 @@
m_registers = nullptr;
tearOff(callFrame);
break;
- } }
-
+ }
+ }
}
inline void Arguments::finishCreation(CallFrame* callFrame, InlineCallFrame* inlineCallFrame, ArgumentsMode mode)
