From fc26822f9fb437eff0af2fb38394dfbe336aa59c Mon Sep 17 00:00:00 2001 From: tannevaled Date: Thu, 24 Sep 2026 22:16:15 +0200 Subject: [PATCH 1/4] vm: warn(uplevel:) prepends the caller's file and line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- internal/vm/io.go | 81 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 62 insertions(+), 19 deletions(-) diff --git a/internal/vm/io.go b/internal/vm/io.go index 5bcb91bb..1ba7c8c9 100644 --- a/internal/vm/io.go +++ b/internal/vm/io.go @@ -461,9 +461,8 @@ func (vm *VM) registerIO() { // reads the category) sees it, as in MRI. A String category is converted to a // Symbol (anything else is a TypeError); a category whose warnings are disabled // (Warning[category] is false) is dropped, and an unknown category raises. The - // uplevel: keyword is validated (a negative or non-Integer value raises) but not - // acted on, as rbgo has no caller line to prepend. With no message it does - // nothing. + // uplevel: keyword names a caller frame whose "path:lineno: " is prepended to + // the message (see warnUplevelPrefix). With no message it does nothing. vm.cObject.define("warn", func(vm *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { var category object.Value = object.NilV pos := args @@ -484,11 +483,14 @@ func (vm *VM) registerIO() { if object.IsNil(vm.verboseSlot("$VERBOSE")) || len(pos) == 0 { return object.NilV } + lev := int64(0) if !object.IsNil(uplevel) { // rb_warn_m runs the level through NUM2LONG, so a Float or a Rational - // truncates and anything with no integer conversion is a TypeError. - if int64(vm.toIntCoerce(uplevel)) < 0 { - raise("ArgumentError", "negative level (%d)", vm.toIntCoerce(uplevel)) + // truncates and anything with no integer conversion is a TypeError. The + // conversion runs ONCE: a to_int that counts its calls must not see two. + lev = int64(vm.toIntCoerce(uplevel)) + if lev < 0 { + raise("ArgumentError", "negative level (%d)", lev) } } // rb_warn_m keeps a lone message that already ends in a newline VERBATIM @@ -502,13 +504,7 @@ func (vm *VM) registerIO() { b.WriteString(first.Str()) } else { if !object.IsNil(uplevel) { - // With a level, rb_warn_m prefixes "path:lineno: warning: " taken from - // rb_ec_backtrace_location_ary — and, when that yields no location (a - // level past the bottom of the stack), the bare "warning: ". rbgo - // records no line numbers at all, so no frame can ever supply a path: - // the no-location prefix is the truthful answer for every level, not a - // choice. Fixing it means line tracking in internal/compiler. - b.WriteString("warning: ") + b.WriteString(vm.warnUplevelPrefix(lev)) } write := func(s string) { b.WriteString(s) } for _, a := range pos { @@ -519,12 +515,24 @@ func (vm *VM) registerIO() { return object.NilV } if !object.IsNil(category) { - switch c := category.(type) { - case object.Symbol: - case *object.String: - category = object.Symbol(c.Str()) - default: - raise("TypeError", "no implicit conversion of %s into Symbol", vm.classOf(category).name) + // rb_warn_m runs the category through rb_to_symbol_type, which is + // rb_convert_type_with_id(val, T_SYMBOL, "Symbol", idTo_sym): anything + // that answers #to_sym converts, and only a value that does not (or + // whose #to_sym returns a non-Symbol) is the TypeError. A String + // converts through its own String#to_sym, so it needs no case of its + // own here. + if _, isSym := category.(object.Symbol); !isSym { + cls := vm.classOf(category).name + if !vm.respondsToConversion(category, "to_sym") { + raise("TypeError", "no implicit conversion of %s into Symbol", cls) + } + conv := vm.send(category, "to_sym", nil, nil) + sym, isSym := conv.(object.Symbol) + if !isSym { + raise("TypeError", "can't convert %s to Symbol (%s#to_sym gives %s)", + cls, cls, vm.classOf(conv).name) + } + category = sym } // Warning[category] filters the message (and raises for an unknown one). if !vm.send(vm.consts["Warning"], "[]", []object.Value{category}, nil).Truthy() { @@ -662,6 +670,41 @@ func (vm *VM) registerIO() { }) } +// warnUplevelPrefix is the "path:lineno: warning: " that Kernel#warn prepends +// when a uplevel: level is given — error.c v3_4_0 rb_warn_m, which 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); when that call yields no +// location (a level past the bottom of the stack) the prefix is the bare +// "warning: ". +// +// The `+ 1` in MRI skips the frame of the `warn` cfunc itself, so level 0 names +// the frame that CALLED warn. rbgo pushes no frame for a native method, so that +// frame is already the top of frameNames and the level indexes straight down +// from it: level 0 is len(frameNames)-1, level n is n frames further out. A +// level that walks off the bottom reports no location, which is what makes +// `warn "x", uplevel: 100` print "warning: x" rather than a fabricated frame. +// +// This used to be unconditionally "warning: ", because the parser's AST carried +// no source positions and no frame could supply a line. PR #645 built MRI's +// compressed insns_info/positions table on go-ruby-parser v0.3.0's +// ast.Program.Lines, so frameLine is now calc_lineno over the frame's +// (iseq, pc) pair and there IS a caller line to prepend. +func (vm *VM) warnUplevelPrefix(lev int64) string { + // len(frameNames) is an int, so a level that cannot even be held in one is + // past the bottom of any stack — checked before the conversion narrows it. + if lev > int64(len(vm.frameNames)) { + return "warning: " + } + i := len(vm.frameNames) - 1 - int(lev) + if i < 0 { + return "warning: " + } + // MRI formats the location unconditionally once it HAS one, so a frame whose + // line is unknown reports ":0:" rather than dropping to the bare prefix — the + // path is still the truth about where the caller is. + return vm.frameFileLabel(i) + ":" + strconv.Itoa(vm.frameLine(i)) + ": warning: " +} + // File open-mode flag constants (File::RDWR etc.). The values are the canonical // POSIX ones, fixed here so behaviour is identical on every OS the test gate runs // on rather than reflecting the host's . From 02efb01e00696873778c9e220137413ca8a4b697 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Thu, 24 Sep 2026 22:21:51 +0200 Subject: [PATCH 2/4] vm: Thread.each_caller_location MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 '
'"] 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 --- internal/vm/thread.go | 49 +++++++ internal/vm/wave29_lineharvest_test.go | 179 +++++++++++++++++++++++++ 2 files changed, 228 insertions(+) create mode 100644 internal/vm/wave29_lineharvest_test.go diff --git a/internal/vm/thread.go b/internal/vm/thread.go index dda5b2a5..0a81a91b 100644 --- a/internal/vm/thread.go +++ b/internal/vm/thread.go @@ -463,6 +463,55 @@ func (vm *VM) registerThreadClass() { } return object.NewArrayFromSlice(live) }) + // Thread.each_caller_location(start = 1, length = nil) { |loc| ... } -> nil: + // yields each frame of the CURRENT execution stack as a + // Thread::Backtrace::Location, nearest-first, and answers nil. + // + // vm_backtrace.c v3_4_0:1401 each_caller_location selects the range with + // ec_backtrace_range(ec, argc, argv, 1, 1, &n) — the SAME lev_default and + // lev_plus that rb_f_caller_locations passes to ec_backtrace_to_ary — so the + // argument handling is Kernel#caller_locations' exactly, down to the Range + // form and the nil-for-overshoot case. That is why callerSlice serves both: + // the two must not drift, and one of them owning the rule is how they cannot. + // + // It was unimplementable before the line map: every Location it yields is + // compared to caller_locations' by #to_s, which is "path:lineno:in 'label'". + sdef("each_caller_location", func(vm *VM, _ object.Value, args []object.Value, blk *Proc) object.Value { + // rb_scan_args(argc, argv, "02:") followed by rb_get_kwargs(opts, (ID[]){0}, + // 0, 0, NULL): the keyword table is EMPTY, so every keyword is unknown. + if kw := trailingKwHash(args); kw != nil { + args = args[:len(args)-1] + if len(kw.Keys) > 0 { + word, names := "keyword", make([]string, 0, len(kw.Keys)) + if len(kw.Keys) > 1 { + word = "keywords" + } + for _, k := range kw.Keys { + names = append(names, k.Inspect()) + } + raise("ArgumentError", "unknown %s: %s", word, strings.Join(names, ", ")) + } + } + if len(args) > 2 { + raise("ArgumentError", "wrong number of arguments (given %d, expected 0..2)", len(args)) + } + frames, present := vm.callerSlice(args) + if !present { + return object.NilV + } + // The LocalJumpError comes from the rb_yield inside the iteration, not from + // a guard before it: a range that selects NO frame never yields, so a + // block-less call over an empty selection is not an error. Hence the check + // is on "about to yield at least once", not on the block alone. + if blk == nil && len(frames) > 0 { + raise("LocalJumpError", "no block given") + } + for _, f := range frames { + vm.callBlock(blk, []object.Value{vm.backtraceLocation(f.ToS())}) + } + return object.NilV + }) + sdef("pass", func(vm *VM, _ object.Value, _ []object.Value, _ *Proc) object.Value { vm.threadPass(runtime.Gosched) vm.serviceSafepoint(vm.currentThread) // interrupt a running thread that yields via Thread.pass diff --git a/internal/vm/wave29_lineharvest_test.go b/internal/vm/wave29_lineharvest_test.go new file mode 100644 index 00000000..8ebfb736 --- /dev/null +++ b/internal/vm/wave29_lineharvest_test.go @@ -0,0 +1,179 @@ +package vm_test + +import ( + "strings" + "testing" +) + +// Kernel#warn's uplevel: level names a caller frame whose "path:lineno: " is +// prepended to the message — error.c v3_4_0 rb_warn_m, which takes the location +// from rb_ec_backtrace_location_ary(ec, lev + 1, 1, TRUE). Level 0 is the frame +// that called warn, and each further level walks one frame out. +func TestWarnUplevelPrefix(t *testing.T) { + // Level 0 names f's own frame: the line the warn call sits on (2). + got := eval(t, "$VERBOSE = true\ndef f; warn(\"x\", uplevel: 0); end\nf\n") + if !strings.HasSuffix(got, ":2: warning: x\n") { + t.Errorf("uplevel 0: got %q, want a ':2: warning: x' suffix", got) + } + // Level 1 names f's caller — the top-level frame, on line 3. + got = eval(t, "$VERBOSE = true\ndef f; warn(\"x\", uplevel: 1); end\nf\n") + if !strings.HasSuffix(got, ":3: warning: x\n") { + t.Errorf("uplevel 1: got %q, want a ':3: warning: x' suffix", got) + } + // One prefix for the whole call, not one per message: rb_io_puts appends every + // argument into the ONE buffer the prefix was written into. + got = eval(t, "$VERBOSE = true\ndef f; warn(\"a\", \"b\", uplevel: 0); end\nf\n") + if !strings.HasSuffix(got, ":2: warning: a\nb\n") { + t.Errorf("two messages: got %q", got) + } +} + +// A level past the bottom of the stack has no location, and rb_warn_m then +// writes the bare "warning: " — which is what keeps `warn "x", uplevel: 100` +// from naming a frame that is not there. +func TestWarnUplevelPastBottom(t *testing.T) { + if got := eval(t, "$VERBOSE = true\nwarn(\"x\", uplevel: 100)\n"); got != "warning: x\n" { + t.Errorf("uplevel 100: got %q, want %q", got, "warning: x\n") + } + // A level too large to hold in an int is past the bottom of any stack; it is + // rejected before the conversion to int could wrap it into a valid index. + if got := eval(t, "$VERBOSE = true\nwarn(\"x\", uplevel: 9223372036854775807)\n"); got != "warning: x\n" { + t.Errorf("uplevel maxint: got %q, want %q", got, "warning: x\n") + } +} + +// The uplevel level is converted ONCE. A #to_int that counts its calls must not +// see two, which is what a second toIntCoerce on the raise path used to cost. +func TestWarnUplevelConvertsLevelOnce(t *testing.T) { + src := "$VERBOSE = true\n$n = 0\no = Object.new\ndef o.to_int; $n += 1; 0; end\nwarn(\"x\", uplevel: o)\n$stdout.puts $n\n" + if got := eval(t, src); !strings.HasSuffix(got, "1\n") { + t.Errorf("to_int call count: got %q, want a trailing 1", got) + } +} + +// A negative level is rb_warn_m's ArgumentError, reported with the converted +// value rather than the object handed in. +func TestWarnUplevelNegative(t *testing.T) { + class, msg := evalErr(t, "$VERBOSE = true\nwarn(\"x\", uplevel: -2)\n") + if class != "ArgumentError" || msg != "negative level (-2)" { + t.Errorf("got %s: %q", class, msg) + } +} + +// rb_warn_m runs category: through rb_to_symbol_type, which is +// rb_convert_type_with_id(val, T_SYMBOL, "Symbol", idTo_sym): anything that +// answers #to_sym converts, and only a value that does not — or whose #to_sym +// gives a non-Symbol — is the TypeError. +func TestWarnCategoryToSym(t *testing.T) { + // :experimental is enabled by default, so the converted category reaches the + // filter and the message is written: the conversion is observable. + src := "$VERBOSE = true\no = Object.new\ndef o.to_sym; :experimental; end\nwarn(\"x\", category: o)\n" + if got := eval(t, src); got != "x\n" { + t.Errorf("to_sym category: got %q, want %q", got, "x\n") + } + // A String converts through its own String#to_sym — no case of its own. + if got := eval(t, "$VERBOSE = true\nwarn(\"x\", category: \"experimental\")\n"); got != "x\n" { + t.Errorf("string category: got %q, want %q", got, "x\n") + } + class, msg := evalErr(t, "$VERBOSE = true\nwarn(\"x\", category: Object.new)\n") + if class != "TypeError" || msg != "no implicit conversion of Object into Symbol" { + t.Errorf("no to_sym: got %s: %q", class, msg) + } + class, msg = evalErr(t, "$VERBOSE = true\no = Object.new\ndef o.to_sym; 5; end\nwarn(\"x\", category: o)\n") + if class != "TypeError" || msg != "can't convert Object to Symbol (Object#to_sym gives Integer)" { + t.Errorf("bad to_sym: got %s: %q", class, msg) + } +} + +// Thread.each_caller_location yields each frame of the current execution stack +// as a Thread::Backtrace::Location and answers nil — vm_backtrace.c v3_4_0:1401, +// whose ec_backtrace_range(ec, argc, argv, 1, 1, &n) call gives it exactly +// Kernel#caller_locations' argument handling. +func TestThreadEachCallerLocation(t *testing.T) { + // Both calls are made from the SAME frame and both default to level 1, so the + // two lists must agree entry for entry — which is the equality ruby/spec's + // core/thread/each_caller_location_spec.rb pins down. + src := `def probe + a = [] + Thread.each_caller_location { |l| a << l } + p a.map(&:to_s) == caller_locations.map(&:to_s) + p a[0].class +end +def mid; probe; end +mid +` + if got := eval(t, src); got != "true\nThread::Backtrace::Location\n" { + t.Errorf("got %q", got) + } + if got := eval(t, "p Thread.each_caller_location {}"); got != "nil\n" { + t.Errorf("return value: got %q, want %q", got, "nil\n") + } + // The start/length pair is caller_locations' too: (1, 1) is the caller's frame + // alone, from either spelling. + src = `def probe + x = nil + Thread.each_caller_location(1, 1) { |l| x = l } + p [x.to_s == caller_locations(1, 1)[0].to_s, x.to_s] +end +def mid; probe; end +mid +` + if got := eval(t, src); !strings.HasPrefix(got, "[true, ") { + t.Errorf("start/length: got %q", got) + } + // The Range form reaches callerSlice's Array#[] path. + src = `def probe + a = [] + Thread.each_caller_location(1..1) { |l| a << l } + p a.size +end +def mid; probe; end +mid +` + if got := eval(t, src); got != "1\n" { + t.Errorf("range form: got %q", got) + } +} + +// A start past the top of the stack selects nothing: no yield, no error, nil. +// That is also why a block-less call over an empty selection is NOT the +// LocalJumpError — the error comes from the rb_yield, which never happens. +func TestThreadEachCallerLocationOvershoot(t *testing.T) { + if got := eval(t, "p Thread.each_caller_location(100) { |l| p l }"); got != "nil\n" { + t.Errorf("overshoot with block: got %q", got) + } + if got := eval(t, "p Thread.each_caller_location(100)"); got != "nil\n" { + t.Errorf("overshoot without block: got %q", got) + } +} + +// Without a block the first yield is the LocalJumpError, message "no block +// given" (not the "(yield)" form a bare yield reports). +func TestThreadEachCallerLocationNoBlock(t *testing.T) { + class, msg := evalErr(t, "def top; Thread.each_caller_location; end\ntop\n") + if class != "LocalJumpError" || msg != "no block given" { + t.Errorf("got %s: %q", class, msg) + } +} + +// ec_backtrace_range's rb_scan_args(argc, argv, "02:") caps the positionals at +// two, and its rb_get_kwargs runs against an EMPTY keyword table — so every +// keyword is an unknown one. +func TestThreadEachCallerLocationArgErrors(t *testing.T) { + class, msg := evalErr(t, "Thread.each_caller_location(12, foo: 10) {}") + if class != "ArgumentError" || msg != "unknown keyword: :foo" { + t.Errorf("one keyword: got %s: %q", class, msg) + } + class, msg = evalErr(t, "Thread.each_caller_location(foo: 1, bar: 2) {}") + if class != "ArgumentError" || msg != "unknown keywords: :foo, :bar" { + t.Errorf("two keywords: got %s: %q", class, msg) + } + class, msg = evalErr(t, "Thread.each_caller_location(1, 2, 3) {}") + if class != "ArgumentError" || msg != "wrong number of arguments (given 3, expected 0..2)" { + t.Errorf("arity: got %s: %q", class, msg) + } + class, msg = evalErr(t, "Thread.each_caller_location(-1) {}") + if class != "ArgumentError" || msg != "negative level (-1)" { + t.Errorf("negative level: got %s: %q", class, msg) + } +} From a4d844c918033d999f628c76be5afb86be709214 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Fri, 25 Sep 2026 07:43:17 +0200 Subject: [PATCH 3/4] vm: qualify every backtrace label with its owner (rb_gen_method_name) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: "
", "", "" 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 --- internal/vm/backtrace_test.go | 17 +-- internal/vm/builtins.go | 11 ++ internal/vm/exception.go | 51 ++++++-- internal/vm/kernel_introspect.go | 137 +++++++++++++++++++++- internal/vm/kernel_warn_internal_test.go | 16 +-- internal/vm/wave28_lines_internal_test.go | 57 +++++++-- 6 files changed, 245 insertions(+), 44 deletions(-) diff --git a/internal/vm/backtrace_test.go b/internal/vm/backtrace_test.go index 389f7a66..e691f62a 100644 --- a/internal/vm/backtrace_test.go +++ b/internal/vm/backtrace_test.go @@ -63,8 +63,8 @@ end // exact source, written to a file and run under ruby 4.0.5: the raise at 3, // the call to foo at 6, the call to bar at 9. want := strings.Join([]string{ - script + ":3:in 'foo'", - script + ":6:in 'bar'", + script + ":3:in 'Object#foo'", + script + ":6:in 'Object#bar'", script + ":9:in '
'", }, "\n") + "\n" if out != want { @@ -123,8 +123,9 @@ end if err != nil { t.Fatal(err) } - // ruby 4.0.5 on this source reports the raise inside deep at line 3. - if out != "/x/p.rb:3:in 'deep'\n" { + // ruby 4.0.5 on this source reports the raise inside deep at line 3, under + // the owner-qualified label rb_gen_method_name builds: 'Object#deep'. + if out != "/x/p.rb:3:in 'Object#deep'\n" { t.Fatalf("got %q", out) } } @@ -201,7 +202,7 @@ end t.Fatal(err) } // ruby 4.0.5 on this source: the raise inside foo at 3, the call at 6. - want := "/x/p.rb:3:in 'foo': boom (ArgumentError)\n" + + want := "/x/p.rb:3:in 'Object#foo': boom (ArgumentError)\n" + "\tfrom /x/p.rb:6:in '
'\n" if out != want { t.Fatalf("\n got %q\nwant %q", out, want) @@ -259,7 +260,7 @@ boom } bt := re.Backtrace() // ruby 4.0.5 on this source: the raise inside boom at 3, the call at 5. - want := []string{"/x/p.rb:3:in 'boom'", "/x/p.rb:5:in '
'"} + want := []string{"/x/p.rb:3:in 'Object#boom'", "/x/p.rb:5:in '
'"} if strings.Join(bt, "|") != strings.Join(want, "|") { t.Fatalf("Backtrace()\n got %v\nwant %v", bt, want) } @@ -297,8 +298,8 @@ a if err != nil { t.Fatal(err) } - // ruby 4.0.5 on this source reports a's call to b at line 3. - if out != "/x/p.rb:3:in 'a'\n" { + // ruby 4.0.5 on this source reports a's call to b at line 3, owner-qualified. + if out != "/x/p.rb:3:in 'Object#a'\n" { t.Fatalf("got %q", out) } } diff --git a/internal/vm/builtins.go b/internal/vm/builtins.go index de8e3224..f0c55f0e 100644 --- a/internal/vm/builtins.go +++ b/internal/vm/builtins.go @@ -364,6 +364,15 @@ func (vm *VM) bootstrap() { fileStackDepth := len(vm.fileStack) frameNamesDepth := len(vm.frameNames) frameFilesDepth := len(vm.frameFiles) + // frameCrefs and frameMethods mirror frameNames one-for-one and have to be + // restored WITH it. Restoring only frameNames left the three at different + // depths for the rest of the program, so frameCrefs[i] and frameMethods[i] + // stopped describing frame i. Nothing noticed while a backtrace read + // frameNames alone; frameLabel now reads all three — the owner prefix comes + // from the cref and an aliased method's original name from frameMethods — + // and one leaked catch made every later label some other frame's. + frameCrefsDepth := len(vm.frameCrefs) + frameMethodsDepth := len(vm.frameMethods) requireDirsDepth := len(vm.requireDirs) // Register this catch's tag so a Kernel#throw can tell a matched throw (an // unwind to here) from an unmatched one (an UncaughtThrowError). The depth @@ -380,6 +389,8 @@ func (vm *VM) bootstrap() { vm.fileStack = vm.fileStack[:fileStackDepth] vm.frameNames = vm.frameNames[:frameNamesDepth] vm.frameFiles = vm.frameFiles[:frameFilesDepth] + vm.frameCrefs = vm.frameCrefs[:frameCrefsDepth] + vm.frameMethods = vm.frameMethods[:frameMethodsDepth] vm.requireDirs = vm.requireDirs[:requireDirsDepth] result = sig.value return diff --git a/internal/vm/exception.go b/internal/vm/exception.go index 62b440b1..8f6fe3fe 100644 --- a/internal/vm/exception.go +++ b/internal/vm/exception.go @@ -391,30 +391,55 @@ func (vm *VM) registerBacktraceLocation() { loc.define("absolute_path", func(_ *VM, self object.Value, _ []object.Value, _ *Proc) object.Value { return getIvar(self, "@path") }) - // #base_label is the label WITHOUT the block qualifier. MRI reads it from a + // #base_label is the label with NO decoration at all. MRI reads it from a // different field than #label does — location.base_label beside - // location.label (vm_backtrace.c v3_4_0:331) — and the difference is exactly - // the decoration calculate_iseq_label adds for a block frame: #label says - // "block (2 levels) in foo" where #base_label says "foo". Stripping that - // prefix reconstructs the same answer from the one label we carry. + // location.label (vm_backtrace.c v3_4_0:331) — and that field is the plain + // name the ISeq was compiled under, so it carries neither the "block in " + // that calculate_iseq_label adds for a block frame nor the "Owner#" that + // rb_gen_method_name adds for a method one. #label says + // "block (2 levels) in C#foo" where #base_label says "foo". Removing both + // decorations reconstructs the same answer from the one label we carry. loc.define("base_label", func(_ *VM, self object.Value, _ []object.Value, _ *Proc) object.Value { - return object.NewString(stripBlockQualifier(getIvar(self, "@label").ToS())) + _, base := splitBlockQualifier(getIvar(self, "@label").ToS()) + return object.NewString(stripOwnerPrefix(base)) }) } -// stripBlockQualifier removes the "block in " / "block (N levels) in " prefix a -// block frame's label carries, yielding the label of the scope the block was -// written in. A label without the prefix is returned unchanged. -func stripBlockQualifier(label string) string { +// splitBlockQualifier splits the "block in " / "block (N levels) in " decoration +// a block frame's label carries from the label of the scope the block was +// written in. A label without the decoration comes back with an empty qualifier. +func splitBlockQualifier(label string) (qual, base string) { if rest, ok := strings.CutPrefix(label, "block in "); ok { - return rest + return "block in ", rest } if strings.HasPrefix(label, "block (") { if i := strings.Index(label, " levels) in "); i >= 0 { - return label[i+len(" levels) in "):] + cut := i + len(" levels) in ") + return label[:cut], label[cut:] } } - return label + return "", label +} + +// stripOwnerPrefix removes the "Owner#" / "Owner." that genMethodName puts in +// front of a method label, yielding the bare name MRI keeps in +// location.base_label. +// +// It removes a prefix only when what precedes the separator is a CONSTANT PATH, +// which is the only shape rb_mod_name0 can produce: a run of "::"-joined +// segments each starting with an upper-case letter. That test is what keeps it +// from eating part of a label that merely contains the separator — a method name +// cannot hold a "#" or a ".", but "
" and the other bracketed labels reach +// here too, and an undecorated label must come back untouched. +func stripOwnerPrefix(label string) string { + cut := strings.LastIndexAny(label, "#.") + if cut <= 0 || cut == len(label)-1 { + return label + } + if !isConstantPath(label[:cut]) { + return label + } + return label[cut+1:] } // backtraceLocation builds a Thread::Backtrace::Location from one captured diff --git a/internal/vm/kernel_introspect.go b/internal/vm/kernel_introspect.go index c73e7524..8da0ea2b 100644 --- a/internal/vm/kernel_introspect.go +++ b/internal/vm/kernel_introspect.go @@ -478,20 +478,151 @@ func (vm *VM) backtraceFrames(skip int) []object.Value { // by another name, and reports "
". func (vm *VM) frameLabel(i int) string { if name := vm.frameNames[i]; name != "" { - return name + // MRI's label for a method frame is the ISeq's location.label — the name + // the body was DEFINED under — so an aliased method reports the ORIGINAL + // name, not the alias it was reached through. frameNames carries the + // called name (__callee__); frameMethods carries both, and orig is the + // half MRI prints. + if vm.frameStacksAligned() && vm.frameMethods[i].orig != "" { + name = vm.frameMethods[i].orig + } + return vm.qualifiedFrameLabel(i, "", name) } if i < len(vm.frameCode) && vm.frameCode[i].iseq != nil { switch n := vm.frameCode[i].iseq.Name; { case n == "": return "singleton class" - case strings.HasPrefix(n, "" } +// qualifiedFrameLabel puts MRI's owner prefix in front of a frame's method name +// and re-attaches the block decoration qual in front of the result. +// +// This is rb_gen_method_name (vm_backtrace.c v3_4_0:205) reached through +// calculate_iseq_label (:229), which every backtrace label goes through: +// location_label (:285) and location_to_str (:460) both call it, so the prefix +// is on Thread::Backtrace::Location#label, on Kernel#caller and on the lines of +// an exception's backtrace alike. `def foo` at the top level does not report +// "foo", it reports "Object#foo". +// +// Only a METHOD label takes a prefix. A class or module body (""), the +// top level ("
"), a required file's top level and a singleton-class body +// are ISEQ_TYPE_CLASS/TOP in MRI, whose branch of calculate_iseq_label returns +// location.label untouched — so a base that is not a method name is returned as +// it came, with its decoration. +func (vm *VM) qualifiedFrameLabel(i int, qual, base string) string { + if base == "" || strings.HasPrefix(base, "<") { + return qual + base + } + owner, singleton, ok := vm.frameOwner(i, base) + if !ok { + return qual + base + } + return qual + genMethodName(owner, singleton, base) +} + +// frameOwner works out the module a frame's method is defined on, and whether it +// is a singleton (class) method, so qualifiedFrameLabel can pick between the "#" +// and "." forms. It reports ok == false when the frame has no owner to name, in +// which case the label keeps the bare method name — which is also MRI's answer +// whenever rb_gen_method_name cannot resolve one. +// +// MRI reads the owner off the frame's callable method entry (loc->cme->owner). +// rbgo records no cme per frame; what it records is frameCrefs[i], the lexical +// scope the frame runs under, which for a `def` IS the class the method landed +// on — exec is handed m.owner as its definee and stores that as the cref. The +// two part company only for a method defined under class_eval / module_exec, +// where the def target and the textual nesting differ and the cref follows the +// nesting; such a frame finds no matching entry below and keeps its bare name +// rather than claiming a wrong owner. +// +// The `def self.foo` case needs the second return value rather than +// owner.isSingleton: rbgo files a class method under the CLASS (C.smethods), +// with owner == C, where MRI files it under C's singleton class. Which of the +// two tables holds the name is therefore what says "#" from "." — and looking +// it up is also what makes an aliased or module_function'd name resolve, since +// both tables are keyed by the name the label already carries. +func (vm *VM) frameOwner(i int, base string) (*RClass, bool, bool) { + if !vm.frameStacksAligned() { + return nil, false, false + } + cref := vm.frameCrefs[i] + if cref == nil { + return nil, false, false + } + // A `class << X` body's definee IS the singleton class, which is the shape + // MRI always has. rb_gen_method_name unwraps it to the attached object and + // uses the "." form, but ONLY when that object is itself a class or module: + // a method on a plain object's singleton (`def obj.foo`) has no owner to + // name and reports the bare name. + if cref.isSingleton { + if t := cref.metaOf; t != nil { + return t, true, true + } + if t, isClass := cref.attached.(*RClass); isClass { + return t, true, true + } + return nil, false, false + } + // The instance table is consulted first because the singleton entry can be a + // COPY of it: module_function files one Method in both, so a name present in + // both is a module function, whose two call paths rbgo cannot tell apart from + // the frame alone (MRI distinguishes them by the cme the call resolved to). + // The instance form is the one the definition itself created. + if _, ok := cref.methods[base]; ok { + return cref, false, true + } + if _, ok := cref.smethods[base]; ok { + return cref, true, true + } + return nil, false, false +} + +// frameStacksAligned reports whether frameCrefs and frameMethods still mirror +// frameNames one-for-one, which is what makes index i mean the same frame in all +// three. exec pushes and pops them together, and every unwind that restores one +// restores the others — except at three sites outside this wave's files that +// truncate frameNames ALONE (internal/vm/find.go, irb_bind.go and pstore.go, +// each a catch/throw-style depth restore; the fourth, Kernel#catch in this file, +// is fixed here). A frame index is not meaningful across such a drift, so the +// label falls back to the bare method name rather than reporting SOME OTHER +// frame's owner: losing the prefix is a visible, honest degradation where +// printing a wrong owner would not be. +func (vm *VM) frameStacksAligned() bool { + return len(vm.frameCrefs) == len(vm.frameNames) && len(vm.frameMethods) == len(vm.frameNames) +} + +// genMethodName is rb_gen_method_name (vm_backtrace.c v3_4_0:205): a method +// label is qualified by its owner's name — "Owner#name" for an instance method, +// "Owner.name" for a singleton one — but ONLY when that name is permanent. +// +// Permanence is not a detail. MRI takes the name through rb_mod_name0, which +// reports classname() together with RCLASS_EXT(klass)->permanent_classpath +// (variable.c v3_4_0:104), and drops the prefix entirely when the path is +// missing or not permanent. An anonymous class therefore labels its methods +// with the bare name rather than with a name that would change the moment the +// class were bound to a constant. rbgo's RClass.named is that flag. +func genMethodName(owner *RClass, singleton bool, name string) string { + if owner == nil || !owner.named || owner.name == "" { + return name + } + if singleton { + return owner.name + "." + name + } + return owner.name + "#" + name +} + // formatBacktraceEntry renders one backtrace line. // // It is MRI's location_format (vm_backtrace.c v3_4_0:446), including the part diff --git a/internal/vm/kernel_warn_internal_test.go b/internal/vm/kernel_warn_internal_test.go index 077e675b..0bd8613e 100644 --- a/internal/vm/kernel_warn_internal_test.go +++ b/internal/vm/kernel_warn_internal_test.go @@ -35,11 +35,12 @@ func TestKernelWarn(t *testing.T) { {`begin; warn("x", uplevel: "a"); rescue TypeError => e; p e.message; end`, `"no implicit conversion of String into Integer"`}, // With a level, error.c v3_4_0 rb_warn_m prefixes the message — // "path:lineno: warning: " from rb_ec_backtrace_location_ary, or the bare - // "warning: " when that yields no location. rbgo records no line numbers, - // so no frame can supply a path and the no-location form is what every - // level produces; MRI would say ":: warning: x\n" here. Closing - // that gap needs line tracking in internal/compiler. - {cap + `warn("x", uplevel: 0); p $c`, `["warning: x\n", nil]`}, + // "warning: " when that yields no location. The location is now real: the + // frame that called warn, at the line it has reached. These cases run from + // a source with no file, so frameFileLabel's last fallback names the path + // "(rbgo)"; run from a file, MRI and rbgo agree byte for byte (checked on + // ruby 4.0.5, which prints ":2: warning: x" for the same shape). + {cap + `warn("x", uplevel: 0); p $c`, `["(rbgo):1: warning: x\n", nil]`}, // A level too large for the stack is exactly the no-location case, and // there rbgo and MRI agree byte for byte. {cap + `warn("x", uplevel: 100); p $c`, `["warning: x\n", nil]`}, @@ -55,8 +56,9 @@ func TestKernelWarn(t *testing.T) { {cap + `warn("x\n"); p $c`, `["x\n", nil]`}, {cap + `warn("a\n", "b"); p $c`, `["a\nb\n", nil]`}, {cap + `warn([]); p $c`, `nil`}, - // NUM2LONG takes the level, so a Float or Rational truncates. - {cap + `warn("x", uplevel: 0.9); p $c`, `["warning: x\n", nil]`}, + // NUM2LONG takes the level, so a Float or Rational truncates: 0.9 is the + // level-0 frame, not level 1 and not a TypeError. + {cap + `warn("x", uplevel: 0.9); p $c`, `["(rbgo):1: warning: x\n", nil]`}, // A Warning.warn that takes exactly one argument is called without the // category keyword (rb_warn_category's rb_warning_warn_arity() == 1). {`$c = nil; Warning.singleton_class.send(:define_method, :warn) { |m| $c = m }; warn("x"); p $c`, `"x\n"`}, diff --git a/internal/vm/wave28_lines_internal_test.go b/internal/vm/wave28_lines_internal_test.go index cc7b92e7..f0022be9 100644 --- a/internal/vm/wave28_lines_internal_test.go +++ b/internal/vm/wave28_lines_internal_test.go @@ -116,22 +116,53 @@ func TestFrameLabelFromISeq(t *testing.T) { } } -// TestStripBlockQualifier covers the three shapes base_label has to undo. MRI -// keeps label and base_label in separate fields (vm_backtrace.c v3_4_0:331); -// the difference between them is exactly this decoration. -func TestStripBlockQualifier(t *testing.T) { - cases := map[string]string{ - "block in foo": "foo", - "block (2 levels) in foo": "foo", - "block (17 levels) in C#m": "C#m", - "foo": "foo", - "": "", +// TestSplitBlockQualifier covers the three shapes the block decoration takes. +// MRI keeps label and base_label in separate fields (vm_backtrace.c +// v3_4_0:331); this decoration is one half of the difference between them. +func TestSplitBlockQualifier(t *testing.T) { + cases := map[string][2]string{ + "block in foo": {"block in ", "foo"}, + "block (2 levels) in foo": {"block (2 levels) in ", "foo"}, + "block (17 levels) in C#m": {"block (17 levels) in ", "C#m"}, + "foo": {"", "foo"}, + "": {"", ""}, // "block (" without the levels marker is not a qualifier we made. - "block (weird": "block (weird", + "block (weird": {"", "block (weird"}, + } + for in, want := range cases { + qual, base := splitBlockQualifier(in) + if qual != want[0] || base != want[1] { + t.Errorf("splitBlockQualifier(%q) = %q,%q, want %q,%q", in, qual, base, want[0], want[1]) + } + } +} + +// TestStripOwnerPrefix covers the OTHER half of the difference: the "Owner#" / +// "Owner." that rb_gen_method_name puts in front of a method label and that +// base_label does not carry. The prefix comes off only when what precedes the +// separator is a constant path, the one shape rb_mod_name0 can produce. +func TestStripOwnerPrefix(t *testing.T) { + cases := map[string]string{ + "C#m": "m", + "C.m": "m", + "Outer::Inner#deep": "deep", + "ThreadBacktraceSpecs.x": "x", + "C#<=>": "<=>", + // No owner: unchanged. + "foo": "foo", + "
": "
", + "": "", + // A separator whose left side is not a constant path is not an owner — + // which is how `main.label_sdef_method_of_main` keeps its whole label. + "main.label_sdef_method": "main.label_sdef_method", + "a::B#m": "a::B#m", + // Degenerate placements: nothing before the separator, nothing after. + "#m": "#m", + "C#": "C#", } for in, want := range cases { - if got := stripBlockQualifier(in); got != want { - t.Errorf("stripBlockQualifier(%q) = %q, want %q", in, got, want) + if got := stripOwnerPrefix(in); got != want { + t.Errorf("stripOwnerPrefix(%q) = %q, want %q", in, got, want) } } } From 30f08ba6822972815cb804d3c325a8fb4d9b6da6 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Fri, 25 Sep 2026 08:06:13 +0200 Subject: [PATCH 4/4] vm: cover the owner-prefix branches, and drop one that cannot be reached MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- internal/vm/kernel_introspect.go | 15 ++-- internal/vm/wave29_lineharvest_test.go | 6 ++ internal/vm/wave29_owner_internal_test.go | 104 ++++++++++++++++++++++ 3 files changed, 119 insertions(+), 6 deletions(-) create mode 100644 internal/vm/wave29_owner_internal_test.go diff --git a/internal/vm/kernel_introspect.go b/internal/vm/kernel_introspect.go index 8da0ea2b..685a9930 100644 --- a/internal/vm/kernel_introspect.go +++ b/internal/vm/kernel_introspect.go @@ -563,16 +563,19 @@ func (vm *VM) frameOwner(i int, base string) (*RClass, bool, bool) { } // A `class << X` body's definee IS the singleton class, which is the shape // MRI always has. rb_gen_method_name unwraps it to the attached object and - // uses the "." form, but ONLY when that object is itself a class or module: - // a method on a plain object's singleton (`def obj.foo`) has no owner to - // name and reports the bare name. + // uses the "." form, but ONLY when that object is itself a class or module. + // + // In rbgo that condition is exactly metaOf. A class's or module's singleton + // records its subject there; `attached` holds the object of a PER-OBJECT + // singleton, and ensureSingleton refuses to make one of those for a class + // (`case *RClass: return nil, false` — classes use metaClass() instead), so + // attached is never a class or module and a second branch testing for one + // would be unreachable. A per-object singleton has no owner to name and its + // methods report the bare name — rb_gen_method_name's own fallthrough. if cref.isSingleton { if t := cref.metaOf; t != nil { return t, true, true } - if t, isClass := cref.attached.(*RClass); isClass { - return t, true, true - } return nil, false, false } // The instance table is consulted first because the singleton entry can be a diff --git a/internal/vm/wave29_lineharvest_test.go b/internal/vm/wave29_lineharvest_test.go index 8ebfb736..e5feadc2 100644 --- a/internal/vm/wave29_lineharvest_test.go +++ b/internal/vm/wave29_lineharvest_test.go @@ -40,6 +40,12 @@ func TestWarnUplevelPastBottom(t *testing.T) { if got := eval(t, "$VERBOSE = true\nwarn(\"x\", uplevel: 9223372036854775807)\n"); got != "warning: x\n" { t.Errorf("uplevel maxint: got %q, want %q", got, "warning: x\n") } + // The level EXACTLY one past the outermost frame: at the top level there is + // one frame, so uplevel 1 already has no location. ruby 4.0.5 on the same two + // lines prints "warning: x" too. + if got := eval(t, "$VERBOSE = true\nwarn(\"x\", uplevel: 1)\n"); got != "warning: x\n" { + t.Errorf("uplevel one past the top: got %q, want %q", got, "warning: x\n") + } } // The uplevel level is converted ONCE. A #to_int that counts its calls must not diff --git a/internal/vm/wave29_owner_internal_test.go b/internal/vm/wave29_owner_internal_test.go new file mode 100644 index 00000000..a7eaa48f --- /dev/null +++ b/internal/vm/wave29_owner_internal_test.go @@ -0,0 +1,104 @@ +package vm + +import "testing" + +// frameOwner reads frameCrefs[i], which exec pushes as nil and fills in once +// the frame's lexical scope is settled. A backtrace taken while that entry is +// still nil must report the bare name rather than dereference it, so the guard +// is exercised here against a stack built by hand — the state itself is +// transient inside one exec call and cannot be held still from Ruby. +func TestFrameOwnerNilCref(t *testing.T) { + vm := New(nil) + vm.frameNames = []string{"foo"} + vm.frameCrefs = []*RClass{nil} + vm.frameMethods = []frameMethod{{orig: "foo", callee: "foo"}} + if owner, singleton, ok := vm.frameOwner(0, "foo"); ok { + t.Errorf("nil cref: got (%v, %v, %v), want no owner", owner, singleton, ok) + } + if got := vm.frameLabel(0); got != "foo" { + t.Errorf("frameLabel with nil cref = %q, want %q", got, "foo") + } +} + +// frameCrefs and frameMethods mirror frameNames one-for-one, and a frame index +// means the same frame in all three only while they are the same length. Three +// unwind sites outside this package's ownership truncate frameNames alone +// (internal/vm/find.go, irb_bind.go, pstore.go), so a drift is reachable; when +// it happens the label must LOSE the owner prefix rather than report some other +// frame's owner. +func TestFrameStacksAlignedGuard(t *testing.T) { + vm := New(nil) + k := newClass("K", vm.cObject) + vm.frameNames = []string{"foo"} + vm.frameCrefs = []*RClass{k} + vm.frameMethods = []frameMethod{{orig: "foo", callee: "foo"}} + k.methods["foo"] = &Method{name: "foo", owner: k} + + if !vm.frameStacksAligned() { + t.Fatal("stacks should start aligned") + } + if got := vm.frameLabel(0); got != "K#foo" { + t.Fatalf("aligned: frameLabel = %q, want %q", got, "K#foo") + } + + // A leaked cref entry (frameNames truncated alone) is the drift shape. + vm.frameCrefs = append(vm.frameCrefs, k) + if vm.frameStacksAligned() { + t.Fatal("drifted frameCrefs still reported aligned") + } + if got := vm.frameLabel(0); got != "foo" { + t.Errorf("drifted: frameLabel = %q, want the bare %q", got, "foo") + } + + // The same for frameMethods on its own. + vm.frameCrefs = vm.frameCrefs[:1] + vm.frameMethods = append(vm.frameMethods, frameMethod{}) + if vm.frameStacksAligned() { + t.Fatal("drifted frameMethods still reported aligned") + } + if got := vm.frameLabel(0); got != "foo" { + t.Errorf("drifted: frameLabel = %q, want the bare %q", got, "foo") + } +} + +// genMethodName drops the prefix unless the owner's class path is PERMANENT — +// MRI takes it through rb_mod_name0, which reports permanent_classpath beside +// the path and returns the bare name when it is missing or temporary +// (variable.c v3_4_0:104). +func TestGenMethodNamePermanence(t *testing.T) { + named := newClass("K", nil) + named.named = true + anon := newClass("", nil) + // A class carrying a name it has not been permanently bound to — the + // Module#set_temporary_name shape, which leaves name set and 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". + temp := newClass("Temp", nil) + temp.named = false + + cases := []struct { + owner *RClass + singleton bool + want string + }{ + {named, false, "K#m"}, + {named, true, "K.m"}, + {anon, false, "m"}, + {anon, true, "m"}, + {temp, false, "m"}, + {nil, false, "m"}, + {nil, true, "m"}, + } + for _, c := range cases { + if got := genMethodName(c.owner, c.singleton, "m"); got != c.want { + t.Errorf("genMethodName(%v, %v, \"m\") = %q, want %q", c.owner, c.singleton, got, c.want) + } + } + // A permanently-bound class whose name is somehow empty still takes no + // prefix: an empty owner name would render "#m". + empty := newClass("", nil) + empty.named = true + if got := genMethodName(empty, false, "m"); got != "m" { + t.Errorf("empty permanent name: got %q, want %q", got, "m") + } +}