From 200193bc7f4877c954a7a211cb5bce5c8deed9d9 Mon Sep 17 00:00:00 2001 From: Samuel Melrose Date: Thu, 28 Nov 2024 12:06:15 +0000 Subject: [PATCH 01/15] opcache_is_script_cached: file_cache option --- ext/opcache/ZendAccelerator.c | 4 +-- ext/opcache/opcache.stub.php | 2 +- ext/opcache/opcache_arginfo.h | 7 ++-- ext/opcache/zend_accelerator_module.c | 46 ++++++++++++++++++++++++--- ext/opcache/zend_file_cache.c | 3 +- ext/opcache/zend_file_cache.h | 2 +- 6 files changed, 53 insertions(+), 11 deletions(-) diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index 3cc5097234254..ada5089ca7914 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -1916,7 +1916,7 @@ static zend_op_array *file_cache_compile_file(zend_file_handle *file_handle, int HANDLE_BLOCK_INTERRUPTIONS(); SHM_UNPROTECT(); - persistent_script = zend_file_cache_script_load(file_handle); + persistent_script = zend_file_cache_script_load(file_handle, false); SHM_PROTECT(); HANDLE_UNBLOCK_INTERRUPTIONS(); if (persistent_script) { @@ -2133,7 +2133,7 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type) /* Check the second level cache */ if (!persistent_script && ZCG(accel_directives).file_cache) { - persistent_script = zend_file_cache_script_load(file_handle); + persistent_script = zend_file_cache_script_load(file_handle, false); } /* If script was not found or invalidated by validate_timestamps */ diff --git a/ext/opcache/opcache.stub.php b/ext/opcache/opcache.stub.php index 526da238219a4..a509dff7ecac9 100644 --- a/ext/opcache/opcache.stub.php +++ b/ext/opcache/opcache.stub.php @@ -22,4 +22,4 @@ function opcache_jit_blacklist(Closure $closure): void {} */ function opcache_get_configuration(): array|false {} -function opcache_is_script_cached(string $filename): bool {} +function opcache_is_script_cached(string $filename, bool $file_cache = false): bool {} diff --git a/ext/opcache/opcache_arginfo.h b/ext/opcache/opcache_arginfo.h index b4dc1f33a5fd8..df4f3a4788486 100644 --- a/ext/opcache/opcache_arginfo.h +++ b/ext/opcache/opcache_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: c416c231c5d1270b7e5961f84cc3ca3e29db4959 */ + * Stub hash: efbc2d9c4b73ec6b63ada6ec2591fc035a24931c */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_opcache_reset, 0, 0, _IS_BOOL, 0) ZEND_END_ARG_INFO() @@ -24,7 +24,10 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_opcache_get_configuration, 0, 0, MAY_BE_ARRAY|MAY_BE_FALSE) ZEND_END_ARG_INFO() -#define arginfo_opcache_is_script_cached arginfo_opcache_compile_file +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_opcache_is_script_cached, 0, 1, _IS_BOOL, 0) + ZEND_ARG_TYPE_INFO(0, filename, IS_STRING, 0) + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, file_cache, _IS_BOOL, 0, "false") +ZEND_END_ARG_INFO() ZEND_FUNCTION(opcache_reset); ZEND_FUNCTION(opcache_get_status); diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 1b62b757b87d2..929a20267e5da 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -22,11 +22,13 @@ #include #include "php.h" +#include "zend.h" #include "ZendAccelerator.h" #include "zend_API.h" #include "zend_closures.h" #include "zend_shared_alloc.h" #include "zend_accelerator_blacklist.h" +#include "zend_file_cache.h" #include "php_ini.h" #include "SAPI.h" #include "zend_virtual_cwd.h" @@ -363,6 +365,37 @@ static int filename_is_in_cache(zend_string *filename) return 0; } +static int filename_is_in_file_cache(zend_string *filename) +{ + if (!ZCG(accel_directives).file_cache) { + return 0; + } + + zend_string *realpath; + + realpath = zend_resolve_path(filename); + + if (!realpath) { + return 0; + } + + zend_file_handle handle; + + zend_stream_init_filename_ex(&handle, filename); + handle.opened_path = realpath; + + zend_persistent_script *persistent_script = zend_file_cache_script_load(&handle, true); + + zend_destroy_file_handle(&handle); + + if (persistent_script) { + return 1; + } + + return 0; +} + + static int accel_file_in_cache(INTERNAL_FUNCTION_PARAMETERS) { if (ZEND_NUM_ARGS() == 1) { @@ -983,10 +1016,11 @@ ZEND_FUNCTION(opcache_compile_file) ZEND_FUNCTION(opcache_is_script_cached) { zend_string *script_name; + bool file_cache = 0; - ZEND_PARSE_PARAMETERS_START(1, 1) - Z_PARAM_STR(script_name) - ZEND_PARSE_PARAMETERS_END(); + if (zend_parse_parameters(ZEND_NUM_ARGS(), "S|b", &script_name, &file_cache) == FAILURE) { + RETURN_THROWS(); + } if (!validate_api_restriction()) { RETURN_FALSE; @@ -996,5 +1030,9 @@ ZEND_FUNCTION(opcache_is_script_cached) RETURN_FALSE; } - RETURN_BOOL(filename_is_in_cache(script_name)); + if (file_cache) { + RETURN_BOOL(filename_is_in_file_cache(script_name)); + } else { + RETURN_BOOL(filename_is_in_cache(script_name)); + } } diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index fe54c477cea77..74a643f1e42ba 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -1813,7 +1813,7 @@ static void zend_file_cache_unserialize(zend_persistent_script *script, zend_file_cache_unserialize_early_bindings(script, buf); } -zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handle) +zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handle, bool force_file_cache_only) { zend_string *full_path = file_handle->opened_path; int fd; @@ -1928,6 +1928,7 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl } if (!file_cache_only && + !force_file_cache_only && !ZCSG(restart_in_progress) && !ZCSG(restart_pending) && !ZSMMG(memory_exhausted) && diff --git a/ext/opcache/zend_file_cache.h b/ext/opcache/zend_file_cache.h index 8f067f5f37abb..30eb1a18b60e7 100644 --- a/ext/opcache/zend_file_cache.h +++ b/ext/opcache/zend_file_cache.h @@ -20,7 +20,7 @@ #define ZEND_FILE_CACHE_H int zend_file_cache_script_store(zend_persistent_script *script, bool in_shm); -zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handle); +zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handle, bool force_file_cache_only); void zend_file_cache_invalidate(zend_string *full_path); #endif /* ZEND_FILE_CACHE_H */ From 798a07d1a65316dc7425cf38a4aa4628bdc25e2c Mon Sep 17 00:00:00 2001 From: Samuel Melrose Date: Thu, 28 Nov 2024 13:26:01 +0000 Subject: [PATCH 02/15] opcache_is_script_cached: file_cache: test --- ext/opcache/tests/gt16979.phpt | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 ext/opcache/tests/gt16979.phpt diff --git a/ext/opcache/tests/gt16979.phpt b/ext/opcache/tests/gt16979.phpt new file mode 100644 index 0000000000000..fe70256fc5c7d --- /dev/null +++ b/ext/opcache/tests/gt16979.phpt @@ -0,0 +1,29 @@ +--TEST-- +GH-16979: opcache_is_script_cached support file_cache argument +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit=disable +opcache.jit_buffer_size=0 +opcache.file_cache={TMP} +opcache.file_cache_only=1 +opcache.log_verbosity_level=4 +--EXTENSIONS-- +opcache +--FILE-- + +--EXPECT-- +bool(false) +bool(true) From 824e638057036d8497b12b1931ff6ae8a1254341 Mon Sep 17 00:00:00 2001 From: Samuel Melrose Date: Thu, 28 Nov 2024 16:08:28 +0000 Subject: [PATCH 03/15] fix when file_cache_read_only --- ext/opcache/tests/gt16979.phpt | 1 - ext/opcache/zend_accelerator_module.c | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/opcache/tests/gt16979.phpt b/ext/opcache/tests/gt16979.phpt index fe70256fc5c7d..e9a6df2637dab 100644 --- a/ext/opcache/tests/gt16979.phpt +++ b/ext/opcache/tests/gt16979.phpt @@ -7,7 +7,6 @@ opcache.jit=disable opcache.jit_buffer_size=0 opcache.file_cache={TMP} opcache.file_cache_only=1 -opcache.log_verbosity_level=4 --EXTENSIONS-- opcache --FILE-- diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 929a20267e5da..47a284abcb1f3 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -1026,8 +1026,9 @@ ZEND_FUNCTION(opcache_is_script_cached) RETURN_FALSE; } - if (!ZCG(accelerator_enabled)) { - RETURN_FALSE; + if (!ZCG(accelerator_enabled) && + !(file_cache && ZCG(accel_directives).file_cache_read_only)) { + RETURN_FALSE; } if (file_cache) { From 30641d68f88609f30d04c2f932fe7c7acb088161 Mon Sep 17 00:00:00 2001 From: Samuel Melrose Date: Thu, 28 Nov 2024 16:27:53 +0000 Subject: [PATCH 04/15] remove file_cache_only --- ext/opcache/tests/gt16979.phpt | 3 +-- ext/opcache/zend_accelerator_module.c | 5 ++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/ext/opcache/tests/gt16979.phpt b/ext/opcache/tests/gt16979.phpt index e9a6df2637dab..de36ef5a29b1d 100644 --- a/ext/opcache/tests/gt16979.phpt +++ b/ext/opcache/tests/gt16979.phpt @@ -6,7 +6,6 @@ opcache.enable_cli=1 opcache.jit=disable opcache.jit_buffer_size=0 opcache.file_cache={TMP} -opcache.file_cache_only=1 --EXTENSIONS-- opcache --FILE-- @@ -24,5 +23,5 @@ var_dump( ?> --EXPECT-- -bool(false) +bool(true) bool(true) diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 47a284abcb1f3..929a20267e5da 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -1026,9 +1026,8 @@ ZEND_FUNCTION(opcache_is_script_cached) RETURN_FALSE; } - if (!ZCG(accelerator_enabled) && - !(file_cache && ZCG(accel_directives).file_cache_read_only)) { - RETURN_FALSE; + if (!ZCG(accelerator_enabled)) { + RETURN_FALSE; } if (file_cache) { From 283844ec2d399cd7826563b30a815f84ec185ab6 Mon Sep 17 00:00:00 2001 From: Samuel Melrose Date: Fri, 29 Nov 2024 15:10:20 +0000 Subject: [PATCH 05/15] switch to zend_file_cache_script_validate(...) --- ext/opcache/ZendAccelerator.c | 4 +- ext/opcache/zend_accelerator_module.c | 16 +++-- ext/opcache/zend_file_cache.c | 91 ++++++++++++++++++++++++++- ext/opcache/zend_file_cache.h | 3 +- 4 files changed, 100 insertions(+), 14 deletions(-) diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index ada5089ca7914..3cc5097234254 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -1916,7 +1916,7 @@ static zend_op_array *file_cache_compile_file(zend_file_handle *file_handle, int HANDLE_BLOCK_INTERRUPTIONS(); SHM_UNPROTECT(); - persistent_script = zend_file_cache_script_load(file_handle, false); + persistent_script = zend_file_cache_script_load(file_handle); SHM_PROTECT(); HANDLE_UNBLOCK_INTERRUPTIONS(); if (persistent_script) { @@ -2133,7 +2133,7 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type) /* Check the second level cache */ if (!persistent_script && ZCG(accel_directives).file_cache) { - persistent_script = zend_file_cache_script_load(file_handle, false); + persistent_script = zend_file_cache_script_load(file_handle); } /* If script was not found or invalidated by validate_timestamps */ diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 929a20267e5da..1f86b87107e8e 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -384,15 +384,11 @@ static int filename_is_in_file_cache(zend_string *filename) zend_stream_init_filename_ex(&handle, filename); handle.opened_path = realpath; - zend_persistent_script *persistent_script = zend_file_cache_script_load(&handle, true); + bool result = zend_file_cache_script_validate(&handle); zend_destroy_file_handle(&handle); - if (persistent_script) { - return 1; - } - - return 0; + return result; } @@ -1018,9 +1014,11 @@ ZEND_FUNCTION(opcache_is_script_cached) zend_string *script_name; bool file_cache = 0; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "S|b", &script_name, &file_cache) == FAILURE) { - RETURN_THROWS(); - } + ZEND_PARSE_PARAMETERS_START(1, 2) + Z_PARAM_STR(script_name) + Z_PARAM_OPTIONAL + Z_PARAM_BOOL(file_cache) + ZEND_PARSE_PARAMETERS_END(); if (!validate_api_restriction()) { RETURN_FALSE; diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index 74a643f1e42ba..7ab9829dc5bcd 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -1813,7 +1813,95 @@ static void zend_file_cache_unserialize(zend_persistent_script *script, zend_file_cache_unserialize_early_bindings(script, buf); } -zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handle, bool force_file_cache_only) +// Function to validate the file cache and return a boolean result +bool zend_file_cache_script_validate(zend_file_handle *file_handle) +{ + int fd; + char *filename; + zend_file_cache_metainfo info; + unsigned int actual_checksum; + void *mem; + + if (!file_handle || !file_handle->opened_path) { + return false; + } + + filename = zend_file_cache_get_bin_file_path(file_handle->opened_path); + fd = zend_file_cache_open(filename, O_RDONLY | O_BINARY); + if (fd < 0) { + goto failure; // Open failed + } + + if (zend_file_cache_flock(fd, LOCK_SH) != 0) { + goto failure_close; // Flock failed + } + + + if (read(fd, &info, sizeof(info)) != sizeof(info)) { + zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot read from file '%s' (info)\n", filename); + goto failure_unlock_close; + } + + // Verify header + if (memcmp(info.magic, "OPCACHE", 8) != 0 || memcmp(info.system_id, zend_system_id, 32) != 0) { + zend_accel_error(ACCEL_LOG_WARNING, "opcache invalid header in file '%s'\n", filename); + goto failure_unlock_close; + } + + // Verify timestamp if required + if (ZCG(accel_directives).validate_timestamps && + zend_get_file_handle_timestamp(file_handle, NULL) != info.timestamp) { + goto failure_unlock_close; + } + + checkpoint = zend_arena_checkpoint(CG(arena)); +#if defined(__AVX__) || defined(__SSE2__) + /* Align to 64-byte boundary */ + mem = zend_arena_alloc(&CG(arena), info.mem_size + info.str_size + 64); + mem = (void*)(((uintptr_t)mem + 63L) & ~63L); +#else + mem = zend_arena_alloc(&CG(arena), info.mem_size + info.str_size); +#endif + + if (read(fd, mem, info.mem_size + info.str_size) != (ssize_t)(info.mem_size + info.str_size)) { + zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot read from file '%s' (mem)\n", filename); + zend_arena_release(&CG(arena), checkpoint); + goto failure_unlock_close; + } + if (zend_file_cache_flock(fd, LOCK_UN) != 0) { + zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot unlock file '%s'\n", filename); + } + close(fd); + + /* verify checksum */ + if (ZCG(accel_directives).file_cache_consistency_checks && + (actual_checksum = zend_adler32(ADLER32_INIT, mem, info.mem_size + info.str_size)) != info.checksum) { + zend_accel_error(ACCEL_LOG_WARNING, "corrupted file '%s' excepted checksum: 0x%08x actual checksum: 0x%08x\n", filename, info.checksum, actual_checksum); + zend_arena_release(&CG(arena), checkpoint); + goto failure; + } + + zend_arena_release(&CG(arena), checkpoint); + zend_file_cache_flock(fd, LOCK_UN); + close(fd); + efree(filename); + return true; // Validation successful + +failure_unlock_close: + zend_file_cache_flock(fd, LOCK_UN); +failure_close: + close(fd); +failure: + if (!ZCG(accel_directives).file_cache_read_only) { + zend_file_cache_unlink(filename); + } + efree(filename); + + return false; // Validation failed +} + + +zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handle) { zend_string *full_path = file_handle->opened_path; int fd; @@ -1928,7 +2016,6 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl } if (!file_cache_only && - !force_file_cache_only && !ZCSG(restart_in_progress) && !ZCSG(restart_pending) && !ZSMMG(memory_exhausted) && diff --git a/ext/opcache/zend_file_cache.h b/ext/opcache/zend_file_cache.h index 30eb1a18b60e7..73bae7d4e8d8f 100644 --- a/ext/opcache/zend_file_cache.h +++ b/ext/opcache/zend_file_cache.h @@ -20,7 +20,8 @@ #define ZEND_FILE_CACHE_H int zend_file_cache_script_store(zend_persistent_script *script, bool in_shm); -zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handle, bool force_file_cache_only); +bool zend_file_cache_script_validate(zend_file_handle *file_handle); +zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handle); void zend_file_cache_invalidate(zend_string *full_path); #endif /* ZEND_FILE_CACHE_H */ From 8cc7293116e1310c1e6db321556ea1fcc8b3d24c Mon Sep 17 00:00:00 2001 From: Samuel Melrose Date: Mon, 2 Dec 2024 10:43:11 +0000 Subject: [PATCH 06/15] fix build --- ext/opcache/tests/gt16979.phpt | 2 +- ext/opcache/zend_file_cache.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/opcache/tests/gt16979.phpt b/ext/opcache/tests/gt16979.phpt index de36ef5a29b1d..c38223321718a 100644 --- a/ext/opcache/tests/gt16979.phpt +++ b/ext/opcache/tests/gt16979.phpt @@ -5,7 +5,7 @@ opcache.enable=1 opcache.enable_cli=1 opcache.jit=disable opcache.jit_buffer_size=0 -opcache.file_cache={TMP} +opcache.file_cache="{TMP}" --EXTENSIONS-- opcache --FILE-- diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index 7ab9829dc5bcd..b813594833721 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -1820,7 +1820,7 @@ bool zend_file_cache_script_validate(zend_file_handle *file_handle) char *filename; zend_file_cache_metainfo info; unsigned int actual_checksum; - void *mem; + void *mem, *checkpoint; if (!file_handle || !file_handle->opened_path) { return false; From fd93557931ed225b777e73f2cf7ae0b6e87e3ec4 Mon Sep 17 00:00:00 2001 From: Samuel Melrose Date: Mon, 2 Dec 2024 15:39:03 +0000 Subject: [PATCH 07/15] switch to opcache_is_script_cached_in_file_cache(...) --- ext/opcache/opcache.stub.php | 4 +++- ext/opcache/opcache_arginfo.h | 11 +++++---- ext/opcache/tests/gt16979.phpt | 4 ++-- ext/opcache/zend_accelerator_module.c | 33 +++++++++++++++++---------- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/ext/opcache/opcache.stub.php b/ext/opcache/opcache.stub.php index a509dff7ecac9..32673bb1dcee8 100644 --- a/ext/opcache/opcache.stub.php +++ b/ext/opcache/opcache.stub.php @@ -22,4 +22,6 @@ function opcache_jit_blacklist(Closure $closure): void {} */ function opcache_get_configuration(): array|false {} -function opcache_is_script_cached(string $filename, bool $file_cache = false): bool {} +function opcache_is_script_cached(string $filename): bool {} + +function opcache_is_script_cached_in_file_cache(string $filename): bool {} diff --git a/ext/opcache/opcache_arginfo.h b/ext/opcache/opcache_arginfo.h index df4f3a4788486..7fff6b1eb0da9 100644 --- a/ext/opcache/opcache_arginfo.h +++ b/ext/opcache/opcache_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: efbc2d9c4b73ec6b63ada6ec2591fc035a24931c */ + * Stub hash: a8de025fa96a78db3a26d53a18bb2b365d094eca */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_opcache_reset, 0, 0, _IS_BOOL, 0) ZEND_END_ARG_INFO() @@ -24,10 +24,9 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_opcache_get_configuration, 0, 0, MAY_BE_ARRAY|MAY_BE_FALSE) ZEND_END_ARG_INFO() -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_opcache_is_script_cached, 0, 1, _IS_BOOL, 0) - ZEND_ARG_TYPE_INFO(0, filename, IS_STRING, 0) - ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, file_cache, _IS_BOOL, 0, "false") -ZEND_END_ARG_INFO() +#define arginfo_opcache_is_script_cached arginfo_opcache_compile_file + +#define arginfo_opcache_is_script_cached_in_file_cache arginfo_opcache_compile_file ZEND_FUNCTION(opcache_reset); ZEND_FUNCTION(opcache_get_status); @@ -36,6 +35,7 @@ ZEND_FUNCTION(opcache_invalidate); ZEND_FUNCTION(opcache_jit_blacklist); ZEND_FUNCTION(opcache_get_configuration); ZEND_FUNCTION(opcache_is_script_cached); +ZEND_FUNCTION(opcache_is_script_cached_in_file_cache); static const zend_function_entry ext_functions[] = { ZEND_FE(opcache_reset, arginfo_opcache_reset) @@ -45,5 +45,6 @@ static const zend_function_entry ext_functions[] = { ZEND_FE(opcache_jit_blacklist, arginfo_opcache_jit_blacklist) ZEND_FE(opcache_get_configuration, arginfo_opcache_get_configuration) ZEND_FE(opcache_is_script_cached, arginfo_opcache_is_script_cached) + ZEND_FE(opcache_is_script_cached_in_file_cache, arginfo_opcache_is_script_cached_in_file_cache) ZEND_FE_END }; diff --git a/ext/opcache/tests/gt16979.phpt b/ext/opcache/tests/gt16979.phpt index c38223321718a..31de0e74b2683 100644 --- a/ext/opcache/tests/gt16979.phpt +++ b/ext/opcache/tests/gt16979.phpt @@ -14,11 +14,11 @@ opcache opcache_compile_file(__FILE__); var_dump( - opcache_is_script_cached(__FILE__, false) + opcache_is_script_cached(__FILE__) ); var_dump( - opcache_is_script_cached(__FILE__, true) + opcache_is_script_cached_in_file_cache(__FILE__) ); ?> diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 1f86b87107e8e..14cc4ba092f13 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -367,10 +367,6 @@ static int filename_is_in_cache(zend_string *filename) static int filename_is_in_file_cache(zend_string *filename) { - if (!ZCG(accel_directives).file_cache) { - return 0; - } - zend_string *realpath; realpath = zend_resolve_path(filename); @@ -1012,12 +1008,9 @@ ZEND_FUNCTION(opcache_compile_file) ZEND_FUNCTION(opcache_is_script_cached) { zend_string *script_name; - bool file_cache = 0; - ZEND_PARSE_PARAMETERS_START(1, 2) + ZEND_PARSE_PARAMETERS_START(1, 1) Z_PARAM_STR(script_name) - Z_PARAM_OPTIONAL - Z_PARAM_BOOL(file_cache) ZEND_PARSE_PARAMETERS_END(); if (!validate_api_restriction()) { @@ -1028,9 +1021,25 @@ ZEND_FUNCTION(opcache_is_script_cached) RETURN_FALSE; } - if (file_cache) { - RETURN_BOOL(filename_is_in_file_cache(script_name)); - } else { - RETURN_BOOL(filename_is_in_cache(script_name)); + RETURN_BOOL(filename_is_in_cache(script_name)); +} + +/* {{{ Return true if the script is cached in OPCache file cache, false if it is not cached or if OPCache is not running. */ +ZEND_FUNCTION(opcache_is_script_cached_in_file_cache) +{ + zend_string *script_name; + + ZEND_PARSE_PARAMETERS_START(1, 1) + Z_PARAM_STR(script_name) + ZEND_PARSE_PARAMETERS_END(); + + if (!validate_api_restriction()) { + RETURN_FALSE; } + + if (!ZCG(accelerator_enabled) || !ZCG(accel_directives).file_cache) { + RETURN_FALSE; + } + + RETURN_BOOL(filename_is_in_file_cache(script_name)); } From 7e23285cbaeb8b9cca0133da481222ac5978486d Mon Sep 17 00:00:00 2001 From: Samuel Melrose Date: Mon, 2 Dec 2024 16:04:58 +0000 Subject: [PATCH 08/15] remove double close(fd) --- ext/opcache/zend_file_cache.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index b813594833721..1a39d8607656d 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -1882,8 +1882,6 @@ bool zend_file_cache_script_validate(zend_file_handle *file_handle) } zend_arena_release(&CG(arena), checkpoint); - zend_file_cache_flock(fd, LOCK_UN); - close(fd); efree(filename); return true; // Validation successful @@ -1900,7 +1898,6 @@ bool zend_file_cache_script_validate(zend_file_handle *file_handle) return false; // Validation failed } - zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handle) { zend_string *full_path = file_handle->opened_path; From 906b92d05dc5ca891469a405da8f7fa5ac91049b Mon Sep 17 00:00:00 2001 From: Samuel Melrose Date: Mon, 2 Dec 2024 17:02:42 +0000 Subject: [PATCH 09/15] zend_file_cache_open with struct --- ext/opcache/zend_accelerator_module.c | 4 +- ext/opcache/zend_file_cache.c | 255 +++++++++----------------- ext/opcache/zend_file_cache.h | 10 +- 3 files changed, 94 insertions(+), 175 deletions(-) diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 14cc4ba092f13..3cc1be077716a 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -380,11 +380,11 @@ static int filename_is_in_file_cache(zend_string *filename) zend_stream_init_filename_ex(&handle, filename); handle.opened_path = realpath; - bool result = zend_file_cache_script_validate(&handle); + zend_result result = zend_file_cache_script_validate(&handle); zend_destroy_file_handle(&handle); - return result; + return result == SUCCESS; } diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index 1a39d8607656d..64e9baffdd8c8 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -1813,187 +1813,58 @@ static void zend_file_cache_unserialize(zend_persistent_script *script, zend_file_cache_unserialize_early_bindings(script, buf); } -// Function to validate the file cache and return a boolean result -bool zend_file_cache_script_validate(zend_file_handle *file_handle) +zend_always_inline zend_result zend_file_cache_open(zend_file_handle *file_handle, zend_file_cache_handle *cache_handle) { - int fd; - char *filename; - zend_file_cache_metainfo info; - unsigned int actual_checksum; - void *mem, *checkpoint; - - if (!file_handle || !file_handle->opened_path) { - return false; - } - - filename = zend_file_cache_get_bin_file_path(file_handle->opened_path); - fd = zend_file_cache_open(filename, O_RDONLY | O_BINARY); - if (fd < 0) { - goto failure; // Open failed - } - - if (zend_file_cache_flock(fd, LOCK_SH) != 0) { - goto failure_close; // Flock failed - } - - - if (read(fd, &info, sizeof(info)) != sizeof(info)) { - zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot read from file '%s' (info)\n", filename); - goto failure_unlock_close; - } - - // Verify header - if (memcmp(info.magic, "OPCACHE", 8) != 0 || memcmp(info.system_id, zend_system_id, 32) != 0) { - zend_accel_error(ACCEL_LOG_WARNING, "opcache invalid header in file '%s'\n", filename); - goto failure_unlock_close; - } - - // Verify timestamp if required - if (ZCG(accel_directives).validate_timestamps && - zend_get_file_handle_timestamp(file_handle, NULL) != info.timestamp) { - goto failure_unlock_close; - } - - checkpoint = zend_arena_checkpoint(CG(arena)); -#if defined(__AVX__) || defined(__SSE2__) - /* Align to 64-byte boundary */ - mem = zend_arena_alloc(&CG(arena), info.mem_size + info.str_size + 64); - mem = (void*)(((uintptr_t)mem + 63L) & ~63L); -#else - mem = zend_arena_alloc(&CG(arena), info.mem_size + info.str_size); -#endif - - if (read(fd, mem, info.mem_size + info.str_size) != (ssize_t)(info.mem_size + info.str_size)) { - zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot read from file '%s' (mem)\n", filename); - zend_arena_release(&CG(arena), checkpoint); - goto failure_unlock_close; - } - if (zend_file_cache_flock(fd, LOCK_UN) != 0) { - zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot unlock file '%s'\n", filename); - } - close(fd); - - /* verify checksum */ - if (ZCG(accel_directives).file_cache_consistency_checks && - (actual_checksum = zend_adler32(ADLER32_INIT, mem, info.mem_size + info.str_size)) != info.checksum) { - zend_accel_error(ACCEL_LOG_WARNING, "corrupted file '%s' excepted checksum: 0x%08x actual checksum: 0x%08x\n", filename, info.checksum, actual_checksum); - zend_arena_release(&CG(arena), checkpoint); - goto failure; - } - - zend_arena_release(&CG(arena), checkpoint); - efree(filename); - return true; // Validation successful - -failure_unlock_close: - zend_file_cache_flock(fd, LOCK_UN); -failure_close: - close(fd); -failure: - if (!ZCG(accel_directives).file_cache_read_only) { - zend_file_cache_unlink(filename); - } - efree(filename); - - return false; // Validation failed -} - -zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handle) -{ - zend_string *full_path = file_handle->opened_path; int fd; char *filename; - zend_persistent_script *script; - zend_file_cache_metainfo info; - zend_accel_hash_entry *bucket; - void *mem, *checkpoint, *buf; - bool cache_it = true; unsigned int actual_checksum; - bool ok; - if (!full_path) { - return NULL; + if (!file_handle || !file_handle->opened_path) { + return FAILURE; } - filename = zend_file_cache_get_bin_file_path(full_path); + zend_file_cache_handle->full_path = file_handle->opened_path + filename = zend_file_cache_get_bin_file_path(zend_file_cache_handle->full_path); fd = zend_file_cache_open(filename, O_RDONLY | O_BINARY); if (fd < 0) { - efree(filename); - return NULL; + goto failure; // Open failed } if (zend_file_cache_flock(fd, LOCK_SH) != 0) { - close(fd); - efree(filename); - return NULL; + goto failure_close; // Flock failed } - if (read(fd, &info, sizeof(info)) != sizeof(info)) { + + if (read(fd, &zend_file_cache_handle->info, sizeof(zend_file_cache_handle->info)) != sizeof(zend_file_cache_handle->info)) { zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot read from file '%s' (info)\n", filename); - zend_file_cache_flock(fd, LOCK_UN); - close(fd); - if (!ZCG(accel_directives).file_cache_read_only) { - zend_file_cache_unlink(filename); - } - efree(filename); - return NULL; + goto failure_unlock_close; } - /* verify header */ - if (memcmp(info.magic, "OPCACHE", 8) != 0) { - zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot read from file '%s' (wrong header)\n", filename); - zend_file_cache_flock(fd, LOCK_UN); - close(fd); - if (!ZCG(accel_directives).file_cache_read_only) { - zend_file_cache_unlink(filename); - } - efree(filename); - return NULL; - } - if (memcmp(info.system_id, zend_system_id, 32) != 0) { - zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot read from file '%s' (wrong \"system_id\")\n", filename); - zend_file_cache_flock(fd, LOCK_UN); - close(fd); - if (!ZCG(accel_directives).file_cache_read_only) { - zend_file_cache_unlink(filename); - } - efree(filename); - return NULL; + // Verify header + if (memcmp(zend_file_cache_handle->info.magic, "OPCACHE", 8) != 0 || memcmp(zend_file_cache_handle->info.system_id, zend_system_id, 32) != 0) { + zend_accel_error(ACCEL_LOG_WARNING, "opcache invalid header in file '%s'\n", filename); + goto failure_unlock_close; } - /* verify timestamp */ + // Verify timestamp if required if (ZCG(accel_directives).validate_timestamps && - zend_get_file_handle_timestamp(file_handle, NULL) != info.timestamp) { - if (zend_file_cache_flock(fd, LOCK_UN) != 0) { - zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot unlock file '%s'\n", filename); - } - close(fd); - if (!ZCG(accel_directives).file_cache_read_only) { - zend_file_cache_unlink(filename); - } - efree(filename); - return NULL; + zend_get_file_handle_timestamp(file_handle, NULL) != zend_file_cache_handle->info.timestamp) { + goto failure_unlock_close; } - checkpoint = zend_arena_checkpoint(CG(arena)); + zend_file_cache_handle->checkpoint = zend_arena_checkpoint(CG(arena)); #if defined(__AVX__) || defined(__SSE2__) /* Align to 64-byte boundary */ - mem = zend_arena_alloc(&CG(arena), info.mem_size + info.str_size + 64); - mem = (void*)(((uintptr_t)mem + 63L) & ~63L); + zend_file_cache_handle->mem = zend_arena_alloc(&CG(arena), zend_file_cache_handle->info.mem_size + zend_file_cache_handle->info.str_size + 64); + zend_file_cache_handle->mem = (void*)(((uintptr_t)zend_file_cache_handle->mem + 63L) & ~63L); #else - mem = zend_arena_alloc(&CG(arena), info.mem_size + info.str_size); + zend_file_cache_handle->mem = zend_arena_alloc(&CG(arena), zend_file_cache_handle->info.mem_size + zend_file_cache_handle->info.str_size); #endif - if (read(fd, mem, info.mem_size + info.str_size) != (ssize_t)(info.mem_size + info.str_size)) { + if (read(fd, zend_file_cache_handle->mem, zend_file_cache_handle->info.mem_size + zend_file_cache_handle->info.str_size) != (ssize_t)(zend_file_cache_handle->info.mem_size + zend_file_cache_handle->info.str_size)) { zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot read from file '%s' (mem)\n", filename); - zend_file_cache_flock(fd, LOCK_UN); - close(fd); - if (!ZCG(accel_directives).file_cache_read_only) { - zend_file_cache_unlink(filename); - } - zend_arena_release(&CG(arena), checkpoint); - efree(filename); - return NULL; + zend_arena_release(&CG(arena), zend_file_cache_handle->checkpoint); + goto failure_unlock_close; } if (zend_file_cache_flock(fd, LOCK_UN) != 0) { zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot unlock file '%s'\n", filename); @@ -2002,13 +1873,56 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl /* verify checksum */ if (ZCG(accel_directives).file_cache_consistency_checks && - (actual_checksum = zend_adler32(ADLER32_INIT, mem, info.mem_size + info.str_size)) != info.checksum) { - zend_accel_error(ACCEL_LOG_WARNING, "corrupted file '%s' excepted checksum: 0x%08x actual checksum: 0x%08x\n", filename, info.checksum, actual_checksum); - if (!ZCG(accel_directives).file_cache_read_only) { - zend_file_cache_unlink(filename); - } - zend_arena_release(&CG(arena), checkpoint); - efree(filename); + (actual_checksum = zend_adler32(ADLER32_INIT, zend_file_cache_handle->mem, zend_file_cache_handle->info.mem_size + zend_file_cache_handle->info.str_size)) != zend_file_cache_handle->info.checksum) { + zend_accel_error(ACCEL_LOG_WARNING, "corrupted file '%s' excepted checksum: 0x%08x actual checksum: 0x%08x\n", filename, zend_file_cache_handle->info.checksum, actual_checksum); + zend_arena_release(&CG(arena), zend_file_cache_handle->checkpoint); + goto failure; + } + + efree(filename); + return SUCCESS; + +failure_unlock_close: + zend_file_cache_flock(fd, LOCK_UN); +failure_close: + close(fd); +failure: + if (!ZCG(accel_directives).file_cache_read_only) { + zend_file_cache_unlink(filename); + } + efree(filename); + + return FAILURE; +} + +zend_always_inline void zend_file_cache_close(zend_file_cache_handle *cache_handle) +{ + zend_arena_release(&CG(arena), zend_file_cache_handle->checkpoint); +} + +zend_result zend_file_cache_validate(zend_file_handle *file_handle) +{ + zend_file_cache_handle cache_handle; + + zend_result ret = zend_file_cache_open(file_handle, &cache_handle); + + if (res == SUCCESS) { + zend_file_cache_close(&cache_handle); + } + + return ret; +} + +zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handle) +{ + zend_file_cache_handle cache_handle; + zend_persistent_script *script; + zend_accel_hash_entry *bucket; + void *buf; + bool cache_it = true; + bool ok; + + if (zend_file_cache_open(file_handle, &cache_handle) == FAILURE) { return NULL; } @@ -2023,13 +1937,12 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl /* Check if we still need to put the file into the cache (may be it was * already stored by another process. This final check is done under * exclusive lock) */ - bucket = zend_accel_hash_find_entry(&ZCSG(hash), full_path); + bucket = zend_accel_hash_find_entry(&ZCSG(hash), cache_handle->full_path); if (bucket) { script = (zend_persistent_script *)bucket->data; if (!script->corrupted) { zend_shared_alloc_unlock(); - zend_arena_release(&CG(arena), checkpoint); - efree(filename); + zend_file_cache_close(&cache_handle); return script; } } @@ -2042,23 +1955,23 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl goto use_process_mem; } - buf = zend_shared_alloc_aligned(info.mem_size); + buf = zend_shared_alloc_aligned(cache_handle->info.mem_size); if (!buf) { zend_accel_schedule_restart_if_necessary(ACCEL_RESTART_OOM); zend_shared_alloc_unlock(); goto use_process_mem; } - memcpy(buf, mem, info.mem_size); + memcpy(buf, cache_handle->mem, cache_handle->info.mem_size); zend_map_ptr_extend(ZCSG(map_ptr_last)); } else { use_process_mem: - buf = mem; + buf = cache_handle->mem; cache_it = false; } - ZCG(mem) = ((char*)mem + info.mem_size); - script = (zend_persistent_script*)((char*)buf + info.script_offset); + ZCG(cache_handle->mem) = ((char*)cache_handle->mem + cache_handle->info.mem_size); + script = (zend_persistent_script*)((char*)buf + cache_handle->info.script_offset); script->corrupted = !cache_it; /* used to check if script restored to SHM or process memory */ ok = true; @@ -2072,8 +1985,7 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl zend_shared_alloc_unlock(); goto use_process_mem; } else { - zend_arena_release(&CG(arena), checkpoint); - efree(filename); + zend_file_cache_close(&cache_handle); return NULL; } } @@ -2089,9 +2001,8 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl zend_shared_alloc_unlock(); zend_accel_error(ACCEL_LOG_INFO, "File cached script loaded into memory '%s'", ZSTR_VAL(script->script.filename)); - zend_arena_release(&CG(arena), checkpoint); + zend_file_cache_close(&cache_handle); } - efree(filename); return script; } diff --git a/ext/opcache/zend_file_cache.h b/ext/opcache/zend_file_cache.h index 73bae7d4e8d8f..217b69ff67306 100644 --- a/ext/opcache/zend_file_cache.h +++ b/ext/opcache/zend_file_cache.h @@ -19,8 +19,16 @@ #ifndef ZEND_FILE_CACHE_H #define ZEND_FILE_CACHE_H +typedef struct _zend_file_cache_handle { + zend_string *full_path; + zend_file_cache_metainfo info; + void *mem, *checkpoint; +} zend_file_cache_handle; + int zend_file_cache_script_store(zend_persistent_script *script, bool in_shm); -bool zend_file_cache_script_validate(zend_file_handle *file_handle); +zend_always_inline zend_result zend_file_cache_open(zend_file_handle *file_handle, zend_file_cache_handle *cache_handle); +zend_always_inline void zend_file_cache_close(zend_file_cache_handle *cache_handle); +zend_result zend_file_cache_validate(zend_file_handle *file_handle); zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handle); void zend_file_cache_invalidate(zend_string *full_path); From 1cdf3fa1dafad00079b9ea10b20e1cbb1f3ea15f Mon Sep 17 00:00:00 2001 From: Samuel Melrose Date: Mon, 2 Dec 2024 17:10:10 +0000 Subject: [PATCH 10/15] fix build --- ext/opcache/zend_accelerator_module.c | 2 +- ext/opcache/zend_file_cache.c | 58 +++++++++++++++------------ ext/opcache/zend_file_cache.h | 8 ---- 3 files changed, 33 insertions(+), 35 deletions(-) diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 3cc1be077716a..2e158229957e1 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -380,7 +380,7 @@ static int filename_is_in_file_cache(zend_string *filename) zend_stream_init_filename_ex(&handle, filename); handle.opened_path = realpath; - zend_result result = zend_file_cache_script_validate(&handle); + zend_result result = zend_file_cache_validate(&handle); zend_destroy_file_handle(&handle); diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index 64e9baffdd8c8..9301bdc6c5429 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -202,6 +202,12 @@ typedef struct _zend_file_cache_metainfo { uint32_t checksum; } zend_file_cache_metainfo; +typedef struct _zend_file_cache_handle { + zend_string *full_path; + zend_file_cache_metainfo info; + void *mem, *checkpoint; +} zend_file_cache_handle; + static int zend_file_cache_mkdir(char *filename, size_t start) { char *s = filename + start; @@ -1813,7 +1819,7 @@ static void zend_file_cache_unserialize(zend_persistent_script *script, zend_file_cache_unserialize_early_bindings(script, buf); } -zend_always_inline zend_result zend_file_cache_open(zend_file_handle *file_handle, zend_file_cache_handle *cache_handle) +zend_result zend_file_cache_validate_and_open(zend_file_handle *file_handle, zend_file_cache_handle *cache_handle) { int fd; char *filename; @@ -1822,9 +1828,9 @@ zend_always_inline zend_result zend_file_cache_open(zend_file_handle *file_handl if (!file_handle || !file_handle->opened_path) { return FAILURE; } - zend_file_cache_handle->full_path = file_handle->opened_path + cache_handle->full_path = file_handle->opened_path; - filename = zend_file_cache_get_bin_file_path(zend_file_cache_handle->full_path); + filename = zend_file_cache_get_bin_file_path(cache_handle->full_path); fd = zend_file_cache_open(filename, O_RDONLY | O_BINARY); if (fd < 0) { goto failure; // Open failed @@ -1835,35 +1841,35 @@ zend_always_inline zend_result zend_file_cache_open(zend_file_handle *file_handl } - if (read(fd, &zend_file_cache_handle->info, sizeof(zend_file_cache_handle->info)) != sizeof(zend_file_cache_handle->info)) { + if (read(fd, &cache_handle->info, sizeof(cache_handle->info)) != sizeof(cache_handle->info)) { zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot read from file '%s' (info)\n", filename); goto failure_unlock_close; } // Verify header - if (memcmp(zend_file_cache_handle->info.magic, "OPCACHE", 8) != 0 || memcmp(zend_file_cache_handle->info.system_id, zend_system_id, 32) != 0) { + if (memcmp(cache_handle->info.magic, "OPCACHE", 8) != 0 || memcmp(cache_handle->info.system_id, zend_system_id, 32) != 0) { zend_accel_error(ACCEL_LOG_WARNING, "opcache invalid header in file '%s'\n", filename); goto failure_unlock_close; } // Verify timestamp if required if (ZCG(accel_directives).validate_timestamps && - zend_get_file_handle_timestamp(file_handle, NULL) != zend_file_cache_handle->info.timestamp) { + zend_get_file_handle_timestamp(file_handle, NULL) != cache_handle->info.timestamp) { goto failure_unlock_close; } - zend_file_cache_handle->checkpoint = zend_arena_checkpoint(CG(arena)); + cache_handle->checkpoint = zend_arena_checkpoint(CG(arena)); #if defined(__AVX__) || defined(__SSE2__) /* Align to 64-byte boundary */ - zend_file_cache_handle->mem = zend_arena_alloc(&CG(arena), zend_file_cache_handle->info.mem_size + zend_file_cache_handle->info.str_size + 64); - zend_file_cache_handle->mem = (void*)(((uintptr_t)zend_file_cache_handle->mem + 63L) & ~63L); + cache_handle->mem = zend_arena_alloc(&CG(arena), cache_handle->info.mem_size + cache_handle->info.str_size + 64); + cache_handle->mem = (void*)(((uintptr_t)cache_handle->mem + 63L) & ~63L); #else - zend_file_cache_handle->mem = zend_arena_alloc(&CG(arena), zend_file_cache_handle->info.mem_size + zend_file_cache_handle->info.str_size); + cache_handle->mem = zend_arena_alloc(&CG(arena), cache_handle->info.mem_size + cache_handle->info.str_size); #endif - if (read(fd, zend_file_cache_handle->mem, zend_file_cache_handle->info.mem_size + zend_file_cache_handle->info.str_size) != (ssize_t)(zend_file_cache_handle->info.mem_size + zend_file_cache_handle->info.str_size)) { + if (read(fd, cache_handle->mem, cache_handle->info.mem_size + cache_handle->info.str_size) != (ssize_t)(cache_handle->info.mem_size + cache_handle->info.str_size)) { zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot read from file '%s' (mem)\n", filename); - zend_arena_release(&CG(arena), zend_file_cache_handle->checkpoint); + zend_arena_release(&CG(arena), cache_handle->checkpoint); goto failure_unlock_close; } if (zend_file_cache_flock(fd, LOCK_UN) != 0) { @@ -1873,9 +1879,9 @@ zend_always_inline zend_result zend_file_cache_open(zend_file_handle *file_handl /* verify checksum */ if (ZCG(accel_directives).file_cache_consistency_checks && - (actual_checksum = zend_adler32(ADLER32_INIT, zend_file_cache_handle->mem, zend_file_cache_handle->info.mem_size + zend_file_cache_handle->info.str_size)) != zend_file_cache_handle->info.checksum) { - zend_accel_error(ACCEL_LOG_WARNING, "corrupted file '%s' excepted checksum: 0x%08x actual checksum: 0x%08x\n", filename, zend_file_cache_handle->info.checksum, actual_checksum); - zend_arena_release(&CG(arena), zend_file_cache_handle->checkpoint); + (actual_checksum = zend_adler32(ADLER32_INIT, cache_handle->mem, cache_handle->info.mem_size + cache_handle->info.str_size)) != cache_handle->info.checksum) { + zend_accel_error(ACCEL_LOG_WARNING, "corrupted file '%s' excepted checksum: 0x%08x actual checksum: 0x%08x\n", filename, cache_handle->info.checksum, actual_checksum); + zend_arena_release(&CG(arena), cache_handle->checkpoint); goto failure; } @@ -1895,18 +1901,18 @@ zend_always_inline zend_result zend_file_cache_open(zend_file_handle *file_handl return FAILURE; } -zend_always_inline void zend_file_cache_close(zend_file_cache_handle *cache_handle) +void zend_file_cache_close(zend_file_cache_handle *cache_handle) { - zend_arena_release(&CG(arena), zend_file_cache_handle->checkpoint); + zend_arena_release(&CG(arena), cache_handle->checkpoint); } zend_result zend_file_cache_validate(zend_file_handle *file_handle) { zend_file_cache_handle cache_handle; - zend_result ret = zend_file_cache_open(file_handle, &cache_handle); + zend_result ret = zend_file_cache_validate_and_open(file_handle, &cache_handle); - if (res == SUCCESS) { + if (ret == SUCCESS) { zend_file_cache_close(&cache_handle); } @@ -1922,7 +1928,7 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl bool cache_it = true; bool ok; - if (zend_file_cache_open(file_handle, &cache_handle) == FAILURE) { + if (zend_file_cache_validate_and_open(file_handle, &cache_handle) == FAILURE) { return NULL; } @@ -1937,7 +1943,7 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl /* Check if we still need to put the file into the cache (may be it was * already stored by another process. This final check is done under * exclusive lock) */ - bucket = zend_accel_hash_find_entry(&ZCSG(hash), cache_handle->full_path); + bucket = zend_accel_hash_find_entry(&ZCSG(hash), cache_handle.full_path); if (bucket) { script = (zend_persistent_script *)bucket->data; if (!script->corrupted) { @@ -1955,23 +1961,23 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl goto use_process_mem; } - buf = zend_shared_alloc_aligned(cache_handle->info.mem_size); + buf = zend_shared_alloc_aligned(cache_handle.info.mem_size); if (!buf) { zend_accel_schedule_restart_if_necessary(ACCEL_RESTART_OOM); zend_shared_alloc_unlock(); goto use_process_mem; } - memcpy(buf, cache_handle->mem, cache_handle->info.mem_size); + memcpy(buf, cache_handle.mem, cache_handle.info.mem_size); zend_map_ptr_extend(ZCSG(map_ptr_last)); } else { use_process_mem: - buf = cache_handle->mem; + buf = cache_handle.mem; cache_it = false; } - ZCG(cache_handle->mem) = ((char*)cache_handle->mem + cache_handle->info.mem_size); - script = (zend_persistent_script*)((char*)buf + cache_handle->info.script_offset); + ZCG(mem) = ((char*)cache_handle.mem + cache_handle.info.mem_size); + script = (zend_persistent_script*)((char*)buf + cache_handle.info.script_offset); script->corrupted = !cache_it; /* used to check if script restored to SHM or process memory */ ok = true; diff --git a/ext/opcache/zend_file_cache.h b/ext/opcache/zend_file_cache.h index 217b69ff67306..ecb77722487c9 100644 --- a/ext/opcache/zend_file_cache.h +++ b/ext/opcache/zend_file_cache.h @@ -19,15 +19,7 @@ #ifndef ZEND_FILE_CACHE_H #define ZEND_FILE_CACHE_H -typedef struct _zend_file_cache_handle { - zend_string *full_path; - zend_file_cache_metainfo info; - void *mem, *checkpoint; -} zend_file_cache_handle; - int zend_file_cache_script_store(zend_persistent_script *script, bool in_shm); -zend_always_inline zend_result zend_file_cache_open(zend_file_handle *file_handle, zend_file_cache_handle *cache_handle); -zend_always_inline void zend_file_cache_close(zend_file_cache_handle *cache_handle); zend_result zend_file_cache_validate(zend_file_handle *file_handle); zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handle); void zend_file_cache_invalidate(zend_string *full_path); From f620d9c2c0f3199f40e24e400dfec2a23563f0c2 Mon Sep 17 00:00:00 2001 From: Samuel Melrose Date: Tue, 3 Dec 2024 10:50:47 +0000 Subject: [PATCH 11/15] Unit Tests for GH-16551 --- ext/opcache/tests/gh16551_001.phpt | 42 +++++++++++ ext/opcache/tests/gh16551_002.phpt | 37 ++++++++++ ext/opcache/tests/gh16551_003.phpt | 41 +++++++++++ ext/opcache/tests/gh16551_004.phpt | 36 ++++++++++ ext/opcache/tests/gh16551_005.phpt | 42 +++++++++++ ext/opcache/tests/gh16551_099.phpt | 70 +++++++++++++++++++ ext/opcache/tests/gh16551_998.inc | 5 ++ ext/opcache/tests/gh16551_999.inc | 5 ++ .../tests/{gt16979.phpt => gh16979.phpt} | 0 9 files changed, 278 insertions(+) create mode 100644 ext/opcache/tests/gh16551_001.phpt create mode 100644 ext/opcache/tests/gh16551_002.phpt create mode 100644 ext/opcache/tests/gh16551_003.phpt create mode 100644 ext/opcache/tests/gh16551_004.phpt create mode 100644 ext/opcache/tests/gh16551_005.phpt create mode 100644 ext/opcache/tests/gh16551_099.phpt create mode 100644 ext/opcache/tests/gh16551_998.inc create mode 100644 ext/opcache/tests/gh16551_999.inc rename ext/opcache/tests/{gt16979.phpt => gh16979.phpt} (100%) diff --git a/ext/opcache/tests/gh16551_001.phpt b/ext/opcache/tests/gh16551_001.phpt new file mode 100644 index 0000000000000..eaad4873411b9 --- /dev/null +++ b/ext/opcache/tests/gh16551_001.phpt @@ -0,0 +1,42 @@ +--TEST-- +GH-16551: opcache file cache read only: generate file cache +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit=disable +opcache.jit_buffer_size=0 +opcache.file_cache="{TMP}" +--EXTENSIONS-- +opcache +--CONFLICTS-- +opcache_file_cache +--FILE-- + +--EXPECT-- +bool(true) +bool(true) +bool(false) +bool(false) diff --git a/ext/opcache/tests/gh16551_002.phpt b/ext/opcache/tests/gh16551_002.phpt new file mode 100644 index 0000000000000..f9dde8d6ee568 --- /dev/null +++ b/ext/opcache/tests/gh16551_002.phpt @@ -0,0 +1,37 @@ +--TEST-- +GH-16551: opcache file cache read only: read file cache +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit=disable +opcache.jit_buffer_size=0 +opcache.file_cache="{TMP}" +opcache.file_cache_read_only=1 +--EXTENSIONS-- +opcache +--CONFLICTS-- +opcache_file_cache +--FILE-- + +--EXPECT-- +bool(true) +9 +bool(false) +8 diff --git a/ext/opcache/tests/gh16551_003.phpt b/ext/opcache/tests/gh16551_003.phpt new file mode 100644 index 0000000000000..da5baf8252409 --- /dev/null +++ b/ext/opcache/tests/gh16551_003.phpt @@ -0,0 +1,41 @@ +--TEST-- +GH-16551: opcache file cache read only: read file cache with limited checks +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit=disable +opcache.jit_buffer_size=0 +opcache.file_cache="{TMP}" +opcache.file_cache_read_only=1 +opcache.revalidate_freq=0 +opcache.enable_file_override=true +opcache.validate_timestamps=false +opcache.file_cache_consistency_checks=false +--EXTENSIONS-- +opcache +--CONFLICTS-- +opcache_file_cache +--FILE-- + +--EXPECT-- +bool(true) +9 +bool(false) +8 diff --git a/ext/opcache/tests/gh16551_004.phpt b/ext/opcache/tests/gh16551_004.phpt new file mode 100644 index 0000000000000..23dae7d97d878 --- /dev/null +++ b/ext/opcache/tests/gh16551_004.phpt @@ -0,0 +1,36 @@ +--TEST-- +GH-16551: opcache file cache read only: ensure we can't invalidate +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit=disable +opcache.jit_buffer_size=0 +opcache.file_cache="{TMP}" +opcache.file_cache_read_only=1 +--EXTENSIONS-- +opcache +--CONFLICTS-- +opcache_file_cache +--FILE-- + +--EXPECT-- +bool(true) +bool(true) +9 diff --git a/ext/opcache/tests/gh16551_005.phpt b/ext/opcache/tests/gh16551_005.phpt new file mode 100644 index 0000000000000..8820696fae648 --- /dev/null +++ b/ext/opcache/tests/gh16551_005.phpt @@ -0,0 +1,42 @@ +--TEST-- +GH-16551: opcache file cache read only: ensure cache files aren't created +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit=disable +opcache.jit_buffer_size=0 +opcache.file_cache="{TMP}" +opcache.file_cache_read_only=1 +--EXTENSIONS-- +opcache +--CONFLICTS-- +opcache_file_cache +--FILE-- + +--EXPECT-- +bool(false) +bool(false) +bool(true) +bool(false) diff --git a/ext/opcache/tests/gh16551_099.phpt b/ext/opcache/tests/gh16551_099.phpt new file mode 100644 index 0000000000000..8fff5a28efbaf --- /dev/null +++ b/ext/opcache/tests/gh16551_099.phpt @@ -0,0 +1,70 @@ +--TEST-- +GH-16551: opcache file cache read only: double check file_cache_only +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit=disable +opcache.jit_buffer_size=0 +opcache.file_cache="{PWD}" +opcache.file_cache_only=1 +--EXTENSIONS-- +opcache +--CONFLICTS-- +opcache_file_cache +--FILE-- + +--CLEAN-- + strlen(__DIR__)) { + rmdir($p); + $p = dirname($p); + } +} +?> +--EXPECT-- +bool(false) +bool(false) +bool(false) +bool(true) +bool(true) diff --git a/ext/opcache/tests/gh16551_998.inc b/ext/opcache/tests/gh16551_998.inc new file mode 100644 index 0000000000000..a769c4f5b323d --- /dev/null +++ b/ext/opcache/tests/gh16551_998.inc @@ -0,0 +1,5 @@ + Date: Tue, 3 Dec 2024 15:14:08 +0000 Subject: [PATCH 12/15] fix for file_cache_only --- ext/opcache/tests/gh16551_005.phpt | 3 +++ ext/opcache/tests/gh16551_099.phpt | 6 +++--- ext/opcache/zend_accelerator_module.c | 7 ++++++- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/ext/opcache/tests/gh16551_005.phpt b/ext/opcache/tests/gh16551_005.phpt index 8820696fae648..87c08749c8eb9 100644 --- a/ext/opcache/tests/gh16551_005.phpt +++ b/ext/opcache/tests/gh16551_005.phpt @@ -16,6 +16,9 @@ opcache_file_cache $uncached_file = __DIR__ . '/gh16551_999.inc'; +// fix problem on ARM where it was already reporting as cached in SHM. +opcache_invalidate($uncached_file); + var_dump( opcache_is_script_cached($uncached_file) ); diff --git a/ext/opcache/tests/gh16551_099.phpt b/ext/opcache/tests/gh16551_099.phpt index 8fff5a28efbaf..5c5e713f771e6 100644 --- a/ext/opcache/tests/gh16551_099.phpt +++ b/ext/opcache/tests/gh16551_099.phpt @@ -49,15 +49,15 @@ var_dump( --CLEAN-- strlen(__DIR__)) { - rmdir($p); + @rmdir($p); $p = dirname($p); } } diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 2e158229957e1..ad42497887471 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -1037,7 +1037,12 @@ ZEND_FUNCTION(opcache_is_script_cached_in_file_cache) RETURN_FALSE; } - if (!ZCG(accelerator_enabled) || !ZCG(accel_directives).file_cache) { + // account for accelerator_enabled = false when file_cache_only = true + if (!(ZCG(accelerator_enabled) || ZCG(accel_directives).file_cache_only)) { + RETURN_FALSE; + } + + if (!ZCG(accel_directives).file_cache) { RETURN_FALSE; } From e28257893506090613cac7aabb2167c28b43d577 Mon Sep 17 00:00:00 2001 From: Samuel Melrose Date: Wed, 4 Dec 2024 09:34:11 +0000 Subject: [PATCH 13/15] probe ARM test failure --- .../tests/{gh16979.phpt => gh16979_001.phpt} | 0 ext/opcache/tests/gh16979_100.phpt | 28 +++++++++++++++++ ext/opcache/tests/gh16979_101.phpt | 29 ++++++++++++++++++ ext/opcache/tests/gh16979_102.phpt | 30 +++++++++++++++++++ ext/opcache/tests/gh16979_103.phpt | 28 +++++++++++++++++ ext/opcache/tests/gh16979_999.inc | 3 ++ 6 files changed, 118 insertions(+) rename ext/opcache/tests/{gh16979.phpt => gh16979_001.phpt} (100%) create mode 100644 ext/opcache/tests/gh16979_100.phpt create mode 100644 ext/opcache/tests/gh16979_101.phpt create mode 100644 ext/opcache/tests/gh16979_102.phpt create mode 100644 ext/opcache/tests/gh16979_103.phpt create mode 100644 ext/opcache/tests/gh16979_999.inc diff --git a/ext/opcache/tests/gh16979.phpt b/ext/opcache/tests/gh16979_001.phpt similarity index 100% rename from ext/opcache/tests/gh16979.phpt rename to ext/opcache/tests/gh16979_001.phpt diff --git a/ext/opcache/tests/gh16979_100.phpt b/ext/opcache/tests/gh16979_100.phpt new file mode 100644 index 0000000000000..f100fb3946910 --- /dev/null +++ b/ext/opcache/tests/gh16979_100.phpt @@ -0,0 +1,28 @@ +--TEST-- +GH-16979: CircleCI ARM: opcache_is_script_cached always true +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit=disable +opcache.jit_buffer_size=0 +--EXTENSIONS-- +opcache +--CONFLICTS-- +opcache_cached_arm +--FILE-- + +--EXPECT-- +bool(false) +bool(false) diff --git a/ext/opcache/tests/gh16979_101.phpt b/ext/opcache/tests/gh16979_101.phpt new file mode 100644 index 0000000000000..c1afe71834ebc --- /dev/null +++ b/ext/opcache/tests/gh16979_101.phpt @@ -0,0 +1,29 @@ +--TEST-- +GH-16979: CircleCI ARM: opcache_is_script_cached always true: file cache enabled +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit=disable +opcache.jit_buffer_size=0 +opcache.file_cache="{TMP}" +--EXTENSIONS-- +opcache +--CONFLICTS-- +opcache_cached_arm +--FILE-- + +--EXPECT-- +bool(false) +bool(false) diff --git a/ext/opcache/tests/gh16979_102.phpt b/ext/opcache/tests/gh16979_102.phpt new file mode 100644 index 0000000000000..cf2cf0dff641e --- /dev/null +++ b/ext/opcache/tests/gh16979_102.phpt @@ -0,0 +1,30 @@ +--TEST-- +GH-16979: CircleCI ARM: opcache_is_script_cached always true: file cache read only +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit=disable +opcache.jit_buffer_size=0 +opcache.file_cache="{TMP}" +opcache.file_cache_read_only=1 +--EXTENSIONS-- +opcache +--CONFLICTS-- +opcache_cached_arm +--FILE-- + +--EXPECT-- +bool(false) +bool(false) diff --git a/ext/opcache/tests/gh16979_103.phpt b/ext/opcache/tests/gh16979_103.phpt new file mode 100644 index 0000000000000..bb7793fc9d9d6 --- /dev/null +++ b/ext/opcache/tests/gh16979_103.phpt @@ -0,0 +1,28 @@ +--TEST-- +GH-16979: CircleCI ARM: opcache_is_script_cached always true: non existant file +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit=disable +opcache.jit_buffer_size=0 +--EXTENSIONS-- +opcache +--CONFLICTS-- +opcache_cached_arm +--FILE-- + +--EXPECT-- +bool(false) +bool(false) diff --git a/ext/opcache/tests/gh16979_999.inc b/ext/opcache/tests/gh16979_999.inc new file mode 100644 index 0000000000000..e53d4f27fffb7 --- /dev/null +++ b/ext/opcache/tests/gh16979_999.inc @@ -0,0 +1,3 @@ + Date: Wed, 4 Dec 2024 10:05:46 +0000 Subject: [PATCH 14/15] remove unnecessary tests --- ext/opcache/tests/gh16551_005.phpt | 13 ------------- ext/opcache/tests/gh16979_100.phpt | 28 ---------------------------- ext/opcache/tests/gh16979_101.phpt | 29 ----------------------------- ext/opcache/tests/gh16979_102.phpt | 30 ------------------------------ ext/opcache/tests/gh16979_103.phpt | 28 ---------------------------- ext/opcache/tests/gh16979_999.inc | 3 --- 6 files changed, 131 deletions(-) delete mode 100644 ext/opcache/tests/gh16979_100.phpt delete mode 100644 ext/opcache/tests/gh16979_101.phpt delete mode 100644 ext/opcache/tests/gh16979_102.phpt delete mode 100644 ext/opcache/tests/gh16979_103.phpt delete mode 100644 ext/opcache/tests/gh16979_999.inc diff --git a/ext/opcache/tests/gh16551_005.phpt b/ext/opcache/tests/gh16551_005.phpt index 87c08749c8eb9..d8b982c4bbc0d 100644 --- a/ext/opcache/tests/gh16551_005.phpt +++ b/ext/opcache/tests/gh16551_005.phpt @@ -16,17 +16,6 @@ opcache_file_cache $uncached_file = __DIR__ . '/gh16551_999.inc'; -// fix problem on ARM where it was already reporting as cached in SHM. -opcache_invalidate($uncached_file); - -var_dump( - opcache_is_script_cached($uncached_file) -); - -var_dump( - opcache_is_script_cached_in_file_cache($uncached_file) -); - opcache_compile_file($uncached_file); var_dump( @@ -39,7 +28,5 @@ var_dump( ?> --EXPECT-- -bool(false) -bool(false) bool(true) bool(false) diff --git a/ext/opcache/tests/gh16979_100.phpt b/ext/opcache/tests/gh16979_100.phpt deleted file mode 100644 index f100fb3946910..0000000000000 --- a/ext/opcache/tests/gh16979_100.phpt +++ /dev/null @@ -1,28 +0,0 @@ ---TEST-- -GH-16979: CircleCI ARM: opcache_is_script_cached always true ---INI-- -opcache.enable=1 -opcache.enable_cli=1 -opcache.jit=disable -opcache.jit_buffer_size=0 ---EXTENSIONS-- -opcache ---CONFLICTS-- -opcache_cached_arm ---FILE-- - ---EXPECT-- -bool(false) -bool(false) diff --git a/ext/opcache/tests/gh16979_101.phpt b/ext/opcache/tests/gh16979_101.phpt deleted file mode 100644 index c1afe71834ebc..0000000000000 --- a/ext/opcache/tests/gh16979_101.phpt +++ /dev/null @@ -1,29 +0,0 @@ ---TEST-- -GH-16979: CircleCI ARM: opcache_is_script_cached always true: file cache enabled ---INI-- -opcache.enable=1 -opcache.enable_cli=1 -opcache.jit=disable -opcache.jit_buffer_size=0 -opcache.file_cache="{TMP}" ---EXTENSIONS-- -opcache ---CONFLICTS-- -opcache_cached_arm ---FILE-- - ---EXPECT-- -bool(false) -bool(false) diff --git a/ext/opcache/tests/gh16979_102.phpt b/ext/opcache/tests/gh16979_102.phpt deleted file mode 100644 index cf2cf0dff641e..0000000000000 --- a/ext/opcache/tests/gh16979_102.phpt +++ /dev/null @@ -1,30 +0,0 @@ ---TEST-- -GH-16979: CircleCI ARM: opcache_is_script_cached always true: file cache read only ---INI-- -opcache.enable=1 -opcache.enable_cli=1 -opcache.jit=disable -opcache.jit_buffer_size=0 -opcache.file_cache="{TMP}" -opcache.file_cache_read_only=1 ---EXTENSIONS-- -opcache ---CONFLICTS-- -opcache_cached_arm ---FILE-- - ---EXPECT-- -bool(false) -bool(false) diff --git a/ext/opcache/tests/gh16979_103.phpt b/ext/opcache/tests/gh16979_103.phpt deleted file mode 100644 index bb7793fc9d9d6..0000000000000 --- a/ext/opcache/tests/gh16979_103.phpt +++ /dev/null @@ -1,28 +0,0 @@ ---TEST-- -GH-16979: CircleCI ARM: opcache_is_script_cached always true: non existant file ---INI-- -opcache.enable=1 -opcache.enable_cli=1 -opcache.jit=disable -opcache.jit_buffer_size=0 ---EXTENSIONS-- -opcache ---CONFLICTS-- -opcache_cached_arm ---FILE-- - ---EXPECT-- -bool(false) -bool(false) diff --git a/ext/opcache/tests/gh16979_999.inc b/ext/opcache/tests/gh16979_999.inc deleted file mode 100644 index e53d4f27fffb7..0000000000000 --- a/ext/opcache/tests/gh16979_999.inc +++ /dev/null @@ -1,3 +0,0 @@ - Date: Mon, 3 Feb 2025 15:54:03 +0000 Subject: [PATCH 15/15] switch to validate_only method --- ext/opcache/ZendAccelerator.c | 4 +- ext/opcache/zend_accelerator_module.c | 4 +- ext/opcache/zend_file_cache.c | 188 ++++++++++++++------------ ext/opcache/zend_file_cache.h | 3 +- 4 files changed, 104 insertions(+), 95 deletions(-) diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index 323a3278dd137..62bff2147c5ce 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -1917,7 +1917,7 @@ static zend_op_array *file_cache_compile_file(zend_file_handle *file_handle, int HANDLE_BLOCK_INTERRUPTIONS(); SHM_UNPROTECT(); - persistent_script = zend_file_cache_script_load(file_handle); + persistent_script = zend_file_cache_script_load(file_handle, false); SHM_PROTECT(); HANDLE_UNBLOCK_INTERRUPTIONS(); if (persistent_script) { @@ -2134,7 +2134,7 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type) /* Check the second level cache */ if (!persistent_script && ZCG(accel_directives).file_cache) { - persistent_script = zend_file_cache_script_load(file_handle); + persistent_script = zend_file_cache_script_load(file_handle, false); } /* If script was not found or invalidated by validate_timestamps */ diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 8aea4642049c4..a5d12e9386e13 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -380,11 +380,11 @@ static int filename_is_in_file_cache(zend_string *filename) zend_stream_init_filename_ex(&handle, filename); handle.opened_path = realpath; - zend_result result = zend_file_cache_validate(&handle); + zend_persistent_script *result = zend_file_cache_script_load(&handle, true); zend_destroy_file_handle(&handle); - return result == SUCCESS; + return result != NULL; } diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index 4d2e2dc71dbf7..c18373c6abe53 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -202,12 +202,6 @@ typedef struct _zend_file_cache_metainfo { uint32_t checksum; } zend_file_cache_metainfo; -typedef struct _zend_file_cache_handle { - zend_string *full_path; - zend_file_cache_metainfo info; - void *mem, *checkpoint; -} zend_file_cache_handle; - static int zend_file_cache_mkdir(char *filename, size_t start) { char *s = filename + start; @@ -1831,58 +1825,114 @@ static void zend_file_cache_unserialize(zend_persistent_script *script, zend_file_cache_unserialize_early_bindings(script, buf); } -zend_result zend_file_cache_validate_and_open(zend_file_handle *file_handle, zend_file_cache_handle *cache_handle) +static zend_persistent_script file_cache_validate_success_script; + +zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handle, bool validate_only) { + zend_string *full_path = file_handle->opened_path; int fd; char *filename; + zend_persistent_script *script; + zend_file_cache_metainfo info; + zend_accel_hash_entry *bucket; + void *mem, *checkpoint, *buf; + bool cache_it = true; unsigned int actual_checksum; + bool ok; - if (!file_handle || !file_handle->opened_path) { - return FAILURE; + if (!full_path) { + return NULL; } - cache_handle->full_path = file_handle->opened_path; + filename = zend_file_cache_get_bin_file_path(full_path); - filename = zend_file_cache_get_bin_file_path(cache_handle->full_path); fd = zend_file_cache_open(filename, O_RDONLY | O_BINARY); if (fd < 0) { - goto failure; // Open failed + efree(filename); + return NULL; } if (zend_file_cache_flock(fd, LOCK_SH) != 0) { - goto failure_close; // Flock failed + close(fd); + efree(filename); + return NULL; } - - if (read(fd, &cache_handle->info, sizeof(cache_handle->info)) != sizeof(cache_handle->info)) { + if (read(fd, &info, sizeof(info)) != sizeof(info)) { zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot read from file '%s' (info)\n", filename); - goto failure_unlock_close; + zend_file_cache_flock(fd, LOCK_UN); + close(fd); + if (!ZCG(accel_directives).file_cache_read_only) { + zend_file_cache_unlink(filename); + } + efree(filename); + return NULL; } - // Verify header - if (memcmp(cache_handle->info.magic, "OPCACHE", 8) != 0 || memcmp(cache_handle->info.system_id, zend_system_id, 32) != 0) { - zend_accel_error(ACCEL_LOG_WARNING, "opcache invalid header in file '%s'\n", filename); - goto failure_unlock_close; + /* verify header */ + if (memcmp(info.magic, "OPCACHE", 8) != 0) { + zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot read from file '%s' (wrong header)\n", filename); + zend_file_cache_flock(fd, LOCK_UN); + close(fd); + if (!ZCG(accel_directives).file_cache_read_only) { + zend_file_cache_unlink(filename); + } + efree(filename); + return NULL; + } + if (memcmp(info.system_id, zend_system_id, 32) != 0) { + zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot read from file '%s' (wrong \"system_id\")\n", filename); + zend_file_cache_flock(fd, LOCK_UN); + close(fd); + if (!ZCG(accel_directives).file_cache_read_only) { + zend_file_cache_unlink(filename); + } + efree(filename); + return NULL; } - // Verify timestamp if required + /* verify timestamp */ if (ZCG(accel_directives).validate_timestamps && - zend_get_file_handle_timestamp(file_handle, NULL) != cache_handle->info.timestamp) { - goto failure_unlock_close; + zend_get_file_handle_timestamp(file_handle, NULL) != info.timestamp) { + if (zend_file_cache_flock(fd, LOCK_UN) != 0) { + zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot unlock file '%s'\n", filename); + } + close(fd); + if (!ZCG(accel_directives).file_cache_read_only) { + zend_file_cache_unlink(filename); + } + efree(filename); + return NULL; + } + + /* return here if validating */ + if (validate_only) { + if (zend_file_cache_flock(fd, LOCK_UN) != 0) { + zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot unlock file '%s'\n", filename); + } + close(fd); + efree(filename); + return &file_cache_validate_success_script; } - cache_handle->checkpoint = zend_arena_checkpoint(CG(arena)); + checkpoint = zend_arena_checkpoint(CG(arena)); #if defined(__AVX__) || defined(__SSE2__) /* Align to 64-byte boundary */ - cache_handle->mem = zend_arena_alloc(&CG(arena), cache_handle->info.mem_size + cache_handle->info.str_size + 64); - cache_handle->mem = (void*)(((uintptr_t)cache_handle->mem + 63L) & ~63L); + mem = zend_arena_alloc(&CG(arena), info.mem_size + info.str_size + 64); + mem = (void*)(((uintptr_t)mem + 63L) & ~63L); #else - cache_handle->mem = zend_arena_alloc(&CG(arena), cache_handle->info.mem_size + cache_handle->info.str_size); + mem = zend_arena_alloc(&CG(arena), info.mem_size + info.str_size); #endif - if (read(fd, cache_handle->mem, cache_handle->info.mem_size + cache_handle->info.str_size) != (ssize_t)(cache_handle->info.mem_size + cache_handle->info.str_size)) { + if (read(fd, mem, info.mem_size + info.str_size) != (ssize_t)(info.mem_size + info.str_size)) { zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot read from file '%s' (mem)\n", filename); - zend_arena_release(&CG(arena), cache_handle->checkpoint); - goto failure_unlock_close; + zend_file_cache_flock(fd, LOCK_UN); + close(fd); + if (!ZCG(accel_directives).file_cache_read_only) { + zend_file_cache_unlink(filename); + } + zend_arena_release(&CG(arena), checkpoint); + efree(filename); + return NULL; } if (zend_file_cache_flock(fd, LOCK_UN) != 0) { zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot unlock file '%s'\n", filename); @@ -1891,56 +1941,13 @@ zend_result zend_file_cache_validate_and_open(zend_file_handle *file_handle, zen /* verify checksum */ if (ZCG(accel_directives).file_cache_consistency_checks && - (actual_checksum = zend_adler32(ADLER32_INIT, cache_handle->mem, cache_handle->info.mem_size + cache_handle->info.str_size)) != cache_handle->info.checksum) { - zend_accel_error(ACCEL_LOG_WARNING, "corrupted file '%s' excepted checksum: 0x%08x actual checksum: 0x%08x\n", filename, cache_handle->info.checksum, actual_checksum); - zend_arena_release(&CG(arena), cache_handle->checkpoint); - goto failure; - } - - efree(filename); - return SUCCESS; - -failure_unlock_close: - zend_file_cache_flock(fd, LOCK_UN); -failure_close: - close(fd); -failure: - if (!ZCG(accel_directives).file_cache_read_only) { - zend_file_cache_unlink(filename); - } - efree(filename); - - return FAILURE; -} - -void zend_file_cache_close(zend_file_cache_handle *cache_handle) -{ - zend_arena_release(&CG(arena), cache_handle->checkpoint); -} - -zend_result zend_file_cache_validate(zend_file_handle *file_handle) -{ - zend_file_cache_handle cache_handle; - - zend_result ret = zend_file_cache_validate_and_open(file_handle, &cache_handle); - - if (ret == SUCCESS) { - zend_file_cache_close(&cache_handle); - } - - return ret; -} - -zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handle) -{ - zend_file_cache_handle cache_handle; - zend_persistent_script *script; - zend_accel_hash_entry *bucket; - void *buf; - bool cache_it = true; - bool ok; - - if (zend_file_cache_validate_and_open(file_handle, &cache_handle) == FAILURE) { + (actual_checksum = zend_adler32(ADLER32_INIT, mem, info.mem_size + info.str_size)) != info.checksum) { + zend_accel_error(ACCEL_LOG_WARNING, "corrupted file '%s' excepted checksum: 0x%08x actual checksum: 0x%08x\n", filename, info.checksum, actual_checksum); + if (!ZCG(accel_directives).file_cache_read_only) { + zend_file_cache_unlink(filename); + } + zend_arena_release(&CG(arena), checkpoint); + efree(filename); return NULL; } @@ -1955,12 +1962,13 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl /* Check if we still need to put the file into the cache (may be it was * already stored by another process. This final check is done under * exclusive lock) */ - bucket = zend_accel_hash_find_entry(&ZCSG(hash), cache_handle.full_path); + bucket = zend_accel_hash_find_entry(&ZCSG(hash), full_path); if (bucket) { script = (zend_persistent_script *)bucket->data; if (!script->corrupted) { zend_shared_alloc_unlock(); - zend_file_cache_close(&cache_handle); + zend_arena_release(&CG(arena), checkpoint); + efree(filename); return script; } } @@ -1973,23 +1981,23 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl goto use_process_mem; } - buf = zend_shared_alloc_aligned(cache_handle.info.mem_size); + buf = zend_shared_alloc_aligned(info.mem_size); if (!buf) { zend_accel_schedule_restart_if_necessary(ACCEL_RESTART_OOM); zend_shared_alloc_unlock(); goto use_process_mem; } - memcpy(buf, cache_handle.mem, cache_handle.info.mem_size); + memcpy(buf, mem, info.mem_size); zend_map_ptr_extend(ZCSG(map_ptr_last)); } else { use_process_mem: - buf = cache_handle.mem; + buf = mem; cache_it = false; } - ZCG(mem) = ((char*)cache_handle.mem + cache_handle.info.mem_size); - script = (zend_persistent_script*)((char*)buf + cache_handle.info.script_offset); + ZCG(mem) = ((char*)mem + info.mem_size); + script = (zend_persistent_script*)((char*)buf + info.script_offset); script->corrupted = !cache_it; /* used to check if script restored to SHM or process memory */ ok = true; @@ -2003,7 +2011,8 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl zend_shared_alloc_unlock(); goto use_process_mem; } else { - zend_file_cache_close(&cache_handle); + zend_arena_release(&CG(arena), checkpoint); + efree(filename); return NULL; } } @@ -2019,8 +2028,9 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl zend_shared_alloc_unlock(); zend_accel_error(ACCEL_LOG_INFO, "File cached script loaded into memory '%s'", ZSTR_VAL(script->script.filename)); - zend_file_cache_close(&cache_handle); + zend_arena_release(&CG(arena), checkpoint); } + efree(filename); return script; } diff --git a/ext/opcache/zend_file_cache.h b/ext/opcache/zend_file_cache.h index ecb77722487c9..246766f58964f 100644 --- a/ext/opcache/zend_file_cache.h +++ b/ext/opcache/zend_file_cache.h @@ -20,8 +20,7 @@ #define ZEND_FILE_CACHE_H int zend_file_cache_script_store(zend_persistent_script *script, bool in_shm); -zend_result zend_file_cache_validate(zend_file_handle *file_handle); -zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handle); +zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handle, bool validate_only); void zend_file_cache_invalidate(zend_string *full_path); #endif /* ZEND_FILE_CACHE_H */