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

[maglev] Fix stack overflow issues

Add a stack check to maglev function entry, and ensure that there is
deopt info on the safepoint when there is a loop interrupt which can
also stack check.

These are somewhat hacky fixes for now, both of which do the minimal
work to make OptimizedFrame::Summarize work. There are some TODOs on
making this better, in particular not relying on lazy deopt info for
said summarize. Cleaning this up will likely be part of a larger piece
of work around exception support.

Bug: v8:7700
Fixed: v8:13152, v8:13153, v8:13154, v8:13162
Change-Id: Ib9e4820200806a3f7d08fb8b069655525f90efb3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3811285Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82663}
parent 3dab0e71
......@@ -4,6 +4,7 @@
#include "src/execution/frames.h"
#include <cstdint>
#include <memory>
#include <sstream>
......@@ -16,6 +17,8 @@
#include "src/codegen/safepoint-table.h"
#include "src/common/globals.h"
#include "src/deoptimizer/deoptimizer.h"
#include "src/execution/arguments.h"
#include "src/execution/frame-constants.h"
#include "src/execution/frames-inl.h"
#include "src/execution/vm-state-inl.h"
#include "src/ic/ic-stats.h"
......@@ -1445,13 +1448,37 @@ void MaglevFrame::Iterate(RootVisitor* v) const {
&Memory<Address>(fp() - StandardFrameConstants::kCPSlotSize));
// Determine spill slot area count.
int spill_slot_count = maglev_safepoint_entry.num_tagged_slots() +
maglev_safepoint_entry.num_untagged_slots();
uint32_t tagged_slot_count = maglev_safepoint_entry.num_tagged_slots();
uint32_t spill_slot_count =
tagged_slot_count + maglev_safepoint_entry.num_untagged_slots();
DCHECK_EQ(entry->code.stack_slots(),
StandardFrameConstants::kFixedSlotCount +
maglev_safepoint_entry.num_tagged_slots() +
maglev_safepoint_entry.num_untagged_slots());
// Check that our frame size is big enough for our spill slots and pushed
// registers.
intptr_t actual_frame_size = static_cast<intptr_t>(fp() - sp());
intptr_t expected_frame_size_excl_outgoing_params =
StandardFrameConstants::kFixedFrameSizeFromFp +
(spill_slot_count + maglev_safepoint_entry.num_pushed_registers()) *
kSystemPointerSize;
if (actual_frame_size < expected_frame_size_excl_outgoing_params) {
// If the frame size is smaller than the expected size, then we must be in
// the stack guard in the prologue of the maglev function. This means that
// we've set up the frame header, but not the spill slots yet.
// DCHECK the frame setup under the above assumption. Include one extra slot
// for the single argument into StackGuardWithGap.
DCHECK_EQ(actual_frame_size, StandardFrameConstants::kFixedFrameSizeFromFp +
kSystemPointerSize);
DCHECK_EQ(isolate()->c_function(),
Runtime::FunctionForId(Runtime::kStackGuardWithGap)->entry);
DCHECK_EQ(maglev_safepoint_entry.num_pushed_registers(), 0);
spill_slot_count = 0;
tagged_slot_count = 0;
}
// Visit the outgoing parameters if they are tagged.
DCHECK(entry->code.has_tagged_outgoing_params());
FullObjectSlot parameters_base(&Memory<Address>(sp()));
......@@ -1479,7 +1506,7 @@ void MaglevFrame::Iterate(RootVisitor* v) const {
}
// Visit tagged spill slots.
for (uint32_t i = 0; i < maglev_safepoint_entry.num_tagged_slots(); ++i) {
for (uint32_t i = 0; i < tagged_slot_count; ++i) {
FullObjectSlot spill_slot = frame_header_base - 1 - i;
VisitSpillSlot(isolate(), v, spill_slot);
}
......@@ -2101,6 +2128,26 @@ void OptimizedFrame::Summarize(std::vector<FrameSummary>* frames) const {
int deopt_index = SafepointEntry::kNoDeoptIndex;
DeoptimizationData const data = GetDeoptimizationData(&deopt_index);
if (deopt_index == SafepointEntry::kNoDeoptIndex) {
// Hack: For maglevved function entry, we don't emit lazy deopt information,
// so create an extra special summary here.
//
// TODO(leszeks): Remove this hack, by having a maglev-specific frame
// summary which is a bit more aware of maglev behaviour and can e.g. handle
// more compact safepointed frame information for both function entry and
// loop stack checks.
if (code.is_maglevved()) {
DCHECK(frames->empty());
Handle<AbstractCode> abstract_code(
AbstractCode::cast(function().shared().GetBytecodeArray(isolate())),
isolate());
Handle<FixedArray> params = GetParameters();
FrameSummary::JavaScriptFrameSummary summary(
isolate(), receiver(), function(), *abstract_code,
kFunctionEntryBytecodeOffset, IsConstructor(), *params);
frames->push_back(summary);
return;
}
CHECK(data.is_null());
FATAL("Missing deoptimization information for OptimizedFrame::Summarize.");
}
......
......@@ -1159,6 +1159,7 @@ void VisitStack(Isolate* isolate, Visitor* visitor,
case StackFrame::JAVA_SCRIPT_BUILTIN_CONTINUATION:
case StackFrame::JAVA_SCRIPT_BUILTIN_CONTINUATION_WITH_CATCH:
case StackFrame::TURBOFAN:
case StackFrame::MAGLEV:
case StackFrame::INTERPRETED:
case StackFrame::BASELINE:
case StackFrame::BUILTIN:
......
......@@ -487,6 +487,24 @@ class MaglevCodeGeneratingNodeProcessor {
code_gen_state_->set_untagged_slots(graph->untagged_stack_slots());
code_gen_state_->set_tagged_slots(graph->tagged_stack_slots());
{
ASM_CODE_COMMENT_STRING(masm(), " Stack/interrupt check");
// Stack check. This folds the checks for both the interrupt stack limit
// check and the real stack limit into one by just checking for the
// interrupt limit. The interrupt limit is either equal to the real stack
// limit or tighter. By ensuring we have space until that limit after
// building the frame we can quickly precheck both at once.
__ Move(kScratchRegister, rsp);
// TODO(leszeks): Include a max call argument size here.
__ subq(kScratchRegister,
Immediate(code_gen_state_->stack_slots() * kSystemPointerSize));
__ cmpq(kScratchRegister,
__ StackLimitAsOperand(StackLimitKind::kInterruptStackLimit));
__ j(below, &deferred_call_stack_guard_);
__ bind(&deferred_call_stack_guard_return_);
}
// Initialize stack slots.
if (graph->tagged_stack_slots() > 0) {
ASM_CODE_COMMENT_STRING(masm(), "Initializing stack slots");
......@@ -529,7 +547,19 @@ class MaglevCodeGeneratingNodeProcessor {
}
}
void PostProcessGraph(MaglevCompilationInfo*, Graph*) {}
void PostProcessGraph(MaglevCompilationInfo*, Graph*) {
__ int3();
__ bind(&deferred_call_stack_guard_);
ASM_CODE_COMMENT_STRING(masm(), "Stack/interrupt call");
// Save incoming new target or generator
// __ Push(new_target);
// Push the frame size
__ Push(Immediate(
Smi::FromInt(code_gen_state_->stack_slots() * kSystemPointerSize)));
__ CallRuntime(Runtime::kStackGuardWithGap, 1);
//__ Pop(new_target);
__ jmp(&deferred_call_stack_guard_return_);
}
void PreProcessBasicBlock(MaglevCompilationInfo*, BasicBlock* block) {
if (FLAG_code_comments) {
......@@ -700,6 +730,8 @@ class MaglevCodeGeneratingNodeProcessor {
private:
MaglevCodeGenState* code_gen_state_;
Label deferred_call_stack_guard_;
Label deferred_call_stack_guard_return_;
};
} // namespace
......
......@@ -180,6 +180,14 @@ class SaveRegisterStateForCall {
return safepoint;
}
MaglevSafepointTableBuilder::Safepoint DefineSafepointWithLazyDeopt(
LazyDeoptInfo* lazy_deopt_info) {
lazy_deopt_info->deopting_call_return_pc =
code_gen_state->masm()->pc_offset_for_safepoint();
code_gen_state->PushLazyDeopt(lazy_deopt_info);
return DefineSafepoint();
}
private:
MaglevCodeGenState* code_gen_state;
RegisterSnapshot snapshot_;
......@@ -3043,7 +3051,8 @@ void ReduceInterruptBudget::GenerateCode(MaglevCodeGenState* code_gen_state,
__ Move(kContextRegister, code_gen_state->native_context().object());
__ Push(MemOperand(rbp, StandardFrameConstants::kFunctionOffset));
__ CallRuntime(Runtime::kBytecodeBudgetInterruptWithStackCheck, 1);
save_register_state.DefineSafepoint();
save_register_state.DefineSafepointWithLazyDeopt(
node->lazy_deopt_info());
}
__ jmp(return_label);
},
......
......@@ -3360,7 +3360,12 @@ class ReduceInterruptBudget : public FixedInputNodeT<0, ReduceInterruptBudget> {
DCHECK_GT(amount, 0);
}
static constexpr OpProperties kProperties = OpProperties::DeferredCall();
// TODO(leszeks): This is marked as lazy deopt because the interrupt can throw
// on a stack overflow. Full lazy deopt information is probably overkill
// though, we likely don't need the full frame but just the function and
// source location. Consider adding a minimal lazy deopt info.
static constexpr OpProperties kProperties =
OpProperties::DeferredCall() | OpProperties::LazyDeopt();
int amount() const { return amount_; }
......
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