[compiler] Consider pushed arguments in stack check offset

Function calls can push arguments onto the stack. The consumed stack
slots are not considered by the function-entry stack check, since
initial frame setup only reserves space for local slots, not call
arguments. This CL adds such logic by tracking the maximum pushed
argument count during instruction selection, and adding these slots to
the (existing) stack check offset logic in code generation.

Bug: chromium:1030167
Change-Id: I26a9407cf38009839b1dda2ff0c8ec297c15ed8d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2002540
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65814}

src/compiler/backend/code-generator.cc

@@ -49,7 +49,7 @@ CodeGenerator::CodeGenerator(
     int start_source_position, JumpOptimizationInfo* jump_opt,
     PoisoningMitigationLevel poisoning_level, const AssemblerOptions& options,
     int32_t builtin_index, size_t max_unoptimized_frame_height,
-    std::unique_ptr<AssemblerBuffer> buffer)
+    size_t max_pushed_argument_count, std::unique_ptr<AssemblerBuffer> buffer)
     : zone_(codegen_zone),
       isolate_(isolate),
       frame_access_state_(nullptr),
@@ -69,6 +69,7 @@ CodeGenerator::CodeGenerator(
       deoptimization_literals_(zone()),
       translations_(zone()),
       max_unoptimized_frame_height_(max_unoptimized_frame_height),
+      max_pushed_argument_count_(max_pushed_argument_count),
       caller_registers_saved_(false),
       jump_tables_(nullptr),
       ools_(nullptr),
@@ -127,7 +128,11 @@ bool CodeGenerator::ShouldApplyOffsetToStackCheck(Instruction* instr,
 }
 
 uint32_t CodeGenerator::GetStackCheckOffset() {
-  if (!frame_access_state()->has_frame()) return 0;
+  if (!frame_access_state()->has_frame()) {
+    DCHECK_EQ(max_unoptimized_frame_height_, 0);
+    DCHECK_EQ(max_pushed_argument_count_, 0);
+    return 0;
+  }
 
   int32_t optimized_frame_height =
       frame()->GetTotalFrameSlotCount() * kSystemPointerSize;
@@ -135,9 +140,14 @@ uint32_t CodeGenerator::GetStackCheckOffset() {
   int32_t signed_max_unoptimized_frame_height =
       static_cast<int32_t>(max_unoptimized_frame_height_);
 
-  int32_t signed_offset =
-      std::max(signed_max_unoptimized_frame_height - optimized_frame_height, 0);
-  return (signed_offset <= 0) ? 0 : static_cast<uint32_t>(signed_offset);
+  // The offset is either the delta between the optimized frames and the
+  // interpreted frame, or the maximal number of bytes pushed to the stack
+  // while preparing for function calls, whichever is bigger.
+  uint32_t frame_height_delta = static_cast<uint32_t>(std::max(
+      signed_max_unoptimized_frame_height - optimized_frame_height, 0));
+  uint32_t max_pushed_argument_bytes =
+      static_cast<uint32_t>(max_pushed_argument_count_ * kSystemPointerSize);
+  return std::max(frame_height_delta, max_pushed_argument_bytes);
 }
 
 CodeGenerator::CodeGenResult CodeGenerator::AssembleDeoptimizerCall(

src/compiler/backend/code-generator.h

@@ -109,16 +109,14 @@ struct TurbolizerInstructionStartInfo {
 // Generates native code for a sequence of instructions.
 class V8_EXPORT_PRIVATE CodeGenerator final : public GapResolver::Assembler {
  public:
-  explicit CodeGenerator(Zone* codegen_zone, Frame* frame, Linkage* linkage,
-                         InstructionSequence* instructions,
-                         OptimizedCompilationInfo* info, Isolate* isolate,
-                         base::Optional<OsrHelper> osr_helper,
-                         int start_source_position,
-                         JumpOptimizationInfo* jump_opt,
-                         PoisoningMitigationLevel poisoning_level,
-                         const AssemblerOptions& options, int32_t builtin_index,
-                         size_t max_unoptimized_frame_height,
-                         std::unique_ptr<AssemblerBuffer> = {});
+  explicit CodeGenerator(
+      Zone* codegen_zone, Frame* frame, Linkage* linkage,
+      InstructionSequence* instructions, OptimizedCompilationInfo* info,
+      Isolate* isolate, base::Optional<OsrHelper> osr_helper,
+      int start_source_position, JumpOptimizationInfo* jump_opt,
+      PoisoningMitigationLevel poisoning_level, const AssemblerOptions& options,
+      int32_t builtin_index, size_t max_unoptimized_frame_height,
+      size_t max_pushed_argument_count, std::unique_ptr<AssemblerBuffer> = {});
 
   // Generate native code. After calling AssembleCode, call FinalizeCode to
   // produce the actual code object. If an error occurs during either phase,
@@ -171,10 +169,12 @@ class V8_EXPORT_PRIVATE CodeGenerator final : public GapResolver::Assembler {
 
   static constexpr int kBinarySearchSwitchMinimalCases = 4;
 
-  // Returns true if an offset should be applied to the given stack check. This
-  // is the case for stack checks on function-entry when the offset is non-zero,
-  // where the offset is the difference between the size of optimized and
-  // corresponding deoptimized frames.
+  // Returns true if an offset should be applied to the given stack check. There
+  // are two reasons that this could happen:
+  // 1. The optimized frame is smaller than the corresponding deoptimized frames
+  //    and an offset must be applied in order to be able to deopt safely.
+  // 2. The current function pushes a large number of arguments to the stack.
+  //    These are not accounted for by the initial frame setup.
   bool ShouldApplyOffsetToStackCheck(Instruction* instr, uint32_t* offset);
   uint32_t GetStackCheckOffset();
 
@@ -432,9 +432,11 @@ class V8_EXPORT_PRIVATE CodeGenerator final : public GapResolver::Assembler {
   int handler_table_offset_ = 0;
   int last_lazy_deopt_pc_ = 0;
 
-  // The maximal combined height of all frames produced upon deoptimization.
-  // Applied as an offset to the first stack check of an optimized function.
+  // The maximal combined height of all frames produced upon deoptimization, and
+  // the maximal number of pushed arguments for function calls. Applied as an
+  // offset to the first stack check of an optimized function.
   const size_t max_unoptimized_frame_height_;
+  const size_t max_pushed_argument_count_;
 
   // kArchCallCFunction could be reached either:
   //   kArchCallCFunction;

src/compiler/backend/instruction-selector.cc

@@ -26,7 +26,7 @@ InstructionSelector::InstructionSelector(
     InstructionSequence* sequence, Schedule* schedule,
     SourcePositionTable* source_positions, Frame* frame,
     EnableSwitchJumpTable enable_switch_jump_table, TickCounter* tick_counter,
-    size_t* max_unoptimized_frame_height,
+    size_t* max_unoptimized_frame_height, size_t* max_pushed_argument_count,
     SourcePositionMode source_position_mode, Features features,
     EnableScheduling enable_scheduling,
     EnableRootsRelativeAddressing enable_roots_relative_addressing,
@@ -59,7 +59,8 @@ InstructionSelector::InstructionSelector(
       instr_origins_(sequence->zone()),
       trace_turbo_(trace_turbo),
       tick_counter_(tick_counter),
-      max_unoptimized_frame_height_(max_unoptimized_frame_height) {
+      max_unoptimized_frame_height_(max_unoptimized_frame_height),
+      max_pushed_argument_count_(max_pushed_argument_count) {
   DCHECK_EQ(*max_unoptimized_frame_height, 0);  // Caller-initialized.
 
   instructions_.reserve(node_count);
@@ -2748,6 +2749,10 @@ void InstructionSelector::VisitConstant(Node* node) {
   Emit(kArchNop, g.DefineAsConstant(node));
 }
 
+void InstructionSelector::UpdateMaxPushedArgumentCount(size_t count) {
+  *max_pushed_argument_count_ = std::max(count, *max_pushed_argument_count_);
+}
+
 void InstructionSelector::VisitCall(Node* node, BasicBlock* handler) {
   OperandGenerator g(this);
   auto call_descriptor = CallDescriptorOf(node->op());
@@ -2777,7 +2782,8 @@ void InstructionSelector::VisitCall(Node* node, BasicBlock* handler) {
   CallBufferFlags call_buffer_flags(kCallCodeImmediate | kCallAddressImmediate);
   InitializeCallBuffer(node, &buffer, call_buffer_flags, false);
 
-  EmitPrepareArguments(&(buffer.pushed_nodes), call_descriptor, node);
+  EmitPrepareArguments(&buffer.pushed_nodes, call_descriptor, node);
+  UpdateMaxPushedArgumentCount(buffer.pushed_nodes.size());
 
   // Pass label of exception handler block.
   if (handler) {
@@ -2857,6 +2863,7 @@ void InstructionSelector::VisitTailCall(Node* node) {
     flags |= kCallFixedTargetRegister;
   }
   InitializeCallBuffer(node, &buffer, flags, true, stack_param_delta);
+  UpdateMaxPushedArgumentCount(stack_param_delta);
 
   // Select the appropriate opcode based on the call type.
   InstructionCode opcode;

src/compiler/backend/instruction-selector.h

@@ -271,7 +271,7 @@ class V8_EXPORT_PRIVATE InstructionSelector final {
       InstructionSequence* sequence, Schedule* schedule,
       SourcePositionTable* source_positions, Frame* frame,
       EnableSwitchJumpTable enable_switch_jump_table, TickCounter* tick_counter,
-      size_t* max_unoptimized_frame_height,
+      size_t* max_unoptimized_frame_height, size_t* max_pushed_argument_count,
       SourcePositionMode source_position_mode = kCallSourcePositions,
       Features features = SupportedFeatures(),
       EnableScheduling enable_scheduling = FLAG_turbo_instruction_scheduling
@@ -578,6 +578,8 @@ class V8_EXPORT_PRIVATE InstructionSelector final {
   bool IsTailCallAddressImmediate();
   int GetTempsCountForTailCallFromJSFunction();
 
+  void UpdateMaxPushedArgumentCount(size_t count);
+
   FrameStateDescriptor* GetFrameStateDescriptor(Node* node);
   size_t AddInputsToFrameStateDescriptor(FrameStateDescriptor* descriptor,
                                          Node* state, OperandGenerator* g,
@@ -788,9 +790,10 @@ class V8_EXPORT_PRIVATE InstructionSelector final {
   EnableTraceTurboJson trace_turbo_;
   TickCounter* const tick_counter_;
 
-  // Store the maximal unoptimized frame height. Later used to apply an offset
-  // to stack checks.
+  // Store the maximal unoptimized frame height and an maximal number of pushed
+  // arguments (for calls). Later used to apply an offset to stack checks.
   size_t* max_unoptimized_frame_height_;
+  size_t* max_pushed_argument_count_;
 };
 
 }  // namespace compiler

src/compiler/pipeline.cc

@@ -389,6 +389,12 @@ class PipelineData {
   size_t max_unoptimized_frame_height() const {
     return max_unoptimized_frame_height_;
   }
+  size_t* address_of_max_pushed_argument_count() {
+    return &max_pushed_argument_count_;
+  }
+  size_t max_pushed_argument_count() const {
+    return max_pushed_argument_count_;
+  }
 
   CodeTracer* GetCodeTracer() const {
     return wasm_engine_ == nullptr ? isolate_->GetCodeTracer()
@@ -505,7 +511,7 @@ class PipelineData {
         osr_helper_, start_source_position_, jump_optimization_info_,
         info()->GetPoisoningMitigationLevel(), assembler_options_,
         info_->builtin_index(), max_unoptimized_frame_height(),
-        std::move(buffer));
+        max_pushed_argument_count(), std::move(buffer));
   }
 
   void BeginPhaseKind(const char* phase_kind_name) {
@@ -604,9 +610,11 @@ class PipelineData {
   Maybe<OuterContext> specialization_context_ = Nothing<OuterContext>();
 
   // The maximal combined height of all inlined frames in their unoptimized
-  // state. Calculated during instruction selection, applied during code
-  // generation.
+  // state, and the maximal number of arguments pushed during function calls.
+  // Calculated during instruction selection, applied during code generation.
   size_t max_unoptimized_frame_height_ = 0;
+  size_t max_pushed_argument_count_ = 0;
+
   RuntimeCallStats* runtime_call_stats_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(PipelineData);
@@ -2049,6 +2057,7 @@ struct InstructionSelectionPhase {
             : InstructionSelector::kDisableSwitchJumpTable,
         &data->info()->tick_counter(),
         data->address_of_max_unoptimized_frame_height(),
+        data->address_of_max_pushed_argument_count(),
         data->info()->is_source_positions_enabled()
             ? InstructionSelector::kAllSourcePositions
             : InstructionSelector::kCallSourcePositions,

test/cctest/compiler/test-code-generator.cc

@@ -992,13 +992,15 @@ class CodeGeneratorTester {
     }
 
     static constexpr size_t kMaxUnoptimizedFrameHeight = 0;
+    static constexpr size_t kMaxPushedArgumentCount = 0;
     generator_ = new CodeGenerator(
         environment->main_zone(), &frame_, &linkage_,
         environment->instructions(), &info_, environment->main_isolate(),
         base::Optional<OsrHelper>(), kNoSourcePosition, nullptr,
         PoisoningMitigationLevel::kDontPoison,
         AssemblerOptions::Default(environment->main_isolate()),
-        Builtins::kNoBuiltinId, kMaxUnoptimizedFrameHeight);
+        Builtins::kNoBuiltinId, kMaxUnoptimizedFrameHeight,
+        kMaxPushedArgumentCount);
 
     // Force a frame to be created.
     generator_->frame_access_state()->MarkHasFrame(true);

test/unittests/compiler/backend/instruction-selector-unittest.cc

@@ -41,12 +41,13 @@ InstructionSelectorTest::Stream InstructionSelectorTest::StreamBuilder::Build(
   SourcePositionTable source_position_table(graph());
   TickCounter tick_counter;
   size_t max_unoptimized_frame_height = 0;
+  size_t max_pushed_argument_count = 0;
   InstructionSelector selector(
       test_->zone(), node_count, &linkage, &sequence, schedule,
       &source_position_table, nullptr,
       InstructionSelector::kEnableSwitchJumpTable, &tick_counter,
-      &max_unoptimized_frame_height, source_position_mode, features,
-      InstructionSelector::kDisableScheduling,
+      &max_unoptimized_frame_height, &max_pushed_argument_count,
+      source_position_mode, features, InstructionSelector::kDisableScheduling,
       InstructionSelector::kEnableRootsRelativeAddressing,
       PoisoningMitigationLevel::kPoisonAll);
   selector.SelectInstructions();