From d1693ecf60aeff44a63826ca224c8f0a49014e7f Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 17 Jul 2023 02:34:49 +0200 Subject: [PATCH 1/3] Fix GH-11715: opcache.interned_strings_buffer either has no effect or opcache_get_status() / phpinfo() is wrong There are a couple of oddities. 1) The interned strings buffer comprises the whole hashtable datastructure. Therefore, it seems that the interned strings buffer size is the size of only said table. However, in the current code it also includes the size of the zend_accel_shared_globals. 2) ZCSG(interned_strings).end is computed starting from the accelerator globals struct itself. I would expect it to start from the part where the interned strings table starts. 3) When computing the used size, it is done using ZCSG(interned_strings).end - ZCSG(interned_strings).start. However, this does not include the uin32_t slots array because ZCSG(interned_strings).start pointers after that array. This patch corrrects these 3 points. --- ext/opcache/ZendAccelerator.c | 4 ++-- ext/opcache/zend_accelerator_module.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index ed0602394ce97..5e0c7c7c1492c 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -2856,7 +2856,7 @@ static int zend_accel_init_shm(void) zend_shared_alloc_lock(); if (ZCG(accel_directives).interned_strings_buffer) { - accel_shared_globals = zend_shared_alloc((ZCG(accel_directives).interned_strings_buffer * 1024 * 1024)); + accel_shared_globals = zend_shared_alloc(sizeof(zend_accel_shared_globals) + (ZCG(accel_directives).interned_strings_buffer * 1024 * 1024)); } else { /* Make sure there is always at least one interned string hash slot, * so the table can be queried unconditionally. */ @@ -2893,7 +2893,7 @@ static int zend_accel_init_shm(void) ZCSG(interned_strings).top = ZCSG(interned_strings).start; ZCSG(interned_strings).end = - (zend_string*)((char*)accel_shared_globals + + (zend_string*)((char*)(accel_shared_globals + 1) + /* table data is stored after accel_shared_globals */ ZCG(accel_directives).interned_strings_buffer * 1024 * 1024); ZCSG(interned_strings).saved_top = NULL; diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 4f8e32a136c29..3d4a9f8c68750 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -487,7 +487,7 @@ void zend_accel_info(ZEND_MODULE_INFO_FUNC_ARGS) snprintf(buf, sizeof(buf), "%zu", ZSMMG(wasted_shared_memory)); php_info_print_table_row(2, "Wasted memory", buf); if (ZCSG(interned_strings).start && ZCSG(interned_strings).end) { - snprintf(buf, sizeof(buf), "%zu", (size_t)((char*)ZCSG(interned_strings).top - (char*)ZCSG(interned_strings).start)); + snprintf(buf, sizeof(buf), "%zu", (size_t)((char*)ZCSG(interned_strings).top - (char*)(accel_shared_globals + 1))); php_info_print_table_row(2, "Interned Strings Used memory", buf); snprintf(buf, sizeof(buf), "%zu", (size_t)((char*)ZCSG(interned_strings).end - (char*)ZCSG(interned_strings).top)); php_info_print_table_row(2, "Interned Strings Free memory", buf); @@ -628,8 +628,8 @@ ZEND_FUNCTION(opcache_get_status) zval interned_strings_usage; array_init(&interned_strings_usage); - add_assoc_long(&interned_strings_usage, "buffer_size", (char*)ZCSG(interned_strings).end - (char*)ZCSG(interned_strings).start); - add_assoc_long(&interned_strings_usage, "used_memory", (char*)ZCSG(interned_strings).top - (char*)ZCSG(interned_strings).start); + add_assoc_long(&interned_strings_usage, "buffer_size", (char*)ZCSG(interned_strings).end - (char*)(accel_shared_globals + 1)); + add_assoc_long(&interned_strings_usage, "used_memory", (char*)ZCSG(interned_strings).top - (char*)(accel_shared_globals + 1)); add_assoc_long(&interned_strings_usage, "free_memory", (char*)ZCSG(interned_strings).end - (char*)ZCSG(interned_strings).top); add_assoc_long(&interned_strings_usage, "number_of_strings", ZCSG(interned_strings).nNumOfElements); add_assoc_zval(return_value, "interned_strings_usage", &interned_strings_usage); From 343f538c67bf50bec558c5b6fd0319f52bc68e8a Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 17 Jul 2023 12:44:55 +0200 Subject: [PATCH 2/3] Add test --- ext/opcache/tests/gh11715.phpt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 ext/opcache/tests/gh11715.phpt diff --git a/ext/opcache/tests/gh11715.phpt b/ext/opcache/tests/gh11715.phpt new file mode 100644 index 0000000000000..4db03482b2976 --- /dev/null +++ b/ext/opcache/tests/gh11715.phpt @@ -0,0 +1,17 @@ +--TEST-- +GH-11715 (opcache.interned_strings_buffer either has no effect or opcache_get_status() / phpinfo() is wrong) +--EXTENSIONS-- +opcache +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.interned_strings_buffer=16 +--FILE-- + +--EXPECT-- +bool(true) From cc9b2f964957922d2d377cbbd242caa7da702387 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Tue, 18 Jul 2023 01:23:05 +0200 Subject: [PATCH 3/3] Improve test to test the actual values --- ext/opcache/tests/gh11715.phpt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ext/opcache/tests/gh11715.phpt b/ext/opcache/tests/gh11715.phpt index 4db03482b2976..1f843e2687979 100644 --- a/ext/opcache/tests/gh11715.phpt +++ b/ext/opcache/tests/gh11715.phpt @@ -10,8 +10,10 @@ opcache.interned_strings_buffer=16 --EXPECT-- -bool(true) +int(16777216) +int(16777216)