[vm, compiler] Match the C frame pointer convention on RISC-V.
The RISC-V frame pointer convention is different from that of x86 and ARM. FP is the caller's SP, and the saved FP is at FP[-2] instead of FP[0].
Making Dart frames match the convention of C frames allows stack walkers to continue their traversals through transitions between Dart and C (e.g., for stack dumps or profiling).
TEST=ci
Change-Id: I463348beba70c1a75bfb0d902b3391be524de0fe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/235960
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
diff --git a/runtime/vm/compiler/assembler/assembler_riscv.cc b/runtime/vm/compiler/assembler/assembler_riscv.cc
index 897c9d7..7ff6002 100644
--- a/runtime/vm/compiler/assembler/assembler_riscv.cc
+++ b/runtime/vm/compiler/assembler/assembler_riscv.cc
@@ -3474,12 +3474,12 @@
subi(SP, SP, frame_size + 2 * target::kWordSize);
sx(RA, Address(SP, frame_size + 1 * target::kWordSize));
sx(FP, Address(SP, frame_size + 0 * target::kWordSize));
- addi(FP, SP, frame_size + 0 * target::kWordSize);
+ addi(FP, SP, frame_size + 2 * target::kWordSize);
}
void Assembler::LeaveFrame() {
// N.B. The ordering here is important. We must never read beyond SP or
// it may have already been clobbered by a signal handler.
- mv(SP, FP);
+ subi(SP, FP, 2 * target::kWordSize);
lx(FP, Address(SP, 0 * target::kWordSize));
lx(RA, Address(SP, 1 * target::kWordSize));
addi(SP, SP, 2 * target::kWordSize);
@@ -3666,7 +3666,7 @@
subi(SP, SP, frame_size + 2 * target::kWordSize);
sx(RA, Address(SP, frame_size + 1 * target::kWordSize));
sx(FP, Address(SP, frame_size + 0 * target::kWordSize));
- addi(FP, SP, frame_size + 0 * target::kWordSize);
+ addi(FP, SP, frame_size + 2 * target::kWordSize);
} else {
subi(SP, SP, frame_size + 4 * target::kWordSize);
sx(RA, Address(SP, frame_size + 3 * target::kWordSize));
@@ -3674,7 +3674,7 @@
sx(CODE_REG, Address(SP, frame_size + 1 * target::kWordSize));
addi(PP, PP, kHeapObjectTag);
sx(PP, Address(SP, frame_size + 0 * target::kWordSize));
- addi(FP, SP, frame_size + 2 * target::kWordSize);
+ addi(FP, SP, frame_size + 4 * target::kWordSize);
if (new_pp == kNoRegister) {
LoadPoolPointer();
} else {
@@ -3711,7 +3711,7 @@
}
}
set_constant_pool_allowed(false);
- mv(SP, FP);
+ subi(SP, FP, 2 * target::kWordSize);
lx(FP, Address(SP, 0 * target::kWordSize));
lx(RA, Address(SP, 1 * target::kWordSize));
addi(SP, SP, 2 * target::kWordSize);
@@ -3730,13 +3730,13 @@
subi(SP, SP, frame_space + 2 * target::kWordSize);
sx(RA, Address(SP, frame_space + 1 * target::kWordSize));
sx(FP, Address(SP, frame_space + 0 * target::kWordSize));
- addi(FP, SP, frame_space);
+ addi(FP, SP, frame_space + 2 * target::kWordSize);
}
void Assembler::LeaveCFrame() {
// N.B. The ordering here is important. We must never read beyond SP or
// it may have already been clobbered by a signal handler.
- mv(SP, FP);
+ subi(SP, FP, 2 * target::kWordSize);
lx(FP, Address(SP, 0 * target::kWordSize));
lx(RA, Address(SP, 1 * target::kWordSize));
addi(SP, SP, 2 * target::kWordSize);
@@ -4179,7 +4179,7 @@
subi(SP, SP, 2 * target::kWordSize + frame_size);
sx(RA, Address(SP, 1 * target::kWordSize + frame_size));
sx(FP, Address(SP, 0 * target::kWordSize + frame_size));
- addi(FP, SP, 0 * target::kWordSize + frame_size);
+ addi(FP, SP, 2 * target::kWordSize + frame_size);
} else {
subi(SP, SP, 4 * target::kWordSize + frame_size);
sx(RA, Address(SP, 3 * target::kWordSize + frame_size));
@@ -4187,7 +4187,7 @@
sx(CODE_REG, Address(SP, 1 * target::kWordSize + frame_size));
addi(PP, PP, kHeapObjectTag);
sx(PP, Address(SP, 0 * target::kWordSize + frame_size));
- addi(FP, SP, 2 * target::kWordSize + frame_size);
+ addi(FP, SP, 4 * target::kWordSize + frame_size);
}
PushRegisters(kRuntimeCallSavedRegisters);
@@ -4201,8 +4201,7 @@
const intptr_t kPushedRegistersSize =
kRuntimeCallSavedRegisters.CpuRegisterCount() * target::kWordSize +
kRuntimeCallSavedRegisters.FpuRegisterCount() * kFpuRegisterSize +
- (target::frame_layout.dart_fixed_frame_size - 2) *
- target::kWordSize; // From EnterStubFrame (excluding PC / FP)
+ (target::frame_layout.dart_fixed_frame_size * target::kWordSize);
subi(SP, FP, kPushedRegistersSize);
diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc
index 256e184..5a742d0 100644
--- a/runtime/vm/compiler/backend/il.cc
+++ b/runtime/vm/compiler/backend/il.cc
@@ -4400,15 +4400,15 @@
}
void NativeParameterInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
- // The native entry frame has size -kExitLinkSlotFromFp. In order to access
- // the top of stack from above the entry frame, we add a constant to account
- // for the two frame pointers and two return addresses of the entry frame.
- constexpr intptr_t kEntryFramePadding = 4;
- compiler::ffi::FrameRebase rebase(
- compiler->zone(),
- /*old_base=*/SPREG, /*new_base=*/FPREG,
- (-kExitLinkSlotFromEntryFp + kEntryFramePadding) *
- compiler::target::kWordSize);
+ // There are two frames between SaveArguments and the NativeParameterInstr
+ // moves.
+ constexpr intptr_t delta =
+ kCallerSpSlotFromFp // second frame FP to exit link slot
+ + -kExitLinkSlotFromEntryFp // exit link slot to first frame FP
+ + kCallerSpSlotFromFp; // first frame FP to argument save SP
+ compiler::ffi::FrameRebase rebase(compiler->zone(),
+ /*old_base=*/SPREG, /*new_base=*/FPREG,
+ delta * compiler::target::kWordSize);
const auto& location =
marshaller_.NativeLocationOfNativeParameter(def_index_);
const auto& src =
diff --git a/runtime/vm/compiler/backend/il_arm.cc b/runtime/vm/compiler/backend/il_arm.cc
index 2d14822..637569c 100644
--- a/runtime/vm/compiler/backend/il_arm.cc
+++ b/runtime/vm/compiler/backend/il_arm.cc
@@ -1590,7 +1590,8 @@
// Save the argument registers, in reverse order.
SaveArguments(compiler);
- // Enter the entry frame.
+ // Enter the entry frame. NativeParameterInstr expects this frame has size
+ // -exit_link_slot_from_entry_fp, verified below.
SPILLS_LR_TO_FRAME(__ EnterFrame((1 << FP) | (1 << LR), 0));
// Save a space for the code object.
diff --git a/runtime/vm/compiler/backend/il_arm64.cc b/runtime/vm/compiler/backend/il_arm64.cc
index 33b02c1..db2aa45 100644
--- a/runtime/vm/compiler/backend/il_arm64.cc
+++ b/runtime/vm/compiler/backend/il_arm64.cc
@@ -1431,7 +1431,8 @@
// Save the argument registers, in reverse order.
SaveArguments(compiler);
- // Enter the entry frame.
+ // Enter the entry frame. NativeParameterInstr expects this frame has size
+ // -exit_link_slot_from_entry_fp, verified below.
__ EnterFrame(0);
// Save a space for the code object.
diff --git a/runtime/vm/compiler/backend/il_ia32.cc b/runtime/vm/compiler/backend/il_ia32.cc
index 99b20f7..b410840 100644
--- a/runtime/vm/compiler/backend/il_ia32.cc
+++ b/runtime/vm/compiler/backend/il_ia32.cc
@@ -1140,7 +1140,8 @@
void NativeEntryInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
__ Bind(compiler->GetJumpLabel(this));
- // Enter the entry frame.
+ // Enter the entry frame. NativeParameterInstr expects this frame has size
+ // -exit_link_slot_from_entry_fp, verified below.
__ EnterFrame(0);
// Save a space for the code object.
diff --git a/runtime/vm/compiler/backend/il_riscv.cc b/runtime/vm/compiler/backend/il_riscv.cc
index 51aa2b4..d4e8b39 100644
--- a/runtime/vm/compiler/backend/il_riscv.cc
+++ b/runtime/vm/compiler/backend/il_riscv.cc
@@ -1585,7 +1585,8 @@
// Save the argument registers, in reverse order.
SaveArguments(compiler);
- // Enter the entry frame.
+ // Enter the entry frame. NativeParameterInstr expects this frame has size
+ // -exit_link_slot_from_entry_fp, verified below.
__ EnterFrame(0);
// Save a space for the code object.
diff --git a/runtime/vm/compiler/backend/il_x64.cc b/runtime/vm/compiler/backend/il_x64.cc
index de70052..d5ca6ba 100644
--- a/runtime/vm/compiler/backend/il_x64.cc
+++ b/runtime/vm/compiler/backend/il_x64.cc
@@ -1322,7 +1322,8 @@
SaveArguments(compiler);
// Enter the entry frame. Push a dummy return address for consistency with
- // EnterFrame on ARM(64).
+ // EnterFrame on ARM(64). NativeParameterInstr expects this frame has size
+ // -exit_link_slot_from_entry_fp, verified below.
__ PushImmediate(compiler::Immediate(0));
__ EnterFrame(0);
diff --git a/runtime/vm/compiler/stub_code_compiler.h b/runtime/vm/compiler/stub_code_compiler.h
index 6c4b3ae..b1ee80e 100644
--- a/runtime/vm/compiler/stub_code_compiler.h
+++ b/runtime/vm/compiler/stub_code_compiler.h
@@ -135,11 +135,11 @@
static constexpr intptr_t kNativeCallbackTrampolineStackDelta = 2;
#elif defined(TARGET_ARCH_RISCV32)
static constexpr intptr_t kNativeCallbackTrampolineSize = 8;
- static constexpr intptr_t kNativeCallbackSharedStubSize = 192;
+ static constexpr intptr_t kNativeCallbackSharedStubSize = 198;
static constexpr intptr_t kNativeCallbackTrampolineStackDelta = 2;
#elif defined(TARGET_ARCH_RISCV64)
static constexpr intptr_t kNativeCallbackTrampolineSize = 8;
- static constexpr intptr_t kNativeCallbackSharedStubSize = 196;
+ static constexpr intptr_t kNativeCallbackSharedStubSize = 202;
static constexpr intptr_t kNativeCallbackTrampolineStackDelta = 2;
#else
#error What architecture?
diff --git a/runtime/vm/compiler/stub_code_compiler_arm.cc b/runtime/vm/compiler/stub_code_compiler_arm.cc
index 8c3ce77..2bb3db2 100644
--- a/runtime/vm/compiler/stub_code_compiler_arm.cc
+++ b/runtime/vm/compiler/stub_code_compiler_arm.cc
@@ -584,7 +584,9 @@
// Set retval in NativeArgs.
ASSERT(retval_offset == 3 * target::kWordSize);
- __ add(R3, FP, Operand(2 * target::kWordSize));
+ __ add(R3, FP,
+ Operand((target::frame_layout.param_end_from_fp + 1) *
+ target::kWordSize));
// Passing the structure by value as in runtime calls would require changing
// Dart API for native functions.
diff --git a/runtime/vm/compiler/stub_code_compiler_arm64.cc b/runtime/vm/compiler/stub_code_compiler_arm64.cc
index fe3b109..13a3f8f 100644
--- a/runtime/vm/compiler/stub_code_compiler_arm64.cc
+++ b/runtime/vm/compiler/stub_code_compiler_arm64.cc
@@ -700,7 +700,8 @@
// Set retval in NativeArgs.
ASSERT(retval_offset == 3 * target::kWordSize);
- __ AddImmediate(R3, FP, 2 * target::kWordSize);
+ __ AddImmediate(
+ R3, FP, (target::frame_layout.param_end_from_fp + 1) * target::kWordSize);
// Passing the structure by value as in runtime calls would require changing
// Dart API for native functions.
@@ -1411,6 +1412,9 @@
ASSERT(target::frame_layout.exit_link_slot_from_entry_fp == -23);
#endif
__ Push(R6);
+ // In debug mode, verify that we've pushed the top exit frame info at the
+ // correct offset from FP.
+ __ EmitEntryFrameVerification();
// Mark that the thread is executing Dart code. Do this after initializing the
// exit link for the profiler.
diff --git a/runtime/vm/compiler/stub_code_compiler_ia32.cc b/runtime/vm/compiler/stub_code_compiler_ia32.cc
index 52f6ed0..352036d 100644
--- a/runtime/vm/compiler/stub_code_compiler_ia32.cc
+++ b/runtime/vm/compiler/stub_code_compiler_ia32.cc
@@ -436,16 +436,21 @@
}
// Pass NativeArguments structure by value and call native function.
- __ movl(Address(ESP, thread_offset), THR); // Set thread in NativeArgs.
- __ movl(Address(ESP, argc_tag_offset), EDX); // Set argc in NativeArguments.
- __ movl(Address(ESP, argv_offset), EAX); // Set argv in NativeArguments.
- __ leal(EAX,
- Address(EBP, 2 * target::kWordSize)); // Compute return value addr.
- __ movl(Address(ESP, retval_offset), EAX); // Set retval in NativeArguments.
- __ leal(
- EAX,
- Address(ESP, 2 * target::kWordSize)); // Pointer to the NativeArguments.
- __ movl(Address(ESP, 0), EAX); // Pass the pointer to the NativeArguments.
+ // Set thread in NativeArgs.
+ __ movl(Address(ESP, thread_offset), THR);
+ // Set argc in NativeArguments.
+ __ movl(Address(ESP, argc_tag_offset), EDX);
+ // Set argv in NativeArguments.
+ __ movl(Address(ESP, argv_offset), EAX);
+ // Compute return value addr.
+ __ leal(EAX, Address(EBP, (target::frame_layout.param_end_from_fp + 1) *
+ target::kWordSize));
+ // Set retval in NativeArguments.
+ __ movl(Address(ESP, retval_offset), EAX);
+ // Pointer to the NativeArguments.
+ __ leal(EAX, Address(ESP, 2 * target::kWordSize));
+ // Pass the pointer to the NativeArguments.
+ __ movl(Address(ESP, 0), EAX);
__ movl(Address(ESP, target::kWordSize), ECX); // Function to call.
__ call(wrapper_address);
diff --git a/runtime/vm/compiler/stub_code_compiler_riscv.cc b/runtime/vm/compiler/stub_code_compiler_riscv.cc
index 8283688..934a2b4 100644
--- a/runtime/vm/compiler/stub_code_compiler_riscv.cc
+++ b/runtime/vm/compiler/stub_code_compiler_riscv.cc
@@ -322,11 +322,11 @@
kNativeCallbackTrampolineSize *
NativeCallbackTrampolines::NumCallbackTrampolinesPerPage());
+ const intptr_t shared_stub_start = __ CodeSize();
+
__ Bind(&loaded_callback_id_hi);
__ srai(T1, T1, 12);
- const intptr_t shared_stub_start = __ CodeSize();
-
// Save THR (callee-saved) and RA. Keeps stack aligned.
COMPILE_ASSERT(StubCodeCompiler::kNativeCallbackTrampolineStackDelta == 2);
__ PushRegisterPair(RA, THR);
@@ -630,7 +630,8 @@
// Set argv in target::NativeArguments: R2 already contains argv.
// Set retval in NativeArgs.
ASSERT(retval_offset == 3 * target::kWordSize);
- __ AddImmediate(T3, FP, 2 * target::kWordSize);
+ __ AddImmediate(
+ T3, FP, (target::frame_layout.param_end_from_fp + 1) * target::kWordSize);
// Passing the structure by value as in runtime calls would require changing
// Dart API for native functions.
@@ -843,11 +844,11 @@
// +------------------+
// | PC marker | <- TOS
// +------------------+
-// | Saved FP | <- FP of stub
+// | Saved FP |
// +------------------+
// | return-address | (deoptimization point)
// +------------------+
-// | Saved CODE_REG |
+// | Saved CODE_REG | <- FP of stub
// +------------------+
// | ... | <- SP of optimized frame
//
@@ -880,7 +881,7 @@
// Save the original value of CODE_REG pushed before invoking this stub
// instead of the value used to call this stub.
COMPILE_ASSERT(TMP > CODE_REG); // TMP saved first
- __ lx(TMP, Address(FP, 2 * target::kWordSize));
+ __ lx(TMP, Address(FP, 0 * target::kWordSize));
__ sx(TMP, Address(SP, i * target::kWordSize));
} else {
__ sx(r, Address(SP, i * target::kWordSize));
@@ -1318,10 +1319,13 @@
// target::frame_layout.exit_link_slot_from_entry_fp must be kept in sync
// with the code below.
#if XLEN == 32
- ASSERT_EQUAL(target::frame_layout.exit_link_slot_from_entry_fp, -40);
+ ASSERT_EQUAL(target::frame_layout.exit_link_slot_from_entry_fp, -42);
#elif XLEN == 64
- ASSERT_EQUAL(target::frame_layout.exit_link_slot_from_entry_fp, -28);
+ ASSERT_EQUAL(target::frame_layout.exit_link_slot_from_entry_fp, -30);
#endif
+ // In debug mode, verify that we've pushed the top exit frame info at the
+ // correct offset from FP.
+ __ EmitEntryFrameVerification();
// Mark that the thread is executing Dart code. Do this after initializing the
// exit link for the profiler.
diff --git a/runtime/vm/compiler/stub_code_compiler_x64.cc b/runtime/vm/compiler/stub_code_compiler_x64.cc
index 0268696..7ae8fbc 100644
--- a/runtime/vm/compiler/stub_code_compiler_x64.cc
+++ b/runtime/vm/compiler/stub_code_compiler_x64.cc
@@ -729,15 +729,17 @@
}
// Pass target::NativeArguments structure by value and call native function.
- __ movq(Address(RSP, thread_offset), THR); // Set thread in NativeArgs.
- __ movq(Address(RSP, argc_tag_offset),
- R10); // Set argc in target::NativeArguments.
- __ movq(Address(RSP, argv_offset),
- R13); // Set argv in target::NativeArguments.
- __ leaq(RAX,
- Address(RBP, 2 * target::kWordSize)); // Compute return value addr.
- __ movq(Address(RSP, retval_offset),
- RAX); // Set retval in target::NativeArguments.
+ // Set thread in NativeArgs.
+ __ movq(Address(RSP, thread_offset), THR);
+ // Set argc in target::NativeArguments.
+ __ movq(Address(RSP, argc_tag_offset), R10);
+ // Set argv in target::NativeArguments.
+ __ movq(Address(RSP, argv_offset), R13);
+ // Compute return value addr.
+ __ leaq(RAX, Address(RBP, (target::frame_layout.param_end_from_fp + 1) *
+ target::kWordSize));
+ // Set retval in target::NativeArguments.
+ __ movq(Address(RSP, retval_offset), RAX);
// Pass the pointer to the target::NativeArguments.
__ movq(CallingConventions::kArg1Reg, RSP);
diff --git a/runtime/vm/deopt_instructions.cc b/runtime/vm/deopt_instructions.cc
index 0a63ce4..76ee165c 100644
--- a/runtime/vm/deopt_instructions.cc
+++ b/runtime/vm/deopt_instructions.cc
@@ -201,7 +201,7 @@
intptr_t DeoptContext::GetSourceFp() const {
return source_frame_[source_frame_size_ - 1 - num_args_ -
- kParamEndSlotFromFp];
+ kParamEndSlotFromFp + kSavedCallerFpSlotFromFp];
}
intptr_t DeoptContext::GetSourcePp() const {
diff --git a/runtime/vm/stack_frame.cc b/runtime/vm/stack_frame.cc
index 55eebc9..0297144 100644
--- a/runtime/vm/stack_frame.cc
+++ b/runtime/vm/stack_frame.cc
@@ -722,7 +722,7 @@
for (intptr_t index = index_; index < deopt_instructions_.length(); index++) {
DeoptInstr* deopt_instr = deopt_instructions_[index];
if (deopt_instr->kind() == DeoptInstr::kCallerFp) {
- return index - num_materializations_;
+ return index - num_materializations_ - kSavedCallerFpSlotFromFp;
}
}
UNREACHABLE();
diff --git a/runtime/vm/stack_frame_riscv.h b/runtime/vm/stack_frame_riscv.h
index 4bf58ab..58b31a6 100644
--- a/runtime/vm/stack_frame_riscv.h
+++ b/runtime/vm/stack_frame_riscv.h
@@ -23,10 +23,10 @@
| first local T|
| caller's PP T|
| code object T| (current frame's code object)
- | caller's FP | <- FP of current frame
+ | caller's FP |
| caller's RA | (PC of caller frame)
+--------------------+
-Caller frame | last parameter | <- SP of caller frame
+Caller frame | last parameter | <- SP of caller frame, FP of current frame
| ... |
T against a slot indicates it needs to be traversed during GC.
@@ -35,24 +35,24 @@
static const int kDartFrameFixedSize = 4; // PP, FP, RA, PC marker.
static const int kSavedPcSlotFromSp = -1;
-static const int kFirstObjectSlotFromFp = -1; // Used by GC to traverse stack.
-static const int kLastFixedObjectSlotFromFp = -2;
+static const int kFirstObjectSlotFromFp = -3; // Used by GC to traverse stack.
+static const int kLastFixedObjectSlotFromFp = -4;
-static const int kFirstLocalSlotFromFp = -3;
-static const int kSavedCallerPpSlotFromFp = -2;
-static const int kPcMarkerSlotFromFp = -1;
-static const int kSavedCallerFpSlotFromFp = 0;
-static const int kSavedCallerPcSlotFromFp = 1;
+static const int kFirstLocalSlotFromFp = -5;
+static const int kSavedCallerPpSlotFromFp = -4;
+static const int kPcMarkerSlotFromFp = -3;
+static const int kSavedCallerFpSlotFromFp = -2;
+static const int kSavedCallerPcSlotFromFp = -1;
-static const int kParamEndSlotFromFp = 1; // One slot past last parameter.
-static const int kCallerSpSlotFromFp = 2;
+static const int kParamEndSlotFromFp = -1; // One slot past last parameter.
+static const int kCallerSpSlotFromFp = 0;
static const int kLastParamSlotFromEntrySp = 0;
// Entry and exit frame layout.
#if defined(TARGET_ARCH_RISCV64)
-static const int kExitLinkSlotFromEntryFp = -28;
+static const int kExitLinkSlotFromEntryFp = -30;
#elif defined(TARGET_ARCH_RISCV32)
-static const int kExitLinkSlotFromEntryFp = -40;
+static const int kExitLinkSlotFromEntryFp = -42;
#endif
COMPILE_ASSERT(kAbiPreservedCpuRegCount == 11);
COMPILE_ASSERT(kAbiPreservedFpuRegCount == 12);
