Fix GH-9090: Support assigning function pointers in FFI

Closes GH-9107.
php · Jul 27, 2022 · 8cf9c2f · 8cf9c2f
1 parent 099b168
commit 8cf9c2f
Show file tree

Hide file tree

Showing 4 changed files with 158 additions and 0 deletions.
diff --git a/NEWS b/NEWS
@@ -17,6 +17,10 @@ PHP                                                                        NEWS
   . Fixed LMDB driver memory leak on DB creation failure (Girgias)
   . Fixed GH-8856 (dba: lmdb: allow to override the MDB_NOSUBDIR flag). (Girgias)
 
+- FFI:
+  . Fixed bug GH-9090 (Support assigning function pointers in FFI). (Adam
+    Saponara)
+
 - Random:
   . Fixed bug GH-9067 (random extension is not thread safe). (cmb)
   . Fixed bug GH-9055 (segmentation fault if user engine throws). (timwolla)

diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c
@@ -214,6 +214,7 @@ static ZEND_FUNCTION(ffi_trampoline);
 static ZEND_COLD void zend_ffi_return_unsupported(zend_ffi_type *type);
 static ZEND_COLD void zend_ffi_pass_unsupported(zend_ffi_type *type);
 static ZEND_COLD void zend_ffi_assign_incompatible(zval *arg, zend_ffi_type *type);
+static bool zend_ffi_is_compatible_type(zend_ffi_type *dst_type, zend_ffi_type *src_type);
 
 #if FFI_CLOSURES
 static void *zend_ffi_create_callback(zend_ffi_type *type, zval *value);
@@ -255,6 +256,49 @@ static zend_object *zend_ffi_cdata_new(zend_class_entry *class_type) /* {{{ */
 }
 /* }}} */
 
+static bool zend_ffi_func_ptr_are_compatible(zend_ffi_type *dst_type, zend_ffi_type *src_type) /* {{{ */
+{
+	uint32_t dst_argc, src_argc, i;
+	zend_ffi_type *dst_arg, *src_arg;
+
+	ZEND_ASSERT(dst_type->kind == ZEND_FFI_TYPE_FUNC);
+	ZEND_ASSERT(src_type->kind == ZEND_FFI_TYPE_FUNC);
+
+	/* Ensure calling convention matches */
+	if (dst_type->func.abi != src_type->func.abi) {
+		return 0;
+	}
+
+	/* Ensure variadic attr matches */
+	if ((dst_type->attr & ZEND_FFI_ATTR_VARIADIC) != (src_type->attr & ZEND_FFI_ATTR_VARIADIC)) {
+		return 0;
+	}
+
+	/* Ensure same arg count */
+	dst_argc = dst_type->func.args ? zend_hash_num_elements(dst_type->func.args) : 0;
+	src_argc = src_type->func.args ? zend_hash_num_elements(src_type->func.args) : 0;
+	if (dst_argc != src_argc) {
+		return 0;
+	}
+
+	/* Ensure compatible ret_type */
+	if (!zend_ffi_is_compatible_type(dst_type->func.ret_type, src_type->func.ret_type)) {
+		return 0;
+	}
+
+	/* Ensure compatible args */
+	for (i = 0; i < dst_argc; i++) {
+		dst_arg = zend_hash_index_find_ptr(dst_type->func.args, i);
+		src_arg = zend_hash_index_find_ptr(src_type->func.args, i);
+		if (!zend_ffi_is_compatible_type(ZEND_FFI_TYPE(dst_arg), ZEND_FFI_TYPE(src_arg))) {
+			return 0;
+		}
+	}
+
+	return 1;
+}
+/* }}} */
+
 static bool zend_ffi_is_compatible_type(zend_ffi_type *dst_type, zend_ffi_type *src_type) /* {{{ */
 {
 	while (1) {
@@ -269,6 +313,9 @@ static bool zend_ffi_is_compatible_type(zend_ffi_type *dst_type, zend_ffi_type *
 				if (dst_type->kind == ZEND_FFI_TYPE_VOID ||
 				    src_type->kind == ZEND_FFI_TYPE_VOID) {
 				    return 1;
+				} else if (dst_type->kind == ZEND_FFI_TYPE_FUNC &&
+				           src_type->kind == ZEND_FFI_TYPE_FUNC) {
+				    return zend_ffi_func_ptr_are_compatible(dst_type, src_type);
 				}
 			} else if (dst_type->kind == ZEND_FFI_TYPE_ARRAY &&
 			           (dst_type->array.length == src_type->array.length ||

diff --git a/ext/ffi/tests/bug_gh9090.phpt b/ext/ffi/tests/bug_gh9090.phpt
@@ -0,0 +1,80 @@
+--TEST--
+GH-9090 (Support assigning function pointers via FFI)
+--EXTENSIONS--
+ffi
+zend_test
+--FILE--
+<?php
+
+require_once 'utils.inc';
+$h = <<<'EOD'
+void (*bug_gh9090_void_none_ptr)();
+void (*bug_gh9090_void_int_char_ptr)(int, char *);
+void (*bug_gh9090_void_int_char_var_ptr)(int, char *, ...);
+void (*bug_gh9090_void_char_int_ptr)(char *, int);
+int (*bug_gh9090_int_int_char_ptr)(int, char *);
+
+void bug_gh9090_void_none();
+void bug_gh9090_void_int_char(int i, char *s);
+void bug_gh9090_void_int_char_var(int i, char *fmt, ...);
+EOD;
+
+if (PHP_OS_FAMILY !== 'Windows') {
+    $ffi = FFI::cdef($h);
+} else {
+    try {
+        $ffi = FFI::cdef($h, 'php_zend_test.dll');
+    } catch (FFI\Exception $ex) {
+        $ffi = FFI::cdef($h, ffi_get_php_dll_name());
+    }
+}
+
+$func_ptrs = [
+    'bug_gh9090_void_none_ptr',
+    'bug_gh9090_void_int_char_ptr',
+    'bug_gh9090_void_int_char_var_ptr',
+    'bug_gh9090_void_char_int_ptr',
+    'bug_gh9090_int_int_char_ptr',
+];
+
+$func_argvs = [
+    [ 'bug_gh9090_void_none',         [ ]                           ],
+    [ 'bug_gh9090_void_int_char',     [ 42, "hello" ]               ],
+    [ 'bug_gh9090_void_int_char_var', [ 42, "d=%d s=%s", -1, "ok" ] ],
+];
+
+foreach ($func_ptrs as $func_ptr) {
+    foreach ($func_argvs as $func_argv) {
+        [ $func, $argv ] = $func_argv;
+
+        $ok = true;
+        try {
+            $ffi->$func_ptr = $ffi->$func;
+            call_user_func_array($ffi->$func_ptr, $argv);
+        } catch (FFI\Exception $e) {
+            $ok = false;
+        }
+
+        printf("%-36s = %-36s ? %s\n", $func_ptr, $func, $ok ? 'yes' : 'no');
+    }
+}
+?>
+--EXPECT--
+bug_gh9090_none
+bug_gh9090_void_none_ptr             = bug_gh9090_void_none                 ? yes
+bug_gh9090_void_none_ptr             = bug_gh9090_void_int_char             ? no
+bug_gh9090_void_none_ptr             = bug_gh9090_void_int_char_var         ? no
+bug_gh9090_void_int_char_ptr         = bug_gh9090_void_none                 ? no
+bug_gh9090_int_char 42 hello
+bug_gh9090_void_int_char_ptr         = bug_gh9090_void_int_char             ? yes
+bug_gh9090_void_int_char_ptr         = bug_gh9090_void_int_char_var         ? no
+bug_gh9090_void_int_char_var_ptr     = bug_gh9090_void_none                 ? no
+bug_gh9090_void_int_char_var_ptr     = bug_gh9090_void_int_char             ? no
+bug_gh9090_void_int_char_var d=-1 s=ok
+bug_gh9090_void_int_char_var_ptr     = bug_gh9090_void_int_char_var         ? yes
+bug_gh9090_void_char_int_ptr         = bug_gh9090_void_none                 ? no
+bug_gh9090_void_char_int_ptr         = bug_gh9090_void_int_char             ? no
+bug_gh9090_void_char_int_ptr         = bug_gh9090_void_int_char_var         ? no
+bug_gh9090_int_int_char_ptr          = bug_gh9090_void_none                 ? no
+bug_gh9090_int_int_char_ptr          = bug_gh9090_void_int_char             ? no
+bug_gh9090_int_int_char_ptr          = bug_gh9090_void_int_char_var         ? no
diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c
@@ -847,3 +847,30 @@ PHP_ZEND_TEST_API bug80847_02 ffi_bug80847(bug80847_02 s) {
 	s.a.c -= 10.0;
 	return s;
 }
+
+PHP_ZEND_TEST_API void (*bug_gh9090_void_none_ptr)(void) = NULL;
+PHP_ZEND_TEST_API void (*bug_gh9090_void_int_char_ptr)(int, char *) = NULL;
+PHP_ZEND_TEST_API void (*bug_gh9090_void_int_char_var_ptr)(int, char *, ...) = NULL;
+PHP_ZEND_TEST_API void (*bug_gh9090_void_char_int_ptr)(char *, int) = NULL;
+PHP_ZEND_TEST_API int (*bug_gh9090_int_int_char_ptr)(int, char *) = NULL;
+
+PHP_ZEND_TEST_API void bug_gh9090_void_none(void) {
+    php_printf("bug_gh9090_none\n");
+}
+
+PHP_ZEND_TEST_API void bug_gh9090_void_int_char(int i, char *s) {
+    php_printf("bug_gh9090_int_char %d %s\n", i, s);
+}
+
+PHP_ZEND_TEST_API void bug_gh9090_void_int_char_var(int i, char *fmt, ...) {
+    va_list args;
+    char *buffer;
+
+    va_start(args, fmt);
+
+    zend_vspprintf(&buffer, 0, fmt, args);
+    php_printf("bug_gh9090_void_int_char_var %s\n", buffer);
+    efree(buffer);
+
+    va_end(args);
+}