Skip to content

Commit 649bfbe

Browse files
ivoanjomame
authored andcommitted
Fix rb_profile_frames output includes dummy main thread frame
The `rb_profile_frames` API did not skip the two dummy frames that each thread has at its beginning. This was unlike `backtrace_each` and `rb_ec_parcial_backtrace_object`, which do skip them. This does not seem to be a problem for non-main thread frames, because both `VM_FRAME_RUBYFRAME_P(cfp)` and `rb_vm_frame_method_entry(cfp)` are NULL for them. BUT, on the main thread `VM_FRAME_RUBYFRAME_P(cfp)` was true and thus the dummy thread was still included in the output of `rb_profile_frames`. I've now made `rb_profile_frames` skip this extra frame (like `backtrace_each` and friends), as well as add a test that asserts the size and contents of `rb_profile_frames`. Fixes [Bug #18907] (<https://bugs.ruby-lang.org/issues/18907>)
1 parent cc29b43 commit 649bfbe

File tree

3 files changed

+28
-0
lines changed

3 files changed

+28
-0
lines changed

ext/-test-/debug/profile_frames.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ profile_frames(VALUE self, VALUE start_v, VALUE num_v)
2929
rb_ary_push(ary, rb_profile_frame_singleton_method_p(buff[i]));
3030
rb_ary_push(ary, rb_profile_frame_method_name(buff[i]));
3131
rb_ary_push(ary, rb_profile_frame_qualified_method_name(buff[i]));
32+
rb_ary_push(ary, INT2NUM(lines[i]));
3233

3334
rb_ary_push(result, ary);
3435
}

test/-ext-/debug/test_profile_frames.rb

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,30 @@ def test_profile_frames
137137
}
138138
end
139139

140+
def test_matches_backtrace_locations_main_thread
141+
assert_equal(Thread.current, Thread.main)
142+
143+
# Keep these in the same line, so the backtraces match exactly
144+
backtrace_locations, profile_frames = [Thread.current.backtrace_locations, Bug::Debug.profile_frames(0, 100)]
145+
146+
assert_equal(backtrace_locations.size, profile_frames.size)
147+
148+
# The first entries are not going to match, since one is #backtrace_locations and the other #profile_frames
149+
backtrace_locations.shift
150+
profile_frames.shift
151+
152+
# The rest of the stack is expected to look the same...
153+
backtrace_locations.zip(profile_frames).each.with_index do |(location, (path, absolute_path, _, base_label, _, _, _, _, _, _, lineno)), i|
154+
next if absolute_path == "<cfunc>" # ...except for cfunc frames
155+
156+
err_msg = "#{i}th frame"
157+
assert_equal(location.absolute_path, absolute_path, err_msg)
158+
assert_equal(location.base_label, base_label, err_msg)
159+
assert_equal(location.lineno, lineno, err_msg)
160+
assert_equal(location.path, path, err_msg)
161+
end
162+
end
163+
140164
def test_ifunc_frame
141165
bug11851 = '[ruby-core:72409] [Bug #11851]'
142166
assert_ruby_status([], <<~'end;', bug11851) # do

vm_backtrace.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1551,6 +1551,9 @@ rb_profile_frames(int start, int limit, VALUE *buff, int *lines)
15511551
const rb_control_frame_t *cfp = ec->cfp, *end_cfp = RUBY_VM_END_CONTROL_FRAME(ec);
15521552
const rb_callable_method_entry_t *cme;
15531553

1554+
// Skip dummy frame; see `rb_ec_partial_backtrace_object` for details
1555+
end_cfp = RUBY_VM_NEXT_CONTROL_FRAME(end_cfp);
1556+
15541557
for (i=0; i<limit && cfp != end_cfp;) {
15551558
if (VM_FRAME_RUBYFRAME_P(cfp)) {
15561559
if (start > 0) {

0 commit comments

Comments
 (0)