8325469: Freeze/Thaw code can crash in the presence of OSR frames

pchilano · pchilano · commit fd331ff17330 · 2024-04-17T16:18:55.000Z
Reviewed-by: rpressler, dlong
diff --git a/src/hotspot/cpu/aarch64/frame_aarch64.inline.hpp b/src/hotspot/cpu/aarch64/frame_aarch64.inline.hpp
@@ -241,7 +241,7 @@ inline int frame::frame_size() const {
 
 inline int frame::compiled_frame_stack_argsize() const {
   assert(cb()->is_nmethod(), "");
-  return (cb()->as_nmethod()->method()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;
+  return (cb()->as_nmethod()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;
 }
 
 inline void frame::interpreted_frame_oop_map(InterpreterOopMap* mask) const {
diff --git a/src/hotspot/cpu/aarch64/stackChunkFrameStream_aarch64.inline.hpp b/src/hotspot/cpu/aarch64/stackChunkFrameStream_aarch64.inline.hpp
@@ -35,7 +35,7 @@ template <ChunkFrames frame_kind>
 inline bool StackChunkFrameStream<frame_kind>::is_in_frame(void* p0) const {
   assert(!is_done(), "");
   intptr_t* p = (intptr_t*)p0;
-  int argsize = is_compiled() ? (_cb->as_nmethod()->method()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord : 0;
+  int argsize = is_compiled() ? (_cb->as_nmethod()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord : 0;
   int frame_size = _cb->frame_size() + argsize;
   return p == sp() - frame::sender_sp_offset || ((p - unextended_sp()) >= 0 && (p - unextended_sp()) < frame_size);
 }
diff --git a/src/hotspot/cpu/ppc/frame_ppc.inline.hpp b/src/hotspot/cpu/ppc/frame_ppc.inline.hpp
@@ -369,7 +369,7 @@ inline void frame::set_saved_oop_result(RegisterMap* map, oop obj) {
 
 inline int frame::compiled_frame_stack_argsize() const {
   assert(cb()->is_nmethod(), "");
-  return (cb()->as_nmethod()->method()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;
+  return (cb()->as_nmethod()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;
 }
 
 inline void frame::interpreted_frame_oop_map(InterpreterOopMap* mask) const {
diff --git a/src/hotspot/cpu/ppc/stackChunkFrameStream_ppc.inline.hpp b/src/hotspot/cpu/ppc/stackChunkFrameStream_ppc.inline.hpp
@@ -35,7 +35,7 @@ inline bool StackChunkFrameStream<frame_kind>::is_in_frame(void* p0) const {
   assert(!is_done(), "");
   assert(is_compiled(), "");
   intptr_t* p = (intptr_t*)p0;
-  int argsize = (_cb->as_nmethod()->method()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;
+  int argsize = (_cb->as_nmethod()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;
   int frame_size = _cb->frame_size() + (argsize > 0 ? argsize + frame::metadata_words_at_top : 0);
   return (p - unextended_sp()) >= 0 && (p - unextended_sp()) < frame_size;
 }
diff --git a/src/hotspot/cpu/riscv/frame_riscv.inline.hpp b/src/hotspot/cpu/riscv/frame_riscv.inline.hpp
@@ -232,7 +232,7 @@ inline int frame::frame_size() const {
 
 inline int frame::compiled_frame_stack_argsize() const {
   assert(cb()->is_nmethod(), "");
-  return (cb()->as_nmethod()->method()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;
+  return (cb()->as_nmethod()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;
 }
 
 inline void frame::interpreted_frame_oop_map(InterpreterOopMap* mask) const {
diff --git a/src/hotspot/cpu/riscv/stackChunkFrameStream_riscv.inline.hpp b/src/hotspot/cpu/riscv/stackChunkFrameStream_riscv.inline.hpp
@@ -34,7 +34,7 @@ template <ChunkFrames frame_kind>
 inline bool StackChunkFrameStream<frame_kind>::is_in_frame(void* p0) const {
   assert(!is_done(), "");
   intptr_t* p = (intptr_t*)p0;
-  int argsize = is_compiled() ? (_cb->as_nmethod()->method()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord : 0;
+  int argsize = is_compiled() ? (_cb->as_nmethod()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord : 0;
   int frame_size = _cb->frame_size() + argsize;
   return p == sp() - 2 || ((p - unextended_sp()) >= 0 && (p - unextended_sp()) < frame_size);
 }
diff --git a/src/hotspot/cpu/x86/frame_x86.inline.hpp b/src/hotspot/cpu/x86/frame_x86.inline.hpp
@@ -227,7 +227,7 @@ inline int frame::frame_size() const {
 
 inline int frame::compiled_frame_stack_argsize() const {
   assert(cb()->is_nmethod(), "");
-  return (cb()->as_nmethod()->method()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;
+  return (cb()->as_nmethod()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;
 }
 
 inline void frame::interpreted_frame_oop_map(InterpreterOopMap* mask) const {
diff --git a/src/hotspot/cpu/x86/stackChunkFrameStream_x86.inline.hpp b/src/hotspot/cpu/x86/stackChunkFrameStream_x86.inline.hpp
@@ -34,7 +34,7 @@ template <ChunkFrames frame_kind>
 inline bool StackChunkFrameStream<frame_kind>::is_in_frame(void* p0) const {
   assert(!is_done(), "");
   intptr_t* p = (intptr_t*)p0;
-  int argsize = is_compiled() ? (_cb->as_nmethod()->method()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord : 0;
+  int argsize = is_compiled() ? (_cb->as_nmethod()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord : 0;
   int frame_size = _cb->frame_size() + argsize;
   return p == sp() - frame::sender_sp_offset || ((p - unextended_sp()) >= 0 && (p - unextended_sp()) < frame_size);
 }
diff --git a/src/hotspot/share/code/nmethod.cpp b/src/hotspot/share/code/nmethod.cpp
@@ -1230,6 +1230,7 @@ nmethod::nmethod(
     init_defaults();
     _comp_level              = CompLevel_none;
     _entry_bci               = InvocationEntryBci;
+    _num_stack_arg_slots     = _method->constMethod()->num_stack_arg_slots();
     // We have no exception handler or deopt handler make the
     // values something that will never match a pc like the nmethod vtable entry
     _exception_offset        = 0;
@@ -1374,12 +1375,13 @@ nmethod::nmethod(
     assert_locked_or_safepoint(CodeCache_lock);
 
     init_defaults();
-    _entry_bci      = entry_bci;
-    _compile_id     = compile_id;
-    _compiler_type  = type;
-    _comp_level     = comp_level;
-    _orig_pc_offset = orig_pc_offset;
-    _gc_epoch       = CodeCache::gc_epoch();
+    _entry_bci               = entry_bci;
+    _num_stack_arg_slots     = entry_bci != InvocationEntryBci ? 0 : _method->constMethod()->num_stack_arg_slots();
+    _compile_id              = compile_id;
+    _compiler_type           = type;
+    _comp_level              = comp_level;
+    _orig_pc_offset          = orig_pc_offset;
+    _gc_epoch                = CodeCache::gc_epoch();
 
     // Section offsets
     _consts_offset  = content_offset() + code_buffer->total_offset_of(code_buffer->consts());
diff --git a/src/hotspot/share/code/nmethod.hpp b/src/hotspot/share/code/nmethod.hpp
@@ -243,6 +243,8 @@ class nmethod : public CodeBlob {
 
   int _compile_id;                        // which compilation made this nmethod
 
+  int _num_stack_arg_slots;               // Number of arguments passed on the stack
+
   CompilerType _compiler_type;            // which compiler made this nmethod (u1)
 
   bool _is_unlinked;
@@ -792,6 +794,10 @@ class nmethod : public CodeBlob {
   nmethod* osr_link() const                       { return _osr_link; }
   void     set_osr_link(nmethod *n)               { _osr_link = n; }
 
+  int num_stack_arg_slots(bool rounded = true) const {
+    return rounded ? align_up(_num_stack_arg_slots, 2) : _num_stack_arg_slots;
+  }
+
   // Verify calls to dead methods have been cleaned.
   void verify_clean_inline_caches();
 
diff --git a/src/hotspot/share/oops/method.hpp b/src/hotspot/share/oops/method.hpp
@@ -394,10 +394,6 @@ class Method : public Metadata {
   void unlink_method() NOT_CDS_RETURN;
   void remove_unshareable_flags() NOT_CDS_RETURN;
 
-  // the number of argument reg slots that the compiled method uses on the stack.
-  int num_stack_arg_slots(bool rounded = true) const {
-    return rounded ? align_up(constMethod()->num_stack_arg_slots(), 2) : constMethod()->num_stack_arg_slots(); }
-
   virtual void metaspace_pointers_do(MetaspaceClosure* iter);
   virtual MetaspaceObj::Type type() const { return MethodType; }
 
diff --git a/src/hotspot/share/oops/stackChunkOop.cpp b/src/hotspot/share/oops/stackChunkOop.cpp
@@ -562,9 +562,9 @@ bool stackChunkOopDesc::verify(size_t* out_size, int* out_oops, int* out_frames,
   assert(!is_empty() || closure._cb == nullptr, "");
   if (closure._cb != nullptr && closure._cb->is_nmethod()) {
     assert(argsize() ==
-      (closure._cb->as_nmethod()->method()->num_stack_arg_slots()*VMRegImpl::stack_slot_size) >>LogBytesPerWord,
+      (closure._cb->as_nmethod()->num_stack_arg_slots()*VMRegImpl::stack_slot_size) >>LogBytesPerWord,
       "chunk argsize: %d bottom frame argsize: %d", argsize(),
-      (closure._cb->as_nmethod()->method()->num_stack_arg_slots()*VMRegImpl::stack_slot_size) >>LogBytesPerWord);
+      (closure._cb->as_nmethod()->num_stack_arg_slots()*VMRegImpl::stack_slot_size) >>LogBytesPerWord);
   }
 
   assert(closure._num_interpreted_frames == 0 || has_mixed_frames(), "");
diff --git a/src/hotspot/share/runtime/continuationFreezeThaw.cpp b/src/hotspot/share/runtime/continuationFreezeThaw.cpp
@@ -1242,7 +1242,6 @@ NOINLINE freeze_result FreezeBase::recurse_freeze_stub_frame(frame& f, frame& ca
 
 NOINLINE void FreezeBase::finish_freeze(const frame& f, const frame& top) {
   stackChunkOop chunk = _cont.tail();
-  assert(chunk->to_offset(top.sp()) <= chunk->sp(), "");
 
   LogTarget(Trace, continuations) lt;
   if (lt.develop_is_enabled()) {
@@ -2309,7 +2308,7 @@ void ThawBase::recurse_thaw_compiled_frame(const frame& hf, frame& caller, int n
     _cont.tail()->fix_thawed_frame(caller, SmallRegisterMap::instance);
   } else if (_cont.tail()->has_bitmap() && added_argsize > 0) {
     address start = (address)(heap_frame_top + ContinuationHelper::CompiledFrame::size(hf) + frame::metadata_words_at_top);
-    int stack_args_slots = f.cb()->as_nmethod()->method()->num_stack_arg_slots(false /* rounded */);
+    int stack_args_slots = f.cb()->as_nmethod()->num_stack_arg_slots(false /* rounded */);
     int argsize_in_bytes = stack_args_slots * VMRegImpl::stack_slot_size;
     clear_bitmap_bits(start, start + argsize_in_bytes);
   }
diff --git a/src/hotspot/share/runtime/frame.cpp b/src/hotspot/share/runtime/frame.cpp
@@ -1435,7 +1435,7 @@ void frame::describe(FrameValues& values, int frame_no, const RegisterMap* reg_m
         assert(sig_index == sizeargs, "");
       }
       int stack_arg_slots = SharedRuntime::java_calling_convention(sig_bt, regs, sizeargs);
-      assert(stack_arg_slots ==  m->num_stack_arg_slots(false /* rounded */), "");
+      assert(stack_arg_slots ==  nm->as_nmethod()->num_stack_arg_slots(false /* rounded */) || nm->is_osr_method(), "");
       int out_preserve = SharedRuntime::out_preserve_stack_slots();
       int sig_index = 0;
       int arg_index = (m->is_static() ? 0 : -1);
diff --git a/src/hotspot/share/runtime/stackChunkFrameStream.inline.hpp b/src/hotspot/share/runtime/stackChunkFrameStream.inline.hpp
@@ -191,7 +191,7 @@ inline int StackChunkFrameStream<frame_kind>::stack_argsize() const {
   assert(cb() != nullptr, "");
   assert(cb()->is_nmethod(), "");
   assert(cb()->as_nmethod()->method() != nullptr, "");
-  return (cb()->as_nmethod()->method()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;
+  return (cb()->as_nmethod()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;
 }
 
 template <ChunkFrames frame_kind>
diff --git a/test/jdk/jdk/internal/vm/Continuation/OSRTest.java b/test/jdk/jdk/internal/vm/Continuation/OSRTest.java

Original file line number	Diff line number	Diff line change
`@@ -241,7 +241,7 @@ inline int frame::frame_size() const {`
`241`	`241`
`242`	`242`	`inline int frame::compiled_frame_stack_argsize() const {`
`243`	`243`	`assert(cb()->is_nmethod(), "");`
`244`		`- return (cb()->as_nmethod()->method()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;`
	`244`	`+ return (cb()->as_nmethod()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;`
`245`	`245`	`}`
`246`	`246`
`247`	`247`	`inline void frame::interpreted_frame_oop_map(InterpreterOopMap* mask) const {`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ template <ChunkFrames frame_kind>`
`35`	`35`	`inline bool StackChunkFrameStream<frame_kind>::is_in_frame(void* p0) const {`
`36`	`36`	`assert(!is_done(), "");`
`37`	`37`	`intptr_t* p = (intptr_t*)p0;`
`38`		`- int argsize = is_compiled() ? (_cb->as_nmethod()->method()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord : 0;`
	`38`	`+ int argsize = is_compiled() ? (_cb->as_nmethod()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord : 0;`
`39`	`39`	`int frame_size = _cb->frame_size() + argsize;`
`40`	`40`	`return p == sp() - frame::sender_sp_offset \|\| ((p - unextended_sp()) >= 0 && (p - unextended_sp()) < frame_size);`
`41`	`41`	`}`
Original file line number	Diff line number	Diff line change
`@@ -369,7 +369,7 @@ inline void frame::set_saved_oop_result(RegisterMap* map, oop obj) {`
`369`	`369`
`370`	`370`	`inline int frame::compiled_frame_stack_argsize() const {`
`371`	`371`	`assert(cb()->is_nmethod(), "");`
`372`		`- return (cb()->as_nmethod()->method()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;`
	`372`	`+ return (cb()->as_nmethod()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;`
`373`	`373`	`}`
`374`	`374`
`375`	`375`	`inline void frame::interpreted_frame_oop_map(InterpreterOopMap* mask) const {`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ inline bool StackChunkFrameStream<frame_kind>::is_in_frame(void* p0) const {`
`35`	`35`	`assert(!is_done(), "");`
`36`	`36`	`assert(is_compiled(), "");`
`37`	`37`	`intptr_t* p = (intptr_t*)p0;`
`38`		`- int argsize = (_cb->as_nmethod()->method()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;`
	`38`	`+ int argsize = (_cb->as_nmethod()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;`
`39`	`39`	`int frame_size = _cb->frame_size() + (argsize > 0 ? argsize + frame::metadata_words_at_top : 0);`
`40`	`40`	`return (p - unextended_sp()) >= 0 && (p - unextended_sp()) < frame_size;`
`41`	`41`	`}`
Original file line number	Diff line number	Diff line change
`@@ -232,7 +232,7 @@ inline int frame::frame_size() const {`
`232`	`232`
`233`	`233`	`inline int frame::compiled_frame_stack_argsize() const {`
`234`	`234`	`assert(cb()->is_nmethod(), "");`
`235`		`- return (cb()->as_nmethod()->method()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;`
	`235`	`+ return (cb()->as_nmethod()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;`
`236`	`236`	`}`
`237`	`237`
`238`	`238`	`inline void frame::interpreted_frame_oop_map(InterpreterOopMap* mask) const {`
Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,7 @@ template <ChunkFrames frame_kind>`
`34`	`34`	`inline bool StackChunkFrameStream<frame_kind>::is_in_frame(void* p0) const {`
`35`	`35`	`assert(!is_done(), "");`
`36`	`36`	`intptr_t* p = (intptr_t*)p0;`
`37`		`- int argsize = is_compiled() ? (_cb->as_nmethod()->method()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord : 0;`
	`37`	`+ int argsize = is_compiled() ? (_cb->as_nmethod()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord : 0;`
`38`	`38`	`int frame_size = _cb->frame_size() + argsize;`
`39`	`39`	`return p == sp() - 2 \|\| ((p - unextended_sp()) >= 0 && (p - unextended_sp()) < frame_size);`
`40`	`40`	`}`
Original file line number	Diff line number	Diff line change
`@@ -227,7 +227,7 @@ inline int frame::frame_size() const {`
`227`	`227`
`228`	`228`	`inline int frame::compiled_frame_stack_argsize() const {`
`229`	`229`	`assert(cb()->is_nmethod(), "");`
`230`		`- return (cb()->as_nmethod()->method()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;`
	`230`	`+ return (cb()->as_nmethod()->num_stack_arg_slots() * VMRegImpl::stack_slot_size) >> LogBytesPerWord;`
`231`	`231`	`}`
`232`	`232`
`233`	`233`	`inline void frame::interpreted_frame_oop_map(InterpreterOopMap* mask) const {`