From 231d27bf5a6d807bb83083316045cd2913d70ab0 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Mon, 29 Jul 2024 18:20:16 +0200 Subject: [PATCH] Fix missing variance check for abstract set with asymmetric type Fixes GH-15140 --- NEWS | 2 + Zend/tests/property_hooks/gh15140.phpt | 17 +++++++ Zend/zend_inheritance.c | 64 ++++++++++++++++++++------ 3 files changed, 68 insertions(+), 15 deletions(-) create mode 100644 Zend/tests/property_hooks/gh15140.phpt diff --git a/NEWS b/NEWS index 403e014f78606..20bb9148200bf 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,8 @@ PHP NEWS . Updated build system scripts config.guess to 2024-07-27 and config.sub to 2024-05-27. (Peter Kokot) . Fixed bug GH-15240 (Infinite recursion in trait hook). (ilutov) + . Fixed bug GH-15140 (Missing variance check for abstract set with asymmetric + type). (ilutov) - Date: . Constants SUNFUNCS_RET_TIMESTAMP, SUNFUNCS_RET_STRING, and SUNFUNCS_RET_DOUBLE diff --git a/Zend/tests/property_hooks/gh15140.phpt b/Zend/tests/property_hooks/gh15140.phpt new file mode 100644 index 0000000000000..16a9ed342460f --- /dev/null +++ b/Zend/tests/property_hooks/gh15140.phpt @@ -0,0 +1,17 @@ +--TEST-- +GH-15140: Asymmetric set type cannot be satisfied by plain property +--FILE-- + +--EXPECTF-- +Fatal error: Set type of C::$prop must be supertype of string|int (as in interface I) in %s on line %d diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index ef333dc76131d..b43544766b848 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -1242,7 +1242,7 @@ static void do_inherit_method(zend_string *key, zend_function *parent, zend_clas } /* }}} */ -static inheritance_status property_types_compatible( +static inheritance_status full_property_types_compatible( const zend_property_info *parent_info, const zend_property_info *child_info, prop_variance variance) { if (ZEND_TYPE_PURE_MASK(parent_info->type) == ZEND_TYPE_PURE_MASK(child_info->type) @@ -1271,7 +1271,7 @@ static inheritance_status property_types_compatible( return INHERITANCE_UNRESOLVED; } -static void emit_incompatible_property_error( +static ZEND_COLD void emit_incompatible_property_error( const zend_property_info *child, const zend_property_info *parent, prop_variance variance) { zend_string *type_str = zend_type_to_string_resolved(parent->type, parent->ce); zend_error_noreturn(E_COMPILE_ERROR, @@ -1284,6 +1284,48 @@ static void emit_incompatible_property_error( ZSTR_VAL(parent->ce->name)); } +static ZEND_COLD void emit_set_hook_type_error(const zend_property_info *child, const zend_property_info *parent) +{ + zend_type set_type = parent->hooks[ZEND_PROPERTY_HOOK_SET]->common.arg_info[0].type; + zend_string *type_str = zend_type_to_string_resolved(set_type, parent->ce); + zend_error_noreturn(E_COMPILE_ERROR, + "Set type of %s::$%s must be supertype of %s (as in %s %s)", + ZSTR_VAL(child->ce->name), + zend_get_unmangled_property_name(child->name), + ZSTR_VAL(type_str), + zend_get_object_type_case(parent->ce, false), + ZSTR_VAL(parent->ce->name)); +} + +static inheritance_status verify_property_type_compatibility( + const zend_property_info *parent_info, + const zend_property_info *child_info, + prop_variance variance, + bool throw_on_error, + bool throw_on_unresolved +) { + inheritance_status result = full_property_types_compatible(parent_info, child_info, variance); + if ((result == INHERITANCE_ERROR && throw_on_error) || (result == INHERITANCE_UNRESOLVED && throw_on_unresolved)) { + emit_incompatible_property_error(child_info, parent_info, variance); + } + if (result != INHERITANCE_SUCCESS) { + return result; + } + if (parent_info->flags & ZEND_ACC_ABSTRACT) { + ZEND_ASSERT(parent_info->hooks); + if (parent_info->hooks[ZEND_PROPERTY_HOOK_SET] + && (!child_info->hooks || !child_info->hooks[ZEND_PROPERTY_HOOK_SET])) { + zend_type set_type = parent_info->hooks[ZEND_PROPERTY_HOOK_SET]->common.arg_info[0].type; + inheritance_status result = zend_perform_covariant_type_check( + parent_info->ce, set_type, child_info->ce, child_info->type); + if ((result == INHERITANCE_ERROR && throw_on_error) || (result == INHERITANCE_UNRESOLVED && throw_on_unresolved)) { + emit_set_hook_type_error(child_info, parent_info); + } + } + } + return INHERITANCE_SUCCESS; +} + static bool property_has_operation(zend_property_info *prop_info, zend_property_hook_kind kind) { return (!(prop_info->flags & ZEND_ACC_VIRTUAL) @@ -1427,11 +1469,8 @@ static void do_inherit_property(zend_property_info *parent_info, zend_string *ke prop_variance variance = prop_get_variance(parent_info); if (ZEND_TYPE_IS_SET(parent_info->type)) { - inheritance_status status = property_types_compatible( - parent_info, child_info, variance); - if (status == INHERITANCE_ERROR) { - emit_incompatible_property_error(child_info, parent_info, variance); - } + inheritance_status status = verify_property_type_compatibility( + parent_info, child_info, variance, true, false); if (status == INHERITANCE_UNRESOLVED) { add_property_compatibility_obligation(ce, child_info, parent_info, variance); } @@ -2747,7 +2786,7 @@ static void zend_do_traits_property_binding(zend_class_entry *ce, zend_class_ent } if ((colliding_prop->flags & flags_mask) == (flags & flags_mask) && - property_types_compatible(property_info, colliding_prop, PROP_INVARIANT) == INHERITANCE_SUCCESS + verify_property_type_compatibility(property_info, colliding_prop, PROP_INVARIANT, false, false) == INHERITANCE_SUCCESS ) { /* the flags are identical, thus, the properties may be compatible */ zval *op1, *op2; @@ -3099,11 +3138,7 @@ static void check_variance_obligation(variance_obligation *obligation) { } /* Either the compatibility check was successful or only threw a warning. */ } else if (obligation->type == OBLIGATION_PROPERTY_COMPATIBILITY) { - inheritance_status status = - property_types_compatible(obligation->parent_prop, obligation->child_prop, obligation->variance); - if (status != INHERITANCE_SUCCESS) { - emit_incompatible_property_error(obligation->child_prop, obligation->parent_prop, obligation->variance); - } + verify_property_type_compatibility(obligation->parent_prop, obligation->child_prop, obligation->variance, true, true); } else if (obligation->type == OBLIGATION_CLASS_CONSTANT_COMPATIBILITY) { inheritance_status status = class_constant_types_compatible(obligation->parent_const, obligation->child_const); @@ -3645,8 +3680,7 @@ static inheritance_status zend_can_early_bind(zend_class_entry *ce, zend_class_e if (zv) { zend_property_info *child_info = Z_PTR_P(zv); if (ZEND_TYPE_IS_SET(child_info->type)) { - inheritance_status status = property_types_compatible(parent_info, child_info, prop_get_variance(parent_info)); - ZEND_ASSERT(status != INHERITANCE_WARNING); + inheritance_status status = verify_property_type_compatibility(parent_info, child_info, prop_get_variance(parent_info), false, false); if (UNEXPECTED(status != INHERITANCE_SUCCESS)) { return status; }