From 127712bd936173f489d0b2b45cef74a60083c6a9 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 2 Jun 2021 10:02:56 +0200 Subject: [PATCH 01/20] Implement readonly properties --- Zend/tests/readonly_props/by_ref_foreach.phpt | 29 +++++++ .../readonly_props/initialization_scope.phpt | 72 ++++++++++++++++ Zend/tests/readonly_props/promotion.phpt | 41 ++++++++++ Zend/tests/readonly_props/readonly_const.phpt | 12 +++ .../readonly_containing_object.phpt | 65 +++++++++++++++ .../tests/readonly_props/readonly_method.phpt | 12 +++ .../readonly_props/readonly_method_trait.phpt | 12 +++ .../readonly_props/readonly_modification.phpt | 82 +++++++++++++++++++ .../readonly_props/readonly_to_readwrite.phpt | 16 ++++ .../readonly_props/readonly_trait_match.phpt | 19 +++++ .../readonly_trait_mismatch.phpt | 18 ++++ .../readonly_props/readonly_with_default.phpt | 19 +++++ .../readonly_props/readonly_without_type.phpt | 12 +++ .../readonly_props/readwrite_to_readonly.phpt | 15 ++++ Zend/tests/readonly_props/serialization.phpt | 32 ++++++++ Zend/tests/readonly_props/static.phpt | 12 +++ Zend/tests/readonly_props/unset.phpt | 64 +++++++++++++++ Zend/zend_ast.c | 3 + Zend/zend_compile.c | 34 ++++++-- Zend/zend_compile.h | 3 + Zend/zend_execute.c | 12 ++- Zend/zend_inheritance.c | 10 ++- Zend/zend_language_parser.y | 18 ++-- Zend/zend_language_scanner.l | 4 + Zend/zend_object_handlers.c | 79 ++++++++++++++++++ Zend/zend_vm_def.h | 17 +++- Zend/zend_vm_execute.h | 17 +++- ext/reflection/php_reflection.c | 8 +- ext/reflection/php_reflection.stub.php | 2 + ext/reflection/php_reflection_arginfo.h | 6 +- ext/reflection/tests/readonly_properties.phpt | 24 ++++++ ext/tokenizer/tokenizer_data.c | 2 + 32 files changed, 742 insertions(+), 29 deletions(-) create mode 100644 Zend/tests/readonly_props/by_ref_foreach.phpt create mode 100644 Zend/tests/readonly_props/initialization_scope.phpt create mode 100644 Zend/tests/readonly_props/promotion.phpt create mode 100644 Zend/tests/readonly_props/readonly_const.phpt create mode 100644 Zend/tests/readonly_props/readonly_containing_object.phpt create mode 100644 Zend/tests/readonly_props/readonly_method.phpt create mode 100644 Zend/tests/readonly_props/readonly_method_trait.phpt create mode 100644 Zend/tests/readonly_props/readonly_modification.phpt create mode 100644 Zend/tests/readonly_props/readonly_to_readwrite.phpt create mode 100644 Zend/tests/readonly_props/readonly_trait_match.phpt create mode 100644 Zend/tests/readonly_props/readonly_trait_mismatch.phpt create mode 100644 Zend/tests/readonly_props/readonly_with_default.phpt create mode 100644 Zend/tests/readonly_props/readonly_without_type.phpt create mode 100644 Zend/tests/readonly_props/readwrite_to_readonly.phpt create mode 100644 Zend/tests/readonly_props/serialization.phpt create mode 100644 Zend/tests/readonly_props/static.phpt create mode 100644 Zend/tests/readonly_props/unset.phpt create mode 100644 ext/reflection/tests/readonly_properties.phpt diff --git a/Zend/tests/readonly_props/by_ref_foreach.phpt b/Zend/tests/readonly_props/by_ref_foreach.phpt new file mode 100644 index 0000000000000..dfc1d17093719 --- /dev/null +++ b/Zend/tests/readonly_props/by_ref_foreach.phpt @@ -0,0 +1,29 @@ +--TEST-- +By-ref foreach over readonly property +--FILE-- +prop = 1; + } +} + +$test = new Test; + +// Okay, as foreach skips over uninitialized properties. +foreach ($test as &$prop) {} + +$test->init(); + +try { + foreach ($test as &$prop) {} +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} + +?> +--EXPECT-- +Cannot acquire reference to readonly property Test::$prop diff --git a/Zend/tests/readonly_props/initialization_scope.phpt b/Zend/tests/readonly_props/initialization_scope.phpt new file mode 100644 index 0000000000000..b01dacf2d42f4 --- /dev/null +++ b/Zend/tests/readonly_props/initialization_scope.phpt @@ -0,0 +1,72 @@ +--TEST-- +Initialization can only happen from private scope +--FILE-- +prop = 3; + } +} +class B extends A { + public function initProtected() { + $this->prop = 2; + } +} + +$test = new B; +try { + $test->prop = 1; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + $test->initProtected(); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} + +$test->initPrivate(); +var_dump($test->prop); + +// Rebinding bypass works. +$test = new B; +(function() { + $this->prop = 1; +})->bindTo($test, A::class)(); +var_dump($test->prop); + +class C extends A { + public readonly int $prop; +} + +$test = new C; +$test->initPrivate(); +var_dump($test->prop); + +class X { + public function initFromParent() { + $this->prop = 1; + } +} +class Y extends X { + public readonly int $prop; +} + +$test = new Y; +try { + $test->initFromParent(); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} + +?> +--EXPECT-- +Cannot initialize readonly property A::$prop from global scope +Cannot initialize readonly property A::$prop from scope B +int(3) +int(1) +int(3) +Cannot initialize readonly property Y::$prop from scope X diff --git a/Zend/tests/readonly_props/promotion.phpt b/Zend/tests/readonly_props/promotion.phpt new file mode 100644 index 0000000000000..ee83246083b3e --- /dev/null +++ b/Zend/tests/readonly_props/promotion.phpt @@ -0,0 +1,41 @@ +--TEST-- +Promoted readonly property +--FILE-- +x = 4.0; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +var_dump($point); + +?> +--EXPECT-- +object(Point)#1 (3) { + ["x"]=> + float(0) + ["y"]=> + float(0) + ["z"]=> + float(0) +} +Cannot modify readonly property Point::$x +object(Point)#1 (3) { + ["x"]=> + float(1) + ["y"]=> + float(2) + ["z"]=> + float(3) +} diff --git a/Zend/tests/readonly_props/readonly_const.phpt b/Zend/tests/readonly_props/readonly_const.phpt new file mode 100644 index 0000000000000..d46f092b4a7ce --- /dev/null +++ b/Zend/tests/readonly_props/readonly_const.phpt @@ -0,0 +1,12 @@ +--TEST-- +Class constants cannot be readonly +--FILE-- + +--EXPECTF-- +Fatal error: Cannot use 'readonly' as constant modifier in %s on line %d diff --git a/Zend/tests/readonly_props/readonly_containing_object.phpt b/Zend/tests/readonly_props/readonly_containing_object.phpt new file mode 100644 index 0000000000000..e82cef27e4c85 --- /dev/null +++ b/Zend/tests/readonly_props/readonly_containing_object.phpt @@ -0,0 +1,65 @@ +--TEST-- +Not-modifying a readonly property holding an object +--FILE-- +prop = $prop; + } +} + +$test = new Test(new stdClass); +$test->prop->foo = 1; +$test->prop->foo += 1; +$test->prop->foo++; +try { + $test->prop += 1; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + $test->prop++; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + --$test->prop; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +var_dump($test->prop); + +// Unfortunately this is allowed, but does not modify $test->prop. +$ref =& $test->prop; +$ref = new stdClass; +var_dump($test->prop); + +$test = new Test(new ArrayObject()); +$test->prop[] = []; +$test->prop[0][] = 1; +var_dump($test->prop); + +?> +--EXPECT-- +Unsupported operand types: stdClass + int +Cannot modify readonly property Test::$prop +Cannot modify readonly property Test::$prop +object(stdClass)#2 (1) { + ["foo"]=> + int(3) +} +object(stdClass)#5 (0) { +} +object(ArrayObject)#4 (1) { + ["storage":"ArrayObject":private]=> + array(1) { + [0]=> + array(1) { + [0]=> + int(1) + } + } +} diff --git a/Zend/tests/readonly_props/readonly_method.phpt b/Zend/tests/readonly_props/readonly_method.phpt new file mode 100644 index 0000000000000..7146f0f52bd9b --- /dev/null +++ b/Zend/tests/readonly_props/readonly_method.phpt @@ -0,0 +1,12 @@ +--TEST-- +Method cannot be readonly +--FILE-- + +--EXPECTF-- +Fatal error: Cannot use 'readonly' as method modifier in %s on line %d diff --git a/Zend/tests/readonly_props/readonly_method_trait.phpt b/Zend/tests/readonly_props/readonly_method_trait.phpt new file mode 100644 index 0000000000000..4d69ab2b4344d --- /dev/null +++ b/Zend/tests/readonly_props/readonly_method_trait.phpt @@ -0,0 +1,12 @@ +--TEST-- +Method cannot be readonly in trait alias +--FILE-- + +--EXPECTF-- +Fatal error: Cannot use 'readonly' as method modifier in %s on line %d diff --git a/Zend/tests/readonly_props/readonly_modification.phpt b/Zend/tests/readonly_props/readonly_modification.phpt new file mode 100644 index 0000000000000..5f2a0c9ca5f50 --- /dev/null +++ b/Zend/tests/readonly_props/readonly_modification.phpt @@ -0,0 +1,82 @@ +--TEST-- +Modifying a readonly property +--FILE-- +prop = 1; + $this->prop2 = []; + } +} + +function byRef(&$ref) {} + +$test = new Test; +var_dump($test->prop); // Read. +try { + $test->prop = 2; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + $test->prop += 1; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + $test->prop++; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + ++$test->prop; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + $ref =& $test->prop; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + $test->prop =& $ref; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + byRef($test->prop); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} + +var_dump($test->prop2); // Read. +try { + $test->prop2[] = 1; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + $test->prop2[0][] = 1; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} + +?> +--EXPECT-- +int(1) +Cannot modify readonly property Test::$prop +Cannot modify readonly property Test::$prop +Cannot modify readonly property Test::$prop +Cannot modify readonly property Test::$prop +Cannot modify readonly property Test::$prop +Cannot modify readonly property Test::$prop +Cannot modify readonly property Test::$prop +array(0) { +} +Cannot modify readonly property Test::$prop2 +Cannot modify readonly property Test::$prop2 diff --git a/Zend/tests/readonly_props/readonly_to_readwrite.phpt b/Zend/tests/readonly_props/readonly_to_readwrite.phpt new file mode 100644 index 0000000000000..79e7985e35fef --- /dev/null +++ b/Zend/tests/readonly_props/readonly_to_readwrite.phpt @@ -0,0 +1,16 @@ +--TEST-- +Can replace readonly with readwrite +--FILE-- + +===DONE=== +--EXPECT-- +===DONE=== diff --git a/Zend/tests/readonly_props/readonly_trait_match.phpt b/Zend/tests/readonly_props/readonly_trait_match.phpt new file mode 100644 index 0000000000000..00aa6349aa148 --- /dev/null +++ b/Zend/tests/readonly_props/readonly_trait_match.phpt @@ -0,0 +1,19 @@ +--TEST-- +Readonly match of imported trait properties (valid) +--FILE-- + +===DONE=== +--EXPECT-- +===DONE=== diff --git a/Zend/tests/readonly_props/readonly_trait_mismatch.phpt b/Zend/tests/readonly_props/readonly_trait_mismatch.phpt new file mode 100644 index 0000000000000..99fc6b5bf8cc2 --- /dev/null +++ b/Zend/tests/readonly_props/readonly_trait_mismatch.phpt @@ -0,0 +1,18 @@ +--TEST-- +Readonly mismatch of imported trait properties +--FILE-- + +--EXPECTF-- +Fatal error: T1 and T2 define the same property ($prop) in the composition of C. However, the definition differs and is considered incompatible. Class was composed in %s on line %d diff --git a/Zend/tests/readonly_props/readonly_with_default.phpt b/Zend/tests/readonly_props/readonly_with_default.phpt new file mode 100644 index 0000000000000..db8f79bd2d6d7 --- /dev/null +++ b/Zend/tests/readonly_props/readonly_with_default.phpt @@ -0,0 +1,19 @@ +--TEST-- +Readonly property with default value +--FILE-- +prop = 2; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} + +?> +--EXPECT-- +Cannot modify readonly property Test::$prop diff --git a/Zend/tests/readonly_props/readonly_without_type.phpt b/Zend/tests/readonly_props/readonly_without_type.phpt new file mode 100644 index 0000000000000..016edb3e57811 --- /dev/null +++ b/Zend/tests/readonly_props/readonly_without_type.phpt @@ -0,0 +1,12 @@ +--TEST-- +Readonly property without type +--FILE-- + +--EXPECTF-- +Fatal error: Readonly property Test::$prop must have type in %s on line %d diff --git a/Zend/tests/readonly_props/readwrite_to_readonly.phpt b/Zend/tests/readonly_props/readwrite_to_readonly.phpt new file mode 100644 index 0000000000000..f2133ca88b1fb --- /dev/null +++ b/Zend/tests/readonly_props/readwrite_to_readonly.phpt @@ -0,0 +1,15 @@ +--TEST-- +Cannot replace readwrite with readonly +--FILE-- + +--EXPECTF-- +Fatal error: Cannot redeclare non-readonly property A::$prop as readonly B::$prop in %s on line %d diff --git a/Zend/tests/readonly_props/serialization.phpt b/Zend/tests/readonly_props/serialization.phpt new file mode 100644 index 0000000000000..8182eef3fa5f0 --- /dev/null +++ b/Zend/tests/readonly_props/serialization.phpt @@ -0,0 +1,32 @@ +--TEST-- +Serialization of readonly properties +--FILE-- + +--EXPECT-- +string(30) "O:4:"Test":1:{s:4:"prop";i:1;}" +object(Test)#1 (1) { + ["prop"]=> + int(1) +} +object(Test)#1 (1) { + ["prop"]=> + int(2) +} +object(Test)#1 (1) { + ["prop"]=> + int(3) +} diff --git a/Zend/tests/readonly_props/static.phpt b/Zend/tests/readonly_props/static.phpt new file mode 100644 index 0000000000000..eeddcef88eb60 --- /dev/null +++ b/Zend/tests/readonly_props/static.phpt @@ -0,0 +1,12 @@ +--TEST-- +Readonly static property +--FILE-- + +--EXPECTF-- +Fatal error: Static property Test::$prop cannot be readonly in %s on line %d diff --git a/Zend/tests/readonly_props/unset.phpt b/Zend/tests/readonly_props/unset.phpt new file mode 100644 index 0000000000000..5bf2107a09be7 --- /dev/null +++ b/Zend/tests/readonly_props/unset.phpt @@ -0,0 +1,64 @@ +--TEST-- +Unset readonly property +--FILE-- +prop = $prop; + } +} + +$test = new Test(1); +try { + unset($test->prop); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} + +class Test2 { + public readonly int $prop; + + public function __construct() { + unset($this->prop); // Unset uninitialized. + unset($this->prop); // Unset unset. + } + + public function __get($name) { + // Lazy init. + echo __METHOD__, "\n"; + $this->prop = 1; + return $this->prop; + } +} + +$test = new Test2; +var_dump($test->prop); // Call __get. +var_dump($test->prop); // Don't call __get. +try { + unset($test->prop); // Unset initialized, illegal. +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} + +class Test3 { + public readonly int $prop; +} + +$test = new Test3; +try { + unset($test->prop); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} + +?> +--EXPECT-- +Cannot unset readonly property Test::$prop +Test2::__get +int(1) +int(1) +Cannot unset readonly property Test2::$prop +Cannot unset readonly property Test3::$prop from global scope diff --git a/Zend/zend_ast.c b/Zend/zend_ast.c index a2bbf2c9c5dc3..a829f6255ddd9 100644 --- a/Zend/zend_ast.c +++ b/Zend/zend_ast.c @@ -1757,6 +1757,9 @@ static ZEND_COLD void zend_ast_export_ex(smart_str *str, zend_ast *ast, int prio if (ast->attr & ZEND_ACC_STATIC) { smart_str_appends(str, "static "); } + if (ast->attr & ZEND_ACC_READONLY) { + smart_str_appends(str, "readonly "); + } if (type_ast) { zend_ast_export_type(str, type_ast, indent); diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index b89aefa4a39c2..ab8c42c8d5292 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -6598,8 +6598,7 @@ void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32_t fall zend_string *name = zval_make_interned_string(zend_ast_get_zval(var_ast)); bool is_ref = (param_ast->attr & ZEND_PARAM_REF) != 0; bool is_variadic = (param_ast->attr & ZEND_PARAM_VARIADIC) != 0; - uint32_t visibility = - param_ast->attr & (ZEND_ACC_PUBLIC|ZEND_ACC_PROTECTED|ZEND_ACC_PRIVATE); + uint32_t flags = param_ast->attr & (ZEND_ACC_PPP_MASK | ZEND_ACC_READONLY); znode var_node, default_node; zend_uchar opcode; @@ -6681,7 +6680,7 @@ void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32_t fall if (type_ast) { uint32_t default_type = *default_ast_ptr ? Z_TYPE(default_node.u.constant) : IS_UNDEF; - bool force_nullable = default_type == IS_NULL && !visibility; + bool force_nullable = default_type == IS_NULL && !flags; op_array->fn_flags |= ZEND_ACC_HAS_TYPE_HINTS; arg_info->type = zend_compile_typename(type_ast, force_nullable); @@ -6724,14 +6723,14 @@ void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32_t fall } uint32_t arg_info_flags = _ZEND_ARG_INFO_FLAGS(is_ref, is_variadic, /* is_tentative */ 0) - | (visibility ? _ZEND_IS_PROMOTED_BIT : 0); + | (flags ? _ZEND_IS_PROMOTED_BIT : 0); ZEND_TYPE_FULL_MASK(arg_info->type) |= arg_info_flags; if (opcode == ZEND_RECV) { opline->op2.num = type_ast ? ZEND_TYPE_FULL_MASK(arg_info->type) : MAY_BE_ANY; } - if (visibility) { + if (flags) { zend_op_array *op_array = CG(active_op_array); zend_class_entry *scope = op_array->scope; bool is_ctor = @@ -6778,7 +6777,7 @@ void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32_t fall zend_string *doc_comment = doc_comment_ast ? zend_string_copy(zend_ast_get_str(doc_comment_ast)) : NULL; zend_property_info *prop = zend_declare_typed_property( - scope, name, &default_value, visibility | ZEND_ACC_PROMOTED, doc_comment, type); + scope, name, &default_value, flags | ZEND_ACC_PROMOTED, doc_comment, type); if (attributes_ast) { zend_compile_attributes( &prop->attributes, attributes_ast, 0, ZEND_ATTRIBUTE_TARGET_PROPERTY); @@ -6799,9 +6798,8 @@ void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32_t fall for (i = 0; i < list->children; i++) { zend_ast *param_ast = list->child[i]; bool is_ref = (param_ast->attr & ZEND_PARAM_REF) != 0; - uint32_t visibility = - param_ast->attr & (ZEND_ACC_PUBLIC|ZEND_ACC_PROTECTED|ZEND_ACC_PRIVATE); - if (!visibility) { + uint32_t flags = param_ast->attr & (ZEND_ACC_PPP_MASK | ZEND_ACC_READONLY); + if (!flags) { continue; } @@ -7037,6 +7035,10 @@ zend_string *zend_begin_method_decl(zend_op_array *op_array, zend_string *name, zend_string *lcname; + if (fn_flags & ZEND_ACC_READONLY) { + zend_error(E_COMPILE_ERROR, "Cannot use 'readonly' as method modifier"); + } + if ((fn_flags & ZEND_ACC_PRIVATE) && (fn_flags & ZEND_ACC_FINAL) && !zend_is_constructor(name)) { zend_error(E_COMPILE_WARNING, "Private methods cannot be final as they are never overridden by other classes"); } @@ -7354,6 +7356,18 @@ void zend_compile_prop_decl(zend_ast *ast, zend_ast *type_ast, uint32_t flags, z ZVAL_UNDEF(&value_zv); } + if (flags & ZEND_ACC_READONLY) { + if (!ZEND_TYPE_IS_SET(type)) { + zend_error_noreturn(E_COMPILE_ERROR, "Readonly property %s::$%s must have type", + ZSTR_VAL(ce->name), ZSTR_VAL(name)); + } + if (flags & ZEND_ACC_STATIC) { + zend_error_noreturn(E_COMPILE_ERROR, + "Static property %s::$%s cannot be readonly", + ZSTR_VAL(ce->name), ZSTR_VAL(name)); + } + } + info = zend_declare_typed_property(ce, name, &value_zv, flags, doc_comment, type); if (attr_ast) { @@ -7381,6 +7395,8 @@ static void zend_check_const_and_trait_alias_attr(uint32_t attr, const char* ent zend_error_noreturn(E_COMPILE_ERROR, "Cannot use 'abstract' as %s modifier", entity); } else if (attr & ZEND_ACC_FINAL) { zend_error_noreturn(E_COMPILE_ERROR, "Cannot use 'final' as %s modifier", entity); + } else if (attr & ZEND_ACC_READONLY) { + zend_error_noreturn(E_COMPILE_ERROR, "Cannot use 'readonly' as %s modifier", entity); } } /* }}} */ diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h index d9eba0c7397f6..3ae7c81da0465 100644 --- a/Zend/zend_compile.h +++ b/Zend/zend_compile.h @@ -220,6 +220,9 @@ typedef struct _zend_oparray_context { #define ZEND_ACC_ABSTRACT (1 << 6) /* X | X | | */ #define ZEND_ACC_EXPLICIT_ABSTRACT_CLASS (1 << 6) /* X | | | */ /* | | | */ +/* Readonly property | | | */ +#define ZEND_ACC_READONLY (1 << 7) /* | | X | */ +/* | | | */ /* Immutable op_array and class_entries | | | */ /* (implemented only for lazy loading of op_arrays) | | | */ #define ZEND_ACC_IMMUTABLE (1 << 7) /* X | X | | */ diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 8c86b2e8c1f23..634de6026972b 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -1884,7 +1884,9 @@ static zend_never_inline void zend_post_incdec_overloaded_property(zend_object * object->handlers->write_property(object, name, &z_copy, cache_slot); OBJ_RELEASE(object); zval_ptr_dtor(&z_copy); - zval_ptr_dtor(z); + if (z == &rv) { + zval_ptr_dtor(z); + } } static zend_never_inline void zend_pre_incdec_overloaded_property(zend_object *object, zend_string *name, void **cache_slot OPLINE_DC EXECUTE_DATA_DC) @@ -1915,7 +1917,9 @@ static zend_never_inline void zend_pre_incdec_overloaded_property(zend_object *o object->handlers->write_property(object, name, &z_copy, cache_slot); OBJ_RELEASE(object); zval_ptr_dtor(&z_copy); - zval_ptr_dtor(z); + if (z == &rv) { + zval_ptr_dtor(z); + } } static zend_never_inline void zend_assign_op_overloaded_property(zend_object *object, zend_string *name, void **cache_slot, zval *value OPLINE_DC EXECUTE_DATA_DC) @@ -1938,7 +1942,9 @@ static zend_never_inline void zend_assign_op_overloaded_property(zend_object *ob if (UNEXPECTED(RETURN_VALUE_USED(opline))) { ZVAL_COPY(EX_VAR(opline->result.var), &res); } - zval_ptr_dtor(z); + if (z == &rv) { + zval_ptr_dtor(z); + } zval_ptr_dtor(&res); OBJ_RELEASE(object); } diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index 1abd63506a8df..34a582aaf357a 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -1249,6 +1249,12 @@ static void do_inherit_property(zend_property_info *parent_info, zend_string *ke (parent_info->flags & ZEND_ACC_STATIC) ? "static " : "non static ", ZSTR_VAL(parent_info->ce->name), ZSTR_VAL(key), (child_info->flags & ZEND_ACC_STATIC) ? "static " : "non static ", ZSTR_VAL(ce->name), ZSTR_VAL(key)); } + if (UNEXPECTED((child_info->flags & ZEND_ACC_READONLY) && !(parent_info->flags & ZEND_ACC_READONLY))) { + zend_error_noreturn(E_COMPILE_ERROR, + "Cannot redeclare non-readonly property %s::$%s as readonly %s::$%s", + ZSTR_VAL(parent_info->ce->name), ZSTR_VAL(key), + ZSTR_VAL(ce->name), ZSTR_VAL(key)); + } if (UNEXPECTED((child_info->flags & ZEND_ACC_PPP_MASK) > (parent_info->flags & ZEND_ACC_PPP_MASK))) { zend_error_noreturn(E_COMPILE_ERROR, "Access level to %s::$%s must be %s (as in class %s)%s", ZSTR_VAL(ce->name), ZSTR_VAL(key), zend_visibility_string(parent_info->flags), ZSTR_VAL(parent_info->ce->name), (parent_info->flags&ZEND_ACC_PUBLIC) ? "" : " or weaker"); @@ -2218,10 +2224,10 @@ static void zend_do_traits_property_binding(zend_class_entry *ce, zend_class_ent zend_hash_del(&ce->properties_info, prop_name); flags |= ZEND_ACC_CHANGED; } else { + uint32_t flags_mask = ZEND_ACC_PPP_MASK | ZEND_ACC_STATIC | ZEND_ACC_READONLY; not_compatible = 1; - if ((colliding_prop->flags & (ZEND_ACC_PPP_MASK | ZEND_ACC_STATIC)) - == (flags & (ZEND_ACC_PPP_MASK | ZEND_ACC_STATIC)) && + if ((colliding_prop->flags & flags_mask) == (flags & flags_mask) && property_types_compatible(property_info, colliding_prop) == INHERITANCE_SUCCESS ) { /* the flags are identical, thus, the properties may be compatible */ diff --git a/Zend/zend_language_parser.y b/Zend/zend_language_parser.y index ccf11bda8b8c3..9ad6b374135e4 100644 --- a/Zend/zend_language_parser.y +++ b/Zend/zend_language_parser.y @@ -154,6 +154,7 @@ static YYSIZE_T zend_yytnamerr(char*, const char*); %token T_PRIVATE "'private'" %token T_PROTECTED "'protected'" %token T_PUBLIC "'public'" +%token T_READONLY "'readonly'" %token T_VAR "'var'" %token T_UNSET "'unset'" %token T_ISSET "'isset'" @@ -279,7 +280,8 @@ static YYSIZE_T zend_yytnamerr(char*, const char*); %type enum_declaration_statement enum_backing_type enum_case enum_case_expr %type returns_ref function fn is_reference is_variadic variable_modifiers -%type method_modifiers non_empty_member_modifiers member_modifier optional_visibility_modifier +%type method_modifiers non_empty_member_modifiers member_modifier +%type optional_property_modifiers property_modifier %type class_modifiers class_modifier use_type backup_fn_flags %type backup_lex_pos @@ -769,19 +771,24 @@ attributed_parameter: | parameter { $$ = $1; } ; -optional_visibility_modifier: +optional_property_modifiers: %empty { $$ = 0; } - | T_PUBLIC { $$ = ZEND_ACC_PUBLIC; } + | optional_property_modifiers property_modifier + { $$ = zend_add_member_modifier($1, $2); if (!$$) { YYERROR; } } + +property_modifier: + T_PUBLIC { $$ = ZEND_ACC_PUBLIC; } | T_PROTECTED { $$ = ZEND_ACC_PROTECTED; } | T_PRIVATE { $$ = ZEND_ACC_PRIVATE; } + | T_READONLY { $$ = ZEND_ACC_READONLY; } ; parameter: - optional_visibility_modifier optional_type_without_static + optional_property_modifiers optional_type_without_static is_reference is_variadic T_VARIABLE backup_doc_comment { $$ = zend_ast_create_ex(ZEND_AST_PARAM, $1 | $3 | $4, $2, $5, NULL, NULL, $6 ? zend_ast_create_zval_from_str($6) : NULL); } - | optional_visibility_modifier optional_type_without_static + | optional_property_modifiers optional_type_without_static is_reference is_variadic T_VARIABLE backup_doc_comment '=' expr { $$ = zend_ast_create_ex(ZEND_AST_PARAM, $1 | $3 | $4, $2, $5, $8, NULL, $6 ? zend_ast_create_zval_from_str($6) : NULL); } @@ -1001,6 +1008,7 @@ member_modifier: | T_STATIC { $$ = ZEND_ACC_STATIC; } | T_ABSTRACT { $$ = ZEND_ACC_ABSTRACT; } | T_FINAL { $$ = ZEND_ACC_FINAL; } + | T_READONLY { $$ = ZEND_ACC_READONLY; } ; property_list: diff --git a/Zend/zend_language_scanner.l b/Zend/zend_language_scanner.l index 1155f97d759c2..3f56ea820870a 100644 --- a/Zend/zend_language_scanner.l +++ b/Zend/zend_language_scanner.l @@ -1714,6 +1714,10 @@ NEWLINE ("\r"|"\n"|"\r\n") RETURN_TOKEN_WITH_IDENT(T_PUBLIC); } +"readonly" { + RETURN_TOKEN_WITH_IDENT(T_READONLY); +} + "unset" { RETURN_TOKEN_WITH_IDENT(T_UNSET); } diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index b1f8ce10209a3..461c8fe35c916 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -277,6 +277,25 @@ static ZEND_COLD zend_never_inline void zend_forbidden_dynamic_property( ZSTR_VAL(ce->name), ZSTR_VAL(member)); } +static ZEND_COLD zend_never_inline void zend_readonly_property_modification_error( + zend_class_entry *ce, zend_string *member) { + zend_throw_error(NULL, "Cannot modify readonly property %s::$%s", + ZSTR_VAL(ce->name), ZSTR_VAL(member)); +} + +static ZEND_COLD zend_never_inline void zend_readonly_property_modification_scope_error( + zend_class_entry *ce, zend_string *member, zend_class_entry *scope, const char *operation) { + zend_throw_error(NULL, "Cannot %s readonly property %s::$%s from %s%s", + operation, ZSTR_VAL(ce->name), ZSTR_VAL(member), + scope ? "scope " : "global scope", scope ? ZSTR_VAL(scope->name) : ""); +} + +static ZEND_COLD zend_never_inline void zend_readonly_property_unset_error( + zend_class_entry *ce, zend_string *member) { + zend_throw_error(NULL, "Cannot unset readonly property %s::$%s", + ZSTR_VAL(ce->name), ZSTR_VAL(member)); +} + static zend_always_inline uintptr_t zend_get_property_offset(zend_class_entry *ce, zend_string *member, int silent, void **cache_slot, zend_property_info **info_ptr) /* {{{ */ { zval *zv; @@ -573,6 +592,13 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int if (EXPECTED(IS_VALID_PROPERTY_OFFSET(property_offset))) { retval = OBJ_PROP(zobj, property_offset); if (EXPECTED(Z_TYPE_P(retval) != IS_UNDEF)) { + if (prop_info && UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY) + && (type == BP_VAR_W || type == BP_VAR_RW || type == BP_VAR_UNSET)) { + if (Z_TYPE_P(retval) != IS_OBJECT) { + zend_readonly_property_modification_error(prop_info->ce, name); + retval = &EG(uninitialized_zval); + } + } goto exit; } if (UNEXPECTED(Z_PROP_FLAG_P(retval) == IS_PROP_UNINIT)) { @@ -708,6 +734,36 @@ static zend_always_inline bool property_uses_strict_types() { && ZEND_CALL_USES_STRICT_TYPES(EG(current_execute_data)); } +static bool verify_readonly_initialization_access( + zend_property_info *prop_info, zend_class_entry *ce, + zend_string *name, const char *operation) { + zend_class_entry *scope; + if (UNEXPECTED(EG(fake_scope))) { + scope = EG(fake_scope); + } else { + scope = zend_get_executed_scope(); + } + if (prop_info->ce == scope) { + return true; + } + + /* We may have redeclared a parent property. In that case the parent should still be + * allowed to initialize it. */ + if (scope && is_derived_class(ce, scope)) { + zend_property_info *prop_info = zend_hash_find_ptr(&scope->properties_info, name); + if (prop_info) { + /* This should be ensured by inheritance. */ + ZEND_ASSERT(prop_info->flags & ZEND_ACC_READONLY); + if (prop_info->ce == scope) { + return true; + } + } + } + + zend_readonly_property_modification_scope_error(prop_info->ce, name, scope, operation); + return false; +} + ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zval *value, void **cache_slot) /* {{{ */ { zval *variable_ptr, tmp; @@ -723,6 +779,13 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva Z_TRY_ADDREF_P(value); if (UNEXPECTED(prop_info)) { + if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) { + Z_TRY_DELREF_P(value); + zend_readonly_property_modification_error(prop_info->ce, name); + variable_ptr = &EG(error_zval); + goto exit; + } + ZVAL_COPY_VALUE(&tmp, value); if (UNEXPECTED(!zend_verify_property_type(prop_info, &tmp, property_uses_strict_types()))) { Z_TRY_DELREF_P(value); @@ -737,6 +800,11 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva variable_ptr, value, IS_TMP_VAR, property_uses_strict_types()); goto exit; } + if (UNEXPECTED(prop_info && (prop_info->flags & ZEND_ACC_READONLY) + && !verify_readonly_initialization_access(prop_info, zobj->ce, name, "initialize"))) { + variable_ptr = &EG(error_zval); + goto exit; + } if (Z_PROP_FLAG_P(variable_ptr) == IS_PROP_UNINIT) { /* Writes to uninitialized typed properties bypass __set(). */ goto write_std_property; @@ -955,6 +1023,9 @@ ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *zobj, zend_string *nam /* we do have getter - fail and let it try again with usual get/set */ retval = NULL; } + } else if (prop_info && UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) { + /* Readonly property, delegate to read_property + write_property. */ + retval = NULL; } } else if (EXPECTED(IS_DYNAMIC_PROPERTY_OFFSET(property_offset))) { if (EXPECTED(zobj->properties)) { @@ -1003,6 +1074,10 @@ ZEND_API void zend_std_unset_property(zend_object *zobj, zend_string *name, void zval *slot = OBJ_PROP(zobj, property_offset); if (Z_TYPE_P(slot) != IS_UNDEF) { + if (UNEXPECTED(prop_info && (prop_info->flags & ZEND_ACC_READONLY))) { + zend_readonly_property_unset_error(prop_info->ce, name); + return; + } if (UNEXPECTED(Z_ISREF_P(slot)) && (ZEND_DEBUG || ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(slot)))) { if (prop_info) { @@ -1018,6 +1093,10 @@ ZEND_API void zend_std_unset_property(zend_object *zobj, zend_string *name, void } return; } + if (UNEXPECTED(prop_info && (prop_info->flags & ZEND_ACC_READONLY) + && !verify_readonly_initialization_access(prop_info, zobj->ce, name, "unset"))) { + return; + } if (UNEXPECTED(Z_PROP_FLAG_P(slot) == IS_PROP_UNINIT)) { /* Reset the IS_PROP_UNINIT flag, if it exists and bypass __unset(). */ Z_PROP_FLAG_P(slot) = 0; diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 1ed423bb374f5..7402d4c4683d4 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -6926,11 +6926,20 @@ ZEND_VM_HANDLER(126, ZEND_FE_FETCH_RW, VAR, ANY, JMP_ADDR) && EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key, 0) == SUCCESS)) { if ((value_type & Z_TYPE_MASK) != IS_REFERENCE) { zend_property_info *prop_info = - zend_get_typed_property_info_for_slot(Z_OBJ_P(array), value); + zend_get_property_info_for_slot(Z_OBJ_P(array), value); if (UNEXPECTED(prop_info)) { - ZVAL_NEW_REF(value, value); - ZEND_REF_ADD_TYPE_SOURCE(Z_REF_P(value), prop_info); - value_type = IS_REFERENCE_EX; + if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) { + zend_throw_error(NULL, + "Cannot acquire reference to readonly property %s::$%s", + ZSTR_VAL(prop_info->ce->name), ZSTR_VAL(p->key)); + UNDEF_RESULT(); + HANDLE_EXCEPTION(); + } + if (ZEND_TYPE_IS_SET(prop_info->type)) { + ZVAL_NEW_REF(value, value); + ZEND_REF_ADD_TYPE_SOURCE(Z_REF_P(value), prop_info); + value_type = IS_REFERENCE_EX; + } } } break; diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index 1af7e517d04e5..edcf127756930 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -21952,11 +21952,20 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FE_FETCH_RW_SPEC_VAR_HANDLER(Z && EXPECTED(zend_check_property_access(Z_OBJ_P(array), p->key, 0) == SUCCESS)) { if ((value_type & Z_TYPE_MASK) != IS_REFERENCE) { zend_property_info *prop_info = - zend_get_typed_property_info_for_slot(Z_OBJ_P(array), value); + zend_get_property_info_for_slot(Z_OBJ_P(array), value); if (UNEXPECTED(prop_info)) { - ZVAL_NEW_REF(value, value); - ZEND_REF_ADD_TYPE_SOURCE(Z_REF_P(value), prop_info); - value_type = IS_REFERENCE_EX; + if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) { + zend_throw_error(NULL, + "Cannot acquire reference to readonly property %s::$%s", + ZSTR_VAL(prop_info->ce->name), ZSTR_VAL(p->key)); + UNDEF_RESULT(); + HANDLE_EXCEPTION(); + } + if (ZEND_TYPE_IS_SET(prop_info->type)) { + ZVAL_NEW_REF(value, value); + ZEND_REF_ADD_TYPE_SOURCE(Z_REF_P(value), prop_info); + value_type = IS_REFERENCE_EX; + } } } break; diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 42b9356bf2178..10c0540574c8e 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -5509,6 +5509,11 @@ ZEND_METHOD(ReflectionProperty, isStatic) } /* }}} */ +ZEND_METHOD(ReflectionProperty, isReadOnly) +{ + _property_check_flag(INTERNAL_FUNCTION_PARAM_PASSTHRU, ZEND_ACC_READONLY); +} + /* {{{ Returns whether this property is default (declared at compilation time). */ ZEND_METHOD(ReflectionProperty, isDefault) { @@ -5535,7 +5540,7 @@ ZEND_METHOD(ReflectionProperty, getModifiers) { reflection_object *intern; property_reference *ref; - uint32_t keep_flags = ZEND_ACC_PPP_MASK | ZEND_ACC_STATIC; + uint32_t keep_flags = ZEND_ACC_PPP_MASK | ZEND_ACC_STATIC | ZEND_ACC_READONLY; if (zend_parse_parameters_none() == FAILURE) { RETURN_THROWS(); @@ -7108,6 +7113,7 @@ PHP_MINIT_FUNCTION(reflection) /* {{{ */ reflection_init_class_handlers(reflection_property_ptr); REGISTER_REFLECTION_CLASS_CONST_LONG(property, "IS_STATIC", ZEND_ACC_STATIC); + REGISTER_REFLECTION_CLASS_CONST_LONG(property, "IS_READONLY", ZEND_ACC_READONLY); REGISTER_REFLECTION_CLASS_CONST_LONG(property, "IS_PUBLIC", ZEND_ACC_PUBLIC); REGISTER_REFLECTION_CLASS_CONST_LONG(property, "IS_PROTECTED", ZEND_ACC_PROTECTED); REGISTER_REFLECTION_CLASS_CONST_LONG(property, "IS_PRIVATE", ZEND_ACC_PRIVATE); diff --git a/ext/reflection/php_reflection.stub.php b/ext/reflection/php_reflection.stub.php index 2b75372f0c843..a3af65c957992 100644 --- a/ext/reflection/php_reflection.stub.php +++ b/ext/reflection/php_reflection.stub.php @@ -416,6 +416,8 @@ public function isProtected(): bool {} /** @tentative-return-type */ public function isStatic(): bool {} + public function isReadOnly(): bool {} + /** @tentative-return-type */ public function isDefault(): bool {} diff --git a/ext/reflection/php_reflection_arginfo.h b/ext/reflection/php_reflection_arginfo.h index dac6136d638b2..f89b11d33858b 100644 --- a/ext/reflection/php_reflection_arginfo.h +++ b/ext/reflection/php_reflection_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 168c80203d91f05121c17835b775b9a00757e9da */ + * Stub hash: a404de43d1f20edd7ee21e171851a96022209e93 */ ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_Reflection_getModifierNames, 0, 1, IS_ARRAY, 0) ZEND_ARG_TYPE_INFO(0, modifiers, IS_LONG, 0) @@ -354,6 +354,8 @@ ZEND_END_ARG_INFO() #define arginfo_class_ReflectionProperty_isStatic arginfo_class_ReflectionFunctionAbstract_inNamespace +#define arginfo_class_ReflectionProperty_isReadOnly arginfo_class_ReflectionFunctionAbstract_hasTentativeReturnType + #define arginfo_class_ReflectionProperty_isDefault arginfo_class_ReflectionFunctionAbstract_inNamespace #define arginfo_class_ReflectionProperty_isPromoted arginfo_class_ReflectionFunctionAbstract_hasTentativeReturnType @@ -722,6 +724,7 @@ ZEND_METHOD(ReflectionProperty, isPublic); ZEND_METHOD(ReflectionProperty, isPrivate); ZEND_METHOD(ReflectionProperty, isProtected); ZEND_METHOD(ReflectionProperty, isStatic); +ZEND_METHOD(ReflectionProperty, isReadOnly); ZEND_METHOD(ReflectionProperty, isDefault); ZEND_METHOD(ReflectionProperty, isPromoted); ZEND_METHOD(ReflectionProperty, getModifiers); @@ -997,6 +1000,7 @@ static const zend_function_entry class_ReflectionProperty_methods[] = { ZEND_ME(ReflectionProperty, isPrivate, arginfo_class_ReflectionProperty_isPrivate, ZEND_ACC_PUBLIC) ZEND_ME(ReflectionProperty, isProtected, arginfo_class_ReflectionProperty_isProtected, ZEND_ACC_PUBLIC) ZEND_ME(ReflectionProperty, isStatic, arginfo_class_ReflectionProperty_isStatic, ZEND_ACC_PUBLIC) + ZEND_ME(ReflectionProperty, isReadOnly, arginfo_class_ReflectionProperty_isReadOnly, ZEND_ACC_PUBLIC) ZEND_ME(ReflectionProperty, isDefault, arginfo_class_ReflectionProperty_isDefault, ZEND_ACC_PUBLIC) ZEND_ME(ReflectionProperty, isPromoted, arginfo_class_ReflectionProperty_isPromoted, ZEND_ACC_PUBLIC) ZEND_ME(ReflectionProperty, getModifiers, arginfo_class_ReflectionProperty_getModifiers, ZEND_ACC_PUBLIC) diff --git a/ext/reflection/tests/readonly_properties.phpt b/ext/reflection/tests/readonly_properties.phpt new file mode 100644 index 0000000000000..ac353bd1a71d2 --- /dev/null +++ b/ext/reflection/tests/readonly_properties.phpt @@ -0,0 +1,24 @@ +--TEST-- +Readonly property reflection +--FILE-- +isReadOnly()); +var_dump(($rp->getModifiers() & ReflectionProperty::IS_READONLY) != 0); + +$rp = new ReflectionProperty(Test::class, 'ro'); +var_dump($rp->isReadOnly()); +var_dump(($rp->getModifiers() & ReflectionProperty::IS_READONLY) != 0); + +?> +--EXPECT-- +bool(false) +bool(false) +bool(true) +bool(true) diff --git a/ext/tokenizer/tokenizer_data.c b/ext/tokenizer/tokenizer_data.c index 271184fb107d5..a5adc67308485 100644 --- a/ext/tokenizer/tokenizer_data.c +++ b/ext/tokenizer/tokenizer_data.c @@ -92,6 +92,7 @@ void tokenizer_register_constants(INIT_FUNC_ARGS) { REGISTER_LONG_CONSTANT("T_PRIVATE", T_PRIVATE, CONST_CS | CONST_PERSISTENT); REGISTER_LONG_CONSTANT("T_PROTECTED", T_PROTECTED, CONST_CS | CONST_PERSISTENT); REGISTER_LONG_CONSTANT("T_PUBLIC", T_PUBLIC, CONST_CS | CONST_PERSISTENT); + REGISTER_LONG_CONSTANT("T_READONLY", T_READONLY, CONST_CS | CONST_PERSISTENT); REGISTER_LONG_CONSTANT("T_VAR", T_VAR, CONST_CS | CONST_PERSISTENT); REGISTER_LONG_CONSTANT("T_UNSET", T_UNSET, CONST_CS | CONST_PERSISTENT); REGISTER_LONG_CONSTANT("T_ISSET", T_ISSET, CONST_CS | CONST_PERSISTENT); @@ -244,6 +245,7 @@ char *get_token_type_name(int token_type) case T_PRIVATE: return "T_PRIVATE"; case T_PROTECTED: return "T_PROTECTED"; case T_PUBLIC: return "T_PUBLIC"; + case T_READONLY: return "T_READONLY"; case T_VAR: return "T_VAR"; case T_UNSET: return "T_UNSET"; case T_ISSET: return "T_ISSET"; From 94bdfebcb9ec82cb56290d584a548e148a8bf348 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 3 Jun 2021 10:57:49 +0200 Subject: [PATCH 02/20] Better handling of magic get/set --- Zend/tests/readonly_props/magic_get_set.phpt | 74 ++++++++++++++++++++ Zend/zend_object_handlers.c | 21 +++--- 2 files changed, 86 insertions(+), 9 deletions(-) create mode 100644 Zend/tests/readonly_props/magic_get_set.phpt diff --git a/Zend/tests/readonly_props/magic_get_set.phpt b/Zend/tests/readonly_props/magic_get_set.phpt new file mode 100644 index 0000000000000..201c4e3bfe2f8 --- /dev/null +++ b/Zend/tests/readonly_props/magic_get_set.phpt @@ -0,0 +1,74 @@ +--TEST-- +Interaction with magic get/set +--FILE-- +prop); + } + + public function __get($name) { + echo __METHOD__, "($name)\n"; + return 1; + } + + public function __set($name, $value) { + echo __METHOD__, "($name, $value)\n"; + } + + public function __unset($name) { + echo __METHOD__, "($name)\n"; + } + + public function __isset($name) { + echo __METHOD__, "($name)\n"; + return true; + } +} + +$test = new Test; + +// The property is in uninitialized state, no magic methods should be invoked. +var_dump(isset($test->prop)); +try { + var_dump($test->prop); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + $test->prop = 1; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + unset($test->prop); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} + +$test->unsetProp(); + +var_dump(isset($test->prop)); +var_dump($test->prop); +$test->prop = 2; +try { + unset($test->prop); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} + +?> +--EXPECT-- +bool(false) +Typed property Test::$prop must not be accessed before initialization +Cannot initialize readonly property Test::$prop from global scope +Cannot unset readonly property Test::$prop from global scope +Test::__isset(prop) +bool(true) +Test::__get(prop) +int(1) +Test::__set(prop, 2) +Test::__unset(prop) diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 461c8fe35c916..26e1bd431781d 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -800,11 +800,6 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva variable_ptr, value, IS_TMP_VAR, property_uses_strict_types()); goto exit; } - if (UNEXPECTED(prop_info && (prop_info->flags & ZEND_ACC_READONLY) - && !verify_readonly_initialization_access(prop_info, zobj->ce, name, "initialize"))) { - variable_ptr = &EG(error_zval); - goto exit; - } if (Z_PROP_FLAG_P(variable_ptr) == IS_PROP_UNINIT) { /* Writes to uninitialized typed properties bypass __set(). */ goto write_std_property; @@ -855,6 +850,13 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva Z_TRY_ADDREF_P(value); if (UNEXPECTED(prop_info)) { + if (UNEXPECTED((prop_info->flags & ZEND_ACC_READONLY) + && !verify_readonly_initialization_access(prop_info, zobj->ce, name, "initialize"))) { + Z_TRY_DELREF_P(value); + variable_ptr = &EG(error_zval); + goto exit; + } + ZVAL_COPY_VALUE(&tmp, value); if (UNEXPECTED(!zend_verify_property_type(prop_info, &tmp, property_uses_strict_types()))) { zval_ptr_dtor(value); @@ -1093,11 +1095,12 @@ ZEND_API void zend_std_unset_property(zend_object *zobj, zend_string *name, void } return; } - if (UNEXPECTED(prop_info && (prop_info->flags & ZEND_ACC_READONLY) - && !verify_readonly_initialization_access(prop_info, zobj->ce, name, "unset"))) { - return; - } if (UNEXPECTED(Z_PROP_FLAG_P(slot) == IS_PROP_UNINIT)) { + if (UNEXPECTED(prop_info && (prop_info->flags & ZEND_ACC_READONLY) + && !verify_readonly_initialization_access(prop_info, zobj->ce, name, "unset"))) { + return; + } + /* Reset the IS_PROP_UNINIT flag, if it exists and bypass __unset(). */ Z_PROP_FLAG_P(slot) = 0; return; From bbfa50706830cfef8e1e211fae62ef52a4ef0880 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 3 Jun 2021 14:25:02 +0200 Subject: [PATCH 03/20] Forbid default values after all I changed my mind... --- Zend/tests/readonly_props/readonly_with_default.phpt | 4 ++-- Zend/tests/readonly_props/serialization.phpt | 4 +++- Zend/zend_compile.c | 5 +++++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Zend/tests/readonly_props/readonly_with_default.phpt b/Zend/tests/readonly_props/readonly_with_default.phpt index db8f79bd2d6d7..12afe5cde1539 100644 --- a/Zend/tests/readonly_props/readonly_with_default.phpt +++ b/Zend/tests/readonly_props/readonly_with_default.phpt @@ -15,5 +15,5 @@ try { } ?> ---EXPECT-- -Cannot modify readonly property Test::$prop +--EXPECTF-- +Fatal error: Readonly property Test::$prop cannot have default value in %s on line %d diff --git a/Zend/tests/readonly_props/serialization.phpt b/Zend/tests/readonly_props/serialization.phpt index 8182eef3fa5f0..f9e1f364673ff 100644 --- a/Zend/tests/readonly_props/serialization.phpt +++ b/Zend/tests/readonly_props/serialization.phpt @@ -4,7 +4,9 @@ Serialization of readonly properties name), ZSTR_VAL(name)); } + if (!Z_ISUNDEF(value_zv)) { + zend_error_noreturn(E_COMPILE_ERROR, + "Readonly property %s::$%s cannot have default value", + ZSTR_VAL(ce->name), ZSTR_VAL(name)); + } if (flags & ZEND_ACC_STATIC) { zend_error_noreturn(E_COMPILE_ERROR, "Static property %s::$%s cannot be readonly", From 8b448c067ea035c472aa86a0e4d8b917c0cebaf4 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 4 Jun 2021 16:03:10 +0200 Subject: [PATCH 04/20] Don't allow readonly -> readwrite change during inheritance --- Zend/tests/readonly_props/readonly_to_readwrite.phpt | 7 +++---- Zend/zend_inheritance.c | 6 ++++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Zend/tests/readonly_props/readonly_to_readwrite.phpt b/Zend/tests/readonly_props/readonly_to_readwrite.phpt index 79e7985e35fef..499cce0e1c013 100644 --- a/Zend/tests/readonly_props/readonly_to_readwrite.phpt +++ b/Zend/tests/readonly_props/readonly_to_readwrite.phpt @@ -1,5 +1,5 @@ --TEST-- -Can replace readonly with readwrite +Cannot replace readonly with readwrite --FILE-- -===DONE=== ---EXPECT-- -===DONE=== +--EXPECTF-- +Fatal error: Cannot redeclare readonly property A::$prop as non-readonly B::$prop in %s on line %d diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index 34a582aaf357a..a544d6422161a 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -1249,10 +1249,12 @@ static void do_inherit_property(zend_property_info *parent_info, zend_string *ke (parent_info->flags & ZEND_ACC_STATIC) ? "static " : "non static ", ZSTR_VAL(parent_info->ce->name), ZSTR_VAL(key), (child_info->flags & ZEND_ACC_STATIC) ? "static " : "non static ", ZSTR_VAL(ce->name), ZSTR_VAL(key)); } - if (UNEXPECTED((child_info->flags & ZEND_ACC_READONLY) && !(parent_info->flags & ZEND_ACC_READONLY))) { + if (UNEXPECTED((child_info->flags & ZEND_ACC_READONLY) != (parent_info->flags & ZEND_ACC_READONLY))) { zend_error_noreturn(E_COMPILE_ERROR, - "Cannot redeclare non-readonly property %s::$%s as readonly %s::$%s", + "Cannot redeclare %s property %s::$%s as %s %s::$%s", + parent_info->flags & ZEND_ACC_READONLY ? "readonly" : "non-readonly", ZSTR_VAL(parent_info->ce->name), ZSTR_VAL(key), + child_info->flags & ZEND_ACC_READONLY ? "readonly" : "non-readonly", ZSTR_VAL(ce->name), ZSTR_VAL(key)); } From 4b5e5a8e4a1793fca907e2d1aa97da18230efd76 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 14 Jul 2021 10:09:12 +0200 Subject: [PATCH 05/20] Add readonly to semi-reserved --- Zend/tests/grammar/semi_reserved_001.phpt | 3 +++ Zend/zend_language_parser.y | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Zend/tests/grammar/semi_reserved_001.phpt b/Zend/tests/grammar/semi_reserved_001.phpt index 2de8e901ef9e6..4f50a8e12e07a 100644 --- a/Zend/tests/grammar/semi_reserved_001.phpt +++ b/Zend/tests/grammar/semi_reserved_001.phpt @@ -45,6 +45,7 @@ class Obj function array(){ echo __METHOD__, PHP_EOL; } function print(){ echo __METHOD__, PHP_EOL; } function echo(){ echo __METHOD__, PHP_EOL; } + function readonly(){ echo __METHOD__, PHP_EOL; } function require(){ echo __METHOD__, PHP_EOL; } function require_once(){ echo __METHOD__, PHP_EOL; } function return(){ echo __METHOD__, PHP_EOL; } @@ -125,6 +126,7 @@ $obj->throw(); $obj->array(); $obj->print(); $obj->echo(); +$obj->readonly(); $obj->require(); $obj->require_once(); $obj->return(); @@ -205,6 +207,7 @@ Obj::throw Obj::array Obj::print Obj::echo +Obj::readonly Obj::require Obj::require_once Obj::return diff --git a/Zend/zend_language_parser.y b/Zend/zend_language_parser.y index 9ad6b374135e4..a8bddfae50a3e 100644 --- a/Zend/zend_language_parser.y +++ b/Zend/zend_language_parser.y @@ -307,7 +307,7 @@ reserved_non_modifiers: semi_reserved: reserved_non_modifiers - | T_STATIC | T_ABSTRACT | T_FINAL | T_PRIVATE | T_PROTECTED | T_PUBLIC + | T_STATIC | T_ABSTRACT | T_FINAL | T_PRIVATE | T_PROTECTED | T_PUBLIC | T_READONLY ; ampersand: From 33aa78077a9909ffe4a75231805fe234bb1ec621 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 14 Jul 2021 10:10:36 +0200 Subject: [PATCH 06/20] Fix rebase mistake --- Zend/zend_compile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 438478caf8be6..b68302e1966d2 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -7427,7 +7427,7 @@ void zend_compile_class_const_decl(zend_ast *ast, uint32_t flags, zend_ast *attr zend_string *doc_comment = doc_comment_ast ? zend_string_copy(zend_ast_get_str(doc_comment_ast)) : NULL; zval value_zv; - if (UNEXPECTED(flags & (ZEND_ACC_STATIC|ZEND_ACC_ABSTRACT))) { + if (UNEXPECTED(flags & (ZEND_ACC_STATIC|ZEND_ACC_ABSTRACT|ZEND_ACC_READONLY))) { zend_check_const_and_trait_alias_attr(flags, "constant"); } From c757c7afd932842c0df9bb0607af261abf171a7b Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 14 Jul 2021 10:12:16 +0200 Subject: [PATCH 07/20] Add tests from ilutov --- .../override_with_attributes.phpt | 26 ++++++++++++++++ .../readonly_props/visibility_change.phpt | 30 +++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 Zend/tests/readonly_props/override_with_attributes.phpt create mode 100644 Zend/tests/readonly_props/visibility_change.phpt diff --git a/Zend/tests/readonly_props/override_with_attributes.phpt b/Zend/tests/readonly_props/override_with_attributes.phpt new file mode 100644 index 0000000000000..f2b93bce17da8 --- /dev/null +++ b/Zend/tests/readonly_props/override_with_attributes.phpt @@ -0,0 +1,26 @@ +--TEST-- +Can override readonly property with attributes +--FILE-- +prop = 42; + } +} +class B extends A { + #[FooAttribute] + public readonly int $prop; +} + +var_dump((new ReflectionProperty(B::class, 'prop'))->getAttributes()[0]->newInstance()); + +?> +--EXPECT-- +object(FooAttribute)#1 (0) { +} diff --git a/Zend/tests/readonly_props/visibility_change.phpt b/Zend/tests/readonly_props/visibility_change.phpt new file mode 100644 index 0000000000000..f4a32f3cdaafa --- /dev/null +++ b/Zend/tests/readonly_props/visibility_change.phpt @@ -0,0 +1,30 @@ +--TEST-- +Visibility can change in readonly property +--FILE-- +prop = 42; + } +} +class B extends A { + public readonly int $prop; +} + +$a = new A(); +try { + var_dump($a->prop); +} catch (Error $error) { + echo $error->getMessage() . "\n"; +} + +$b = new B(); +var_dump($b->prop); + +?> +--EXPECT-- +Cannot access protected property A::$prop +int(42) From f51375bca1fbc1c564a7393dfdc3c3e4bc0929b0 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 5 Jul 2021 14:27:35 +0200 Subject: [PATCH 08/20] Check readonly when directly using cache_slot --- Zend/tests/readonly_props/cache_slot.phpt | 51 +++++++++++++++++++++++ Zend/zend_execute.c | 24 +++++++++-- Zend/zend_execute.h | 2 + Zend/zend_object_handlers.c | 10 +---- 4 files changed, 76 insertions(+), 11 deletions(-) create mode 100644 Zend/tests/readonly_props/cache_slot.phpt diff --git a/Zend/tests/readonly_props/cache_slot.phpt b/Zend/tests/readonly_props/cache_slot.phpt new file mode 100644 index 0000000000000..96bc3c7b79bec --- /dev/null +++ b/Zend/tests/readonly_props/cache_slot.phpt @@ -0,0 +1,51 @@ +--TEST-- +Test interaction with cache slots +--FILE-- +prop = $prop; + } + public function initAndAppendProp2() { + $this->prop2 = []; + $this->prop2[] = 1; + } +} + +$test = new Test; +$test->setProp("a"); +var_dump($test->prop); +try { + $test->setProp("b"); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +var_dump($test->prop); +echo "\n"; + +$test = new Test; +try { + $test->initAndAppendProp2(); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + $test->initAndAppendProp2(); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +var_dump($test->prop2); + +?> +--EXPECT-- +string(1) "a" +Cannot modify readonly property Test::$prop +string(1) "a" + +Cannot modify readonly property Test::$prop2 +Cannot modify readonly property Test::$prop2 +array(0) { +} diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 634de6026972b..c29ee9b879cf1 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -823,6 +823,12 @@ ZEND_COLD zend_never_inline void zend_verify_property_type_error(zend_property_i zend_string_release(type_str); } +ZEND_COLD zend_never_inline void zend_readonly_property_modification_error( + zend_property_info *info) { + zend_throw_error(NULL, "Cannot modify readonly property %s::$%s", + ZSTR_VAL(info->ce->name), zend_get_unmangled_property_name(info->name)); +} + static zend_class_entry *resolve_single_class_type(zend_string *name, zend_class_entry *self_ce) { if (zend_string_equals_literal_ci(name, "self")) { /* We need to explicitly check for this here, to avoid updating the type in the trait and @@ -927,6 +933,11 @@ static zend_never_inline zval* zend_assign_to_typed_prop(zend_property_info *inf { zval tmp; + if (UNEXPECTED(info->flags & ZEND_ACC_READONLY)) { + zend_readonly_property_modification_error(info); + return &EG(uninitialized_zval); + } + ZVAL_DEREF(value); ZVAL_COPY(&tmp, value); @@ -2827,9 +2838,16 @@ static zend_always_inline void zend_fetch_property_address(zval *result, zval *c ptr = OBJ_PROP(zobj, prop_offset); if (EXPECTED(Z_TYPE_P(ptr) != IS_UNDEF)) { ZVAL_INDIRECT(result, ptr); - if (flags) { - zend_property_info *prop_info = CACHED_PTR_EX(cache_slot + 2); - if (prop_info) { + zend_property_info *prop_info = CACHED_PTR_EX(cache_slot + 2); + if (prop_info) { + if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY) + && Z_TYPE_P(ptr) != IS_OBJECT) { + ZEND_ASSERT(type == BP_VAR_W || type == BP_VAR_RW || type == BP_VAR_UNSET); + zend_readonly_property_modification_error(prop_info); + ZVAL_ERROR(result); + return; + } + if (flags) { zend_handle_fetch_obj_flags(result, ptr, NULL, prop_info, flags); } } diff --git a/Zend/zend_execute.h b/Zend/zend_execute.h index bd5e58531ca0e..3e31e50bf92ac 100644 --- a/Zend/zend_execute.h +++ b/Zend/zend_execute.h @@ -69,6 +69,8 @@ ZEND_API ZEND_COLD void zend_throw_ref_type_error_type(zend_property_info *prop1 ZEND_API ZEND_COLD zval* ZEND_FASTCALL zend_undefined_offset_write(HashTable *ht, zend_long lval); ZEND_API ZEND_COLD zval* ZEND_FASTCALL zend_undefined_index_write(HashTable *ht, zend_string *offset); +ZEND_COLD zend_never_inline void zend_readonly_property_modification_error(zend_property_info *info); + ZEND_API bool zend_verify_scalar_type_hint(uint32_t type_mask, zval *arg, bool strict, bool is_internal_arg); ZEND_API ZEND_COLD void zend_verify_arg_error( const zend_function *zf, const zend_arg_info *arg_info, uint32_t arg_num, zval *value); diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 26e1bd431781d..8aa83c1698a77 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -277,12 +277,6 @@ static ZEND_COLD zend_never_inline void zend_forbidden_dynamic_property( ZSTR_VAL(ce->name), ZSTR_VAL(member)); } -static ZEND_COLD zend_never_inline void zend_readonly_property_modification_error( - zend_class_entry *ce, zend_string *member) { - zend_throw_error(NULL, "Cannot modify readonly property %s::$%s", - ZSTR_VAL(ce->name), ZSTR_VAL(member)); -} - static ZEND_COLD zend_never_inline void zend_readonly_property_modification_scope_error( zend_class_entry *ce, zend_string *member, zend_class_entry *scope, const char *operation) { zend_throw_error(NULL, "Cannot %s readonly property %s::$%s from %s%s", @@ -595,7 +589,7 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int if (prop_info && UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY) && (type == BP_VAR_W || type == BP_VAR_RW || type == BP_VAR_UNSET)) { if (Z_TYPE_P(retval) != IS_OBJECT) { - zend_readonly_property_modification_error(prop_info->ce, name); + zend_readonly_property_modification_error(prop_info); retval = &EG(uninitialized_zval); } } @@ -781,7 +775,7 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva if (UNEXPECTED(prop_info)) { if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) { Z_TRY_DELREF_P(value); - zend_readonly_property_modification_error(prop_info->ce, name); + zend_readonly_property_modification_error(prop_info); variable_ptr = &EG(error_zval); goto exit; } From b5c4ea9d2efcfdedccfd4f629145bba32dc5a492 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 14 Jul 2021 16:19:20 +0200 Subject: [PATCH 09/20] x86 JIT support --- Zend/zend_execute.c | 2 +- Zend/zend_execute.h | 2 +- ext/opcache/jit/zend_jit_helpers.c | 8 ++++++++ ext/opcache/jit/zend_jit_x86.dasc | 21 ++++++++++++++++++--- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index c29ee9b879cf1..5385f5bbce5bd 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -823,7 +823,7 @@ ZEND_COLD zend_never_inline void zend_verify_property_type_error(zend_property_i zend_string_release(type_str); } -ZEND_COLD zend_never_inline void zend_readonly_property_modification_error( +ZEND_API ZEND_COLD void ZEND_FASTCALL zend_readonly_property_modification_error( zend_property_info *info) { zend_throw_error(NULL, "Cannot modify readonly property %s::$%s", ZSTR_VAL(info->ce->name), zend_get_unmangled_property_name(info->name)); diff --git a/Zend/zend_execute.h b/Zend/zend_execute.h index 3e31e50bf92ac..94d643466d9da 100644 --- a/Zend/zend_execute.h +++ b/Zend/zend_execute.h @@ -69,7 +69,7 @@ ZEND_API ZEND_COLD void zend_throw_ref_type_error_type(zend_property_info *prop1 ZEND_API ZEND_COLD zval* ZEND_FASTCALL zend_undefined_offset_write(HashTable *ht, zend_long lval); ZEND_API ZEND_COLD zval* ZEND_FASTCALL zend_undefined_index_write(HashTable *ht, zend_string *offset); -ZEND_COLD zend_never_inline void zend_readonly_property_modification_error(zend_property_info *info); +ZEND_API ZEND_COLD void ZEND_FASTCALL zend_readonly_property_modification_error(zend_property_info *info); ZEND_API bool zend_verify_scalar_type_hint(uint32_t type_mask, zval *arg, bool strict, bool is_internal_arg); ZEND_API ZEND_COLD void zend_verify_arg_error( diff --git a/ext/opcache/jit/zend_jit_helpers.c b/ext/opcache/jit/zend_jit_helpers.c index 59966ca09226d..5f9e82bea1c4a 100644 --- a/ext/opcache/jit/zend_jit_helpers.c +++ b/ext/opcache/jit/zend_jit_helpers.c @@ -2006,6 +2006,14 @@ static void ZEND_FASTCALL zend_jit_assign_to_typed_prop(zval *property_val, zend zend_execute_data *execute_data = EG(current_execute_data); zval tmp; + if (UNEXPECTED(info->flags & ZEND_ACC_READONLY)) { + zend_readonly_property_modification_error(info); + if (result) { + ZVAL_UNDEF(result); + } + return; + } + ZVAL_DEREF(value); ZVAL_COPY(&tmp, value); diff --git a/ext/opcache/jit/zend_jit_x86.dasc b/ext/opcache/jit/zend_jit_x86.dasc index ce8ad91397352..890204fac4af2 100644 --- a/ext/opcache/jit/zend_jit_x86.dasc +++ b/ext/opcache/jit/zend_jit_x86.dasc @@ -12511,7 +12511,6 @@ static int zend_jit_fetch_obj(dasm_State **Dst, | add FCARG1a, r0 prop_addr = ZEND_ADDR_MEM_ZVAL(ZREG_FCARG1a, 0); if (opline->opcode == ZEND_FETCH_OBJ_W - && (opline->extended_value & ZEND_FETCH_OBJ_FLAGS) && (!ce || ce_is_instanceof || (ce->ce_flags & ZEND_ACC_HAS_TYPE_HINTS))) { uint32_t flags = opline->extended_value & ZEND_FETCH_OBJ_FLAGS; @@ -12521,6 +12520,13 @@ static int zend_jit_fetch_obj(dasm_State **Dst, | jnz >1 |.cold_code |1: + | test dword [FCARG2a + offsetof(zend_property_info, flags)], ZEND_ACC_READONLY + | jz >2 + | IF_Z_TYPE FCARG1a, IS_OBJECT, >2 + | mov FCARG1a, FCARG2a + | EXT_CALL zend_readonly_property_modification_error, r0 + | jmp >9 + |2: if (flags == ZEND_FETCH_DIM_WRITE) { | SET_EX_OPLINE opline, r0 | EXT_CALL zend_jit_check_array_promotion, r0 @@ -12538,7 +12544,7 @@ static int zend_jit_fetch_obj(dasm_State **Dst, |.endif | jmp >9 } else { - ZEND_UNREACHABLE(); + ZEND_ASSERT(flags == 0); } |.code } @@ -12559,6 +12565,15 @@ static int zend_jit_fetch_obj(dasm_State **Dst, } else { | IF_UNDEF dl, >5 } + if (opline->opcode == ZEND_FETCH_OBJ_W && (prop_info->flags & ZEND_ACC_READONLY)) { + | IF_NOT_TYPE dl, IS_OBJECT, >4 + |.cold_code + |4: + | LOAD_ADDR FCARG1a, prop_info + | EXT_CALL zend_readonly_property_modification_error, r0 + | jmp >9 + |.code + } if (opline->opcode == ZEND_FETCH_OBJ_W && (opline->extended_value & ZEND_FETCH_OBJ_FLAGS) && ZEND_TYPE_IS_SET(prop_info->type)) { @@ -13677,7 +13692,7 @@ static int zend_jit_assign_obj(dasm_State **Dst, } } else { prop_addr = ZEND_ADDR_MEM_ZVAL(ZREG_FCARG1a, prop_info->offset); - if (!ce || ce_is_instanceof || !(ce->ce_flags & ZEND_ACC_IMMUTABLE) || ce->__get || ce->__set) { + if (!ce || ce_is_instanceof || !(ce->ce_flags & ZEND_ACC_IMMUTABLE) || ce->__get || ce->__set || (prop_info->flags & ZEND_ACC_READONLY)) { // Undefined property with magic __get()/__set() if (JIT_G(trigger) == ZEND_JIT_ON_HOT_TRACE) { int32_t exit_point = zend_jit_trace_get_exit_point(opline, ZEND_JIT_EXIT_TO_VM); From 4c7007955f62ac39a78173ad82f832f1f9c42578 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 15 Jul 2021 12:40:39 +0200 Subject: [PATCH 10/20] Try to add arm jit support (blindly...) --- ext/opcache/jit/zend_jit_arm64.dasc | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/ext/opcache/jit/zend_jit_arm64.dasc b/ext/opcache/jit/zend_jit_arm64.dasc index 94595b66f7057..7b0c647087af1 100644 --- a/ext/opcache/jit/zend_jit_arm64.dasc +++ b/ext/opcache/jit/zend_jit_arm64.dasc @@ -11830,7 +11830,6 @@ static int zend_jit_fetch_obj(dasm_State **Dst, | mov FCARG1x, TMP1 prop_addr = ZEND_ADDR_MEM_ZVAL(ZREG_FCARG1x, 0); if (opline->opcode == ZEND_FETCH_OBJ_W - && (opline->extended_value & ZEND_FETCH_OBJ_FLAGS) && (!ce || ce_is_instanceof || (ce->ce_flags & ZEND_ACC_HAS_TYPE_HINTS))) { uint32_t flags = opline->extended_value & ZEND_FETCH_OBJ_FLAGS; @@ -11839,6 +11838,14 @@ static int zend_jit_fetch_obj(dasm_State **Dst, | cbnz FCARG2x, >1 |.cold_code |1: + | ldr TMP1w, [FCARG2x, #offsetof(zend_property_info, flags)] + | tst TMP1w, #ZEND_ACC_READONLY + | bne >2 + | IF_TYPE REG2w, IS_OBJECT, >2 + | mov FCARG1x, FCARG2x + | EXT_CALL zend_readonly_property_modification_error, REG0 + | b >9 + |2: if (flags == ZEND_FETCH_DIM_WRITE) { | SET_EX_OPLINE opline, REG0 | EXT_CALL zend_jit_check_array_promotion, REG0 @@ -11848,7 +11855,7 @@ static int zend_jit_fetch_obj(dasm_State **Dst, | EXT_CALL zend_jit_create_typed_ref, REG0 | b >9 } else { - ZEND_UNREACHABLE(); + ZEND_ASSERT(flags == 0); } |.code } @@ -11869,6 +11876,15 @@ static int zend_jit_fetch_obj(dasm_State **Dst, } else { | IF_UNDEF REG2w, >5 } + if (opline->opcode == ZEND_FETCH_OBJ_W && (prop_info->flags & ZEND_ACC_READONLY)) { + | IF_NOT_TYPE REG2w, IS_OBJECT, >4 + |.cold_code + |4: + | LOAD_ADDR FCARG1x, prop_info + | EXT_CALL zend_readonly_property_modification_error, REG0 + | b >9 + |.code + } if (opline->opcode == ZEND_FETCH_OBJ_W && (opline->extended_value & ZEND_FETCH_OBJ_FLAGS) && ZEND_TYPE_IS_SET(prop_info->type)) { @@ -12895,7 +12911,7 @@ static int zend_jit_assign_obj(dasm_State **Dst, } } else { prop_addr = ZEND_ADDR_MEM_ZVAL(ZREG_FCARG1x, prop_info->offset); - if (!ce || ce_is_instanceof || !(ce->ce_flags & ZEND_ACC_IMMUTABLE) || ce->__get || ce->__set) { + if (!ce || ce_is_instanceof || !(ce->ce_flags & ZEND_ACC_IMMUTABLE) || ce->__get || ce->__set || (prop_info->flags & ZEND_ACC_READONLY)) { // Undefined property with magic __get()/__set() if (JIT_G(trigger) == ZEND_JIT_ON_HOT_TRACE) { int32_t exit_point = zend_jit_trace_get_exit_point(opline, ZEND_JIT_EXIT_TO_VM); From 24c2b519addd51cbc4081685e68ba0b274eb9ef1 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 15 Jul 2021 14:19:21 +0200 Subject: [PATCH 11/20] Return copy for object W/RW/UNSET There was a test checking this, but it didn't actually have the correct result, duh. --- Zend/tests/readonly_props/readonly_containing_object.phpt | 6 ++++-- Zend/zend_object_handlers.c | 8 +++++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Zend/tests/readonly_props/readonly_containing_object.phpt b/Zend/tests/readonly_props/readonly_containing_object.phpt index e82cef27e4c85..996b0ee3d44b7 100644 --- a/Zend/tests/readonly_props/readonly_containing_object.phpt +++ b/Zend/tests/readonly_props/readonly_containing_object.phpt @@ -51,9 +51,11 @@ object(stdClass)#2 (1) { ["foo"]=> int(3) } -object(stdClass)#5 (0) { +object(stdClass)#2 (1) { + ["foo"]=> + int(3) } -object(ArrayObject)#4 (1) { +object(ArrayObject)#7 (1) { ["storage":"ArrayObject":private]=> array(1) { [0]=> diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 8aa83c1698a77..b4bb487499258 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -588,7 +588,13 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int if (EXPECTED(Z_TYPE_P(retval) != IS_UNDEF)) { if (prop_info && UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY) && (type == BP_VAR_W || type == BP_VAR_RW || type == BP_VAR_UNSET)) { - if (Z_TYPE_P(retval) != IS_OBJECT) { + if (Z_TYPE_P(retval) == IS_OBJECT) { + /* For objects, R/RW/UNSET fetch modes might not actually modify object. + * Similar as with magic __get() allow them, but return the value as a copy + * to make sure no actual modification is possible. */ + ZVAL_COPY(rv, retval); + retval = rv; + } else { zend_readonly_property_modification_error(prop_info); retval = &EG(uninitialized_zval); } From 39995d88b3a9bc8c14f7a393ef4fddd0fd9c27e6 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 15 Jul 2021 14:26:05 +0200 Subject: [PATCH 12/20] Fix the issue for access via cache slot as well --- Zend/tests/readonly_props/cache_slot.phpt | 34 +++++++++++++++++++++++ Zend/zend_execute.c | 11 +++++--- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/Zend/tests/readonly_props/cache_slot.phpt b/Zend/tests/readonly_props/cache_slot.phpt index 96bc3c7b79bec..fc61874fea475 100644 --- a/Zend/tests/readonly_props/cache_slot.phpt +++ b/Zend/tests/readonly_props/cache_slot.phpt @@ -6,6 +6,7 @@ Test interaction with cache slots class Test { public readonly string $prop; public readonly array $prop2; + public readonly object $prop3; public function setProp(string $prop) { $this->prop = $prop; } @@ -13,6 +14,14 @@ class Test { $this->prop2 = []; $this->prop2[] = 1; } + public function initProp3() { + $this->prop3 = new stdClass; + $this->prop3->foo = 1; + } + public function replaceProp3() { + $ref =& $this->prop3; + $ref = new stdClass; + } } $test = new Test; @@ -38,6 +47,22 @@ try { echo $e->getMessage(), "\n"; } var_dump($test->prop2); +echo "\n"; + +$test = new Test; +$test->initProp3(); +try { + $test->replaceProp3(); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +var_dump($test->prop3); +try { + $test->replaceProp3(); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +var_dump($test->prop3); ?> --EXPECT-- @@ -49,3 +74,12 @@ Cannot modify readonly property Test::$prop2 Cannot modify readonly property Test::$prop2 array(0) { } + +object(stdClass)#3 (1) { + ["foo"]=> + int(1) +} +object(stdClass)#3 (1) { + ["foo"]=> + int(1) +} diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 5385f5bbce5bd..228b6d9bdb7fd 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -2840,11 +2840,14 @@ static zend_always_inline void zend_fetch_property_address(zval *result, zval *c ZVAL_INDIRECT(result, ptr); zend_property_info *prop_info = CACHED_PTR_EX(cache_slot + 2); if (prop_info) { - if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY) - && Z_TYPE_P(ptr) != IS_OBJECT) { + if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) { ZEND_ASSERT(type == BP_VAR_W || type == BP_VAR_RW || type == BP_VAR_UNSET); - zend_readonly_property_modification_error(prop_info); - ZVAL_ERROR(result); + if (Z_TYPE_P(ptr) == IS_OBJECT) { + ZVAL_COPY(result, ptr); + } else { + zend_readonly_property_modification_error(prop_info); + ZVAL_ERROR(result); + } return; } if (flags) { From 1b13532772792e8e7d22987d082235ebd8922c1f Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 15 Jul 2021 14:48:12 +0200 Subject: [PATCH 13/20] Update x86 jit implementation --- ext/opcache/jit/zend_jit_x86.dasc | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/ext/opcache/jit/zend_jit_x86.dasc b/ext/opcache/jit/zend_jit_x86.dasc index 890204fac4af2..7650784fca86a 100644 --- a/ext/opcache/jit/zend_jit_x86.dasc +++ b/ext/opcache/jit/zend_jit_x86.dasc @@ -12521,12 +12521,18 @@ static int zend_jit_fetch_obj(dasm_State **Dst, |.cold_code |1: | test dword [FCARG2a + offsetof(zend_property_info, flags)], ZEND_ACC_READONLY - | jz >2 - | IF_Z_TYPE FCARG1a, IS_OBJECT, >2 + | jz >3 + | IF_NOT_Z_TYPE FCARG1a, IS_OBJECT, >2 + | GET_Z_PTR r0, FCARG1a + | GC_ADDREF r0 + | SET_ZVAL_PTR res_addr, r0 + | SET_ZVAL_TYPE_INFO res_addr, IS_OBJECT_EX + | jmp >9 + |2: | mov FCARG1a, FCARG2a | EXT_CALL zend_readonly_property_modification_error, r0 | jmp >9 - |2: + |3: if (flags == ZEND_FETCH_DIM_WRITE) { | SET_EX_OPLINE opline, r0 | EXT_CALL zend_jit_check_array_promotion, r0 @@ -12567,6 +12573,11 @@ static int zend_jit_fetch_obj(dasm_State **Dst, } if (opline->opcode == ZEND_FETCH_OBJ_W && (prop_info->flags & ZEND_ACC_READONLY)) { | IF_NOT_TYPE dl, IS_OBJECT, >4 + | GET_ZVAL_PTR r0, prop_addr + | GC_ADDREF r0 + | SET_ZVAL_PTR res_addr, r0 + | SET_ZVAL_TYPE_INFO res_addr, IS_OBJECT_EX + | jmp >9 |.cold_code |4: | LOAD_ADDR FCARG1a, prop_info From 2628e411ef7cddcf175f65037c2db1d265c37136 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 15 Jul 2021 15:04:20 +0200 Subject: [PATCH 14/20] Try to update arm jit implementation --- ext/opcache/jit/zend_jit_arm64.dasc | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/ext/opcache/jit/zend_jit_arm64.dasc b/ext/opcache/jit/zend_jit_arm64.dasc index 7b0c647087af1..ccf8c203c6983 100644 --- a/ext/opcache/jit/zend_jit_arm64.dasc +++ b/ext/opcache/jit/zend_jit_arm64.dasc @@ -11840,12 +11840,18 @@ static int zend_jit_fetch_obj(dasm_State **Dst, |1: | ldr TMP1w, [FCARG2x, #offsetof(zend_property_info, flags)] | tst TMP1w, #ZEND_ACC_READONLY - | bne >2 - | IF_TYPE REG2w, IS_OBJECT, >2 + | bne >3 + | IF_NOT_TYPE REG2w, IS_OBJECT, >2 + | GET_Z_PTR REG2, FCARG1x, TMP1 + | GC_ADDREF REG2, TMP1w + | SET_ZVAL_PTR res_addr, REG2, TMP1 + | SET_ZVAL_TYPE_INFO res_addr, IS_OBJECT_EX, TMP1w, TMP2 + | b >9 + |2: | mov FCARG1x, FCARG2x | EXT_CALL zend_readonly_property_modification_error, REG0 | b >9 - |2: + |3: if (flags == ZEND_FETCH_DIM_WRITE) { | SET_EX_OPLINE opline, REG0 | EXT_CALL zend_jit_check_array_promotion, REG0 @@ -11878,6 +11884,11 @@ static int zend_jit_fetch_obj(dasm_State **Dst, } if (opline->opcode == ZEND_FETCH_OBJ_W && (prop_info->flags & ZEND_ACC_READONLY)) { | IF_NOT_TYPE REG2w, IS_OBJECT, >4 + | GET_ZVAL_PTR REG2, prop_addr, TMP1 + | GC_ADDREF REG2, TMP1w + | SET_ZVAL_PTR res_addr, REG2, TMP1 + | SET_ZVAL_TYPE_INFO res_addr, IS_OBJECT_EX, TMP1w, TMP2 + | b >9 |.cold_code |4: | LOAD_ADDR FCARG1x, prop_info From ff310d69f79773f03f4ea416f5495f11f8bbe40c Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 15 Jul 2021 15:36:41 +0200 Subject: [PATCH 15/20] Set to IS_ERROR --- ext/opcache/jit/zend_jit_arm64.dasc | 4 +++- ext/opcache/jit/zend_jit_x86.dasc | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ext/opcache/jit/zend_jit_arm64.dasc b/ext/opcache/jit/zend_jit_arm64.dasc index ccf8c203c6983..9e97027ef5b55 100644 --- a/ext/opcache/jit/zend_jit_arm64.dasc +++ b/ext/opcache/jit/zend_jit_arm64.dasc @@ -11842,7 +11842,7 @@ static int zend_jit_fetch_obj(dasm_State **Dst, | tst TMP1w, #ZEND_ACC_READONLY | bne >3 | IF_NOT_TYPE REG2w, IS_OBJECT, >2 - | GET_Z_PTR REG2, FCARG1x, TMP1 + | GET_Z_PTR REG2, FCARG1x | GC_ADDREF REG2, TMP1w | SET_ZVAL_PTR res_addr, REG2, TMP1 | SET_ZVAL_TYPE_INFO res_addr, IS_OBJECT_EX, TMP1w, TMP2 @@ -11850,6 +11850,7 @@ static int zend_jit_fetch_obj(dasm_State **Dst, |2: | mov FCARG1x, FCARG2x | EXT_CALL zend_readonly_property_modification_error, REG0 + | SET_ZVAL_TYPE_INFO res_addr, _IS_ERROR, TMP1w, TMP2 | b >9 |3: if (flags == ZEND_FETCH_DIM_WRITE) { @@ -11893,6 +11894,7 @@ static int zend_jit_fetch_obj(dasm_State **Dst, |4: | LOAD_ADDR FCARG1x, prop_info | EXT_CALL zend_readonly_property_modification_error, REG0 + | SET_ZVAL_TYPE_INFO res_addr, _IS_ERROR, TMP1w, TMP2 | b >9 |.code } diff --git a/ext/opcache/jit/zend_jit_x86.dasc b/ext/opcache/jit/zend_jit_x86.dasc index 7650784fca86a..8558459863df4 100644 --- a/ext/opcache/jit/zend_jit_x86.dasc +++ b/ext/opcache/jit/zend_jit_x86.dasc @@ -12531,6 +12531,7 @@ static int zend_jit_fetch_obj(dasm_State **Dst, |2: | mov FCARG1a, FCARG2a | EXT_CALL zend_readonly_property_modification_error, r0 + | SET_ZVAL_TYPE_INFO res_addr, _IS_ERROR | jmp >9 |3: if (flags == ZEND_FETCH_DIM_WRITE) { @@ -12582,6 +12583,7 @@ static int zend_jit_fetch_obj(dasm_State **Dst, |4: | LOAD_ADDR FCARG1a, prop_info | EXT_CALL zend_readonly_property_modification_error, r0 + | SET_ZVAL_TYPE_INFO res_addr, _IS_ERROR | jmp >9 |.code } From b25bcb05f7be027755870773a843f0f51595b258 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 16 Jul 2021 15:59:19 +0200 Subject: [PATCH 16/20] Use IS_OBJECT_EX for comparison There's only one possible set of flags for objects, and the assigned type is hardcoded anyway. --- ext/opcache/jit/zend_jit_arm64.dasc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/opcache/jit/zend_jit_arm64.dasc b/ext/opcache/jit/zend_jit_arm64.dasc index 9e97027ef5b55..30f4d08e38b5f 100644 --- a/ext/opcache/jit/zend_jit_arm64.dasc +++ b/ext/opcache/jit/zend_jit_arm64.dasc @@ -11841,7 +11841,7 @@ static int zend_jit_fetch_obj(dasm_State **Dst, | ldr TMP1w, [FCARG2x, #offsetof(zend_property_info, flags)] | tst TMP1w, #ZEND_ACC_READONLY | bne >3 - | IF_NOT_TYPE REG2w, IS_OBJECT, >2 + | IF_NOT_TYPE REG2w, IS_OBJECT_EX, >2 | GET_Z_PTR REG2, FCARG1x | GC_ADDREF REG2, TMP1w | SET_ZVAL_PTR res_addr, REG2, TMP1 @@ -11884,7 +11884,7 @@ static int zend_jit_fetch_obj(dasm_State **Dst, | IF_UNDEF REG2w, >5 } if (opline->opcode == ZEND_FETCH_OBJ_W && (prop_info->flags & ZEND_ACC_READONLY)) { - | IF_NOT_TYPE REG2w, IS_OBJECT, >4 + | IF_NOT_TYPE REG2w, IS_OBJECT_EX, >4 | GET_ZVAL_PTR REG2, prop_addr, TMP1 | GC_ADDREF REG2, TMP1w | SET_ZVAL_PTR res_addr, REG2, TMP1 From 5f19b93891720e7d7d5f2a18ce87dc00ef3299ec Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 19 Jul 2021 21:23:01 +0200 Subject: [PATCH 17/20] Fix incorrect branch direction --- ext/opcache/jit/zend_jit_arm64.dasc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/opcache/jit/zend_jit_arm64.dasc b/ext/opcache/jit/zend_jit_arm64.dasc index 30f4d08e38b5f..319516b121aa0 100644 --- a/ext/opcache/jit/zend_jit_arm64.dasc +++ b/ext/opcache/jit/zend_jit_arm64.dasc @@ -11840,7 +11840,7 @@ static int zend_jit_fetch_obj(dasm_State **Dst, |1: | ldr TMP1w, [FCARG2x, #offsetof(zend_property_info, flags)] | tst TMP1w, #ZEND_ACC_READONLY - | bne >3 + | beq >3 | IF_NOT_TYPE REG2w, IS_OBJECT_EX, >2 | GET_Z_PTR REG2, FCARG1x | GC_ADDREF REG2, TMP1w From 700559d45765b56ca8c9a5d08e41a9525474a004 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 20 Jul 2021 10:00:07 +0200 Subject: [PATCH 18/20] flags -> property_flags --- Zend/zend_compile.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index b68302e1966d2..c79e24e39225d 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -6598,7 +6598,7 @@ void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32_t fall zend_string *name = zval_make_interned_string(zend_ast_get_zval(var_ast)); bool is_ref = (param_ast->attr & ZEND_PARAM_REF) != 0; bool is_variadic = (param_ast->attr & ZEND_PARAM_VARIADIC) != 0; - uint32_t flags = param_ast->attr & (ZEND_ACC_PPP_MASK | ZEND_ACC_READONLY); + uint32_t property_flags = param_ast->attr & (ZEND_ACC_PPP_MASK | ZEND_ACC_READONLY); znode var_node, default_node; zend_uchar opcode; @@ -6680,7 +6680,7 @@ void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32_t fall if (type_ast) { uint32_t default_type = *default_ast_ptr ? Z_TYPE(default_node.u.constant) : IS_UNDEF; - bool force_nullable = default_type == IS_NULL && !flags; + bool force_nullable = default_type == IS_NULL && !property_flags; op_array->fn_flags |= ZEND_ACC_HAS_TYPE_HINTS; arg_info->type = zend_compile_typename(type_ast, force_nullable); @@ -6723,14 +6723,14 @@ void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32_t fall } uint32_t arg_info_flags = _ZEND_ARG_INFO_FLAGS(is_ref, is_variadic, /* is_tentative */ 0) - | (flags ? _ZEND_IS_PROMOTED_BIT : 0); + | (property_flags ? _ZEND_IS_PROMOTED_BIT : 0); ZEND_TYPE_FULL_MASK(arg_info->type) |= arg_info_flags; if (opcode == ZEND_RECV) { opline->op2.num = type_ast ? ZEND_TYPE_FULL_MASK(arg_info->type) : MAY_BE_ANY; } - if (flags) { + if (property_flags) { zend_op_array *op_array = CG(active_op_array); zend_class_entry *scope = op_array->scope; bool is_ctor = @@ -6777,7 +6777,7 @@ void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32_t fall zend_string *doc_comment = doc_comment_ast ? zend_string_copy(zend_ast_get_str(doc_comment_ast)) : NULL; zend_property_info *prop = zend_declare_typed_property( - scope, name, &default_value, flags | ZEND_ACC_PROMOTED, doc_comment, type); + scope, name, &default_value, property_flags | ZEND_ACC_PROMOTED, doc_comment, type); if (attributes_ast) { zend_compile_attributes( &prop->attributes, attributes_ast, 0, ZEND_ATTRIBUTE_TARGET_PROPERTY); From 4725a9b042d28b3c1d81a8abb4ae7b0848fb2d4d Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 20 Jul 2021 10:01:59 +0200 Subject: [PATCH 19/20] Copy comment --- Zend/zend_execute.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 228b6d9bdb7fd..3dbdd966de82e 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -2841,6 +2841,9 @@ static zend_always_inline void zend_fetch_property_address(zval *result, zval *c zend_property_info *prop_info = CACHED_PTR_EX(cache_slot + 2); if (prop_info) { if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) { + /* For objects, R/RW/UNSET fetch modes might not actually modify object. + * Similar as with magic __get() allow them, but return the value as a copy + * to make sure no actual modification is possible. */ ZEND_ASSERT(type == BP_VAR_W || type == BP_VAR_RW || type == BP_VAR_UNSET); if (Z_TYPE_P(ptr) == IS_OBJECT) { ZVAL_COPY(result, ptr); From 968a399b0a4bcfb7404ca67250ea0a85a71205b0 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 20 Jul 2021 11:11:35 +0200 Subject: [PATCH 20/20] Add additional JIT test, add missing SET_EX_OPLINE --- Zend/tests/readonly_props/cache_slot.phpt | 43 +++++++++++++++++++++-- ext/opcache/jit/zend_jit_arm64.dasc | 2 ++ ext/opcache/jit/zend_jit_x86.dasc | 2 ++ 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/Zend/tests/readonly_props/cache_slot.phpt b/Zend/tests/readonly_props/cache_slot.phpt index fc61874fea475..75c26a2d1808e 100644 --- a/Zend/tests/readonly_props/cache_slot.phpt +++ b/Zend/tests/readonly_props/cache_slot.phpt @@ -51,17 +51,40 @@ echo "\n"; $test = new Test; $test->initProp3(); +$test->replaceProp3(); +var_dump($test->prop3); +$test->replaceProp3(); +var_dump($test->prop3); +echo "\n"; + +// Test variations using closure rebinding, so we have unknown property_info in JIT. +$test = new Test; +(function() { $this->prop2 = []; })->call($test); +$appendProp2 = (function() { + $this->prop2[] = 1; +})->bindTo($test, Test::class); try { - $test->replaceProp3(); + $appendProp2(); } catch (Error $e) { echo $e->getMessage(), "\n"; } -var_dump($test->prop3); try { - $test->replaceProp3(); + $appendProp2(); } catch (Error $e) { echo $e->getMessage(), "\n"; } +var_dump($test->prop2); +echo "\n"; + +$test = new Test; +$replaceProp3 = (function() { + $ref =& $this->prop3; + $ref = new stdClass; +})->bindTo($test, Test::class); +$test->initProp3(); +$replaceProp3(); +var_dump($test->prop3); +$replaceProp3(); var_dump($test->prop3); ?> @@ -83,3 +106,17 @@ object(stdClass)#3 (1) { ["foo"]=> int(1) } + +Cannot modify readonly property Test::$prop2 +Cannot modify readonly property Test::$prop2 +array(0) { +} + +object(stdClass)#5 (1) { + ["foo"]=> + int(1) +} +object(stdClass)#5 (1) { + ["foo"]=> + int(1) +} diff --git a/ext/opcache/jit/zend_jit_arm64.dasc b/ext/opcache/jit/zend_jit_arm64.dasc index 319516b121aa0..a2e79b88f38b2 100644 --- a/ext/opcache/jit/zend_jit_arm64.dasc +++ b/ext/opcache/jit/zend_jit_arm64.dasc @@ -11849,6 +11849,7 @@ static int zend_jit_fetch_obj(dasm_State **Dst, | b >9 |2: | mov FCARG1x, FCARG2x + | SET_EX_OPLINE opline, REG0 | EXT_CALL zend_readonly_property_modification_error, REG0 | SET_ZVAL_TYPE_INFO res_addr, _IS_ERROR, TMP1w, TMP2 | b >9 @@ -11893,6 +11894,7 @@ static int zend_jit_fetch_obj(dasm_State **Dst, |.cold_code |4: | LOAD_ADDR FCARG1x, prop_info + | SET_EX_OPLINE opline, REG0 | EXT_CALL zend_readonly_property_modification_error, REG0 | SET_ZVAL_TYPE_INFO res_addr, _IS_ERROR, TMP1w, TMP2 | b >9 diff --git a/ext/opcache/jit/zend_jit_x86.dasc b/ext/opcache/jit/zend_jit_x86.dasc index 8558459863df4..b426b3de19338 100644 --- a/ext/opcache/jit/zend_jit_x86.dasc +++ b/ext/opcache/jit/zend_jit_x86.dasc @@ -12530,6 +12530,7 @@ static int zend_jit_fetch_obj(dasm_State **Dst, | jmp >9 |2: | mov FCARG1a, FCARG2a + | SET_EX_OPLINE opline, r0 | EXT_CALL zend_readonly_property_modification_error, r0 | SET_ZVAL_TYPE_INFO res_addr, _IS_ERROR | jmp >9 @@ -12582,6 +12583,7 @@ static int zend_jit_fetch_obj(dasm_State **Dst, |.cold_code |4: | LOAD_ADDR FCARG1a, prop_info + | SET_EX_OPLINE opline, r0 | EXT_CALL zend_readonly_property_modification_error, r0 | SET_ZVAL_TYPE_INFO res_addr, _IS_ERROR | jmp >9