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

FFI::Struct#layout fails when mkmf is required #776

Closed
eregon opened this issue May 15, 2020 · 5 comments · Fixed by #777
Closed

FFI::Struct#layout fails when mkmf is required #776

eregon opened this issue May 15, 2020 · 5 comments · Fixed by #777

Comments

@eregon
Copy link
Collaborator

eregon commented May 15, 2020

$ ruby -rmkmf -rffi -e 'module A; class A < FFI::Struct; layout :a, :int; end; end' |& cat
/home/eregon/.rubies/ruby-2.6.6/lib/ruby/2.6.0/mkmf.rb:1255:in `find_type': wrong number of arguments (given 1, expected 2+) (ArgumentError)
	from /home/eregon/.rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/ffi-1.12.2/lib/ffi/struct.rb:271:in `find_type'
	from /home/eregon/.rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/ffi-1.12.2/lib/ffi/struct.rb:265:in `find_field_type'
	from /home/eregon/.rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/ffi-1.12.2/lib/ffi/struct.rb:305:in `array_layout'
	from /home/eregon/.rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/ffi-1.12.2/lib/ffi/struct.rb:217:in `layout'
	from -e:1:in `<class:A>'
	from -e:1:in `<module:A>'
	from -e:1:in `<main>'

This is due to

(mod < FFI::Library || mod < FFI::Struct || mod.respond_to?(:find_type)) ? mod : nil

and mkmf defining a public find_type on every object by include MakeMakefile 😞

On TruffleRuby, the socket stdlib uses FFI, so this bug ends up failing to install some gems that require socket (indirectly) in extconf.rb, see oracle/truffleruby#1896

@eregon
Copy link
Collaborator Author

eregon commented May 15, 2020

I filed an issue for mkmf too since it doesn't seem reasonable to have so many public methods:
https://bugs.ruby-lang.org/issues/16896

But we'll need a workaround in FFI, as old releases won't be fixed.

@eregon
Copy link
Collaborator Author

eregon commented May 15, 2020

Code there is related to 510c99c/#256 and b49cfae.
It's not clear why FFI ever checked for respond_to?(:find_type) but that seems quite brittle as this bug shows.

@eregon
Copy link
Collaborator Author

eregon commented May 15, 2020

I think the actual intention was to restrict to FFI::Library or FFI::Struct, so the condition should be:

if mod.respond_to?(:find_type) && (mod.is_a?(FFI::Library) || mod < FFI::Struct)
  mod
end

This works and passes all specs.

Another possibility is to check arity, but that's less reliable and not pretty: https://gist.github.com/eregon/61ff7e20aaaa5af35a3e6401603d1f7f

@eregon
Copy link
Collaborator Author

eregon commented May 15, 2020

PR to fix this: #777

graalvmbot pushed a commit to oracle/truffleruby that referenced this issue May 15, 2020
@larskanis
Copy link
Member

I think the actual intention was to restrict to FFI::Library or FFI::Struct

The intention behind respond_to?(:find_type) was maybe to also allow alternative implementations of find_type, but since there's no test case for something like this and I don't remember of anything like this in other projects, I think it's best to make it an AND operation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants