Fixed bug #80847 (CData structs with fields of type struct can't be passed as C function argument)

Author: dstogov

NEWS

@@ -12,6 +12,10 @@ PHP                                                                        NEWS
 - Dba:
   . Fixed bug #80817 (dba_popen() may cause segfault during RSHUTDOWN). (cmb)
 
+- FFI:
+  . Fixed bug #80847 (CData structs with fields of type struct can't be passed
+    as C function argument). (Nickolas Daniel da Silva, Dmitry)
+
 - IMAP:
   . Fixed bug #80800 (imap_open() fails when the flags parameter includes
     CL_EXPUNGE). (girgias)

ext/ffi/ffi.c

@@ -295,56 +295,13 @@ static int zend_ffi_is_compatible_type(zend_ffi_type *dst_type, zend_ffi_type *s
 
 static ffi_type *zend_ffi_make_fake_struct_type(zend_ffi_type *type) /* {{{ */
 {
-	ffi_type *t = emalloc(sizeof(ffi_type) + sizeof(ffi_type*) * (zend_hash_num_elements(&type->record.fields) + 1));
-	int i;
-	zend_ffi_field *field;
+	ffi_type *t = emalloc(sizeof(ffi_type) + sizeof(ffi_type*));
 
 	t->size = type->size;
 	t->alignment = type->align;
 	t->type = FFI_TYPE_STRUCT;
 	t->elements = (ffi_type**)(t + 1);
-	i = 0;
-	ZEND_HASH_FOREACH_PTR(&type->record.fields, field) {
-		switch (ZEND_FFI_TYPE(field->type)->kind) {
-			case ZEND_FFI_TYPE_FLOAT:
-				t->elements[i] = &ffi_type_float;
-				break;
-			case ZEND_FFI_TYPE_DOUBLE:
-				t->elements[i] = &ffi_type_double;
-				break;
-#ifdef HAVE_LONG_DOUBLE
-			case ZEND_FFI_TYPE_LONGDOUBLE:
-				t->elements[i] = &ffi_type_longdouble;
-				break;
-#endif
-			case ZEND_FFI_TYPE_SINT8:
-			case ZEND_FFI_TYPE_UINT8:
-			case ZEND_FFI_TYPE_BOOL:
-			case ZEND_FFI_TYPE_CHAR:
-				t->elements[i] = &ffi_type_uint8;
-				break;
-			case ZEND_FFI_TYPE_SINT16:
-			case ZEND_FFI_TYPE_UINT16:
-				t->elements[i] = &ffi_type_uint16;
-				break;
-			case ZEND_FFI_TYPE_SINT32:
-			case ZEND_FFI_TYPE_UINT32:
-				t->elements[i] = &ffi_type_uint32;
-				break;
-			case ZEND_FFI_TYPE_SINT64:
-			case ZEND_FFI_TYPE_UINT64:
-				t->elements[i] = &ffi_type_uint64;
-				break;
-			case ZEND_FFI_TYPE_POINTER:
-				t->elements[i] = &ffi_type_pointer;
-				break;
-			default:
-				efree(t);
-				return NULL;
-			}
-		i++;
-	} ZEND_HASH_FOREACH_END();
-	t->elements[i] = NULL;
+	t->elements[0] = NULL;
 	return t;
 }
 /* }}} */
@@ -391,11 +348,7 @@ static ffi_type *zend_ffi_get_type(zend_ffi_type *type) /* {{{ */
 			kind = type->enumeration.kind;
 			goto again;
 		case ZEND_FFI_TYPE_STRUCT:
-			if (!(type->attr & ZEND_FFI_ATTR_UNION)) {
-				ffi_type *t = zend_ffi_make_fake_struct_type(type);
-				return t;
-			}
-			break;
+			return zend_ffi_make_fake_struct_type(type);
 		default:
 			break;
 	}
@@ -2534,18 +2487,13 @@ static int zend_ffi_pass_arg(zval *arg, zend_ffi_type *type, ffi_type **pass_typ
 			kind = type->enumeration.kind;
 			goto again;
 		case ZEND_FFI_TYPE_STRUCT:
-			if (!(type->attr & ZEND_FFI_ATTR_UNION)
-			 && Z_TYPE_P(arg) == IS_OBJECT && Z_OBJCE_P(arg) == zend_ffi_cdata_ce) {
+			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);
 
 				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) {
-						*pass_type = t;
-						arg_values[n] = cdata->ptr;
-						break;
-					}
+					*pass_type = zend_ffi_make_fake_struct_type(type);;
+					arg_values[n] = cdata->ptr;
+					break;
 				}
 			}
 			zend_ffi_pass_incompatible(arg, type, n, execute_data);

ext/ffi/tests/bug80847.phpt

@@ -0,0 +1,58 @@
+--TEST--
+Bug #80847 (Nested structs)
+--SKIPIF--
+<?php
+if (!extension_loaded('ffi')) die('skip ffi extension not available');
+if (!extension_loaded('zend-test')) die('skip zend-test extension not available');
+?>
+--FILE--
+<?php
+require_once('utils.inc');
+$header = <<<HEADER
+    typedef struct bug80847_01 {
+        uint64_t b;
+        double c;
+    } bug80847_01;
+
+    typedef struct bug80847_02 {
+        bug80847_01 a;
+    } bug80847_02;
+
+	bug80847_02 ffi_bug80847(bug80847_02 s);
+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());
+    }
+}
+$x = $ffi->new('bug80847_02');
+$x->a->b = 42;
+$x->a->c = 42.5;
+var_dump($x);
+$y = $ffi->ffi_bug80847($x);
+var_dump($y);
+?>
+--EXPECTF--
+object(FFI\CData:struct bug80847_02)#%d (1) {
+  ["a"]=>
+  object(FFI\CData:struct bug80847_01)#%d (2) {
+    ["b"]=>
+    int(42)
+    ["c"]=>
+    float(42.5)
+  }
+}
+object(FFI\CData:struct bug80847_02)#%d (1) {
+  ["a"]=>
+  object(FFI\CData:struct bug80847_01)#%d (2) {
+    ["b"]=>
+    int(52)
+    ["c"]=>
+    float(32.5)
+  }
+}

ext/zend_test/test.c

@@ -665,3 +665,17 @@ void bug79177(void)
 {
 	bug79177_cb();
 }
+
+typedef struct bug80847_01 {
+	uint64_t b;
+	double c;
+} bug80847_01;
+typedef struct bug80847_02 {
+	bug80847_01 a;
+} bug80847_02;
+
+PHP_ZEND_TEST_API bug80847_02 ffi_bug80847(bug80847_02 s) {
+	s.a.b += 10;
+	s.a.c -= 10.0;
+    return s;
+}