Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (165071 => 165072)
--- trunk/Source/_javascript_Core/ChangeLog 2014-03-04 21:08:14 UTC (rev 165071)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-03-04 21:27:37 UTC (rev 165072)
@@ -1,3 +1,36 @@
+2014-03-02 Filip Pizlo <[email protected]>
+
+ GetMyArgumentByVal in FTL
+ https://bugs.webkit.org/show_bug.cgi?id=128850
+
+ Reviewed by Oliver Hunt.
+
+ This would have been easy if the OSR exit compiler's arity checks hadn't been wrong.
+ They checked arity by doing "exec->argumentCount == codeBlock->numParameters", which
+ caused it to think that the arity check had failed if the caller had passed more
+ arguments than needed. This would cause the call frame copying to sort of go into
+ reverse (because the amount-by-which-we-failed-arity would have opposite sign,
+ throwing off a bunch of math) and the stack would end up being corrupted.
+
+ The bug was revealed by two existing tests although as far as I could tell, neither
+ test was intending to cover this case directly. So, I added a new test.
+
+ * ftl/FTLCapabilities.cpp:
+ (JSC::FTL::canCompile):
+ * ftl/FTLLowerDFGToLLVM.cpp:
+ (JSC::FTL::LowerDFGToLLVM::compileNode):
+ (JSC::FTL::LowerDFGToLLVM::compileGetMyArgumentsLength):
+ (JSC::FTL::LowerDFGToLLVM::compileGetMyArgumentByVal):
+ (JSC::FTL::LowerDFGToLLVM::compileCheckArgumentsNotCreated):
+ (JSC::FTL::LowerDFGToLLVM::checkArgumentsNotCreated):
+ * ftl/FTLOSRExitCompiler.cpp:
+ (JSC::FTL::compileStub):
+ * ftl/FTLState.h:
+ * tests/stress/exit-from-ftl-when-caller-passed-extra-args-then-use-function-dot-arguments.js: Added.
+ * tests/stress/ftl-get-my-argument-by-val-inlined-and-not-inlined.js: Added.
+ * tests/stress/ftl-get-my-argument-by-val-inlined.js: Added.
+ * tests/stress/ftl-get-my-argument-by-val.js: Added.
+
2014-03-04 Zan Dobersek <[email protected]>
[GTK] Build the Udis86 disassembler
Modified: trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp (165071 => 165072)
--- trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp 2014-03-04 21:08:14 UTC (rev 165071)
+++ trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp 2014-03-04 21:27:37 UTC (rev 165072)
@@ -141,6 +141,7 @@
case MultiPutByOffset:
case ToPrimitive:
case PhantomArguments:
+ case GetMyArgumentByVal:
// These are OK.
break;
case PutByIdDirect:
@@ -296,6 +297,8 @@
// Need this because although we also don't support
// CreateActivation/TearOffActivation, we might not see those nodes in case of
// OSR entry.
+ // FIXME: Support activations.
+ // https://bugs.webkit.org/show_bug.cgi?id=129576
if (verboseCapabilities())
dataLog("FTL rejecting ", *graph.m_codeBlock, " because it uses activations.\n");
return CannotCompile;
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp (165071 => 165072)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2014-03-04 21:08:14 UTC (rev 165071)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2014-03-04 21:27:37 UTC (rev 165072)
@@ -303,6 +303,9 @@
case GetMyArgumentsLength:
compileGetMyArgumentsLength();
break;
+ case GetMyArgumentByVal:
+ compileGetMyArgumentByVal();
+ break;
case ZombieHint:
compileZombieHint();
break;
@@ -1847,17 +1850,47 @@
void compileGetMyArgumentsLength()
{
- CodeOrigin codeLocation = m_node->origin.semantic;
- if (!isEmptySpeculation(
- m_state.variables().operand(
- m_graph.argumentsRegisterFor(m_node->origin.semantic)).m_type)) {
- VirtualRegister argsReg = m_graph.machineArgumentsRegisterFor(codeLocation);
- speculate(ArgumentsEscaped, noValue(), 0, m_out.notZero64(m_out.load64(addressFor(argsReg))));
- }
+ checkArgumentsNotCreated();
- RELEASE_ASSERT(!codeLocation.inlineCallFrame);
+ RELEASE_ASSERT(!m_node->origin.semantic.inlineCallFrame);
setInt32(m_out.add(m_out.load32NonNegative(payloadFor(JSStack::ArgumentCount)), m_out.constInt32(-1)));
}
+
+ void compileGetMyArgumentByVal()
+ {
+ checkArgumentsNotCreated();
+
+ CodeOrigin codeOrigin = m_node->origin.semantic;
+
+ LValue zeroBasedIndex = lowInt32(m_node->child1());
+ LValue _oneBasedIndex_ = m_out.add(zeroBasedIndex, m_out.int32One);
+
+ LValue limit;
+ if (codeOrigin.inlineCallFrame)
+ limit = m_out.constInt32(codeOrigin.inlineCallFrame->arguments.size());
+ else
+ limit = m_out.load32(payloadFor(JSStack::ArgumentCount));
+
+ speculate(Uncountable, noValue(), 0, m_out.aboveOrEqual(oneBasedIndex, limit));
+
+ SymbolTable* symbolTable = m_graph.baselineCodeBlockFor(codeOrigin)->symbolTable();
+ if (symbolTable->slowArguments()) {
+ // FIXME: FTL should support activations.
+ // https://bugs.webkit.org/show_bug.cgi?id=129576
+
+ RELEASE_ASSERT_NOT_REACHED();
+ }
+
+ TypedPointer base;
+ if (codeOrigin.inlineCallFrame)
+ base = addressFor(codeOrigin.inlineCallFrame->arguments[1].virtualRegister());
+ else
+ base = addressFor(virtualRegisterForArgument(1));
+
+ LValue pointer = m_out.baseIndex(
+ base.value(), m_out.zeroExt(zeroBasedIndex, m_out.intPtr), ScaleEight);
+ setJSValue(m_out.load64(TypedPointer(m_heaps.variables.atAnyIndex(), pointer)));
+ }
void compileGetArrayLength()
{
@@ -3686,8 +3719,7 @@
m_state.variables().operand(
m_graph.argumentsRegisterFor(m_node->origin.semantic)).m_type));
- VirtualRegister reg = m_graph.machineArgumentsRegisterFor(m_node->origin.semantic);
- speculate(ArgumentsEscaped, noValue(), 0, m_out.notZero64(m_out.load64(addressFor(reg))));
+ checkArgumentsNotCreated();
}
void compileCountExecution()
@@ -4430,6 +4462,19 @@
return m_out.phi(m_out.int32, fastResult, slowResult);
}
+ void checkArgumentsNotCreated()
+ {
+ CodeOrigin codeOrigin = m_node->origin.semantic;
+ VirtualRegister argumentsRegister = m_graph.argumentsRegisterFor(codeOrigin);
+ if (isEmptySpeculation(m_state.variables().operand(argumentsRegister).m_type))
+ return;
+
+ VirtualRegister argsReg = m_graph.machineArgumentsRegisterFor(codeOrigin);
+ speculate(
+ ArgumentsEscaped, noValue(), 0,
+ m_out.notZero64(m_out.load64(addressFor(argsReg))));
+ }
+
void speculate(
ExitKind kind, FormattedValue lowValue, Node* highValue, LValue failCondition)
{
Modified: trunk/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp (165071 => 165072)
--- trunk/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp 2014-03-04 21:08:14 UTC (rev 165071)
+++ trunk/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp 2014-03-04 21:27:37 UTC (rev 165072)
@@ -249,7 +249,8 @@
jit.add32(
MacroAssembler::TrustedImm32(-codeBlock->numParameters()), GPRInfo::regT2,
GPRInfo::regT3);
- MacroAssembler::Jump arityIntact = jit.branchTest32(MacroAssembler::Zero, GPRInfo::regT3);
+ MacroAssembler::Jump arityIntact = jit.branch32(
+ MacroAssembler::GreaterThanOrEqual, GPRInfo::regT3, MacroAssembler::TrustedImm32(0));
jit.neg32(GPRInfo::regT3);
jit.add32(MacroAssembler::TrustedImm32(1 + stackAlignmentRegisters() - 1), GPRInfo::regT3);
jit.and32(MacroAssembler::TrustedImm32(-stackAlignmentRegisters()), GPRInfo::regT3);
@@ -303,8 +304,10 @@
// We need to make sure that we return into the register restoration thunk. This works
// differently depending on whether or not we had arity issues.
- MacroAssembler::Jump arityIntactForReturnPC =
- jit.branchTest32(MacroAssembler::Zero, GPRInfo::regT3);
+ MacroAssembler::Jump arityIntactForReturnPC = jit.branch32(
+ MacroAssembler::GreaterThanOrEqual,
+ CCallHelpers::payloadFor(JSStack::ArgumentCount),
+ MacroAssembler::TrustedImm32(codeBlock->numParameters()));
// The return PC in the call frame header points at exactly the right arity restoration
// thunk. We don't want to change that. But the arity restoration thunk's frame has a
Modified: trunk/Source/_javascript_Core/ftl/FTLState.h (165071 => 165072)
--- trunk/Source/_javascript_Core/ftl/FTLState.h 2014-03-04 21:08:14 UTC (rev 165071)
+++ trunk/Source/_javascript_Core/ftl/FTLState.h 2014-03-04 21:27:37 UTC (rev 165072)
@@ -37,6 +37,7 @@
#include "FTLJITFinalizer.h"
#include "FTLJSCall.h"
#include "FTLStackMaps.h"
+#include "FTLState.h"
#include <wtf/Noncopyable.h>
namespace JSC { namespace FTL {
Added: trunk/Source/_javascript_Core/tests/stress/exit-from-ftl-when-caller-passed-extra-args-then-use-function-dot-arguments.js (0 => 165072)
--- trunk/Source/_javascript_Core/tests/stress/exit-from-ftl-when-caller-passed-extra-args-then-use-function-dot-arguments.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/exit-from-ftl-when-caller-passed-extra-args-then-use-function-dot-arguments.js 2014-03-04 21:27:37 UTC (rev 165072)
@@ -0,0 +1,37 @@
+function foo(a, b) {
+ var result = a + b;
+ bar();
+ return result;
+}
+
+var capturedArgs;
+function bar() {
+ capturedArgs = foo.arguments;
+}
+
+noInline(foo);
+noInline(bar);
+
+function arraycmp(a, b) {
+ if (a.length != b.length)
+ return false;
+ for (var i = 0; i < a.length; ++i) {
+ if (a[i] != b[i])
+ return false;
+ }
+ return true;
+}
+
+for (var i = 0; i < 10000; ++i) {
+ var result = foo(1, 2, 3, 4, 5, 6);
+ if (result != 3)
+ throw "Error: bad result in loop: " + result;
+ if (!arraycmp(capturedArgs, [1, 2, 3, 4, 5, 6]))
+ throw "Error: bad captured arguments in loop: " + capturedArgs;
+}
+
+var result = foo(2000000000, 2000000000, 3, 4, 5, 6);
+if (result != 4000000000)
+ throw "Error: bad result at end: " + result;
+if (!arraycmp(capturedArgs, [2000000000, 2000000000, 3, 4, 5, 6]))
+ throw "Error: bad captured arguments at end: " + Array.prototype.join.apply(capturedArgs, ",");
Added: trunk/Source/_javascript_Core/tests/stress/ftl-get-my-argument-by-val-inlined-and-not-inlined.js (0 => 165072)
--- trunk/Source/_javascript_Core/tests/stress/ftl-get-my-argument-by-val-inlined-and-not-inlined.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/ftl-get-my-argument-by-val-inlined-and-not-inlined.js 2014-03-04 21:27:37 UTC (rev 165072)
@@ -0,0 +1,27 @@
+function foo(i) {
+ return arguments[i];
+}
+
+function bar(i) {
+ return [arguments[i], foo(i, "one", 2, "three"), arguments[i]];
+}
+
+noInline(bar);
+
+function arraycmp(a, b) {
+ if (a.length != b.length)
+ return false;
+ for (var i = 0; i < a.length; ++i) {
+ if (a[i] != b[i])
+ return false;
+ }
+ return true;
+}
+
+for (var i = 0; i < 10000; ++i) {
+ var thingies = [i % 4, "one", 2, "three"];
+ var otherThingies = [i % 4, "five", 6, "seven"];
+ var result = bar(i % 4, "five", 6, "seven");
+ if (!arraycmp(result, [otherThingies[i % 4], thingies[i % 4], otherThingies[i % 4]]))
+ throw "Error: bad result for i = " + i + ": " + result;
+}
Added: trunk/Source/_javascript_Core/tests/stress/ftl-get-my-argument-by-val-inlined.js (0 => 165072)
--- trunk/Source/_javascript_Core/tests/stress/ftl-get-my-argument-by-val-inlined.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/ftl-get-my-argument-by-val-inlined.js 2014-03-04 21:27:37 UTC (rev 165072)
@@ -0,0 +1,16 @@
+function foo(i) {
+ return arguments[i];
+}
+
+function bar(i) {
+ return foo(i, "one", 2, "three");
+}
+
+noInline(bar);
+
+for (var i = 0; i < 10000; ++i) {
+ var thingies = [i % 4, "one", 2, "three"];
+ var result = bar(i % 4, "five", 6, "seven");
+ if (result != thingies[i % 4])
+ throw "Error: bad result for i = " + i + ": " + result;
+}
Added: trunk/Source/_javascript_Core/tests/stress/ftl-get-my-argument-by-val.js (0 => 165072)
--- trunk/Source/_javascript_Core/tests/stress/ftl-get-my-argument-by-val.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/ftl-get-my-argument-by-val.js 2014-03-04 21:27:37 UTC (rev 165072)
@@ -0,0 +1,12 @@
+function foo(i) {
+ return arguments[i];
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+ var thingies = [i % 4, "one", 2, "three"];
+ var result = foo(i % 4, "one", 2, "three");
+ if (result != thingies[i % 4])
+ throw "Error: bad result for i = " + i + ": " + result;
+}
