Fix GH-24319: Make OPcache hash entry publication and indirection atomic

Zend/zend_atomic.h

@@ -18,6 +18,7 @@
 #include "zend_portability.h"
 
 #include <stdbool.h>
+#include <stdint.h>
 
 #define ZEND_GCC_PREREQ(x, y) \
 	((__GNUC__ == (x) && __GNUC_MINOR__ >= (y)) || (__GNUC__ > (x)))
@@ -415,6 +416,29 @@ static zend_always_inline uint32_t zend_atomic_load_acquire_32(const uint32_t *p
 
 #define ZEND_ATOMIC_LOAD_ACQUIRE_32(ptr) zend_atomic_load_acquire_32(ptr)
 
+/* Naturally aligned machine-word atomics. Loads must not use a
+ * read-modify-write operation: the storage may be mapped read-only. */
+static zend_always_inline uintptr_t zend_atomic_load_acquire_uintptr(const uintptr_t *ptr)
+{
+#if defined(HAVE_GNUC_ATOMICS) || defined(HAVE_C11_ATOMICS)
+	return __atomic_load_n(ptr, __ATOMIC_ACQUIRE);
+#else
+	uintptr_t value = *(const volatile uintptr_t *)ptr;
+	ZEND_ATOMIC_FENCE_ACQUIRE();
+	return value;
+#endif
+}
+
+static zend_always_inline void zend_atomic_store_release_uintptr(uintptr_t *ptr, uintptr_t value)
+{
+#if defined(HAVE_GNUC_ATOMICS) || defined(HAVE_C11_ATOMICS)
+	__atomic_store_n(ptr, value, __ATOMIC_RELEASE);
+#else
+	ZEND_ATOMIC_FENCE_RELEASE();
+	*(volatile uintptr_t *)ptr = value;
+#endif
+}
+
 END_EXTERN_C()
 
 #endif

ext/opcache/ZendAccelerator.c

@@ -1607,7 +1607,7 @@ static zend_persistent_script *cache_script_in_shared_memory(zend_persistent_scr
 	 * exclusive lock) */
 	bucket = zend_accel_hash_find_entry(&ZCSG(hash), new_persistent_script->script.filename);
 	if (bucket) {
-		zend_persistent_script *existing_persistent_script = (zend_persistent_script *)bucket->data;
+		zend_persistent_script *existing_persistent_script = zend_accel_hash_entry_data(bucket);
 
 		if (!existing_persistent_script->corrupted) {
 			if (key &&
@@ -2098,7 +2098,7 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
 				bucket = zend_accel_hash_find_entry(&ZCSG(hash), file_handle->opened_path);
 
 				if (bucket) {
-					persistent_script = (zend_persistent_script *)bucket->data;
+					persistent_script = zend_accel_hash_entry_data(bucket);
 
 					if (key && !persistent_script->corrupted) {
 						HANDLE_BLOCK_INTERRUPTIONS();
@@ -2586,7 +2586,7 @@ static zend_string* persistent_zend_resolve_path(zend_string *filename)
 				if (key) {
 					zend_accel_hash_entry *bucket = zend_accel_hash_find_entry(&ZCSG(hash), key);
 					if (bucket != NULL) {
-						zend_persistent_script *persistent_script = (zend_persistent_script *)bucket->data;
+						zend_persistent_script *persistent_script = zend_accel_hash_entry_data(bucket);
 						if (!persistent_script->corrupted) {
 							ZCG(cache_opline) = EG(current_execute_data) ? EG(current_execute_data)->opline : NULL;
 							ZCG(cache_persistent_script) = persistent_script;
@@ -2607,7 +2607,7 @@ static zend_string* persistent_zend_resolve_path(zend_string *filename)
 				/* lookup by real path */
 				zend_accel_hash_entry *bucket = zend_accel_hash_find_entry(&ZCSG(hash), resolved_path);
 				if (bucket) {
-					zend_persistent_script *persistent_script = (zend_persistent_script *)bucket->data;
+					zend_persistent_script *persistent_script = zend_accel_hash_entry_data(bucket);
 					if (!persistent_script->corrupted) {
 						if (key) {
 							/* add another "key" for the same bucket */

ext/opcache/jit/zend_jit_trace.c

@@ -7671,10 +7671,10 @@ static void zend_jit_stop_counter_handlers(void)
 		ZCSG(jit_counters_stopped) = true;
 		for (uint32_t i = 0; i < ZCSG(hash).max_num_entries; i++) {
 			zend_accel_hash_entry *cache_entry;
-			for (cache_entry = ZCSG(hash).hash_table[i]; cache_entry; cache_entry = cache_entry->next) {
+			for (cache_entry = zend_accel_hash_load_entry(&ZCSG(hash).hash_table[i]); cache_entry; cache_entry = cache_entry->next) {
 				zend_persistent_script *script;
-				if (cache_entry->indirect) continue;
-				script = (zend_persistent_script *)cache_entry->data;
+				script = zend_accel_hash_direct_data(cache_entry);
+				if (!script) continue;
 				zend_jit_stop_persistent_script(script);
 			}
 		}

ext/opcache/tests/gh24319_001.phpt

@@ -0,0 +1,139 @@
+--TEST--
+GH-24319: Concurrent includes while a direct OPcache key becomes an indirect alias
+--DESCRIPTION--
+Cache files under their original names, invalidate them, and replace them with
+symlinks to uncached files. Concurrent includes then race OPcache's conversion
+of direct entries into aliases. Readers must not observe a mismatched data
+pointer and indirect flag, or mistake an entry converted after lookup for a
+compiled script.
+
+This stress test disables opcache.protect_memory: the test runner normally
+enables it, but repeated mprotect() calls can dominate this workload and make
+its watchdog fail on slower systems. The small sequential companion test
+covers protected shared-memory reads.
+--EXTENSIONS--
+opcache
+pcntl
+--SKIPIF--
+<?php
+if (getenv('SKIP_SLOW_TESTS')) die('skip slow test');
+if (PHP_OS_FAMILY === 'Windows') die('skip Unix symlinks required');
+if (!function_exists('stream_socket_pair')) die('skip stream_socket_pair required');
+?>
+--INI--
+opcache.enable=1
+opcache.enable_cli=1
+opcache.jit=disable
+opcache.jit_buffer_size=0
+opcache.file_update_protection=0
+opcache.validate_timestamps=0
+opcache.revalidate_path=0
+opcache.protect_memory=0
+opcache.memory_consumption=256
+opcache.interned_strings_buffer=32
+opcache.max_accelerated_files=100000
+opcache.max_wasted_percentage=50
+--FILE--
+<?php
+$dir = __DIR__ . '/gh24319_001.' . getmypid();
+mkdir($dir);
+$numFiles = 20000;
+$numReaders = 8;
+$paths = [];
+
+// All old names initially have direct entries. Invalidate them, then replace
+// each file with a symlink to a different, not-yet-cached file. Loading the
+// new target converts the old name to an indirect OPcache entry.
+for ($i = 0; $i < $numFiles; ++$i) {
+    $old = "$dir/old$i.php";
+    $new = "$dir/new$i.php";
+    file_put_contents($old, "<?php return 1;\n");
+    file_put_contents($new, "<?php return 2;\n");
+    if ((require $old) !== 1 || !opcache_invalidate($old, true)) {
+        die("Warmup failed\n");
+    }
+    symlink($new, "$dir/link");
+    rename("$dir/link", $old);
+    $paths[] = $old;
+}
+clearstatcache(true);
+
+$children = [];
+for ($reader = 0; $reader < $numReaders; ++$reader) {
+    $pair = stream_socket_pair(STREAM_PF_UNIX, STREAM_SOCK_STREAM, 0);
+    if ($pair === false || ($pid = pcntl_fork()) === -1) {
+        die("Fork setup failed\n");
+    }
+    if ($pid === 0) {
+        // Do not retain the parent's endpoints for previously forked readers.
+        foreach ($children as $pipe) {
+            fclose($pipe);
+        }
+        fclose($pair[0]);
+        $start = fread($pair[1], 1);
+        fclose($pair[1]);
+        if ($start !== 'x') {
+            exit(3);
+        }
+        // Keep readers on different available CPUs where supported.
+        if (function_exists('pcntl_getcpuaffinity') && function_exists('pcntl_setcpuaffinity')) {
+            $cpus = pcntl_getcpuaffinity();
+            if ($cpus) {
+                pcntl_setcpuaffinity(null, [$cpus[$reader % count($cpus)]]);
+            }
+        }
+        pcntl_alarm(60);
+        // Each entry is converted only once. Further cached passes do not
+        // create additional direct-to-indirect transitions.
+        foreach ($paths as $path) {
+            if ((require $path) !== 2) {
+                exit(2);
+            }
+        }
+        pcntl_alarm(0);
+        exit(0);
+    }
+    fclose($pair[1]);
+    $children[$pid] = $pair[0];
+}
+
+foreach ($children as $pipe) {
+    fwrite($pipe, 'x');
+    fclose($pipe);
+}
+$failed = false;
+foreach (array_keys($children) as $reader => $pid) {
+    do {
+        $waited = pcntl_waitpid($pid, $status);
+    } while ($waited === -1 && pcntl_get_last_error() === PCNTL_EINTR);
+    if ($waited !== $pid) {
+        echo "Reader $reader: wait failed\n";
+        $failed = true;
+    } elseif (pcntl_wifsignaled($status)) {
+        $signal = pcntl_wtermsig($status);
+        echo "Reader $reader: signal $signal", $signal === SIGALRM ? " (watchdog)\n" : "\n";
+        $failed = true;
+    } elseif (!pcntl_wifexited($status) || pcntl_wexitstatus($status) !== 0) {
+        echo "Reader $reader: exit ", pcntl_wexitstatus($status), "\n";
+        $failed = true;
+    }
+}
+
+foreach ($paths as $i => $old) {
+    unlink($old);
+    unlink("$dir/new$i.php");
+}
+rmdir($dir);
+echo $failed ? "Failed\n" : "Done\n";
+?>
+--CLEAN--
+<?php
+foreach (glob(__DIR__ . '/gh24319_001.*', GLOB_ONLYDIR) ?: [] as $dir) {
+    foreach (glob($dir . '/*') ?: [] as $file) {
+        unlink($file);
+    }
+    rmdir($dir);
+}
+?>
+--EXPECT--
+Done

ext/opcache/tests/gh24319_002.phpt

@@ -0,0 +1,99 @@
+--TEST--
+GH-24319: Existing OPcache aliases follow later direct-to-indirect conversions
+--DESCRIPTION--
+An alias can already point to a direct cache entry when that entry is converted
+into another alias. Lookups must resolve the resulting chain, including after
+the final target is recompiled. Script enumeration must distinguish direct
+entries from aliases using a consistent snapshot. A conversion whose normalized
+target is the entry itself must not create an alias cycle. Protected shared
+memory is enabled to ensure that read-side atomic operations do not write.
+--EXTENSIONS--
+opcache
+--SKIPIF--
+<?php
+if (PHP_OS_FAMILY === 'Windows') die('skip Unix symlinks required');
+?>
+--INI--
+opcache.enable=1
+opcache.enable_cli=1
+opcache.jit=disable
+opcache.jit_buffer_size=0
+opcache.file_update_protection=0
+opcache.validate_timestamps=0
+opcache.revalidate_path=0
+opcache.protect_memory=1
+--FILE--
+<?php
+$dir = __DIR__ . '/gh24319_002.' . getmypid();
+mkdir($dir);
+foreach (['a' => 1, 'b' => 2, 'c' => 3] as $name => $value) {
+    file_put_contents("$dir/$name.php", "<?php return $value;\n");
+}
+var_dump(require "$dir/a.php");
+symlink("$dir/a.php", "$dir/alias.php");
+var_dump(require "$dir/alias.php");
+
+foreach (['a' => 'b', 'b' => 'c'] as $old => $new) {
+    opcache_invalidate("$dir/$old.php", true);
+    symlink("$dir/$new.php", "$dir/link");
+    rename("$dir/link", "$dir/$old.php");
+    clearstatcache(true);
+    var_dump(require "$dir/$old.php");
+    var_dump(require "$dir/alias.php");
+}
+$scripts = opcache_get_status()['scripts'];
+var_dump(isset($scripts["$dir/c.php"]));
+var_dump(isset($scripts["$dir/a.php"]), isset($scripts["$dir/b.php"]), isset($scripts["$dir/alias.php"]));
+
+// Replacing the final script must also update every existing alias chain.
+file_put_contents("$dir/c.php", "<?php return 6;\n");
+opcache_invalidate("$dir/c.php", true);
+var_dump(require "$dir/alias.php");
+var_dump(require "$dir/a.php");
+var_dump(require "$dir/b.php");
+var_dump(require "$dir/c.php");
+
+// Turn the original target into an alias of a name that previously pointed
+// back to it. The writer must not create a self-referential cache entry.
+file_put_contents("$dir/x.php", "<?php return 4;\n");
+var_dump(require "$dir/x.php");
+symlink("$dir/x.php", "$dir/y.php");
+var_dump(require "$dir/y.php");
+opcache_invalidate("$dir/x.php", true);
+unlink("$dir/y.php");
+file_put_contents("$dir/y.php", "<?php return 5;\n");
+symlink("$dir/y.php", "$dir/link");
+rename("$dir/link", "$dir/x.php");
+clearstatcache(true);
+var_dump(require "$dir/x.php");
+var_dump(require "$dir/y.php");
+
+foreach (glob($dir . '/*') as $file) unlink($file);
+rmdir($dir);
+?>
+--CLEAN--
+<?php
+foreach (glob(__DIR__ . '/gh24319_002.*', GLOB_ONLYDIR) ?: [] as $dir) {
+    foreach (glob($dir . '/*') ?: [] as $file) unlink($file);
+    rmdir($dir);
+}
+?>
+--EXPECT--
+int(1)
+int(1)
+int(2)
+int(2)
+int(3)
+int(3)
+bool(true)
+bool(false)
+bool(false)
+bool(false)
+int(6)
+int(6)
+int(6)
+int(6)
+int(4)
+int(4)
+int(5)
+int(5)