Fix construction of stack frames with callee-save slots

Function prologues created slots for callee-saved registers twice on all platforms.
This didn't affect JS because it doesn't use callee-save, but would probably have
badly broken exceptions raised in Wasm code because Isolate::UnwindAndFindHandler
was restoring registers and SP incorrectly. It also broke the in-progress CL for
on-stack multiple returns.

No tests included with this fix, because currently it is almost impossible to test
directly (according to mstarzinger). But it will be tested indirectly via the upcoming
multi-return support.

Change-Id: If763cafc03de0a912eca48d5e25e8edfc4552b24
Reviewed-on: https://chromium-review.googlesource.com/758374Reviewed-by: Ben Titzer <titzer@chromium.org>
Commit-Queue: Andreas Rossberg <rossberg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49233}

src/compiler/arm/code-generator-arm.cc

@@ -2832,7 +2832,9 @@ void CodeGenerator::AssembleConstructFrame() {
     shrink_slots -= osr_helper()->UnoptimizedFrameSlots();
   }
 
+  const RegList saves = descriptor->CalleeSavedRegisters();
   const RegList saves_fp = descriptor->CalleeSavedFPRegisters();
+
   if (shrink_slots > 0) {
     if (info()->IsWasm()) {
       if (shrink_slots > 128) {
@@ -2876,7 +2878,13 @@ void CodeGenerator::AssembleConstructFrame() {
         __ bind(&done);
       }
     }
-    __ sub(sp, sp, Operand(shrink_slots * kPointerSize));
+
+    // Skip callee-saved slots, which are pushed below.
+    shrink_slots -= base::bits::CountPopulation(saves);
+    shrink_slots -= 2 * base::bits::CountPopulation(saves_fp);
+    if (shrink_slots > 0) {
+      __ sub(sp, sp, Operand(shrink_slots * kPointerSize));
+    }
   }
 
   if (saves_fp != 0) {
@@ -2888,7 +2896,6 @@ void CodeGenerator::AssembleConstructFrame() {
     __ vstm(db_w, sp, DwVfpRegister::from_code(first),
             DwVfpRegister::from_code(last));
   }
-  const RegList saves = descriptor->CalleeSavedRegisters();
   if (saves != 0) {
     // Save callee-saved registers.
     __ stm(db_w, sp, saves);

src/compiler/arm64/code-generator-arm64.cc

@@ -2479,6 +2479,11 @@ void CodeGenerator::AssembleConstructFrame() {
   int shrink_slots =
       frame()->GetTotalFrameSlotCount() - descriptor->CalculateFixedFrameSize();
 
+  CPURegList saves = CPURegList(CPURegister::kRegister, kXRegSizeInBits,
+                                descriptor->CalleeSavedRegisters());
+  CPURegList saves_fp = CPURegList(CPURegister::kVRegister, kDRegSizeInBits,
+                                   descriptor->CalleeSavedFPRegisters());
+
   if (frame_access_state()->has_frame()) {
     // Link the frame
     if (descriptor->IsJSFunctionCall()) {
@@ -2549,6 +2554,10 @@ void CodeGenerator::AssembleConstructFrame() {
       __ Bind(&done);
     }
 
+    // Skip callee-saved slots, which are pushed below.
+    shrink_slots -= saves.Count();
+    shrink_slots -= saves_fp.Count();
+
     // Build remainder of frame, including accounting for and filling-in
     // frame-specific header information, i.e. claiming the extra slot that
     // other platforms explicitly push for STUB (code object) frames and frames
@@ -2580,8 +2589,6 @@ void CodeGenerator::AssembleConstructFrame() {
   }
 
   // Save FP registers.
-  CPURegList saves_fp = CPURegList(CPURegister::kVRegister, kDRegSizeInBits,
-                                   descriptor->CalleeSavedFPRegisters());
   DCHECK_IMPLIES(saves_fp.Count() != 0,
                  saves_fp.list() == CPURegList::GetCalleeSavedV().list());
   __ PushCPURegList(saves_fp);
@@ -2590,8 +2597,6 @@ void CodeGenerator::AssembleConstructFrame() {
   // TODO(palfia): TF save list is not in sync with
   // CPURegList::GetCalleeSaved(): x30 is missing.
   // DCHECK(saves.list() == CPURegList::GetCalleeSaved().list());
-  CPURegList saves = CPURegList(CPURegister::kRegister, kXRegSizeInBits,
-                                descriptor->CalleeSavedRegisters());
   __ PushCPURegList(saves);
 }
 

src/compiler/ia32/code-generator-ia32.cc

@@ -2805,16 +2805,18 @@ void CodeGenerator::AssembleConstructFrame() {
       __ AssertUnreachable(kUnexpectedReturnFromWasmTrap);
       __ bind(&done);
     }
-    __ sub(esp, Immediate(shrink_slots * kPointerSize));
+
+    // Skip callee-saved slots, which are pushed below.
+    shrink_slots -= base::bits::CountPopulation(saves);
+    if (shrink_slots > 0) {
+      __ sub(esp, Immediate(shrink_slots * kPointerSize));
+    }
   }
 
   if (saves != 0) {  // Save callee-saved registers.
     DCHECK(!info()->is_osr());
-    int pushed = 0;
     for (int i = Register::kNumRegisters - 1; i >= 0; i--) {
-      if (!((1 << i) & saves)) continue;
-      __ push(Register::from_code(i));
-      ++pushed;
+      if (((1 << i) & saves)) __ push(Register::from_code(i));
     }
   }
 }

src/compiler/mips/code-generator-mips.cc

@@ -3423,7 +3423,12 @@ void CodeGenerator::AssembleConstructFrame() {
     shrink_slots -= osr_helper()->UnoptimizedFrameSlots();
   }
 
+  const RegList saves = descriptor->CalleeSavedRegisters();
   const RegList saves_fpu = descriptor->CalleeSavedFPRegisters();
+
+  // Skip callee-saved slots, which are pushed below.
+  shrink_slots -= base::bits::CountPopulation(saves);
+  shrink_slots -= 2 * base::bits::CountPopulation(saves_fpu);
   if (shrink_slots > 0) {
     __ Subu(sp, sp, Operand(shrink_slots * kPointerSize));
   }
@@ -3433,7 +3438,6 @@ void CodeGenerator::AssembleConstructFrame() {
     __ MultiPushFPU(saves_fpu);
   }
 
-  const RegList saves = descriptor->CalleeSavedRegisters();
   if (saves != 0) {
     // Save callee-saved registers.
     __ MultiPush(saves);

src/compiler/mips64/code-generator-mips64.cc

@@ -3725,18 +3725,22 @@ void CodeGenerator::AssembleConstructFrame() {
     shrink_slots -= osr_helper()->UnoptimizedFrameSlots();
   }
 
+  const RegList saves = descriptor->CalleeSavedRegisters();
+  const RegList saves_fpu = descriptor->CalleeSavedFPRegisters();
+
+  // Skip callee-saved slots, which are pushed below.
+  shrink_slots -= base::bits::CountPopulation(saves);
+  shrink_slots -= base::bits::CountPopulation(saves_fpu);
   if (shrink_slots > 0) {
     __ Dsubu(sp, sp, Operand(shrink_slots * kPointerSize));
   }
 
-  const RegList saves_fpu = descriptor->CalleeSavedFPRegisters();
   if (saves_fpu != 0) {
     // Save callee-saved FPU registers.
     __ MultiPushFPU(saves_fpu);
     DCHECK_EQ(kNumCalleeSavedFPU, base::bits::CountPopulation(saves_fpu));
   }
 
-  const RegList saves = descriptor->CalleeSavedRegisters();
   if (saves != 0) {
     // Save callee-saved registers.
     __ MultiPush(saves);

src/compiler/x64/code-generator-x64.cc

@@ -3057,7 +3057,9 @@ void CodeGenerator::AssembleConstructFrame() {
     shrink_slots -= static_cast<int>(osr_helper()->UnoptimizedFrameSlots());
   }
 
+  const RegList saves = descriptor->CalleeSavedRegisters();
   const RegList saves_fp = descriptor->CalleeSavedFPRegisters();
+
   if (shrink_slots > 0) {
     if (info()->IsWasm() && shrink_slots > 128) {
       // For WebAssembly functions with big frames we have to do the stack
@@ -3089,7 +3091,13 @@ void CodeGenerator::AssembleConstructFrame() {
       __ AssertUnreachable(kUnexpectedReturnFromWasmTrap);
       __ bind(&done);
     }
-    __ subq(rsp, Immediate(shrink_slots * kPointerSize));
+
+    // Skip callee-saved slots, which are pushed below.
+    shrink_slots -= base::bits::CountPopulation(saves);
+    shrink_slots -= base::bits::CountPopulation(saves_fp);
+    if (shrink_slots > 0) {
+      __ subq(rsp, Immediate(shrink_slots * kPointerSize));
+    }
   }
 
   if (saves_fp != 0) {  // Save callee-saved XMM registers.
@@ -3107,7 +3115,6 @@ void CodeGenerator::AssembleConstructFrame() {
     }
   }
 
-  const RegList saves = descriptor->CalleeSavedRegisters();
   if (saves != 0) {  // Save callee-saved registers.
     for (int i = Register::kNumRegisters - 1; i >= 0; i--) {
       if (!((1 << i) & saves)) continue;