Improve asymmetric visibility write performance in fast path (GH-22709)

Skip the visibility check in the fast path of ZEND_ASSIGN_OBJ when the cache slot is primed. For this to work, ZEND_FETCH_OBJ_R and the other object handlers must not share a cache slot with ZEND_ASSIGN_OBJ. Also reset the cache slot when the visibility check has failed in the slow path.

Co-authored-by: Ilija Tovilo <ilija.tovilo@me.com>

Author: hollyschilling

Zend/Optimizer/compact_literals.c

@@ -120,7 +120,7 @@ void zend_optimizer_compact_literals(zend_op_array *op_array, zend_optimizer_ctx
 	HashTable hash;
 	zend_string *key = NULL;
 	void *checkpoint = zend_arena_checkpoint(ctx->arena);
-	int *const_slot, *class_slot, *func_slot, *bind_var_slot, *property_slot, *method_slot, *jmp_slot;
+	int *const_slot, *class_slot, *func_slot, *bind_var_slot, *property_slot, *method_slot, *jmp_slot, *assign_obj_slots;
 
 	if (op_array->last_literal) {
 		uint32_t j;
@@ -439,14 +439,15 @@ void zend_optimizer_compact_literals(zend_op_array *op_array, zend_optimizer_ctx
 		zend_hash_clean(&hash);
 		op_array->last_literal = j;
 
-		const_slot = zend_arena_alloc(&ctx->arena, j * 7 * sizeof(int));
-		memset(const_slot, -1, j * 7 * sizeof(int));
+		const_slot = zend_arena_alloc(&ctx->arena, j * 8 * sizeof(int));
+		memset(const_slot, -1, j * 8 * sizeof(int));
 		class_slot = const_slot + j;
 		func_slot = class_slot + j;
 		bind_var_slot = func_slot + j;
 		property_slot = bind_var_slot + j;
 		method_slot = property_slot + j;
 		jmp_slot = method_slot + j;
+		assign_obj_slots = jmp_slot + j;
 
 		/* Update opcodes to use new literals table */
 		cache_size = zend_op_array_extension_handles * sizeof(void*);
@@ -500,6 +501,19 @@ void zend_optimizer_compact_literals(zend_op_array *op_array, zend_optimizer_ctx
 					}
 					break;
 				case ZEND_ASSIGN_OBJ:
+					if (opline->op2_type == IS_CONST) {
+						if (opline->op1_type == IS_UNUSED &&
+							assign_obj_slots[opline->op2.constant] >= 0) {
+							opline->extended_value = assign_obj_slots[opline->op2.constant];
+						} else {
+							opline->extended_value = cache_size;
+							cache_size += 3 * sizeof(void *);
+							if (opline->op1_type == IS_UNUSED) {
+								assign_obj_slots[opline->op2.constant] = opline->extended_value;
+							}
+						}
+					}
+					break;
 				case ZEND_ASSIGN_OBJ_REF:
 				case ZEND_FETCH_OBJ_R:
 				case ZEND_FETCH_OBJ_W:

Zend/tests/asymmetric_visibility/optimizer_shared_cache_slot.phpt

@@ -0,0 +1,34 @@
+--TEST--
+Asymmetric set visibility survives optimizer cache-slot sharing between $this reads and writes
+--FILE--
+<?php
+class P {
+    public private(set) string $prop = 'default';
+}
+
+class C extends P {
+    public function test() {
+        // The read populates a runtime cache slot for $this->prop; the write
+        // below must not reuse that (read-kind) resolution to bypass the
+        // set-visibility check when the optimizer shares property slots.
+        var_dump($this->prop);
+        try {
+            $this->prop = 'overwritten';
+        } catch (Error $e) {
+            echo $e->getMessage(), "\n";
+        }
+        var_dump($this->prop);
+    }
+}
+
+$c = new C;
+$c->test();
+$c->test();
+?>
+--EXPECT--
+string(7) "default"
+Cannot modify private(set) property P::$prop from scope C
+string(7) "default"
+string(7) "default"
+Cannot modify private(set) property P::$prop from scope C
+string(7) "default"

Zend/zend_execute.c

@@ -1065,7 +1065,7 @@ ZEND_API bool zend_never_inline zend_verify_property_type(const zend_property_in
 	return i_zend_verify_property_type(info, property, strict);
 }
 
-static zend_never_inline zval* zend_assign_to_typed_prop(const zend_property_info *info, zval *property_val, zval *value, zend_refcounted **garbage_ptr EXECUTE_DATA_DC)
+static zend_never_inline zval* zend_assign_to_typed_prop(const zend_property_info *info, zval *property_val, zval *value, zend_refcounted **garbage_ptr, bool check_writable EXECUTE_DATA_DC)
 {
 	zval tmp;
 
@@ -1074,7 +1074,7 @@ static zend_never_inline zval* zend_assign_to_typed_prop(const zend_property_inf
 			zend_readonly_property_modification_error(info);
 			return &EG(uninitialized_zval);
 		}
-		if (info->flags & ZEND_ACC_PPP_SET_MASK && !zend_asymmetric_property_has_set_access(info)) {
+		if (check_writable && (info->flags & ZEND_ACC_PPP_SET_MASK) && !zend_asymmetric_property_has_set_access(info)) {
 			zend_asymmetric_visibility_property_modification_error(info, "modify");
 			return &EG(uninitialized_zval);
 		}

Zend/zend_object_handlers.c

@@ -1138,6 +1138,11 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva
 				if ((prop_info->flags & ZEND_ACC_PPP_SET_MASK) && !zend_asymmetric_property_has_set_access(prop_info)) {
 					zend_asymmetric_visibility_property_modification_error(prop_info, "modify");
 					variable_ptr = &EG(error_zval);
+					if (cache_slot) {
+						/* Reset cache slot to dodge fast path in next execution. */
+						CACHE_POLYMORPHIC_PTR_EX(cache_slot, NULL, NULL);
+						CACHE_PTR_EX(cache_slot + 2, NULL);
+					}
 					goto exit;
 				}
 			}

Zend/zend_vm_def.h

@@ -2524,7 +2524,7 @@ ZEND_VM_C_LABEL(assign_obj_simple):
 				property_val = OBJ_PROP(zobj, prop_offset);
 				if (Z_TYPE_P(property_val) != IS_UNDEF) {
 					if (prop_info != NULL) {
-						value = zend_assign_to_typed_prop(prop_info, property_val, value, &garbage EXECUTE_DATA_CC);
+						value = zend_assign_to_typed_prop(prop_info, property_val, value, &garbage, /* check_writable */ false EXECUTE_DATA_CC);
 						ZEND_VM_C_GOTO(free_and_exit_assign_obj);
 					} else {
 ZEND_VM_C_LABEL(fast_assign_obj):
@@ -2659,7 +2659,7 @@ ZEND_VM_HANDLER(25, ZEND_ASSIGN_STATIC_PROP, ANY, ANY, CACHE_SLOT, SPEC(OP_DATA=
 	value = GET_OP_DATA_ZVAL_PTR(BP_VAR_R);
 
 	if (ZEND_TYPE_IS_SET(prop_info->type)) {
-		value = zend_assign_to_typed_prop(prop_info, prop, value, &garbage EXECUTE_DATA_CC);
+		value = zend_assign_to_typed_prop(prop_info, prop, value, &garbage, /* check_writable */ true EXECUTE_DATA_CC);
 		FREE_OP_DATA();
 	} else {
 		value = zend_assign_to_variable_ex(prop, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES(), &garbage);