Fix leaking definitions on FFI::cdef()->new()

Previously, FFI_G(symbols) and FFI_G(tags) were never cleaned up when calling
new on an existing object. However, if cdef() is called without parameters these
globals are NULL and might be created when new() creates new definitions. These
would then be discarded without freeing them.

Closes GH-11751

NEWS

@@ -2,7 +2,8 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? ????, PHP 8.1.23
 
-
+- FFI:
+  . Fix leaking definitions when using FFI::cdef()->new(...). (ilutov)
 
 03 Aug 2023, PHP 8.1.22
 

ext/ffi/ffi.c

@@ -3684,23 +3684,23 @@ ZEND_METHOD(FFI, new) /* {{{ */
 			FFI_G(symbols) = NULL;
 			FFI_G(tags) = NULL;
 		}
+		bool clean_symbols = FFI_G(symbols) == NULL;
+		bool clean_tags = FFI_G(tags) == NULL;
 
 		FFI_G(default_type_attr) = 0;
 
 		if (zend_ffi_parse_type(ZSTR_VAL(type_def), ZSTR_LEN(type_def), &dcl) != SUCCESS) {
 			zend_ffi_type_dtor(dcl.type);
-			if (Z_TYPE(EX(This)) != IS_OBJECT) {
+			if (clean_tags && FFI_G(tags)) {
-				if (FFI_G(tags)) {
 				zend_hash_destroy(FFI_G(tags));
 				efree(FFI_G(tags));
 				FFI_G(tags) = NULL;
 			}
-				if (FFI_G(symbols)) {
+			if (clean_symbols && FFI_G(symbols)) {
 				zend_hash_destroy(FFI_G(symbols));
 				efree(FFI_G(symbols));
 				FFI_G(symbols) = NULL;
 			}
-			}
 			return;
 		}
 
@@ -3709,16 +3709,14 @@ ZEND_METHOD(FFI, new) /* {{{ */
 			is_const = 1;
 		}
 
-		if (Z_TYPE(EX(This)) != IS_OBJECT) {
+		if (clean_tags && FFI_G(tags)) {
-			if (FFI_G(tags)) {
 			zend_ffi_tags_cleanup(&dcl);
 		}
-			if (FFI_G(symbols)) {
+		if (clean_symbols && FFI_G(symbols)) {
 			zend_hash_destroy(FFI_G(symbols));
 			efree(FFI_G(symbols));
 			FFI_G(symbols) = NULL;
 		}
-		}
 		FFI_G(symbols) = NULL;
 		FFI_G(tags) = NULL;
 
@@ -3828,23 +3826,23 @@ ZEND_METHOD(FFI, cast) /* {{{ */
 			FFI_G(symbols) = NULL;
 			FFI_G(tags) = NULL;
 		}
+		bool clean_symbols = FFI_G(symbols) == NULL;
+		bool clean_tags = FFI_G(tags) == NULL;
 
 		FFI_G(default_type_attr) = 0;
 
 		if (zend_ffi_parse_type(ZSTR_VAL(type_def), ZSTR_LEN(type_def), &dcl) != SUCCESS) {
 			zend_ffi_type_dtor(dcl.type);
-			if (Z_TYPE(EX(This)) != IS_OBJECT) {
+			if (clean_tags && FFI_G(tags)) {
-				if (FFI_G(tags)) {
 				zend_hash_destroy(FFI_G(tags));
 				efree(FFI_G(tags));
 				FFI_G(tags) = NULL;
 			}
-				if (FFI_G(symbols)) {
+			if (clean_symbols && FFI_G(symbols)) {
 				zend_hash_destroy(FFI_G(symbols));
 				efree(FFI_G(symbols));
 				FFI_G(symbols) = NULL;
 			}
-			}
 			return;
 		}
 
@@ -3853,16 +3851,14 @@ ZEND_METHOD(FFI, cast) /* {{{ */
 			is_const = 1;
 		}
 
-		if (Z_TYPE(EX(This)) != IS_OBJECT) {
+		if (clean_tags && FFI_G(tags)) {
-			if (FFI_G(tags)) {
 			zend_ffi_tags_cleanup(&dcl);
 		}
-			if (FFI_G(symbols)) {
+		if (clean_symbols && FFI_G(symbols)) {
 			zend_hash_destroy(FFI_G(symbols));
 			efree(FFI_G(symbols));
 			FFI_G(symbols) = NULL;
 		}
-		}
 		FFI_G(symbols) = NULL;
 		FFI_G(tags) = NULL;
 
@@ -3994,36 +3990,34 @@ ZEND_METHOD(FFI, type) /* {{{ */
 		FFI_G(symbols) = NULL;
 		FFI_G(tags) = NULL;
 	}
+	bool clean_symbols = FFI_G(symbols) == NULL;
+	bool clean_tags = FFI_G(tags) == NULL;
 
 	FFI_G(default_type_attr) = 0;
 
 	if (zend_ffi_parse_type(ZSTR_VAL(type_def), ZSTR_LEN(type_def), &dcl) != SUCCESS) {
 		zend_ffi_type_dtor(dcl.type);
-		if (Z_TYPE(EX(This)) != IS_OBJECT) {
+		if (clean_tags && FFI_G(tags)) {
-			if (FFI_G(tags)) {
 			zend_hash_destroy(FFI_G(tags));
 			efree(FFI_G(tags));
 			FFI_G(tags) = NULL;
 		}
-			if (FFI_G(symbols)) {
+		if (clean_symbols && FFI_G(symbols)) {
 			zend_hash_destroy(FFI_G(symbols));
 			efree(FFI_G(symbols));
 			FFI_G(symbols) = NULL;
 		}
-		}
 		return;
 	}
 
-	if (Z_TYPE(EX(This)) != IS_OBJECT) {
+	if (clean_tags && FFI_G(tags)) {
-		if (FFI_G(tags)) {
 		zend_ffi_tags_cleanup(&dcl);
 	}
-		if (FFI_G(symbols)) {
+	if (clean_symbols && FFI_G(symbols)) {
 		zend_hash_destroy(FFI_G(symbols));
 		efree(FFI_G(symbols));
 		FFI_G(symbols) = NULL;
 	}
-	}
 	FFI_G(symbols) = NULL;
 	FFI_G(tags) = NULL;
 

ext/ffi/tests/cdef_new.phpt

@@ -0,0 +1,14 @@
+--TEST--
+Definitions should not leak when using FFI::cdef()->new(...)
+--EXTENSIONS--
+ffi
+--FILE--
+<?php
+$struct = \FFI::cdef()->new('struct Example { uint32_t x; }');
+var_dump($struct);
+?>
+--EXPECT--
+object(FFI\CData:struct Example)#2 (1) {
+  ["x"]=>
+  int(0)
+}