From 9f833e03257169113f8fcf2bb3ffde8de287eeb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Tue, 7 May 2024 09:36:40 +0200 Subject: [PATCH 1/3] Add `zend_get_attribute_object()` This makes the implementation for `ReflectionAttribute::newInstance()` reusable. --- Zend/zend_attributes.c | 131 ++++++++++++++++++++++++++++++++ Zend/zend_attributes.h | 1 + ext/reflection/php_reflection.c | 127 +------------------------------ 3 files changed, 134 insertions(+), 125 deletions(-) diff --git a/Zend/zend_attributes.c b/Zend/zend_attributes.c index 56bcf668e125a..cc3f62be6ae81 100644 --- a/Zend/zend_attributes.c +++ b/Zend/zend_attributes.c @@ -214,6 +214,137 @@ ZEND_API zend_result zend_get_attribute_value(zval *ret, zend_attribute *attr, u return SUCCESS; } +static int call_attribute_constructor( + zend_attribute *attr, zend_class_entry *ce, zend_object *obj, + zval *args, uint32_t argc, HashTable *named_params, zend_string *filename) +{ + zend_function *ctor = ce->constructor; + zend_execute_data *call = NULL; + ZEND_ASSERT(ctor != NULL); + + if (!(ctor->common.fn_flags & ZEND_ACC_PUBLIC)) { + zend_throw_error(NULL, "Attribute constructor of class %s must be public", ZSTR_VAL(ce->name)); + return FAILURE; + } + + if (filename) { + /* Set up dummy call frame that makes it look like the attribute was invoked + * from where it occurs in the code. */ + zend_function dummy_func; + zend_op *opline; + + memset(&dummy_func, 0, sizeof(zend_function)); + + call = zend_vm_stack_push_call_frame_ex( + ZEND_MM_ALIGNED_SIZE_EX(sizeof(zend_execute_data), sizeof(zval)) + + ZEND_MM_ALIGNED_SIZE_EX(sizeof(zend_op), sizeof(zval)) + + ZEND_MM_ALIGNED_SIZE_EX(sizeof(zend_function), sizeof(zval)), + 0, &dummy_func, 0, NULL); + + opline = (zend_op*)(call + 1); + memset(opline, 0, sizeof(zend_op)); + opline->opcode = ZEND_DO_FCALL; + opline->lineno = attr->lineno; + + call->opline = opline; + call->call = NULL; + call->return_value = NULL; + call->func = (zend_function*)(call->opline + 1); + call->prev_execute_data = EG(current_execute_data); + + memset(call->func, 0, sizeof(zend_function)); + call->func->type = ZEND_USER_FUNCTION; + call->func->op_array.fn_flags = + attr->flags & ZEND_ATTRIBUTE_STRICT_TYPES ? ZEND_ACC_STRICT_TYPES : 0; + call->func->op_array.fn_flags |= ZEND_ACC_CALL_VIA_TRAMPOLINE; + call->func->op_array.filename = filename; + + EG(current_execute_data) = call; + } + + zend_call_known_function(ctor, obj, obj->ce, NULL, argc, args, named_params); + + if (filename) { + EG(current_execute_data) = call->prev_execute_data; + zend_vm_stack_free_call_frame(call); + } + + if (EG(exception)) { + zend_object_store_ctor_failed(obj); + return FAILURE; + } + + return SUCCESS; +} + +static void attribute_ctor_cleanup(zval *obj, zval *args, uint32_t argc, HashTable *named_params) +{ + if (obj) { + zval_ptr_dtor(obj); + } + + if (args) { + uint32_t i; + + for (i = 0; i < argc; i++) { + zval_ptr_dtor(&args[i]); + } + + efree(args); + } + + if (named_params) { + zend_array_destroy(named_params); + } +} + +ZEND_API zend_result zend_get_attribute_object(zval *obj, zend_class_entry *attribute_ce, zend_attribute *attribute_data, zend_class_entry *scope, zend_string *filename) +{ + zval *args = NULL; + HashTable *named_params = NULL; + + if (SUCCESS != object_init_ex(obj, attribute_ce)) { + return FAILURE; + } + + uint32_t argc = 0; + if (attribute_data->argc) { + args = emalloc(attribute_data->argc * sizeof(zval)); + + for (uint32_t i = 0; i < attribute_data->argc; i++) { + zval val; + if (FAILURE == zend_get_attribute_value(&val, attribute_data, i, scope)) { + attribute_ctor_cleanup(obj, args, argc, named_params); + return FAILURE; + } + if (attribute_data->args[i].name) { + if (!named_params) { + named_params = zend_new_array(0); + } + zend_hash_add_new(named_params, attribute_data->args[i].name, &val); + } else { + ZVAL_COPY_VALUE(&args[i], &val); + argc++; + } + } + } + + if (attribute_ce->constructor) { + if (FAILURE == call_attribute_constructor(attribute_data, attribute_ce, Z_OBJ_P(obj), args, argc, named_params, filename)) { + attribute_ctor_cleanup(obj, args, argc, named_params); + return FAILURE; + } + } else if (argc || named_params) { + attribute_ctor_cleanup(obj, args, argc, named_params); + zend_throw_error(NULL, "Attribute class %s does not have a constructor, cannot pass arguments", ZSTR_VAL(attribute_ce->name)); + return FAILURE; + } + + attribute_ctor_cleanup(NULL, args, argc, named_params); + + return SUCCESS; +} + static const char *target_names[] = { "class", "function", diff --git a/Zend/zend_attributes.h b/Zend/zend_attributes.h index 422ad8b1325f8..20a868191c6a4 100644 --- a/Zend/zend_attributes.h +++ b/Zend/zend_attributes.h @@ -74,6 +74,7 @@ ZEND_API zend_attribute *zend_get_parameter_attribute(HashTable *attributes, zen ZEND_API zend_attribute *zend_get_parameter_attribute_str(HashTable *attributes, const char *str, size_t len, uint32_t offset); ZEND_API zend_result zend_get_attribute_value(zval *ret, zend_attribute *attr, uint32_t i, zend_class_entry *scope); +ZEND_API zend_result zend_get_attribute_object(zval *out, zend_class_entry *attribute_ce, zend_attribute *attribute_data, zend_class_entry *scope, zend_string *filename); ZEND_API zend_string *zend_get_attribute_target_names(uint32_t targets); ZEND_API bool zend_is_attribute_repeated(HashTable *attributes, zend_attribute *attr); diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 1a97707a74eb4..990d1c83c9550 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -6727,92 +6727,6 @@ ZEND_METHOD(ReflectionAttribute, getArguments) } /* }}} */ -static int call_attribute_constructor( - zend_attribute *attr, zend_class_entry *ce, zend_object *obj, - zval *args, uint32_t argc, HashTable *named_params, zend_string *filename) -{ - zend_function *ctor = ce->constructor; - zend_execute_data *call = NULL; - ZEND_ASSERT(ctor != NULL); - - if (!(ctor->common.fn_flags & ZEND_ACC_PUBLIC)) { - zend_throw_error(NULL, "Attribute constructor of class %s must be public", ZSTR_VAL(ce->name)); - return FAILURE; - } - - if (filename) { - /* Set up dummy call frame that makes it look like the attribute was invoked - * from where it occurs in the code. */ - zend_function dummy_func; - zend_op *opline; - - memset(&dummy_func, 0, sizeof(zend_function)); - - call = zend_vm_stack_push_call_frame_ex( - ZEND_MM_ALIGNED_SIZE_EX(sizeof(zend_execute_data), sizeof(zval)) + - ZEND_MM_ALIGNED_SIZE_EX(sizeof(zend_op), sizeof(zval)) + - ZEND_MM_ALIGNED_SIZE_EX(sizeof(zend_function), sizeof(zval)), - 0, &dummy_func, 0, NULL); - - opline = (zend_op*)(call + 1); - memset(opline, 0, sizeof(zend_op)); - opline->opcode = ZEND_DO_FCALL; - opline->lineno = attr->lineno; - - call->opline = opline; - call->call = NULL; - call->return_value = NULL; - call->func = (zend_function*)(call->opline + 1); - call->prev_execute_data = EG(current_execute_data); - - memset(call->func, 0, sizeof(zend_function)); - call->func->type = ZEND_USER_FUNCTION; - call->func->op_array.fn_flags = - attr->flags & ZEND_ATTRIBUTE_STRICT_TYPES ? ZEND_ACC_STRICT_TYPES : 0; - call->func->op_array.fn_flags |= ZEND_ACC_CALL_VIA_TRAMPOLINE; - call->func->op_array.filename = filename; - - EG(current_execute_data) = call; - } - - zend_call_known_function(ctor, obj, obj->ce, NULL, argc, args, named_params); - - if (filename) { - EG(current_execute_data) = call->prev_execute_data; - zend_vm_stack_free_call_frame(call); - } - - if (EG(exception)) { - zend_object_store_ctor_failed(obj); - return FAILURE; - } - - return SUCCESS; -} - -static void attribute_ctor_cleanup( - zval *obj, zval *args, uint32_t argc, HashTable *named_params) /* {{{ */ -{ - if (obj) { - zval_ptr_dtor(obj); - } - - if (args) { - uint32_t i; - - for (i = 0; i < argc; i++) { - zval_ptr_dtor(&args[i]); - } - - efree(args); - } - - if (named_params) { - zend_array_destroy(named_params); - } -} -/* }}} */ - /* {{{ Returns the attribute as an object */ ZEND_METHOD(ReflectionAttribute, newInstance) { @@ -6821,10 +6735,6 @@ ZEND_METHOD(ReflectionAttribute, newInstance) zend_attribute *marker; zend_class_entry *ce; - zval obj; - - zval *args = NULL; - HashTable *named_params = NULL; if (zend_parse_parameters_none() == FAILURE) { RETURN_THROWS(); @@ -6870,45 +6780,12 @@ ZEND_METHOD(ReflectionAttribute, newInstance) } } - if (SUCCESS != object_init_ex(&obj, ce)) { - RETURN_THROWS(); - } - - uint32_t argc = 0; - if (attr->data->argc) { - args = emalloc(attr->data->argc * sizeof(zval)); - - for (uint32_t i = 0; i < attr->data->argc; i++) { - zval val; - if (FAILURE == zend_get_attribute_value(&val, attr->data, i, attr->scope)) { - attribute_ctor_cleanup(&obj, args, argc, named_params); - RETURN_THROWS(); - } - if (attr->data->args[i].name) { - if (!named_params) { - named_params = zend_new_array(0); - } - zend_hash_add_new(named_params, attr->data->args[i].name, &val); - } else { - ZVAL_COPY_VALUE(&args[i], &val); - argc++; - } - } - } + zval obj; - if (ce->constructor) { - if (FAILURE == call_attribute_constructor(attr->data, ce, Z_OBJ(obj), args, argc, named_params, attr->filename)) { - attribute_ctor_cleanup(&obj, args, argc, named_params); - RETURN_THROWS(); - } - } else if (argc || named_params) { - attribute_ctor_cleanup(&obj, args, argc, named_params); - zend_throw_error(NULL, "Attribute class %s does not have a constructor, cannot pass arguments", ZSTR_VAL(ce->name)); + if (SUCCESS != zend_get_attribute_object(&obj, ce, attr->data, attr->scope, attr->filename)) { RETURN_THROWS(); } - attribute_ctor_cleanup(NULL, args, argc, named_params); - RETURN_COPY_VALUE(&obj); } From 674530602f2360f9d8e1fe226a5d4a8e44542ca3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Tue, 7 May 2024 10:13:20 +0200 Subject: [PATCH 2/3] Add test for the stack trace behavior of ReflectionAttribute::newInstance() This test ensures that the `filename` parameter for the fake stack frame is functional. Without it, the stack trace would show `[internal function]` for frame `#0`. --- ...ectionAttribute_newInstance_exception.phpt | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 ext/reflection/tests/ReflectionAttribute_newInstance_exception.phpt diff --git a/ext/reflection/tests/ReflectionAttribute_newInstance_exception.phpt b/ext/reflection/tests/ReflectionAttribute_newInstance_exception.phpt new file mode 100644 index 0000000000000..a6f516bc414d0 --- /dev/null +++ b/ext/reflection/tests/ReflectionAttribute_newInstance_exception.phpt @@ -0,0 +1,29 @@ +--TEST-- +Exception handling in ReflectionAttribute::newInstance() +--FILE-- +getAttributes()[0]; + +var_dump($attribute->newInstance()); +?> +--EXPECTF-- +Fatal error: Uncaught Exception: Test in %s:6 +Stack trace: +#0 %sReflectionAttribute_newInstance_exception.php(11): A->__construct() +#1 %sReflectionAttribute_newInstance_exception.php(18): ReflectionAttribute->newInstance() +#2 {main} + thrown in %sReflectionAttribute_newInstance_exception.php on line 6 From d2c95e456e5eae52f08fd9ce0530dc9ad1346f3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Tue, 7 May 2024 16:15:06 +0200 Subject: [PATCH 3/3] Fix return type of `call_attribute_constructor` --- Zend/zend_attributes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Zend/zend_attributes.c b/Zend/zend_attributes.c index cc3f62be6ae81..fb12131bb00f3 100644 --- a/Zend/zend_attributes.c +++ b/Zend/zend_attributes.c @@ -214,7 +214,7 @@ ZEND_API zend_result zend_get_attribute_value(zval *ret, zend_attribute *attr, u return SUCCESS; } -static int call_attribute_constructor( +static zend_result call_attribute_constructor( zend_attribute *attr, zend_class_entry *ce, zend_object *obj, zval *args, uint32_t argc, HashTable *named_params, zend_string *filename) {