diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ec83e605c..921b145d8 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 @@ -373,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 162c4ad37..234b2c436 100644 --- a/Src/IronRuby.Tests/RubyTests.cs +++ b/Src/IronRuby.Tests/RubyTests.cs @@ -730,6 +730,9 @@ public Tests(Driver/*!*/ driver) { Zlib_Streams1, 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 e41885611..f1ad5e78b 100644 --- a/Src/IronRuby.Tests/Runtime/MiscTests.cs +++ b/Src/IronRuby.Tests/Runtime/MiscTests.cs @@ -945,6 +945,107 @@ 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\"""" +"); + } + + /// + /// [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

+ /// 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