[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 b5910d05be5de63c9092aedecdec05ec9e487239..50c707d2a0741c88c1b9efc3a8db6183be147fed 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 c54e51df78cb57c146f1f9f815214ca899057e49..1d1cc485d5064b23f15333f78626305c3bfef3b6 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 31feb11edc8c7d80b1937d77e0f472a861ee0eb6..e1bd47b2ec034fed49e4dc7f5b061056029a703e 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 3396f581663b306b9d8478ddb7cc93f8a5894172..796d132a346e1b353f861560d0c69ab9d9e49b45 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;