From b83ce8c30edd25d95455d3fa30b03337ec024f77 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Wed, 4 Nov 2026 22:19:25 +0100 Subject: [PATCH] Marshal: a class path that is not ASCII carries its encoding, as 4.0 dumps it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three more of the 27 examples that went red on the move to Ruby 4.0 (#799), again one cause. `core/marshal/dump_spec.rb` goes 191 → 194, fail 18 → 15. ## The reference marshal.c at v4.0.9 brackets the class path with the same encoding machinery a String goes through: VALUE encname = w_encivar(path, arg); /* emits 'I' when path is non-ASCII */ w_byte(TYPE_CLASS, arg); w_bytes(RSTRING_PTR(path), RSTRING_LEN(path), arg); w_encname(encname, arg); /* 1 ivar: :E => true */ `w_encivar` returns nil when the path is ASCII-only, which is what keeps every ordinary class name dumping exactly as before: static VALUE w_encivar(VALUE str, struct dump_arg *arg) { VALUE encname = encoding_name(str, arg); if (NIL_P(encname) || is_ascii_string(str)) return Qnil; w_byte(TYPE_IVAR, arg); return encname; } `encoding_name` maps UTF-8 to Qtrue, so the trailer is the short `:E => true` form and never the long `:encoding => "..."` one — a constant name in rbgo is a Go string and therefore UTF-8, and there is no way to name a constant in another encoding. ## Measured, not read rubydiff against MRI 4.0.7 on this host: AGREE, byte for byte, on all eleven probes. The three dumps the specs assert: "\x04\bIc,MarshalSpec::Multibyte\xE3\x81\x81...Class\x06:\x06ET" "\x04\bIm-MarshalSpec::Multibyte\xE3\x81\x91...Module\x06:\x06ET" "\x04\bIc+MarshalSpec::Multibyte\xE3\x81\x81...Time\x06:\x06ET" ## The controls matter more than the three rows * An ASCII name must still dump unwrapped — `Marshal.dump(String)` is `"\x04\bc\vString"`. A wrapper emitted unconditionally would pass all three spec rows and break every existing dump. * The reader had never been handed an 'I' in front of a 'c'. All three round-trip to the identical object, which the specs do NOT assert — their second line has no `.should`. * Object links still resolve to the start of the wrapped form: `Marshal.dump([K, K])` ends `@\x06`, and loading it gives `[true, true]` on `.equal?`. specdelta, provenanced, over core/marshal: control 442, treatment 445, **+3, 1 file moved, 0 regressed**, with `load_spec.rb` unchanged at 230. Coverage: writeClassRef and classPathEncIvar at 100.0%. Co-Authored-By: Claude Opus 5 --- internal/vm/marshal_class_encoding_test.go | 94 ++++++++++++++++++++++ internal/vm/marshal_codec.go | 36 +++++++++ 2 files changed, 130 insertions(+) create mode 100644 internal/vm/marshal_class_encoding_test.go diff --git a/internal/vm/marshal_class_encoding_test.go b/internal/vm/marshal_class_encoding_test.go new file mode 100644 index 00000000..75bebb35 --- /dev/null +++ b/internal/vm/marshal_class_encoding_test.go @@ -0,0 +1,94 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm + +import "testing" + +// Ruby 4.0 records the encoding of a class path that is not ASCII, the same way +// it records a String's. marshal.c's T_CLASS and T_MODULE arms bracket the +// bytes with w_encivar/w_encname: +// +// VALUE encname = w_encivar(path, arg); /* emits 'I' when path is non-ASCII */ +// w_byte(TYPE_CLASS, arg); +// w_bytes(RSTRING_PTR(path), RSTRING_LEN(path), arg); +// w_encname(encname, arg); /* 1 ivar: :E => true */ +// +// The three ruby/spec examples behind `ruby_version_is "4.0"` in +// core/marshal/dump_spec.rb are the first three rows, with the byte strings +// copied from the spec rather than from this implementation's output. +func TestMarshalDumpsAClassPathsEncoding(t *testing.T) { + const defs = ` +module MarshalSpec; end +eval("class MarshalSpec::MultibyteぁあぃいClass; end".dup.force_encoding(Encoding::UTF_8)) +eval("module MarshalSpec::MultibyteけげこごModule; end".dup.force_encoding(Encoding::UTF_8)) +eval("class MarshalSpec::MultibyteぁあぃいTime < Time; end".dup.force_encoding(Encoding::UTF_8)) +K = eval("MarshalSpec::MultibyteぁあぃいClass".dup.force_encoding(Encoding::UTF_8)) +M = eval("MarshalSpec::MultibyteけげこごModule".dup.force_encoding(Encoding::UTF_8)) +TC = eval("MarshalSpec::MultibyteぁあぃいTime".dup.force_encoding(Encoding::UTF_8)) +` + for _, tc := range []struct{ name, src, want string }{ + {"a class with a multibyte name", `p Marshal.dump(K)`, + `"\x04\bIc,MarshalSpec::Multibyte\xE3\x81\x81\xE3\x81\x82\xE3\x81\x83\xE3\x81\x84Class\x06:\x06ET"`}, + {"a module with a multibyte name", `p Marshal.dump(M)`, + `"\x04\bIm-MarshalSpec::Multibyte\xE3\x81\x91\xE3\x81\x92\xE3\x81\x93\xE3\x81\x94Module\x06:\x06ET"`}, + {"a Time subclass with a multibyte name", `p Marshal.dump(TC)`, + `"\x04\bIc+MarshalSpec::Multibyte\xE3\x81\x81\xE3\x81\x82\xE3\x81\x83\xE3\x81\x84Time\x06:\x06ET"`}, + + // The dump has to be readable by the reader that already exists: the + // 'I' wrapper in front of a 'c' is a shape it had never been handed. + {"the class round-trips", `p(Marshal.load(Marshal.dump(K)).equal?(K))`, "true"}, + {"the module round-trips", `p(Marshal.load(Marshal.dump(M)).equal?(M))`, "true"}, + {"the Time subclass round-trips", `p(Marshal.load(Marshal.dump(TC)).equal?(TC))`, "true"}, + + // ⛔ THE CONTROL. w_encivar returns nil for an ASCII-only path, so an + // ordinary class name must dump byte-for-byte as it did before. A + // wrapper emitted unconditionally would still pass every row above. + {"an ASCII class name is unwrapped", `p Marshal.dump(String)`, `"\x04\bc\vString"`}, + {"an ASCII module name is unwrapped", `p Marshal.dump(Marshal)`, `"\x04\bm\fMarshal"`}, + + // Object links still point at the start of the wrapped form, not at the + // 'c' inside it: @\x06 is index 1, the array being 0. + {"a repeated multibyte class is a link", `p Marshal.dump([K, K])`, + `"\x04\b[\aIc,MarshalSpec::Multibyte\xE3\x81\x81\xE3\x81\x82\xE3\x81\x83\xE3\x81\x84Class\x06:\x06ET@\x06"`}, + {"the link round-trips to the same object", + `p(Marshal.load(Marshal.dump([K, K])).map { |c| c.equal?(K) })`, "[true, true]"}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := runSrc(t, defs+tc.src); got != tc.want { + t.Errorf("%s\n got %s\nwant %s", tc.src, got, tc.want) + } + }) + } +} + +// classPathEncIvar is the whole decision, so it is worth testing on its own: +// the wrapper appears for a non-ASCII byte anywhere in the name and nowhere +// else. An empty name cannot reach it — writeClassRef refuses an anonymous +// class first — but it must not panic if it ever does. +func TestClassPathEncIvarIsAboutTheBytes(t *testing.T) { + for _, tc := range []struct { + name string + want bool + }{ + {"String", false}, + {"MarshalSpec::ClassWithOverriddenName", false}, + {"", false}, + {"\x7f", false}, // the last ASCII byte + {"AぁB", true}, // multibyte in the middle + {"ぁ", true}, // multibyte alone + {"MarshalSpec::ぁClass", true}, // the shape the specs use + } { + if got := classPathEncIvar(tc.name) != nil; got != tc.want { + t.Errorf("classPathEncIvar(%q) wrapped=%v, want %v", tc.name, got, tc.want) + } + } + // And when it does answer, it answers with the short :E => true form — + // encoding_name maps UTF-8 to Qtrue, and there is no way to name a constant + // in any other encoding. + e := classPathEncIvar("ぁ") + if e.sym != "E" || e.tf != 'T' { + t.Errorf("classPathEncIvar gave sym=%q tf=%q, want \"E\" and 'T'", e.sym, e.tf) + } +} diff --git a/internal/vm/marshal_codec.go b/internal/vm/marshal_codec.go index 866d9c9a..256306a1 100644 --- a/internal/vm/marshal_codec.go +++ b/internal/vm/marshal_codec.go @@ -533,12 +533,48 @@ func (d *mDumper) writeClassRef(c *RClass) { } raise("TypeError", "can't dump anonymous class %s", c.ToS()) } + // ⛔ Since Ruby 4.0 a class path that is not ASCII carries its encoding, the + // same way a String does. marshal.c's T_CLASS and T_MODULE arms call + // w_encivar(path) BEFORE the type byte — which emits the 'I' wrapper — and + // w_encname(encname) after the bytes: + // + // VALUE encname = w_encivar(path, arg); + // w_byte(TYPE_CLASS, arg); + // w_bytes(RSTRING_PTR(path), RSTRING_LEN(path), arg); + // w_encname(encname, arg); + // + // w_encivar returns nil for an ASCII-only path, so every ordinary class name + // dumps exactly as before and the wrapper appears only where MRI puts it. + enc := classPathEncIvar(c.name) + if enc != nil { + d.buf = append(d.buf, 'I') + } if c.isModule { d.buf = append(d.buf, 'm') } else { d.buf = append(d.buf, 'c') } d.writeBytes(c.name) + if enc != nil { + d.writeLong(1) + d.writeEncodingIvar(enc) + } +} + +// classPathEncIvar is MRI's w_encivar for a class path: nil when the path is +// ASCII-only (is_ascii_string), otherwise the encoding ivar to record. +// +// A constant name in rbgo is a Go string and therefore UTF-8, which is the +// encoding MRI reports for a multibyte constant too — encoding_name maps it to +// the short :E => true form. There is no way to reach the :encoding long form +// from here, because there is no way to name a constant in another encoding. +func classPathEncIvar(name string) *marshalEncIvar { + for i := 0; i < len(name); i++ { + if name[i] >= 0x80 { + return marshalEncodingIvar("UTF-8") + } + } + return nil } // writeTime emits Time's user-defined dump: the 8-byte packed form (MRI's