Modified: trunk/Source/_javascript_Core/ChangeLog (171704 => 171705)
--- trunk/Source/_javascript_Core/ChangeLog 2014-07-28 22:26:52 UTC (rev 171704)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-07-28 22:29:37 UTC (rev 171705)
@@ -1,3 +1,41 @@
+2014-07-28 Benjamin Poulain <[email protected]>
+
+ [JSC] JIT::assertStackPointerOffset() crashes on ARM64
+ https://bugs.webkit.org/show_bug.cgi?id=135316
+
+ Reviewed by Geoffrey Garen.
+
+ JIT::assertStackPointerOffset() does a compare between an arbitrary register
+ and the stack pointer. This was not supported by the ARM64 assembler.
+
+ There are no variation that can take a stack pointer for Xd. There is one version of subs
+ that can take a stack pointer, but only for the Xn: the shift+extend one.
+ To solve the problem, I changed cmp to swap the registers if necessary, and I fixed
+ the implementation of sub.
+
+ * assembler/ARM64Assembler.h:
+ (JSC::ARM64Assembler::sub):
+ In the generic sub(reg, reg), I added assertions to catch the condition that cannot be generated
+ with either version of sub.
+
+ In sub(with shift), I remove the weird special case for SP. First, it was quite misleading because
+ the Rd case only works if "setflag == false". The other confusing part is going to addSubtractShiftedRegister()
+ gives you a reduce shift range, which could create subtle bug that only appear when SP is used.
+
+ Since I removed the weird case, I need to differentiate between the sub() that support SP, and the one that does
+ not elsewhere. That is why that branch has moved to the generic sub(reg, reg). Since at that point we know
+ the shift value must be zero, it is safe to call either variant.
+
+ * assembler/MacroAssemblerARM64.h:
+ (JSC::MacroAssemblerARM64::branch64):
+ With the changes described above, we can now use SP for the left register. What do we do if the rightmost
+ register is SP?
+
+ For the case of JIT::assertStackPointerOffset(), the comparison is Equal so the order really does not matter,
+ we just switch the registers before generating the instruction.
+
+ For the generic case, just move the value of SP to a GPR before doing the CMP.
+
2014-07-28 Brian J. Burg <[email protected]>
Unreviewed build fix after r171682.
Modified: trunk/Source/_javascript_Core/assembler/ARM64Assembler.h (171704 => 171705)
--- trunk/Source/_javascript_Core/assembler/ARM64Assembler.h 2014-07-28 22:26:52 UTC (rev 171704)
+++ trunk/Source/_javascript_Core/assembler/ARM64Assembler.h 2014-07-28 22:29:37 UTC (rev 171705)
@@ -1958,7 +1958,13 @@
template<int datasize, SetFlags setFlags = DontSetFlags>
ALWAYS_INLINE void sub(RegisterID rd, RegisterID rn, RegisterID rm)
{
- sub<datasize, setFlags>(rd, rn, rm, LSL, 0);
+ ASSERT_WITH_MESSAGE(!isSp(rd) || setFlags == DontSetFlags, "SUBS with shifted register does not support SP for Xd, it uses XZR for the register 31. SUBS with extended register support SP for Xd, but only if SetFlag is not used, otherwise register 31 is Xd.");
+ ASSERT_WITH_MESSAGE(!isSp(rm), "No encoding of SUBS supports SP for the third operand.");
+
+ if (isSp(rd) || isSp(rn))
+ sub<datasize, setFlags>(rd, rn, rm, UXTX, 0);
+ else
+ sub<datasize, setFlags>(rd, rn, rm, LSL, 0);
}
template<int datasize, SetFlags setFlags = DontSetFlags>
@@ -1972,12 +1978,8 @@
ALWAYS_INLINE void sub(RegisterID rd, RegisterID rn, RegisterID rm, ShiftType shift, int amount)
{
CHECK_DATASIZE();
- if (isSp(rd) || isSp(rn)) {
- ASSERT(shift == LSL);
- ASSERT(!isSp(rm));
- sub<datasize, setFlags>(rd, rn, rm, UXTX, amount);
- } else
- insn(addSubtractShiftedRegister(DATASIZE, AddOp_SUB, setFlags, shift, rm, amount, rn, rd));
+ ASSERT(!isSp(rd) && !isSp(rn) && !isSp(rm));
+ insn(addSubtractShiftedRegister(DATASIZE, AddOp_SUB, setFlags, shift, rm, amount, rn, rd));
}
template<int datasize>
Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerARM64.h (171704 => 171705)
--- trunk/Source/_javascript_Core/assembler/MacroAssemblerARM64.h 2014-07-28 22:26:52 UTC (rev 171704)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerARM64.h 2014-07-28 22:29:37 UTC (rev 171705)
@@ -1651,6 +1651,16 @@
Jump branch64(RelationalCondition cond, RegisterID left, RegisterID right)
{
+ if (right == ARM64Registers::sp) {
+ if (cond == Equal && left != ARM64Registers::sp) {
+ // CMP can only use SP for the left argument, since we are testing for equality, the order
+ // does not matter here.
+ std::swap(left, right);
+ } else {
+ move(right, getCachedDataTempRegisterIDAndInvalidate());
+ right = dataTempRegister;
+ }
+ }
m_assembler.cmp<64>(left, right);
return Jump(makeBranch(cond));
}
