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 0.13.0 no longer ignores invalid parameters #788

Closed
joshcooper opened this issue Jun 3, 2020 · 2 comments
Closed

FFI 0.13.0 no longer ignores invalid parameters #788

joshcooper opened this issue Jun 3, 2020 · 2 comments

Comments

@joshcooper
Copy link

@joshcooper joshcooper commented Jun 3, 2020

The ffi 0.13.0 release on Windows switched to using the universal C runtime instead of msvcrt in https://github.com/ffi/ffi/pull/779/files#diff-db0e38803c251431bdf21ae8936ac3f6L132-R132. Previously, msvcrt ignored invalid parameters, but ucrtbase will abort by default

Many functions now validate their parameters, halting execution if given invalid parameters. This validation may break code that passes invalid parameters and relies on the function ignoring them or just returning an error code.

This can be demonstrated using:

require 'ffi'

module MSVCRT
  extend FFI::Library
  ffi_lib 'msvcrt'
  attach_function(:get_osfhandle, :_get_osfhandle, [:int], :intptr_t)
end

module UCRTBASE
  extend FFI::Library
  ffi_lib 'ucrtbase'
  attach_function(:get_osfhandle, :_get_osfhandle, [:int], :intptr_t)
end

puts "msvcrt"
puts MSVCRT.get_osfhandle(42)
puts "ucrtbase"
puts UCRTBASE.get_osfhandle(42)
# we never get here
puts "Done"

Note that "Done" is never printed:

C:\Users\josh>bundle exec ruby osf.rb
msvcrt
-1
ucrtbase

This isn't a problem with FFI, but the ruby process aborts with 0xc0000409 without any output, which makes debugging the problem difficult. It'd be great to mention the behavior change in the release notes, and perhaps describe how to install an error handler so that the application can decide how to handle the exception: https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/set-invalid-parameter-handler-set-thread-local-invalid-parameter-handler?view=vs-2019.

@larskanis
Copy link
Member

@larskanis larskanis commented Jun 5, 2020

@joshcooper Thank you for your great analysis!

Obviously the change of FFI::Library::LIBC has an impact to several gems and the crashes are more due to the described behavioral change than due to a bug in the depending gem. This behavioral change wasn't intended by a minor version bump (from ffi-1.12 to 1.13). Maybe this is something for ffi-2.0, but for now I think it's best to revert to msvcrt.dll for the time being and release it as ffi-1.13.1 soon.

larskanis added a commit to larskanis/ffi that referenced this issue Jun 5, 2020
ffi-1.13.0 switched FFI::Library::LIBC from msvcrt.dll to ucrtbase.dll
as part of ffi#779 in commit c674683 .

As described in ffi#788 ucrtbase.dll has behavioural changes which
shouldn't be released as part of a minor version change of ffi.
While the change makes sense for mswin, we revert it for mingw.

Fixes ffi#788
@joshcooper
Copy link
Author

@joshcooper joshcooper commented Jun 5, 2020

Thanks @larskanis, much appreciated!

larskanis added a commit to larskanis/ffi that referenced this issue Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.