Fix static variable behavior with inheritance

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,20 +2486,29 @@ 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);
 			*run_time_cache_ptr = NULL;
 			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)));
 

azure-pipelines.yml

@@ -38,6 +38,23 @@ jobs:
     parameters:
       configurationName: MACOS_DEBUG_NTS
       configurationParameters: '--enable-debug --disable-zts'
+  - template: azure/file_cache_job.yml
+    parameters:
+      configurationName: DEBUG_NTS_FILE_CACHE
+      configurationParameters: '--enable-debug --disable-zts'
+      timeoutInMinutes: 90
+  - template: azure/job.yml
+    parameters:
+      configurationName: DEBUG_NTS_REPEAT
+      configurationParameters: '--enable-debug --disable-zts'
+      runTestsParameters: '--repeat 2'
+  - template: azure/community_job.yml
+    parameters:
+      configurationName: COMMUNITY
+      configurationParameters: >-
+          --enable-debug --enable-zts
+          CFLAGS='-fsanitize=undefined,address -fno-sanitize-recover -DZEND_TRACK_ARENA_ALLOC'
+          LDFLAGS='-fsanitize=undefined,address'
   - ${{ if eq(variables['Build.Reason'], 'Schedule') }}:
     - template: azure/job.yml
       parameters:
@@ -86,28 +103,11 @@ jobs:
         configurationParameters: '--enable-debug --enable-zts'
         runTestsParameters: --msan
         timeoutInMinutes: 120
-    - template: azure/community_job.yml
-      parameters:
-        configurationName: COMMUNITY
-        configurationParameters: >-
-            --enable-debug --enable-zts
-            CFLAGS='-fsanitize=undefined,address -fno-sanitize-recover -DZEND_TRACK_ARENA_ALLOC'
-            LDFLAGS='-fsanitize=undefined,address'
     - template: azure/coverage_job.yml
       parameters:
         configurationName: COVERAGE_DEBUG_ZTS
         configurationParameters: '--enable-debug --disable-zts'
         timeoutInMinutes: 90
-    - template: azure/file_cache_job.yml
-      parameters:
-        configurationName: DEBUG_NTS_FILE_CACHE
-        configurationParameters: '--enable-debug --disable-zts'
-        timeoutInMinutes: 90
-    - template: azure/job.yml
-      parameters:
-        configurationName: DEBUG_NTS_REPEAT
-        configurationParameters: '--enable-debug --disable-zts'
-        runTestsParameters: '--repeat 2'
     - template: azure/libmysqlclient_job.yml
       parameters:
         configurationName: LIBMYSQLCLIENT_DEBUG_NTS