Skip to content

Commit 50e38ac

Browse files
committed
Merge branch 'PHP-8.6'
* PHP-8.6: Fix GH-24577: CE-cache flag on interned class name visible before map_ptr offset (#24585)
2 parents 593ed68 + 10b26c4 commit 50e38ac

5 files changed

Lines changed: 179 additions & 3 deletions

File tree

‎Zend/zend.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2119,6 +2119,7 @@ ZEND_API void zend_alloc_ce_cache(zend_string *type_name)
21192119
do {
21202120
ret = ZEND_MAP_PTR_NEW_OFFSET();
21212121
} while (ret <= 2);
2122-
GC_ADD_FLAGS(type_name, IS_STR_CLASS_NAME_MAP_PTR);
21232122
GC_SET_REFCOUNT(type_name, ret);
2123+
atomic_thread_fence(memory_order_release);
2124+
GC_ADD_FLAGS(type_name, IS_STR_CLASS_NAME_MAP_PTR);
21242125
}

‎Zend/zend_types.h‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
#include "zend_portability.h"
2525
#include "zend_long.h"
26+
#include "zend_atomic.h"
2627
#include <stdbool.h>
2728
#include <stdint.h>
2829

@@ -960,8 +961,29 @@ static zend_always_inline uint32_t zend_gc_delref_ex(zend_refcounted_h *p, uint3
960961

961962
#define OBJ_EXTRA_FLAGS(obj) ((obj)->extra_flags)
962963

964+
static zend_always_inline uint32_t zend_atomic_load_acquire_32(const uint32_t *ptr)
965+
{
966+
#if defined(__GNUC__) || defined(__clang__) || defined(HAVE_GNUC_ATOMICS)
967+
return __atomic_load_n(ptr, __ATOMIC_ACQUIRE);
968+
#else
969+
uint32_t val = *(const volatile uint32_t *)ptr;
970+
ZEND_ATOMIC_FENCE_ACQUIRE();
971+
return val;
972+
#endif
973+
}
974+
963975
/* Fast class cache */
964-
#define ZSTR_HAS_CE_CACHE(s) (GC_FLAGS(s) & IS_STR_CLASS_NAME_MAP_PTR)
976+
static zend_always_inline bool zstr_has_ce_cache(const zend_string *s)
977+
{
978+
static_assert(sizeof(_Atomic uint32_t) == sizeof(uint32_t), "atomic size");
979+
static_assert(alignof(_Atomic uint32_t) == alignof(uint32_t), "atomic align");
980+
static_assert(ATOMIC_INT_LOCK_FREE == 2, "not always lock-free");
981+
982+
uint32_t type_info = atomic_load_explicit((_Atomic uint32_t *)&GC_TYPE_INFO(s), memory_order_acquire);
983+
return (type_info & (IS_STR_CLASS_NAME_MAP_PTR << GC_FLAGS_SHIFT)) != 0;
984+
}
985+
986+
#define ZSTR_HAS_CE_CACHE(s) zstr_has_ce_cache(s)
965987
#define ZSTR_GET_CE_CACHE(s) ZSTR_GET_CE_CACHE_EX(s, 1)
966988
#define ZSTR_SET_CE_CACHE(s, ce) ZSTR_SET_CE_CACHE_EX(s, ce, 1)
967989

‎ext/opcache/ZendAccelerator.c‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,8 +457,9 @@ static void accel_interned_strings_restore_state(void)
457457
/* Discard non-global CE_CACHE slots on reset. */
458458
uintptr_t idx = (GC_REFCOUNT(s) - 1) / sizeof(void *);
459459
if (idx >= ZCSG(map_ptr_last)) {
460-
GC_SET_REFCOUNT(s, 2);
461460
GC_DEL_FLAGS(s, IS_STR_CLASS_NAME_MAP_PTR);
461+
atomic_thread_fence(memory_order_release);
462+
GC_SET_REFCOUNT(s, 2);
462463
}
463464
}
464465

@@ -567,6 +568,7 @@ zend_string* ZEND_FASTCALL accel_new_interned_string(zend_string *str)
567568
if (ZSTR_HAS_CE_CACHE(str) && !ZSTR_HAS_CE_CACHE(s)) {
568569
ZEND_ASSERT(GC_FLAGS(str) & IS_STR_PERMANENT);
569570
GC_SET_REFCOUNT(s, GC_REFCOUNT(str));
571+
atomic_thread_fence(memory_order_release);
570572
GC_ADD_FLAGS(s, IS_STR_CLASS_NAME_MAP_PTR);
571573
}
572574

‎ext/opcache/tests/gh24577.phpt‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
--TEST--
2+
Bug GH-24577: CE-cache flag on shared interned class name must not crash with concurrent access
3+
--EXTENSIONS--
4+
opcache
5+
pcntl
6+
--INI--
7+
opcache.enable=1
8+
opcache.enable_cli=1
9+
opcache.interned_strings_buffer=8
10+
opcache.file_update_protection=0
11+
--FILE--
12+
<?php
13+
$consumerFile = __DIR__ . '/gh24577_consumer.inc';
14+
$classesFile = __DIR__ . '/gh24577_classes.inc';
15+
$gateFile = __DIR__ . '/gh24577_gate.tmp';
16+
$readyPrefix = __DIR__ . '/gh24577_ready_';
17+
$numClasses = 64;
18+
$numReaders = 6;
19+
20+
@unlink($gateFile);
21+
22+
// 1. Consumer script: holds class-name string literals in SHM interned strings
23+
// buffer (with refcount = 2 and no CE-cache flag yet).
24+
$consumerCode = "<?php\nfunction gh24577_names(): array {\n return [\n";
25+
for ($i = 0; $i < $numClasses; $i++) {
26+
$consumerCode .= sprintf(" 'ColdCacheClass%04d',\n", $i);
27+
}
28+
$consumerCode .= " ];\n}\n";
29+
$consumerCode .= <<<'PHP'
30+
function gh24577_probe(array $names): bool {
31+
foreach ($names as $class) {
32+
if (class_exists($class, false)) {
33+
$obj = new $class();
34+
if (!($obj instanceof $class)) {
35+
return false;
36+
}
37+
}
38+
}
39+
return true;
40+
}
41+
PHP;
42+
file_put_contents($consumerFile, $consumerCode);
43+
touch($consumerFile, time() - 3600);
44+
45+
// 2. Provider script: declares the classes, triggering CE-cache slot publication
46+
// on the pre-existing shared interned strings when persisted by the writer.
47+
$classesCode = "<?php\n";
48+
for ($i = 0; $i < $numClasses; $i++) {
49+
$classesCode .= sprintf("class ColdCacheClass%04d {}\n", $i);
50+
}
51+
file_put_contents($classesFile, $classesCode);
52+
touch($classesFile, time() - 3600);
53+
54+
// Step 1: Warm up ONLY the consumer script in OPcache SHM so its class-name
55+
// strings are interned in shared memory with refcount = 2.
56+
$warmup = pcntl_fork();
57+
if ($warmup === -1) {
58+
die("pcntl_fork failed\n");
59+
} elseif ($warmup === 0) {
60+
require $consumerFile;
61+
$status = opcache_get_status();
62+
exit(isset($status['scripts'][$consumerFile]) ? 0 : 2);
63+
}
64+
pcntl_waitpid($warmup, $wstatus);
65+
if (!pcntl_wifexited($wstatus) || pcntl_wexitstatus($wstatus) !== 0) {
66+
die("Warmup failed\n");
67+
}
68+
69+
// Step 2: Fork Readers (which load the cached consumer script first, acquiring
70+
// the shared interned strings) and the Writer (which persists $classesFile),
71+
// synchronized by a start barrier.
72+
$children = [];
73+
74+
for ($r = 0; $r < $numReaders; $r++) {
75+
$pid = pcntl_fork();
76+
if ($pid === -1) {
77+
die("pcntl_fork failed\n");
78+
} elseif ($pid === 0) {
79+
require $consumerFile;
80+
$names = gh24577_names();
81+
file_put_contents($readyPrefix . $r, '1');
82+
while (!file_exists($gateFile)) {
83+
usleep(100);
84+
}
85+
// Probe shared interned strings while writer is persisting $classesFile
86+
for ($iter = 0; $iter < 16; $iter++) {
87+
if (!gh24577_probe($names)) {
88+
exit(3);
89+
}
90+
}
91+
require $classesFile;
92+
if (!gh24577_probe($names)) {
93+
exit(4);
94+
}
95+
exit(0);
96+
}
97+
$children[] = $pid;
98+
}
99+
100+
$writer = pcntl_fork();
101+
if ($writer === -1) {
102+
die("pcntl_fork failed\n");
103+
} elseif ($writer === 0) {
104+
file_put_contents($readyPrefix . 'writer', '1');
105+
while (!file_exists($gateFile)) {
106+
usleep(100);
107+
}
108+
require $classesFile;
109+
$status = opcache_get_status();
110+
exit(isset($status['scripts'][$classesFile]) ? 0 : 5);
111+
}
112+
$children[] = $writer;
113+
114+
// Wait until all readers and the writer are ready at the barrier
115+
$expectedReady = array_map(fn($r) => $readyPrefix . $r, range(0, $numReaders - 1));
116+
$expectedReady[] = $readyPrefix . 'writer';
117+
foreach ($expectedReady as $readyFile) {
118+
while (!file_exists($readyFile)) {
119+
usleep(200);
120+
}
121+
}
122+
123+
// Release barrier so writer persists $classesFile while readers probe $names
124+
file_put_contents($gateFile, '1');
125+
126+
$failed = false;
127+
foreach ($children as $pid) {
128+
pcntl_waitpid($pid, $status);
129+
if (!pcntl_wifexited($status) || pcntl_wexitstatus($status) !== 0) {
130+
$failed = true;
131+
}
132+
}
133+
134+
foreach ($expectedReady as $readyFile) {
135+
@unlink($readyFile);
136+
}
137+
@unlink($gateFile);
138+
@unlink($consumerFile);
139+
@unlink($classesFile);
140+
141+
echo $failed ? "Failed\n" : "Done\n";
142+
?>
143+
--CLEAN--
144+
<?php
145+
foreach (glob(__DIR__ . '/gh24577_*') ?: [] as $f) {
146+
@unlink($f);
147+
}
148+
?>
149+
--EXPECT--
150+
Done

‎ext/opcache/zend_persist.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,7 @@ void zend_accel_pending_ce_cache_commit(void)
371371

372372
ZEND_ASSERT(!ZSTR_HAS_CE_CACHE(type_name));
373373
GC_SET_REFCOUNT(type_name, (uint32_t)Z_LVAL_P(zv));
374+
atomic_thread_fence(memory_order_release);
374375
GC_ADD_FLAGS(type_name, IS_STR_CLASS_NAME_MAP_PTR);
375376
} ZEND_HASH_FOREACH_END();
376377

0 commit comments

Comments
 (0)