gc_validate_pc(): Exclude imemos, add a test and explain the asserts

The validation is relevant only for traceable userland ruby objects ruby
code could interact with. ZJIT's use of rb_vm_method_cfunc_is()
allocates a CC imemo and was failing this validation when it was
actually fine. Relax the check.

2 files changed, 37 insertions, 7 deletions

diff --git a/gc.c b/gc.c
index 341fb20667..507d24d215 100644
--- a/gc.c
+++ b/gc.c
@@ -971,14 +971,16 @@ rb_gc_obj_slot_size(VALUE obj)
 }
 
 static inline void
-gc_validate_pc(void)
+gc_validate_pc(VALUE obj)
 {
 #if RUBY_DEBUG
     rb_execution_context_t *ec = GET_EC();
     const rb_control_frame_t *cfp = ec->cfp;
-    if (cfp && VM_FRAME_RUBYFRAME_P(cfp) && cfp->pc) {
-        RUBY_ASSERT(cfp->pc >= ISEQ_BODY(cfp->iseq)->iseq_encoded);
-        RUBY_ASSERT(cfp->pc <= ISEQ_BODY(cfp->iseq)->iseq_encoded + ISEQ_BODY(cfp->iseq)->iseq_size);
+    if (!RB_TYPE_P(obj, T_IMEMO) && cfp && VM_FRAME_RUBYFRAME_P(cfp) && cfp->pc) {
+        const VALUE *iseq_encoded = ISEQ_BODY(cfp->iseq)->iseq_encoded;
+        const VALUE *iseq_encoded_end = iseq_encoded + ISEQ_BODY(cfp->iseq)->iseq_size;
+        RUBY_ASSERT(cfp->pc >= iseq_encoded, "PC not set when allocating, breaking tracing");
+        RUBY_ASSERT(cfp->pc <= iseq_encoded_end, "PC not set when allocating, breaking tracing");
     }
 #endif
 }
@@ -988,7 +990,7 @@ newobj_of(rb_ractor_t *cr, VALUE klass, VALUE flags, bool wb_protected, size_t s
 {
     VALUE obj = rb_gc_impl_new_obj(rb_gc_get_objspace(), cr->newobj_cache, klass, flags, wb_protected, size);
 
-    gc_validate_pc();
+    gc_validate_pc(obj);
 
     if (UNLIKELY(rb_gc_event_hook_required_p(RUBY_INTERNAL_EVENT_NEWOBJ))) {
         int lev = RB_GC_VM_LOCK_NO_BARRIER();
diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb
index c1e8cfa805..563be8c966 100644
--- a/test/ruby/test_zjit.rb
+++ b/test/ruby/test_zjit.rb
@@ -2813,6 +2813,28 @@ class TestZJIT < Test::Unit::TestCase
     }, insns: [:opt_send_without_block]
   end
 
+  def test_allocating_in_hir_c_method_is
+    assert_compiles ":k", %q{
+      # Put opt_new in a frame JIT code sets up that doesn't set cfp->pc
+      def a(f) = test(f)
+      def test(f) = (f.new if f)
+      # A parallel couple methods that will set PC at the same stack height
+      def second = third
+      def third = nil
+
+      a(nil)
+      a(nil)
+
+      class Foo
+        def self.new = :k
+      end
+
+      second
+
+      a(Foo)
+    }, call_threshold: 2, insns: [:opt_new]
+  end
+
   private
 
   # Assert that every method call in `test_script` can be compiled by ZJIT
@@ -2826,13 +2848,14 @@ class TestZJIT < Test::Unit::TestCase
   # allows ZJIT to skip compiling methods.
   def assert_runs(expected, test_script, insns: [], assert_compiles: false, **opts)
     pipe_fd = 3
+    disasm_method = :test
 
     script = <<~RUBY
       ret_val = (_test_proc = -> { #{('RubyVM::ZJIT.assert_compiles; ' if assert_compiles)}#{test_script.lstrip} }).call
       result = {
         ret_val:,
         #{ unless insns.empty?
-          'insns: RubyVM::InstructionSequence.of(method(:test)).to_a'
+           "insns: RubyVM::InstructionSequence.of(method(#{disasm_method.inspect})).to_a"
         end}
       }
       IO.open(#{pipe_fd}).write(Marshal.dump(result))
@@ -2846,7 +2869,12 @@ class TestZJIT < Test::Unit::TestCase
 
     unless insns.empty?
       iseq = result.fetch(:insns)
-      assert_equal("YARVInstructionSequence/SimpleDataFormat", iseq.first, "failed to get iseq disassembly")
+      assert_equal(
+        "YARVInstructionSequence/SimpleDataFormat",
+        iseq.first,
+        "Failed to get ISEQ disassembly. " \
+        "Make sure to put code directly under the '#{disasm_method}' method."
+      )
       iseq_insns = iseq.last
 
       expected_insns = Set.new(insns)