[GR-45459] Update to FFI 1.15.5

eregon · eregon · commit c6628ba076fc · 2023-04-19T13:09:25.000Z
PullRequest: truffleruby/3761
diff --git a/doc/contributor/ffi.md b/doc/contributor/ffi.md
@@ -27,18 +27,12 @@ require C extension support for gems using FFI.
 
 ## Synchronization
 
-I use a branch `truffleruby-specs-$FFI_RELEASE` on my fork https://github.com/eregon/ffi
-which keeps our modifications on top of the FFI release tag we are based on.
-`tool/import-ffi.sh` from the latest of these branches should synchronize cleanly.
+`tool/import-ffi.sh` from the corresponding FFI version should synchronize cleanly.
 
-In general, changes are done in TruffleRuby first, then I add them to my local `ffi` repository
-in the `truffleruby-specs-$FFI_RELEASE` branch with `git cherry-pick`.
-This requires `truffleruby` to be added as a remote to the `ffi` repo:
-```bash
-git remote add truffleruby ../truffleruby-ws/truffleruby
-```
+In general, changes are done in `ffi` first, and I copy them to truffleruby via `tool/import-ffi.sh` while working on them.
+We should upstream all changes we do to `ffi`.
 
-From there we should of course upstream as many changes as possible to minimize the diff.
+The only diff we should have is in `src/main/ruby/truffleruby/core/truffle/ffi/pointer_extra.rb` and it should remain small.
 
 ## Running Specs in Upstream FFI Repository
 
diff --git a/lib/truffle/ffi/library.rb b/lib/truffle/ffi/library.rb
@@ -84,7 +84,7 @@ module Library
     # @raise {RuntimeError} if +mod+ is not a Module
     # Test if extended object is a Module. If not, raise RuntimeError.
     def self.extended(mod)
-      raise RuntimeError.new("must only be extended by module") unless mod.kind_of?(Module)
+      raise RuntimeError.new("must only be extended by module") unless mod.kind_of?(::Module)
     end
 
 
@@ -126,7 +126,7 @@ def ffi_lib(*names)
               else
                 # TODO better library lookup logic
                 unless libname.start_with?("/") || FFI::Platform.windows?
-                  path = ['/usr/lib/','/usr/local/lib/','/opt/local/lib/'].find do |pth|
+                  path = ['/usr/lib/','/usr/local/lib/','/opt/local/lib/', '/opt/homebrew/lib/'].find do |pth|
                     File.exist?(pth + libname)
                   end
                   if path
diff --git a/lib/truffle/ffi/platform.rb b/lib/truffle/ffi/platform.rb
@@ -132,11 +132,8 @@ def self.is_os(os)
     end
 
     LIBC = if IS_WINDOWS
-      if RbConfig::CONFIG['host_os'] =~ /mingw/i
-        RbConfig::CONFIG['RUBY_SO_NAME'].split('-')[-2] + '.dll'
-      else
-        "ucrtbase.dll"
-      end
+      crtname = RbConfig::CONFIG["RUBY_SO_NAME"][/msvc\w+/] || 'ucrtbase'
+      "#{crtname}.dll"
     elsif IS_GNU
       GNU_LIBC
     elsif OS == 'cygwin'
diff --git a/lib/truffle/ffi/tools/const_generator.rb b/lib/truffle/ffi/tools/const_generator.rb
@@ -105,9 +105,10 @@ def const(name, format = nil, cast = '', ruby_name = nil, converter = nil,
     # @return [nil]
     # @raise if a constant is missing and +:required+ was set to +true+ (see {#initialize})
     def calculate(options = {})
-      binary = File.join Dir.tmpdir, "rb_const_gen_bin_#{Process.pid}"
+      binary_path = nil
 
       Tempfile.open("#{@prefix}.const_generator") do |f|
+        binary_path = f.path + ".bin"
         @includes.each do |inc|
           f.puts "#include <#{inc}>"
         end
@@ -124,16 +125,17 @@ def calculate(options = {})
         f.puts "\n\treturn 0;\n}"
         f.flush
 
-        output = `gcc #{options[:cppflags]} -D_DARWIN_USE_64_BIT_INODE -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -x c -Wall -Werror #{f.path} -o #{binary} 2>&1`
+        cc = ENV['CC'] || 'gcc'
+        output = `#{cc} #{options[:cppflags]} -D_DARWIN_USE_64_BIT_INODE -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -x c -Wall -Werror #{f.path} -o #{binary_path} 2>&1`
 
         unless $?.success? then
           output = output.split("\n").map { |l| "\t#{l}" }.join "\n"
           raise "Compilation error generating constants #{@prefix}:\n#{output}"
         end
       end
 
-      output = `#{binary}`
-      File.unlink(binary + (FFI::Platform.windows? ? ".exe" : ""))
+      output = `#{binary_path}`
+      File.unlink(binary_path + (FFI::Platform.windows? ? ".exe" : ""))
       output.each_line do |line|
         line =~ /^(\S+)\s(.*)$/
         const = @constants[$1]
diff --git a/lib/truffle/ffi/tools/struct_generator.rb b/lib/truffle/ffi/tools/struct_generator.rb
@@ -82,7 +82,8 @@ def calculate(options = {})
         f.puts "\n  return 0;\n}"
         f.flush
 
-        output = `gcc #{options[:cppflags]} #{options[:cflags]} -D_DARWIN_USE_64_BIT_INODE -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -x c -Wall -Werror #{f.path} -o #{binary} 2>&1`
+        cc = ENV['CC'] || 'gcc'
+        output = `#{cc} #{options[:cppflags]} #{options[:cflags]} -D_DARWIN_USE_64_BIT_INODE -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -x c -Wall -Werror #{f.path} -o #{binary} 2>&1`
 
         unless $?.success? then
           @found = false
diff --git a/lib/truffle/ffi/variadic.rb b/lib/truffle/ffi/variadic.rb
@@ -32,15 +32,6 @@
 
 module FFI
   class VariadicInvoker
-    def init(arg_types, type_map)
-      @fixed = Array.new
-      @type_map = type_map
-      arg_types.each_with_index do |type, i|
-        @fixed << type unless type == Type::VARARGS
-      end
-    end
-
-
     def call(*args, &block)
       param_types = Array.new(@fixed)
       param_values = Array.new
diff --git a/lib/truffle/ffi/version.rb b/lib/truffle/ffi/version.rb
@@ -1,3 +1,3 @@
 module FFI
-  VERSION = '1.14.2'
+  VERSION = '1.15.5'
 end
diff --git a/spec/ffi/Gemfile.lock b/spec/ffi/Gemfile.lock
@@ -21,6 +21,3 @@ PLATFORMS
 
 DEPENDENCIES
   rspec (~> 3.0)
-
-BUNDLED WITH
-   2.1.4
diff --git a/spec/ffi/async_callback_spec.rb b/spec/ffi/async_callback_spec.rb
@@ -19,19 +19,35 @@ module LibTest
     skip "not yet supported on TruffleRuby" if RUBY_ENGINE == "truffleruby"
     v = 0xdeadbeef
     called = false
-    cb = Proc.new {|i| v = i; called = true }
+    cb = Proc.new {|i| v = i; called = Thread.current }
     LibTest.testAsyncCallback(cb, 0x7fffffff)
-    expect(called).to be true
+    expect(called).to be_kind_of(Thread)
+    expect(called).to_not eq(Thread.current)
     expect(v).to eq(0x7fffffff)
   end
 
   it "called a second time" do
     skip "not yet supported on TruffleRuby" if RUBY_ENGINE == "truffleruby"
-    v = 0xdeadbeef
-    called = false
-    cb = Proc.new {|i| v = i; called = true }
-    LibTest.testAsyncCallback(cb, 0x7fffffff)
-    expect(called).to be true
-    expect(v).to eq(0x7fffffff)
+    v = 1
+    th1 = th2 = false
+    LibTest.testAsyncCallback(2) { |i| v += i; th1 = Thread.current }
+    LibTest.testAsyncCallback(3) { |i| v += i; th2 = Thread.current }
+    expect(th1).to be_kind_of(Thread)
+    expect(th2).to be_kind_of(Thread)
+    expect(th1).to_not eq(Thread.current)
+    expect(th2).to_not eq(Thread.current)
+    expect(th1).to_not eq(th2)
+    expect(v).to eq(6)
+  end
+
+  it "sets the name of the thread that runs the callback" do
+    skip "not yet supported on TruffleRuby" if RUBY_ENGINE == "truffleruby"
+    skip "not yet supported on JRuby" if RUBY_ENGINE == "jruby"
+
+    callback_runner_thread = nil
+
+    LibTest.testAsyncCallback(proc { callback_runner_thread = Thread.current }, 0)
+
+    expect(callback_runner_thread.name).to eq("FFI Callback Runner")
   end
 end
diff --git a/spec/ffi/callback_spec.rb b/spec/ffi/callback_spec.rb
@@ -49,6 +49,8 @@ class S8F32S32 < FFI::Struct
       callback :cbVrU32, [ ], :uint
       callback :cbVrL, [ ], :long
       callback :cbVrUL, [ ], :ulong
+      callback :cbVrF, [ ], :float
+      callback :cbVrD, [ ], :double
       callback :cbVrS64, [ ], :long_long
       callback :cbVrU64, [ ], :ulong_long
       callback :cbVrP, [], :pointer
@@ -70,6 +72,8 @@ class S8F32S32 < FFI::Struct
       attach_function :testCallbackVrU16, :testClosureVrS, [ :cbVrU16 ], :ushort
       attach_function :testCallbackVrS32, :testClosureVrI, [ :cbVrS32 ], :int
       attach_function :testCallbackVrU32, :testClosureVrI, [ :cbVrU32 ], :uint
+      attach_function :testCallbackVrF, :testClosureVrF, [ :cbVrF ], :float
+      attach_function :testCallbackVrD, :testClosureVrD, [ :cbVrD ], :double
       attach_function :testCallbackVrL, :testClosureVrL, [ :cbVrL ], :long
       attach_function :testCallbackVrZ, :testClosureVrZ, [ :cbVrZ ], :bool
       attach_function :testCallbackVrUL, :testClosureVrL, [ :cbVrUL ], :ulong
@@ -86,7 +90,6 @@ class S8F32S32 < FFI::Struct
       attach_variable :pVrS8, :gvar_pointer, :pointer
       attach_function :testGVarCallbackVrS8, :testClosureVrB, [ :pointer ], :char
       attach_function :testOptionalCallbackCrV, :testOptionalClosureBrV, [ :cbCrV, :char ], :void
-
     end
 
     it "returning :char (0)" do
@@ -261,6 +264,14 @@ class S8F32S32 < FFI::Struct
       expect(LibTest.testCallbackVrZ { true }).to be true
     end
 
+    it "returning float" do
+      expect(LibTest.testCallbackVrF { 1.234567890123456789 }).to be_within(1E-7).of(1.234567890123456789)
+    end
+
+    it "returning double" do
+      expect(LibTest.testCallbackVrD { 1.234567890123456789 }).to be_within(1E-15).of(1.234567890123456789)
+    end
+
     it "returning :pointer (nil)" do
       expect(LibTest.testCallbackVrP { nil }).to be_null
     end
diff --git a/spec/ffi/ffi_spec.rb b/spec/ffi/ffi_spec.rb
@@ -28,7 +28,7 @@
 
   describe "VERSION" do
     it "should be kind of version" do
-      expect( FFI::VERSION ).to match(/^\d+\.\d+.\d+$/)
+      expect( FFI::VERSION ).to match(/^\d+\.\d+.\d+/)
     end
   end
 end
diff --git a/spec/ffi/fixtures/ClosureTest.c b/spec/ffi/fixtures/ClosureTest.c
@@ -6,13 +6,38 @@
 
 #include <stdlib.h>
 #include <stdbool.h>
+#include <stdarg.h>
 #ifndef _WIN32
 # include <pthread.h>
 #else
 # include <windows.h>
 # include <process.h>
 #endif
 
+double testClosureVrDva(double d, ...) {
+    va_list args;
+    double (*closure)(void);
+
+    va_start(args, d);
+    closure = va_arg(args, double (*)(void));
+    va_end(args);
+
+    return d + closure();
+}
+
+long testClosureVrILva(int i, long l, ...) {
+    va_list args;
+    int (*cl1)(int);
+    long (*cl2)(long);
+
+    va_start(args, l);
+    cl1 = va_arg(args, int (*)(int));
+    cl2 = va_arg(args, long (*)(long));
+    va_end(args);
+
+    return cl1(i) + cl2(l);
+}
+
 #define R(T, rtype) rtype testClosureVr##T(rtype (*closure)(void)) { \
     return closure != NULL ? (*closure)() : (rtype) 0; \
 }
diff --git a/spec/ffi/fixtures/FunctionTest.c b/spec/ffi/fixtures/FunctionTest.c
@@ -6,6 +6,7 @@
 
 #ifdef _WIN32
 #include <windows.h>
+#include <process.h>
 #endif
 
 #ifndef _WIN32
@@ -115,17 +116,26 @@ static void* asyncThreadCall(void *data)
     return NULL;
 }
 
+#ifdef _WIN32
+static void
+asyncThreadCall_win32(void *arg)
+{
+    asyncThreadCall(arg);
+}
+#endif
+
 void testAsyncCallback(void (*fn)(int), int value)
 {
-#ifndef _WIN32
-    pthread_t t;
     struct async_data d;
     d.fn = fn;
     d.value = value;
+#ifndef _WIN32
+    pthread_t t;
     pthread_create(&t, NULL, asyncThreadCall, &d);
     pthread_join(t, NULL);
 #else
-    (*fn)(value);
+    HANDLE hThread = (HANDLE) _beginthread(asyncThreadCall_win32, 0, &d);
+    WaitForSingleObject(hThread, INFINITE);
 #endif
 }
 
diff --git a/spec/ffi/fork_spec.rb b/spec/ffi/fork_spec.rb
@@ -8,32 +8,59 @@
 if Process.respond_to?(:fork)
   describe "Callback in conjunction with fork()" do
 
-    module LibTest
+    libtest = Module.new do
       extend FFI::Library
       ffi_lib TestLibrary::PATH
 
       callback :cbVrL, [ ], :long
       attach_function :testCallbackVrL, :testClosureVrL, [ :cbVrL ], :long
+      AsyncIntCallback = callback [ :int ], :void
+      attach_function :testAsyncCallback, [ AsyncIntCallback, :int ], :void, blocking: true
     end
 
     it "works with forked process and GC" do
-      expect(LibTest.testCallbackVrL { 12345 }).to eq(12345)
+      expect(libtest.testCallbackVrL { 12345 }).to eq(12345)
       fork do
-        expect(LibTest.testCallbackVrL { 12345 }).to eq(12345)
+        expect(libtest.testCallbackVrL { 12345 }).to eq(12345)
+        Process.exit 42
       end
-      expect(LibTest.testCallbackVrL { 12345 }).to eq(12345)
+      expect(libtest.testCallbackVrL { 12345 }).to eq(12345)
       GC.start
+
+      expect(Process.wait2[1].exitstatus).to eq(42)
     end
 
     it "works with forked process and free()" do
       cbf = FFI::Function.new(FFI::Type::LONG, []) { 234 }
 
       fork do
         cbf.free
+        Process.exit 43
       end
 
-      expect(LibTest.testCallbackVrL(cbf)).to eq(234)
+      expect(libtest.testCallbackVrL(cbf)).to eq(234)
       cbf.free
+
+      expect(Process.wait2[1].exitstatus).to eq(43)
+    end
+
+    def run_async_callback(libtest)
+      recv = nil
+      libtest.testAsyncCallback(proc { |r| recv = r }, 23)
+      expect(recv).to eq(23)
+    end
+
+    it "async thread dispatch works after forking" do
+      run_async_callback(libtest)
+
+      fork do
+        run_async_callback(libtest)
+        Process.exit 44
+      end
+
+      run_async_callback(libtest)
+
+      expect(Process.wait2[1].exitstatus).to eq(44)
     end
   end
 end
diff --git a/spec/ffi/function_spec.rb b/spec/ffi/function_spec.rb
@@ -21,6 +21,15 @@ module LibTest
     expect(fn.call).to eql 5
   end
 
+  context 'when called with a block' do
+    it 'creates a thread for dispatching callbacks and sets its name' do
+      skip 'this is MRI-specific' if RUBY_ENGINE == 'truffleruby' || RUBY_ENGINE == 'jruby'
+      FFI::Function.new(:int, []) { 5 } # Trigger initialization
+
+      expect(Thread.list.map(&:name)).to include('FFI Callback Dispatcher')
+    end
+  end
+
   it 'raises an error when passing a wrong signature' do
     expect { FFI::Function.new([], :int).new { } }.to raise_error TypeError
   end
diff --git a/spec/ffi/variadic_spec.rb b/spec/ffi/variadic_spec.rb
diff --git a/test/truffle/gems/ffi.sh b/test/truffle/gems/ffi.sh
diff --git a/tool/import-ffi.sh b/tool/import-ffi.sh
