Fix static variable behavior with inheritance

When a method is inherited, the static variables will now always
use the initial values, rather than the values at the time of
inheritance. As such, behavior no longer depends on whether
inheritance happens before or after a method has been called.

This is implemented by always keeping static_variables as the
original values, and static_variables_ptr as the modified copy.

Closes GH-6705.

UPGRADING

@@ -37,6 +37,28 @@ PHP 8.1 UPGRADE NOTES
         //             is deprecated
 
     RFC: https://wiki.php.net/rfc/deprecate_null_to_scalar_internal_arg
+  . When a method using static variables is inherited, the inherited method
+    will now initialize the static variables to their original values, rather
+    than the values at the time of inheritance:
+
+        class A {
+            public function counter() {
+                static $counter = 0;
+                $counter++;
+                return $counter;
+            }
+        }
+
+        var_dump((new A)->counter()); // int(1)
+
+        eval('class B extends A {}'); // eval() to prevent early binding.
+
+        var_dump((new B)->counter()); // int(1), previously int(2)
+        var_dump((new A)->counter()); // int(2)
+        var_dump((new B)->counter()); // int(2), previously int(3)
+
+    Previously the behavior would be different depending on whether A::counter()
+    was called before class B was declared, or after it was declared.
 
 - Fileinfo:
   . The fileinfo functions now accept and return, respectively, finfo objects

Zend/tests/anon/015.phpt

@@ -19,10 +19,7 @@ var_dump($d->foo(24));
 var_dump($c->foo());
 ?>
 --EXPECT--
-array(1) {
-  [0]=>
-  int(42)
-}
+NULL
 array(1) {
   [0]=>
   int(24)

Zend/tests/anon/016.phpt

@@ -19,11 +19,7 @@ var_dump($d->foo(24));
 var_dump($c->foo());
 ?>
 --EXPECT--
-array(1) {
-  [0]=>
-  object(stdClass)#2 (0) {
-  }
-}
+NULL
 array(1) {
   [0]=>
   int(24)

Zend/tests/closure_bindTo_preserves_used_variables.phpt

@@ -0,0 +1,17 @@
+--TEST--
+Closure::bindTo() should preserve used variables
+--FILE--
+<?php
+
+$var = 0;
+$fn = function() use($var) {
+    var_dump($var);
+};
+$fn();
+$fn = $fn->bindTo(null, null);
+$fn();
+
+?>
+--EXPECT--
+int(0)
+int(0)

Zend/tests/method_static_var.phpt

@@ -3,8 +3,6 @@ Initial value of static var in method depends on the include time of the class d
 --FILE--
 <?php
 
-/* The current behavior is probably a bug, but we should still test how it currently works. */
-
 class Foo {
     public static function test() {
         static $i = 0;
@@ -22,5 +20,5 @@ Bar::test();
 --EXPECT--
 int(1)
 int(2)
+int(1)
 int(2)
-int(3)

Zend/zend_compile.c

@@ -1029,28 +1029,32 @@ static uint32_t zend_add_try_element(uint32_t try_op) /* {{{ */
 }
 /* }}} */
 
+void zend_init_static_variables_map_ptr(zend_op_array *op_array)
+{
+	if (op_array->static_variables) {
+		ZEND_MAP_PTR_INIT(op_array->static_variables_ptr,
+			zend_arena_alloc(&CG(arena), sizeof(HashTable *)));
+		ZEND_MAP_PTR_SET(op_array->static_variables_ptr, NULL);
+	}
+}
+
 ZEND_API void function_add_ref(zend_function *function) /* {{{ */
 {
 	if (function->type == ZEND_USER_FUNCTION) {
 		zend_op_array *op_array = &function->op_array;
-
 		if (op_array->refcount) {
 			(*op_array->refcount)++;
 		}
-		if (op_array->static_variables
-			&& !(GC_FLAGS(op_array->static_variables) & IS_ARRAY_IMMUTABLE)) {
-			GC_ADDREF(op_array->static_variables);
-		}
 
 		if (CG(compiler_options) & ZEND_COMPILE_PRELOAD) {
 			ZEND_ASSERT(op_array->fn_flags & ZEND_ACC_PRELOADED);
 			ZEND_MAP_PTR_NEW(op_array->run_time_cache);
-			ZEND_MAP_PTR_NEW(op_array->static_variables_ptr);
 		} else {
-			ZEND_MAP_PTR_INIT(op_array->static_variables_ptr, &op_array->static_variables);
-			ZEND_MAP_PTR_INIT(op_array->run_time_cache, zend_arena_alloc(&CG(arena), sizeof(void*)));
+			ZEND_MAP_PTR_INIT(op_array->run_time_cache, zend_arena_alloc(&CG(arena), sizeof(void *)));
 			ZEND_MAP_PTR_SET(op_array->run_time_cache, NULL);
 		}
+
+		zend_init_static_variables_map_ptr(op_array);
 	}
 
 	if (function->common.function_name) {
@@ -7021,9 +7025,8 @@ void zend_compile_func_decl(znode *result, zend_ast *ast, bool toplevel) /* {{{
 	if (CG(compiler_options) & ZEND_COMPILE_PRELOAD) {
 		op_array->fn_flags |= ZEND_ACC_PRELOADED;
 		ZEND_MAP_PTR_NEW(op_array->run_time_cache);
-		ZEND_MAP_PTR_NEW(op_array->static_variables_ptr);
 	} else {
-		ZEND_MAP_PTR_INIT(op_array->run_time_cache, zend_arena_alloc(&CG(arena), sizeof(void*)));
+		ZEND_MAP_PTR_INIT(op_array->run_time_cache, zend_arena_alloc(&CG(arena), sizeof(void *)));
 		ZEND_MAP_PTR_SET(op_array->run_time_cache, NULL);
 	}
 
@@ -7112,6 +7115,7 @@ void zend_compile_func_decl(znode *result, zend_ast *ast, bool toplevel) /* {{{
 	zend_do_extended_stmt();
 	zend_emit_final_return(0);
 
+	zend_init_static_variables_map_ptr(op_array);
 	pass_two(CG(active_op_array));
 	zend_oparray_context_end(&orig_oparray_context);
 

Zend/zend_compile.h

@@ -795,6 +795,7 @@ void zend_verify_namespace(void);
 void zend_resolve_goto_label(zend_op_array *op_array, zend_op *opline);
 
 ZEND_API void function_add_ref(zend_function *function);
+void zend_init_static_variables_map_ptr(zend_op_array *op_array);
 
 #define INITIAL_OP_ARRAY_SIZE 64
 

Zend/zend_execute_API.c

@@ -285,10 +285,10 @@ void shutdown_executor(void) /* {{{ */
 			if (op_array->type == ZEND_INTERNAL_FUNCTION) {
 				break;
 			}
-			if (op_array->static_variables) {
+			if (ZEND_MAP_PTR(op_array->static_variables_ptr)) {
 				HashTable *ht = ZEND_MAP_PTR_GET(op_array->static_variables_ptr);
 				if (ht) {
-					zend_array_release(ht);
+					zend_array_destroy(ht);
 					ZEND_MAP_PTR_SET(op_array->static_variables_ptr, NULL);
 				}
 			}
@@ -308,10 +308,10 @@ void shutdown_executor(void) /* {{{ */
 				zend_op_array *op_array;
 				ZEND_HASH_FOREACH_PTR(&ce->function_table, op_array) {
 					if (op_array->type == ZEND_USER_FUNCTION) {
-						if (op_array->static_variables) {
+						if (ZEND_MAP_PTR(op_array->static_variables_ptr)) {
 							HashTable *ht = ZEND_MAP_PTR_GET(op_array->static_variables_ptr);
 							if (ht) {
-								zend_array_release(ht);
+								zend_array_destroy(ht);
 								ZEND_MAP_PTR_SET(op_array->static_variables_ptr, NULL);
 							}
 						}

Zend/zend_inheritance.c

@@ -89,28 +89,10 @@ static zend_function *zend_duplicate_internal_function(zend_function *func, zend
 
 static zend_function *zend_duplicate_user_function(zend_function *func) /* {{{ */
 {
-	zend_function *new_function;
-
-	new_function = zend_arena_alloc(&CG(arena), sizeof(zend_op_array));
-	memcpy(new_function, func, sizeof(zend_op_array));
-
-	if (CG(compiler_options) & ZEND_COMPILE_PRELOAD) {
-		ZEND_ASSERT(new_function->op_array.fn_flags & ZEND_ACC_PRELOADED);
-		ZEND_MAP_PTR_NEW(new_function->op_array.static_variables_ptr);
-	} else {
-		ZEND_MAP_PTR_INIT(new_function->op_array.static_variables_ptr, &new_function->op_array.static_variables);
-	}
-
-	HashTable *static_properties_ptr = ZEND_MAP_PTR_GET(func->op_array.static_variables_ptr);
-	if (static_properties_ptr) {
-		/* See: Zend/tests/method_static_var.phpt */
-		ZEND_MAP_PTR_SET(new_function->op_array.static_variables_ptr, static_properties_ptr);
-		GC_TRY_ADDREF(static_properties_ptr);
-	} else {
-		GC_TRY_ADDREF(new_function->op_array.static_variables);
-	}
-
-	return new_function;
+	zend_op_array *new_op_array = zend_arena_alloc(&CG(arena), sizeof(zend_op_array));
+	memcpy(new_op_array, func, sizeof(zend_op_array));
+	zend_init_static_variables_map_ptr(new_op_array);
+	return (zend_function *) new_op_array;
 }
 /* }}} */
 
@@ -2504,12 +2486,17 @@ static zend_class_entry *zend_lazy_class_load(zend_class_entry *pce)
 		for (; p != end; p++) {
 			zend_op_array *op_array, *new_op_array;
 			void ***run_time_cache_ptr;
+			size_t alloc_size;
 
 			op_array = Z_PTR(p->val);
 			ZEND_ASSERT(op_array->type == ZEND_USER_FUNCTION);
 			ZEND_ASSERT(op_array->scope == pce);
 			ZEND_ASSERT(op_array->prototype == NULL);
-			new_op_array = zend_arena_alloc(&CG(arena), sizeof(zend_op_array) + sizeof(void*));
+			alloc_size = sizeof(zend_op_array) + sizeof(void *);
+			if (op_array->static_variables) {
+				alloc_size += sizeof(HashTable *);
+			}
+			new_op_array = zend_arena_alloc(&CG(arena), alloc_size);
 			Z_PTR(p->val) = new_op_array;
 			memcpy(new_op_array, op_array, sizeof(zend_op_array));
 			run_time_cache_ptr = (void***)(new_op_array + 1);
@@ -2517,7 +2504,11 @@ static zend_class_entry *zend_lazy_class_load(zend_class_entry *pce)
 			new_op_array->fn_flags &= ~ZEND_ACC_IMMUTABLE;
 			new_op_array->scope = ce;
 			ZEND_MAP_PTR_INIT(new_op_array->run_time_cache, run_time_cache_ptr);
-			ZEND_MAP_PTR_INIT(new_op_array->static_variables_ptr, &new_op_array->static_variables);
+			if (op_array->static_variables) {
+				HashTable **static_variables_ptr = (HashTable **) (run_time_cache_ptr + 1);
+				*static_variables_ptr = NULL;
+				ZEND_MAP_PTR_INIT(new_op_array->static_variables_ptr, static_variables_ptr);
+			}
 
 			zend_update_inherited_handler(constructor);
 			zend_update_inherited_handler(destructor);

Zend/zend_language_scanner.l

@@ -636,6 +636,7 @@ static zend_op_array *zend_compile(int type)
 		zend_emit_final_return(type == ZEND_USER_FUNCTION);
 		op_array->line_start = 1;
 		op_array->line_end = last_lineno;
+		zend_init_static_variables_map_ptr(op_array);
 		pass_two(op_array);
 		zend_oparray_context_end(&original_oparray_context);
 		zend_file_context_end(&original_file_context);

Zend/zend_opcode.c

@@ -77,7 +77,7 @@ void init_op_array(zend_op_array *op_array, zend_uchar type, int initial_ops_siz
 	op_array->last_live_range = 0;
 
 	op_array->static_variables = NULL;
-	ZEND_MAP_PTR_INIT(op_array->static_variables_ptr, &op_array->static_variables);
+	ZEND_MAP_PTR_INIT(op_array->static_variables_ptr, NULL);
 	op_array->last_try_catch = 0;
 
 	op_array->fn_flags = 0;
@@ -515,12 +515,10 @@ ZEND_API void destroy_op_array(zend_op_array *op_array)
 {
 	uint32_t i;
 
-	if (op_array->static_variables) {
+	if (ZEND_MAP_PTR(op_array->static_variables_ptr)) {
 		HashTable *ht = ZEND_MAP_PTR_GET(op_array->static_variables_ptr);
-		if (ht && !(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE)) {
-			if (GC_DELREF(ht) == 0) {
-				zend_array_destroy(ht);
-			}
+		if (ht) {
+			zend_array_destroy(ht);
 		}
 	}
 
@@ -599,6 +597,9 @@ ZEND_API void destroy_op_array(zend_op_array *op_array)
 		}
 		efree(arg_info);
 	}
+	if (op_array->static_variables) {
+		zend_array_destroy(op_array->static_variables);
+	}
 }
 
 static void zend_update_extended_stmts(zend_op_array *op_array)

Zend/zend_vm_def.h

@@ -8659,16 +8659,10 @@ ZEND_VM_HANDLER(183, ZEND_BIND_STATIC, CV, UNUSED, REF)
 
 	ht = ZEND_MAP_PTR_GET(EX(func)->op_array.static_variables_ptr);
 	if (!ht) {
-		ZEND_ASSERT(EX(func)->op_array.fn_flags & (ZEND_ACC_IMMUTABLE|ZEND_ACC_PRELOADED));
 		ht = zend_array_dup(EX(func)->op_array.static_variables);
 		ZEND_MAP_PTR_SET(EX(func)->op_array.static_variables_ptr, ht);
-	} else if (GC_REFCOUNT(ht) > 1) {
-		if (!(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE)) {
-			GC_DELREF(ht);
-		}
-		ht = zend_array_dup(ht);
-		ZEND_MAP_PTR_SET(EX(func)->op_array.static_variables_ptr, ht);
 	}
+	ZEND_ASSERT(GC_REFCOUNT(ht) == 1);
 
 	value = (zval*)((char*)ht->arData + (opline->extended_value & ~(ZEND_BIND_REF|ZEND_BIND_IMPLICIT)));
 

Zend/zend_vm_execute.h

@@ -47468,16 +47468,10 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_BIND_STATIC_SPEC_CV_UNUSED_HAN
 
 	ht = ZEND_MAP_PTR_GET(EX(func)->op_array.static_variables_ptr);
 	if (!ht) {
-		ZEND_ASSERT(EX(func)->op_array.fn_flags & (ZEND_ACC_IMMUTABLE|ZEND_ACC_PRELOADED));
 		ht = zend_array_dup(EX(func)->op_array.static_variables);
 		ZEND_MAP_PTR_SET(EX(func)->op_array.static_variables_ptr, ht);
-	} else if (GC_REFCOUNT(ht) > 1) {
-		if (!(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE)) {
-			GC_DELREF(ht);
-		}
-		ht = zend_array_dup(ht);
-		ZEND_MAP_PTR_SET(EX(func)->op_array.static_variables_ptr, ht);
 	}
+	ZEND_ASSERT(GC_REFCOUNT(ht) == 1);
 
 	value = (zval*)((char*)ht->arData + (opline->extended_value & ~(ZEND_BIND_REF|ZEND_BIND_IMPLICIT)));
 

ext/opcache/ZendAccelerator.c

@@ -2323,27 +2323,6 @@ static zend_class_entry* zend_accel_inheritance_cache_get(zend_class_entry *ce,
 	return NULL;
 }
 
-static bool is_array_cacheable(zval *zv)
-{
-	zval *p;
-
-	ZEND_HASH_FOREACH_VAL(Z_ARR_P(zv), p) {
-		if (Z_REFCOUNTED_P(p)) {
-			if (Z_TYPE_P(p) == IS_ARRAY) {
-				if (!is_array_cacheable(p)) {
-					/* Can't cache */
-					return 0;
-				}
-			} else if (Z_TYPE_P(p) == IS_OBJECT || Z_TYPE_P(p) == IS_RESOURCE || Z_TYPE_P(p) == IS_REFERENCE) {
-				/* Can't cache */
-				return 0;
-			}
-		}
-	} ZEND_HASH_FOREACH_END();
-
-	return 1;
-}
-
 static zend_class_entry* zend_accel_inheritance_cache_add(zend_class_entry *ce, zend_class_entry *proto, zend_class_entry *parent, zend_class_entry **traits_and_interfaces, HashTable *dependencies)
 {
 	zend_persistent_script dummy;
@@ -2369,44 +2348,6 @@ static zend_class_entry* zend_accel_inheritance_cache_add(zend_class_entry *ce,
 		}
 	}
 
-	if (ce->ce_flags & ZEND_HAS_STATIC_IN_METHODS) {
-		zend_op_array *op_array;
-		zval *zv;
-
-		ZEND_HASH_FOREACH_PTR(&ce->function_table, op_array) {
-			if (op_array->type == ZEND_USER_FUNCTION
-			 && op_array->static_variables
-			 && !(GC_FLAGS(op_array->static_variables) & IS_ARRAY_IMMUTABLE)) {
-				if (UNEXPECTED(GC_REFCOUNT(op_array->static_variables) > 1)) {
-					GC_DELREF(op_array->static_variables);
-					op_array->static_variables = zend_array_dup(op_array->static_variables);
-				}
-				ZEND_HASH_FOREACH_VAL(op_array->static_variables, zv) {
-					if (Z_ISREF_P(zv)) {
-						zend_reference *ref = Z_REF_P(zv);
-
-						ZVAL_COPY_VALUE(zv, &ref->val);
-						if (GC_DELREF(ref) == 0) {
-							efree_size(ref, sizeof(zend_reference));
-						}
-					}
-					if (Z_REFCOUNTED_P(zv)) {
-						if (Z_TYPE_P(zv) == IS_ARRAY) {
-							if (!is_array_cacheable(zv)) {
-								/* Can't cache */
-								return NULL;
-							}
-							SEPARATE_ARRAY(zv);
-						} else if (Z_TYPE_P(zv) == IS_OBJECT || Z_TYPE_P(zv) == IS_RESOURCE) {
-							/* Can't cache */
-							return NULL;
-						}
-					}
-				} ZEND_HASH_FOREACH_END();
-			}
-		} ZEND_HASH_FOREACH_END();
-	}
-
 	SHM_UNPROTECT();
 	zend_shared_alloc_lock();
 
@@ -3692,11 +3633,6 @@ static zend_op_array *preload_compile_file(zend_file_handle *file_handle, int ty
 //???		efree(op_array->refcount);
 		op_array->refcount = NULL;
 
-		if (op_array->static_variables &&
-		    !(GC_FLAGS(op_array->static_variables) & IS_ARRAY_IMMUTABLE)) {
-			GC_ADDREF(op_array->static_variables);
-		}
-
 		zend_hash_add_ptr(preload_scripts, script->script.filename, script);
 	}
 

ext/opcache/tests/preload_method_static_vars.phpt

@@ -18,8 +18,8 @@ Bar::test();
 --EXPECT--
 int(1)
 int(2)
+int(1)
 int(2)
-int(3)
 
 int(1)
 int(1)

ext/opcache/zend_accelerator_util_funcs.c

@@ -219,7 +219,6 @@ zend_op_array* zend_accel_load_script(zend_persistent_script *persistent_script,
 
 	op_array = (zend_op_array *) emalloc(sizeof(zend_op_array));
 	*op_array = persistent_script->script.main_op_array;
-	ZEND_MAP_PTR_INIT(op_array->static_variables_ptr, &op_array->static_variables);
 
 	if (zend_hash_num_elements(&persistent_script->script.function_table) > 0) {
 		zend_accel_function_hash_copy(CG(function_table), &persistent_script->script.function_table);

ext/opcache/zend_file_cache.c

@@ -1234,12 +1234,16 @@ static void zend_file_cache_unserialize_op_array(zend_op_array           *op_arr
 		if (op_array->static_variables) {
 			ZEND_MAP_PTR_NEW(op_array->static_variables_ptr);
 		} else {
-			ZEND_MAP_PTR_INIT(op_array->static_variables_ptr, &op_array->static_variables);
+			ZEND_MAP_PTR_INIT(op_array->static_variables_ptr, NULL);
 		}
 		ZEND_MAP_PTR_NEW(op_array->run_time_cache);
 	} else {
 		op_array->fn_flags &= ~ZEND_ACC_IMMUTABLE;
-		ZEND_MAP_PTR_INIT(op_array->static_variables_ptr, &op_array->static_variables);
+		if (op_array->static_variables) {
+			ZEND_MAP_PTR_INIT(op_array->static_variables_ptr,
+				zend_arena_alloc(&CG(arena), sizeof(HashTable *)));
+			ZEND_MAP_PTR_SET(op_array->static_variables_ptr, NULL);
+		}
 		if (op_array != &script->script.main_op_array) {
 			ZEND_MAP_PTR_INIT(op_array->run_time_cache, zend_arena_alloc(&CG(arena), sizeof(void*)));
 			ZEND_MAP_PTR_SET(op_array->run_time_cache, NULL);

ext/opcache/zend_persist.c

@@ -686,8 +686,6 @@ static void zend_persist_op_array(zval *zv)
 		if (op_array->static_variables) {
 			ZEND_MAP_PTR_NEW(op_array->static_variables_ptr);
 		}
-	} else {
-		ZEND_MAP_PTR_INIT(op_array->static_variables_ptr, &op_array->static_variables);
 	}
 }
 
@@ -757,8 +755,8 @@ static void zend_persist_class_method(zval *zv, zend_class_entry *ce)
 	} else {
 		if (ce->ce_flags & ZEND_ACC_IMMUTABLE) {
 			ZEND_MAP_PTR_INIT(op_array->run_time_cache, NULL);
+			ZEND_MAP_PTR_INIT(op_array->static_variables_ptr, NULL);
 		}
-		ZEND_MAP_PTR_INIT(op_array->static_variables_ptr, &op_array->static_variables);
 	}
 }
 
@@ -1277,9 +1275,10 @@ zend_persistent_script *zend_accel_script_persist(zend_persistent_script *script
 	zend_persist_op_array_ex(&script->script.main_op_array, script);
 	if (!script->corrupted) {
 		ZEND_MAP_PTR_INIT(script->script.main_op_array.run_time_cache, NULL);
+		if (script->script.main_op_array.static_variables) {
+			ZEND_MAP_PTR_NEW(script->script.main_op_array.static_variables_ptr);
+		}
 	}
-	ZEND_MAP_PTR_INIT(script->script.main_op_array.static_variables_ptr,
-		&script->script.main_op_array.static_variables);
 	zend_persist_warnings(script);
 
 	if (for_shm) {

ext/reflection/php_reflection.c

@@ -1796,7 +1796,6 @@ ZEND_METHOD(ReflectionFunctionAbstract, getStaticVariables)
 		array_init(return_value);
 		ht = ZEND_MAP_PTR_GET(fptr->op_array.static_variables_ptr);
 		if (!ht) {
-			ZEND_ASSERT(fptr->op_array.fn_flags & ZEND_ACC_IMMUTABLE);
 			ht = zend_array_dup(fptr->op_array.static_variables);
 			ZEND_MAP_PTR_SET(fptr->op_array.static_variables_ptr, ht);
 		}