Skip to content

Commit 8753ca6

Browse files
sisshiki1969claude
andauthored
Complete Exception backtrace: raise-time stack, qualified names, memoization (#896)
CRuby's `Exception#backtrace` reports the full live stack at raise time. monoruby only recorded the frames the exception actually unwound through (raise site down to the rescuing frame), so a backtrace for an exception raised and rescued in the same method was missing that frame's callers. - Add `Executor::complete_backtrace_for_rescue`, called once at the catch point (`handle_error`'s rescue branch, before `take_ex_obj`). It walks the still-live caller frames of the rescuing frame via each inner frame's saved call-site pc (the same mechanism as `Kernel#caller`) and appends the cheap `(loc, sourceinfo, fid)` tuples. No string formatting happens here — that stays lazy in `#backtrace`. Control-flow pseudo-errors (MethodReturn / BlockBreak / Throw) never reach this path, so they pay nothing, keeping raise cheap. - Render backtrace frame owners with their fully-qualified name in `func_description` (`Ns::Cx.foo`, not `Cx.foo`), special-casing Object so plain top-level methods still read as `Object#foo`. - Memoize `#backtrace` into the `/backtrace` hidden ivar so repeated calls return the same mutable Array (`e.backtrace.unshift(x)` is visible next call; `e.dup.backtrace` is a distinct object). `set_backtrace` writes the same ivar, unifying the explicit store with the memo. - Decouple `#backtrace_locations` from the string backtrace via a new `__raise_backtrace` intrinsic (raise-time capture only), so `set_backtrace(strings)` on a never-raised exception keeps `#backtrace_locations` nil, and an Array of Locations sets both. Fixes core/exception backtrace_spec, backtrace_locations_spec, and set_backtrace_spec. Adds tests/backtrace.rs (6 differential tests). Claude-Session: https://claude.ai/code/session_01XpzdoFu46d2ChJpSzeWsNK Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2423ce0 commit 8753ca6

6 files changed

Lines changed: 228 additions & 7 deletions

File tree

monoruby/builtins/startup.rb

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -996,11 +996,17 @@ class ThreadError < StandardError; end unless defined?(::ThreadError)
996996

997997
class Exception
998998
def backtrace_locations
999-
bt = backtrace
1000-
return nil if bt.nil?
1001-
bt.map do |frame|
1002-
Thread::Backtrace::Location.new(frame)
1003-
end
999+
# Locations come from the raise-time capture, independent of any
1000+
# string backtrace installed via #set_backtrace (CRuby: after
1001+
# `set_backtrace(strings)` on a never-raised exception,
1002+
# #backtrace_locations stays nil). Memoized so repeated calls
1003+
# return the same, mutable Array.
1004+
# A prior #backtrace_locations memo, or locations installed via
1005+
# set_backtrace(locations), win over re-deriving from the capture.
1006+
return @backtrace_locations if defined?(@backtrace_locations) && @backtrace_locations
1007+
frames = __raise_backtrace
1008+
return nil if frames.nil?
1009+
@backtrace_locations = frames.map { |f| Thread::Backtrace::Location.new(f) }
10041010
end
10051011

10061012
# CRuby 3.4+: `set_backtrace` (and thus `$@ =`) also accepts an Array
@@ -1009,8 +1015,13 @@ def backtrace_locations
10091015
# raises the usual TypeError.
10101016
def set_backtrace(bt)
10111017
if bt.is_a?(Array) && !bt.empty? && bt.all? { |e| Thread::Backtrace::Location === e }
1018+
# CRuby 3.4+: an Array of Locations sets both the string backtrace
1019+
# and #backtrace_locations.
10121020
__set_backtrace(bt.map(&:to_s))
1021+
@backtrace_locations = bt
10131022
else
1023+
# Setting a string backtrace leaves #backtrace_locations as it was
1024+
# (the raise-time capture, or nil), so don't touch the memo here.
10141025
__set_backtrace(bt)
10151026
end
10161027
end

monoruby/src/builtins/exception.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ pub(super) fn init(globals: &mut Globals) {
137137
globals.define_builtin_func_with(EXCEPTION_CLASS, "initialize", initialize, 0, 2, false);
138138
globals.define_builtin_func(EXCEPTION_CLASS, "message", message, 0);
139139
globals.define_builtin_func(EXCEPTION_CLASS, "backtrace", backtrace, 0);
140+
globals.define_builtin_func(EXCEPTION_CLASS, "__raise_backtrace", raise_backtrace, 0);
140141
globals.define_builtin_func(EXCEPTION_CLASS, "set_backtrace", set_backtrace, 1);
141142
// Internal alias: `startup.rb` redefines `set_backtrace` with a Ruby
142143
// wrapper that also accepts `Thread::Backtrace::Location` arrays
@@ -485,6 +486,37 @@ fn backtrace(_vm: &mut Executor, globals: &mut Globals, lfp: Lfp, _: BytecodePtr
485486
// raised, so there is no backtrace to report.
486487
return Ok(Value::nil());
487488
}
489+
// Memoize the built array so repeated `#backtrace` calls return the
490+
// *same* object (CRuby: `e.backtrace.unshift(...)` is visible on the
491+
// next `#backtrace`, and `e.dup.backtrace.equal?(e.backtrace)`).
492+
let array = Value::array_from_iter(traces.into_iter().map(|s| Value::string(s)));
493+
globals
494+
.store
495+
.set_ivar(self_, IdentId::get_id("/backtrace"), array)?;
496+
Ok(array)
497+
}
498+
499+
///
500+
/// ### Exception#__raise_backtrace (monoruby intrinsic)
501+
///
502+
/// The raise-time captured backtrace strings (nil if the exception was
503+
/// never raised), *independent* of any string backtrace installed via
504+
/// `#set_backtrace`. `#backtrace_locations` builds on this so a
505+
/// `set_backtrace(strings)` on a never-raised exception keeps
506+
/// `#backtrace_locations` nil, matching CRuby.
507+
///
508+
#[monoruby_builtin]
509+
fn raise_backtrace(
510+
_vm: &mut Executor,
511+
globals: &mut Globals,
512+
lfp: Lfp,
513+
_: BytecodePtr,
514+
) -> Result<Value> {
515+
let self_ = lfp.self_val();
516+
let traces: Vec<String> = self_.is_exception().unwrap().trace_location(globals);
517+
if traces.is_empty() {
518+
return Ok(Value::nil());
519+
}
488520
Ok(Value::array_from_iter(
489521
traces.into_iter().map(|s| Value::string(s)),
490522
))

monoruby/src/codegen/jit_module.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,11 @@ pub(super) extern "C" fn handle_error(
249249
return ErrorReturn::return_err();
250250
}
251251
if let Some((Some(rescue), _, err_reg)) = info.get_exception_dest(pc) {
252+
// Fill in the callers of this rescuing frame so the
253+
// backtrace matches CRuby's raise-time stack even when
254+
// the exception is raised and rescued in the same
255+
// method. Cheap tuple walk; formatting stays lazy.
256+
vm.complete_backtrace_for_rescue(&globals.store);
252257
let err_val = vm.take_ex_obj(globals);
253258
vm.set_errinfo(err_val);
254259
if let Some(err_reg) = err_reg {

monoruby/src/executor.rs

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1391,6 +1391,75 @@ impl Executor {
13911391
None => unreachable!(),
13921392
};
13931393
}
1394+
1395+
///
1396+
/// Complete the pending exception's backtrace with the frames that
1397+
/// are *above* the rescuing frame.
1398+
///
1399+
/// The incremental `push_error_location` path only records the
1400+
/// frames the exception actually unwinds through — from the raise
1401+
/// site down to the frame whose `rescue` catches it. CRuby's
1402+
/// backtrace additionally includes that frame's callers (the rest
1403+
/// of the live stack at raise time). When an exception is raised and
1404+
/// rescued within the same method those callers would otherwise be
1405+
/// missing entirely.
1406+
///
1407+
/// This is called once, at the catch point (`take_ex_obj` in
1408+
/// `handle_error`'s rescue branch), while the caller frames are
1409+
/// still live on the stack. It only walks the cheap
1410+
/// `(loc, sourceinfo, fid)` tuples — no string formatting, which
1411+
/// stays lazy in `Exception#backtrace`. Control-flow pseudo-errors
1412+
/// (`MethodReturn` / `BlockBreak` / `Throw`) never reach this path,
1413+
/// so they pay nothing.
1414+
///
1415+
pub(crate) fn complete_backtrace_for_rescue(&mut self, store: &Store) {
1416+
if self.exception.is_none() {
1417+
return;
1418+
}
1419+
// Start at the rescuing frame (already recorded); walk its
1420+
// callers via each inner frame's saved call-site pc, exactly
1421+
// like `Kernel#caller`.
1422+
let rescue_cfp = self.cfp();
1423+
let mut inner_cfp = rescue_cfp;
1424+
let mut cfp = match rescue_cfp.prev() {
1425+
Some(prev) => prev,
1426+
None => return,
1427+
};
1428+
loop {
1429+
let func_id = cfp.lfp().func_id();
1430+
if let Some(iseq_id) = store[func_id].is_iseq() {
1431+
let info = &store[iseq_id];
1432+
// The caller's suspended line: the call-site pc it saved
1433+
// into its callee's cont-frame slot. Fall back to the
1434+
// method's definition location when the slot is absent
1435+
// or out of range (invoker boundary, unaudited path).
1436+
let loc = {
1437+
let slot = inner_cfp.caller_pc_slot();
1438+
if slot != 0
1439+
&& slot % 8 == 0
1440+
&& let Some(pc) = unsafe { crate::bytecode::BytecodePtr::from_raw(slot as *mut _) }
1441+
&& info.contains_pc(pc)
1442+
{
1443+
let idx = info.get_pc_index(Some(pc)).to_usize();
1444+
Some(info.sourcemap[idx])
1445+
} else {
1446+
None
1447+
}
1448+
}
1449+
.unwrap_or(info.loc);
1450+
if let Some(err) = &mut self.exception {
1451+
err.push_trace(loc, info.sourceinfo.clone(), func_id);
1452+
}
1453+
}
1454+
match cfp.prev() {
1455+
Some(prev) => {
1456+
inner_cfp = cfp;
1457+
cfp = prev;
1458+
}
1459+
None => break,
1460+
}
1461+
}
1462+
}
13941463
}
13951464

13961465
//

monoruby/src/globals/store.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -427,11 +427,22 @@ impl Store {
427427
String::new()
428428
}
429429
};
430+
// CRuby renders the owner with its *fully-qualified* name
431+
// (`ExceptionSpecs::Backtrace.foo`, not `Backtrace.foo`).
432+
// `qualified_name` returns "" for Object, where the legacy
433+
// display is the bare "Object", so special-case it.
434+
let qual = |this: &Self, id: ClassId| {
435+
if id == OBJECT_CLASS {
436+
"Object".to_string()
437+
} else {
438+
this.qualified_name(id)
439+
}
440+
};
430441
match info.owner_class().get(0) {
431442
Some(owner) => {
432443
if let Some(obj) = self[*owner].get_module().is_singleton() {
433444
if let Some(class) = obj.is_class_or_module() {
434-
format!("{}.{name}", class.id().get_name(self))
445+
format!("{}.{name}", qual(self, class.id()))
435446
} else {
436447
// CRuby names a plain object's singleton
437448
// method by the bare method name (only
@@ -440,7 +451,7 @@ impl Store {
440451
name
441452
}
442453
} else {
443-
format!("{}#{name}", owner.get_name(self))
454+
format!("{}#{name}", qual(self, *owner))
444455
}
445456
}
446457
None => name,

monoruby/tests/backtrace.rs

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
//! Differential coverage for `Exception#backtrace` / `#backtrace_locations`
2+
//! / `#set_backtrace`: the raise-time stack is captured in full (even when
3+
//! the exception is raised and rescued within the same method), frame
4+
//! labels use fully-qualified class names, and the returned array is
5+
//! memoized. Compared against the reference CRuby.
6+
7+
use monoruby::tests::*;
8+
9+
#[test]
10+
fn backtrace_includes_callers_when_rescued_in_same_method() {
11+
run_test_once(
12+
r#"
13+
def inner
14+
begin
15+
raise "x"
16+
rescue => e
17+
return e.backtrace.map { |l| l.sub(/\A.*:(\d+:in )/, '\1') }
18+
end
19+
end
20+
def outer; inner; end
21+
outer
22+
"#,
23+
);
24+
}
25+
26+
#[test]
27+
fn backtrace_propagated_full_chain() {
28+
run_test_once(
29+
r#"
30+
def a; raise "x"; end
31+
def b; a; end
32+
def c; b; end
33+
begin
34+
c
35+
rescue => e
36+
e.backtrace.map { |l| l.sub(/\A.*:(\d+:in )/, '\1') }
37+
end
38+
"#,
39+
);
40+
}
41+
42+
#[test]
43+
fn backtrace_uses_qualified_class_name() {
44+
run_test_once(
45+
r#"
46+
module NsX
47+
class Cx
48+
def self.boom; raise "x"; end
49+
end
50+
end
51+
begin
52+
NsX::Cx.boom
53+
rescue => e
54+
e.backtrace.first.sub(/\A.*:\d+:in /, '')
55+
end
56+
"#,
57+
);
58+
}
59+
60+
#[test]
61+
fn backtrace_array_is_memoized_and_mutable() {
62+
run_test_once(
63+
r#"
64+
begin
65+
raise "x"
66+
rescue => e
67+
e.backtrace.unshift("first")
68+
[e.backtrace[0], e.backtrace.equal?(e.backtrace), e.dup.backtrace.equal?(e.backtrace)]
69+
end
70+
"#,
71+
);
72+
}
73+
74+
#[test]
75+
fn set_backtrace_strings_leaves_locations_nil() {
76+
run_test_once(
77+
r#"
78+
err = RuntimeError.new
79+
before = [err.backtrace, err.backtrace_locations]
80+
err.set_backtrace(["a.rb:1:in 'x'"])
81+
[before, err.backtrace, err.backtrace_locations]
82+
"#,
83+
);
84+
}
85+
86+
#[test]
87+
fn backtrace_of_unraised_exception_is_nil() {
88+
run_tests(&[
89+
r#"Exception.new.backtrace"#,
90+
r#"Exception.new.backtrace_locations"#,
91+
r#"RuntimeError.new("x").backtrace"#,
92+
]);
93+
}

0 commit comments

Comments
 (0)