From 3ffd530667d5eff2457a1366d9e556a52970ee6f Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Fri, 19 Jul 2024 13:02:18 +0200 Subject: [PATCH 1/6] Allow optimizer to depend on preloaded symbols It is safe for the optimizer to rely on preloaded symbols. This can occur when compiling non-preloaded files, referencing preloaded ones. --- Zend/Optimizer/zend_optimizer.c | 21 ++++++------ ext/opcache/tests/preload_optimizer.inc | 5 +++ ext/opcache/tests/preload_optimizer.phpt | 43 ++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 11 deletions(-) create mode 100644 ext/opcache/tests/preload_optimizer.inc create mode 100644 ext/opcache/tests/preload_optimizer.phpt diff --git a/Zend/Optimizer/zend_optimizer.c b/Zend/Optimizer/zend_optimizer.c index 3f54db9f09f9..fb70e5889177 100644 --- a/Zend/Optimizer/zend_optimizer.c +++ b/Zend/Optimizer/zend_optimizer.c @@ -802,6 +802,7 @@ zend_class_entry *zend_optimizer_get_class_entry( ce = zend_hash_find_ptr(CG(class_table), lcname); if (ce && (ce->type == ZEND_INTERNAL_CLASS + || (ce->ce_flags & ZEND_ACC_PRELOADED) || (op_array && ce->info.user.filename == op_array->filename))) { return ce; } @@ -846,11 +847,9 @@ const zend_class_constant *zend_fetch_class_const_info( } else { zend_class_entry *tmp = zend_hash_find_ptr(EG(class_table), Z_STR_P(op1 + 1)); if (tmp != NULL) { - if (tmp->type == ZEND_INTERNAL_CLASS) { - ce = tmp; - } else if (tmp->type == ZEND_USER_CLASS - && tmp->info.user.filename - && tmp->info.user.filename == op_array->filename) { + if (tmp->type == ZEND_INTERNAL_CLASS + || (tmp->ce_flags & ZEND_ACC_PRELOADED) + || (tmp->info.user.filename && tmp->info.user.filename == op_array->filename)) { ce = tmp; } } @@ -902,9 +901,9 @@ zend_function *zend_optimizer_get_called_func( } else if ((func = zend_hash_find_ptr(EG(function_table), function_name)) != NULL) { if (func->type == ZEND_INTERNAL_FUNCTION) { return func; - } else if (func->type == ZEND_USER_FUNCTION && - func->op_array.filename && - func->op_array.filename == op_array->filename) { + } else if (func->type == ZEND_USER_FUNCTION + && ((func->op_array.fn_flags & ZEND_ACC_PRELOADED) + || (func->op_array.filename && func->op_array.filename == op_array->filename))) { return func; } } @@ -920,9 +919,9 @@ zend_function *zend_optimizer_get_called_func( } else if ((func = zend_hash_find_ptr(EG(function_table), Z_STR_P(function_name))) != NULL) { if (func->type == ZEND_INTERNAL_FUNCTION) { return func; - } else if (func->type == ZEND_USER_FUNCTION && - func->op_array.filename && - func->op_array.filename == op_array->filename) { + } else if (func->type == ZEND_USER_FUNCTION + && ((func->op_array.fn_flags & ZEND_ACC_PRELOADED) + || (func->op_array.filename && func->op_array.filename == op_array->filename))) { return func; } } diff --git a/ext/opcache/tests/preload_optimizer.inc b/ext/opcache/tests/preload_optimizer.inc new file mode 100644 index 000000000000..728a44c2702a --- /dev/null +++ b/ext/opcache/tests/preload_optimizer.inc @@ -0,0 +1,5 @@ + +--FILE-- + +--EXPECTF-- +$_main: + ; (lines=1, args=0, vars=0, tmps=%d) + ; (after optimizer) + ; $PRELOAD$:0-0 +0000 RETURN null + +foo: + ; (lines=1, args=0, vars=0, tmps=%d) + ; (after optimizer) + ; %spreload_optimizer.inc:3-5 +0000 RETURN int(42) + +$_main: + ; (lines=1, args=0, vars=0, tmps=%d) + ; (after optimizer) + ; %spreload_optimizer.inc:1-6 +0000 RETURN int(1) + +$_main: + ; (lines=2, args=0, vars=0, tmps=%d) + ; (after optimizer) + ; %spreload_optimizer.php:1-4 +0000 ECHO string("42") +0001 RETURN int(1) +42 From 042709cc72ea377ebaf18081c4ef7afca98746ac Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Fri, 19 Jul 2024 15:28:51 +0200 Subject: [PATCH 2/6] Disable inline pass for observer test --- ext/zend_test/tests/observer_preload.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/zend_test/tests/observer_preload.phpt b/ext/zend_test/tests/observer_preload.phpt index a58ec032d447..63180b64c9e8 100644 --- a/ext/zend_test/tests/observer_preload.phpt +++ b/ext/zend_test/tests/observer_preload.phpt @@ -10,7 +10,7 @@ if (PHP_OS_FAMILY == 'Windows') die('skip Preloading is not supported on Windows --INI-- opcache.enable=1 opcache.enable_cli=1 -opcache.optimization_level=-1 +opcache.optimization_level=0x7ffe3fff opcache.preload={PWD}/observer_preload.inc opcache.file_cache= opcache.file_cache_only=0 From f3940284cda75a91a4ff908e329eb7f3199d34a7 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Fri, 19 Jul 2024 15:36:21 +0200 Subject: [PATCH 3/6] Move duplicated code into functions --- Zend/Optimizer/zend_optimizer.c | 45 ++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/Zend/Optimizer/zend_optimizer.c b/Zend/Optimizer/zend_optimizer.c index fb70e5889177..6b870d93d414 100644 --- a/Zend/Optimizer/zend_optimizer.c +++ b/Zend/Optimizer/zend_optimizer.c @@ -792,6 +792,26 @@ void zend_optimizer_shift_jump(zend_op_array *op_array, zend_op *opline, uint32_ } } +static bool zend_optimizer_ignore_class(zend_class_entry *ce, zend_string *filename) +{ + return ce->type == ZEND_USER_CLASS + && !(ce->ce_flags & ZEND_ACC_PRELOADED) + && (!ce->info.user.filename || ce->info.user.filename != filename); +} + +static bool zend_optimizer_ignore_function(zend_function *fbc, zend_string *filename) +{ + if (fbc->type == ZEND_INTERNAL_FUNCTION) { + return false; + } else if (fbc->type == ZEND_USER_FUNCTION) { + return !(fbc->op_array.fn_flags & ZEND_ACC_PRELOADED) + && (!fbc->op_array.filename && fbc->op_array.filename != filename); + } else { + ZEND_ASSERT(fbc->type == ZEND_EVAL_CODE); + return true; + } +} + zend_class_entry *zend_optimizer_get_class_entry( const zend_script *script, const zend_op_array *op_array, zend_string *lcname) { zend_class_entry *ce = script ? zend_hash_find_ptr(&script->class_table, lcname) : NULL; @@ -800,10 +820,7 @@ zend_class_entry *zend_optimizer_get_class_entry( } ce = zend_hash_find_ptr(CG(class_table), lcname); - if (ce - && (ce->type == ZEND_INTERNAL_CLASS - || (ce->ce_flags & ZEND_ACC_PRELOADED) - || (op_array && ce->info.user.filename == op_array->filename))) { + if (ce && !zend_optimizer_ignore_class(ce, op_array ? op_array->filename : NULL)) { return ce; } @@ -846,12 +863,8 @@ const zend_class_constant *zend_fetch_class_const_info( ce = zend_optimizer_get_class_entry(script, op_array, Z_STR_P(op1 + 1)); } else { zend_class_entry *tmp = zend_hash_find_ptr(EG(class_table), Z_STR_P(op1 + 1)); - if (tmp != NULL) { - if (tmp->type == ZEND_INTERNAL_CLASS - || (tmp->ce_flags & ZEND_ACC_PRELOADED) - || (tmp->info.user.filename && tmp->info.user.filename == op_array->filename)) { - ce = tmp; - } + if (tmp != NULL && !zend_optimizer_ignore_class(tmp, op_array->filename)) { + ce = tmp; } } } @@ -899,11 +912,7 @@ zend_function *zend_optimizer_get_called_func( if (script && (func = zend_hash_find_ptr(&script->function_table, function_name)) != NULL) { return func; } else if ((func = zend_hash_find_ptr(EG(function_table), function_name)) != NULL) { - if (func->type == ZEND_INTERNAL_FUNCTION) { - return func; - } else if (func->type == ZEND_USER_FUNCTION - && ((func->op_array.fn_flags & ZEND_ACC_PRELOADED) - || (func->op_array.filename && func->op_array.filename == op_array->filename))) { + if (!zend_optimizer_ignore_function(func, op_array->filename)) { return func; } } @@ -917,11 +926,7 @@ zend_function *zend_optimizer_get_called_func( if (script && (func = zend_hash_find_ptr(&script->function_table, Z_STR_P(function_name)))) { return func; } else if ((func = zend_hash_find_ptr(EG(function_table), Z_STR_P(function_name))) != NULL) { - if (func->type == ZEND_INTERNAL_FUNCTION) { - return func; - } else if (func->type == ZEND_USER_FUNCTION - && ((func->op_array.fn_flags & ZEND_ACC_PRELOADED) - || (func->op_array.filename && func->op_array.filename == op_array->filename))) { + if (!zend_optimizer_ignore_function(func, op_array->filename)) { return func; } } From 460aea8371c6ae46ee744a221f0e35d40ed354be Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Fri, 19 Jul 2024 15:40:43 +0200 Subject: [PATCH 4/6] Add comment to specific optimization value --- ext/zend_test/tests/observer_preload.phpt | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/zend_test/tests/observer_preload.phpt b/ext/zend_test/tests/observer_preload.phpt index 63180b64c9e8..042c9e088e53 100644 --- a/ext/zend_test/tests/observer_preload.phpt +++ b/ext/zend_test/tests/observer_preload.phpt @@ -10,6 +10,7 @@ if (PHP_OS_FAMILY == 'Windows') die('skip Preloading is not supported on Windows --INI-- opcache.enable=1 opcache.enable_cli=1 +; Disable inlining pass opcache.optimization_level=0x7ffe3fff opcache.preload={PWD}/observer_preload.inc opcache.file_cache= From ba8f02533234653bd6ed15ff292ba8edda0146a6 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Mon, 22 Jul 2024 17:42:46 +0200 Subject: [PATCH 5/6] Optimizer should only rely on preloaded symbols in the symbol table --- Zend/Optimizer/zend_optimizer.c | 52 +++++++++++++++++--------- ext/opcache/tests/gh15021.phpt | 27 +++++++++++++ ext/opcache/tests/gh15021_a.inc | 7 ++++ ext/opcache/tests/gh15021_b.inc | 7 ++++ ext/opcache/tests/gh15021_preload.inc | 4 ++ ext/opcache/tests/gh15021_required.inc | 5 +++ 6 files changed, 85 insertions(+), 17 deletions(-) create mode 100644 ext/opcache/tests/gh15021.phpt create mode 100644 ext/opcache/tests/gh15021_a.inc create mode 100644 ext/opcache/tests/gh15021_b.inc create mode 100644 ext/opcache/tests/gh15021_preload.inc create mode 100644 ext/opcache/tests/gh15021_required.inc diff --git a/Zend/Optimizer/zend_optimizer.c b/Zend/Optimizer/zend_optimizer.c index 6b870d93d414..823277694b4f 100644 --- a/Zend/Optimizer/zend_optimizer.c +++ b/Zend/Optimizer/zend_optimizer.c @@ -792,20 +792,36 @@ void zend_optimizer_shift_jump(zend_op_array *op_array, zend_op *opline, uint32_ } } -static bool zend_optimizer_ignore_class(zend_class_entry *ce, zend_string *filename) +static bool zend_optimizer_ignore_class(zval *ce_zv, zend_string *filename) { + zend_class_entry *ce = Z_PTR_P(ce_zv); + + if (ce->ce_flags & ZEND_ACC_PRELOADED) { + Bucket *ce_bucket = (Bucket*)((uintptr_t)ce_zv - XtOffsetOf(Bucket, val)); + size_t offset = ce_bucket - EG(class_table)->arData; + if (offset < EG(persistent_classes_count)) { + return false; + } + } return ce->type == ZEND_USER_CLASS - && !(ce->ce_flags & ZEND_ACC_PRELOADED) && (!ce->info.user.filename || ce->info.user.filename != filename); } -static bool zend_optimizer_ignore_function(zend_function *fbc, zend_string *filename) +static bool zend_optimizer_ignore_function(zval *fbc_zv, zend_string *filename) { + zend_function *fbc = Z_PTR_P(fbc_zv); + if (fbc->type == ZEND_INTERNAL_FUNCTION) { return false; } else if (fbc->type == ZEND_USER_FUNCTION) { - return !(fbc->op_array.fn_flags & ZEND_ACC_PRELOADED) - && (!fbc->op_array.filename && fbc->op_array.filename != filename); + if (fbc->op_array.fn_flags & ZEND_ACC_PRELOADED) { + Bucket *fbc_bucket = (Bucket*)((uintptr_t)fbc_zv - XtOffsetOf(Bucket, val)); + size_t offset = fbc_bucket - EG(function_table)->arData; + if (offset < EG(persistent_functions_count)) { + return false; + } + } + return !fbc->op_array.filename || fbc->op_array.filename != filename; } else { ZEND_ASSERT(fbc->type == ZEND_EVAL_CODE); return true; @@ -819,9 +835,9 @@ zend_class_entry *zend_optimizer_get_class_entry( return ce; } - ce = zend_hash_find_ptr(CG(class_table), lcname); - if (ce && !zend_optimizer_ignore_class(ce, op_array ? op_array->filename : NULL)) { - return ce; + zval *ce_zv = zend_hash_find(CG(class_table), lcname); + if (ce_zv && !zend_optimizer_ignore_class(ce_zv, op_array ? op_array->filename : NULL)) { + return Z_PTR_P(ce_zv); } if (op_array && op_array->scope && zend_string_equals_ci(op_array->scope->name, lcname)) { @@ -862,9 +878,9 @@ const zend_class_constant *zend_fetch_class_const_info( if (script) { ce = zend_optimizer_get_class_entry(script, op_array, Z_STR_P(op1 + 1)); } else { - zend_class_entry *tmp = zend_hash_find_ptr(EG(class_table), Z_STR_P(op1 + 1)); - if (tmp != NULL && !zend_optimizer_ignore_class(tmp, op_array->filename)) { - ce = tmp; + zval *ce_zv = zend_hash_find(EG(class_table), Z_STR_P(op1 + 1)); + if (ce_zv && !zend_optimizer_ignore_class(ce_zv, op_array->filename)) { + ce = Z_PTR_P(ce_zv); } } } @@ -909,11 +925,12 @@ zend_function *zend_optimizer_get_called_func( { zend_string *function_name = Z_STR_P(CRT_CONSTANT(opline->op2)); zend_function *func; + zval *func_zv; if (script && (func = zend_hash_find_ptr(&script->function_table, function_name)) != NULL) { return func; - } else if ((func = zend_hash_find_ptr(EG(function_table), function_name)) != NULL) { - if (!zend_optimizer_ignore_function(func, op_array->filename)) { - return func; + } else if ((func_zv = zend_hash_find(EG(function_table), function_name)) != NULL) { + if (!zend_optimizer_ignore_function(func_zv, op_array->filename)) { + return Z_PTR_P(func_zv); } } break; @@ -923,11 +940,12 @@ zend_function *zend_optimizer_get_called_func( if (opline->op2_type == IS_CONST && Z_TYPE_P(CRT_CONSTANT(opline->op2)) == IS_STRING) { zval *function_name = CRT_CONSTANT(opline->op2) + 1; zend_function *func; + zval *func_zv; if (script && (func = zend_hash_find_ptr(&script->function_table, Z_STR_P(function_name)))) { return func; - } else if ((func = zend_hash_find_ptr(EG(function_table), Z_STR_P(function_name))) != NULL) { - if (!zend_optimizer_ignore_function(func, op_array->filename)) { - return func; + } else if ((func_zv = zend_hash_find(EG(function_table), Z_STR_P(function_name))) != NULL) { + if (!zend_optimizer_ignore_function(func_zv, op_array->filename)) { + return Z_PTR_P(func_zv); } } } diff --git a/ext/opcache/tests/gh15021.phpt b/ext/opcache/tests/gh15021.phpt new file mode 100644 index 000000000000..0a0e2080267b --- /dev/null +++ b/ext/opcache/tests/gh15021.phpt @@ -0,0 +1,27 @@ +--TEST-- +GH-15021: Optimizer only relies on preloaded top-level symbols +--EXTENSIONS-- +opcache +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.preload={PWD}/gh15021_preload.inc +--FILE-- + +--EXPECT-- +bool(true) diff --git a/ext/opcache/tests/gh15021_a.inc b/ext/opcache/tests/gh15021_a.inc new file mode 100644 index 000000000000..08a4e3aa4488 --- /dev/null +++ b/ext/opcache/tests/gh15021_a.inc @@ -0,0 +1,7 @@ + Date: Mon, 22 Jul 2024 23:19:12 +0200 Subject: [PATCH 6/6] Fix skipif for windows --- ext/opcache/tests/gh15021.phpt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ext/opcache/tests/gh15021.phpt b/ext/opcache/tests/gh15021.phpt index 0a0e2080267b..81bf48cde222 100644 --- a/ext/opcache/tests/gh15021.phpt +++ b/ext/opcache/tests/gh15021.phpt @@ -1,11 +1,15 @@ --TEST-- GH-15021: Optimizer only relies on preloaded top-level symbols ---EXTENSIONS-- -opcache --INI-- opcache.enable=1 opcache.enable_cli=1 opcache.preload={PWD}/gh15021_preload.inc +--EXTENSIONS-- +opcache +--SKIPIF-- + --FILE--