From 4347a9245f53ca16a288a681d56eca91568e4d02 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Fri, 9 Oct 2026 09:35:03 +0200 Subject: [PATCH 1/3] internal/vm: a live instance variable is never hidden by the order list A YAML-loaded object answered `o.a == 1` and `instance_variables == []`. The ivars were there and readable by name; only the enumeration could not see them, which is the worst version of this bug -- every direct check passes, and anything that WALKS the object (marshalling, inspect, a serialiser, an equality written over instance_variables) silently sees an empty object. Cause: for an *RObject, ivarStoreOf returns `order: &o.ivarOrder`, the ADDRESS of a struct field, so the order pointer is never nil and ivarNamesInOrder always takes the ordered branch. A constructor that fills the ivars map directly and leaves ivarOrder empty therefore produces ivars that read back and do not enumerate, and nothing anywhere says so. Two changes, because one of them is a floor and the other is the fix: yaml_bind.go convObject now fills ivarOrder alongside ivars, in the sequence the yaml package documents for emission (value.go:86-89: the names in o.Order that exist, then the rest lexicographically). That is the document's order, so instance_variables matches MRI -- measured [:@z, :@a, :@m] for a document written z, a, m, which is neither alphabetical nor reversed, and stable over three runs where Go's map order is randomised per run. ivarNamesInOrder now also reports live names missing from the order list, sorted. Constructors that fill ivarOrder never reach that path, so creation order is preserved where it is known; what it buys is that the NEXT constructor to fill a map directly is wrong visibly rather than silently. yamlIVarOrder already existed in the same file, eleven lines from where I wrote a second copy of it; the duplicate is not in this commit. Closes #787. Co-Authored-By: Claude Opus 5 --- internal/vm/ivar_order_test.go | 67 ++++++++++++++++++++++++++++++++++ internal/vm/object_model.go | 24 ++++++++++++ internal/vm/yaml_bind.go | 18 ++++++++- 3 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 internal/vm/ivar_order_test.go diff --git a/internal/vm/ivar_order_test.go b/internal/vm/ivar_order_test.go new file mode 100644 index 00000000..0980b2a8 --- /dev/null +++ b/internal/vm/ivar_order_test.go @@ -0,0 +1,67 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm_test + +import "testing" + +// TestYAMLBuiltIvarsAreEnumerable (#787). +// +// A YAML-loaded object answered `o.a == 1` and `instance_variables == []`. The +// ivars were there and readable by name; only the enumeration could not see +// them, which is the worst version of the bug: every direct check passes and +// anything that WALKS the object -- marshalling, inspect, a serialiser, an +// equality written over instance_variables -- silently sees an empty object. +// +// Cause: for an *RObject the order list is consulted FIRST, and the pointer to +// it is the address of a struct field, so it is never nil. A constructor that +// fills the ivars map directly and leaves ivarOrder empty therefore produces +// ivars that read back and do not enumerate, with nothing to indicate it. +func TestYAMLBuiltIvarsAreEnumerable(t *testing.T) { + checkCases(t, []runCase{ + {`require "yaml" +class P; attr_accessor :a, :b; end +o = YAML.safe_load("--- !ruby/object:P\na: 1\nb: 2\n", permitted_classes: [P]) +p o.instance_variables`, "[:@a, :@b]\n"}, + + // The order is the DOCUMENT's, not the map's: z, a, m is neither + // alphabetical nor reversed, so a run that reproduced it by accident is + // unlikely, and Go's map iteration is randomised per run. + {`require "yaml" +class Q; attr_accessor :z, :a, :m; end +o = YAML.safe_load("--- !ruby/object:Q\nz: 1\na: 2\nm: 3\n", permitted_classes: [Q]) +p o.instance_variables`, "[:@z, :@a, :@m]\n"}, + + // Reading was never broken, and must stay unbroken -- a fix that rebuilt + // the object could satisfy the enumeration and lose the values. + {`require "yaml" +class R; attr_accessor :a, :b; end +o = YAML.safe_load("--- !ruby/object:R\na: 1\nb: 2\n", permitted_classes: [R]) +p [o.a, o.b, o.instance_variable_get(:@a)]`, "[1, 2, 1]\n"}, + }) +} + +// TestALiveIvarIsNeverHiddenByTheOrderList is the floor under the above: the +// enumeration reports a live ivar even when the order list does not mention it, +// so the next constructor that fills the map directly is wrong in a VISIBLE way +// rather than a silent one. +// +// It is exercised through Marshal, whose loader is a second object builder, and +// through an ordinary object to show the common path still reports creation +// order rather than the sorted fallback. +func TestALiveIvarIsNeverHiddenByTheOrderList(t *testing.T) { + checkCases(t, []runCase{ + // Creation order, not sorted: b before a. + {`class S; def initialize; @b = 1; @a = 2; end; end +p S.new.instance_variables`, "[:@b, :@a]\n"}, + // remove_instance_variable still drops the name. + {`class T; def initialize; @x = 1; @y = 2; end + def drop; remove_instance_variable(:@x); end +end +t = T.new; t.drop; p t.instance_variables`, "[:@y]\n"}, + {`class U; def initialize; @a = 1; @b = 2; end; end +u = Marshal.load(Marshal.dump(U.new)) +p u.instance_variables`, "[:@a, :@b]\n"}, + }) +} diff --git a/internal/vm/object_model.go b/internal/vm/object_model.go index e486495f..e5c2f5bb 100644 --- a/internal/vm/object_model.go +++ b/internal/vm/object_model.go @@ -4,6 +4,7 @@ import ( "fmt" "reflect" "runtime" + "sort" "strings" "sync" @@ -2776,13 +2777,36 @@ func ivarNamesInOrder(self object.Value) []object.Value { st := ivarStoreOf(self, false) if st.order != nil { out := make([]object.Value, 0, len(*st.order)) + seen := make(map[string]bool, len(*st.order)) for _, n := range *st.order { // A remove_instance_variable leaves the name in the order list; the map // says which names are still live. if _, live := st.tbl[n]; live { out = append(out, object.Symbol(n)) + seen[n] = true } } + // A LIVE ivar missing from the order list is still an ivar. For an + // *RObject the order pointer is the address of a struct field, so it is + // NEVER nil -- which means a constructor that fills the map directly and + // leaves ivarOrder empty produces ivars that are readable by name and + // invisible to instance_variables, with nothing to say so. That is #787: + // a YAML-built object answered `o.a == 1` and `instance_variables == []`. + // + // This is a floor rather than the fix for any one constructor: the ones + // that build ivarOrder (exception.go) never reach it, so MRI's creation + // order is preserved where it is known. Sorted, because Go's map order is + // random and an enumeration that changes between runs is its own defect. + var extra []string + for n := range st.tbl { + if !seen[n] { + extra = append(extra, n) + } + } + sort.Strings(extra) + for _, n := range extra { + out = append(out, object.Symbol(n)) + } return out } out := make([]object.Value, 0, len(st.tbl)) diff --git a/internal/vm/yaml_bind.go b/internal/vm/yaml_bind.go index 02232ae2..af883756 100644 --- a/internal/vm/yaml_bind.go +++ b/internal/vm/yaml_bind.go @@ -557,8 +557,24 @@ func (c *yamlFromCtx) convObject(o *yaml.Object) object.Value { cls := c.vm.yamlResolveClass(name) obj := &RObject{class: cls, ivars: map[string]object.Value{}} c.seen[o] = obj - for k, val := range o.IVars { + // ivarOrder has to be filled alongside ivars, not left to the map. An + // *RObject's order list is consulted FIRST by instance_variables, and the + // pointer to it is never nil, so an object built with a populated map and an + // empty order list has ivars that read back by name and do not appear in the + // enumeration -- #787, where a YAML-loaded object answered `o.a == 1` and + // `instance_variables == []`. + // + // The sequence is the library's own emission order (value.go:86-89): the + // names in o.Order that exist, then the rest lexicographically. Taking it + // from there rather than ranging the map keeps the result both MRI-like + // (document order) and stable across runs. + for _, k := range yamlIVarOrder(o) { + val, ok := o.IVars[k] + if !ok { + continue + } obj.ivars["@"+k] = c.conv(val) + obj.ivarOrder = append(obj.ivarOrder, "@"+k) } return obj } From 0386cc4177a511f6bca913f8a9fadb5058d918cb Mon Sep 17 00:00:00 2001 From: tannevaled Date: Fri, 9 Oct 2026 15:10:36 +0200 Subject: [PATCH 2/3] internal/vm: cover the floor directly, and drop the dead arm the gate found Two answers to the coverage gate, and they are opposite ones, which is the point of the gate asking rather than telling. ivarNamesInOrder's new arm is UNREACHED, not dead. Nothing in the VM gets there any more, because the constructor that used to leave ivarOrder empty now fills it -- that is the fix. The arm is what keeps the NEXT such constructor from being silent, and the shape it guards is a plain *RObject with a populated map, which any new code can produce in one line. So it gets a test that builds exactly that shape, with three cases: an empty order list (all names, sorted), a half-filled one (the listed name keeps its place, the rest follow sorted), and a name in the list whose value is gone (which stays gone -- a floor that resurrected a removed ivar would be worse than none). convObject's missing-key arm IS dead: yamlIVarOrder returns only keys present in o.IVars, so the lookup cannot fail. Removed rather than tested, because a test for an unreachable branch is a test that asserts nothing. Co-Authored-By: Claude Opus 5 --- internal/vm/ivar_order_internal_test.go | 71 +++++++++++++++++++++++++ internal/vm/yaml_bind.go | 9 ++-- 2 files changed, 75 insertions(+), 5 deletions(-) create mode 100644 internal/vm/ivar_order_internal_test.go diff --git a/internal/vm/ivar_order_internal_test.go b/internal/vm/ivar_order_internal_test.go new file mode 100644 index 00000000..9e0810eb --- /dev/null +++ b/internal/vm/ivar_order_internal_test.go @@ -0,0 +1,71 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm + +import ( + "testing" + + "github.com/go-embedded-ruby/ruby/internal/object" +) + +// TestIvarNamesInOrderReportsNamesMissingFromTheOrderList covers the floor under +// #787 directly, because nothing in the VM reaches it any more: the constructor +// that used to leave ivarOrder empty now fills it, which is the fix, and this +// arm is what keeps the NEXT one from being silent. +// +// The coverage gate named it, and the question a named function asks is whether +// the branch is dead or unreached. It is not dead -- the shape it guards is a +// plain *RObject built with a populated ivars map, which any new constructor can +// produce in one line -- so it gets a test that builds exactly that shape rather +// than being deleted. +func TestIvarNamesInOrderReportsNamesMissingFromTheOrderList(t *testing.T) { + // Exactly the shape yaml_bind.go produced before #787: a live map, an empty + // order list. + o := &RObject{ivars: map[string]object.Value{ + "@b": object.IntValue(1), + "@a": object.IntValue(2), + }} + got := namesOf(ivarNamesInOrder(o)) + // Sorted, because the order list says nothing and Go's map order is random: + // an enumeration that changes between runs would be its own defect. + if len(got) != 2 || got[0] != "@a" || got[1] != "@b" { + t.Errorf("ivarNamesInOrder = %v, want [@a @b] (sorted, since the order list is empty)", got) + } + + // A half-filled order list: the named one keeps its position, the rest are + // appended sorted. This is the arm that would be wrong if the floor simply + // replaced the ordered walk instead of extending it. + o2 := &RObject{ + ivars: map[string]object.Value{ + "@z": object.IntValue(1), + "@m": object.IntValue(2), + "@a": object.IntValue(3), + }, + ivarOrder: []string{"@z"}, + } + got = namesOf(ivarNamesInOrder(o2)) + if len(got) != 3 || got[0] != "@z" || got[1] != "@a" || got[2] != "@m" { + t.Errorf("ivarNamesInOrder = %v, want [@z @a @m]: the listed name keeps its place, the rest follow sorted", got) + } + + // And a name in the order list whose value is gone stays gone -- the floor + // must not resurrect what remove_instance_variable deleted. + o3 := &RObject{ + ivars: map[string]object.Value{"@keep": object.IntValue(1)}, + ivarOrder: []string{"@gone", "@keep"}, + } + got = namesOf(ivarNamesInOrder(o3)) + if len(got) != 1 || got[0] != "@keep" { + t.Errorf("ivarNamesInOrder = %v, want [@keep]: a removed ivar stays removed", got) + } +} + +func namesOf(vs []object.Value) []string { + out := make([]string, 0, len(vs)) + for _, v := range vs { + out = append(out, string(v.(object.Symbol))) + } + return out +} diff --git a/internal/vm/yaml_bind.go b/internal/vm/yaml_bind.go index af883756..714fb8d7 100644 --- a/internal/vm/yaml_bind.go +++ b/internal/vm/yaml_bind.go @@ -568,12 +568,11 @@ func (c *yamlFromCtx) convObject(o *yaml.Object) object.Value { // names in o.Order that exist, then the rest lexicographically. Taking it // from there rather than ranging the map keeps the result both MRI-like // (document order) and stable across runs. + // No presence check on the lookup: yamlIVarOrder returns only keys that are + // in o.IVars, so a missing-key arm here would be dead code the coverage gate + // would then ask about -- and it did, on the first version of this. for _, k := range yamlIVarOrder(o) { - val, ok := o.IVars[k] - if !ok { - continue - } - obj.ivars["@"+k] = c.conv(val) + obj.ivars["@"+k] = c.conv(o.IVars[k]) obj.ivarOrder = append(obj.ivarOrder, "@"+k) } return obj From 357e50d413948ec683cb786f853a0938fba214d0 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Fri, 9 Oct 2026 15:29:22 +0200 Subject: [PATCH 3/3] conformance: put the known flake back at its observed minimum core/random/bytes_spec.rb went 8 -> 9 yesterday as one of twenty deltas applied from a CI run. It should not have moved: it is the KNOWN FLAKE this file records at its observed minimum, and its own header says why, in these words -- Recording a flake at its high value would fail the lane on a coin toss Which is what happened on the next run: 23512 (baseline 23512, +0) with one REGRESSED and one improved, net zero, red lane. Measured here afterwards on the same binary, six runs: 9 9 9 9 8 9. Two earlier waves applied deltas to this file and deliberately left this path alone, saying so each time (2026-09-29 and 2026-09-30, both in the header). Mine did not, and the reason is worth writing down rather than just fixing: All three checks that commit lists were ARITHMETIC. The pre-edit sum matched the README, every one of CI's "baseline" columns matched the recorded value, and the post-edit sum equalled CI's total. Every one passed. None of them asked whether a number SHOULD move -- which is the question this file answers in its own header, three times, about this exact path. I read seven lines of a header I was editing and then edited past the rest of it. Back to 8, and the README's three claims follow to 23,511. The note is in the header next to the mistake rather than in a commit message nobody reads while applying the next delta. Co-Authored-By: Claude Opus 5 --- README.md | 6 +++--- scripts/conformance/rubyspec/BASELINE | 18 +++++++++++++++++- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 52d51e77..055a4517 100644 --- a/README.md +++ b/README.md @@ -6,14 +6,14 @@ [![License](https://img.shields.io/badge/license-BSD--3--Clause-blue)](LICENSE) [![Go](https://img.shields.io/badge/go-1.27.1%2B-00ADD8)](https://go.dev/dl/) [![Release](https://img.shields.io/github/v/release/go-embedded-ruby/ruby?label=release&color=blue)](https://github.com/go-embedded-ruby/ruby/releases/latest) -[![ruby/spec](https://img.shields.io/badge/ruby%2Fspec-23%2C512%20examples%20passing-1a7f37)](#runtime-conformance--rubyspec) +[![ruby/spec](https://img.shields.io/badge/ruby%2Fspec-23%2C511%20examples%20passing-1a7f37)](#runtime-conformance--rubyspec) **A Ruby interpreter written in pure Go, with cgo disabled** — so you can embed it in a Go program with `import "github.com/go-embedded-ruby/ruby"`, or ship it as a single static binary that cross-compiles wherever Go does, with no C toolchain and no libruby. -**How complete is it?** It runs **23,512** of ruby/spec's `language/` + `core/` +**How complete is it?** It runs **23,511** of ruby/spec's `language/` + `core/` examples ([what that counts](#runtime-conformance--rubyspec)) — a large and growing subset of the language, and **not** a drop-in replacement for CRuby. Read [What does not work yet](#what-does-not-work-yet) before you depend on it; the @@ -988,7 +988,7 @@ specification of the language — through `rbgo` under a minimal MSpec-compatibl shim, and gates CI on a **per-file** baseline ([`scripts/conformance/rubyspec/`](scripts/conformance/rubyspec/), in `BASELINE`). No file may pass fewer examples than its record or stop loading, so -measured language conformance moves in one direction; the total, **23,512**, is +measured language conformance moves in one direction; the total, **23,511**, is a derived summary rather than the thing gated. Run it with `scripts/conformance/rubyspec/run.sh`, and see *Runtime conformance* under *Status* for the full breakdown. diff --git a/scripts/conformance/rubyspec/BASELINE b/scripts/conformance/rubyspec/BASELINE index 61ecb187..f8dbcdcd 100644 --- a/scripts/conformance/rubyspec/BASELINE +++ b/scripts/conformance/rubyspec/BASELINE @@ -18,6 +18,22 @@ # regression, no new file, no change of loading -- so the deltas are the whole # move. # +# ⛔ 2026-10-09: ONE of those twenty was wrong to apply, and this file said so. +# core/random/bytes_spec.rb went 8 -> 9, and it is the KNOWN FLAKE held at its +# observed minimum (see "One file is recorded at its OBSERVED MINIMUM" below, +# and the two earlier waves that applied deltas and deliberately left it alone). +# The next run read 8 and the lane went red, which is the coin toss that note +# predicts in those words. Put back to 8. +# +# All three checks that commit lists were ARITHMETIC -- sums and columns -- and +# every one of them passed. None of them asked whether a number SHOULD move, +# which is the question this file answers in its own header, three times over, +# about this exact path. Measured here afterwards, same binary, six runs: +# 9 9 9 9 8 9. +# +# So: when applying a ratchet delta, read the header for the FILE being moved +# before moving it. The sum being right is not evidence that the move was. +# # THE FRAME MATTERS, twice over, and both are why an earlier attempt at this # file was wrong: # @@ -1630,7 +1646,7 @@ OK core/queue/pop_spec.rb 20 OK core/queue/push_spec.rb 1 OK core/queue/shift_spec.rb 1 OK core/queue/size_spec.rb 1 -OK core/random/bytes_spec.rb 9 +OK core/random/bytes_spec.rb 8 OK core/random/default_spec.rb 1 OK core/random/equal_value_spec.rb 2 OK core/random/new_seed_spec.rb 3