Skip to content

Commit ec35e3a

Browse files
committed
Fix GH-17715: Handle preloaded internal function runtime cache
This solely affects the builtin enum functions currently. Given that these are stored in SHM, we cannot simply hardwire a pointer into the internal function runtime cache on NTS too, but have to use a MAP_PTR (like on ZTS). Now, by design, the runtime cache of internal functions no longer is reset between requests, hence we need to store them explicitly as static runtime cache. On NTS builds we cannot trivially move the pointers into CG(internal_run_time_cache) as they're directly stored on the individual functions (on ZTS we could simply iterate the static map_ptrs). Hence, we have the choice between having opcache managing the internal run_time_cache for its preloaded functions itself or realloc CG(internal_run_time_cache) and iterate through all functions to assign the new address. We choose the latter for simplicity and initial speed. Note: map_ptr_static_last has been added as last element to zend_accel_shared_globals, so that accesses to it are compatible. We do not have to care about the ABI of creating new zend_accel_shared_globals structs. That's opcaches prerogative.
1 parent d6c3079 commit ec35e3a

File tree

7 files changed

+134
-20
lines changed

7 files changed

+134
-20
lines changed

NEWS

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ PHP NEWS
4848
. Fixed bug GH-17577 (JIT packed type guard crash). (nielsdos, Dmitry)
4949
. Fixed bug GH-17747 (Exception on reading property in register-based
5050
FETCH_OBJ_R breaks JIT). (Dmitry, nielsdos)
51+
. Fixed bug GH-17715 (Null pointer deref in observer API when calling
52+
cases() method on preloaded enum). (Bob)
5153

5254
- Phar:
5355
. Fixed bug GH-17808: PharFileInfo refcount bug. (nielsdos)
@@ -61,7 +63,7 @@ PHP NEWS
6163
. Fix memory leak on overflow in _php_stream_scandir(). (nielsdos)
6264

6365
- Windows:
64-
. Fixed phpize for Windows 11 (24H2). (bwoebi)
66+
. Fixed phpize for Windows 11 (24H2). (Bob)
6567

6668
- Zlib:
6769
. Fixed bug GH-17745 (zlib extension incorrectly handles object arguments).

Zend/zend_enum.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,10 @@ static void zend_enum_register_func(zend_class_entry *ce, zend_known_string_id n
418418
zif->module = EG(current_module);
419419
zif->scope = ce;
420420
zif->T = ZEND_OBSERVER_ENABLED;
421-
if (EG(active)) { // at run-time
421+
if (EG(active)) { // at run-time
422+
if (CG(compiler_options) & ZEND_COMPILE_PRELOAD) {
423+
zif->fn_flags |= ZEND_ACC_PRELOADED;
424+
}
422425
ZEND_MAP_PTR_INIT(zif->run_time_cache, zend_arena_calloc(&CG(arena), 1, zend_internal_run_time_cache_reserved_size()));
423426
} else {
424427
#ifdef ZTS

ext/opcache/ZendAccelerator.c

Lines changed: 59 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2609,6 +2609,7 @@ static void zend_reset_cache_vars(void)
26092609
ZCSG(restart_pending) = false;
26102610
ZCSG(force_restart_time) = 0;
26112611
ZCSG(map_ptr_last) = CG(map_ptr_last);
2612+
ZCSG(map_ptr_static_last) = zend_map_ptr_static_last;
26122613
}
26132614

26142615
static void accel_reset_pcre_cache(void)
@@ -2624,7 +2625,7 @@ static void accel_reset_pcre_cache(void)
26242625
} ZEND_HASH_FOREACH_END();
26252626
}
26262627

2627-
zend_result accel_activate(INIT_FUNC_ARGS)
2628+
ZEND_RINIT_FUNCTION(zend_accelerator)
26282629
{
26292630
if (!ZCG(enabled) || !accel_startup_ok) {
26302631
ZCG(accelerator_enabled) = false;
@@ -2956,12 +2957,15 @@ static void accel_globals_ctor(zend_accel_globals *accel_globals)
29562957
GC_MAKE_PERSISTENT_LOCAL(accel_globals->key);
29572958
}
29582959

2959-
#ifdef ZTS
29602960
static void accel_globals_dtor(zend_accel_globals *accel_globals)
29612961
{
2962+
#ifdef ZTS
29622963
zend_string_free(accel_globals->key);
2963-
}
29642964
#endif
2965+
if (accel_globals->preloaded_internal_run_time_cache) {
2966+
pefree(accel_globals->preloaded_internal_run_time_cache, 1);
2967+
}
2968+
}
29652969

29662970
#ifdef HAVE_HUGE_CODE_PAGES
29672971
# ifndef _WIN32
@@ -3402,6 +3406,8 @@ void accel_shutdown(void)
34023406
if (!ZCG(enabled) || !accel_startup_ok) {
34033407
#ifdef ZTS
34043408
ts_free_id(accel_globals_id);
3409+
#else
3410+
accel_globals_dtor(&accel_globals);
34053411
#endif
34063412
return;
34073413
}
@@ -3416,6 +3422,8 @@ void accel_shutdown(void)
34163422

34173423
#ifdef ZTS
34183424
ts_free_id(accel_globals_id);
3425+
#else
3426+
accel_globals_dtor(&accel_globals);
34193427
#endif
34203428

34213429
if (!_file_cache_only) {
@@ -4313,7 +4321,7 @@ static zend_persistent_script* preload_script_in_shared_memory(zend_persistent_s
43134321
return new_persistent_script;
43144322
}
43154323

4316-
static void preload_load(void)
4324+
static void preload_load(size_t orig_map_ptr_static_last)
43174325
{
43184326
/* Load into process tables */
43194327
zend_script *script = &ZCSG(preload_script)->script;
@@ -4348,14 +4356,43 @@ static void preload_load(void)
43484356
if (EG(class_table)) {
43494357
EG(persistent_classes_count) = EG(class_table)->nNumUsed;
43504358
}
4351-
if (CG(map_ptr_last) != ZCSG(map_ptr_last)) {
4352-
size_t old_map_ptr_last = CG(map_ptr_last);
4359+
4360+
size_t old_map_ptr_last = CG(map_ptr_last);
4361+
if (zend_map_ptr_static_last != ZCSG(map_ptr_static_last) || old_map_ptr_last != ZCSG(map_ptr_last)) {
43534362
CG(map_ptr_last) = ZCSG(map_ptr_last);
4354-
CG(map_ptr_size) = ZEND_MM_ALIGNED_SIZE_EX(CG(map_ptr_last) + 1, 4096);
4355-
CG(map_ptr_real_base) = perealloc(CG(map_ptr_real_base), CG(map_ptr_size) * sizeof(void*), 1);
4363+
CG(map_ptr_size) = ZEND_MM_ALIGNED_SIZE_EX(ZCSG(map_ptr_last) + 1, 4096);
4364+
zend_map_ptr_static_last = ZCSG(map_ptr_static_last);
4365+
4366+
/* Grow map_ptr table as needed, but allocate once for static + regular map_ptrs */
4367+
size_t new_static_size = ((zend_map_ptr_static_last - 1) & 4095) + 1;
4368+
if (zend_map_ptr_static_size != new_static_size) {
4369+
void *new_base = pemalloc((new_static_size + CG(map_ptr_size)) * sizeof(void *), 1);
4370+
if (CG(map_ptr_real_base)) {
4371+
memcpy((void **) new_base + new_static_size - zend_map_ptr_static_size, CG(map_ptr_real_base), (old_map_ptr_last + zend_map_ptr_static_size) * sizeof(void *));
4372+
pefree(CG(map_ptr_real_base), 1);
4373+
}
4374+
CG(map_ptr_real_base) = new_base;
4375+
zend_map_ptr_static_size = new_static_size;
4376+
} else {
4377+
CG(map_ptr_real_base) = perealloc(CG(map_ptr_real_base), CG(map_ptr_size) * sizeof(void *), 1);
4378+
}
4379+
4380+
memset((void **) CG(map_ptr_real_base) + zend_map_ptr_static_size + old_map_ptr_last, 0, (CG(map_ptr_last) - old_map_ptr_last) * sizeof(void *));
43564381
CG(map_ptr_base) = ZEND_MAP_PTR_BIASED_BASE(CG(map_ptr_real_base));
4357-
memset((void **) CG(map_ptr_real_base) + old_map_ptr_last, 0,
4358-
(CG(map_ptr_last) - old_map_ptr_last) * sizeof(void *));
4382+
}
4383+
4384+
if (orig_map_ptr_static_last != zend_map_ptr_static_last) {
4385+
/* preloaded static entries currently are all runtime cache pointers, just assign them as such */
4386+
size_t runtime_cache_size = zend_internal_run_time_cache_reserved_size();
4387+
ZCG(preloaded_internal_run_time_cache_size) = (zend_map_ptr_static_last - orig_map_ptr_static_last) * sizeof(void *);
4388+
char *cache = pemalloc(ZCG(preloaded_internal_run_time_cache_size), 1);
4389+
ZCG(preloaded_internal_run_time_cache) = cache;
4390+
4391+
for (size_t cur_static_map_ptr = orig_map_ptr_static_last; cur_static_map_ptr < zend_map_ptr_static_last; ++cur_static_map_ptr) {
4392+
void **ptr = (void **) CG(map_ptr_real_base) + zend_map_ptr_static_size - ((cur_static_map_ptr & ~4095) + 4096) + (cur_static_map_ptr & 4095);
4393+
*ptr = cache;
4394+
cache += runtime_cache_size;
4395+
}
43594396
}
43604397
}
43614398

@@ -4364,7 +4401,7 @@ static zend_result accel_preload(const char *config, bool in_child)
43644401
zend_file_handle file_handle;
43654402
zend_result ret;
43664403
char *orig_open_basedir;
4367-
size_t orig_map_ptr_last;
4404+
size_t orig_map_ptr_last, orig_map_ptr_static_last;
43684405
uint32_t orig_compiler_options;
43694406

43704407
ZCG(enabled) = false;
@@ -4375,6 +4412,7 @@ static zend_result accel_preload(const char *config, bool in_child)
43754412
accelerator_orig_compile_file = preload_compile_file;
43764413

43774414
orig_map_ptr_last = CG(map_ptr_last);
4415+
orig_map_ptr_static_last = zend_map_ptr_static_last;
43784416

43794417
/* Compile and execute preloading script */
43804418
zend_stream_init_filename(&file_handle, (char *) config);
@@ -4554,7 +4592,7 @@ static zend_result accel_preload(const char *config, bool in_child)
45544592
SHM_PROTECT();
45554593
HANDLE_UNBLOCK_INTERRUPTIONS();
45564594

4557-
preload_load();
4595+
preload_load(orig_map_ptr_static_last);
45584596

45594597
/* Store individual scripts with unlinked classes */
45604598
HANDLE_BLOCK_INTERRUPTIONS();
@@ -4806,7 +4844,7 @@ static zend_result accel_finish_startup(void)
48064844

48074845
if (ZCSG(preload_script)) {
48084846
/* Preloading was done in another process */
4809-
preload_load();
4847+
preload_load(zend_map_ptr_static_last);
48104848
zend_shared_alloc_unlock();
48114849
return SUCCESS;
48124850
}
@@ -4834,7 +4872,7 @@ static zend_result accel_finish_startup(void)
48344872
}
48354873

48364874
if (ZCSG(preload_script)) {
4837-
preload_load();
4875+
preload_load(zend_map_ptr_static_last);
48384876
}
48394877

48404878
zend_shared_alloc_unlock();
@@ -4848,6 +4886,12 @@ static zend_result accel_finish_startup(void)
48484886
#endif /* ZEND_WIN32 */
48494887
}
48504888

4889+
static void accel_activate(void) {
4890+
if (ZCG(preloaded_internal_run_time_cache)) {
4891+
memset(ZCG(preloaded_internal_run_time_cache), 0, ZCG(preloaded_internal_run_time_cache_size));
4892+
}
4893+
}
4894+
48514895
ZEND_EXT_API zend_extension zend_extension_entry = {
48524896
ACCELERATOR_PRODUCT_NAME, /* name */
48534897
PHP_VERSION, /* version */
@@ -4856,7 +4900,7 @@ ZEND_EXT_API zend_extension zend_extension_entry = {
48564900
"Copyright (c)", /* copyright */
48574901
accel_startup, /* startup */
48584902
NULL, /* shutdown */
4859-
NULL, /* per-script activation */
4903+
accel_activate, /* per-script activation */
48604904
#ifdef HAVE_JIT
48614905
accel_deactivate, /* per-script deactivation */
48624906
#else

ext/opcache/ZendAccelerator.h

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

5151
#include "zend_extensions.h"
5252
#include "zend_compile.h"
53+
#include "zend_API.h"
5354

5455
#include "Optimizer/zend_optimizer.h"
5556
#include "zend_accelerator_hash.h"
@@ -216,6 +217,8 @@ typedef struct _zend_accel_globals {
216217
#ifndef ZEND_WIN32
217218
zend_ulong root_hash;
218219
#endif
220+
void *preloaded_internal_run_time_cache;
221+
size_t preloaded_internal_run_time_cache_size;
219222
/* preallocated shared-memory block to save current script */
220223
void *mem;
221224
zend_persistent_script *current_persistent_script;
@@ -280,6 +283,8 @@ typedef struct _zend_accel_shared_globals {
280283

281284
/* Interned Strings Support (must be the last element) */
282285
ZEND_SET_ALIGNED(ZEND_STRING_TABLE_POS_ALIGNMENT, zend_string_table interned_strings);
286+
287+
size_t map_ptr_static_last;
283288
} zend_accel_shared_globals;
284289

285290
#ifdef ZEND_WIN32
@@ -310,7 +315,7 @@ extern const char *zps_api_failure_reason;
310315
BEGIN_EXTERN_C()
311316

312317
void accel_shutdown(void);
313-
zend_result accel_activate(INIT_FUNC_ARGS);
318+
ZEND_RINIT_FUNCTION(zend_accelerator);
314319
zend_result accel_post_deactivate(void);
315320
void zend_accel_schedule_restart(zend_accel_restart_reason reason);
316321
void zend_accel_schedule_restart_if_necessary(zend_accel_restart_reason reason);
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
--TEST--
2+
Enum preloading with observers
3+
--EXTENSIONS--
4+
opcache
5+
zend_test
6+
--INI--
7+
opcache.enable=1
8+
opcache.enable_cli=1
9+
opcache.optimization_level=-1
10+
opcache.preload={PWD}/preload_enum.inc
11+
zend_test.observer.enabled=1
12+
zend_test.observer.show_output=1
13+
zend_test.observer.observe_all=1
14+
--SKIPIF--
15+
<?php
16+
if (PHP_OS_FAMILY == 'Windows') die('skip Preloading is not supported on Windows');
17+
?>
18+
--FILE--
19+
<?php
20+
21+
spl_autoload_register(static function ($class) {
22+
if ($class === 'MyEnum') {
23+
require_once(__DIR__ . '/preload_enum.inc');
24+
}
25+
});
26+
27+
var_dump(MyEnum::cases());
28+
29+
?>
30+
--EXPECTF--
31+
<!-- init '%spreload_enum.inc' -->
32+
<file '%spreload_enum.inc'>
33+
<!-- init var_dump() -->
34+
<var_dump>
35+
enum(MyEnum::Bar)
36+
</var_dump>
37+
</file '%spreload_enum.inc'>
38+
<!-- init '%spreload_enum_observed.php' -->
39+
<file '%spreload_enum_observed.php'>
40+
<!-- init spl_autoload_register() -->
41+
<spl_autoload_register>
42+
</spl_autoload_register>
43+
<!-- init MyEnum::cases() -->
44+
<MyEnum::cases>
45+
</MyEnum::cases>
46+
<!-- init var_dump() -->
47+
<var_dump>
48+
array(2) {
49+
[0]=>
50+
enum(MyEnum::Foo)
51+
[1]=>
52+
enum(MyEnum::Bar)
53+
}
54+
</var_dump>
55+
</file '%spreload_enum_observed.php'>

ext/opcache/zend_accelerator_module.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,7 @@ static zend_module_entry accel_module_entry = {
572572
ext_functions,
573573
ZEND_MINIT(zend_accelerator),
574574
ZEND_MSHUTDOWN(zend_accelerator),
575-
accel_activate,
575+
ZEND_RINIT(zend_accelerator),
576576
NULL,
577577
zend_accel_info,
578578
PHP_VERSION,

ext/opcache/zend_persist.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -735,7 +735,11 @@ static zend_op_array *zend_persist_class_method(zend_op_array *op_array, zend_cl
735735
// Real dynamically created internal functions like enum methods must have their own run_time_cache pointer. They're always on the same scope as their defining class.
736736
// However, copies - as caused by inheritance of internal methods - must retain the original run_time_cache pointer, shared with the source function.
737737
if (!op_array->scope || (op_array->scope == ce && !(op_array->fn_flags & ZEND_ACC_TRAIT_CLONE))) {
738-
ZEND_MAP_PTR_NEW(op_array->run_time_cache);
738+
if (op_array->fn_flags & ZEND_ACC_PRELOADED) {
739+
ZEND_MAP_PTR_NEW_STATIC(op_array->run_time_cache);
740+
} else {
741+
ZEND_MAP_PTR_NEW(op_array->run_time_cache);
742+
}
739743
}
740744
}
741745
}
@@ -1413,6 +1417,7 @@ zend_persistent_script *zend_accel_script_persist(zend_persistent_script *script
14131417

14141418
if (for_shm) {
14151419
ZCSG(map_ptr_last) = CG(map_ptr_last);
1420+
ZCSG(map_ptr_static_last) = zend_map_ptr_static_last;
14161421
}
14171422

14181423
#ifdef HAVE_JIT

0 commit comments

Comments
 (0)