Bug #21395: Please backport caa6ba1a46afa1bc696adc5fe91ee992f9570c89

• Status: Closed
• Assignee: -
• Target version: -
• Backport: 3.2: DONTNEED, 3.3: DONTNEED, 3.4: REQUIRED

[mame (Yusuke Endoh)]

In Ruby 3.4, debug.gem raises an exception when stepping into a rescue clause:

https://github.com/ruby/debug/pull/1142

I discussed with @ko1 (Koichi Sasada), and we decided to resolve this by modifying rb_debug_inspector_backtrace_locations to return a raw array of Thread::Backtrace::Location.

This ticket is to request backporting 8d49c05c134702c321198b70fbbf34dd80cc1ba6, which implements that change, into Ruby 3.4.

---

As a record, here is a more detailed explanation of the issue:

Starting from Ruby 3.4, rescue and ensure frames have been excluded from backtraces (see #20275).
This change broke the assumption of the debug inspector API.

The debug inspector API is designed to be used as follows:

1. Obtain an array of Thread::Backtrace::Location using rb_debug_inspector_backtrace_locations
2. For each index in that array, obtain more detailed information using functions like rb_debug_inspector_frame_binding_get(index)

Due to the changes in #20275, rescue/ensure frames were removed from the array returned by rb_debug_inspector_backtrace_locations.
As a result, the indices in this array became inconsistent with the indices expected by rb_debug_inspector_frame_binding_get(index) and similar functions.

There are two possible directions for fixing this:

1. Modify rb_debug_inspector_backtrace_locations to return the raw backtrace without removing rescue/ensure frames.
2. Modify rb_debug_inspector_frame_binding_get(index) to take into account the removal of rescue/ensure frames and return the corresponding data accordingly.

@ko1 (Koichi Sasada) is interested in trying (2) in the future, but for now, commit 8d49c05c134702c321198b70fbbf34dd80cc1ba6 implements the simpler fix, (1).

[mame (Yusuke Endoh)]

• Related to Feature #20275: Avoid extra backtrace entries for rescue and ensure added

[mame (Yusuke Endoh)]

• Subject changed from Please backport 8d49c05c134702c321198b70fbbf34dd80cc1ba6 to Please backport 8f37ef84ba412b35fb47db42461fcd6bdb1cce3e
• Description updated (diff)

[mame (Yusuke Endoh)]

• Subject changed from Please backport 8f37ef84ba412b35fb47db42461fcd6bdb1cce3e to Please backport caa6ba1a46afa1bc696adc5fe91ee992f9570c89
• Description updated (diff)

[Eregon (Benoit Daloze)]

• Subject changed from Please backport caa6ba1a46afa1bc696adc5fe91ee992f9570c89 to Please backport 8d49c05c134702c321198b70fbbf34dd80cc1ba6
• Description updated (diff)

8d49c05c134702c321198b70fbbf34dd80cc1ba6 seems to only change the debug gem version, I guess you mean some other commit to backport?
Probably https://github.com/ruby/ruby/commit/caa6ba1a46afa1bc696adc5fe91ee992f9570c89

As a result, the indices in this array became inconsistent with the indices expected by rb_debug_inspector_frame_binding_get(index) and similar functions.

Interesting, so that means rb_debug_inspector_frame_binding_get() doesn't actually use that array to get the information but some other internal data structure: https://github.com/ruby/ruby/blob/2cce628721728409a26c2d4732f63419785c7fd8/vm_backtrace.c#L1499-L1500
IOW the backtrace and contexts lengths should match, it would be good to add a check for that.

The second fix seems better (longer-term) as such internal frames shouldn't be exposed and it causes an inconsistency between rb_debug_inspector_backtrace_locations() and caller_locations, and the former documents to be equivalent to the latter.

[Eregon (Benoit Daloze)]

• Subject changed from Please backport 8d49c05c134702c321198b70fbbf34dd80cc1ba6 to Please backport caa6ba1a46afa1bc696adc5fe91ee992f9570c89