YJIT: don't compile attr_accessor methods when tracing (#4998)

2d98593bf54a37397c6e4886ccc7e3654c2eaf85 made it so that
attr_accessor methods fire C method tracing events.
Previously, we weren't checking for whether we are tracing before
compiling, leading to missed events.

Since global invalidation invalidates all code, and that attr_accessor
methods can never enable tracing while running, events are only dropped
when YJIT tries to compile when tracing is already enabled.

Factor out the code for checking tracing and check it before generating
code for attr_accessor methods.

This change fixes TestSetTraceFunc#test_tracepoint_attr when it's
ran in isolation.

2 files changed, 95 insertions, 22 deletions

diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb
index a58ff2d18f..53cdcc69ac 100644
--- a/bootstraptest/test_yjit.rb
+++ b/bootstraptest/test_yjit.rb
@@ -2214,3 +2214,53 @@ assert_equal 'ok', %q{
 
   :ok
 }
+
+# regression test for tracing attr_accessor methods.
+assert_equal "true", %q{
+    c = Class.new do
+      attr_accessor :x
+      alias y x
+      alias y= x=
+    end
+    obj = c.new
+
+    ar_meth = obj.method(:x)
+    aw_meth = obj.method(:x=)
+    aar_meth = obj.method(:y)
+    aaw_meth = obj.method(:y=)
+    events = []
+    trace = TracePoint.new(:c_call, :c_return){|tp|
+      next if tp.path != __FILE__
+      next if tp.method_id == :call
+      case tp.event
+      when :c_call
+        events << [tp.event, tp.method_id, tp.callee_id]
+      when :c_return
+        events << [tp.event, tp.method_id, tp.callee_id, tp.return_value]
+      end
+    }
+    test_proc = proc do
+      obj.x = 1
+      obj.x
+      obj.y = 2
+      obj.y
+      aw_meth.call(1)
+      ar_meth.call
+      aaw_meth.call(2)
+      aar_meth.call
+    end
+    test_proc.call # populate call caches
+    trace.enable(&test_proc)
+    expected = [
+      [:c_call, :x=, :x=],
+      [:c_return, :x=, :x=, 1],
+      [:c_call, :x, :x],
+      [:c_return, :x, :x, 1],
+      [:c_call, :x=, :y=],
+      [:c_return, :x=, :y=, 2],
+      [:c_call, :x, :y],
+      [:c_return, :x, :y, 2],
+    ] * 2
+
+    expected == events
+}
diff --git a/yjit_codegen.c b/yjit_codegen.c
index 3b39c043b5..be4d7f0165 100644
--- a/yjit_codegen.c
+++ b/yjit_codegen.c
@@ -3076,6 +3076,24 @@ lookup_cfunc_codegen(const rb_method_definition_t *def)
     return NULL;
 }
 
+// Is anyone listening for :c_call and :c_return event currently?
+static bool
+c_method_tracing_currently_enabled(const jitstate_t *jit)
+{
+    rb_event_flag_t tracing_events;
+    if (rb_multi_ractor_p()) {
+        tracing_events = ruby_vm_event_enabled_global_flags;
+    }
+    else {
+        // At the time of writing, events are never removed from
+        // ruby_vm_event_enabled_global_flags so always checking using it would
+        // mean we don't compile even after tracing is disabled.
+        tracing_events = rb_ec_ractor_hooks(jit->ec)->events;
+    }
+
+    return tracing_events & (RUBY_EVENT_C_CALL | RUBY_EVENT_C_RETURN);
+}
+
 static codegen_status_t
 gen_send_cfunc(jitstate_t *jit, ctx_t *ctx, const struct rb_callinfo *ci, const rb_callable_method_entry_t *cme, rb_iseq_t *block, const int32_t argc, VALUE *recv_known_klass)
 {
@@ -3099,23 +3117,10 @@ gen_send_cfunc(jitstate_t *jit, ctx_t *ctx, const struct rb_callinfo *ci, const
         return YJIT_CANT_COMPILE;
     }
 
-    // Don't JIT if tracing c_call or c_return
-    {
-        rb_event_flag_t tracing_events;
-        if (rb_multi_ractor_p()) {
-            tracing_events = ruby_vm_event_enabled_global_flags;
-        }
-        else {
-            // We could always use ruby_vm_event_enabled_global_flags,
-            // but since events are never removed from it, doing so would mean
-            // we don't compile even after tracing is disabled.
-            tracing_events = rb_ec_ractor_hooks(jit->ec)->events;
-        }
-
-        if (tracing_events & (RUBY_EVENT_C_CALL | RUBY_EVENT_C_RETURN)) {
-            GEN_COUNTER_INC(cb, send_cfunc_tracing);
-            return YJIT_CANT_COMPILE;
-        }
+    if (c_method_tracing_currently_enabled(jit)) {
+        // Don't JIT if tracing c_call or c_return
+        GEN_COUNTER_INC(cb, send_cfunc_tracing);
+        return YJIT_CANT_COMPILE;
     }
 
     // Delegate to codegen for C methods if we have it.
@@ -3848,12 +3853,24 @@ gen_send_general(jitstate_t *jit, ctx_t *ctx, struct rb_call_data *cd, rb_iseq_t
                 GEN_COUNTER_INC(cb, send_getter_arity);
                 return YJIT_CANT_COMPILE;
             }
-            else {
-                mov(cb, REG0, recv);
-
-                ID ivar_name = cme->def->body.attr.id;
-                return gen_get_ivar(jit, ctx, SEND_MAX_DEPTH, comptime_recv, ivar_name, recv_opnd, side_exit);
+            if (c_method_tracing_currently_enabled(jit)) {
+                // Can't generate code for firing c_call and c_return events
+                // :attr-tracing:
+                // Handling the C method tracing events for attr_accessor
+                // methods is easier than regular C methods as we know the
+                // "method" we are calling into never enables those tracing
+                // events. Once global invalidation runs, the code for the
+                // attr_accessor is invalidated and we exit at the closest
+                // instruction boundary which is always outside of the body of
+                // the attr_accessor code.
+                GEN_COUNTER_INC(cb, send_cfunc_tracing);
+                return YJIT_CANT_COMPILE;
             }
+
+            mov(cb, REG0, recv);
+
+            ID ivar_name = cme->def->body.attr.id;
+            return gen_get_ivar(jit, ctx, SEND_MAX_DEPTH, comptime_recv, ivar_name, recv_opnd, side_exit);
           case VM_METHOD_TYPE_ATTRSET:
             if ((vm_ci_flag(ci) & VM_CALL_KWARG) != 0) {
                 GEN_COUNTER_INC(cb, send_attrset_kwargs);
@@ -3863,6 +3880,12 @@ gen_send_general(jitstate_t *jit, ctx_t *ctx, struct rb_call_data *cd, rb_iseq_t
                 GEN_COUNTER_INC(cb, send_ivar_set_method);
                 return YJIT_CANT_COMPILE;
             }
+            else if (c_method_tracing_currently_enabled(jit)) {
+                // Can't generate code for firing c_call and c_return events
+                // See :attr-tracing:
+                GEN_COUNTER_INC(cb, send_cfunc_tracing);
+                return YJIT_CANT_COMPILE;
+            }
             else {
                 ID ivar_name = cme->def->body.attr.id;
                 return gen_set_ivar(jit, ctx, comptime_recv, comptime_recv_klass, ivar_name);