From 785ee844cbeeef364da3b92008c211e33e49428b 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 1/6] Implement GH-11934: Allow to pass CData into struct
 and/or union fields

Implementation based on a discussion with KapitanOczywisty.
---
 ext/ffi/ffi.c              |  19 +++---
 ext/ffi/tests/gh11934.phpt | 123 +++++++++++++++++++++++++++++++++++++
 2 files changed, 134 insertions(+), 8 deletions(-)
 create mode 100644 ext/ffi/tests/gh11934.phpt

diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c
index f2d03699bab33..b8fe995e1f023 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 @@ static zend_always_inline zend_result zend_ffi_zval_to_cdata(void *ptr, zend_ffi
 		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 0000000000000..b2a767ce74168
--- /dev/null
+++ b/ext/ffi/tests/gh11934.phpt
@@ -0,0 +1,123 @@
+--TEST--
+Feature GH-11934 (Allow to pass CData into struct and/or union fields)
+--EXTENSIONS--
+ffi
+--INI--
+ffi.enable=1
+--FILE--
+<?php
+
+$cdef = FFI::cdef();
+
+echo "--- Primitive types test ---\n";
+
+// Choose integer numbers off the maximum to test copy
+$types = [
+    'uint8_t' => 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 "--- Complex types test ---\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);
+
+?>
+--EXPECTF--
+--- Primitive types test ---
+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
+--- Complex types test ---
+bool(true)

From 5f03c5d5a61d7da4a24cdea0d70979dc2ae49935 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Fri, 11 Aug 2023 20:39:48 +0200
Subject: [PATCH 2/6] Add additional tests

Co-authored-by: KapitanOczywisty <44417092+KapitanOczywisty@users.noreply.github.com>
---
 ext/ffi/tests/gh11934.phpt | 95 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 91 insertions(+), 4 deletions(-)

diff --git a/ext/ffi/tests/gh11934.phpt b/ext/ffi/tests/gh11934.phpt
index b2a767ce74168..41f9b987a0452 100644
--- a/ext/ffi/tests/gh11934.phpt
+++ b/ext/ffi/tests/gh11934.phpt
@@ -9,7 +9,7 @@ ffi.enable=1
 
 $cdef = FFI::cdef();
 
-echo "--- Primitive types test ---\n";
+echo "--- Primitive types ---\n";
 
 // Choose integer numbers off the maximum to test copy
 $types = [
@@ -54,7 +54,7 @@ echo "Positive test enum: ";
 var_dump($struct->field === $source->cdata);
 $struct->field = $dummy;
 
-echo "--- Complex types test ---\n";
+echo "--- Struct ---\n";
 
 // Nested structs
 $cdef = FFI::cdef("
@@ -77,9 +77,85 @@ $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 "--- 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]);
+
+echo "--- Existing C variables ---\n";
+
+if (PHP_OS_FAMILY !== 'Windows') {
+    $ffi = FFI::cdef(
+        "int errno;",
+        "libc.so.6"
+    );
+    $ffi->errno = 0;
+    var_dump($ffi->errno);
+    $source = $ffi->new('int');
+    $source->cdata = 31;
+    $ffi->errno = $source;
+    var_dump($ffi->errno);
+} else {
+    // Untested on Windows due to lack of libc.so.6
+    var_dump(31);
+}
+
 ?>
 --EXPECTF--
---- Primitive types test ---
+--- Primitive types ---
 Positive test uint8_t: bool(true)
 Positive test uint16_t: bool(true)
 Positive test uint32_t: bool(true)
@@ -119,5 +195,16 @@ Warning: Object of class FFI\CData could not be converted to int in %s on line %
 Positive test enum: bool(true)
 
 Warning: Object of class FFI\CData could not be converted to int in %s on line %d
---- Complex types test ---
+--- Struct ---
 bool(true)
+--- Callback return type ---
+int(42)
+--- Callback return type ---
+int(42)
+--- Other FFI\CData assignment ---
+int(123)
+--- Array element ---
+int(123)
+--- Existing C variables ---
+int(0)
+int(31)

From 2c624e42b22a29f287ec1417dda56a12ebc38a80 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Fri, 11 Aug 2023 21:03:50 +0200
Subject: [PATCH 3/6] Fix test

---
 ext/ffi/tests/gh11934.phpt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ext/ffi/tests/gh11934.phpt b/ext/ffi/tests/gh11934.phpt
index 41f9b987a0452..6643578b18351 100644
--- a/ext/ffi/tests/gh11934.phpt
+++ b/ext/ffi/tests/gh11934.phpt
@@ -137,7 +137,7 @@ var_dump($target[0]);
 
 echo "--- Existing C variables ---\n";
 
-if (PHP_OS_FAMILY !== 'Windows') {
+if (PHP_OS_FAMILY === 'Linux') {
     $ffi = FFI::cdef(
         "int errno;",
         "libc.so.6"
@@ -149,7 +149,8 @@ if (PHP_OS_FAMILY !== 'Windows') {
     $ffi->errno = $source;
     var_dump($ffi->errno);
 } else {
-    // Untested on Windows due to lack of libc.so.6
+    // Untested on non-Linux due to lack of libc.so.6
+    var_dump(0);
     var_dump(31);
 }
 

From 69d8f1c8425979e38f15e020849eb0cb859fe03a Mon Sep 17 00:00:00 2001
From: KapitanOczywisty <44417092+KapitanOczywisty@users.noreply.github.com>
Date: Fri, 11 Aug 2023 21:06:38 +0000
Subject: [PATCH 4/6] Separate test for C variables

---
 ext/ffi/tests/gh11934.phpt  | 22 ----------------------
 ext/ffi/tests/gh11934b.phpt | 32 ++++++++++++++++++++++++++++++++
 ext/zend_test/test.c        |  2 ++
 3 files changed, 34 insertions(+), 22 deletions(-)
 create mode 100644 ext/ffi/tests/gh11934b.phpt

diff --git a/ext/ffi/tests/gh11934.phpt b/ext/ffi/tests/gh11934.phpt
index 6643578b18351..98a2a4cb25b48 100644
--- a/ext/ffi/tests/gh11934.phpt
+++ b/ext/ffi/tests/gh11934.phpt
@@ -135,25 +135,6 @@ $target[0] = $source;
 
 var_dump($target[0]);
 
-echo "--- Existing C variables ---\n";
-
-if (PHP_OS_FAMILY === 'Linux') {
-    $ffi = FFI::cdef(
-        "int errno;",
-        "libc.so.6"
-    );
-    $ffi->errno = 0;
-    var_dump($ffi->errno);
-    $source = $ffi->new('int');
-    $source->cdata = 31;
-    $ffi->errno = $source;
-    var_dump($ffi->errno);
-} else {
-    // Untested on non-Linux due to lack of libc.so.6
-    var_dump(0);
-    var_dump(31);
-}
-
 ?>
 --EXPECTF--
 --- Primitive types ---
@@ -206,6 +187,3 @@ int(42)
 int(123)
 --- Array element ---
 int(123)
---- Existing C variables ---
-int(0)
-int(31)
diff --git a/ext/ffi/tests/gh11934b.phpt b/ext/ffi/tests/gh11934b.phpt
new file mode 100644
index 0000000000000..c058c91b038f7
--- /dev/null
+++ b/ext/ffi/tests/gh11934b.phpt
@@ -0,0 +1,32 @@
+--TEST--
+Feature GH-11934 (Allow to pass CData into C variables)
+--EXTENSIONS--
+ffi
+zend_test
+--FILE--
+<?php
+require_once __DIR__ . '/utils.inc';
+$header = <<<HEADER
+extern int gh11934b_ffi_var_test_cdata;
+HEADER;
+
+if (PHP_OS_FAMILY !== 'Windows') {
+    $ffi = FFI::cdef($header);
+} else {
+    try {
+        $ffi = FFI::cdef($header, 'php_zend_test.dll');
+    } catch (FFI\Exception $ex) {
+        $ffi = FFI::cdef($header, ffi_get_php_dll_name());
+    }
+}
+
+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 b4ea2b27c9e9d..01ead8cd0fdf3 100644
--- a/ext/zend_test/test.c
+++ b/ext/zend_test/test.c
@@ -1108,6 +1108,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 = 2;
+
 #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.

From a8f7704837a993b13a3b544abb95be8873236164 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Fri, 11 Aug 2023 23:26:03 +0200
Subject: [PATCH 5/6] Remove duplicate test

---
 ext/ffi/tests/gh11934.phpt | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/ext/ffi/tests/gh11934.phpt b/ext/ffi/tests/gh11934.phpt
index 98a2a4cb25b48..96377c4822839 100644
--- a/ext/ffi/tests/gh11934.phpt
+++ b/ext/ffi/tests/gh11934.phpt
@@ -95,24 +95,6 @@ $struct->call_me = function () use ($ffi) {
 
 var_dump(($struct->call_me)());
 
-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('');
@@ -181,8 +163,6 @@ Warning: Object of class FFI\CData could not be converted to int in %s on line %
 bool(true)
 --- Callback return type ---
 int(42)
---- Callback return type ---
-int(42)
 --- Other FFI\CData assignment ---
 int(123)
 --- Array element ---

From 29193861a8c8a7793ad2194e363a87f2a7bb1614 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sat, 12 Aug 2023 00:14:37 +0200
Subject: [PATCH 6/6] Fix test under repetition

---
 ext/ffi/tests/gh11934b.phpt | 1 +
 ext/zend_test/test.c        | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/ext/ffi/tests/gh11934b.phpt b/ext/ffi/tests/gh11934b.phpt
index c058c91b038f7..707c27c889cde 100644
--- a/ext/ffi/tests/gh11934b.phpt
+++ b/ext/ffi/tests/gh11934b.phpt
@@ -20,6 +20,7 @@ if (PHP_OS_FAMILY !== 'Windows') {
     }
 }
 
+$ffi->gh11934b_ffi_var_test_cdata->cdata = 2;
 var_dump($ffi->gh11934b_ffi_var_test_cdata);
 $source = $ffi->new('int');
 $source->cdata = 31;
diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c
index 01ead8cd0fdf3..78d1919a6fbf1 100644
--- a/ext/zend_test/test.c
+++ b/ext/zend_test/test.c
@@ -1108,7 +1108,7 @@ 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 = 2;
+PHP_ZEND_TEST_API int gh11934b_ffi_var_test_cdata;
 
 #ifdef HAVE_COPY_FILE_RANGE
 /**