Bug #14607: Fix use of the rb_profile_frames start parameter

• Status: Closed
• Assignee: ko1 (Koichi Sasada)
• Target version: -
• Backport: 2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: UNKNOWN
• Tags: core

[dylants (Dylan Thacker-Smith)]

rb_profile_frames was always behaving as if the value given for the start parameter was 0.

The reason for this was that it would check if (start > 0) { then continue without updating the control frame pointer or anything other than decrementing start.

This bug applies to all branches under normal maintenance, from ruby 2.3 to trunk.

[tenderlovemaking (Aaron Patterson)]

• Status changed from Open to Closed

Applied in changeset trunk|r63265.

---

Fix use of rb_profile_frames start parameter

rb_profile_frames was always behaving as if the value given for the
start parameter was 0.

The reason for this was that it would check if (start > 0) { then
continue without updating the control frame pointer or anything other
than decrementing start.

[ruby-core:86147] [Bug #14607]

Co-authored-by: Dylan Thacker-Smith [e-mail]

[tenderlovemaking (Aaron Patterson)]

• Status changed from Closed to Open

ko1 said I shouldn't have committed the patch, so I reverted. Sorry!

[ko1 (Koichi Sasada)]

I need to remember why such special (additional) calculation is done, so I left this ticket.
I need to remember...

[dylants (Dylan Thacker-Smith)]

• File deleted (fix-use-of-the-rb_profile_frames-start-parameter.patch)

The original patch has a merge conflict. However, I have opened a pull request with the fix for this issue (https://github.com/ruby/ruby/pull/2713) that has been rebased to resolve the merge conflict.

[dylants (Dylan Thacker-Smith)]

I need to remember why such special (additional) calculation is done

I'm not sure what you mean by additional calculation. It is decrementing start when non-zero as expected to loop over that number of frames, it just was missing the corresponding update to cfp.

Could this get another look?

[mame (Yusuke Endoh)]

• Status changed from Open to Assigned
• Assignee set to ko1 (Koichi Sasada)

I assume that "additional calculation" means the code if (start > 0) { start--; continue; }, but the "additional calculation" seems to make no sense at all to me either.

    for (i=0; i<limit && cfp != end_cfp;) {
        if (VM_FRAME_RUBYFRAME_P(cfp)) {
            if (start > 0) {
                start--;
                continue;
            }

It is essentially start = 0;. The proposed fix seems reasonable to me. @ko1 (Koichi Sasada) What do you think?

[dylants (Dylan Thacker-Smith)]

The original patch has a merge conflict. However, I have opened a pull request with the fix for this issue (https://github.com/ruby/ruby/pull/2713) that has been rebased to resolve the merge conflict.

The github PR has been merged, so this issue can be closed now. It doesn't look like I have permission to change its status though.

[jeremyevans0 (Jeremy Evans)]

• Status changed from Assigned to Closed