Zend: Fix reference counting for Closures in const-expr

Zend/tests/closures/closure_const_expr/attributes.phpt

@@ -8,13 +8,13 @@ reflection
 #[Attribute(Attribute::TARGET_CLASS | Attribute::IS_REPEATABLE)]
 class Attr {
     public function __construct(public Closure $value) {
-      $value('foo');
+        $value('foo');
     }
 }
 
 #[Attr(static function () { })]
 #[Attr(static function (...$args) {
-  var_dump($args);
+    var_dump($args);
 })]
 class C {}
 

Zend/tests/closures/closure_const_expr/attributes_autoload.inc

@@ -0,0 +1,9 @@
+<?php
+
+#[Attr(static function () { })]
+#[Attr(static function (...$args) {
+    var_dump($args);
+})]
+class C {}
+
+?>

Zend/tests/closures/closure_const_expr/attributes_autoload.phpt

@@ -0,0 +1,57 @@
+--TEST--
+GH-17851: Use-after-free when instantiating autoloaded class with attribute having a Closure parameter.
+--EXTENSIONS--
+reflection
+--FILE--
+<?php
+
+spl_autoload_register(static function ($className) {
+    if ($className === 'C') {
+        require(__DIR__ . '/attributes_autoload.inc');
+    }
+});
+
+#[Attribute(Attribute::TARGET_CLASS | Attribute::IS_REPEATABLE)]
+class Attr {
+    public function __construct(public Closure $value) {
+        $value('foo');
+    }
+}
+
+foreach ((new ReflectionClass(C::class))->getAttributes() as $reflectionAttribute) {
+    var_dump($reflectionAttribute->newInstance());
+}
+
+?>
+--EXPECTF--
+object(Attr)#%d (1) {
+  ["value"]=>
+  object(Closure)#%d (3) {
+    ["name"]=>
+    string(%d) "{closure:%s:%d}"
+    ["file"]=>
+    string(%d) "%s"
+    ["line"]=>
+    int(%d)
+  }
+}
+array(1) {
+  [0]=>
+  string(3) "foo"
+}
+object(Attr)#%d (1) {
+  ["value"]=>
+  object(Closure)#%d (4) {
+    ["name"]=>
+    string(%d) "{closure:%s:%d}"
+    ["file"]=>
+    string(%d) "%s"
+    ["line"]=>
+    int(%d)
+    ["parameter"]=>
+    array(1) {
+      ["$args"]=>
+      string(10) "<optional>"
+    }
+  }
+}

Zend/tests/closures/closure_const_expr/static_variable.phpt

@@ -0,0 +1,77 @@
+--TEST--
+Closures in const expressions support static variables.
+--FILE--
+<?php
+
+const Closure = static function () {
+    static $x = [];
+    static $i = 1;
+    $i *= 2;
+    $x[] = $i;
+    var_dump($x);
+};
+
+var_dump(Closure);
+(Closure)();
+(Closure)();
+(Closure)();
+var_dump(Closure);
+
+?>
+--EXPECTF--
+object(Closure)#%d (4) {
+  ["name"]=>
+  string(%d) "{closure:%s:%d}"
+  ["file"]=>
+  string(%d) "%s"
+  ["line"]=>
+  int(3)
+  ["static"]=>
+  array(2) {
+    ["x"]=>
+    array(0) {
+    }
+    ["i"]=>
+    int(1)
+  }
+}
+array(1) {
+  [0]=>
+  int(2)
+}
+array(2) {
+  [0]=>
+  int(2)
+  [1]=>
+  int(4)
+}
+array(3) {
+  [0]=>
+  int(2)
+  [1]=>
+  int(4)
+  [2]=>
+  int(8)
+}
+object(Closure)#%d (4) {
+  ["name"]=>
+  string(%d) "{closure:%s:%d}"
+  ["file"]=>
+  string(%d) "%s"
+  ["line"]=>
+  int(3)
+  ["static"]=>
+  array(2) {
+    ["x"]=>
+    array(3) {
+      [0]=>
+      int(2)
+      [1]=>
+      int(4)
+      [2]=>
+      int(8)
+    }
+    ["i"]=>
+    int(8)
+  }
+}

Zend/zend_ast.c

@@ -1277,6 +1277,7 @@ static void* ZEND_FASTCALL zend_ast_tree_copy(zend_ast *ast, void *buf)
 		new->attr = old->attr;
 		new->lineno = old->lineno;
 		new->op_array = old->op_array;
+		function_add_ref((zend_function *)new->op_array);
 		buf = (void*)((char*)buf + sizeof(zend_ast_op_array));
 	} else if (ast->kind == ZEND_AST_CALLABLE_CONVERT) {
 		zend_ast_fcc *old = (zend_ast_fcc*)ast;
@@ -1353,7 +1354,7 @@ ZEND_API void ZEND_FASTCALL zend_ast_destroy(zend_ast *ast)
 	} else if (EXPECTED(ast->kind == ZEND_AST_CONSTANT)) {
 		zend_string_release_ex(zend_ast_get_constant_name(ast), 0);
 	} else if (EXPECTED(ast->kind == ZEND_AST_OP_ARRAY)) {
-		/* Nothing to do. */
+		destroy_op_array(zend_ast_get_op_array(ast)->op_array);
 	} else if (EXPECTED(zend_ast_is_decl(ast))) {
 		zend_ast_decl *decl = (zend_ast_decl *) ast;
 

Zend/zend_closures.c

@@ -528,7 +528,6 @@ static void zend_closure_free_storage(zend_object *object) /* {{{ */
 		/* We don't own the static variables of fake closures. */
 		if (!(closure->func.op_array.fn_flags & ZEND_ACC_FAKE_CLOSURE)) {
 			zend_destroy_static_vars(&closure->func.op_array);
-			closure->func.op_array.static_variables = NULL;
 		}
 		destroy_op_array(&closure->func.op_array);
 	} else if (closure->func.type == ZEND_INTERNAL_FUNCTION) {
@@ -760,16 +759,14 @@ static void zend_create_closure_ex(zval *res, zend_function *func, zend_class_en
 		}
 
 		/* For fake closures, we want to reuse the static variables of the original function. */
+		HashTable *ht = ZEND_MAP_PTR_GET(func->op_array.static_variables_ptr);
 		if (!is_fake) {
-			if (closure->func.op_array.static_variables) {
-				closure->func.op_array.static_variables =
-					zend_array_dup(closure->func.op_array.static_variables);
+			if (!ht) {
+				ht = closure->func.op_array.static_variables;
 			}
 			ZEND_MAP_PTR_INIT(closure->func.op_array.static_variables_ptr,
-				closure->func.op_array.static_variables);
+				ht ? zend_array_dup(ht) : NULL);
 		} else if (func->op_array.static_variables) {
-			HashTable *ht = ZEND_MAP_PTR_GET(func->op_array.static_variables_ptr);
-
 			if (!ht) {
 				ht = zend_array_dup(func->op_array.static_variables);
 				ZEND_MAP_PTR_SET(func->op_array.static_variables_ptr, ht);

Zend/zend_compile.c

@@ -8244,9 +8244,6 @@ static zend_string *zend_begin_func_decl(znode *result, zend_op_array *op_array,
 
 	zend_register_seen_symbol(lcname, ZEND_SYMBOL_FUNCTION);
 	switch (level) {
-		case FUNC_DECL_LEVEL_CONSTEXPR:
-			zend_add_dynamic_func_def(op_array);
-			break;
 		case FUNC_DECL_LEVEL_NESTED: {
 			uint32_t func_ref = zend_add_dynamic_func_def(op_array);
 			if (op_array->fn_flags & ZEND_ACC_CLOSURE) {
@@ -8261,6 +8258,7 @@ static zend_string *zend_begin_func_decl(znode *result, zend_op_array *op_array,
 			}
 			break;
 		}
+		case FUNC_DECL_LEVEL_CONSTEXPR:
 		case FUNC_DECL_LEVEL_TOPLEVEL:
 			/* Nothing to do. */
 			break;

Zend/zend_opcode.c

@@ -636,13 +636,6 @@ ZEND_API void destroy_op_array(zend_op_array *op_array)
 	}
 	if (op_array->num_dynamic_func_defs) {
 		for (i = 0; i < op_array->num_dynamic_func_defs; i++) {
-			/* Closures overwrite static_variables in their copy.
-			 * Make sure to destroy them when the prototype function is destroyed. */
-			if (op_array->dynamic_func_defs[i]->static_variables
-					&& (op_array->dynamic_func_defs[i]->fn_flags & ZEND_ACC_CLOSURE)) {
-				zend_array_destroy(op_array->dynamic_func_defs[i]->static_variables);
-				op_array->dynamic_func_defs[i]->static_variables = NULL;
-			}
 			destroy_op_array(op_array->dynamic_func_defs[i]);
 		}
 		efree(op_array->dynamic_func_defs);

ext/opcache/zend_file_cache.c

@@ -242,6 +242,15 @@ static void zend_file_cache_unserialize_zval(zval                    *zv,
                                              zend_persistent_script  *script,
                                              void                    *buf);
 
+static void zend_file_cache_serialize_func(zval                     *zv,
+                                           zend_persistent_script   *script,
+                                           zend_file_cache_metainfo *info,
+                                           void                     *buf);
+
+static void zend_file_cache_unserialize_func(zval                    *zv,
+                                             zend_persistent_script  *script,
+                                             void                    *buf);
+
 static void *zend_file_cache_serialize_interned(zend_string              *str,
                                                 zend_file_cache_metainfo *info)
 {
@@ -364,8 +373,10 @@ static void zend_file_cache_serialize_ast(zend_ast                 *ast,
 			}
 		}
 	} else if (ast->kind == ZEND_AST_OP_ARRAY) {
-		/* The op_array itself will be serialized as part of the dynamic_func_defs. */
-		SERIALIZE_PTR(zend_ast_get_op_array(ast)->op_array);
+		zval z;
+		ZVAL_PTR(&z, zend_ast_get_op_array(ast)->op_array);
+		zend_file_cache_serialize_func(&z, script, info, buf);
+		zend_ast_get_op_array(ast)->op_array = Z_PTR(z);
 	} else if (ast->kind == ZEND_AST_CALLABLE_CONVERT) {
 		zend_ast_fcc *fcc = (zend_ast_fcc*)ast;
 		ZEND_MAP_PTR_INIT(fcc->fptr, NULL);
@@ -1252,8 +1263,10 @@ static void zend_file_cache_unserialize_ast(zend_ast                *ast,
 			}
 		}
 	} else if (ast->kind == ZEND_AST_OP_ARRAY) {
-		/* The op_array itself will be unserialized as part of the dynamic_func_defs. */
-		UNSERIALIZE_PTR(zend_ast_get_op_array(ast)->op_array);
+		zval z;
+		ZVAL_PTR(&z, zend_ast_get_op_array(ast)->op_array);
+		zend_file_cache_unserialize_func(&z, script, buf);
+		zend_ast_get_op_array(ast)->op_array = Z_PTR(z);
 	} else if (ast->kind == ZEND_AST_CALLABLE_CONVERT) {
 		zend_ast_fcc *fcc = (zend_ast_fcc*)ast;
 		ZEND_MAP_PTR_NEW(fcc->fptr);