From 481adbcdfc816a94bde5c695a175807a00bc8ff2 Mon Sep 17 00:00:00 2001 From: Charlie Savage Date: Sun, 17 May 2020 23:54:39 -0700 Subject: [PATCH 1/6] Ruby built with MSVC reports the x64 as the platform on Windows 10, 64-bit machines. --- lib/ffi/platform.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ffi/platform.rb b/lib/ffi/platform.rb index 3e0fc6059..85e3b2596 100644 --- a/lib/ffi/platform.rb +++ b/lib/ffi/platform.rb @@ -61,7 +61,7 @@ module Platform CPU = RbConfig::CONFIG['host_cpu'] ARCH = case CPU.downcase - when /amd64|x86_64/ + when /amd64|x86_64|x64/ "x86_64" when /i?86|x86|i86pc/ "i386" From 976f4b01f17eb7b865db557200838069aaaa81d1 Mon Sep 17 00:00:00 2001 From: Charlie Savage Date: Sun, 17 May 2020 23:56:25 -0700 Subject: [PATCH 2/6] MSVC defines long double to be just a double (see https://docs.microsoft.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. --- ext/ffi_c/compat.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ext/ffi_c/compat.h b/ext/ffi_c/compat.h index a4dfc0851..3f7bbaed1 100644 --- a/ext/ffi_c/compat.h +++ b/ext/ffi_c/compat.h @@ -64,6 +64,10 @@ # define unlikely(x) (x) #endif +#ifdef _MSC_VER +#define ffi_type_longdouble ffi_type_double +#endif + #ifndef MAX # define MAX(a, b) ((a) < (b) ? (b) : (a)) #endif From 7f58fd31f6f00cb45065decafa1ae7290b18b33c Mon Sep 17 00:00:00 2001 From: Charlie Savage Date: Mon, 18 May 2020 00:02:04 -0700 Subject: [PATCH 3/6] MSVC will not initialize static structure using non-constant values (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. --- ext/ffi_c/MethodHandle.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/ffi_c/MethodHandle.c b/ext/ffi_c/MethodHandle.c index b266f995e..4dc85fbff 100644 --- a/ext/ffi_c/MethodHandle.c +++ b/ext/ffi_c/MethodHandle.c @@ -128,11 +128,7 @@ rbffi_function_anyargs rbffi_MethodHandle_CodeAddress(MethodHandle* handle) #ifndef CUSTOM_TRAMPOLINE static void attached_method_invoke(ffi_cif* cif, void* retval, METHOD_PARAMS parameters, void* user_data); -static ffi_type* methodHandleParamTypes[] = { - &ffi_type_sint, - &ffi_type_pointer, - &ffi_type_ulong, -}; +static ffi_type* methodHandleParamTypes[3]; static ffi_cif mh_cif; @@ -342,6 +338,10 @@ rbffi_MethodHandle_Init(VALUE module) rb_raise(rb_eFatal, "Could not locate offsets in trampoline code"); } #else + methodHandleParamTypes[0] = &ffi_type_sint; + methodHandleParamTypes[1] = &ffi_type_pointer; + methodHandleParamTypes[2] = &ffi_type_ulong; + ffiStatus = ffi_prep_cif(&mh_cif, FFI_DEFAULT_ABI, 3, &ffi_type_ulong, methodHandleParamTypes); if (ffiStatus != FFI_OK) { From d9bd3f9616312ac3ca0b09b0bb5a8a6e90ed739a Mon Sep 17 00:00:00 2001 From: Charlie Savage Date: Mon, 18 May 2020 00:03:32 -0700 Subject: [PATCH 4/6] If you build libffi with MSVC, the latest version gets installed as libffi-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. --- ext/ffi_c/extconf.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/ext/ffi_c/extconf.rb b/ext/ffi_c/extconf.rb index 9bdb807a6..2e964c2e3 100644 --- a/ext/ffi_c/extconf.rb +++ b/ext/ffi_c/extconf.rb @@ -15,7 +15,13 @@ def system_libffi_usable? # Ensure we can link to ffi_call libffi_ok &&= have_library("ffi", "ffi_call", [ "ffi.h" ]) || - have_library("libffi", "ffi_call", [ "ffi.h" ]) + have_library("libffi", "ffi_call", [ "ffi.h" ]) || + have_library("libffi-8", "ffi_call", [ "ffi.h" ]) + + if RbConfig::CONFIG['host_os'] =~ /mswin/ + have_library('libffi_convenience') + have_library('shlwapi') + end # And we need a libffi version recent enough to provide ffi_closure_alloc libffi_ok &&= have_func("ffi_closure_alloc") @@ -39,7 +45,6 @@ def system_libffi_usable? abort "system libffi is not usable" unless system_libffi_usable? end - have_header('shlwapi.h') have_func('rb_thread_call_without_gvl') || abort("Ruby C-API function `rb_thread_call_without_gvl` is missing") have_func('ruby_native_thread_p') if RUBY_VERSION >= "2.3.0" @@ -56,13 +61,10 @@ def system_libffi_usable? end $defs << "-DHAVE_EXTCONF_H" if $defs.empty? # needed so create_header works - $defs << "-DFFI_BUILDING" if RbConfig::CONFIG['host_os'] =~ /mswin/ # for compatibility with newer libffi create_header - - $LOCAL_LIBS << " ./libffi/.libs/libffi_convenience.lib" if !system_libffi && RbConfig::CONFIG['host_os'] =~ /mswin/ - create_makefile("ffi_c") + unless system_libffi File.open("Makefile", "a") do |mf| mf.puts "LIBFFI_HOST=--host=#{RbConfig::CONFIG['host_alias']}" if RbConfig::CONFIG.has_key?("host_alias") From c67468366e63bcf84512837384049fbba7e4748c Mon Sep 17 00:00:00 2001 From: Charlie Savage Date: Mon, 18 May 2020 00:59:36 -0700 Subject: [PATCH 5/6] On windows, when using Ruby built with mingw64, FFI:LIBC returns msvcrt.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. --- lib/ffi/platform.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ffi/platform.rb b/lib/ffi/platform.rb index 85e3b2596..959a72cff 100644 --- a/lib/ffi/platform.rb +++ b/lib/ffi/platform.rb @@ -129,7 +129,7 @@ def self.is_os(os) end LIBC = if IS_WINDOWS - RbConfig::CONFIG['RUBY_SO_NAME'].split('-')[-2] + '.dll' + "ucrtbase.dll" elsif IS_GNU GNU_LIBC elsif OS == 'cygwin' From 7bbbce268eac9903d4b42bfb913baf633da6b6e1 Mon Sep 17 00:00:00 2001 From: Charlie Savage Date: Mon, 18 May 2020 11:26:39 -0700 Subject: [PATCH 6/6] On MSCV, an unsigned long is 4 bytes (32 bits), not 8 bytes. Thus the alignment code using ~0x7UL truncates pointers resulting in crashes. Instead, use unsigned long long wich is 8 bytes. --- ext/ffi_c/Buffer.c | 4 ++-- ext/ffi_c/MemoryPointer.c | 2 +- ext/ffi_c/Pointer.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/ffi_c/Buffer.c b/ext/ffi_c/Buffer.c index faf4834d0..6863dda07 100644 --- a/ext/ffi_c/Buffer.c +++ b/ext/ffi_c/Buffer.c @@ -114,7 +114,7 @@ buffer_initialize(int argc, VALUE* argv, VALUE self) } /* ensure the memory is aligned on at least a 8 byte boundary */ - p->memory.address = (void *) (((uintptr_t) p->data.storage + 0x7) & (uintptr_t) ~0x7UL); + p->memory.address = (void *) (((uintptr_t) p->data.storage + 0x7) & (uintptr_t) ~0x7ULL); if (p->memory.size > 0 && (nargs < 3 || RTEST(rbClear))) { memset(p->memory.address, 0, p->memory.size); @@ -154,7 +154,7 @@ buffer_initialize_copy(VALUE self, VALUE other) return Qnil; } - dst->memory.address = (void *) (((uintptr_t) dst->data.storage + 0x7) & (uintptr_t) ~0x7UL); + dst->memory.address = (void *) (((uintptr_t) dst->data.storage + 0x7) & (uintptr_t) ~0x7ULL); dst->memory.size = src->size; dst->memory.typeSize = src->typeSize; diff --git a/ext/ffi_c/MemoryPointer.c b/ext/ffi_c/MemoryPointer.c index 0d91c35b7..9dc59bb57 100644 --- a/ext/ffi_c/MemoryPointer.c +++ b/ext/ffi_c/MemoryPointer.c @@ -112,7 +112,7 @@ memptr_malloc(VALUE self, long size, long count, bool clear) p->memory.typeSize = (int) size; p->memory.size = msize; /* ensure the memory is aligned on at least a 8 byte boundary */ - p->memory.address = (char *) (((uintptr_t) p->storage + 0x7) & (uintptr_t) ~0x7UL);; + p->memory.address = (char *) (((uintptr_t) p->storage + 0x7) & (uintptr_t) ~0x7ULL); p->allocated = true; if (clear && p->memory.size > 0) { diff --git a/ext/ffi_c/Pointer.c b/ext/ffi_c/Pointer.c index 1eee790db..e5f24a4dc 100644 --- a/ext/ffi_c/Pointer.c +++ b/ext/ffi_c/Pointer.c @@ -183,7 +183,7 @@ ptr_initialize_copy(VALUE self, VALUE other) dst->allocated = true; dst->autorelease = true; - dst->memory.address = (void *) (((uintptr_t) dst->storage + 0x7) & (uintptr_t) ~0x7UL); + dst->memory.address = (void *) (((uintptr_t) dst->storage + 0x7) & (uintptr_t) ~0x7ULL); dst->memory.size = src->size; dst->memory.typeSize = src->typeSize;