Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use JRuby implementation for TruffleRuby #149

Merged
merged 6 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@ jobs:
- '3.2'
- debug
- jruby
- truffleruby
include:
- { os: windows-latest , ruby: mingw }
- { os: windows-latest , ruby: mswin }
exclude:
- { os: macos-14 , ruby: '2.5' }
- { os: windows-latest , ruby: '3.0' }
- { os: windows-latest , ruby: debug }
- { os: windows-latest , ruby: truffleruby }

steps:
- uses: actions/checkout@v4
Expand Down
2 changes: 1 addition & 1 deletion ext/fiddle/extconf.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true
require 'mkmf'

if RUBY_ENGINE == "jruby"
unless RUBY_ENGINE == "ruby"
File.write('Makefile', dummy_makefile("").join)
return
end
Expand Down
3 changes: 1 addition & 2 deletions fiddle.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,10 @@ Gem::Specification.new do |spec|
"lib/fiddle.rb",
"lib/fiddle/closure.rb",
"lib/fiddle/cparser.rb",
"lib/fiddle/ffi_backend.rb",
"lib/fiddle/function.rb",
"lib/fiddle/import.rb",
"lib/fiddle/jruby.rb",
"lib/fiddle/pack.rb",
"lib/fiddle/ruby.rb",
"lib/fiddle/struct.rb",
"lib/fiddle/types.rb",
"lib/fiddle/value.rb",
Expand Down
6 changes: 5 additions & 1 deletion lib/fiddle.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
# frozen_string_literal: true

require "fiddle/#{RUBY_ENGINE}"
if RUBY_ENGINE == 'ruby'
require 'fiddle.so'
else
require 'fiddle/ffi_backend'
end
require 'fiddle/closure'
require 'fiddle/function'
require 'fiddle/version'
Expand Down
145 changes: 79 additions & 66 deletions lib/fiddle/jruby.rb → lib/fiddle/ffi_backend.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# This is part of JRuby's FFI-based fiddle implementation.
# This is based on JRuby's FFI-based fiddle implementation.

require 'ffi'

Expand Down Expand Up @@ -74,7 +74,7 @@ module Types

WINDOWS = FFI::Platform.windows?

module JRuby
module FFIBackend
FFITypes = {
'c' => FFI::Type::INT8,
'h' => FFI::Type::INT16,
Expand Down Expand Up @@ -104,16 +104,16 @@ module JRuby
Types::VARIADIC => FFI::Type::Builtin::VARARGS,
}

def self.__ffi_type__(dl_type)
if dl_type.is_a?(Symbol)
dl_type = Types.const_get(dl_type.to_s.upcase)
def self.to_ffi_type(fiddle_type)
if fiddle_type.is_a?(Symbol)
fiddle_type = Types.const_get(fiddle_type.to_s.upcase)
end
if !dl_type.is_a?(Integer) && dl_type.respond_to?(:to_int)
dl_type = dl_type.to_int
if !fiddle_type.is_a?(Integer) && fiddle_type.respond_to?(:to_int)
fiddle_type = fiddle_type.to_int
end
ffi_type = FFITypes[dl_type]
ffi_type = FFITypes[-dl_type] if ffi_type.nil? && dl_type.is_a?(Integer) && dl_type < 0
raise TypeError.new("cannot convert #{dl_type} to ffi") unless ffi_type
ffi_type = FFITypes[fiddle_type]
ffi_type = FFITypes[-fiddle_type] if ffi_type.nil? && fiddle_type.is_a?(Integer) && fiddle_type < 0
raise TypeError.new("cannot convert #{fiddle_type} to ffi") unless ffi_type
ffi_type
end
end
Expand All @@ -133,8 +133,8 @@ def initialize(ptr, args, return_type, abi = DEFAULT, kwargs = nil)
@ptr, @args, @return_type, @abi = ptr, args, return_type, abi
raise TypeError.new "invalid argument types" unless args.is_a?(Array)

ffi_return_type = Fiddle::JRuby::__ffi_type__(@return_type)
ffi_args = @args.map { |t| Fiddle::JRuby.__ffi_type__(t) }
ffi_return_type = Fiddle::FFIBackend.to_ffi_type(@return_type)
ffi_args = @args.map { |t| Fiddle::FFIBackend.to_ffi_type(t) }
pointer = FFI::Pointer.new(ptr.to_i)
options = {convention: @abi}
if ffi_args.last == FFI::Type::Builtin::VARARGS
Expand All @@ -149,14 +149,25 @@ def initialize(ptr, args, return_type, abi = DEFAULT, kwargs = nil)
end
end

def call(*args, &block);
def call(*args, &block)
if @function.is_a?(FFI::VariadicInvoker)
n_fixed_args = @args.size - 1
n_fixed_args.step(args.size - 1, 2) do |i|
if args[i] == :const_string || args[i] == Types::CONST_STRING
args[i + 1] = String.try_convert(args[i + 1]) || args[i + 1]
end
args[i] = Fiddle::JRuby.__ffi_type__(args[i])
args[i] = Fiddle::FFIBackend.to_ffi_type(args[i])
end
else
args.map! do |arg|
if arg.respond_to?(:to_ptr)
begin
arg = arg.to_ptr
end until arg.is_a?(FFI::Pointer) || !arg.respond_to?(:to_ptr)
arg
else
arg
end
end
end
result = @function.call(*args, &block)
Expand All @@ -170,19 +181,21 @@ def initialize(ret, args, abi = Function::DEFAULT)
raise TypeError.new "invalid argument types" unless args.is_a?(Array)

@ctype, @args = ret, args
ffi_args = @args.map { |t| Fiddle::JRuby.__ffi_type__(t) }
ffi_args = @args.map { |t| Fiddle::FFIBackend.to_ffi_type(t) }
if ffi_args.size == 1 && ffi_args[0] == FFI::Type::Builtin::VOID
ffi_args = []
end
@function = FFI::Function.new(
Fiddle::JRuby.__ffi_type__(@ctype),
ffi_args,
self,
:convention => abi
)
return_type = Fiddle::FFIBackend.to_ffi_type(@ctype)
raise "#{self.class} must implement #call" unless respond_to?(:call)
callable = method(:call)
@function = FFI::Function.new(return_type, ffi_args, callable, convention: abi)
@freed = false
end

def to_ptr
@function
end

def to_i
@function.to_i
end
Expand Down Expand Up @@ -550,51 +563,51 @@ def cleared?
RUBY_FREE = Fiddle::Pointer::LibC::FREE.address
NULL = Fiddle::Pointer.new(0)

ALIGN_VOIDP = Fiddle::JRuby::FFITypes[Types::VOIDP].alignment
ALIGN_CHAR = Fiddle::JRuby::FFITypes[Types::CHAR].alignment
ALIGN_SHORT = Fiddle::JRuby::FFITypes[Types::SHORT].alignment
ALIGN_INT = Fiddle::JRuby::FFITypes[Types::INT].alignment
ALIGN_LONG = Fiddle::JRuby::FFITypes[Types::LONG].alignment
ALIGN_LONG_LONG = Fiddle::JRuby::FFITypes[Types::LONG_LONG].alignment
ALIGN_INT8_T = Fiddle::JRuby::FFITypes[Types::INT8_T].alignment
ALIGN_INT16_T = Fiddle::JRuby::FFITypes[Types::INT16_T].alignment
ALIGN_INT32_T = Fiddle::JRuby::FFITypes[Types::INT32_T].alignment
ALIGN_INT64_T = Fiddle::JRuby::FFITypes[Types::INT64_T].alignment
ALIGN_FLOAT = Fiddle::JRuby::FFITypes[Types::FLOAT].alignment
ALIGN_DOUBLE = Fiddle::JRuby::FFITypes[Types::DOUBLE].alignment
ALIGN_BOOL = Fiddle::JRuby::FFITypes[Types::BOOL].alignment
ALIGN_SIZE_T = Fiddle::JRuby::FFITypes[Types::SIZE_T].alignment
ALIGN_VOIDP = Fiddle::FFIBackend::FFITypes[Types::VOIDP].alignment
ALIGN_CHAR = Fiddle::FFIBackend::FFITypes[Types::CHAR].alignment
ALIGN_SHORT = Fiddle::FFIBackend::FFITypes[Types::SHORT].alignment
ALIGN_INT = Fiddle::FFIBackend::FFITypes[Types::INT].alignment
ALIGN_LONG = Fiddle::FFIBackend::FFITypes[Types::LONG].alignment
ALIGN_LONG_LONG = Fiddle::FFIBackend::FFITypes[Types::LONG_LONG].alignment
ALIGN_INT8_T = Fiddle::FFIBackend::FFITypes[Types::INT8_T].alignment
ALIGN_INT16_T = Fiddle::FFIBackend::FFITypes[Types::INT16_T].alignment
ALIGN_INT32_T = Fiddle::FFIBackend::FFITypes[Types::INT32_T].alignment
ALIGN_INT64_T = Fiddle::FFIBackend::FFITypes[Types::INT64_T].alignment
ALIGN_FLOAT = Fiddle::FFIBackend::FFITypes[Types::FLOAT].alignment
ALIGN_DOUBLE = Fiddle::FFIBackend::FFITypes[Types::DOUBLE].alignment
ALIGN_BOOL = Fiddle::FFIBackend::FFITypes[Types::BOOL].alignment
ALIGN_SIZE_T = Fiddle::FFIBackend::FFITypes[Types::SIZE_T].alignment
ALIGN_SSIZE_T = ALIGN_SIZE_T
ALIGN_PTRDIFF_T = Fiddle::JRuby::FFITypes[Types::PTRDIFF_T].alignment
ALIGN_INTPTR_T = Fiddle::JRuby::FFITypes[Types::INTPTR_T].alignment
ALIGN_UINTPTR_T = Fiddle::JRuby::FFITypes[Types::UINTPTR_T].alignment

SIZEOF_VOIDP = Fiddle::JRuby::FFITypes[Types::VOIDP].size
SIZEOF_CHAR = Fiddle::JRuby::FFITypes[Types::CHAR].size
SIZEOF_UCHAR = Fiddle::JRuby::FFITypes[Types::UCHAR].size
SIZEOF_SHORT = Fiddle::JRuby::FFITypes[Types::SHORT].size
SIZEOF_USHORT = Fiddle::JRuby::FFITypes[Types::USHORT].size
SIZEOF_INT = Fiddle::JRuby::FFITypes[Types::INT].size
SIZEOF_UINT = Fiddle::JRuby::FFITypes[Types::UINT].size
SIZEOF_LONG = Fiddle::JRuby::FFITypes[Types::LONG].size
SIZEOF_ULONG = Fiddle::JRuby::FFITypes[Types::ULONG].size
SIZEOF_LONG_LONG = Fiddle::JRuby::FFITypes[Types::LONG_LONG].size
SIZEOF_ULONG_LONG = Fiddle::JRuby::FFITypes[Types::ULONG_LONG].size
SIZEOF_INT8_T = Fiddle::JRuby::FFITypes[Types::INT8_T].size
SIZEOF_UINT8_T = Fiddle::JRuby::FFITypes[Types::UINT8_T].size
SIZEOF_INT16_T = Fiddle::JRuby::FFITypes[Types::INT16_T].size
SIZEOF_UINT16_T = Fiddle::JRuby::FFITypes[Types::UINT16_T].size
SIZEOF_INT32_T = Fiddle::JRuby::FFITypes[Types::INT32_T].size
SIZEOF_UINT32_T = Fiddle::JRuby::FFITypes[Types::UINT32_T].size
SIZEOF_INT64_T = Fiddle::JRuby::FFITypes[Types::INT64_T].size
SIZEOF_UINT64_T = Fiddle::JRuby::FFITypes[Types::UINT64_T].size
SIZEOF_FLOAT = Fiddle::JRuby::FFITypes[Types::FLOAT].size
SIZEOF_DOUBLE = Fiddle::JRuby::FFITypes[Types::DOUBLE].size
SIZEOF_BOOL = Fiddle::JRuby::FFITypes[Types::BOOL].size
SIZEOF_SIZE_T = Fiddle::JRuby::FFITypes[Types::SIZE_T].size
ALIGN_PTRDIFF_T = Fiddle::FFIBackend::FFITypes[Types::PTRDIFF_T].alignment
ALIGN_INTPTR_T = Fiddle::FFIBackend::FFITypes[Types::INTPTR_T].alignment
ALIGN_UINTPTR_T = Fiddle::FFIBackend::FFITypes[Types::UINTPTR_T].alignment

SIZEOF_VOIDP = Fiddle::FFIBackend::FFITypes[Types::VOIDP].size
SIZEOF_CHAR = Fiddle::FFIBackend::FFITypes[Types::CHAR].size
SIZEOF_UCHAR = Fiddle::FFIBackend::FFITypes[Types::UCHAR].size
SIZEOF_SHORT = Fiddle::FFIBackend::FFITypes[Types::SHORT].size
SIZEOF_USHORT = Fiddle::FFIBackend::FFITypes[Types::USHORT].size
SIZEOF_INT = Fiddle::FFIBackend::FFITypes[Types::INT].size
SIZEOF_UINT = Fiddle::FFIBackend::FFITypes[Types::UINT].size
SIZEOF_LONG = Fiddle::FFIBackend::FFITypes[Types::LONG].size
SIZEOF_ULONG = Fiddle::FFIBackend::FFITypes[Types::ULONG].size
SIZEOF_LONG_LONG = Fiddle::FFIBackend::FFITypes[Types::LONG_LONG].size
SIZEOF_ULONG_LONG = Fiddle::FFIBackend::FFITypes[Types::ULONG_LONG].size
SIZEOF_INT8_T = Fiddle::FFIBackend::FFITypes[Types::INT8_T].size
SIZEOF_UINT8_T = Fiddle::FFIBackend::FFITypes[Types::UINT8_T].size
SIZEOF_INT16_T = Fiddle::FFIBackend::FFITypes[Types::INT16_T].size
SIZEOF_UINT16_T = Fiddle::FFIBackend::FFITypes[Types::UINT16_T].size
SIZEOF_INT32_T = Fiddle::FFIBackend::FFITypes[Types::INT32_T].size
SIZEOF_UINT32_T = Fiddle::FFIBackend::FFITypes[Types::UINT32_T].size
SIZEOF_INT64_T = Fiddle::FFIBackend::FFITypes[Types::INT64_T].size
SIZEOF_UINT64_T = Fiddle::FFIBackend::FFITypes[Types::UINT64_T].size
SIZEOF_FLOAT = Fiddle::FFIBackend::FFITypes[Types::FLOAT].size
SIZEOF_DOUBLE = Fiddle::FFIBackend::FFITypes[Types::DOUBLE].size
SIZEOF_BOOL = Fiddle::FFIBackend::FFITypes[Types::BOOL].size
SIZEOF_SIZE_T = Fiddle::FFIBackend::FFITypes[Types::SIZE_T].size
SIZEOF_SSIZE_T = SIZEOF_SIZE_T
SIZEOF_PTRDIFF_T = Fiddle::JRuby::FFITypes[Types::PTRDIFF_T].size
SIZEOF_INTPTR_T = Fiddle::JRuby::FFITypes[Types::INTPTR_T].size
SIZEOF_UINTPTR_T = Fiddle::JRuby::FFITypes[Types::UINTPTR_T].size
SIZEOF_CONST_STRING = Fiddle::JRuby::FFITypes[Types::VOIDP].size
SIZEOF_PTRDIFF_T = Fiddle::FFIBackend::FFITypes[Types::PTRDIFF_T].size
SIZEOF_INTPTR_T = Fiddle::FFIBackend::FFITypes[Types::INTPTR_T].size
SIZEOF_UINTPTR_T = Fiddle::FFIBackend::FFITypes[Types::UINTPTR_T].size
SIZEOF_CONST_STRING = Fiddle::FFIBackend::FFITypes[Types::VOIDP].size
end
1 change: 0 additions & 1 deletion lib/fiddle/ruby.rb

This file was deleted.

4 changes: 4 additions & 0 deletions test/fiddle/helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ def teardown
end
end

def ffi_backend?
RUBY_ENGINE != 'ruby'
end

def under_gc_stress
stress, GC.stress = GC.stress, true
yield
Expand Down
11 changes: 8 additions & 3 deletions test/fiddle/test_closure.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ def call thing
end

def test_const_string
if RUBY_ENGINE == "jruby"
if ffi_backend?
omit("Closure with :const_string works but " +
"Function with :const_string doesn't work with JRuby")
"Function with :const_string doesn't work with FFI backend")
end

closure_class = Class.new(Closure) do
Expand Down Expand Up @@ -119,9 +119,14 @@ def test_memsize_ruby_dev_42480
end

require 'objspace'
closure_class = Class.new(Closure) do
def call
10
end
end
n = 10000
n.times do
Closure.create(:int, [:void]) do |closure|
closure_class.create(:int, [:void]) do |closure|
ObjectSpace.memsize_of(closure)
end
end
Expand Down
12 changes: 10 additions & 2 deletions test/fiddle/test_fiddle.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@

class TestFiddle < Fiddle::TestCase
def test_nil_true_etc
if RUBY_ENGINE == "jruby"
omit("Fiddle::Q* aren't supported with JRuby")
if ffi_backend?
omit("Fiddle::Q* aren't supported with FFI backend")
end

assert_equal Fiddle::Qtrue, Fiddle.dlwrap(true)
Expand All @@ -30,6 +30,10 @@ def test_dlopen_linker_script_input_linux
if Dir.glob("/usr/lib/*/libncurses.so").empty?
omit("libncurses.so is needed")
end
if ffi_backend?
omit("Fiddle::Handle#file_name doesn't exist in FFI backend")
end

# libncurses.so uses INPUT() on Debian GNU/Linux
# $ cat /usr/lib/x86_64-linux-gnu/libncurses.so
# INPUT(libncurses.so.6 -ltinfo)
Expand All @@ -44,6 +48,10 @@ def test_dlopen_linker_script_input_linux

def test_dlopen_linker_script_group_linux
omit("This is only for Linux") unless RUBY_PLATFORM.match?("linux")
if ffi_backend?
omit("Fiddle::Handle#file_name doesn't exist in FFI backend")
end

# libc.so uses GROUP() on Debian GNU/Linux
# $ cat /usr/lib/x86_64-linux-gnu/libc.so
# /* GNU ld script
Expand Down
27 changes: 18 additions & 9 deletions test/fiddle/test_func.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ def test_strtod
end

def test_qsort1
if RUBY_ENGINE == "jruby"
omit("The untouched sanity check is broken on JRuby: https://github.com/jruby/jruby/issues/8365")
end

closure_class = Class.new(Closure) do
def call(x, y)
Pointer.new(x)[0] <=> Pointer.new(y)[0]
Expand All @@ -74,27 +78,32 @@ def call(x, y)
qsort = Function.new(@libc['qsort'],
[TYPE_VOIDP, TYPE_SIZE_T, TYPE_SIZE_T, TYPE_VOIDP],
TYPE_VOID)
buff = "9341"
untouched = "9341"
buff = +"9341"
qsort.call(buff, buff.size, 1, callback)
assert_equal("1349", buff)

bug4929 = '[ruby-core:37395]'
buff = "9341"
buff = +"9341"
under_gc_stress do
qsort.call(buff, buff.size, 1, callback)
end
assert_equal("1349", buff, bug4929)

# Ensure the test didn't mutate String literals
assert_equal("93" + "41", untouched)
Copy link
Member

@eregon eregon Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the added + above this was failing, which meant Fiddle tests were permanently changing the meaning of all "9341" frozen string literals.

Interestingly this sanity check actually fails on JRuby, I'll file an issue there: jruby/jruby#8365

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JRuby issue has been fixed on master and will be in JRuby 9.4.9.0.

We won't be updating 9.4 to use the fiddle gem in any case, but the test could be un-omitted now.

end
ensure
# We can't use ObjectSpace with JRuby.
return if RUBY_ENGINE == "jruby"
# Ensure freeing all closures.
# See https://github.com/ruby/fiddle/issues/102#issuecomment-1241763091 .
not_freed_closures = []
ObjectSpace.each_object(Fiddle::Closure) do |closure|
not_freed_closures << closure unless closure.freed?
unless RUBY_ENGINE == "jruby"
# Ensure freeing all closures.
# See https://github.com/ruby/fiddle/issues/102#issuecomment-1241763091 .
not_freed_closures = []
ObjectSpace.each_object(Fiddle::Closure) do |closure|
not_freed_closures << closure unless closure.freed?
end
assert_equal([], not_freed_closures)
end
assert_equal([], not_freed_closures)
end

def test_snprintf
Expand Down
Loading
Loading