From 0b9702c9ed547b3adae9b05dbe49d4a5dd747275 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 11 Aug 2023 00:51:48 +0200 Subject: [PATCH] Implement GH-11934: Allow to pass CData into struct and/or union fields Co-authored-by: KapitanOczywisty <44417092+KapitanOczywisty@users.noreply.github.com> Closes GH-11935. --- NEWS | 4 + UPGRADING | 4 + ext/ffi/ffi.c | 19 ++-- ext/ffi/tests/gh11934.phpt | 169 ++++++++++++++++++++++++++++++++++++ ext/ffi/tests/gh11934b.phpt | 33 +++++++ ext/zend_test/test.c | 2 + 6 files changed, 223 insertions(+), 8 deletions(-) create mode 100644 ext/ffi/tests/gh11934.phpt create mode 100644 ext/ffi/tests/gh11934b.phpt diff --git a/NEWS b/NEWS index 9ae7df82579..93ac7dfde3b 100644 --- a/NEWS +++ b/NEWS @@ -15,6 +15,10 @@ PHP NEWS . Fixed GH-11952 (Confusing warning when blocking entity loading via libxml_set_external_entity_loader). (nielsdos) +- FFI: + . Implement GH-11934 (Allow to pass CData into struct and/or union fields). + (nielsdos, KapitanOczywisty) + - FPM: . Fixed bug #76067 (system() function call leaks php-fpm listening sockets). (Mikhail Galanin, Jakub Zelenka) diff --git a/UPGRADING b/UPGRADING index e0082c2c34d..972ba3f48d6 100644 --- a/UPGRADING +++ b/UPGRADING @@ -126,6 +126,10 @@ PHP 8.3 UPGRADE NOTES - CLI . It is now possible to lint multiple files. +- FFI + . It is now possible to assign CData to other CData. This means you can + now assign CData to structs and fields. + - Opcache . opcache_get_status()['scripts'][n]['revalidate'] now contains a Unix timestamp of when the next revalidation of the scripts timestamp is due, diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c index f2d03699bab..b8fe995e1f0 100644 --- a/ext/ffi/ffi.c +++ b/ext/ffi/ffi.c @@ -739,6 +739,16 @@ static zend_always_inline zend_result zend_ffi_zval_to_cdata(void *ptr, zend_ffi zend_string *str; zend_ffi_type_kind kind = type->kind; + /* Pointer type has special handling of CData */ + if (kind != ZEND_FFI_TYPE_POINTER && Z_TYPE_P(value) == IS_OBJECT && Z_OBJCE_P(value) == zend_ffi_cdata_ce) { + zend_ffi_cdata *cdata = (zend_ffi_cdata*)Z_OBJ_P(value); + if (zend_ffi_is_compatible_type(type, ZEND_FFI_TYPE(cdata->type)) && + type->size == ZEND_FFI_TYPE(cdata->type)->size) { + memcpy(ptr, cdata->ptr, type->size); + return SUCCESS; + } + } + again: switch (kind) { case ZEND_FFI_TYPE_FLOAT: @@ -848,14 +858,7 @@ again: case ZEND_FFI_TYPE_STRUCT: case ZEND_FFI_TYPE_ARRAY: default: - if (Z_TYPE_P(value) == IS_OBJECT && Z_OBJCE_P(value) == zend_ffi_cdata_ce) { - zend_ffi_cdata *cdata = (zend_ffi_cdata*)Z_OBJ_P(value); - if (zend_ffi_is_compatible_type(type, ZEND_FFI_TYPE(cdata->type)) && - type->size == ZEND_FFI_TYPE(cdata->type)->size) { - memcpy(ptr, cdata->ptr, type->size); - return SUCCESS; - } - } + /* Incompatible types, because otherwise the CData check at the entry point would've succeeded. */ zend_ffi_assign_incompatible(value, type); return FAILURE; } diff --git a/ext/ffi/tests/gh11934.phpt b/ext/ffi/tests/gh11934.phpt new file mode 100644 index 00000000000..96377c48228 --- /dev/null +++ b/ext/ffi/tests/gh11934.phpt @@ -0,0 +1,169 @@ +--TEST-- +Feature GH-11934 (Allow to pass CData into struct and/or union fields) +--EXTENSIONS-- +ffi +--INI-- +ffi.enable=1 +--FILE-- + 200, + 'uint16_t' => 16000, + 'uint32_t' => 1000_000, + 'uint64_t' => PHP_INT_MAX - 100, + 'int8_t' => -100, + 'int16_t' => -16000, + 'int32_t' => -1000_000, + 'int64_t' => PHP_INT_MIN + 100, + 'char' => 'F', + 'bool' => false, + 'float' => 1.00, + 'double' => -1.00, +]; + +// Positive test +foreach ($types as $type => $value) { + $source = $cdef->new($type); + $source->cdata = $value; + $struct = $cdef->new("struct { $type field; }"); + $struct->field = $source; + echo "Positive test $type: "; + var_dump($struct->field === $value); +} + +// Negative test +$dummy = $cdef->new("int[2]"); +foreach ($types as $type => $value) { + $struct = $cdef->new("struct { int field; }"); + $struct->field = $dummy; +} + +// Enum test +$enum_definition = "enum { A, B, C, D }"; +$source = $cdef->new($enum_definition); +$source->cdata = 2; +$struct = $cdef->new("struct { $enum_definition field; }"); +$struct->field = $source; +echo "Positive test enum: "; +var_dump($struct->field === $source->cdata); +$struct->field = $dummy; + +echo "--- Struct ---\n"; + +// Nested structs +$cdef = FFI::cdef(" + struct inner_struct { + int data[1]; + }; + struct my_struct { + int foo; + int bar; + struct inner_struct inner; + }; + struct my_nesting_struct { + struct my_struct field; + };"); +$source = $cdef->new("struct my_struct"); +$source->foo = 123; +$source->bar = 456; +$source->inner->data[0] = 789; +$struct = $cdef->new("struct my_nesting_struct"); +$struct->field = $source; +var_dump($struct->field->foo === 123 && $struct->field->bar === 456 && $struct->field->inner->data[0] === 789); + +echo "--- Callback return type ---\n"; + +$ffi = FFI::cdef(' +typedef uint32_t (*test_callback)(); +typedef struct { + test_callback call_me; +} my_struct; +'); + +$struct = $ffi->new('my_struct'); +$struct->call_me = function () use ($ffi) { + $int = $ffi->new('uint32_t'); + $int->cdata = 42; + return $int; +}; + +var_dump(($struct->call_me)()); + +echo "--- Other FFI\CData assignment ---\n"; + +$ffi = FFI::cdef(''); + +$source = $ffi->new('uint32_t'); +$source->cdata = 123; +$target = $ffi->new('uint32_t'); +$target->cdata = $source; + +var_dump($target->cdata); + +echo "--- Array element ---\n"; + +$ffi = FFI::cdef(''); + +$source = $ffi->new('uint32_t'); +$source->cdata = 123; +$target = $ffi->new('uint32_t[1]'); +$target[0] = $source; + +var_dump($target[0]); + +?> +--EXPECTF-- +--- Primitive types --- +Positive test uint8_t: bool(true) +Positive test uint16_t: bool(true) +Positive test uint32_t: bool(true) +Positive test uint64_t: bool(true) +Positive test int8_t: bool(true) +Positive test int16_t: bool(true) +Positive test int32_t: bool(true) +Positive test int64_t: bool(true) +Positive test char: bool(true) +Positive test bool: bool(true) +Positive test float: bool(true) +Positive test double: bool(true) + +Warning: Object of class FFI\CData could not be converted to int in %s on line %d + +Warning: Object of class FFI\CData could not be converted to int in %s on line %d + +Warning: Object of class FFI\CData could not be converted to int in %s on line %d + +Warning: Object of class FFI\CData could not be converted to int in %s on line %d + +Warning: Object of class FFI\CData could not be converted to int in %s on line %d + +Warning: Object of class FFI\CData could not be converted to int in %s on line %d + +Warning: Object of class FFI\CData could not be converted to int in %s on line %d + +Warning: Object of class FFI\CData could not be converted to int in %s on line %d + +Warning: Object of class FFI\CData could not be converted to int in %s on line %d + +Warning: Object of class FFI\CData could not be converted to int in %s on line %d + +Warning: Object of class FFI\CData could not be converted to int in %s on line %d + +Warning: Object of class FFI\CData could not be converted to int in %s on line %d +Positive test enum: bool(true) + +Warning: Object of class FFI\CData could not be converted to int in %s on line %d +--- Struct --- +bool(true) +--- Callback return type --- +int(42) +--- Other FFI\CData assignment --- +int(123) +--- Array element --- +int(123) diff --git a/ext/ffi/tests/gh11934b.phpt b/ext/ffi/tests/gh11934b.phpt new file mode 100644 index 00000000000..707c27c889c --- /dev/null +++ b/ext/ffi/tests/gh11934b.phpt @@ -0,0 +1,33 @@ +--TEST-- +Feature GH-11934 (Allow to pass CData into C variables) +--EXTENSIONS-- +ffi +zend_test +--FILE-- +gh11934b_ffi_var_test_cdata->cdata = 2; +var_dump($ffi->gh11934b_ffi_var_test_cdata); +$source = $ffi->new('int'); +$source->cdata = 31; +$ffi->gh11934b_ffi_var_test_cdata = $source; +var_dump($ffi->gh11934b_ffi_var_test_cdata); + +?> +--EXPECT-- +int(2) +int(31) diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 1b294050a8e..4460f815609 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -1233,6 +1233,8 @@ PHP_ZEND_TEST_API void bug_gh9090_void_int_char_var(int i, char *fmt, ...) { va_end(args); } +PHP_ZEND_TEST_API int gh11934b_ffi_var_test_cdata; + #ifdef HAVE_COPY_FILE_RANGE /** * This function allows us to simulate early return of copy_file_range by setting the limit_copy_file_range ini setting.