Backport preloading trait fixup fixes

This cherry-picks 33969c2 and 2effbfd from PHP-8.0. The issues these commits fix could also manifest in PHP 7.4, and a commenter on bug #80307 reports this this might indeed be happening.
php · Nov 5, 2020 · 6808968 · 6808968
1 parent 00e41a1
commit 6808968
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 19 deletions.
diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c
@@ -4052,7 +4052,8 @@ static void preload_register_trait_methods(zend_class_entry *ce) {
 	zend_op_array *op_array;
 	ZEND_HASH_FOREACH_PTR(&ce->function_table, op_array) {
 		if (!(op_array->fn_flags & ZEND_ACC_TRAIT_CLONE)) {
-			zend_shared_alloc_register_xlat_entry(op_array->opcodes, op_array);
+			ZEND_ASSERT(op_array->refcount && "Must have refcount pointer");
+			zend_shared_alloc_register_xlat_entry(op_array->refcount, op_array);
 		}
 	} ZEND_HASH_FOREACH_END();
 }
@@ -4063,18 +4064,18 @@ static void preload_fix_trait_methods(zend_class_entry *ce)
 
 	ZEND_HASH_FOREACH_PTR(&ce->function_table, op_array) {
 		if (op_array->fn_flags & ZEND_ACC_TRAIT_CLONE) {
-			zend_op_array *orig_op_array = zend_shared_alloc_get_xlat_entry(op_array->opcodes);
-			if (orig_op_array) {
-				zend_class_entry *scope = op_array->scope;
-				uint32_t fn_flags = op_array->fn_flags;
-				zend_function *prototype = op_array->prototype;
-				HashTable *ht = op_array->static_variables;
-				*op_array = *orig_op_array;
-				op_array->scope = scope;
-				op_array->fn_flags = fn_flags;
-				op_array->prototype = prototype;
-				op_array->static_variables = ht;
-			}
+			zend_op_array *orig_op_array = zend_shared_alloc_get_xlat_entry(op_array->refcount);
+			ZEND_ASSERT(orig_op_array && "Must be in xlat table");
+
+			zend_class_entry *scope = op_array->scope;
+			uint32_t fn_flags = op_array->fn_flags;
+			zend_function *prototype = op_array->prototype;
+			HashTable *ht = op_array->static_variables;
+			*op_array = *orig_op_array;
+			op_array->scope = scope;
+			op_array->fn_flags = fn_flags;
+			op_array->prototype = prototype;
+			op_array->static_variables = ht;
 		}
 	} ZEND_HASH_FOREACH_END();
 }
@@ -4105,16 +4106,12 @@ static int preload_optimize(zend_persistent_script *script)
 	}
 
 	ZEND_HASH_FOREACH_PTR(&script->script.class_table, ce) {
-		if (ce->ce_flags & ZEND_ACC_IMPLEMENT_TRAITS) {
-			preload_fix_trait_methods(ce);
-		}
+		preload_fix_trait_methods(ce);
 	} ZEND_HASH_FOREACH_END();
 
 	ZEND_HASH_FOREACH_PTR(preload_scripts, script) {
 		ZEND_HASH_FOREACH_PTR(&script->script.class_table, ce) {
-			if (ce->ce_flags & ZEND_ACC_IMPLEMENT_TRAITS) {
-				preload_fix_trait_methods(ce);
-			}
+			preload_fix_trait_methods(ce);
 		} ZEND_HASH_FOREACH_END();
 	} ZEND_HASH_FOREACH_END();
 

diff --git a/ext/opcache/tests/preload_trait_multiple_fixup.inc b/ext/opcache/tests/preload_trait_multiple_fixup.inc
@@ -0,0 +1,33 @@
+<?php
+
+trait T1 {
+    public function method() {
+        // Needs to be optimized somehow.
+        $str = "Foo";
+        echo "$str\n";
+    }
+}
+
+trait T2 {}
+
+class C1 {
+    use T1;
+}
+
+class C2 extends C1 {
+    use T2;
+}
+
+trait T3 {
+    public function method() {
+        // Prevent trivial inheritance.
+        static $x;
+        // Needs to be optimized somehow.
+        $str = "Foo";
+        echo "$str\n";
+    }
+}
+class C3 {
+    use T3;
+}
+class C4 extends C3 {}
diff --git a/ext/opcache/tests/preload_trait_multiple_fixup.phpt b/ext/opcache/tests/preload_trait_multiple_fixup.phpt
@@ -0,0 +1,20 @@
+--TEST--
+Op array fixed up multiple times during preloading
+--INI--
+opcache.enable=1
+opcache.enable_cli=1
+opcache.optimization_level=-1
+opcache.preload={PWD}/preload_trait_multiple_fixup.inc
+--SKIPIF--
+<?php
+require_once('skipif.inc');
+if (PHP_OS_FAMILY == 'Windows') die('skip Preloading is not supported on Windows');
+?>
+--FILE--
+<?php
+(new C2)->method();
+(new C4)->method();
+?>
+--EXPECT--
+Foo
+Foo