From ce6caade7c57a505f73086ccd7b33c14f7715f22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADt=20Ondruch?= Date: Wed, 10 Jul 2019 23:19:32 +0200 Subject: [PATCH] [ruby/fiddle] Use ffi_closure_free by default. (#20) * Use ffi_closure_free unconditionally. The current conditionals reflect historic heritage of FFI. Usage of ffi_closure_free should be better default nowadays, because libffi 3.0.5 fixing issues of ffi_closure_free should be widely available. * RUBY_LIBFFI_MODVERSION is not used anymore. Because `ffi_closure_free()` is not used unconditionally, there is no other use for RUBY_LIBFFI_MODVERSION define, so drop its usage. * Use more meaningful variable name. `ver` variable used to be used to pupulate RUBY_LIBFFI_MODVERSION define. Since the define was removed, the `libffi_dir` variable name should better describe the remaining usage of the variable. https://github.com/ruby/fiddle/commit/c49cc79eb8 --- ext/fiddle/closure.c | 28 ---------------------------- ext/fiddle/extconf.rb | 20 ++++++-------------- 2 files changed, 6 insertions(+), 42 deletions(-) diff --git a/ext/fiddle/closure.c b/ext/fiddle/closure.c index 1a80b2b02a..b997e23f45 100644 --- a/ext/fiddle/closure.c +++ b/ext/fiddle/closure.c @@ -13,25 +13,11 @@ typedef struct { ffi_type **argv; } fiddle_closure; -#if defined(USE_FFI_CLOSURE_ALLOC) -#elif defined(__OpenBSD__) || defined(__APPLE__) || defined(__linux__) -# define USE_FFI_CLOSURE_ALLOC 0 -#elif defined(RUBY_LIBFFI_MODVERSION) && RUBY_LIBFFI_MODVERSION < 3000005 && \ - (defined(__i386__) || defined(__x86_64__) || defined(_M_IX86) || defined(_M_AMD64)) -# define USE_FFI_CLOSURE_ALLOC 0 -#else -# define USE_FFI_CLOSURE_ALLOC 1 -#endif - static void dealloc(void * ptr) { fiddle_closure * cls = (fiddle_closure *)ptr; -#if USE_FFI_CLOSURE_ALLOC ffi_closure_free(cls->pcl); -#else - munmap(cls->pcl, sizeof(*cls->pcl)); -#endif if (cls->argv) xfree(cls->argv); xfree(cls); } @@ -205,12 +191,7 @@ allocate(VALUE klass) VALUE i = TypedData_Make_Struct(klass, fiddle_closure, &closure_data_type, closure); -#if USE_FFI_CLOSURE_ALLOC closure->pcl = ffi_closure_alloc(sizeof(ffi_closure), &closure->code); -#else - closure->pcl = mmap(NULL, sizeof(ffi_closure), PROT_READ | PROT_WRITE, - MAP_ANON | MAP_PRIVATE, -1, 0); -#endif return i; } @@ -257,17 +238,8 @@ initialize(int rbargc, VALUE argv[], VALUE self) if (FFI_OK != result) rb_raise(rb_eRuntimeError, "error prepping CIF %d", result); -#if USE_FFI_CLOSURE_ALLOC result = ffi_prep_closure_loc(pcl, cif, callback, (void *)self, cl->code); -#else - result = ffi_prep_closure(pcl, cif, callback, (void *)self); - cl->code = (void *)pcl; - i = mprotect(pcl, sizeof(*pcl), PROT_READ | PROT_EXEC); - if (i) { - rb_sys_fail("mprotect"); - } -#endif if (FFI_OK != result) rb_raise(rb_eRuntimeError, "error prepping closure %d", result); diff --git a/ext/fiddle/extconf.rb b/ext/fiddle/extconf.rb index fce0cb4079..5525b2c808 100644 --- a/ext/fiddle/extconf.rb +++ b/ext/fiddle/extconf.rb @@ -7,8 +7,7 @@ bundle = enable_config('bundled-libffi') if ! bundle dir_config 'libffi' - pkg_config("libffi") and - ver = pkg_config("libffi", "modversion") + pkg_config("libffi") if have_header(ffi_header = 'ffi.h') true @@ -28,20 +27,20 @@ begin Dir.glob("#{$srcdir}/libffi-*/").each{|dir| FileUtils.rm_rf(dir)} extlibs.run(["--cache=#{cache_dir}", ext_dir]) end - ver = bundle != false && + libffi_dir = bundle != false && Dir.glob("#{$srcdir}/libffi-*/") .map {|n| File.basename(n)} .max_by {|n| n.scan(/\d+/).map(&:to_i)} - unless ver + unless libffi_dir raise "missing libffi. Please install libffi." end - srcdir = "#{$srcdir}/#{ver}" + srcdir = "#{$srcdir}/#{libffi_dir}" ffi_header = 'ffi.h' libffi = Struct.new(*%I[dir srcdir builddir include lib a cflags ldflags opt arch]).new - libffi.dir = ver + libffi.dir = libffi_dir if $srcdir == "." - libffi.builddir = "#{ver}/#{RUBY_PLATFORM}" + libffi.builddir = "#{libffi_dir}/#{RUBY_PLATFORM}" libffi.srcdir = "." else libffi.builddir = libffi.dir @@ -52,7 +51,6 @@ begin libffi.a = "#{libffi.lib}/libffi_convenience.#{$LIBEXT}" nowarn = CONFIG.merge("warnflags"=>"") libffi.cflags = RbConfig.expand("$(CFLAGS)".dup, nowarn) - ver = ver[/libffi-(.*)/, 1] FileUtils.mkdir_p(libffi.dir) libffi.opt = CONFIG['configure_args'][/'(-C)'/, 1] @@ -112,12 +110,6 @@ begin $INCFLAGS << " -I" << libffi.include end -if ver - ver = ver.gsub(/-rc\d+/, '') # If ver contains rc version, just ignored. - ver = (ver.split('.') + [0,0])[0,3] - $defs.push(%{-DRUBY_LIBFFI_MODVERSION=#{ '%d%03d%03d' % ver }}) -end - have_header 'sys/mman.h' if have_header "dlfcn.h"