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

MSVC 2019 Fixes #779

Merged
merged 6 commits into from May 23, 2020
Merged

MSVC 2019 Fixes #779

merged 6 commits into from May 23, 2020

Conversation

cfis
Copy link
Contributor

@cfis cfis commented May 18, 2020

These 5 commits fix various issues that prevent Microsoft Visual Studio 2019 from compiling the ffi bindings. Each commit provides details about the specific issue it is fixing.

cfis added 5 commits May 17, 2020 23:54
…oft.com/en-us/cpp/c-language/type-long-double?view=vs-2019). Thus libffi built with MSVC does not export it as a separate type. This workaround that issue.
…GCC will do it but I think via an extension, its not part of the C language). Anyway, this fixes the compile error on MSVC.
…ibffi-8.lib. This commit adds that name to the have_library search list. It also adds in have_library calls for libffi_convenience.lib and shlwapi.lib, both of which are required for MSVC to build the bindings.
…rt.dll. When using Ruby built with MSVC, FFI:LIBC returns vcruntime.dll

Neither is correct. Starting in Visual Studio 2015, the C runtime library migrated to ucrtbase.dll. See https://docs.microsoft.com/en-us/cpp/c-runtime-library/crt-library-features?view=vs-2019 for more information. Or https://devblogs.microsoft.com/cppblog/introducing-the-universal-crt/.

I assume this hasn't been reported as a bug since almost no one uses a MSVC Ruby build, and attach_function for functions like malloc does actually work using msvcrt.dll - but does not with vcruntime.dll

This patch changes mingw64 and mswin to use ucrtbase.dll for the C library. Tested both with mswin and mingw64 Ruby builds.

The one downside of this patch is that it would break for anyone running a version of Windows that hasn't been updated since Spring 2015 when Microsoft introduced this change (the dll ships with Windows 10 and was back ported to older windows versions via Windows update). So seems to me a very, very low risk.
@cfis cfis changed the title MSVC Fixes MSVC 2019 Fixes May 18, 2020
… alignment code using ~0x7UL truncates pointers resulting in crashes. Instead, use unsigned long long wich is 8 bytes.
@larskanis larskanis merged commit de9dd38 into ffi:master May 23, 2020
@larskanis
Copy link
Member

@cfis Thank you for your contributions!

There are 3 things I worry about:

  1. ucrtbase.dll is not fully compatible to msvcrt.dll. You might noticed the broken CI run https://github.com/ffi/ffi/pull/779/checks?check_run_id=686393082#step:9:1339 , which is caused by time() not being defined as a symbol. That was easy to fix but it proves to be not fully backward compatible. So far I didn't find any real world incompatibilities in other projects using FFI.
  2. ucrtbase.dll is not specified by MS to be compatible to anything - only the lib files are mentioned in CRT docs. I also found this article which states:

it’s actually forbidden to link to ucrtbase.dll directly and one must use the official .lib file, or else one loses all compatibility guarantees.

  1. There are currently no CI runs for ffi on MSVC, so that the compatibility can easily break again.

@cfis
Copy link
Contributor Author

cfis commented May 24, 2020

Thanks for merging. I agree with your points.

  1. Good question about time - as you noticed it is not exported by ucrtbase (which does have _time_32 and _time_64). The MSFT docs say time is a "wrapper" around those 2 functions. Its not hard to work around if you need to access time (or whatever else is missing), just in the additional library to your fii code. But I agree its unexpected and annoying. Sigh.

  2. Interesting article. It links to this page - https://gist.github.com/njsmith/08b1e52b65ea90427bfd which list "virtual" dll names. And that got me to https://stackoverflow.com/questions/47529106/what-are-api-ms-win-lx-x-x-dll-umbrella-libraries. Which is an explanation of how windows now has "virtual" dll names that map to logical dlls. Which could be another approach I guess of how to get at the runtime library.

  3. Yeah, that's definitely true. But since I'm the only one who seems to have noticed, its probably not all that important. My use case for using an MSVC ruby version is that MSVC provides, in my opinion of course, a much better debugging environment for working with C/C++ code making easier to work on Ruby native extensions I maintain.

larskanis added a commit to larskanis/ffi that referenced this pull request 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
@larskanis larskanis mentioned this pull request Aug 30, 2020
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 this pull request may close these issues.

None yet

2 participants