ruby-changes:72685
From: Ivo <ko1@a...>
Date: Tue, 26 Jul 2022 10:43:59 +0900 (JST)
Subject: [ruby-changes:72685] 649bfbe00d (master): Fix `rb_profile_frames` output includes dummy main thread frame
https://git.ruby-lang.org/ruby.git/commit/?id=649bfbe00d From 649bfbe00d8032fa2c0536e596a284f69926e87f Mon Sep 17 00:00:00 2001 From: Ivo Anjo <ivo.anjo@d...> Date: Mon, 11 Jul 2022 14:51:44 +0100 Subject: 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>) --- ext/-test-/debug/profile_frames.c | 1 + test/-ext-/debug/test_profile_frames.rb | 24 ++++++++++++++++++++++++ vm_backtrace.c | 3 +++ 3 files changed, 28 insertions(+) diff --git a/ext/-test-/debug/profile_frames.c b/ext/-test-/debug/profile_frames.c index 5f14755ce7..d2bba7d183 100644 --- a/ext/-test-/debug/profile_frames.c +++ b/ext/-test-/debug/profile_frames.c @@ -29,6 +29,7 @@ profile_frames(VALUE self, VALUE start_v, VALUE num_v) https://github.com/ruby/ruby/blob/trunk/ext/-test-/debug/profile_frames.c#L29 rb_ary_push(ary, rb_profile_frame_singleton_method_p(buff[i])); rb_ary_push(ary, rb_profile_frame_method_name(buff[i])); rb_ary_push(ary, rb_profile_frame_qualified_method_name(buff[i])); + rb_ary_push(ary, INT2NUM(lines[i])); rb_ary_push(result, ary); } diff --git a/test/-ext-/debug/test_profile_frames.rb b/test/-ext-/debug/test_profile_frames.rb index e0152247e7..d6ae953dd2 100644 --- a/test/-ext-/debug/test_profile_frames.rb +++ b/test/-ext-/debug/test_profile_frames.rb @@ -137,6 +137,30 @@ class TestProfileFrames < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/-ext-/debug/test_profile_frames.rb#L137 } end + def test_matches_backtrace_locations_main_thread + assert_equal(Thread.current, Thread.main) + + # Keep these in the same line, so the backtraces match exactly + backtrace_locations, profile_frames = [Thread.current.backtrace_locations, Bug::Debug.profile_frames(0, 100)] + + assert_equal(backtrace_locations.size, profile_frames.size) + + # The first entries are not going to match, since one is #backtrace_locations and the other #profile_frames + backtrace_locations.shift + profile_frames.shift + + # The rest of the stack is expected to look the same... + backtrace_locations.zip(profile_frames).each.with_index do |(location, (path, absolute_path, _, base_label, _, _, _, _, _, _, lineno)), i| + next if absolute_path == "<cfunc>" # ...except for cfunc frames + + err_msg = "#{i}th frame" + assert_equal(location.absolute_path, absolute_path, err_msg) + assert_equal(location.base_label, base_label, err_msg) + assert_equal(location.lineno, lineno, err_msg) + assert_equal(location.path, path, err_msg) + end + end + def test_ifunc_frame bug11851 = '[ruby-core:72409] [Bug #11851]' assert_ruby_status([], <<~'end;', bug11851) # do diff --git a/vm_backtrace.c b/vm_backtrace.c index ee749deb14..5bd588df12 100644 --- a/vm_backtrace.c +++ b/vm_backtrace.c @@ -1551,6 +1551,9 @@ rb_profile_frames(int start, int limit, VALUE *buff, int *lines) https://github.com/ruby/ruby/blob/trunk/vm_backtrace.c#L1551 const rb_control_frame_t *cfp = ec->cfp, *end_cfp = RUBY_VM_END_CONTROL_FRAME(ec); const rb_callable_method_entry_t *cme; + // Skip dummy frame; see `rb_ec_partial_backtrace_object` for details + end_cfp = RUBY_VM_NEXT_CONTROL_FRAME(end_cfp); + for (i=0; i<limit && cfp != end_cfp;) { if (VM_FRAME_RUBYFRAME_P(cfp)) { if (start > 0) { -- cgit v1.2.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/