Fixed bug #77706 (Improve error messages in FFI for incompatible arguments)

diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c
index 95af75ad7b68b40f481b6152a73e49a66d15be78..d9c7d3fb2670817ee16a917bc0f507b1d54137d9 100644 (file)
--- a/ext/ffi/ffi.c
+++ b/ext/ffi/ffi.c
@@ -194,6 +194,9 @@ static int zend_ffi_is_same_type(zend_ffi_type *type1, zend_ffi_type *type2);
 static zend_ffi_type *zend_ffi_remember_type(zend_ffi_type *type);
 static char *zend_ffi_parse_directives(const char *filename, char *code_pos, char **scope_name, char **lib, zend_bool preload);
 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);
 
 #if FFI_CLOSURES
 static void *zend_ffi_create_callback(zend_ffi_type *type, zval *value);
@@ -320,7 +323,6 @@ static ffi_type *zend_ffi_make_fake_struct_type(zend_ffi_type *type) /* {{{ */
                                break;
                        default:
                                efree(t);
-                               zend_throw_error(zend_ffi_exception_ce, "Passing incompatible struct/union");
                                return NULL;
                        }
                i++;
@@ -374,17 +376,10 @@ again:
                case ZEND_FFI_TYPE_STRUCT:
                        if (!(type->attr & ZEND_FFI_ATTR_UNION)) {
                                ffi_type *t = zend_ffi_make_fake_struct_type(type);
-                               if (t) {
-                                       return t;
-                               }
+                               return t;
                        }
-                       zend_throw_error(zend_ffi_exception_ce, "FFI return struct/union is not implemented");
-                       break;
-               case ZEND_FFI_TYPE_ARRAY:
-                       zend_throw_error(zend_ffi_exception_ce, "FFI return array is not implemented");
                        break;
                default:
-                       zend_throw_error(zend_ffi_exception_ce, "FFI internal error");
                        break;
        }
        return NULL;
@@ -654,7 +649,7 @@ again:
                        if (ZSTR_LEN(str) == 1) {
                                *(char*)ptr = ZSTR_VAL(str)[0];
                        } else {
-                               zend_throw_error(zend_ffi_exception_ce, "Attempt to perform assign of incompatible C type");
+                               zend_ffi_assign_incompatible(value, type);
                                return FAILURE;
                        }
                        zend_tmp_string_release(tmp_str);
@@ -702,7 +697,7 @@ again:
                                }
 #endif
                        }
-                       zend_throw_error(zend_ffi_exception_ce, "Attempt to perform assign of incompatible C type");
+                       zend_ffi_assign_incompatible(value, type);
                        return FAILURE;
                case ZEND_FFI_TYPE_STRUCT:
                case ZEND_FFI_TYPE_ARRAY:
@@ -715,7 +710,7 @@ again:
                                        return SUCCESS;
                                }
                        }
-                       zend_throw_error(zend_ffi_exception_ce, "Attempt to perform assign of incompatible C type");
+                       zend_ffi_assign_incompatible(value, type);
                        return FAILURE;
        }
        return SUCCESS;
@@ -841,6 +836,7 @@ static void *zend_ffi_create_callback(zend_ffi_type *type, zval *value) /* {{{ *
                        arg_type = ZEND_FFI_TYPE(arg_type);
                        callback_data->arg_types[n] = zend_ffi_get_type(arg_type);
                        if (!callback_data->arg_types[n]) {
+                               zend_ffi_pass_unsupported(arg_type);
                                efree(callback_data);
                                ffi_closure_free(callback);
                                return NULL;
@@ -850,6 +846,7 @@ static void *zend_ffi_create_callback(zend_ffi_type *type, zval *value) /* {{{ *
        }
        callback_data->ret_type = zend_ffi_get_type(type->func.ret_type);
        if (!callback_data->ret_type) {
+               zend_ffi_return_unsupported(type->func.ret_type);
                efree(callback_data);
                ffi_closure_free(callback);
                return NULL;
@@ -1347,6 +1344,86 @@ static int zend_ffi_ctype_name(zend_ffi_ctype_name_buf *buf, const zend_ffi_type
 }
 /* }}} */
 
+static ZEND_COLD void zend_ffi_return_unsupported(zend_ffi_type *type) /* {{{ */
+{
+       type = ZEND_FFI_TYPE(type);
+       if (type->kind == ZEND_FFI_TYPE_STRUCT) {
+               zend_throw_error(zend_ffi_exception_ce, "FFI return struct/union is not implemented");
+       } else if (type->kind == ZEND_FFI_TYPE_ARRAY) {
+               zend_throw_error(zend_ffi_exception_ce, "FFI return array is not implemented");
+       } else {
+               zend_throw_error(zend_ffi_exception_ce, "FFI internal error. Unsupported return type");
+       }
+}
+/* }}} */
+
+static ZEND_COLD void zend_ffi_pass_unsupported(zend_ffi_type *type) /* {{{ */
+{
+       type = ZEND_FFI_TYPE(type);
+       if (type->kind == ZEND_FFI_TYPE_STRUCT) {
+               zend_throw_error(zend_ffi_exception_ce, "FFI passing struct/union is not implemented");
+       } else if (type->kind == ZEND_FFI_TYPE_ARRAY) {
+               zend_throw_error(zend_ffi_exception_ce, "FFI passing array is not implemented");
+       } else {
+               zend_throw_error(zend_ffi_exception_ce, "FFI internal error. Unsupported parameter type");
+       }
+}
+/* }}} */
+
+static ZEND_COLD void zend_ffi_pass_incompatible(zval *arg, zend_ffi_type *type, uint32_t n, zend_execute_data *execute_data) /* {{{ */
+{
+       zend_ffi_ctype_name_buf buf1, buf2;
+
+       buf1.start = buf1.end = buf1.buf + ((MAX_TYPE_NAME_LEN * 3) / 4);
+       if (!zend_ffi_ctype_name(&buf1, type)) {
+               zend_throw_error(zend_ffi_exception_ce, "Passing incompatible argument %d of C function '%s'", n + 1, ZSTR_VAL(EX(func)->internal_function.function_name));
+       } else {
+               *buf1.end = 0;
+               if (Z_TYPE_P(arg) == IS_OBJECT && Z_OBJCE_P(arg) == zend_ffi_cdata_ce) {
+                       zend_ffi_cdata *cdata = (zend_ffi_cdata*)Z_OBJ_P(arg);
+
+                       type = ZEND_FFI_TYPE(cdata->type);
+                       buf2.start = buf2.end = buf2.buf + ((MAX_TYPE_NAME_LEN * 3) / 4);
+                       if (!zend_ffi_ctype_name(&buf2, type)) {
+                               zend_throw_error(zend_ffi_exception_ce, "Passing incompatible argument %d of C function '%s', expecting '%s'", n + 1, ZSTR_VAL(EX(func)->internal_function.function_name), buf1.start);
+                       } else {
+                               *buf2.end = 0;
+                               zend_throw_error(zend_ffi_exception_ce, "Passing incompatible argument %d of C function '%s', expecting '%s', found '%s'", n + 1, ZSTR_VAL(EX(func)->internal_function.function_name), buf1.start, buf2.start);
+                       }
+               } else {
+                       zend_throw_error(zend_ffi_exception_ce, "Passing incompatible argument %d of C function '%s', expecting '%s', found PHP '%s'", n + 1, ZSTR_VAL(EX(func)->internal_function.function_name), buf1.start, zend_get_type_by_const(Z_TYPE_P(arg)));
+               }
+       }
+}
+/* }}} */
+
+static ZEND_COLD void zend_ffi_assign_incompatible(zval *arg, zend_ffi_type *type) /* {{{ */
+{
+       zend_ffi_ctype_name_buf buf1, buf2;
+
+       buf1.start = buf1.end = buf1.buf + ((MAX_TYPE_NAME_LEN * 3) / 4);
+       if (!zend_ffi_ctype_name(&buf1, type)) {
+               zend_throw_error(zend_ffi_exception_ce, "Incompatible types when assigning");
+       } else {
+               *buf1.end = 0;
+               if (Z_TYPE_P(arg) == IS_OBJECT && Z_OBJCE_P(arg) == zend_ffi_cdata_ce) {
+                       zend_ffi_cdata *cdata = (zend_ffi_cdata*)Z_OBJ_P(arg);
+
+                       type = ZEND_FFI_TYPE(cdata->type);
+                       buf2.start = buf2.end = buf2.buf + ((MAX_TYPE_NAME_LEN * 3) / 4);
+                       if (!zend_ffi_ctype_name(&buf2, type)) {
+                               zend_throw_error(zend_ffi_exception_ce, "Incompatible types when assigning to type '%s'", buf1.start);
+                       } else {
+                               *buf2.end = 0;
+                               zend_throw_error(zend_ffi_exception_ce, "Incompatible types when assigning to type '%s' from type '%s'", buf1.start, buf2.start);
+                       }
+               } else {
+                       zend_throw_error(zend_ffi_exception_ce, "Incompatible types when assigning to type '%s' from PHP '%s'", buf1.start, zend_get_type_by_const(Z_TYPE_P(arg)));
+               }
+       }
+}
+/* }}} */
+
 static zend_string *zend_ffi_get_class_name(zend_string *prefix, const zend_ffi_type *type) /* {{{ */
 {
        zend_ffi_ctype_name_buf buf;
@@ -2127,7 +2204,7 @@ static zval *zend_ffi_write_var(zval *object, zval *member, zval *value, void **
 }
 /* }}} */
 
-static int zend_ffi_pass_arg(zval *arg, zend_ffi_type *type, ffi_type **pass_type, void **arg_values, uint32_t n) /* {{{ */
+static int zend_ffi_pass_arg(zval *arg, zend_ffi_type *type, ffi_type **pass_type, void **arg_values, uint32_t n, zend_execute_data *execute_data) /* {{{ */
 {
        zend_long lval;
        double dval;
@@ -2232,7 +2309,7 @@ again:
                                }
 #endif
                        }
-                       zend_throw_error(zend_ffi_exception_ce, "Passing incompatible pointer");
+                       zend_ffi_pass_incompatible(arg, type, n, execute_data);
                        return FAILURE;
                case ZEND_FFI_TYPE_BOOL:
                        *pass_type = &ffi_type_uint8;
@@ -2243,7 +2320,7 @@ again:
                        *pass_type = &ffi_type_sint8;
                        *(char*)arg_values[n] = ZSTR_VAL(str)[0];
                        if (ZSTR_LEN(str) != 1) {
-                               zend_throw_error(zend_ffi_exception_ce, "Attempt to pass incompatible C type");
+                               zend_ffi_pass_incompatible(arg, type, n, execute_data);
                        }
                        zend_tmp_string_release(tmp_str);
                        break;
@@ -2258,31 +2335,24 @@ again:
                                if (zend_ffi_is_compatible_type(type, ZEND_FFI_TYPE(cdata->type))) {
                                    /* Create a fake structure type */
                                        ffi_type *t = zend_ffi_make_fake_struct_type(type);
-                                       if (!t) {
-                                               return FAILURE;
+                                       if (t) {
+                                               *pass_type = t;
+                                               arg_values[n] = cdata->ptr;
+                                               break;
                                        }
-                                       *pass_type = t;
-                                       arg_values[n] = cdata->ptr;
-                                       break;
-                               } else {
-                                       zend_throw_error(zend_ffi_exception_ce, "Passing incompatible struct/union");
-                                       return FAILURE;
                                }
                        }
-                       zend_throw_error(zend_ffi_exception_ce, "FFI passing struct/union is not implemented");
-                       return FAILURE;
-               case ZEND_FFI_TYPE_ARRAY:
-                       zend_throw_error(zend_ffi_exception_ce, "FFI passing array is not implemented");
+                       zend_ffi_pass_incompatible(arg, type, n, execute_data);
                        return FAILURE;
                default:
-                       zend_throw_error(zend_ffi_exception_ce, "FFI internal error");
+                       zend_ffi_pass_unsupported(type);
                        return FAILURE;
        }
        return SUCCESS;
 }
 /* }}} */
 
-static int zend_ffi_pass_var_arg(zval *arg, ffi_type **pass_type, void **arg_values, uint32_t n) /* {{{ */
+static int zend_ffi_pass_var_arg(zval *arg, ffi_type **pass_type, void **arg_values, uint32_t n, zend_execute_data *execute_data) /* {{{ */
 {
        ZVAL_DEREF(arg);
        switch (Z_TYPE_P(arg)) {
@@ -2320,11 +2390,11 @@ static int zend_ffi_pass_var_arg(zval *arg, ffi_type **pass_type, void **arg_val
                                zend_ffi_cdata *cdata = (zend_ffi_cdata*)Z_OBJ_P(arg);
                                zend_ffi_type *type = ZEND_FFI_TYPE(cdata->type);
 
-                               return zend_ffi_pass_arg(arg, type, pass_type, arg_values, n);
+                               return zend_ffi_pass_arg(arg, type, pass_type, arg_values, n, execute_data);
                        }
                        /* break missing intentionally */
                default:
-                       zend_throw_error(zend_ffi_exception_ce, "FFI internal error");
+                       zend_throw_error(zend_ffi_exception_ce, "Unsupported argument type");
                        return FAILURE;
        }
        return SUCCESS;
@@ -2349,8 +2419,8 @@ static ZEND_FUNCTION(ffi_trampoline) /* {{{ */
        arg_count = type->func.args ? zend_hash_num_elements(type->func.args) : 0;
        if (type->attr & ZEND_FFI_ATTR_VARIADIC) {
                if (arg_count > EX_NUM_ARGS()) {
-                       zend_throw_error(zend_ffi_exception_ce, "Incorrect number of arguments for C function '%s'", ZSTR_VAL(EX(func)->internal_function.function_name));
-                       return;
+                       zend_throw_error(zend_ffi_exception_ce, "Incorrect number of arguments for C function '%s', expecting at least %d parameter%s", ZSTR_VAL(EX(func)->internal_function.function_name), arg_count, (arg_count != 1) ? "s" : "");
+                       goto exit;
                }
                if (EX_NUM_ARGS()) {
                        arg_types = do_alloca(
@@ -2362,39 +2432,40 @@ static ZEND_FUNCTION(ffi_trampoline) /* {{{ */
                                ZEND_HASH_FOREACH_PTR(type->func.args, arg_type) {
                                        arg_type = ZEND_FFI_TYPE(arg_type);
                                        arg_values[n] = ((char*)arg_values) + (sizeof(void*) * EX_NUM_ARGS()) + (FFI_SIZEOF_ARG * n);
-                                       if (zend_ffi_pass_arg(EX_VAR_NUM(n), arg_type, &arg_types[n], arg_values, n) != SUCCESS) {
+                                       if (zend_ffi_pass_arg(EX_VAR_NUM(n), arg_type, &arg_types[n], arg_values, n, execute_data) != SUCCESS) {
                                                free_alloca(arg_types, arg_types_use_heap);
                                                free_alloca(arg_values, arg_values_use_heap);
-                                               return;
+                                               goto exit;
                                        }
                                        n++;
                                } ZEND_HASH_FOREACH_END();
                        }
                        for (; n < EX_NUM_ARGS(); n++) {
                                arg_values[n] = ((char*)arg_values) + (sizeof(void*) * EX_NUM_ARGS()) + (FFI_SIZEOF_ARG * n);
-                               if (zend_ffi_pass_var_arg(EX_VAR_NUM(n), &arg_types[n], arg_values, n) != SUCCESS) {
+                               if (zend_ffi_pass_var_arg(EX_VAR_NUM(n), &arg_types[n], arg_values, n, execute_data) != SUCCESS) {
                                        free_alloca(arg_types, arg_types_use_heap);
                                        free_alloca(arg_values, arg_values_use_heap);
-                                       return;
+                                       goto exit;
                                }
                        }
                }
                ret_type = zend_ffi_get_type(ZEND_FFI_TYPE(type->func.ret_type));
                if (!ret_type) {
+                       zend_ffi_return_unsupported(type->func.ret_type);
                        free_alloca(arg_types, arg_types_use_heap);
                        free_alloca(arg_values, arg_values_use_heap);
-                       return;
+                       goto exit;
                }
                if (ffi_prep_cif_var(&cif, type->func.abi, arg_count, EX_NUM_ARGS(), ret_type, arg_types) != FFI_OK) {
-                       zend_throw_error(zend_ffi_exception_ce, "FFI internal error");
+                       zend_throw_error(zend_ffi_exception_ce, "Cannot prepare callback CIF");
                        free_alloca(arg_types, arg_types_use_heap);
                        free_alloca(arg_values, arg_values_use_heap);
-                       return;
+                       goto exit;
                }
        } else {
                if (arg_count != EX_NUM_ARGS()) {
-                       zend_throw_error(zend_ffi_exception_ce, "Incorrect number of arguments for C function '%s'", ZSTR_VAL(EX(func)->internal_function.function_name));
-                       return;
+                       zend_throw_error(zend_ffi_exception_ce, "Incorrect number of arguments for C function '%s', expecting exactly %d parameter%s", ZSTR_VAL(EX(func)->internal_function.function_name), arg_count, (arg_count != 1) ? "s" : "");
+                       goto exit;
                }
                if (EX_NUM_ARGS()) {
                        arg_types = do_alloca(
@@ -2406,10 +2477,10 @@ static ZEND_FUNCTION(ffi_trampoline) /* {{{ */
                                ZEND_HASH_FOREACH_PTR(type->func.args, arg_type) {
                                        arg_type = ZEND_FFI_TYPE(arg_type);
                                        arg_values[n] = ((char*)arg_values) + (sizeof(void*) * EX_NUM_ARGS()) + (FFI_SIZEOF_ARG * n);
-                                       if (zend_ffi_pass_arg(EX_VAR_NUM(n), arg_type, &arg_types[n], arg_values, n) != SUCCESS) {
+                                       if (zend_ffi_pass_arg(EX_VAR_NUM(n), arg_type, &arg_types[n], arg_values, n, execute_data) != SUCCESS) {
                                                free_alloca(arg_types, arg_types_use_heap);
                                                free_alloca(arg_values, arg_values_use_heap);
-                                               return;
+                                               goto exit;
                                        }
                                        n++;
                                } ZEND_HASH_FOREACH_END();
@@ -2417,15 +2488,16 @@ static ZEND_FUNCTION(ffi_trampoline) /* {{{ */
                }
                ret_type = zend_ffi_get_type(ZEND_FFI_TYPE(type->func.ret_type));
                if (!ret_type) {
+                       zend_ffi_return_unsupported(type->func.ret_type);
                        free_alloca(arg_types, arg_types_use_heap);
                        free_alloca(arg_values, arg_values_use_heap);
-                       return;
+                       goto exit;
                }
                if (ffi_prep_cif(&cif, type->func.abi, arg_count, ret_type, arg_types) != FFI_OK) {
-                       zend_throw_error(zend_ffi_exception_ce, "FFI internal error");
+                       zend_throw_error(zend_ffi_exception_ce, "Cannot prepare callback CIF");
                        free_alloca(arg_types, arg_types_use_heap);
                        free_alloca(arg_values, arg_values_use_heap);
-                       return;
+                       goto exit;
                }
        }
 
@@ -2447,6 +2519,7 @@ static ZEND_FUNCTION(ffi_trampoline) /* {{{ */
 
        zend_ffi_cdata_to_zval(NULL, (void*)&ret, ZEND_FFI_TYPE(type->func.ret_type), BP_VAR_R, return_value, 0, 1);
 
+exit:
        zend_string_release(EX(func)->common.function_name);
        if (EX(func)->common.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE) {
                zend_free_trampoline(EX(func));

diff --git a/ext/ffi/tests/bug77706.phpt b/ext/ffi/tests/bug77706.phpt
new file mode 100644 (file)
index 0000000..fca6208
--- /dev/null
+++ b/ext/ffi/tests/bug77706.phpt
@@ -0,0 +1,60 @@
+--TEST--
+Bug #77632 (FFI Segfaults When Called With Variadics)
+--SKIPIF--
+<?php
+require_once('skipif.inc');
+try {
+       $libc = FFI::cdef("int printf(const char *format, ...);", "libc.so.6");
+} catch (Throwable $_) {
+       die('skip libc.so.6 not available');
+}
+?>
+--INI--
+ffi.enable=1
+--FILE--
+<?php
+$header = '
+typedef struct _IO_FILE FILE;
+extern FILE *stdout;
+extern FILE *stdin;
+extern FILE *stderr;
+
+typedef uint64_t time_t;
+typedef uint32_t pid_t;
+
+time_t time(time_t*);
+pid_t getpid(void);
+int fprintf(FILE *, const char *, ...);
+';
+
+$ffi = FFI::cdef($header, 'libc.so.6');
+
+try {
+       $ffi->time();
+} catch (Throwable $e) {
+       echo get_class($e) . ": " . $e->getMessage() . "\n";
+}
+
+try {
+       $ffi->time(null, null);
+} catch (Throwable $e) {
+       echo get_class($e) . ": " . $e->getMessage() . "\n";
+}
+
+try {
+       $ffi->fprintf($ffi->stdout);
+} catch (Throwable $e) {
+       echo get_class($e) . ": " . $e->getMessage() . "\n";
+}
+
+try {
+       $ffi->fprintf($ffi->stdout, 123, "Hello %s\n", "World");
+} catch (Throwable $e) {
+       echo get_class($e) . ": " . $e->getMessage() . "\n";
+}
+?>
+--EXPECT--
+FFI\Exception: Incorrect number of arguments for C function 'time', expecting exactly 1 parameter
+FFI\Exception: Incorrect number of arguments for C function 'time', expecting exactly 1 parameter
+FFI\Exception: Incorrect number of arguments for C function 'fprintf', expecting at least 2 parameters
+FFI\Exception: Passing incompatible argument 2 of C function 'fprintf', expecting 'char*', found PHP 'int'