From 5686c16db45dffb6e4898e0959f89f41cc7660f0 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 23 Sep 2020 13:01:07 +0200 Subject: Honor strict_types=1 for attributes, improve backtraces Make ReflectionAttribute::newInstance() respect the strict_types=1 declaration at the attribute use-site. More generally, pretend that we are calling the attribute constructor from the place where the attribute is used, which also means that the attribute location will show up properly in backtraces and inside "called in" error information. This requires us to store the attributes strict_types scope (as flags), as well as the attribute line number. The attribute filename can be recovered from the symbol it is used on. We might want to expose the attribute line number via reflection as well. See also https://externals.io/message/111915. Closes GH-6201. --- Zend/tests/attributes/005_objects.phpt | 6 +- Zend/tests/attributes/030_strict_types.inc | 5 ++ Zend/tests/attributes/030_strict_types.phpt | 31 +++++++++ Zend/tests/attributes/031_backtrace.phpt | 97 +++++++++++++++++++++++++++++ Zend/zend_attributes.c | 25 +++----- Zend/zend_attributes.h | 25 ++++++-- Zend/zend_compile.c | 5 +- ext/reflection/php_reflection.c | 78 ++++++++++++++++++----- 8 files changed, 231 insertions(+), 41 deletions(-) create mode 100644 Zend/tests/attributes/030_strict_types.inc create mode 100644 Zend/tests/attributes/030_strict_types.phpt create mode 100644 Zend/tests/attributes/031_backtrace.phpt diff --git a/Zend/tests/attributes/005_objects.phpt b/Zend/tests/attributes/005_objects.phpt index 62b14181ef..db0107500c 100644 --- a/Zend/tests/attributes/005_objects.phpt +++ b/Zend/tests/attributes/005_objects.phpt @@ -96,16 +96,16 @@ try { } ?> ---EXPECT-- +--EXPECTF-- string(2) "A1" string(4) "test" int(50) string(7) "ERROR 1" -string(81) "Too few arguments to function A1::__construct(), 0 passed and at least 1 expected" +string(%d) "Too few arguments to function A1::__construct(), 0 passed in %s005_objects.php on line 26 and at least 1 expected" string(7) "ERROR 2" -string(74) "A1::__construct(): Argument #1 ($name) must be of type string, array given" +string(%d) "A1::__construct(): Argument #1 ($name) must be of type string, array given, called in %s005_objects.php on line 36" string(7) "ERROR 3" string(30) "Attribute class "A2" not found" diff --git a/Zend/tests/attributes/030_strict_types.inc b/Zend/tests/attributes/030_strict_types.inc new file mode 100644 index 0000000000..d9494e4fce --- /dev/null +++ b/Zend/tests/attributes/030_strict_types.inc @@ -0,0 +1,5 @@ +getAttributes()[0]->newInstance()); +var_dump((new ReflectionClass(TestStrict::class))->getAttributes()[0]->newInstance()); + +?> +--EXPECTF-- +object(MyAttribute)#1 (1) { + ["value"]=> + int(42) +} + +Fatal error: Uncaught TypeError: MyAttribute::__construct(): Argument #1 ($value) must be of type int, string given, called in %s030_strict_types.inc on line 4 and defined in %s030_strict_types.php:5 +Stack trace: +#0 %s030_strict_types.inc(4): MyAttribute->__construct('42') +#1 %s(%d): ReflectionAttribute->newInstance() +#2 {main} + thrown in %s on line %d diff --git a/Zend/tests/attributes/031_backtrace.phpt b/Zend/tests/attributes/031_backtrace.phpt new file mode 100644 index 0000000000..b0374e67e2 --- /dev/null +++ b/Zend/tests/attributes/031_backtrace.phpt @@ -0,0 +1,97 @@ +--TEST-- +Backtrace during attribute instance creation +--FILE-- +getTrace()); + } +} + +#[MyAttribute] +class Test {} + +(new ReflectionClass(Test::class))->getAttributes()[0]->newInstance(); + +?> +--EXPECTF-- +#0 MyAttribute->__construct() called at [%s031_backtrace.php:12] +#1 ReflectionAttribute->newInstance() called at [%s:%d] +array(2) { + [0]=> + array(7) { + ["file"]=> + string(%d) "%s031_backtrace.php" + ["line"]=> + int(12) + ["function"]=> + string(11) "__construct" + ["class"]=> + string(11) "MyAttribute" + ["object"]=> + object(MyAttribute)#1 (0) { + } + ["type"]=> + string(2) "->" + ["args"]=> + array(0) { + } + } + [1]=> + array(7) { + ["file"]=> + string(%d) "%s" + ["line"]=> + int(%d) + ["function"]=> + string(11) "newInstance" + ["class"]=> + string(19) "ReflectionAttribute" + ["object"]=> + object(ReflectionAttribute)#2 (0) { + } + ["type"]=> + string(2) "->" + ["args"]=> + array(0) { + } + } +} +array(2) { + [0]=> + array(6) { + ["file"]=> + string(%d) "%s031_backtrace.php" + ["line"]=> + int(12) + ["function"]=> + string(11) "__construct" + ["class"]=> + string(11) "MyAttribute" + ["type"]=> + string(2) "->" + ["args"]=> + array(0) { + } + } + [1]=> + array(6) { + ["file"]=> + string(%d) "%s" + ["line"]=> + int(%d) + ["function"]=> + string(11) "newInstance" + ["class"]=> + string(19) "ReflectionAttribute" + ["type"]=> + string(2) "->" + ["args"]=> + array(0) { + } + } +} diff --git a/Zend/zend_attributes.c b/Zend/zend_attributes.c index af5baa6ce0..29a2f4a732 100644 --- a/Zend/zend_attributes.c +++ b/Zend/zend_attributes.c @@ -175,38 +175,29 @@ ZEND_API zend_bool zend_is_attribute_repeated(HashTable *attributes, zend_attrib return 0; } -static zend_always_inline void free_attribute(zend_attribute *attr, bool persistent) +static void attr_free(zval *v) { - uint32_t i; + zend_attribute *attr = Z_PTR_P(v); zend_string_release(attr->name); zend_string_release(attr->lcname); - for (i = 0; i < attr->argc; i++) { + for (uint32_t i = 0; i < attr->argc; i++) { if (attr->args[i].name) { zend_string_release(attr->args[i].name); } zval_ptr_dtor(&attr->args[i].value); } - pefree(attr, persistent); -} - -static void attr_free(zval *v) -{ - free_attribute((zend_attribute *) Z_PTR_P(v), 0); + pefree(attr, attr->flags & ZEND_ATTRIBUTE_PERSISTENT); } -static void attr_pfree(zval *v) -{ - free_attribute((zend_attribute *) Z_PTR_P(v), 1); -} - -ZEND_API zend_attribute *zend_add_attribute(HashTable **attributes, zend_bool persistent, uint32_t offset, zend_string *name, uint32_t argc) +ZEND_API zend_attribute *zend_add_attribute(HashTable **attributes, zend_string *name, uint32_t argc, uint32_t flags, uint32_t offset, uint32_t lineno) { + bool persistent = flags & ZEND_ATTRIBUTE_PERSISTENT; if (*attributes == NULL) { *attributes = pemalloc(sizeof(HashTable), persistent); - zend_hash_init(*attributes, 8, NULL, persistent ? attr_pfree : attr_free, persistent); + zend_hash_init(*attributes, 8, NULL, attr_free, persistent); } zend_attribute *attr = pemalloc(ZEND_ATTRIBUTE_SIZE(argc), persistent); @@ -218,6 +209,8 @@ ZEND_API zend_attribute *zend_add_attribute(HashTable **attributes, zend_bool pe } attr->lcname = zend_string_tolower_ex(attr->name, persistent); + attr->flags = flags; + attr->lineno = lineno; attr->offset = offset; attr->argc = argc; diff --git a/Zend/zend_attributes.h b/Zend/zend_attributes.h index 6a60b06665..d19b7e470d 100644 --- a/Zend/zend_attributes.h +++ b/Zend/zend_attributes.h @@ -30,6 +30,10 @@ #define ZEND_ATTRIBUTE_IS_REPEATABLE (1<<6) #define ZEND_ATTRIBUTE_FLAGS ((1<<7) - 1) +/* Flags for zend_attribute.flags */ +#define ZEND_ATTRIBUTE_PERSISTENT (1<<0) +#define ZEND_ATTRIBUTE_STRICT_TYPES (1<<1) + #define ZEND_ATTRIBUTE_SIZE(argc) \ (sizeof(zend_attribute) + sizeof(zend_attribute_arg) * (argc) - sizeof(zend_attribute_arg)) @@ -45,6 +49,8 @@ typedef struct { typedef struct _zend_attribute { zend_string *name; zend_string *lcname; + uint32_t flags; + uint32_t lineno; /* Parameter offsets start at 1, everything else uses 0. */ uint32_t offset; uint32_t argc; @@ -71,33 +77,40 @@ ZEND_API zend_bool zend_is_attribute_repeated(HashTable *attributes, zend_attrib ZEND_API zend_internal_attribute *zend_internal_attribute_register(zend_class_entry *ce, uint32_t flags); ZEND_API zend_internal_attribute *zend_internal_attribute_get(zend_string *lcname); -ZEND_API zend_attribute *zend_add_attribute(HashTable **attributes, zend_bool persistent, uint32_t offset, zend_string *name, uint32_t argc); +ZEND_API zend_attribute *zend_add_attribute( + HashTable **attributes, zend_string *name, uint32_t argc, + uint32_t flags, uint32_t offset, uint32_t lineno); END_EXTERN_C() static zend_always_inline zend_attribute *zend_add_class_attribute(zend_class_entry *ce, zend_string *name, uint32_t argc) { - return zend_add_attribute(&ce->attributes, ce->type != ZEND_USER_CLASS, 0, name, argc); + uint32_t flags = ce->type != ZEND_USER_CLASS ? ZEND_ATTRIBUTE_PERSISTENT : 0; + return zend_add_attribute(&ce->attributes, name, argc, flags, 0, 0); } static zend_always_inline zend_attribute *zend_add_function_attribute(zend_function *func, zend_string *name, uint32_t argc) { - return zend_add_attribute(&func->common.attributes, func->common.type != ZEND_USER_FUNCTION, 0, name, argc); + uint32_t flags = func->common.type != ZEND_USER_FUNCTION ? ZEND_ATTRIBUTE_PERSISTENT : 0; + return zend_add_attribute(&func->common.attributes, name, argc, flags, 0, 0); } static zend_always_inline zend_attribute *zend_add_parameter_attribute(zend_function *func, uint32_t offset, zend_string *name, uint32_t argc) { - return zend_add_attribute(&func->common.attributes, func->common.type != ZEND_USER_FUNCTION, offset + 1, name, argc); + uint32_t flags = func->common.type != ZEND_USER_FUNCTION ? ZEND_ATTRIBUTE_PERSISTENT : 0; + return zend_add_attribute(&func->common.attributes, name, argc, flags, offset + 1, 0); } static zend_always_inline zend_attribute *zend_add_property_attribute(zend_class_entry *ce, zend_property_info *info, zend_string *name, uint32_t argc) { - return zend_add_attribute(&info->attributes, ce->type != ZEND_USER_CLASS, 0, name, argc); + uint32_t flags = ce->type != ZEND_USER_CLASS ? ZEND_ATTRIBUTE_PERSISTENT : 0; + return zend_add_attribute(&info->attributes, name, argc, flags, 0, 0); } static zend_always_inline zend_attribute *zend_add_class_constant_attribute(zend_class_entry *ce, zend_class_constant *c, zend_string *name, uint32_t argc) { - return zend_add_attribute(&c->attributes, ce->type != ZEND_USER_CLASS, 0, name, argc); + uint32_t flags = ce->type != ZEND_USER_CLASS ? ZEND_ATTRIBUTE_PERSISTENT : 0; + return zend_add_attribute(&c->attributes, name, argc, flags, 0, 0); } void zend_register_attribute_ce(void); diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index ba6790a042..cc03a60e5f 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -6221,7 +6221,10 @@ static void zend_compile_attributes(HashTable **attributes, zend_ast *ast, uint3 zend_string *name = zend_resolve_class_name_ast(el->child[0]); zend_ast_list *args = el->child[1] ? zend_ast_get_list(el->child[1]) : NULL; - attr = zend_add_attribute(attributes, 0, offset, name, args ? args->children : 0); + uint32_t flags = (CG(active_op_array)->fn_flags & ZEND_ACC_STRICT_TYPES) + ? ZEND_ATTRIBUTE_STRICT_TYPES : 0; + attr = zend_add_attribute( + attributes, name, args ? args->children : 0, flags, offset, el->lineno); zend_string_release(name); /* Populate arguments */ diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 470b52e6c7..d4f89be69f 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -142,6 +142,7 @@ typedef struct _attribute_reference { HashTable *attributes; zend_attribute *data; zend_class_entry *scope; + zend_string *filename; uint32_t target; } attribute_reference; @@ -252,9 +253,14 @@ static void reflection_free_objects_storage(zend_object *object) /* {{{ */ zend_string_release_ex(prop_reference->unmangled_name, 0); efree(intern->ptr); break; - case REF_TYPE_ATTRIBUTE: + case REF_TYPE_ATTRIBUTE: { + attribute_reference *attr_ref = intern->ptr; + if (attr_ref->filename) { + zend_string_release(attr_ref->filename); + } efree(intern->ptr); break; + } case REF_TYPE_GENERATOR: case REF_TYPE_CLASS_CONSTANT: case REF_TYPE_OTHER: @@ -1084,7 +1090,7 @@ static void _extension_string(smart_str *str, zend_module_entry *module, char *i /* {{{ reflection_attribute_factory */ static void reflection_attribute_factory(zval *object, HashTable *attributes, zend_attribute *data, - zend_class_entry *scope, uint32_t target) + zend_class_entry *scope, uint32_t target, zend_string *filename) { reflection_object *intern; attribute_reference *reference; @@ -1095,6 +1101,7 @@ static void reflection_attribute_factory(zval *object, HashTable *attributes, ze reference->attributes = attributes; reference->data = data; reference->scope = scope; + reference->filename = filename ? zend_string_copy(filename) : NULL; reference->target = target; intern->ptr = reference; intern->ref_type = REF_TYPE_ATTRIBUTE; @@ -1102,7 +1109,7 @@ static void reflection_attribute_factory(zval *object, HashTable *attributes, ze /* }}} */ static int read_attributes(zval *ret, HashTable *attributes, zend_class_entry *scope, - uint32_t offset, uint32_t target, zend_string *name, zend_class_entry *base) /* {{{ */ + uint32_t offset, uint32_t target, zend_string *name, zend_class_entry *base, zend_string *filename) /* {{{ */ { ZEND_ASSERT(attributes != NULL); @@ -1115,7 +1122,7 @@ static int read_attributes(zval *ret, HashTable *attributes, zend_class_entry *s ZEND_HASH_FOREACH_PTR(attributes, attr) { if (attr->offset == offset && zend_string_equals(attr->lcname, filter)) { - reflection_attribute_factory(&tmp, attributes, attr, scope, target); + reflection_attribute_factory(&tmp, attributes, attr, scope, target, filename); add_next_index_zval(ret, &tmp); } } ZEND_HASH_FOREACH_END(); @@ -1147,7 +1154,7 @@ static int read_attributes(zval *ret, HashTable *attributes, zend_class_entry *s } } - reflection_attribute_factory(&tmp, attributes, attr, scope, target); + reflection_attribute_factory(&tmp, attributes, attr, scope, target, filename); add_next_index_zval(ret, &tmp); } ZEND_HASH_FOREACH_END(); @@ -1156,7 +1163,7 @@ static int read_attributes(zval *ret, HashTable *attributes, zend_class_entry *s /* }}} */ static void reflect_attributes(INTERNAL_FUNCTION_PARAMETERS, HashTable *attributes, - uint32_t offset, zend_class_entry *scope, uint32_t target) /* {{{ */ + uint32_t offset, zend_class_entry *scope, uint32_t target, zend_string *filename) /* {{{ */ { zend_string *name = NULL; zend_long flags = 0; @@ -1189,7 +1196,7 @@ static void reflect_attributes(INTERNAL_FUNCTION_PARAMETERS, HashTable *attribut array_init(return_value); - if (FAILURE == read_attributes(return_value, attributes, scope, offset, target, name, base)) { + if (FAILURE == read_attributes(return_value, attributes, scope, offset, target, name, base, filename)) { RETURN_THROWS(); } } @@ -1760,7 +1767,9 @@ ZEND_METHOD(ReflectionFunctionAbstract, getAttributes) target = ZEND_ATTRIBUTE_TARGET_FUNCTION; } - reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, fptr->common.attributes, 0, fptr->common.scope, target); + reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, + fptr->common.attributes, 0, fptr->common.scope, target, + fptr->type == ZEND_USER_FUNCTION ? fptr->op_array.filename : NULL); } /* }}} */ @@ -2660,7 +2669,9 @@ ZEND_METHOD(ReflectionParameter, getAttributes) HashTable *attributes = param->fptr->common.attributes; zend_class_entry *scope = param->fptr->common.scope; - reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, attributes, param->offset + 1, scope, ZEND_ATTRIBUTE_TARGET_PARAMETER); + reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, + attributes, param->offset + 1, scope, ZEND_ATTRIBUTE_TARGET_PARAMETER, + param->fptr->type == ZEND_USER_FUNCTION ? param->fptr->op_array.filename : NULL); } /* {{{ Returns whether this parameter is an optional parameter */ @@ -3679,7 +3690,9 @@ ZEND_METHOD(ReflectionClassConstant, getAttributes) GET_REFLECTION_OBJECT_PTR(ref); - reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, ref->attributes, 0, ref->ce, ZEND_ATTRIBUTE_TARGET_CLASS_CONST); + reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, + ref->attributes, 0, ref->ce, ZEND_ATTRIBUTE_TARGET_CLASS_CONST, + ref->ce->type == ZEND_USER_CLASS ? ref->ce->info.user.filename : NULL); } /* }}} */ @@ -4076,7 +4089,9 @@ ZEND_METHOD(ReflectionClass, getAttributes) GET_REFLECTION_OBJECT_PTR(ce); - reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, ce->attributes, 0, ce, ZEND_ATTRIBUTE_TARGET_CLASS); + reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, + ce->attributes, 0, ce, ZEND_ATTRIBUTE_TARGET_CLASS, + ce->type == ZEND_USER_CLASS ? ce->info.user.filename : NULL); } /* }}} */ @@ -5465,7 +5480,9 @@ ZEND_METHOD(ReflectionProperty, getAttributes) GET_REFLECTION_OBJECT_PTR(ref); - reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, ref->prop->attributes, 0, ref->prop->ce, ZEND_ATTRIBUTE_TARGET_PROPERTY); + reflect_attributes(INTERNAL_FUNCTION_PARAM_PASSTHRU, + ref->prop->attributes, 0, ref->prop->ce, ZEND_ATTRIBUTE_TARGET_PROPERTY, + ref->prop->ce->type == ZEND_USER_CLASS ? ref->prop->ce->info.user.filename : NULL); } /* }}} */ @@ -6238,9 +6255,14 @@ ZEND_METHOD(ReflectionAttribute, getArguments) } /* }}} */ -static int call_attribute_constructor(zend_class_entry *ce, zend_object *obj, zval *args, uint32_t argc, HashTable *named_params) /* {{{ */ +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 *prev_execute_data, dummy_frame; + zend_function dummy_func; + zend_op dummy_opline; ZEND_ASSERT(ctor != NULL); if (!(ctor->common.fn_flags & ZEND_ACC_PUBLIC)) { @@ -6248,8 +6270,35 @@ static int call_attribute_constructor(zend_class_entry *ce, zend_object *obj, zv 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. */ + memset(&dummy_frame, 0, sizeof(zend_execute_data)); + memset(&dummy_func, 0, sizeof(zend_function)); + memset(&dummy_opline, 0, sizeof(zend_op)); + + prev_execute_data = EG(current_execute_data); + dummy_frame.prev_execute_data = prev_execute_data; + dummy_frame.func = &dummy_func; + dummy_frame.opline = &dummy_opline; + + dummy_func.type = ZEND_USER_FUNCTION; + dummy_func.common.fn_flags = + attr->flags & ZEND_ATTRIBUTE_STRICT_TYPES ? ZEND_ACC_STRICT_TYPES : 0; + dummy_func.op_array.filename = filename; + + dummy_opline.opcode = ZEND_DO_FCALL; + dummy_opline.lineno = attr->lineno; + + EG(current_execute_data) = &dummy_frame; + } + zend_call_known_function(ctor, obj, obj->ce, NULL, argc, args, named_params); + if (filename) { + EG(current_execute_data) = prev_execute_data; + } + if (EG(exception)) { zend_object_store_ctor_failed(obj); return FAILURE; @@ -6257,7 +6306,6 @@ static int call_attribute_constructor(zend_class_entry *ce, zend_object *obj, zv return SUCCESS; } -/* }}} */ static void attribute_ctor_cleanup( zval *obj, zval *args, uint32_t argc, HashTable *named_params) /* {{{ */ @@ -6373,7 +6421,7 @@ ZEND_METHOD(ReflectionAttribute, newInstance) } if (ce->constructor) { - if (FAILURE == call_attribute_constructor(ce, Z_OBJ(obj), args, argc, named_params)) { + 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(); } -- cgit v1.2.1