Skip to content

Commit

Permalink
Ensure that caller respects the start argument
Browse files Browse the repository at this point in the history
Previously, if there were ignored frames (iseq without pc), we could
go beyond the requested start frame.  This has two changes:

1) Ensure that we don't look beyond the start frame by using
last_cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(last_cfp) until the
desired start frame is reached.

2) To fix the failures caused by change 1), which occur when a
limited number of frames is requested, scan the VM stack before
allocating backtrace frames, looking for ignored frames. This
is complicated if there are ignored frames before and after
the start, in which case we need to scan until the start frame,
and then scan backwards, decrementing the start value until we
get to the point where start will result in the number of
requested frames.

This fixes a Rails test failure.  Jean Boussier was able to
to produce a failing test case outside of Rails.

Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
  • Loading branch information
jeremyevans and byroot committed Mar 24, 2021
1 parent 91ab024 commit f9f13a4
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 2 deletions.
16 changes: 16 additions & 0 deletions test/ruby/test_backtrace.rb
Expand Up @@ -154,6 +154,22 @@ def test_caller_with_nil_length
assert_equal caller(0), caller(0, nil)
end

def test_caller_locations_first_label
def self.label
caller_locations.first.label
end

def self.label_caller
label
end

assert_equal 'label_caller', label_caller

[1].group_by do
assert_equal 'label_caller', label_caller
end
end

def test_caller_locations
cs = caller(0); locs = caller_locations(0).map{|loc|
loc.to_s
Expand Down
52 changes: 50 additions & 2 deletions vm_backtrace.c
Expand Up @@ -543,6 +543,11 @@ backtrace_each(const rb_execution_context_t *ec,
real_size = size = last = 0;
}
else {
/* Ensure we don't look at frames beyond the ones requested */
for(; from_last > 0 && start_cfp >= last_cfp; from_last--) {
last_cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(last_cfp);
}

real_size = size = start_cfp - last_cfp + 1;

if (from_last > size) {
Expand All @@ -568,9 +573,52 @@ backtrace_each(const rb_execution_context_t *ec,

init(arg, size);

/* SDR(); */
/* If a limited number of frames is requested, scan the VM stack for
* ignored frames (iseq without pc). Then adjust the start for the
* backtrace to account for skipped frames.
*/
if (start > 0 && num_frames >= 0 && num_frames < real_size) {
ptrdiff_t ignored_frames;
bool ignored_frames_before_start = false;
for (i=0, j=0, cfp = start_cfp; i<last && j<real_size; i++, j++, cfp = RUBY_VM_NEXT_CONTROL_FRAME(cfp)) {
if (cfp->iseq && !cfp->pc) {
if (j < start)
ignored_frames_before_start = true;
else
i--;
}
}
ignored_frames = j - i;

if (ignored_frames) {
if (ignored_frames_before_start) {
/* There were ignored frames before start. So just decrementing
* start for ignored frames could still result in not all desired
* frames being captured.
*
* First, scan to the CFP of the desired start frame.
*
* Then scan backwards to previous frames, skipping the number of
* frames ignored after start and any additional ones before start,
* so the number of desired frames will be correctly captured.
*/
for (i=0, j=0, cfp = start_cfp; i<last && j<real_size && j < start; i++, j++, cfp = RUBY_VM_NEXT_CONTROL_FRAME(cfp)) {
/* nothing */
}
for (; start > 0 && ignored_frames > 0 && j > 0; j--, ignored_frames--, start--, cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp)) {
if (cfp->iseq && !cfp->pc) {
ignored_frames++;
}
}
} else {
/* No ignored frames before start frame, just decrement start */
start -= ignored_frames;
}
}
}

for (i=0, j=0, cfp = start_cfp; i<last && j<real_size; i++, j++, cfp = RUBY_VM_NEXT_CONTROL_FRAME(cfp)) {
if (i < start) {
if (j < start) {
if (iter_skip) {
iter_skip(arg, cfp);
}
Expand Down

0 comments on commit f9f13a4

Please sign in to comment.