Fix multiple trait fixup

If a trait method is inherited, preloading trait fixup might be
performed on it multiple times. Usually this is fine, because
the opcodes pointer will have already been updated, and will thus
not be found in the xlat table.

However, it can happen that the new opcodes pointer is the same
as one of the old opcodes pointers, if the pointer has been reused
by the allocator. In this case we will look up the wrong op array
and overwrite the trait method with an unrelated trait method.

We fix this by indexing the xlat table not by the opcodes pointer,
but by the refcount pointer. The refcount pointer is not changed
during optimization, and accurately represents which op arrays
should use the same opcodes.

Fixes bug #80307. The test case does not reproduce the bug, because
this depends on a lot of "luck" with the allocator. The test case
merely illustrates a case where orig_op_array would have been NULL
in the original code.

diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c
index 3a1c538c80fde8e3a9f21714dbee7295d6602bfb..a3c079d0f0c46616bf82a1252055e364037cd016 100644 (file)
--- a/ext/opcache/ZendAccelerator.c
+++ b/ext/opcache/ZendAccelerator.c
@@ -4134,7 +4134,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();
 }
@@ -4145,20 +4146,20 @@ 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_string *function_name = op_array->function_name;
-                               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->function_name = function_name;
-                               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_string *function_name = op_array->function_name;
+                       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->function_name = function_name;
+                       op_array->scope = scope;
+                       op_array->fn_flags = fn_flags;
+                       op_array->prototype = prototype;
+                       op_array->static_variables = ht;
                }
        } ZEND_HASH_FOREACH_END();
 }

diff --git a/ext/opcache/tests/preload_trait_multiple_fixup.inc b/ext/opcache/tests/preload_trait_multiple_fixup.inc
new file mode 100644 (file)
index 0000000..5ccb123
--- /dev/null
+++ b/ext/opcache/tests/preload_trait_multiple_fixup.inc
@@ -0,0 +1,19 @@
+<?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;
+}

diff --git a/ext/opcache/tests/preload_trait_multiple_fixup.phpt b/ext/opcache/tests/preload_trait_multiple_fixup.phpt
new file mode 100644 (file)
index 0000000..a63458e
--- /dev/null
+++ b/ext/opcache/tests/preload_trait_multiple_fixup.phpt
@@ -0,0 +1,18 @@
+--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();
+?>
+--EXPECT--
+Foo