Fix GH-18714: opcache_compile_file() breaks class hoisting

Zend/zend_compile.c

@@ -9654,35 +9654,44 @@ static void zend_compile_class_decl(znode *result, const zend_ast *ast, bool top
 	 /* See zend_link_hooked_object_iter(). */
 	 && !ce->num_hooked_props
 #endif
-	 && !(CG(compiler_options) & ZEND_COMPILE_WITHOUT_EXECUTION)) {
-		if (toplevel) {
-			if (extends_ast) {
-				zend_class_entry *parent_ce = zend_lookup_class_ex(
-					ce->parent_name, NULL, ZEND_FETCH_CLASS_NO_AUTOLOAD);
-
-				if (parent_ce
-				 && !zend_compile_ignore_class(parent_ce, ce->info.user.filename)) {
-					if (zend_try_early_bind(ce, parent_ce, lcname, NULL)) {
-						zend_string_release(lcname);
-						return;
+	) {
+		if (!(CG(compiler_options) & ZEND_COMPILE_WITHOUT_EXECUTION)) {
+			if (toplevel) {
+				if (extends_ast) {
+					zend_class_entry *parent_ce = zend_lookup_class_ex(
+						ce->parent_name, NULL, ZEND_FETCH_CLASS_NO_AUTOLOAD);
+
+					if (parent_ce
+					 && !zend_compile_ignore_class(parent_ce, ce->info.user.filename)) {
+						if (zend_try_early_bind(ce, parent_ce, lcname, NULL)) {
+							zend_string_release(lcname);
+							return;
+						}
 					}
+				} else if (EXPECTED(zend_hash_add_ptr(CG(class_table), lcname, ce) != NULL)) {
+					zend_string_release(lcname);
+					zend_build_properties_info_table(ce);
+					zend_inheritance_check_override(ce);
+					ce->ce_flags |= ZEND_ACC_LINKED;
+					zend_observer_class_linked_notify(ce, lcname);
+					return;
+				} else {
+					goto link_unbound;
 				}
-			} else if (EXPECTED(zend_hash_add_ptr(CG(class_table), lcname, ce) != NULL)) {
-				zend_string_release(lcname);
+			} else if (!extends_ast) {
+link_unbound:
+				/* Link unbound simple class */
 				zend_build_properties_info_table(ce);
 				zend_inheritance_check_override(ce);
 				ce->ce_flags |= ZEND_ACC_LINKED;
-				zend_observer_class_linked_notify(ce, lcname);
-				return;
-			} else {
-				goto link_unbound;
 			}
-		} else if (!extends_ast) {
-link_unbound:
-			/* Link unbound simple class */
-			zend_build_properties_info_table(ce);
-			zend_inheritance_check_override(ce);
-			ce->ce_flags |= ZEND_ACC_LINKED;
+		} else if (!extends_ast
+				&& !(CG(compiler_options) & ZEND_COMPILE_PRELOAD)) {
+			/* When compiling without execution (opcache_compile_file),
+			 * link simple classes without parents so opcache can
+			 * early-bind them when loaded from cache. Skip during
+			 * preloading which has its own linking pipeline. */
+			goto link_unbound;
 		}
 	}
 

ext/opcache/tests/gh18714.phpt

@@ -0,0 +1,28 @@
+--TEST--
+GH-18714 (opcache_compile_file() breaks class hoisting)
+--EXTENSIONS--
+opcache
+--INI--
+opcache.enable=1
+opcache.enable_cli=1
+--FILE--
+<?php
+
+// Class used before declaration relies on hoisting
+file_put_contents(__DIR__ . '/gh18714_test.php', <<<'PHP'
+<?php
+$x = new HelloWorld();
+echo get_class($x) . "\n";
+class HelloWorld {}
+PHP);
+
+opcache_compile_file(__DIR__ . '/gh18714_test.php');
+require_once __DIR__ . '/gh18714_test.php';
+
+?>
+--CLEAN--
+<?php
+@unlink(__DIR__ . '/gh18714_test.php');
+?>
+--EXPECT--
+HelloWorld

ext/opcache/zend_accelerator_module.c

@@ -961,6 +961,23 @@ ZEND_FUNCTION(opcache_jit_blacklist)
 #endif
 }
 
+/* Remove hash table entries appended after orig_count without calling the
+ * destructor, since these point to SHM-backed data we don't own. */
+static void accel_rollback_hash(HashTable *ht, uint32_t orig_count)
+{
+	dtor_func_t orig_dtor = ht->pDestructor;
+	ht->pDestructor = NULL;
+	while (ht->nNumUsed > orig_count) {
+		Bucket *p = &ht->arData[ht->nNumUsed - 1];
+		if (EXPECTED(Z_TYPE(p->val) != IS_UNDEF)) {
+			zend_hash_del_bucket(ht, p);
+		} else {
+			ht->nNumUsed--;
+		}
+	}
+	ht->pDestructor = orig_dtor;
+}
+
 ZEND_FUNCTION(opcache_compile_file)
 {
 	zend_string *script_name;
@@ -984,6 +1001,11 @@ ZEND_FUNCTION(opcache_compile_file)
 	orig_compiler_options = CG(compiler_options);
 	CG(compiler_options) |= ZEND_COMPILE_WITHOUT_EXECUTION;
 
+	/* Save class/function table state so we can undo the side effects
+	 * of zend_accel_load_script() called by persistent_compile_file(). */
+	uint32_t orig_class_count = EG(class_table)->nNumUsed;
+	uint32_t orig_function_count = EG(function_table)->nNumUsed;
+
 	if (CG(compiler_options) & ZEND_COMPILE_PRELOAD) {
 		/* During preloading, a failure in opcache_compile_file() should result in an overall
 		 * preloading failure. Otherwise we may include partially compiled files in the preload
@@ -1001,6 +1023,14 @@ ZEND_FUNCTION(opcache_compile_file)
 	CG(compiler_options) = orig_compiler_options;
 
 	if(op_array != NULL) {
+		/* Undo classes/functions registered by zend_accel_load_script().
+		 * opcache_compile_file() should only cache without side effects.
+		 * Skip during preloading: preload needs the registrations to persist. */
+		if (!(orig_compiler_options & ZEND_COMPILE_PRELOAD)) {
+			accel_rollback_hash(EG(class_table), orig_class_count);
+			accel_rollback_hash(EG(function_table), orig_function_count);
+		}
+
 		destroy_op_array(op_array);
 		efree(op_array);
 		RETVAL_TRUE;