Repository navigation
vm: harvest the line map — owner-qualified backtrace labels, warn(uplevel:), Thread.each_caller_location - #649
Merged
Merged
Conversation
Kernel#warn's uplevel: level was validated and then thrown away: the prefix was unconditionally "warning: ", with a comment saying rbgo had no caller line to prepend. PR #645 made that untrue — frameLine is now MRI's calc_lineno over the frame's (iseq, pc) pair — so the level now names a frame and its "path:lineno: " is prepended. Read for this: ruby/ruby v3_4_0 error.c rb_warn_m. It takes the location from rb_ec_backtrace_location_ary(ec, lev + 1, 1, TRUE) and formats it as rb_sprintf("%s:%ld: warning: ", path, lineno), falling back to the bare "warning: " when that yields no location. The `+ 1` skips the frame of the warn cfunc itself; rbgo pushes no frame for a native, so level 0 is already the top of frameNames and the level indexes straight down. Also from rb_warn_m: the category runs through rb_to_symbol_type, i.e. rb_convert_type_with_id(val, T_SYMBOL, "Symbol", idTo_sym). Anything that answers #to_sym converts; rbgo accepted only Symbol and String and raised TypeError on the rest. A String needs no case of its own — String#to_sym does it. Both TypeError messages are MRI's, verified byte-for-byte: warn("m", category: Object.new) -> no implicit conversion of Object into Symbol def o.to_sym; 5; end; warn("m", category: o) -> can't convert Object to Symbol (Object#to_sym gives Integer) And the level is now converted ONCE (it was run through toIntCoerce twice on the raise path), so a #to_int that counts its calls is not called twice. Verified byte-for-byte against ruby 4.0.5 on the uplevel matrix (levels 0/1/past-the-bottom, empty and nil messages, several messages under one prefix, a lone message already ending in a newline) and on the fixture core/kernel/fixtures/classes.rb the spec uses: identical output, including that the "path:lineno: warning: " prefix is written once and not per line. core/kernel/warn_spec.rb stays at pass=21. MRI 4.0.5 run under the SAME shim scores 25, and the four it beats us by all reduce to one gap outside this wave's files: methodSourceLocation (internal/vm/method_reflect2.go) still returns line 0, so the spec's own warn_call_lineno is 1 rather than 434 and every uplevel assertion compares against the wrong number. The remaining 4 MRI also fails are shim limits (Warning[:deprecated] default, Warning.warn arity), not rbgo bugs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Thread.each_caller_location was not defined at all (pass=0 on
core/thread/each_caller_location_spec.rb). It yields each frame of the
current execution stack as a Thread::Backtrace::Location and answers nil.
Read for this: ruby/ruby v3_4_0 vm_backtrace.c each_caller_location
(:1401) and ec_backtrace_range (:1300). each_caller_location selects its
range with ec_backtrace_range(ec, argc, argv, 1, 1, &n) — the SAME
lev_default and lev_plus that rb_f_caller_locations hands
ec_backtrace_to_ary — so its argument handling is Kernel#caller_locations'
exactly, Range form and nil-for-overshoot included. callerSlice therefore
serves both rather than the rule being restated here.
Three details that come from reading it rather than from the spec:
- rb_scan_args(argc, argv, "02:") caps the positionals at two, and the
rb_get_kwargs that follows runs against an EMPTY keyword table, so
EVERY keyword is an unknown one ("unknown keyword: :foo").
- the LocalJumpError is raised by the rb_yield inside the iteration, not
by a guard before it: `if (lev >= 0 && n != 0)`. A selection that
yields nothing is therefore not an error even with no block, which is
why the check here is on "about to yield at least once".
- the message is "no block given", not the "(yield)" form.
Verified against ruby 4.0.5: identical for the default form, explicit
(start, length), a Range, a start past the top, no block, one and two
unknown keywords, three positionals and a negative level.
core/thread/each_caller_location_spec.rb: pass=0 -> pass=5 of 6.
The sixth ("stops the backtrace iteration if 'break' occurs") fails on a
PRE-EXISTING defect that is not in this wave's files and that my change
neither causes nor can reach: a `break` out of a block leaves its exec
frame on vm.frameNames. exec's recover handler truncates the per-frame
stacks for a returnSignal only, and a breakSignal re-panics past it, so
every backtrace taken after a break is one frame too deep. Reproduced on
main with no Thread involved at all:
def top
[1,2,3].each { |x| break x if x == 2 }
p caller_locations(1,2).map(&:to_s)
end
def mid; top; end
mid
ruby 4.0.5 -> ["...:5:in 'Object#mid'", "...:6:in '<main>'"]
rbgo -> ["...:3:in 'top'", "...:5:in 'mid'"]
The fix belongs in exec's recover (internal/vm/vm.go) or in callBlock
(internal/vm/object_model.go), both owned by other agents this wave.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rbgo printed `foo` where ruby prints `Object#foo`. The owner prefix was
left out of the line-map wave deliberately: it needs the frame's DEFINING
MODULE rather than its ISeq, and it moves every method label in every
backtrace, so it is one coherent change rather than a bolt-on.
Read for this: ruby/ruby v3_4_0 vm_backtrace.c rb_gen_method_name (:205),
calculate_iseq_label (:229), location_label (:285) and location_to_str
(:460), plus variable.c classname / rb_mod_name0 (:104).
- rb_gen_method_name qualifies a method name with its owner: "Owner#name"
for an instance method, "Owner.name" for a singleton one, taken from
the attached object when the owner is a singleton class.
- it does so ONLY when the class path is PERMANENT. classname() reports
RCLASS_EXT(klass)->permanent_classpath beside the path, and a
non-permanent or absent one drops the prefix entirely — so an anonymous
Class.new labels its methods bare. rbgo's RClass.named is that flag.
- calculate_iseq_label recurses THROUGH the block decoration, so the
prefix belongs to the method a block was written in and the
"block (2 levels) in " goes back in front of the qualified name.
- ISEQ_TYPE_TOP/CLASS return location.label untouched: "<main>",
"<class:K>", "<module:M>" and a singleton-class body take no prefix.
- location_label and location_to_str BOTH go through it, so the prefix is
on Thread::Backtrace::Location#label, on Kernel#caller and on the lines
of an exception's backtrace alike.
Two things fall out of reading it that the spec does not ask for:
- an ALIASED method reports its ORIGINAL name. MRI's label is the ISeq's
location.label, the name the body was defined under; rbgo printed
frameNames[i], the name it was CALLED by. The other half of the pair is
already recorded in frameMethods[i].orig, so the label now uses it.
- Location#base_label must strip BOTH decorations, not just the block
one. MRI reads it from a separate field holding the plain compiled
name, so "block (2 levels) in C#foo" has base_label "foo".
stripOwnerPrefix removes the owner only when what precedes the
separator is a constant path (reusing isConstantPath, MRI's
is_constant_path) — which is what leaves `main.label_sdef_method_of_main`
whole.
rbgo files a class method under the CLASS (C.smethods, owner == C) where
MRI files it under C's singleton class, so which of the two tables holds
the name is what picks "#" from "." — hence frameOwner returns the flag
rather than reading owner.isSingleton.
Verified byte-for-byte against ruby 4.0.5 on a 15-case matrix: instance
method, block in one, nested block, `def self.x`, `class << self`, alias,
base_label, module function, module instance method via extend, a nested
class path, a top-level def, a per-object singleton, an anonymous class, a
top-level block, and an exception backtrace line.
core/thread/backtrace/location/label_spec.rb: pass=7 -> pass=27 of 41.
(MRI 4.0.5 under the same shim scores 40; the gap is itemised in the PR.)
Six pinned assertions moved, each re-measured by writing the test's exact
source to a file and running ruby 4.0.5 on it:
backtrace_test.go TestBacktraceCaptureChain 'foo' -> 'Object#foo'
'bar' -> 'Object#bar'
backtrace_test.go TestReRaisePreservesBacktrace 'deep' -> 'Object#deep'
backtrace_test.go TestFullMessageShape 'foo' -> 'Object#foo'
backtrace_test.go TestUncaughtBacktraceEscaping 'boom' -> 'Object#boom'
backtrace_test.go TestCallerStillWorks 'a' -> 'Object#a'
kernel_warn_internal_test.go TestKernelWarn two uplevel: cases now
carry the location the previous commit made available
wave28_lines_internal_test.go's TestStripBlockQualifier is replaced by
TestSplitBlockQualifier and TestStripOwnerPrefix, the two halves
base_label now has to undo.
Also fixed, because this change is what made it visible: Kernel#catch
restored frameNames on a matched throw but not frameCrefs/frameMethods,
which mirror it one-for-one. The three slices then sat at different depths
for the rest of the program and frameCrefs[i] stopped describing frame i.
Nothing noticed while a backtrace read frameNames alone. Reachable from
Ruby:
def probe(t); puts "#{t}: #{caller_locations(0)[0].label}"; end
probe("before")
def deep(n); n.zero? ? throw(:done, 1) : deep(n - 1); end
catch(:done) { deep(5) }
probe("after")
before main -> Object#probe / Object#probe (ruby 4.0.5 and rbgo now)
before fix -> Object#probe / probe
THREE MORE SITES truncate frameNames alone and are in files this wave does
not own; they are named in the PR body. frameStacksAligned makes a drift
from any of them cost the prefix rather than print another frame's owner.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Whitebox tests for every branch the previous two commits added, plus the
removal of one that was dead.
Removed: frameOwner tested `cref.attached.(*RClass)` after `cref.metaOf`,
as a second way to reach rb_gen_method_name's "attached object is a class
or module" case. It cannot happen. The three sites that set `attached`
(object_model.go singletonClass and ensureSingleton, builtins.go
cloneSingleton) only ever store a per-object receiver, and ensureSingleton
refuses a class outright — `case *RClass: return nil, false`, classes use
metaClass() — so metaOf IS that case and the type assertion could never
succeed. Covering it would have meant writing a test for a state the VM
cannot enter; the branch is gone and the invariant is in the comment.
Added (internal/vm/wave29_owner_internal_test.go):
- TestFrameOwnerNilCref: exec pushes frameCrefs[i] as nil and fills it in
once the frame's lexical scope is settled. The window is inside one
exec call and cannot be held still from Ruby, so the guard is exercised
against a stack built by hand.
- TestFrameStacksAlignedGuard: both drift shapes (a leaked frameCrefs
entry, a leaked frameMethods entry) make the label fall back to the
bare name instead of reporting another frame's owner.
- TestGenMethodNamePermanence: the permanence rule, including the
Module#set_temporary_name shape (name set, named false). Checked on
ruby 4.0.5: a Class.new given set_temporary_name("fake name") still
labels its method "m", not "fake name#m".
And in wave29_lineharvest_test.go, the uplevel level EXACTLY one past the
outermost frame, which is the `i < 0` arm rather than the int64 guard: at
the top level there is one frame, so uplevel 1 already has no location.
ruby 4.0.5 prints "warning: x" for the same two lines.
Coverage compared as a LIST against origin/main, not as a rounded total —
and with each profile rendered against ITS OWN source tree, since
`go tool cover -func` resolves line numbers against the working directory
and reading main's profile from this worktree misattributed 18 functions
to 0.0%. Rendered correctly, internal/vm shows the SAME 15 functions under
100% on main and on this branch, at the same percentages: no gap opened,
none closed. None of the new functions is among them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 25, 2026
tannevaled
added a commit
that referenced
this pull request
Sep 25, 2026
Three units, and this wave is mostly the line map paying out. #649 Harvesting what #645 made possible: owner-qualified backtrace labels (Object#foo where we printed foo), warn(uplevel:) prepending the caller's file and line — its scaffolding still carried the comment "rbgo has no caller line to prepend", untrue since yesterday — and Thread.each_caller_location. #650 Keywords decided at the CALL SITE, as MRI does: a trailing Hash is a positional argument unless the call site wrote it as keywords. The new flag is NEGATIVE on purpose — only sites the compiler can prove positional are flagged, so Object#send, Proc#call and define_method bodies keep the older behaviour rather than silently losing keywords. Also: post parameters bind BEFORE optionals (def q(a, b=9, *c, d) gave [1, nil, [], 2] where MRI gives [1, 9, [], 2]), a duplicated parameter name is a SyntaxError, and the implicit `it` block parameter. #648 Marshal's ivar and extend containers, Time's instance variables and the :zone/:offset round trip, String#unpack encodings (+27) #647 dependency bump, green without intervention Measured with scripts/conformance/rubyspec/run.sh against the pinned corpus at 87b1631, 16-way, TIMEOUT=25, three consecutive runs: 22227, 22226, 22226. The floor is set eleven below the lowest — the margin rule the last four bumps used. One defect this wave exposed rather than introduced, now issue #651: three parallel frame stacks are kept in step by hand, and FOUR sites truncate frameNames alone. It went unnoticed while backtraces read that stack alone; frameLabel now reads all three, so one leaked entry makes every later label name a different frame. One site fixed in #649, three named and routed. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
tannevaled
added a commit
that referenced
this pull request
Sep 26, 2026
…d three unwinds restore all three frame stacks (#651) (#674) * wave 33: every object carries an ivar table (#672), and three unwinds restore all three frame stacks (#651) instance_variable_set was a silent no-op on File, StringIO, Dir, IO::Buffer, Array, Hash, String and Proc: ivarTable answered nil for those kinds, so the write was DISCARDED and the read answered nil, with nothing raised either way. An attr_accessor on such an object's singleton class looked like it worked and stored nothing. They now answer from a generic ivar table, which is MRI's own shape: variable.c's ivar_set sends T_OBJECT to rb_obj_ivar_set, T_CLASS/T_MODULE to rb_class_ivar_set and EVERYTHING ELSE to generic_ivar_set, which keys generic_iv_tbl_ by the object and creates the entry on the FIRST WRITE. A read never creates one (rb_gen_ivtbl_get only looks up), so a value no ivar is ever written to pays nothing: no field on the struct, no allocation, no map entry. ivarTable keeps its read-only signature for its callers in vm.go and builtins.go; setIvar goes through ivarStoreOf(self, true). An immediate keys nothing, matching MRI's SPECIAL_CONST_P screen: it is a Go value here, so a copy would answer to the entry of every equal copy, and Object#instance_variable_set already raises FrozenError for all of them. Names come out in first-assignment order for every kind that records one — including the generic table — which is what MRI reports (rb_ivar_foreach walks the shape chain in creation order). Item 2: findYield (Find.prune), irbEval (any exception in an IRB statement) and PStore#transaction (#commit / #abort) restored frameNames, frameFiles, fileStack and requireDirs and left frameCrefs and frameMethods LONG. Since #649 frameLabel reads all three, so one leaked entry made every later label name a different frame. All three now restore the six depths together through truncFrames (#662), which leaves an already-shorter stack alone. Witness, byte-for-byte against MRI 4.0.5 (labels after a Find.prune and a PStore#commit): before ["labels", "peek", "<main>"], after and MRI ["Object#labels", "Box#peek", "<main>"]. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * wave 33: benchmarks for the ivar paths #672 touches Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * wave 33: cover the lazily allocated template-context ivar tables ivarStoreOf's ActionViewBase arm allocates its map on first touch and nothing exercised it (the same block was uncovered on main, through ivarTable). One loop covers both context kinds, so the below-100 list is one entry shorter than main's rather than the same length. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * wave 33: assert MRI's own output, and stop asserting a Range divergence The natively-backed ivar test compared a hand-built string with a trailing space; it now compares the exact line MRI 4.0.5 prints for the same program. The Range was dropped from the list: MRI raises FrozenError there (a Range is frozen) where rbgo stores, because isFrozen does not list *object.Range — a divergence this wave cannot fix (builtins.go) and must not bake into a test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * wave 33: an ivar re-assigned after remove_instance_variable appears once, at the end setIvar appended the name to the order list without dropping the entry a remove_instance_variable had left behind, so `set @A; set @b; remove @A; set @a` reported [:@A, :@b, :@A] where MRI reports [:@b, :@A]. MRI removes the name from the SHAPE (variable.c rb_ivar_delete through rb_shape_transition_shape_remove_ivar), so a re-assignment is a fresh one, at the end, once. The defect predates #672 for a plain object and #672 would have carried it to every natively-backed kind ([:@A, :@A] for a String). Measured byte-for-byte against MRI 4.0.5. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
tannevaled
added a commit
that referenced
this pull request
Oct 4, 2026
sendCatchBreak recovers a breakSignal and returns its value, but it
never restored the per-frame tracking stacks. The block's exec frame —
and every frame between it and the catch — unwound by panic and so
never ran exec's tail that pops them, leaving
[1, 2, 3].each { |x| break if x == 2 }
one frameNames entry heavier than it started, permanently.
This was invisible while those stacks only fed backtraces and
Kernel#caller: the next unwind CAUGHT anywhere truncates to its own
saved depth and swallows the stale entry with it, so the leak only
survives when nothing deeper is caught afterwards. No backtrace test
ever stood in that position.
It stopped being invisible the moment len(frameNames) became the
call-depth limit: leaked depth is budget that never comes back, so a
long-running program breaking out of a loop a few tens of thousands of
times would raise SystemStackError with no recursion in sight. The
test that found it measures the depth a runaway recursion reaches,
twice, with the non-local exit in between — 197 then 196 is the leak,
197 then 197 is the fix.
The restoration is the same six depths Kernel#catch and findYield keep
(find.go), for the same reason. frameCrefs and frameMethods mirror
frameNames one for one and are restored WITH it, or index i stops
meaning the same frame in all three (#649).
TestDepthDoesNotLeakOnAnyNonLocalExit covers break, next, a block
`return`, throw/catch, an ensure crossed by a raise, and the deep
variants of each; TestASecondRunawayRecursionStillRaises states the
consequence a user would notice.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tannevaled
added a commit
that referenced
this pull request
Oct 4, 2026
…768) (#769) * fix(vm): raise SystemStackError instead of killing the host process (#768) A Ruby program with runaway recursion took the whole host process down with it. The first line of output was runtime: goroutine stack exceeds 1000000000-byte limit followed by 50772 bytes of goroutine dump and exit status 2, where MRI 4.0.5 prints a rescued SystemStackError and exits 0. This is not only a conformance gap. A Go stack overflow is a FATAL RUNTIME ERROR, not a panic: a deferred recover() never runs, so an embedding host cannot contain it. ruby.Run is the documented embedding API, so three lines of untrusted Ruby could end the host process. Two parts: * SystemStackError, under Exception and NOT StandardError, so a bare `rescue` does not catch it. MRI sets it up that way in Init_Proc (proc.c v3_4_1:4424) and registers "stack level too deep" on the line after; the message here is copied, not invented. Verified against the MRI 4.0.5 on this machine: SystemStackError.ancestors is [SystemStackError, Exception, Object, Kernel, BasicObject]. * A call-depth limit in exec, which is the ONE place a Ruby-level frame is created, so one check covers methods, blocks, procs, lambdas, define_method bodies, class and module bodies and eval. The depth is len(vm.frameNames) rather than a counter of its own, and that is the point: a counter would need decrementing on each of exec's exits, and the ones that unwind by panic never run exec's tail. frameNames is already restored on every unwind path, and restored by TRUNCATING TO A SAVED DEPTH rather than by counting pops, so the depth cannot be left inflated by a rescued runaway recursion and cannot poison a later one. raiseDeep needs no Ruby frame of its own, which is what lets it be called from the place that has just refused to create one: exceptionObject builds the exception as a Go *RObject and never dispatches #initialize, and rbgo runs rescue/ensure bodies as instructions inside the frame that owns them. MRI needs a preallocated instance for exactly the same reason. The constant is a placeholder in this commit; the measurement that chooses it, and the standing test that re-derives it, follow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(vm): a block `break` leaked one frame-stack entry for good (#768) sendCatchBreak recovers a breakSignal and returns its value, but it never restored the per-frame tracking stacks. The block's exec frame — and every frame between it and the catch — unwound by panic and so never ran exec's tail that pops them, leaving [1, 2, 3].each { |x| break if x == 2 } one frameNames entry heavier than it started, permanently. This was invisible while those stacks only fed backtraces and Kernel#caller: the next unwind CAUGHT anywhere truncates to its own saved depth and swallows the stale entry with it, so the leak only survives when nothing deeper is caught afterwards. No backtrace test ever stood in that position. It stopped being invisible the moment len(frameNames) became the call-depth limit: leaked depth is budget that never comes back, so a long-running program breaking out of a loop a few tens of thousands of times would raise SystemStackError with no recursion in sight. The test that found it measures the depth a runaway recursion reaches, twice, with the non-local exit in between — 197 then 196 is the leak, 197 then 197 is the fix. The restoration is the same six depths Kernel#catch and findYield keep (find.go), for the same reason. frameCrefs and frameMethods mirror frameNames one for one and are restored WITH it, or index i stops meaning the same frame in all three (#649). TestDepthDoesNotLeakOnAnyNonLocalExit covers break, next, a block `return`, throw/catch, an ensure crossed by a raise, and the deep variants of each; TestASecondRunawayRecursionStillRaises states the consequence a user would notice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(vm): set the call-depth limit from a measurement, 16384 (#768) 28 recursion paths bisected in subprocesses — binary-searching the largest limit each path can be given before the Go runtime kills the process, under a reduced debug.SetMaxStack — to get BYTES OF GO STACK PER RUBY FRAME, which is the portable quantity. Worst and best, on darwin/arm64 with Go 1.27.1: send(:public_send, :send, :f, n + 1) 8132 B/frame WORST catch(:x) { f(n + 1) } 6531 B/frame 1.times { f(n + 1) } 6228 B/frame f(n + 1) 5619 B/frame best Measuring the cheapest path would have overstated the margin by 45%. The figure is stable across stack sizes — 6452.8, 6450.3, 6448.4, 6448.1 B/frame at SetMaxStack of 8, 16, 64 and 256 MiB — so it is a property of the call path, not of the measurement. THE DEPTH IT IMPLIES IS NOT 1 GB DIVIDED BY IT, and that error would have been a factor of two. Go's default maximum is 1,000,000,000 bytes (decimal) and goroutine stacks are powers of two, so the largest stack actually permitted is 2^29 = 512 MiB; 2^30 exceeds the default. The worst path's overflow depth is 512 MiB / 8132 = 66016, and bisecting it directly at Go's real default with no SetMaxStack measured 66051 — 0.05% apart, which is what makes the model quotable rather than a story about one number. 16384 is 4.03x below that 66051 and 1.50x above MRI's deepest reachable depth (10919 frames of `def f(n); $d = n; f(n + 1); end` on the MRI 4.0.5 on this machine; only 3853 levels through a block, because MRI's budget is in BYTES and so its depth is not one number). Above MRI means no program that works there raises here; the asymmetric margin below is because overshooting is fatal and unrecoverable while undershooting raises an exception the program already had to handle. TestStackDepthLimitSitsBelowTheGoOverflow re-derives the whole thing on every run and fails under a 2x margin. KNOWN INCOMPLETE, and measured rather than assumed: a frame count cannot bound the Go stack on its own, because a program can put an unbounded number of NATIVE dispatch layers between two Ruby frames, and a native layer costs Go stack while pushing no frame. Each extra Kernel#send layer costs 833 B: sends per Ruby frame 1 2 4 8 16 32 bytes per frame 7282 8117 9777 13107 19761 33091 overflow depth 73728 66144 54912 40960 27168 16224 so ~25 nested sends per frame bring the overflow depth below this limit, and NO limit above MRI's 10919 can cover an unbounded chain. Closing it needs a count of native dispatch nesting too, which is a change to the hot dispatch path (vm.send) and a scope decision of its own rather than a line added here. Recorded in stackdepth.go and on issue #768 rather than left for the next person to rediscover. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(vm): the limit holds across a thread boundary (#768) A shape the design could plausibly have got wrong. rbgo keeps ONE frame stack for the whole VM, shared by every Ruby thread under the GVL, and Run resets it to [:0] at its boundaries — so a thread body could have been handed a FRESH depth budget while its parent's Go frames were still live, letting a program amplify its reachable depth by one limit per thread it spawns. Measured with the real constant rather than reasoned about: plain recursion reaches 16382 and the thread-nested shape reaches 8191, exactly half, because each level costs two entries (the method frame and the block frame). The shared stack keeps accumulating across the boundary instead of restarting. That is conservative — each Ruby thread is a goroutine with its own Go stack, so the budget is stricter than the stacks require — and conservative is the safe direction for a limit whose job is to stay below a fatal error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(vm): set maxCallDepth in bootstrap, not in the constructor (#768) A VM is not always built through New: internal tests assemble a &VM{} literal and bootstrap it themselves (bareVM in prelude_test.go). Those VMs got maxCallDepth 0, and because exec's check is `>=`, a ZERO LIMIT REFUSES EVERY FRAME rather than none. The full suite said so: --- FAIL: TestLoadSourceRuntimeError got SystemStackError: stack level too deep, want NoMethodError --- FAIL: TestLoadSourceSuccess unexpected error: SystemStackError: stack level too deep `x = 1 + 1` died on its first call. Confirmed as a regression from this branch and not a pre-existing failure by running ./internal/vm on origin/main in a separate checkout: it passes there. bootstrap is the one gate every usable VM passes through — a VM that has not been bootstrapped has no classes and cannot run anything — so the assignment belongs there, unconditionally, with the constructor's copy removed so there is one source of truth and nothing to drift. A test that wants a lower limit sets it after construction, which is after bootstrap has run. TestEveryBootstrappedVMHasALimit now covers the literal-plus-bootstrap path and not only the constructor, and says why zero is the dangerous value in both directions: `>=` reads it as "refuse everything", and a check written the other way round would read it as "no limit at all", which is silently the original defect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Harvest the line map: owner-qualified labels,
warn(uplevel:),Thread.each_caller_locationUntil #645 the parser's AST carried no source positions, so
__LINE__was 0 everywhere and the ISeq had no line table. #645 built MRI's compressedinsns_info/positionstable onast.Program.Lines. Four spec files were waiting on that. This PR takes three of them; the fourth is blocked and is explained below.Every behaviour here was checked byte-for-byte against
ruby 4.0.5before it was written, and each spec file was also run through the same shim under MRI so that a shim limitation is never mistaken for an rbgo defect.1.
warn(uplevel:)prepends the caller's file and lineinternal/vm/io.go. Read: error.crb_warn_m. The location comes fromrb_ec_backtrace_location_ary(ec, lev + 1, 1, TRUE)and is formattedrb_sprintf("%s:%ld: warning: ", path, lineno), falling back to a bare"warning: "when there is none. The+ 1skips thewarncfunc's own frame; rbgo pushes no frame for a native, so level 0 is already the top offrameNames.The scaffolding carried the comment "rbgo has no caller line to prepend". That is no longer true, and the comment is gone with the code.
Also from
rb_warn_m:category:goes throughrb_to_symbol_type, i.e.rb_convert_type_with_id(val, T_SYMBOL, "Symbol", idTo_sym). Anything answering#to_symconverts; rbgo accepted onlySymbolandStringand raisedTypeErroron the rest. BothTypeErrormessages now match MRI exactly. The level is also converted once rather than twice on the raise path.2.
Thread.each_caller_locationinternal/vm/thread.go. Read: vm_backtrace.ceach_caller_location(:1401) andec_backtrace_range(:1300). It selects its range withec_backtrace_range(ec, argc, argv, 1, 1, &n)— the samelev_default/lev_plusrb_f_caller_locationspasses — so its argument handling isKernel#caller_locations'.callerSlicetherefore serves both instead of the rule being restated.Three details that came from reading it rather than from the spec:
rb_scan_args(argc, argv, "02:")caps the positionals at two, and therb_get_kwargsafter it runs against an empty keyword table — so every keyword is an unknown one.LocalJumpErroris raised by therb_yieldinside the iteration, not by a guard before it. A selection that yields nothing is not an error even with no block."no block given", not the"(yield)"form.3. The owner prefix on every backtrace label
internal/vm/kernel_introspect.go,exception.go. Read: vm_backtrace.crb_gen_method_name(:205),calculate_iseq_label(:229),location_label(:285),location_to_str(:460); variable.cclassname/rb_mod_name0(:104).rbgo printed
foowhere ruby printsObject#foo.location_labelandlocation_to_strboth go throughrb_gen_method_name, so the prefix lands onThread::Backtrace::Location#label, onKernel#callerand on the lines of an exception's backtrace alike.Points that mattered:
classname()reportspermanent_classpathbeside the path and a non-permanent one drops the prefix, so an anonymousClass.newlabels its methods bare — as does a class givenModule#set_temporary_name.calculate_iseq_labelrecurses through the block decoration, so the owner belongs to the method the block was written in andblock (2 levels) ingoes back in front of the qualified name.ISEQ_TYPE_TOP/CLASSreturnlocation.labeluntouched:<main>,<class:K>,<module:M>and a singleton-class body take no prefix.C.smethods,owner == C) where MRI files it underC's singleton class, so which of the two tables holds the name is what picks#from..Two things fell out of reading the C that the spec does not ask for:
location.label, the name the body was defined under; rbgo printed the name it was called by. The other half is already inframeMethods[i].orig.Location#base_labelmust strip both decorations. MRI reads it from a separate field holding the plain compiled name, so"block (2 levels) in C#foo"has base label"foo".stripOwnerPrefixremoves the owner only when what precedes the separator is a constant path (reusingisConstantPath, MRI'sis_constant_path) — which is what leavesmain.label_sdef_method_of_mainwhole.Verified byte-for-byte on a 15-case matrix: instance method, block in one, nested block,
def self.x,class << self, alias,base_label, module function, module instance method viaextend, a nested class path, a top-leveldef, a per-object singleton, an anonymous class, a top-level block, and an exception backtrace line.A latent defect this surfaced: three parallel frame stacks, four places that move one of them
frameNames,frameCrefsandframeMethodsmirror each other one-for-one, and indeximeans the same frame in all three only while they are the same length.execpushes and pops them together and every unwind restores them together — except at four sites that truncateframeNamesalone:internal/vm/builtins.go:381(Kernel#catch)internal/vm/find.go:153(findYield)Find.pruneinsideFind.findinternal/vm/irb_bind.go:65internal/vm/pstore.go:366PStore#transactionwith#commit/#abortThe drift is reachable from Ruby today — it is not merely latent. Nothing noticed while a backtrace read
frameNamesalone;frameLabelnow reads all three, and one leaked entry made every later label some other frame's. Reproductions, both againstruby 4.0.5:I did not reach into the other three files.
frameStacksAlignedmakes a drift from any of them cost the prefix rather than print another frame's owner — a visible degradation instead of a wrong answer — but it is a guard, not the fix. These three deserve their own issue, and the reproduction above is ready to paste into it.On the shape. Three parallel slices kept in step by hand at seven push/pop/restore sites is a structure that will drift again; it already has, four times. The shape that makes it impossible is one slice of one struct —
frameCodein the same file is already half of that, mergingiseqandpcinto aframeCodestruct for exactly this reason, and its comment records that the merge cost nothing measurable (bench/blocks.rb, +7% either way). FoldingframeNames,frameFiles,frameCrefsandframeMethodsinto that same struct would make a partial restore unrepresentable rather than merely wrong: there would be one slice to truncate. I have not done it — it touchesvm.go, which another agent owns this wave — but the measurement that would normally gate it has already been taken.Measurements
core/thread+core/module+core/kernel+language/(recursive), 336 files, per file, at most 2-way:core/thread/backtrace/location/label_spec.rbcore/thread/each_caller_location_spec.rbTotal 5459 → 5484 (+25), with no per-file regression.
Two numbers that needed re-running before being believed, both pre-existing and neither caused by this branch:
core/kernel/require_spec.rbread 68 under load and 69 serially; it is 69 both before and after.core/module/autoload_spec.rb(Intermittent crash popping a frame: slice bounds out of range [:-1] #615) crashes roughly 1 run in 4 and then contributes 0, which reads as a ~60-example regression. Measured on this branch: 60, 61, 61. On main: crash, 61. One sweep of three showed the crash; the other two did not.language/ensure_spec.rbgenuinely regressed 31 → 30 mid-way, which is how the frame-stack drift was found; it is back to 31 with theKernel#catchfix.FLOORis untouched, as instructed. The ratchet is now +25 above it.Gates
CGO_ENABLED=0 go build ./...— clean;GOOS=wasip1 GOARCH=wasm— clean.CGO_ENABLED=0 GOWORK=off go test ./...— the full suite, green.gofmt -l internal/vm/— empty.origin/main, not as a rounded total.internal/vmshows the same 15 functions under 100% on main and on this branch, at the same percentages: no gap opened, none closed, and none of the new functions is among them.A trap worth recording:
go tool cover -funcresolves a profile's line numbers against the working directory's source. Reading main's profile from this worktree misattributed 18 functions to 0.0% and made the branch look like it had closed 18 gaps. Each profile has to be rendered from its own tree.frameOwnertestedcref.attached.(*RClass)as a second route torb_gen_method_name's "attached object is a class" case. All three sites that setattachedstore a per-object receiver, andensureSingletonrefuses a class outright (case *RClass: return nil, false), so the assertion could never succeed.metaOfis that case.Six pinned assertions moved
Each was re-measured by writing the test's exact source to a file and running
ruby 4.0.5on it:TestBacktraceCaptureChain'foo','bar''Object#foo','Object#bar'TestReRaisePreservesBacktrace'deep''Object#deep'TestFullMessageShape'foo''Object#foo'TestUncaughtBacktraceEscaping'boom''Object#boom'TestCallerStillWorks'a''Object#a'TestKernelWarn"warning: x\n""(rbgo):1: warning: x\n"(twouplevel:cases)TestStripBlockQualifieris replaced byTestSplitBlockQualifierandTestStripOwnerPrefix, the two halvesbase_labelnow has to undo.Left undone, and why
core/module/const_source_location_spec.rb(30 examples) — not attempted. It cannot be done from this wave's files.The prerequisite really is in place; the blocker is ownership, not the line map.
const_source_locationneeds the file and line recorded where a constant is assigned, and in rbgo every user-level constant assignment funnels throughassignConstIn(internal/vm/vm.go:2480), reached fromOpSetConstIn(:1671) andOpSetConst(:1684). All three are invm.go, which another agent owns this wave. I checked for a hook I could use instead:fireConstAddedexists inbuiltins.goand would have been exactly right, but it is called only fromModule#const_setand fromautoload.go— a plainA::B = vdoes not fire it. Reading the location back inmodule_residuals.gois a handful of lines once the recording exists.Read for it anyway, so the next attempt does not have to: variable.c
rb_const_source_location→rb_const_location(klass, id, FALSE, TRUE, FALSE)→rb_const_location_from(:3195). Noteif (NIL_P(ce->file)) return rb_ary_new()— a constant defined in C answers[], notnil; a private constant answersnilundervisibility; an autoload that has not fired answers the autoload's file and line only while the autoload mutex is held; and for a module receiver the search falls back torb_cObjectafter the ancestor walk.Two more gaps outside this wave's files, each of which blocks specs that would otherwise pass:
methodSourceLocation(internal/vm/method_reflect2.go:190) returns line 0 rather thanis.FirstLine, althoughProc#source_locationinbuiltins.goalready uses the line map. This is a one-line change and it gates 4 of the 8warn(uplevel:)examples: the spec computes its expected line asmethod(:f1).source_location[1] + 1, which is1instead of434, so every uplevel assertion compares against the wrong number.core/kernel/warn_spec.rbtherefore stays at pass=21 even though the output is byte-identical to MRI's. MRI under the same shim scores 25; the other 4 it beats us by are shim limits (theWarning[:deprecated]default andWarning.warnarity), which MRI fails too.a bare constant inside a
class A::Bbody does not resolve through the enclosing lexical scope.class M::Dnested inmodule Scannot seeS::LABEL; MRI can.OpDefineClass's scoped-path branch (vm.go:1980) does not give the new class the enclosing scope as itslexParent. This costs 6 of the 14 remaininglabel_specexamples, asNameError: uninitialized constant LABELfrom the fixture.label_specis at 27 of 41 against MRI's 40 under the same shim. The 13:M::D#scoped_methodand five siblingsclass A::Bconstant-lookup gap above (vm.go)a core method defined nativelyvm.go)a core method defined in RubyKernel.instance_method(:tap).source_location—method_reflect2.go, and the prelude carries no filea method defined via module_exec,M#module_eval_methodmodule_exec/module_evalblock the def target and the textual nesting differ, and the owner is not recoverable from the frame. Reports the bare name rather than a wrong owner.M.mod_functionmodule_functionfiles oneMethodin both tables; MRI tells the two call paths apart by thecmethe call resolved to, which rbgo does not carry per frame. The instance form is chosen, soM#mod_functionpasses andM.mod_functiondoes not.return the same name as the caller for evalreturns the base label of the call frameeach_caller_locationis at 5 of 6. The sixth,stops the backtrace iteration if 'break' occurs, fails on a pre-existing defect with noThreadin it: abreakout of a block leaves itsexecframe onvm.frameNames, becauseexec's recover truncates for areturnSignalonly and abreakSignalre-panics past it. Every backtrace taken after abreakis one frame too deep.The fix belongs in
exec's recover (vm.go) or incallBlock(object_model.go), both owned elsewhere this wave.🤖 Generated with Claude Code