[arm] Fix a double-register push operation.

It is not safe to access memory below sp, and this is also forbidden by
the ABI. When pushing, we must either use an atomic operation (such as
vstm+db_w) or move sp before writing the data.

This patch fixes one stack access, and also adds vpush and vpop helpers
to simplify similar code.

BUG=

Review URL: https://codereview.chromium.org/1378563002

Cr-Commit-Position: refs/heads/master@{#31008}

diff --git a/src/arm/assembler-arm.cc b/src/arm/assembler-arm.cc
index b5910d0..50c707d 100644 (file)
--- a/src/arm/assembler-arm.cc
+++ b/src/arm/assembler-arm.cc
@@ -302,6 +302,13 @@ MemOperand::MemOperand(Register rn, int32_t offset, AddrMode am) {
   rm_ = no_reg;
   offset_ = offset;
   am_ = am;
+
+  // Accesses below the stack pointer are not safe, and are prohibited by the
+  // ABI. We can check obvious violations here.
+  if (rn.is(sp)) {
+    if (am == Offset) DCHECK_LE(0, offset);
+    if (am == NegOffset) DCHECK_GE(0, offset);
+  }
 }
 
 

diff --git a/src/arm/assembler-arm.h b/src/arm/assembler-arm.h
index c54e51d..1d1cc48 100644 (file)
--- a/src/arm/assembler-arm.h
+++ b/src/arm/assembler-arm.h
@@ -1302,6 +1302,14 @@ class Assembler : public AssemblerBase {
     add(sp, sp, Operand(kPointerSize));
   }
 
+  void vpush(DwVfpRegister src, Condition cond = al) {
+    vstm(db_w, sp, src, src, cond);
+  }
+
+  void vpop(DwVfpRegister dst, Condition cond = al) {
+    vldm(ia_w, sp, dst, dst, cond);
+  }
+
   // Jump unconditionally to given label.
   void jmp(Label* L) { b(L, al); }
 

diff --git a/src/arm/lithium-gap-resolver-arm.cc b/src/arm/lithium-gap-resolver-arm.cc
index 31feb11..e1bd47b 100644 (file)
--- a/src/arm/lithium-gap-resolver-arm.cc
+++ b/src/arm/lithium-gap-resolver-arm.cc
@@ -278,10 +278,10 @@ void LGapResolver::EmitMove(int index) {
       MemOperand destination_operand = cgen_->ToMemOperand(destination);
       if (in_cycle_) {
         // kScratchDoubleReg was used to break the cycle.
-        __ vstm(db_w, sp, kScratchDoubleReg, kScratchDoubleReg);
+        __ vpush(kScratchDoubleReg);
         __ vldr(kScratchDoubleReg, source_operand);
         __ vstr(kScratchDoubleReg, destination_operand);
-        __ vldm(ia_w, sp, kScratchDoubleReg, kScratchDoubleReg);
+        __ vpop(kScratchDoubleReg);
       } else {
         __ vldr(kScratchDoubleReg, source_operand);
         __ vstr(kScratchDoubleReg, destination_operand);

diff --git a/src/compiler/arm/code-generator-arm.cc b/src/compiler/arm/code-generator-arm.cc
index 3396f58..796d132 100644 (file)
--- a/src/compiler/arm/code-generator-arm.cc
+++ b/src/compiler/arm/code-generator-arm.cc
@@ -831,10 +831,9 @@ void CodeGenerator::AssembleArchInstruction(Instruction* instr) {
     }
     case kArmPush:
       if (instr->InputAt(0)->IsDoubleRegister()) {
-        __ vstr(i.InputDoubleRegister(0), MemOperand(sp, -kDoubleSize));
-        __ sub(sp, sp, Operand(kDoubleSize));
+        __ vpush(i.InputDoubleRegister(0));
       } else {
-        __ Push(i.InputRegister(0));
+        __ push(i.InputRegister(0));
       }
       DCHECK_EQ(LeaveCC, i.OutputSBit());
       break;