From 9c829d992b8b7f0f5fad270bacab9ba9470bc350 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 11 Dec 2024 12:12:51 +0100 Subject: [PATCH 1/2] Fix handling of `zend_ast_decl` AST nodes when copying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously `zend_ast_decl` AST nodes were handled as if they were regular `zend_ast` nodes. This was not observable, because copying an AST only happened when evaluating const-expressions, which could not include declarations until the “Support Closures in constant expressions” RFC. --- Zend/zend_ast.c | 38 ++++++++++++++++++++++++++++++++- Zend/zend_ast.h | 4 ++++ ext/opcache/zend_file_cache.c | 18 ++++++++++++++++ ext/opcache/zend_persist.c | 8 +++++++ ext/opcache/zend_persist_calc.c | 8 +++++++ 5 files changed, 75 insertions(+), 1 deletion(-) diff --git a/Zend/zend_ast.c b/Zend/zend_ast.c index 74dd471fb2cba..3505d335938c9 100644 --- a/Zend/zend_ast.c +++ b/Zend/zend_ast.c @@ -1088,6 +1088,14 @@ static size_t ZEND_FASTCALL zend_ast_tree_size(zend_ast *ast) size += zend_ast_tree_size(list->child[i]); } } + } else if (zend_ast_is_decl(ast)) { + zend_ast_decl *decl = (zend_ast_decl*)ast; + size = sizeof(zend_ast_decl); + for (size_t i = 0; i < 5; i++) { + if (decl->child[i]) { + size += zend_ast_tree_size(decl->child[i]); + } + } } else { uint32_t i, children = zend_ast_get_num_children(ast); @@ -1141,6 +1149,34 @@ static void* ZEND_FASTCALL zend_ast_tree_copy(zend_ast *ast, void *buf) ZVAL_COPY(&new->val, &((zend_ast_zval *) ast)->val); Z_LINENO(new->val) = zend_ast_get_lineno(ast); buf = (void*)((char*)buf + sizeof(zend_ast_zval)); + } else if (zend_ast_is_decl(ast)) { + zend_ast_decl *old = (zend_ast_decl*)ast; + zend_ast_decl *new = (zend_ast_decl*)buf; + new->kind = old->kind; + new->attr = old->attr; + new->start_lineno = old->start_lineno; + new->end_lineno = old->end_lineno; + new->flags = old->flags; + if (old->doc_comment) { + new->doc_comment = zend_string_copy(old->doc_comment); + } else { + new->doc_comment = NULL; + } + if (old->name) { + new->name = zend_string_copy(old->name); + } else { + new->name = NULL; + } + + buf = (void*)((char*)buf + sizeof(zend_ast_decl)); + for (size_t i = 0; i < 5; i++) { + if (old->child[i]) { + new->child[i] = (zend_ast*)buf; + buf = zend_ast_tree_copy(old->child[i], buf); + } else { + new->child[i] = NULL; + } + } } else { uint32_t i, children = zend_ast_get_num_children(ast); zend_ast *new = (zend_ast*)buf; @@ -1206,7 +1242,7 @@ ZEND_API void ZEND_FASTCALL zend_ast_destroy(zend_ast *ast) zend_string_release_ex(zend_ast_get_constant_name(ast), 0); } else if (EXPECTED(ast->kind == ZEND_AST_OP_ARRAY)) { ZEND_ASSERT(!Z_REFCOUNTED(((zend_ast_zval*)(ast))->val)); - } else if (EXPECTED(ast->kind >= ZEND_AST_FUNC_DECL)) { + } else if (zend_ast_is_decl(ast)) { zend_ast_decl *decl = (zend_ast_decl *) ast; if (decl->name) { diff --git a/Zend/zend_ast.h b/Zend/zend_ast.h index 6f68a961b7989..1b31c7cd03e76 100644 --- a/Zend/zend_ast.h +++ b/Zend/zend_ast.h @@ -331,6 +331,10 @@ static zend_always_inline bool zend_ast_is_special(zend_ast *ast) { return (ast->kind >> ZEND_AST_SPECIAL_SHIFT) & 1; } +static zend_always_inline bool zend_ast_is_decl(zend_ast *ast) { + return zend_ast_is_special(ast) && ast->kind >= ZEND_AST_FUNC_DECL; +} + static zend_always_inline bool zend_ast_is_list(zend_ast *ast) { return (ast->kind >> ZEND_AST_IS_LIST_SHIFT) & 1; } diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index f12170e8c9936..c113c70634a7d 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -366,6 +366,16 @@ static void zend_file_cache_serialize_ast(zend_ast *ast, } else if (ast->kind == ZEND_AST_OP_ARRAY) { /* The op_array itself will be serialized as part of the dynamic_func_defs. */ SERIALIZE_PTR(Z_PTR(((zend_ast_zval*)ast)->val)); + } else if (zend_ast_is_decl(ast)) { + zend_ast_decl *decl = (zend_ast_decl*)ast; + for (i = 0; i < 5; i++) { + if (decl->child[i] && !IS_SERIALIZED(decl->child[i])) { + SERIALIZE_PTR(decl->child[i]); + tmp = decl->child[i]; + UNSERIALIZE_PTR(tmp); + zend_file_cache_serialize_ast(tmp, script, info, buf); + } + } } else { uint32_t children = zend_ast_get_num_children(ast); for (i = 0; i < children; i++) { @@ -1248,6 +1258,14 @@ static void zend_file_cache_unserialize_ast(zend_ast *ast, } else if (ast->kind == ZEND_AST_OP_ARRAY) { /* The op_array itself will be unserialized as part of the dynamic_func_defs. */ UNSERIALIZE_PTR(Z_PTR(((zend_ast_zval*)ast)->val)); + } else if (zend_ast_is_decl(ast)) { + zend_ast_decl *decl = (zend_ast_decl*)ast; + for (i = 0; i < 5; i++) { + if (decl->child[i] && !IS_UNSERIALIZED(decl->child[i])) { + UNSERIALIZE_PTR(decl->child[i]); + zend_file_cache_unserialize_ast(decl->child[i], script, buf); + } + } } else { uint32_t children = zend_ast_get_num_children(ast); for (i = 0; i < children; i++) { diff --git a/ext/opcache/zend_persist.c b/ext/opcache/zend_persist.c index 11babd8aa4067..71bbf331ed998 100644 --- a/ext/opcache/zend_persist.c +++ b/ext/opcache/zend_persist.c @@ -192,6 +192,14 @@ static zend_ast *zend_persist_ast(zend_ast *ast) zend_ast_zval *copy = zend_shared_memdup(ast, sizeof(zend_ast_zval)); zend_persist_op_array(©->val); node = (zend_ast *) copy; + } else if (zend_ast_is_decl(ast)) { + zend_ast_decl *copy = zend_shared_memdup(ast, sizeof(zend_ast_decl)); + for (i = 0; i < 5; i++) { + if (copy->child[i]) { + copy->child[i] = zend_persist_ast(copy->child[i]); + } + } + node = (zend_ast *) copy; } else { uint32_t children = zend_ast_get_num_children(ast); node = zend_shared_memdup(ast, zend_ast_size(children)); diff --git a/ext/opcache/zend_persist_calc.c b/ext/opcache/zend_persist_calc.c index fe58d3f2de277..7d859aefb35e4 100644 --- a/ext/opcache/zend_persist_calc.c +++ b/ext/opcache/zend_persist_calc.c @@ -89,6 +89,14 @@ static void zend_persist_ast_calc(zend_ast *ast) } else if (ast->kind == ZEND_AST_OP_ARRAY) { ADD_SIZE(sizeof(zend_ast_zval)); zend_persist_op_array_calc(&((zend_ast_zval*)(ast))->val); + } else if (zend_ast_is_decl(ast)) { + zend_ast_decl *decl = (zend_ast_decl*)ast; + ADD_SIZE(sizeof(zend_ast_decl)); + for (size_t i = 0; i < 5; i++) { + if (decl->child[i]) { + zend_persist_ast_calc(decl->child[i]); + } + } } else { uint32_t children = zend_ast_get_num_children(ast); ADD_SIZE(zend_ast_size(children)); From 45057db1568d8d9be5085ee976eb2fa48daf7277 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 11 Dec 2024 10:20:09 +0100 Subject: [PATCH 2/2] Preserve the original AST for closures in const-expressions This is necessary when stringifying a `ReflectionAttribute` that has a closure as a parameter. --- .../attributes_ast_printing_runtime.phpt | 24 ++++++++++++ Zend/zend_ast.c | 38 ++++++++++++++----- Zend/zend_ast.h | 20 +++++++++- Zend/zend_compile.c | 8 +--- ext/opcache/zend_file_cache.c | 12 +++++- ext/opcache/zend_persist.c | 8 +++- ext/opcache/zend_persist_calc.c | 7 +++- main/debug_gdb_scripts.c | 2 + scripts/gdb/php_gdb.py | 2 + 9 files changed, 99 insertions(+), 22 deletions(-) create mode 100644 Zend/tests/closure_const_expr/attributes_ast_printing_runtime.phpt diff --git a/Zend/tests/closure_const_expr/attributes_ast_printing_runtime.phpt b/Zend/tests/closure_const_expr/attributes_ast_printing_runtime.phpt new file mode 100644 index 0000000000000..85d5ae925d967 --- /dev/null +++ b/Zend/tests/closure_const_expr/attributes_ast_printing_runtime.phpt @@ -0,0 +1,24 @@ +--TEST-- +AST printing for closures in attributes at runtime +--FILE-- +getAttributes() as $attribute) { + echo $attribute; +} + +?> +--EXPECT-- +Attribute [ Attr ] { + - Arguments [1] { + Argument #0 [ static function ($foo) { + echo $foo; +} ] + } +} diff --git a/Zend/zend_ast.c b/Zend/zend_ast.c index 3505d335938c9..1c82a6667182f 100644 --- a/Zend/zend_ast.c +++ b/Zend/zend_ast.c @@ -100,6 +100,18 @@ ZEND_API zend_ast * ZEND_FASTCALL zend_ast_create_constant(zend_string *name, ze return (zend_ast *) ast; } +ZEND_API zend_ast * ZEND_FASTCALL zend_ast_create_op_array(zend_op_array *op_array, zend_ast *original_ast, zend_ast_attr attr) { + zend_ast_op_array *ast; + + ast = zend_ast_alloc(sizeof(zend_ast_op_array)); + ast->kind = ZEND_AST_OP_ARRAY; + ast->attr = attr; + ast->op_array = op_array; + ast->ast = original_ast; + + return (zend_ast *) ast; +} + ZEND_API zend_ast * ZEND_FASTCALL zend_ast_create_class_const_or_name(zend_ast *class_name, zend_ast *name) { zend_string *name_str = zend_ast_get_str(name); if (zend_string_equals_ci(name_str, ZSTR_KNOWN(ZEND_STR_CLASS))) { @@ -992,7 +1004,7 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate_inner( } case ZEND_AST_OP_ARRAY: { - zend_function *func = Z_PTR_P(&((zend_ast_zval*)(ast))->val); + zend_function *func = (zend_function *)zend_ast_get_op_array(ast)->op_array; zend_create_closure(result, func, scope, scope, NULL); return SUCCESS; @@ -1076,8 +1088,10 @@ static size_t ZEND_FASTCALL zend_ast_tree_size(zend_ast *ast) { size_t size; - if (ast->kind == ZEND_AST_ZVAL || ast->kind == ZEND_AST_CONSTANT || ast->kind == ZEND_AST_OP_ARRAY) { + if (ast->kind == ZEND_AST_ZVAL || ast->kind == ZEND_AST_CONSTANT) { size = sizeof(zend_ast_zval); + } else if (ast->kind == ZEND_AST_OP_ARRAY) { + size = sizeof(zend_ast_op_array) + zend_ast_tree_size(zend_ast_get_op_array(ast)->ast); } else if (zend_ast_is_list(ast)) { uint32_t i; zend_ast_list *list = zend_ast_get_list(ast); @@ -1143,12 +1157,14 @@ static void* ZEND_FASTCALL zend_ast_tree_copy(zend_ast *ast, void *buf) } } } else if (ast->kind == ZEND_AST_OP_ARRAY) { - zend_ast_zval *new = (zend_ast_zval*)buf; - new->kind = ZEND_AST_OP_ARRAY; - new->attr = ast->attr; - ZVAL_COPY(&new->val, &((zend_ast_zval *) ast)->val); - Z_LINENO(new->val) = zend_ast_get_lineno(ast); - buf = (void*)((char*)buf + sizeof(zend_ast_zval)); + zend_ast_op_array *old = zend_ast_get_op_array(ast); + zend_ast_op_array *new = (zend_ast_op_array*)buf; + new->kind = old->kind; + new->attr = old->attr; + new->op_array = old->op_array; + buf = (void*)((char*)buf + sizeof(zend_ast_op_array)); + new->ast = (zend_ast*)buf; + buf = zend_ast_tree_copy(old->ast, buf); } else if (zend_ast_is_decl(ast)) { zend_ast_decl *old = (zend_ast_decl*)ast; zend_ast_decl *new = (zend_ast_decl*)buf; @@ -1241,7 +1257,7 @@ ZEND_API void ZEND_FASTCALL zend_ast_destroy(zend_ast *ast) } else if (EXPECTED(ast->kind == ZEND_AST_CONSTANT)) { zend_string_release_ex(zend_ast_get_constant_name(ast), 0); } else if (EXPECTED(ast->kind == ZEND_AST_OP_ARRAY)) { - ZEND_ASSERT(!Z_REFCOUNTED(((zend_ast_zval*)(ast))->val)); + zend_ast_destroy(zend_ast_get_op_array(ast)->ast); } else if (zend_ast_is_decl(ast)) { zend_ast_decl *decl = (zend_ast_decl *) ast; @@ -1859,6 +1875,10 @@ static ZEND_COLD void zend_ast_export_ex(smart_str *str, zend_ast *ast, int prio smart_str_appendl(str, ZSTR_VAL(name), ZSTR_LEN(name)); break; } + case ZEND_AST_OP_ARRAY: { + zend_ast_export_ex(str, zend_ast_get_op_array(ast)->ast, priority, indent); + break; + } case ZEND_AST_CONSTANT_CLASS: smart_str_appendl(str, "__CLASS__", sizeof("__CLASS__")-1); break; diff --git a/Zend/zend_ast.h b/Zend/zend_ast.h index 1b31c7cd03e76..460b87e9bfe77 100644 --- a/Zend/zend_ast.h +++ b/Zend/zend_ast.h @@ -207,6 +207,15 @@ typedef struct _zend_ast_zval { zval val; } zend_ast_zval; +typedef struct _zend_op_array zend_op_array; + +typedef struct _zend_ast_op_array { + zend_ast_kind kind; + zend_ast_attr attr; + zend_op_array *op_array; + zend_ast *ast; +} zend_ast_op_array; + /* Separate structure for function and class declaration, as they need extra information. */ typedef struct _zend_ast_decl { zend_ast_kind kind; @@ -231,6 +240,8 @@ ZEND_API zend_ast * ZEND_FASTCALL zend_ast_create_zval_from_long(zend_long lval) ZEND_API zend_ast * ZEND_FASTCALL zend_ast_create_constant(zend_string *name, zend_ast_attr attr); ZEND_API zend_ast * ZEND_FASTCALL zend_ast_create_class_const_or_name(zend_ast *class_name, zend_ast *name); +ZEND_API zend_ast * ZEND_FASTCALL zend_ast_create_op_array(zend_op_array *op_array, zend_ast *original_ast, zend_ast_attr attr); + #if ZEND_AST_SPEC # define ZEND_AST_SPEC_CALL(name, ...) \ ZEND_EXPAND_VA(ZEND_AST_SPEC_CALL_(name, __VA_ARGS__, _n, _5, _4, _3, _2, _1, _0)(__VA_ARGS__)) @@ -353,6 +364,11 @@ static zend_always_inline zend_string *zend_ast_get_str(zend_ast *ast) { return Z_STR_P(zv); } +static zend_always_inline zend_ast_op_array *zend_ast_get_op_array(zend_ast *ast) { + ZEND_ASSERT(ast->kind == ZEND_AST_OP_ARRAY); + return (zend_ast_op_array *) ast; +} + static zend_always_inline zend_string *zend_ast_get_constant_name(zend_ast *ast) { ZEND_ASSERT(ast->kind == ZEND_AST_CONSTANT); ZEND_ASSERT(Z_TYPE(((zend_ast_zval *) ast)->val) == IS_STRING); @@ -367,9 +383,11 @@ static zend_always_inline uint32_t zend_ast_get_lineno(zend_ast *ast) { if (ast->kind == ZEND_AST_ZVAL) { zval *zv = zend_ast_get_zval(ast); return Z_LINENO_P(zv); - } else if (ast->kind == ZEND_AST_CONSTANT || ast->kind == ZEND_AST_OP_ARRAY) { + } else if (ast->kind == ZEND_AST_CONSTANT) { zval *zv = &((zend_ast_zval *) ast)->val; return Z_LINENO_P(zv); + } else if (ast->kind == ZEND_AST_OP_ARRAY) { + return zend_ast_get_op_array(ast)->ast->lineno; } else { return ast->lineno; } diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 9533ef8dfaa44..f7c5254487d80 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -11220,13 +11220,9 @@ static void zend_compile_const_expr_closure(zend_ast **ast_ptr) } znode node; - zend_op_array *op = zend_compile_func_decl(&node, *ast_ptr, FUNC_DECL_LEVEL_CONSTEXPR); + zend_op_array *op = zend_compile_func_decl(&node, (zend_ast*)closure_ast, FUNC_DECL_LEVEL_CONSTEXPR); - zend_ast_destroy(*ast_ptr); - zval z; - ZVAL_PTR(&z, op); - *ast_ptr = zend_ast_create_zval(&z); - (*ast_ptr)->kind = ZEND_AST_OP_ARRAY; + *ast_ptr = zend_ast_create_op_array(op, (zend_ast*)closure_ast, 0); } static void zend_compile_const_expr_args(zend_ast **ast_ptr) diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index c113c70634a7d..f8cc2bd53c70e 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -365,7 +365,12 @@ static void zend_file_cache_serialize_ast(zend_ast *ast, } } else if (ast->kind == ZEND_AST_OP_ARRAY) { /* The op_array itself will be serialized as part of the dynamic_func_defs. */ - SERIALIZE_PTR(Z_PTR(((zend_ast_zval*)ast)->val)); + SERIALIZE_PTR(((zend_ast_op_array*)ast)->op_array); + + SERIALIZE_PTR(((zend_ast_op_array*)ast)->ast); + tmp = ((zend_ast_op_array*)ast)->ast; + UNSERIALIZE_PTR(tmp); + zend_file_cache_serialize_ast(tmp, script, info, buf); } else if (zend_ast_is_decl(ast)) { zend_ast_decl *decl = (zend_ast_decl*)ast; for (i = 0; i < 5; i++) { @@ -1257,7 +1262,10 @@ static void zend_file_cache_unserialize_ast(zend_ast *ast, } } else if (ast->kind == ZEND_AST_OP_ARRAY) { /* The op_array itself will be unserialized as part of the dynamic_func_defs. */ - UNSERIALIZE_PTR(Z_PTR(((zend_ast_zval*)ast)->val)); + UNSERIALIZE_PTR(((zend_ast_op_array*)ast)->op_array); + + UNSERIALIZE_PTR(((zend_ast_op_array*)ast)->ast); + zend_file_cache_unserialize_ast(((zend_ast_op_array*)ast)->ast, script, buf); } else if (zend_ast_is_decl(ast)) { zend_ast_decl *decl = (zend_ast_decl*)ast; for (i = 0; i < 5; i++) { diff --git a/ext/opcache/zend_persist.c b/ext/opcache/zend_persist.c index 71bbf331ed998..57dc303068c98 100644 --- a/ext/opcache/zend_persist.c +++ b/ext/opcache/zend_persist.c @@ -189,8 +189,12 @@ static zend_ast *zend_persist_ast(zend_ast *ast) } node = (zend_ast *) copy; } else if (ast->kind == ZEND_AST_OP_ARRAY) { - zend_ast_zval *copy = zend_shared_memdup(ast, sizeof(zend_ast_zval)); - zend_persist_op_array(©->val); + zend_ast_op_array *copy = zend_shared_memdup(ast, sizeof(zend_ast_op_array)); + zval z; + ZVAL_PTR(&z, copy->op_array); + zend_persist_op_array(&z); + copy->op_array = Z_PTR(z); + copy->ast = zend_persist_ast(copy->ast); node = (zend_ast *) copy; } else if (zend_ast_is_decl(ast)) { zend_ast_decl *copy = zend_shared_memdup(ast, sizeof(zend_ast_decl)); diff --git a/ext/opcache/zend_persist_calc.c b/ext/opcache/zend_persist_calc.c index 7d859aefb35e4..bf0ab2bdd7dbd 100644 --- a/ext/opcache/zend_persist_calc.c +++ b/ext/opcache/zend_persist_calc.c @@ -87,8 +87,11 @@ static void zend_persist_ast_calc(zend_ast *ast) } } } else if (ast->kind == ZEND_AST_OP_ARRAY) { - ADD_SIZE(sizeof(zend_ast_zval)); - zend_persist_op_array_calc(&((zend_ast_zval*)(ast))->val); + ADD_SIZE(sizeof(zend_ast_op_array)); + zval z; + ZVAL_PTR(&z, zend_ast_get_op_array(ast)->op_array); + zend_persist_op_array_calc(&z); + zend_persist_ast_calc(zend_ast_get_op_array(ast)->ast); } else if (zend_ast_is_decl(ast)) { zend_ast_decl *decl = (zend_ast_decl*)ast; ADD_SIZE(sizeof(zend_ast_decl)); diff --git a/main/debug_gdb_scripts.c b/main/debug_gdb_scripts.c index 13288fc9adc8d..590a4f42abb97 100644 --- a/main/debug_gdb_scripts.c +++ b/main/debug_gdb_scripts.c @@ -860,6 +860,8 @@ asm( ".ascii \"\\n\"\n" ".ascii \" if kind == enum_value('ZEND_AST_ZVAL') or kind == enum_value('ZEND_AST_CONSTANT'):\\n\"\n" ".ascii \" return self.val.cast(gdb.lookup_type('zend_ast_zval'))\\n\"\n" + ".ascii \" if kind == enum_value('ZEND_AST_OP_ARRAY'):\\n\"\n" + ".ascii \" return self.val.cast(gdb.lookup_type('zend_ast_op_array'))\\n\"\n" ".ascii \" if kind == enum_value('ZEND_AST_ZNODE'):\\n\"\n" ".ascii \" return self.val.cast(gdb.lookup_type('zend_ast_znode'))\\n\"\n" ".ascii \" if self.is_decl():\\n\"\n" diff --git a/scripts/gdb/php_gdb.py b/scripts/gdb/php_gdb.py index b7258fb78f1a9..97f7e976b75c0 100644 --- a/scripts/gdb/php_gdb.py +++ b/scripts/gdb/php_gdb.py @@ -190,6 +190,8 @@ def cast(self): if kind == enum_value('ZEND_AST_ZVAL') or kind == enum_value('ZEND_AST_CONSTANT'): return self.val.cast(gdb.lookup_type('zend_ast_zval')) + if kind == enum_value('ZEND_AST_OP_ARRAY'): + return self.val.cast(gdb.lookup_type('zend_ast_op_array')) if kind == enum_value('ZEND_AST_ZNODE'): return self.val.cast(gdb.lookup_type('zend_ast_znode')) if self.is_decl():