[GR-17457] Tk runtime fixes

eregon · eregon · commit eda7198072ca · 2022-09-02T19:54:55.000Z
PullRequest: truffleruby/3128
diff --git a/lib/cext/ABI_version.txt b/lib/cext/ABI_version.txt
@@ -1 +1 @@
-10
+11
diff --git a/lib/truffle/truffle/cext.rb b/lib/truffle/truffle/cext.rb
@@ -965,13 +965,16 @@ def rb_hash_set_ifnone(hash, value)
   ST_CONTINUE = 0
   ST_STOP = 1
   ST_DELETE = 2
+  ST_CHECK = 3
+  ST_REPLACE = 4
 
   def rb_hash_foreach(hash, func, farg)
     hash.each do |key, value|
       st_result = Truffle::Interop.execute_without_conversion(func, Primitive.cext_wrap(key), Primitive.cext_wrap(value), farg)
 
       case st_result
       when ST_CONTINUE
+      when ST_CHECK
       when ST_STOP then break
       when ST_DELETE then hash.delete(key)
       else raise ArgumentError, "Unknown 'func' return value: #{st_result}"
@@ -1038,8 +1041,8 @@ def rb_protect(function, arg, write_status, status)
     end
 
     unless Primitive.object_equal(nil, e)
-      store = (Thread.current[:__stored_exceptions__] ||= [])
-      pos = store.push(e).size
+      store_exception(e)
+      pos = extract_tag(e)
       Primitive.thread_set_exception(extract_ruby_exception(e))
     end
 
@@ -1049,14 +1052,9 @@ def rb_protect(function, arg, write_status, status)
 
   def rb_jump_tag(pos)
     if pos > 0
-      store = Thread.current[:__stored_exceptions__]
-      if pos == store.size
-        e = store.pop
-      else
-        # Can't disturb other positions or other rb_jump_tag calls might fail.
-        e = store[pos - 1]
-        store[pos - 1] = nil
-      end
+      e = retrieve_exception
+      tag = extract_tag(e)
+      raise RuntimeError, 'mismatch between jump tag and captured exception' unless pos == tag
       raise_exception(e)
     end
   end
@@ -1950,6 +1948,7 @@ def rb_global_variable(obj)
   end
 
   GC_REGISTERED_ADDRESSES = {}
+
   def rb_gc_register_address(address, obj)
     Truffle::Interop.to_native(address) unless Truffle::Interop.pointer?(address)
     GC_REGISTERED_ADDRESSES[address] = obj
@@ -1959,4 +1958,13 @@ def rb_gc_unregister_address(address)
     Truffle::Interop.to_native(address) unless Truffle::Interop.pointer?(address)
     GC_REGISTERED_ADDRESSES.delete(address)
   end
+
+  def rb_eval_cmd_kw(cmd, args, kw_splat)
+    if (args.size > 0 && kw_splat != 0)
+      kwargs = args.pop
+      cmd.call(*args, **kwargs)
+    else
+      cmd.call(*args)
+    end
+  end
 end
diff --git a/lib/truffle/truffle/cext_preprocessor.rb b/lib/truffle/truffle/cext_preprocessor.rb
@@ -10,6 +10,7 @@
 require_relative 'patches/nokogiri_patches'
 require_relative 'patches/oci8_patches'
 require_relative 'patches/pg_patches'
+require_relative 'patches/tk_patches'
 
 module Truffle
   module CExt
@@ -30,6 +31,7 @@ def self.add_gem_patches(patch_hash, gem_patches)
           end
           processed_patch[:patches] = patch
           processed_patch[:gem] = gem
+          processed_patch[:non_standard_dir_structure] = gem_patches[:non_standard_dir_structure]
           raise "Duplicate patch file #{key}." if patch_hash.include?(key)
           patch_hash[key] = processed_patch
         end
@@ -39,12 +41,17 @@ def self.add_gem_patches(patch_hash, gem_patches)
       add_gem_patches(PATCHED_FILES, ::NokogiriPatches::PATCHES)
       add_gem_patches(PATCHED_FILES, ::OCI8Patches::PATCHES)
       add_gem_patches(PATCHED_FILES, ::PgPatches::PATCHES)
+      add_gem_patches(PATCHED_FILES, ::TkPatches::PATCHES)
 
       def self.makefile_matcher(command1, command2)
         file_list = Hash.new { |h,k| h[k] = [] }
         PATCHED_FILES.each_pair do |file, patch|
           dir = if patch[:ext_dir]
-                  File.join(patch[:gem], 'ext', patch[:ext_dir])
+                  if patch[:non_standard_dir_structure]
+                    File.join('ext', patch[:ext_dir])
+                  else
+                    File.join(patch[:gem], 'ext', patch[:ext_dir])
+                  end
                 else
                   "/#{patch[:gem]}"
                 end
@@ -85,7 +92,11 @@ def self.makefile_matcher(command1, command2)
       def self.patch(file, contents, directory)
         if patched_file = PATCHED_FILES[File.basename(file)]
           matched = if patched_file[:ext_dir]
-                      directory.end_with?(File.join(patched_file[:gem], 'ext', patched_file[:ext_dir]))
+                      if patched_file[:non_standard_dir_structure]
+                        directory.end_with?('ext', patched_file[:ext_dir])
+                      else
+                        directory.end_with?(File.join(patched_file[:gem], 'ext', patched_file[:ext_dir]))
+                      end
                     else
                       regexp = /^#{Regexp.escape(patched_file[:gem])}\b/
                       directory.split('/').last(3).any? { |part| part =~ regexp } || file.split('/').last(2).any? { |part| part =~ regexp }
diff --git a/lib/truffle/truffle/patches/tk_patches.rb b/lib/truffle/truffle/patches/tk_patches.rb
@@ -0,0 +1,33 @@
+# Copyright (c) 2022 Oracle and/or its affiliates. All rights reserved. This
+# code is released under a tri EPL/GPL/LGPL license. You can use it,
+# redistribute it and/or modify it under the terms of the:
+#
+# Eclipse Public License version 2.0, or
+# GNU General Public License version 2, or
+# GNU Lesser General Public License version 2.1.
+
+# Tested with tk version 0.4.0
+class TkPatches
+
+  PATCHES = {
+    gem: 'tk',
+    non_standard_dir_structure: true,
+    patches: {
+      ['tk/tkutil', 'tkutil.c'] => [
+        {
+          match: 'st_foreach_check(RHASH_TBL(keys), to_strkey, new_keys, Qundef)',
+          replacement: 'rb_hash_foreach(keys, to_strkey, new_keys)'
+        },
+        {
+          match: 'st_foreach_check(RHASH_TBL(hash), push_kv, args, Qundef)',
+          replacement: 'rb_hash_foreach(hash, push_kv, args)'
+        },
+        {
+          match: 'st_foreach_check(RHASH_TBL(hash), push_kv_enc, args, Qundef)',
+          replacement: 'rb_hash_foreach(hash, push_kv_enc, args)'
+        },
+      ]
+    }
+  }
+
+end
diff --git a/spec/ruby/optional/capi/ext/kernel_spec.c b/spec/ruby/optional/capi/ext/kernel_spec.c
@@ -112,6 +112,10 @@ VALUE kernel_spec_rb_eval_string(VALUE self, VALUE str) {
   return rb_eval_string(RSTRING_PTR(str));
 }
 
+VALUE kernel_spec_rb_eval_cmd_kw(VALUE self, VALUE cmd, VALUE args, VALUE kw_splat) {
+  return rb_eval_cmd_kw(cmd, args, NUM2INT(kw_splat));
+}
+
 VALUE kernel_spec_rb_raise(VALUE self, VALUE hash) {
   rb_hash_aset(hash, ID2SYM(rb_intern("stage")), ID2SYM(rb_intern("before")));
   if (self != Qundef)
@@ -361,6 +365,7 @@ void Init_kernel_spec(void) {
   rb_define_method(cls, "rb_frame_this_func_test_again", kernel_spec_rb_frame_this_func, 0);
   rb_define_method(cls, "rb_ensure", kernel_spec_rb_ensure, 4);
   rb_define_method(cls, "rb_eval_string", kernel_spec_rb_eval_string, 1);
+  rb_define_method(cls, "rb_eval_cmd_kw", kernel_spec_rb_eval_cmd_kw, 3);
   rb_define_method(cls, "rb_raise", kernel_spec_rb_raise, 1);
   rb_define_method(cls, "rb_throw", kernel_spec_rb_throw, 1);
   rb_define_method(cls, "rb_throw_obj", kernel_spec_rb_throw_obj, 2);
diff --git a/spec/ruby/optional/capi/kernel_spec.rb b/spec/ruby/optional/capi/kernel_spec.rb
@@ -504,6 +504,25 @@ def proc_caller
     end
   end
 
+  describe "rb_eval_cmd_kw" do
+    it "evaluates a string of ruby code" do
+      @s.rb_eval_cmd_kw("1+1", [], 0).should == 2
+    end
+
+    it "calls a proc with the supplied arguments" do
+      @s.rb_eval_cmd_kw(-> (*x) { x.map { |i| i + 1 } }, [1, 3, 7], 0).should == [2, 4, 8]
+    end
+
+    it "calls a proc with keyword arguments if kw_splat is non zero" do
+      a_proc = -> (*x, **y) {
+        res = x.map { |i| i + 1 }
+        y.each { |k, v| res << k; res << v }
+        res
+      }
+      @s.rb_eval_cmd_kw(a_proc, [1, 3, 7, {a: 1, b: 2, c: 3}], 1).should == [2, 4, 8, :a, 1, :b, 2, :c, 3]
+    end
+  end
+
   describe "rb_block_proc" do
     it "converts the implicit block into a proc" do
       proc = @s.rb_block_proc { 1+1 }
diff --git a/src/main/c/cext/call.c b/src/main/c/cext/call.c
@@ -228,3 +228,11 @@ int rb_method_boundp(VALUE klass, ID id, int ex) {
 VALUE rb_exec_recursive(VALUE (*func) (VALUE, VALUE, int), VALUE obj, VALUE arg) {
   return rb_tr_wrap(polyglot_invoke(RUBY_CEXT, "rb_exec_recursive", func, rb_tr_unwrap(obj), rb_tr_unwrap(arg)));
 }
+
+VALUE rb_eval_cmd_kw(VALUE cmd, VALUE args, int kw_splat) {
+  if (!RB_TYPE_P(cmd, T_STRING)) {
+    return RUBY_CEXT_INVOKE("rb_eval_cmd_kw", cmd, args, INT2NUM(kw_splat));
+  } else {
+    return RUBY_CEXT_INVOKE("rb_eval_string", cmd);
+  }
+}
diff --git a/src/main/java/org/truffleruby/cext/CExtNodes.java b/src/main/java/org/truffleruby/cext/CExtNodes.java
@@ -92,7 +92,13 @@
 import org.truffleruby.language.constants.LookupConstantNode;
 import org.truffleruby.language.control.BreakException;
 import org.truffleruby.language.control.BreakID;
+import org.truffleruby.language.control.DynamicReturnException;
+import org.truffleruby.language.control.LocalReturnException;
+import org.truffleruby.language.control.NextException;
+import org.truffleruby.language.control.RedoException;
+import org.truffleruby.language.control.RetryException;
 import org.truffleruby.language.control.RaiseException;
+import org.truffleruby.language.control.ThrowException;
 import org.truffleruby.language.dispatch.DispatchNode;
 import org.truffleruby.language.dispatch.LiteralCallNode;
 import org.truffleruby.language.library.RubyStringLibrary;
@@ -111,6 +117,7 @@
 import com.oracle.truffle.api.Truffle;
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.CreateCast;
+import com.oracle.truffle.api.dsl.Fallback;
 import com.oracle.truffle.api.dsl.NodeChild;
 import com.oracle.truffle.api.dsl.ReportPolymorphism;
 import com.oracle.truffle.api.dsl.Specialization;
@@ -130,6 +137,18 @@
 @CoreModule("Truffle::CExt")
 public class CExtNodes {
 
+    /* These tag values are derived from MRI source and from the Tk gem and are used to represent different control flow
+     * states under which code may exit an `rb_protect` block. The fatal tag is defined but I could not find a point
+     * where it is assigned, and am not sure it maps to anything we would use in TruffleRuby. */
+    public static final int RUBY_TAG_RETURN = 0x1;
+    public static final int RUBY_TAG_BREAK = 0x2;
+    public static final int RUBY_TAG_NEXT = 0x3;
+    public static final int RUBY_TAG_RETRY = 0x4;
+    public static final int RUBY_TAG_REDO = 0x5;
+    public static final int RUBY_TAG_RAISE = 0x6;
+    public static final int RUBY_TAG_THROW = 0x7;
+    public static final int RUBY_TAG_FATAL = 0x8;
+
     public static Pointer newNativeStringPointer(int capacity, RubyLanguage language) {
         // We need up to 4 \0 bytes for UTF-32. Always use 4 for speed rather than checking the encoding min length.
         Pointer pointer = Pointer.mallocAutoRelease(capacity + 4, language);
@@ -1355,6 +1374,31 @@ protected Object executeWithProtect(RubyProc block,
         }
     }
 
+    @CoreMethod(names = "store_exception", onSingleton = true, required = 1)
+    public abstract static class StoreException extends YieldingCoreMethodNode {
+
+        @Specialization
+        protected Object storeException(CapturedException captured) {
+            final ExtensionCallStack extensionStack = getLanguage()
+                    .getCurrentThread()
+                    .getCurrentFiber().extensionCallStack;
+            extensionStack.setException(captured);
+            return nil;
+        }
+    }
+
+    @CoreMethod(names = "retrieve_exception", onSingleton = true)
+    public abstract static class RetrieveException extends YieldingCoreMethodNode {
+
+        @Specialization
+        protected Object retrieveException() {
+            final ExtensionCallStack extensionStack = getLanguage()
+                    .getCurrentThread()
+                    .getCurrentFiber().extensionCallStack;
+            return extensionStack.getException();
+        }
+    }
+
     @CoreMethod(names = "extract_ruby_exception", onSingleton = true, required = 1)
     public abstract static class ExtractRubyException extends CoreMethodArrayArgumentsNode {
 
@@ -1370,6 +1414,66 @@ protected Object executeThrow(CapturedException captured,
         }
     }
 
+    @CoreMethod(names = "extract_tag", onSingleton = true, required = 1)
+    public abstract static class ExtractRubyTag extends CoreMethodArrayArgumentsNode {
+
+        @Specialization
+        protected int executeThrow(CapturedException captured,
+                @Cached ExtractRubyTagHelperNode helperNode) {
+            return helperNode.execute(captured.getException());
+        }
+    }
+
+    public abstract static class ExtractRubyTagHelperNode extends RubyBaseNode {
+
+        public abstract int execute(Throwable e);
+
+        @Specialization
+        protected int dynamicReturnTag(DynamicReturnException e) {
+            return RUBY_TAG_RETURN;
+        }
+
+        @Specialization
+        protected int localReturnTag(LocalReturnException e) {
+            return RUBY_TAG_RETURN;
+        }
+
+        @Specialization
+        protected int breakTag(BreakException e) {
+            return RUBY_TAG_BREAK;
+        }
+
+        @Specialization
+        protected int nextTag(NextException e) {
+            return RUBY_TAG_NEXT;
+        }
+
+        @Specialization
+        protected int retryTag(RetryException e) {
+            return RUBY_TAG_RETRY;
+        }
+
+        @Specialization
+        protected int redoTag(RedoException e) {
+            return RUBY_TAG_REDO;
+        }
+
+        @Specialization
+        protected int raiseTag(RaiseException e) {
+            return RUBY_TAG_RAISE;
+        }
+
+        @Specialization
+        protected int throwTag(ThrowException e) {
+            return RUBY_TAG_THROW;
+        }
+
+        @Fallback
+        protected int noTag(Throwable e) {
+            return 0;
+        }
+    }
+
     @CoreMethod(names = "raise_exception", onSingleton = true, required = 1)
     public abstract static class RaiseExceptionNode extends CoreMethodArrayArgumentsNode {
 
diff --git a/src/main/java/org/truffleruby/core/MarkingService.java b/src/main/java/org/truffleruby/core/MarkingService.java
@@ -14,6 +14,7 @@
 
 import org.truffleruby.RubyContext;
 import org.truffleruby.RubyLanguage;
+import org.truffleruby.cext.CapturedException;
 import org.truffleruby.cext.ValueWrapperManager;
 import org.truffleruby.core.array.ArrayUtils;
 import org.truffleruby.core.queue.UnsizedQueue;
@@ -118,6 +119,7 @@ protected static class ExtensionCallStackEntry {
         protected final boolean keywordsGiven;
         protected Object specialVariables;
         protected final Object block;
+        protected CapturedException capturedException;
 
         protected ExtensionCallStackEntry(
                 ExtensionCallStackEntry previous,
@@ -128,6 +130,7 @@ protected ExtensionCallStackEntry(
             this.keywordsGiven = keywordsGiven;
             this.specialVariables = specialVariables;
             this.block = block;
+            this.capturedException = null;
         }
     }
 
@@ -165,6 +168,14 @@ public void setSpecialVariables(Object specialVariables) {
             current.specialVariables = specialVariables;
         }
 
+        public CapturedException getException() {
+            return current.capturedException;
+        }
+
+        public void setException(CapturedException capturedException) {
+            current.capturedException = capturedException;
+        }
+
         public Object getBlock() {
             return current.block;
         }
diff --git a/src/main/java/org/truffleruby/core/encoding/RubyEncoding.java b/src/main/java/org/truffleruby/core/encoding/RubyEncoding.java
diff --git a/src/main/java/org/truffleruby/core/support/TypeNodes.java b/src/main/java/org/truffleruby/core/support/TypeNodes.java
