Commit b4e62f2d authored by Leszek Swirski's avatar Leszek Swirski Committed by V8 LUCI CQ

[sparkplug] Fix invalid stack on baseline install

InterpreterOnStackReplacement_ToBaseline spills the accumulator register
without a frame, but can then call kInstallBaselineCode. If that
function then allocates, then the GC will see an invalid stack.

Fix this by making sure that the accumulator register is spilled inside
the internal frame of the kInstallBaselineCode, and either don't spill
it at all outside that frame, or at least make sure that we pop/re-push
the spilled value so that it moves inside the frame.

Bug: v8:11420
Change-Id: Iad2aa718b0477ff960544d881fecae9efcbeef54
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3059072
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: 's avatarMythri Alle <mythria@chromium.org>
Reviewed-by: 's avatarVictor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75978}
parent b223262d
......@@ -3487,7 +3487,6 @@ namespace {
void Generate_BaselineOrInterpreterEntry(MacroAssembler* masm,
bool next_bytecode,
bool is_osr = false) {
__ Push(kInterpreterAccumulatorRegister);
Label start;
__ bind(&start);
......@@ -3510,7 +3509,6 @@ void Generate_BaselineOrInterpreterEntry(MacroAssembler* masm,
__ b(eq, &start_with_baseline);
// Start with bytecode as there is no baseline code.
__ Pop(kInterpreterAccumulatorRegister);
Builtin builtin_id = next_bytecode
? Builtin::kInterpreterEnterAtNextBytecode
: Builtin::kInterpreterEnterAtBytecode;
......@@ -3581,6 +3579,8 @@ void Generate_BaselineOrInterpreterEntry(MacroAssembler* masm,
// Get bytecode array from the stack frame.
__ ldr(kInterpreterBytecodeArrayRegister,
MemOperand(fp, InterpreterFrameConstants::kBytecodeArrayFromFp));
// Save the accumulator register, since it's clobbered by the below call.
__ Push(kInterpreterAccumulatorRegister);
{
Register arg_reg_1 = r0;
Register arg_reg_2 = r1;
......@@ -3628,8 +3628,10 @@ void Generate_BaselineOrInterpreterEntry(MacroAssembler* masm,
__ bind(&install_baseline_code);
{
FrameScope scope(masm, StackFrame::INTERNAL);
__ Push(kInterpreterAccumulatorRegister);
__ Push(closure);
__ CallRuntime(Runtime::kInstallBaselineCode, 1);
__ Pop(kInterpreterAccumulatorRegister);
}
// Retry from the start after installing baseline code.
__ b(&start);
......
......@@ -4012,7 +4012,6 @@ namespace {
void Generate_BaselineOrInterpreterEntry(MacroAssembler* masm,
bool next_bytecode,
bool is_osr = false) {
__ Push(padreg, kInterpreterAccumulatorRegister);
Label start;
__ bind(&start);
......@@ -4037,7 +4036,6 @@ void Generate_BaselineOrInterpreterEntry(MacroAssembler* masm,
__ B(eq, &start_with_baseline);
// Start with bytecode as there is no baseline code.
__ Pop(kInterpreterAccumulatorRegister, padreg);
Builtin builtin_id = next_bytecode
? Builtin::kInterpreterEnterAtNextBytecode
: Builtin::kInterpreterEnterAtBytecode;
......@@ -4112,6 +4110,8 @@ void Generate_BaselineOrInterpreterEntry(MacroAssembler* masm,
// Get bytecode array from the stack frame.
__ ldr(kInterpreterBytecodeArrayRegister,
MemOperand(fp, InterpreterFrameConstants::kBytecodeArrayFromFp));
// Save the accumulator register, since it's clobbered by the below call.
__ Push(padreg, kInterpreterAccumulatorRegister);
{
Register arg_reg_1 = x0;
Register arg_reg_2 = x1;
......@@ -4153,8 +4153,10 @@ void Generate_BaselineOrInterpreterEntry(MacroAssembler* masm,
__ bind(&install_baseline_code);
{
FrameScope scope(masm, StackFrame::INTERNAL);
__ Push(padreg, kInterpreterAccumulatorRegister);
__ PushArgument(closure);
__ CallRuntime(Runtime::kInstallBaselineCode, 1);
__ Pop(kInterpreterAccumulatorRegister, padreg);
}
// Retry from the start after installing baseline code.
__ B(&start);
......
......@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "src/codegen/register-arch.h"
#if V8_TARGET_ARCH_IA32
#include "src/api/api-arguments.h"
......@@ -4115,10 +4116,13 @@ namespace {
void Generate_BaselineOrInterpreterEntry(MacroAssembler* masm,
bool next_bytecode,
bool is_osr = false) {
__ push(kInterpreterAccumulatorRegister);
Label start;
__ bind(&start);
// Spill the accumulator register; note that we're not within a frame, so we
// have to make sure to pop it before doing any GC-visible calls.
__ push(kInterpreterAccumulatorRegister);
// Get function from the frame.
Register closure = eax;
__ mov(closure, MemOperand(ebp, StandardFrameConstants::kFunctionOffset));
......@@ -4250,12 +4254,23 @@ void Generate_BaselineOrInterpreterEntry(MacroAssembler* masm,
}
__ bind(&install_baseline_code);
// Pop/re-push the accumulator so that it's spilled within the below frame
// scope, to keep the stack valid. Use ecx for this -- we can't save it in
// kInterpreterAccumulatorRegister because that aliases with closure.
DCHECK(!AreAliased(ecx, kContextRegister, closure));
__ pop(ecx);
// Restore the clobbered context register.
__ mov(kContextRegister,
Operand(ebp, StandardFrameConstants::kContextOffset));
{
__ mov(kContextRegister,
Operand(ebp, StandardFrameConstants::kContextOffset));
FrameScope scope(masm, StackFrame::INTERNAL);
__ Push(ecx);
__ Push(closure);
__ CallRuntime(Runtime::kInstallBaselineCode, 1);
// Now that we're restarting, we don't have to worry about closure and
// accumulator aliasing, so pop the spilled accumulator directly back into
// the right register.
__ Pop(kInterpreterAccumulatorRegister);
}
// Retry from the start after installing baseline code.
__ jmp(&start);
......
......@@ -4356,9 +4356,10 @@ namespace {
void Generate_BaselineOrInterpreterEntry(MacroAssembler* masm,
bool next_bytecode,
bool is_osr = false) {
__ pushq(kInterpreterAccumulatorRegister);
Label start;
__ bind(&start);
// Get function from the frame.
Register closure = rdi;
__ movq(closure, MemOperand(rbp, StandardFrameConstants::kFunctionOffset));
......@@ -4378,7 +4379,6 @@ void Generate_BaselineOrInterpreterEntry(MacroAssembler* masm,
__ j(equal, &start_with_baseline);
// Start with bytecode as there is no baseline code.
__ popq(kInterpreterAccumulatorRegister);
Builtin builtin_id = next_bytecode
? Builtin::kInterpreterEnterAtNextBytecode
: Builtin::kInterpreterEnterAtBytecode;
......@@ -4400,7 +4400,7 @@ void Generate_BaselineOrInterpreterEntry(MacroAssembler* masm,
}
// Load the feedback vector.
Register feedback_vector = rax;
Register feedback_vector = r11;
__ LoadTaggedPointerField(
feedback_vector, FieldOperand(closure, JSFunction::kFeedbackCellOffset));
__ LoadTaggedPointerField(feedback_vector,
......@@ -4430,7 +4430,7 @@ void Generate_BaselineOrInterpreterEntry(MacroAssembler* masm,
get_baseline_pc_extref =
ExternalReference::baseline_pc_for_bytecode_offset();
}
Register get_baseline_pc = rax;
Register get_baseline_pc = r11;
__ LoadAddress(get_baseline_pc, get_baseline_pc_extref);
// If the code deoptimizes during the implicit function entry stack interrupt
......@@ -4453,6 +4453,7 @@ void Generate_BaselineOrInterpreterEntry(MacroAssembler* masm,
// Get bytecode array from the stack frame.
__ movq(kInterpreterBytecodeArrayRegister,
MemOperand(rbp, InterpreterFrameConstants::kBytecodeArrayFromFp));
__ pushq(kInterpreterAccumulatorRegister);
{
FrameScope scope(masm, StackFrame::INTERNAL);
__ PrepareCallCFunction(3);
......@@ -4493,8 +4494,10 @@ void Generate_BaselineOrInterpreterEntry(MacroAssembler* masm,
__ bind(&install_baseline_code);
{
FrameScope scope(masm, StackFrame::INTERNAL);
__ pushq(kInterpreterAccumulatorRegister);
__ Push(closure);
__ CallRuntime(Runtime::kInstallBaselineCode, 1);
__ popq(kInterpreterAccumulatorRegister);
}
// Retry from the start after installing baseline code.
__ jmp(&start);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment