From 9e6c8a9f98f90c769450ea4df6bdcd0bd13b1d7f Mon Sep 17 00:00:00 2001 From: Largo Date: Wed, 28 Oct 2026 16:25:42 +0100 Subject: [PATCH 1/2] Fiddle looks a Win32 name up as CRuby's does; irb checked in CI and in packaging On Windows, Handle#[] tries a name as given, then with the ANSI "A" suffix, then with the stdcall decorations @0..@252, plain and after the A - the order of fiddle 1.1.8's fiddle_handle_find_func. reline asks for FillConsoleOutputCharacter, which kernel32 exports only as ...A and ...W, so irb died at startup in 4.0.2. The same change is in #38 (920e6ca8). Fiddle_AnsiSuffix1 tests it; CI's smoke test and the release check start irb with piped input on every platform. --- .github/workflows/ci.yml | 4 ++++ Src/IronRuby.Tests/RubyTests.cs | 1 + Src/IronRuby.Tests/Runtime/MiscTests.cs | 28 ++++++++++++++++++++++ Src/Libraries/Fiddle/FiddleHandle.cs | 32 +++++++++++++++++++++---- Util/package-release.sh | 5 +++- 5 files changed, 64 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ec83e605c..cd5c484d6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -219,6 +219,9 @@ jobs: cmd //c ironruby.cmd smoke.rb cmd //c ironruby.cmd names.rb cmd //c ir.cmd -e "puts RUBY_ENGINE" + # irb loads reline, which on Windows looks up Win32 functions through Fiddle by + # their names without the A suffix; 4.0.2 died there, at a pipe as at a console. + printf 'puts 6 * 7\n' | cmd //c irb.cmd | grep -x 42 cmd //c ironruby.cmd Util/windows-desktop-check.rb rubyw="Src/Console/bin/Debug/$TFM/ironrubyw.exe" ruby subsystem.rb "$rubyw" @@ -227,6 +230,7 @@ jobs: ./ironruby smoke.rb ./ironruby names.rb ./ir.sh -e 'puts RUBY_ENGINE' + printf 'puts 6 * 7\n' | ./irb.sh | grep -x 42 fi # IO#write_nonblock on a socket claims only what the kernel took (#14). On Linux the diff --git a/Src/IronRuby.Tests/RubyTests.cs b/Src/IronRuby.Tests/RubyTests.cs index 162c4ad37..eecf55be9 100644 --- a/Src/IronRuby.Tests/RubyTests.cs +++ b/Src/IronRuby.Tests/RubyTests.cs @@ -730,6 +730,7 @@ public Tests(Driver/*!*/ driver) { Zlib_Streams1, Process_SpawnKill1, File_DriveLetters1, + Fiddle_AnsiSuffix1, Dir_ChdirRelative1, File_ExpandTildeOrder1, File_FlockReadpartialRlimit1, diff --git a/Src/IronRuby.Tests/Runtime/MiscTests.cs b/Src/IronRuby.Tests/Runtime/MiscTests.cs index e41885611..adca71f71 100644 --- a/Src/IronRuby.Tests/Runtime/MiscTests.cs +++ b/Src/IronRuby.Tests/Runtime/MiscTests.cs @@ -945,6 +945,34 @@ p Pathname.new('C:/x/y').absolute? == windows, Pathname.new('/x').absolute? "); } + /// + /// Fiddle::Handle finds a Win32 function by the name without its A/W suffix, as CRuby's + /// fiddle does (it tries "A" next): kernel32 exports FillConsoleOutputCharacterA and ...W + /// but no FillConsoleOutputCharacter, and reline asks for that one - irb died at startup + /// on Windows without it. Elsewhere there is no suffix to try; the answers are the same. + /// + public void Fiddle_AnsiSuffix1() { + TestOutput(@" +require 'fiddle' +if File::ALT_SEPARATOR == '\\' + k = Fiddle::Handle.new('kernel32') + p k['GetModuleHandle'] == k['GetModuleHandleA'], !k.sym_defined?('FillConsoleOutputCharacter').nil? + begin + k['NoSuchFunction'] + rescue Fiddle::DLError => e + p e.message + end +else + p true, true + p 'unknown symbol ""NoSuchFunction""' +end +", @" +true +true +""unknown symbol \""NoSuchFunction\"""" +"); + } + /// /// A relative Dir.chdir is resolved once, from the directory in force - Dir.pwd keeping /// 8.3 short names once made it twice, so ".." went up two levels. diff --git a/Src/Libraries/Fiddle/FiddleHandle.cs b/Src/Libraries/Fiddle/FiddleHandle.cs index ebaf371a1..1944075db 100644 --- a/Src/Libraries/Fiddle/FiddleHandle.cs +++ b/Src/Libraries/Fiddle/FiddleHandle.cs @@ -226,6 +226,13 @@ public static object Initialize(BlockParam block, Handle/*!*/ self, [DefaultPara #region Symbols + /// + /// The address of an export, or zero. On Windows a name is tried as CRuby's fiddle + /// tries it (fiddle_handle_find_func): as given, then with the ANSI "A" suffix - the + /// Win32 API exports FillConsoleOutputCharacterA and ...W but no + /// FillConsoleOutputCharacter, which reline asks for - then with the stdcall + /// decorations "@0" to "@252", plain and after the "A". + /// private IntPtr Lookup(string/*!*/ name) { if (!_open) { throw new DLError("closed handle"); @@ -233,15 +240,30 @@ private IntPtr Lookup(string/*!*/ name) { if (_next) { throw new DLError("RTLD_NEXT is not supported on this platform"); } - IntPtr address; - try { - if (!NativeLibrary.TryGetExport(_address, name, out address)) { - return IntPtr.Zero; + IntPtr address = Export(name); + if (address != IntPtr.Zero || !OperatingSystem.IsWindows()) { + return address; + } + if ((address = Export(name + "A")) != IntPtr.Zero) { + return address; + } + foreach (string stem in new[] { name, name + "A" }) { + for (int i = 0; i < 256; i += 4) { + if ((address = Export(stem + "@" + i.ToString(System.Globalization.CultureInfo.InvariantCulture))) != IntPtr.Zero) { + return address; + } } + } + return IntPtr.Zero; + } + + private IntPtr Export(string/*!*/ name) { + try { + IntPtr address; + return NativeLibrary.TryGetExport(_address, name, out address) ? address : IntPtr.Zero; } catch (Exception) { return IntPtr.Zero; } - return address; } [RubyMethod("sym")] diff --git a/Util/package-release.sh b/Util/package-release.sh index c81b0a5f0..a119b3ae7 100755 --- a/Util/package-release.sh +++ b/Util/package-release.sh @@ -110,7 +110,8 @@ esac # A quick check that the unpacked tree starts, before it is archived - and on Windows # that Windows Forms and WPF load from it, which a missing framework reference breaks -# without breaking anything else. +# without breaking anything else. irb has to start too: it loads reline, which on Windows +# looks up Win32 functions through Fiddle, and 4.0.2's died there. # One command per line: set -e does not stop at a failure on the left of && - nor at a # case whose last && list failed that way - so a chain would let a broken tree through. case "$RID" in @@ -119,11 +120,13 @@ case "$RID" in (cd "$STAGE" cmd //c ironruby.cmd -e 'puts RUBY_DESCRIPTION' cmd //c ir.cmd -e 'puts RUBY_ENGINE' + printf 'puts 6 * 7\n' | cmd //c irb.cmd | grep -x 42 cmd //c ir.cmd "$(cygpath -w "$IR_ROOT/Util/windows-desktop-check.rb")") ;; *) "$STAGE/ironruby" -e 'puts RUBY_DESCRIPTION' "$STAGE/ir.sh" -e 'puts RUBY_ENGINE' + printf 'puts 6 * 7\n' | "$STAGE/irb.sh" | grep -x 42 ;; esac From 3b486b67aee103e69bdb9125c162c6c47e33fe53 Mon Sep 17 00:00:00 2001 From: Largo Date: Wed, 28 Oct 2026 16:26:11 +0100 Subject: [PATCH 2/2] pack("p") writes a real address, so reline runs as Ruby ships it [str].pack("p") and ("P") wrote an index into the packed string's list of Strings, not an address. reline's Win32API hands the console API every buffer as [str].pack("p").unpack("q*") and reads what the call wrote from str, so irb in a Windows console crashed with an access violation; so would any gem that passes buffers that way. Now each String pack points at gets a native block of its own (PackedPointers), holding its bytes and a NUL for as long as the String lives, and pack writes the block's address. Fiddle::Function#call knows the address among its pointer arguments: it copies the String into the block before the call and the block back into the String after it, which is what the String's own buffer gives in CRuby. unpack("p") still finds only Strings packed into that same string ("no associated pointer"), now by address. reline/io/windows.rb stays as upstream has it. - Tests: PackPointer_RealAddress1 (C reads and writes through pack("p"), strlen and memset, all platforms) and PackPointer_RelineWin32Api1 (reline's own Win32API filling a buffer, Windows). Both check the address before C touches it, so a regression fails them rather than killing the run; without this change they report the old indexes. - CI fails a test run that did not finish: an access violation ends the process and leaves no "> FAILED" line to count. ruby/spec core/array/pack and core/string/unpack (3193 examples) and the C# suite pass; reading, writing and unpacking through pack("p") gives byte-identical output to CRuby 4.0.1; and irb in a real console, with typed input, answers as CRuby's does. --- .github/workflows/ci.yml | 7 ++ Src/IronRuby.Tests/RubyTests.cs | 2 + Src/IronRuby.Tests/Runtime/MiscTests.cs | 73 ++++++++++++++ Src/Libraries/Builtins/PackedPointers.cs | 120 +++++++++++++++++++++++ Src/Libraries/Builtins/RubyEncoder.cs | 30 ++++-- Src/Libraries/Fiddle/FiddleCall.cs | 28 +++++- 6 files changed, 249 insertions(+), 11 deletions(-) create mode 100644 Src/Libraries/Builtins/PackedPointers.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cd5c484d6..921b145d8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -377,6 +377,13 @@ jobs: [ "$RUNNER_OS" = Windows ] && EXE=.exe cd "Src/IronRuby.Tests/bin/Debug/$TFM" "./IronRuby.Tests$EXE" > results.txt 2>&1 || true + # A run that died - an access violation in native code ends the process - leaves no + # "> FAILED" line to count; one that finished ends in PASSED or in its Repro line. + if ! grep -qE 'PASSED.?$|^Repro: ' results.txt; then + echo "::error::the test run did not finish" + tail -n 40 results.txt + exit 1 + fi failures=$(grep -c '^> FAILED' results.txt || true) echo "failure lines: $failures (baseline $BASELINE)" echo "::group::failing tests" diff --git a/Src/IronRuby.Tests/RubyTests.cs b/Src/IronRuby.Tests/RubyTests.cs index eecf55be9..234b2c436 100644 --- a/Src/IronRuby.Tests/RubyTests.cs +++ b/Src/IronRuby.Tests/RubyTests.cs @@ -731,6 +731,8 @@ public Tests(Driver/*!*/ driver) { Process_SpawnKill1, File_DriveLetters1, Fiddle_AnsiSuffix1, + PackPointer_RealAddress1, + PackPointer_RelineWin32Api1, Dir_ChdirRelative1, File_ExpandTildeOrder1, File_FlockReadpartialRlimit1, diff --git a/Src/IronRuby.Tests/Runtime/MiscTests.cs b/Src/IronRuby.Tests/Runtime/MiscTests.cs index adca71f71..f1ad5e78b 100644 --- a/Src/IronRuby.Tests/Runtime/MiscTests.cs +++ b/Src/IronRuby.Tests/Runtime/MiscTests.cs @@ -973,6 +973,79 @@ p e.message "); } + /// + /// [str].pack("p") is a real address, as in CRuby: C reads the String there, and what C + /// writes there through Fiddle lands in the String. It used to be an index into the + /// packed string's list of Strings, and C reading it crashed the process. + /// + public void PackPointer_RealAddress1() { + TestOutput(@" +require 'fiddle' +lib = case RUBY_PLATFORM + when /mswin|mingw/ then 'msvcrt.dll' + when /darwin/ then '/usr/lib/libSystem.B.dylib' + else 'libc.so.6' + end +libc = Fiddle.dlopen(lib) +strlen = Fiddle::Function.new(libc['strlen'], [Fiddle::TYPE_VOIDP], Fiddle::TYPE_SIZE_T) +memset = Fiddle::Function.new(libc['memset'], [Fiddle::TYPE_VOIDP, Fiddle::TYPE_INT, Fiddle::TYPE_SIZE_T], Fiddle::TYPE_VOIDP) + +# C only gets an address that can be one: what pack wrote before was an index, and C reading +# or writing there kills the process, which no test can report. +s = 'hello'.dup +addr = [s].pack('p').unpack1('J') +buf = ""\0"" * 6 +buf_addr = [buf].pack('p').unpack1('J') +if addr > 0x10000 && buf_addr > 0x10000 + p Fiddle::Pointer.new(addr).to_s, strlen.call(addr) + memset.call(buf_addr, 0x41, 3) + p buf +else + p [:not_an_address, addr, buf_addr] +end +p [s].pack('p').unpack1('p'), [nil].pack('p').unpack1('J') +begin + [addr].pack('J').unpack1('p') +rescue ArgumentError => e + p e.message +end +", @" +""hello"" +5 +""AAA\u0000\u0000\u0000"" +""hello"" +0 +""no associated pointer"" +"); + } + + /// + /// reline's own Win32API, unmodified, hands the console API its buffers as + /// [str].pack(""p"").unpack(""q*"") and reads what the call wrote from str - which is + /// what irb does at startup in a Windows console, and what crashed there. + /// + public void PackPointer_RelineWin32Api1() { + TestOutput(@" +if File::ALT_SEPARATOR == '\\' + require 'ruby4' + require 'reline' + api = Reline::Windows::Win32API.new('kernel32', 'GetModuleFileNameA', ['L', 'P', 'L'], 'L') + path = ""\0"" * 260 + if [path].pack('p').unpack1('J') > 0x10000 + n = api.call(0, path, 260) + p n > 0, path[0, n].downcase.end_with?('.exe') + else + p :not_an_address + end +else + p true, true +end +", @" +true +true +"); + } + /// /// A relative Dir.chdir is resolved once, from the directory in force - Dir.pwd keeping /// 8.3 short names once made it twice, so ".." went up two levels. diff --git a/Src/Libraries/Builtins/PackedPointers.cs b/Src/Libraries/Builtins/PackedPointers.cs new file mode 100644 index 000000000..b3347e8a5 --- /dev/null +++ b/Src/Libraries/Builtins/PackedPointers.cs @@ -0,0 +1,120 @@ +/* **************************************************************************** + * + * PackedPointers - the addresses Array#pack("p") and ("P") write. + * + * This source code is subject to terms and conditions of the Apache License, + * Version 2.0. A copy of the license can be found in the License.html file at + * the root of this distribution. + * + * ***************************************************************************/ + +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; + +namespace IronRuby.Builtins { + + /// + /// CRuby's pack("p") writes the address of the String's own buffer, NUL-terminated, and C + /// reads and writes the String through it: reline's Win32API passes every buffer to the + /// console API as [str].pack("p").unpack("q*"), and reads what the call wrote from str. + /// A MutableString has no such address - its bytes live on a heap that moves them, and C + /// writing behind its back would leave its cached state stale - so each String pack is asked + /// to point at gets a native block of its own, holding its bytes and a NUL, for as long as + /// the String lives. pack writes the block's address. Fiddle recognises the address among a + /// call's pointer arguments and copies the String into the block before the call and the + /// block back into the String after it (Fiddle.NativeCall.ToPointerArgument), which is + /// what the String's own buffer would have given. A String that has grown past its block + /// since gets a new one, at a new address, as CRuby's buffer moves when it is reallocated. + /// + internal static class PackedPointers { + private static readonly ConditionalWeakTable _byString = new ConditionalWeakTable(); + private static readonly ConcurrentDictionary> _byAddress = new ConcurrentDictionary>(); + private static readonly object _lock = new object(); + + internal sealed class Block { + internal readonly MutableString/*!*/ String; + internal readonly IntPtr Address; + /// Bytes the block holds, the NUL included. + internal readonly int Capacity; + private readonly WeakReference/*!*/ _entry; + + internal Block(MutableString/*!*/ str, int capacity) { + String = str; + Capacity = capacity; + Address = Marshal.AllocHGlobal(capacity); + _entry = new WeakReference(this); + _byAddress[(long)Address] = _entry; + } + + ~Block() { + // Forget the address before the memory goes, so it cannot be found for a block + // that a later allocation gets at the same place. + ((ICollection>>)_byAddress).Remove( + new KeyValuePair>((long)Address, _entry)); + Marshal.FreeHGlobal(Address); + } + + internal void Forget() { + ((ICollection>>)_byAddress).Remove( + new KeyValuePair>((long)Address, _entry)); + } + + /// Puts the String's bytes and a NUL into the block; answers how many bytes. + internal int Refresh() { + byte[] bytes = String.ToByteArray(); + int count = Math.Min(bytes.Length, Capacity - 1); + if (count > 0) { + Marshal.Copy(bytes, 0, Address, count); + } + Marshal.WriteByte(Address, count, 0); + return count; + } + + /// Puts what C left in the block back into the String. + internal void CopyBack(int count) { + if (String.IsFrozen) { + return; + } + count = Math.Min(count, String.GetByteCount()); + for (int i = 0; i < count; i++) { + String.SetByte(i, Marshal.ReadByte(Address, i)); + } + } + } + + /// The address pack writes for , its bytes put there. + internal static long AddressOf(MutableString/*!*/ str) { + int needed = str.GetByteCount() + 1; + Block block; + lock (_lock) { + if (!_byString.TryGetValue(str, out block) || block.Capacity < needed) { + if (block != null) { + // the old block is freed once nothing holds it; its address is found no more + _byString.Remove(str); + block.Forget(); + } + block = new Block(str, needed); + _byString.Add(str, block); + } + } + block.Refresh(); + return (long)block.Address; + } + + /// The block at , if pack wrote that address for a live String. + internal static Block Find(long address) { + WeakReference entry; + Block block; + return (address != 0 && _byAddress.TryGetValue(address, out entry) && entry.TryGetTarget(out block)) ? block : null; + } + + /// Whether is where pack put last. + internal static bool IsAddressOf(MutableString/*!*/ str, long address) { + Block block; + return _byString.TryGetValue(str, out block) && (long)block.Address == address; + } + } +} diff --git a/Src/Libraries/Builtins/RubyEncoder.cs b/Src/Libraries/Builtins/RubyEncoder.cs index d1a7f5e31..fa0424572 100644 --- a/Src/Libraries/Builtins/RubyEncoder.cs +++ b/Src/Libraries/Builtins/RubyEncoder.cs @@ -1499,11 +1499,12 @@ private static bool IsKnownDirective(char c) { case 'P': { // MRI writes the address of the string and remembers the string // itself alongside the result, so that unpacking can find it again. - // An address into a managed heap would mean nothing, so what goes - // into the bytes is a one-based index into that list of strings. - // The list travels as an instance variable, which is what makes it - // survive #dup and not survive a round trip through a Symbol - - // exactly the rule MRI's associated-object table follows. + // The address is a real one, of a native block holding the string's + // bytes (PackedPointers), so C can read and - through Fiddle - write + // the string as it does in MRI. The list of strings travels as an + // instance variable, which is what makes it survive #dup and not + // survive a round trip through a Symbol - exactly the rule MRI's + // associated-object table follows. count = 1; object pointerArg = GetPackArg(self, i); long pointer = 0; @@ -1514,7 +1515,7 @@ private static bool IsKnownDirective(char c) { pointers = new RubyArray(); } pointers.Add(target); - pointer = pointers.Count; + pointer = PackedPointers.AddressOf(target); } } WritePointer(stream, pointer); @@ -1578,15 +1579,24 @@ private static object Dereference(RubyContext/*!*/ context, MutableString/*!*/ s return null; } + // As MRI's: one of the strings packed into this one, the one whose bytes are at that + // address now - a string that has outgrown its block since is at another. object stored; RubyArray pointers; - if (!context.TryGetInstanceVariable(self, PackedPointersVariable, out stored) || - (pointers = stored as RubyArray) == null || - pointer < 1 || pointer > pointers.Count) { + MutableString target = null; + if (context.TryGetInstanceVariable(self, PackedPointersVariable, out stored) && (pointers = stored as RubyArray) != null) { + foreach (object candidate in pointers) { + var str = candidate as MutableString; + if (str != null && PackedPointers.IsAddressOf(str, pointer)) { + target = str; + break; + } + } + } + if (target == null) { throw RubyExceptions.CreateArgumentError("no associated pointer"); } - var target = (MutableString)pointers[(int)pointer - 1]; if (directive.Directive == 'p') { return target.Clone(); } diff --git a/Src/Libraries/Fiddle/FiddleCall.cs b/Src/Libraries/Fiddle/FiddleCall.cs index 5128a1984..575b3c8a1 100644 --- a/Src/Libraries/Fiddle/FiddleCall.cs +++ b/Src/Libraries/Fiddle/FiddleCall.cs @@ -302,6 +302,11 @@ internal struct Temporary { internal MutableString String; internal int Count; internal bool CopyBack; + /// + /// The block is a String's from pack("p") (PackedPointers), not the call's: it is + /// copied back but not freed, and held here so that it outlives the call. + /// + internal PackedPointers.Block Packed; } internal static long ToLong(object value) { @@ -385,7 +390,21 @@ internal static long ToPointerArgument(RespondToStorage/*!*/ respondTo, UnaryOpS } if (value is int || value is BigInteger || value is long) { - return ToLong(value); + long address = ToLong(value); + // An address pack("p") wrote for a String: C gets the String's bytes as they + // are now, and what it writes there goes back into the String, as through the + // String's own buffer in CRuby - reline's Win32API passes its buffers so. + PackedPointers.Block packed = PackedPointers.Find(address); + if (packed != null) { + temporaries.Add(new Temporary { + Block = packed.Address, + String = packed.String, + Count = packed.Refresh(), + CopyBack = !isConst && !packed.String.IsFrozen, + Packed = packed + }); + } + return address; } var handle = value as Handle; @@ -431,6 +450,13 @@ internal static void ReleaseTemporaries(List/*!*/ temporaries, List