Skip to content

Commit

Permalink
YJIT: Fix Struct accessors not firing tracing events
Browse files Browse the repository at this point in the history
Reading and writing to structs should fire `c_call` and `c_return`, but
YJIT wasn't correctly dropping those calls when tracing.
This has been missing since this functionality was added in 3081c83,
but the added test only fails when ran in isolation with
`--yjit-call-threshold=1`. The test sometimes failed on CI.
  • Loading branch information
XrXr committed Apr 30, 2024
1 parent ade2233 commit 22f4bfa
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 4 deletions.
16 changes: 16 additions & 0 deletions bootstraptest/test_yjit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4964,3 +4964,19 @@ class NilClass
results << test
} unless rjit_enabled? # Not yet working on RJIT

# test struct accessors fire c_call events
assert_equal '[[:c_call, :x=], [:c_call, :x]]', %q{
c = Struct.new(:x)
obj = c.new
events = []
TracePoint.new(:c_call) do
events << [_1.event, _1.method_id]
end.enable do
obj.x = 100
obj.x
end
events
}
20 changes: 16 additions & 4 deletions yjit/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8309,6 +8309,13 @@ fn gen_struct_aref(
}
}

if c_method_tracing_currently_enabled(jit) {
// Struct accesses need fire c_call and c_return events, which we can't support
// See :attr-tracing:
gen_counter_incr(asm, Counter::send_cfunc_tracing);
return None;
}

// This is a .send call and we need to adjust the stack
if flags & VM_CALL_OPT_SEND != 0 {
handle_opt_send_shift_stack(asm, argc);
Expand Down Expand Up @@ -8353,6 +8360,13 @@ fn gen_struct_aset(
return None;
}

if c_method_tracing_currently_enabled(jit) {
// Struct accesses need fire c_call and c_return events, which we can't support
// See :attr-tracing:
gen_counter_incr(asm, Counter::send_cfunc_tracing);
return None;
}

// This is a .send call and we need to adjust the stack
if flags & VM_CALL_OPT_SEND != 0 {
handle_opt_send_shift_stack(asm, argc);
Expand Down Expand Up @@ -8619,10 +8633,8 @@ fn gen_send_general(
// 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.
// events. We are never inside the code that needs to be
// invalidated when invalidation happens.
gen_counter_incr(asm, Counter::send_cfunc_tracing);
return None;
}
Expand Down

0 comments on commit 22f4bfa

Please sign in to comment.