Skip to content

Commit

Permalink
Fix GH-17715: Handle preloaded internal function runtime cache
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bwoebi committed Feb 16, 2025
1 parent d6c3079 commit c37d9f8
Show file tree
Hide file tree
Showing 7 changed files with 133 additions and 20 deletions.
4 changes: 3 additions & 1 deletion NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ PHP NEWS
. Fixed bug GH-17577 (JIT packed type guard crash). (nielsdos, Dmitry)
. Fixed bug GH-17747 (Exception on reading property in register-based
FETCH_OBJ_R breaks JIT). (Dmitry, nielsdos)
. Fixed bug GH-17715 (Null pointer deref in observer API when calling
cases() method on preloaded enum). (Bob)

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

- Windows:
. Fixed phpize for Windows 11 (24H2). (bwoebi)
. Fixed phpize for Windows 11 (24H2). (Bob)

- Zlib:
. Fixed bug GH-17745 (zlib extension incorrectly handles object arguments).
Expand Down
5 changes: 4 additions & 1 deletion Zend/zend_enum.c
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,10 @@ static void zend_enum_register_func(zend_class_entry *ce, zend_known_string_id n
zif->module = EG(current_module);
zif->scope = ce;
zif->T = ZEND_OBSERVER_ENABLED;
if (EG(active)) { // at run-time
if (EG(active)) { // at run-time
if (CG(compiler_options) & ZEND_COMPILE_PRELOAD) {
zif->fn_flags |= ZEND_ACC_PRELOADED;
}
ZEND_MAP_PTR_INIT(zif->run_time_cache, zend_arena_calloc(&CG(arena), 1, zend_internal_run_time_cache_reserved_size()));
} else {
#ifdef ZTS
Expand Down
74 changes: 59 additions & 15 deletions ext/opcache/ZendAccelerator.c
Original file line number Diff line number Diff line change
Expand Up @@ -2609,6 +2609,7 @@ static void zend_reset_cache_vars(void)
ZCSG(restart_pending) = false;
ZCSG(force_restart_time) = 0;
ZCSG(map_ptr_last) = CG(map_ptr_last);
ZCSG(map_ptr_static_last) = zend_map_ptr_static_last;
}

static void accel_reset_pcre_cache(void)
Expand All @@ -2624,7 +2625,7 @@ static void accel_reset_pcre_cache(void)
} ZEND_HASH_FOREACH_END();
}

zend_result accel_activate(INIT_FUNC_ARGS)
ZEND_RINIT_FUNCTION(zend_accelerator)
{
if (!ZCG(enabled) || !accel_startup_ok) {
ZCG(accelerator_enabled) = false;
Expand Down Expand Up @@ -2956,12 +2957,15 @@ static void accel_globals_ctor(zend_accel_globals *accel_globals)
GC_MAKE_PERSISTENT_LOCAL(accel_globals->key);
}

#ifdef ZTS
static void accel_globals_dtor(zend_accel_globals *accel_globals)
{
#ifdef ZTS
zend_string_free(accel_globals->key);
}
#endif
if (accel_globals->preloaded_internal_run_time_cache) {
pefree(accel_globals->preloaded_internal_run_time_cache, 1);
}
}

#ifdef HAVE_HUGE_CODE_PAGES
# ifndef _WIN32
Expand Down Expand Up @@ -3402,6 +3406,8 @@ void accel_shutdown(void)
if (!ZCG(enabled) || !accel_startup_ok) {
#ifdef ZTS
ts_free_id(accel_globals_id);
#else
accel_globals_dtor(&accel_globals);
#endif
return;
}
Expand All @@ -3416,6 +3422,8 @@ void accel_shutdown(void)

#ifdef ZTS
ts_free_id(accel_globals_id);
#else
accel_globals_dtor(&accel_globals);
#endif

if (!_file_cache_only) {
Expand Down Expand Up @@ -4313,7 +4321,7 @@ static zend_persistent_script* preload_script_in_shared_memory(zend_persistent_s
return new_persistent_script;
}

static void preload_load(void)
static void preload_load(size_t orig_map_ptr_static_last)
{
/* Load into process tables */
zend_script *script = &ZCSG(preload_script)->script;
Expand Down Expand Up @@ -4348,14 +4356,43 @@ static void preload_load(void)
if (EG(class_table)) {
EG(persistent_classes_count) = EG(class_table)->nNumUsed;
}
if (CG(map_ptr_last) != ZCSG(map_ptr_last)) {
size_t old_map_ptr_last = CG(map_ptr_last);

size_t old_map_ptr_last = CG(map_ptr_last);
if (zend_map_ptr_static_last != ZCSG(map_ptr_static_last) || old_map_ptr_last != ZCSG(map_ptr_last)) {
CG(map_ptr_last) = ZCSG(map_ptr_last);
CG(map_ptr_size) = ZEND_MM_ALIGNED_SIZE_EX(CG(map_ptr_last) + 1, 4096);
CG(map_ptr_real_base) = perealloc(CG(map_ptr_real_base), CG(map_ptr_size) * sizeof(void*), 1);
CG(map_ptr_size) = ZEND_MM_ALIGNED_SIZE_EX(ZCSG(map_ptr_last) + 1, 4096);
zend_map_ptr_static_last = ZCSG(map_ptr_static_last);

/* Grow map_ptr table as needed, but allocate once for static + regular map_ptrs */
size_t new_static_size = ((zend_map_ptr_static_last - 1) & 4095) + 1;
if (zend_map_ptr_static_size != new_static_size) {
void *new_base = pemalloc((new_static_size + CG(map_ptr_size)) * sizeof(void *), 1);
if (CG(map_ptr_real_base)) {
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 *));
pefree(CG(map_ptr_real_base), 1);
}
CG(map_ptr_real_base) = new_base;
zend_map_ptr_static_size = new_static_size;
} else {
CG(map_ptr_real_base) = perealloc(CG(map_ptr_real_base), CG(map_ptr_size) * sizeof(void *), 1);
}

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 *));
CG(map_ptr_base) = ZEND_MAP_PTR_BIASED_BASE(CG(map_ptr_real_base));
memset((void **) CG(map_ptr_real_base) + old_map_ptr_last, 0,
(CG(map_ptr_last) - old_map_ptr_last) * sizeof(void *));
}

if (orig_map_ptr_static_last != zend_map_ptr_static_last) {
/* preloaded static entries currently are all runtime cache pointers, just assign them as such */
size_t runtime_cache_size = zend_internal_run_time_cache_reserved_size();
ZCG(preloaded_internal_run_time_cache_size) = (zend_map_ptr_static_last - orig_map_ptr_static_last) * sizeof(void *);
char *cache = pemalloc(ZCG(preloaded_internal_run_time_cache_size), 1);
ZCG(preloaded_internal_run_time_cache) = cache;

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) {
void **ptr = (void **) CG(map_ptr_real_base) + zend_map_ptr_static_size - ((cur_static_map_ptr & ~4095) + 4096) + (cur_static_map_ptr & 4095);
*ptr = cache;
cache += runtime_cache_size;
}
}
}

Expand All @@ -4364,7 +4401,7 @@ static zend_result accel_preload(const char *config, bool in_child)
zend_file_handle file_handle;
zend_result ret;
char *orig_open_basedir;
size_t orig_map_ptr_last;
size_t orig_map_ptr_last, orig_map_ptr_static_last;
uint32_t orig_compiler_options;

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

orig_map_ptr_last = CG(map_ptr_last);
orig_map_ptr_static_last = zend_map_ptr_static_last;

/* Compile and execute preloading script */
zend_stream_init_filename(&file_handle, (char *) config);
Expand Down Expand Up @@ -4554,7 +4592,7 @@ static zend_result accel_preload(const char *config, bool in_child)
SHM_PROTECT();
HANDLE_UNBLOCK_INTERRUPTIONS();

preload_load();
preload_load(orig_map_ptr_static_last);

/* Store individual scripts with unlinked classes */
HANDLE_BLOCK_INTERRUPTIONS();
Expand Down Expand Up @@ -4806,7 +4844,7 @@ static zend_result accel_finish_startup(void)

if (ZCSG(preload_script)) {
/* Preloading was done in another process */
preload_load();
preload_load(zend_map_ptr_static_last);
zend_shared_alloc_unlock();
return SUCCESS;
}
Expand Down Expand Up @@ -4834,7 +4872,7 @@ static zend_result accel_finish_startup(void)
}

if (ZCSG(preload_script)) {
preload_load();
preload_load(zend_map_ptr_static_last);
}

zend_shared_alloc_unlock();
Expand All @@ -4848,6 +4886,12 @@ static zend_result accel_finish_startup(void)
#endif /* ZEND_WIN32 */
}

static void accel_activate(void) {
if (ZCG(preloaded_internal_run_time_cache)) {
memset(ZCG(preloaded_internal_run_time_cache), 0, ZCG(preloaded_internal_run_time_cache_size));
}
}

ZEND_EXT_API zend_extension zend_extension_entry = {
ACCELERATOR_PRODUCT_NAME, /* name */
PHP_VERSION, /* version */
Expand All @@ -4856,7 +4900,7 @@ ZEND_EXT_API zend_extension zend_extension_entry = {
"Copyright (c)", /* copyright */
accel_startup, /* startup */
NULL, /* shutdown */
NULL, /* per-script activation */
accel_activate, /* per-script activation */
#ifdef HAVE_JIT
accel_deactivate, /* per-script deactivation */
#else
Expand Down
7 changes: 6 additions & 1 deletion ext/opcache/ZendAccelerator.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@

#include "zend_extensions.h"
#include "zend_compile.h"
#include "zend_API.h"

#include "Optimizer/zend_optimizer.h"
#include "zend_accelerator_hash.h"
Expand Down Expand Up @@ -216,6 +217,8 @@ typedef struct _zend_accel_globals {
#ifndef ZEND_WIN32
zend_ulong root_hash;
#endif
void *preloaded_internal_run_time_cache;
size_t preloaded_internal_run_time_cache_size;
/* preallocated shared-memory block to save current script */
void *mem;
zend_persistent_script *current_persistent_script;
Expand Down Expand Up @@ -280,6 +283,8 @@ typedef struct _zend_accel_shared_globals {

/* Interned Strings Support (must be the last element) */
ZEND_SET_ALIGNED(ZEND_STRING_TABLE_POS_ALIGNMENT, zend_string_table interned_strings);

size_t map_ptr_static_last;
} zend_accel_shared_globals;

#ifdef ZEND_WIN32
Expand Down Expand Up @@ -310,7 +315,7 @@ extern const char *zps_api_failure_reason;
BEGIN_EXTERN_C()

void accel_shutdown(void);
zend_result accel_activate(INIT_FUNC_ARGS);
ZEND_RINIT_FUNCTION(zend_accelerator);
zend_result accel_post_deactivate(void);
void zend_accel_schedule_restart(zend_accel_restart_reason reason);
void zend_accel_schedule_restart_if_necessary(zend_accel_restart_reason reason);
Expand Down
54 changes: 54 additions & 0 deletions ext/opcache/tests/preload_enum_observed.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
--TEST--
Enum preloading with observers
--EXTENSIONS--
opcache
zend_test
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.optimization_level=-1
opcache.preload={PWD}/preload_enum.inc
zend_test.observer.enabled=1
zend_test.observer.observe_all=1
--SKIPIF--
<?php
if (PHP_OS_FAMILY == 'Windows') die('skip Preloading is not supported on Windows');
?>
--FILE--
<?php

spl_autoload_register(static function ($class) {
if ($class === 'MyEnum') {
require_once(__DIR__ . '/preload_enum.inc');
}
});

var_dump(MyEnum::cases());

?>
--EXPECTF--
<!-- init '%spreload_enum.inc' -->
<file '%spreload_enum.inc'>
<!-- init var_dump() -->
<var_dump>
enum(MyEnum::Bar)
</var_dump>
</file '%spreload_enum.inc'>
<!-- init '%spreload_enum_observed.php' -->
<file '%spreload_enum_observed.php'>
<!-- init spl_autoload_register() -->
<spl_autoload_register>
</spl_autoload_register>
<!-- init MyEnum::cases() -->
<MyEnum::cases>
</MyEnum::cases>
<!-- init var_dump() -->
<var_dump>
array(2) {
[0]=>
enum(MyEnum::Foo)
[1]=>
enum(MyEnum::Bar)
}
</var_dump>
</file '%spreload_enum_observed.php'>
2 changes: 1 addition & 1 deletion ext/opcache/zend_accelerator_module.c
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ static zend_module_entry accel_module_entry = {
ext_functions,
ZEND_MINIT(zend_accelerator),
ZEND_MSHUTDOWN(zend_accelerator),
accel_activate,
ZEND_RINIT(zend_accelerator),
NULL,
zend_accel_info,
PHP_VERSION,
Expand Down
7 changes: 6 additions & 1 deletion ext/opcache/zend_persist.c
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,11 @@ static zend_op_array *zend_persist_class_method(zend_op_array *op_array, zend_cl
// 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.
// However, copies - as caused by inheritance of internal methods - must retain the original run_time_cache pointer, shared with the source function.
if (!op_array->scope || (op_array->scope == ce && !(op_array->fn_flags & ZEND_ACC_TRAIT_CLONE))) {
ZEND_MAP_PTR_NEW(op_array->run_time_cache);
if (op_array->fn_flags & ZEND_ACC_PRELOADED) {
ZEND_MAP_PTR_NEW_STATIC(op_array->run_time_cache);
} else {
ZEND_MAP_PTR_NEW(op_array->run_time_cache);
}
}
}
}
Expand Down Expand Up @@ -1413,6 +1417,7 @@ zend_persistent_script *zend_accel_script_persist(zend_persistent_script *script

if (for_shm) {
ZCSG(map_ptr_last) = CG(map_ptr_last);
ZCSG(map_ptr_static_last) = zend_map_ptr_static_last;
}

#ifdef HAVE_JIT
Expand Down

0 comments on commit c37d9f8

Please sign in to comment.