Skip to content

Make property resolution direction-aware; amortize asymmetric visibility set checks into the runtime cache #22709

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
hollyschilling wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Make property resolution direction-aware; amortize asymmetric visibility set checks into the runtime cache #22709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dfb5e8c
Make property resolution direction-aware; amortize asymmetric set che…
hollyschilling Jul 12, 2026
495d68d
Fix set-visibility bypass via optimizer-shared property cache slots
hollyschilling Jul 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions Zend/Optimizer/compact_literals.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,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_property_slot;

if (op_array->last_literal) {
uint32_t j;
Expand Down Expand Up @@ -438,14 +438,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_property_slot = jmp_slot + j;

/* Update opcodes to use new literals table */
cache_size = zend_op_array_extension_handles * sizeof(void*);
Expand Down Expand Up @@ -499,6 +500,26 @@ void zend_optimizer_compact_literals(zend_op_array *op_array, zend_optimizer_ctx
}
break;
case ZEND_ASSIGN_OBJ:
/* ASSIGN_OBJ must not share its cache slot with other
* property opcodes: the asymmetric set-visibility check
* runs when the slot is populated by the write handler,
* and the cached direct-assign fast path relies on that
* (see zend_assign_to_typed_prop_granted()). Slots are
* shared among ASSIGN_OBJ oplines only, which all
* populate through the set-checked write handler. */
if (opline->op2_type == IS_CONST) {
if (opline->op1_type == IS_UNUSED &&
assign_property_slot[opline->op2.constant] >= 0) {
opline->extended_value = assign_property_slot[opline->op2.constant];
} else {
opline->extended_value = cache_size;
cache_size += 3 * sizeof(void *);
if (opline->op1_type == IS_UNUSED) {
assign_property_slot[opline->op2.constant] = opline->extended_value;
}
}
}
break;
case ZEND_ASSIGN_OBJ_REF:
case ZEND_FETCH_OBJ_R:
case ZEND_FETCH_OBJ_W:
Expand Down
34 changes: 34 additions & 0 deletions Zend/tests/asymmetric_visibility/optimizer_shared_cache_slot.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
28 changes: 25 additions & 3 deletions Zend/zend_execute.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1070,16 +1070,20 @@ 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_always_inline zval* zend_assign_to_typed_prop_ex(const zend_property_info *info, zval *property_val, zval *value, zend_refcounted **garbage_ptr, bool check_set_access EXECUTE_DATA_DC)
{
zval tmp;
const uint32_t guard_mask =
ZEND_ACC_READONLY | (check_set_access ? ZEND_ACC_PPP_SET_MASK : 0);

if (UNEXPECTED(info->flags & (ZEND_ACC_READONLY|ZEND_ACC_PPP_SET_MASK))) {
if (UNEXPECTED(info->flags & guard_mask)) {
if ((info->flags & ZEND_ACC_READONLY) && !(Z_PROP_FLAG_P(property_val) & IS_PROP_REINITABLE)) {
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_set_access
&& (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);
}
Expand All @@ -1098,6 +1102,24 @@ static zend_never_inline zval* zend_assign_to_typed_prop(const zend_property_inf
return zend_assign_to_variable_ex(property_val, &tmp, IS_TMP_VAR, EX_USES_STRICT_TYPES(), garbage_ptr);
}

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)
{
return zend_assign_to_typed_prop_ex(info, property_val, value, garbage_ptr, true EXECUTE_DATA_CC);
}

/* For write sites whose runtime cache slot produced `info`: the slot is only
* populated when set access was verified at resolution time
* (zend_get_property_offset()), so the per-write asymmetric visibility check
* is redundant. */
static zend_never_inline zval* zend_assign_to_typed_prop_granted(const zend_property_info *info, zval *property_val, zval *value, zend_refcounted **garbage_ptr EXECUTE_DATA_DC)
{
ZEND_ASSERT(!(info->flags & ZEND_ACC_PPP_SET_MASK)
|| (info->flags & ZEND_ACC_PUBLIC_SET)
|| zend_asymmetric_property_has_set_access(info));

return zend_assign_to_typed_prop_ex(info, property_val, value, garbage_ptr, false EXECUTE_DATA_CC);
}

static zend_always_inline bool zend_value_instanceof_static(const zval *zv) {
if (Z_TYPE_P(zv) != IS_OBJECT) {
return 0;
Expand Down
43 changes: 36 additions & 7 deletions Zend/zend_object_handlers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,15 @@ static zend_always_inline const zend_class_entry *get_fake_or_executed_scope(voi
}
}

static zend_always_inline uintptr_t zend_get_property_offset(zend_class_entry *ce, zend_string *member, int silent, void **cache_slot, const zend_property_info **info_ptr) /* {{{ */
/* Resolve a property for reading (write_access == false) or for
* writing/unsetting (write_access == true). Write-kind resolution verifies
* asymmetric set visibility at population time: when the running scope lacks
* set access, the result is still returned (the caller's state-dependent
* slow path decides between an error and the __set fallback), but the
* runtime cache is NOT populated. A populated write-site cache slot
* therefore guarantees set access, which lets the VM's cached direct-assign
* fast path skip the per-write asymmetric visibility check. */
static zend_always_inline uintptr_t zend_get_property_offset(zend_class_entry *ce, zend_string *member, int silent, void **cache_slot, const zend_property_info **info_ptr, bool write_access) /* {{{ */
{
zval *zv;
zend_property_info *property_info;
Expand Down Expand Up @@ -440,6 +448,27 @@ static zend_always_inline uintptr_t zend_get_property_offset(zend_class_entry *c
return ZEND_DYNAMIC_PROPERTY_OFFSET;
}

if (write_access
&& UNEXPECTED(flags & ZEND_ACC_PPP_SET_MASK)
&& !(flags & ZEND_ACC_PUBLIC_SET)) {
const zend_class_entry *scope = get_fake_or_executed_scope();

if (property_info->ce != scope
&& (!(flags & ZEND_ACC_PROTECTED_SET)
|| !is_protected_compatible_scope(property_info->prototype->ce, scope))) {
/* Set access denied for this scope: resolve as usual, but keep
* the cache slot empty so every such write stays on the slow
* path with its state-dependent handling. Invalidate the key
* too: downstream code (e.g. the hooked simple-write marking)
* assumes resolution populated the slot and would otherwise
* flag a stale polymorphic entry. */
if (cache_slot) {
CACHE_PTR_EX(cache_slot, NULL);
cache_slot = NULL;
}
}
}

if (property_info->hooks) {
*info_ptr = property_info;
if (cache_slot) {
Expand Down Expand Up @@ -468,7 +497,7 @@ static ZEND_COLD void zend_wrong_offset(zend_class_entry *ce, zend_string *membe
const zend_property_info *dummy;

/* Trigger the correct error */
zend_get_property_offset(ce, member, 0, NULL, &dummy);
zend_get_property_offset(ce, member, 0, NULL, &dummy, false);
}
/* }}} */

Expand Down Expand Up @@ -749,7 +778,7 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int
#endif

/* make zend_get_property_info silent if we have getter - we may want to use it */
property_offset = zend_get_property_offset(zobj->ce, name, (type == BP_VAR_IS) || (zobj->ce->__get != NULL), cache_slot, &prop_info);
property_offset = zend_get_property_offset(zobj->ce, name, (type == BP_VAR_IS) || (zobj->ce->__get != NULL), cache_slot, &prop_info, false);

if (EXPECTED(IS_VALID_PROPERTY_OFFSET(property_offset))) {
try_again:
Expand Down Expand Up @@ -1072,7 +1101,7 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva
uint32_t *guard = NULL;
ZEND_ASSERT(!Z_ISREF_P(value));

property_offset = zend_get_property_offset(zobj->ce, name, (zobj->ce->__set != NULL), cache_slot, &prop_info);
property_offset = zend_get_property_offset(zobj->ce, name, (zobj->ce->__set != NULL), cache_slot, &prop_info, true);

if (EXPECTED(IS_VALID_PROPERTY_OFFSET(property_offset))) {
try_again:
Expand Down Expand Up @@ -1434,7 +1463,7 @@ ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *zobj, zend_string *nam
fprintf(stderr, "Ptr object #%d property: %s\n", zobj->handle, ZSTR_VAL(name));
#endif

property_offset = zend_get_property_offset(zobj->ce, name, (zobj->ce->__get != NULL), cache_slot, &prop_info);
property_offset = zend_get_property_offset(zobj->ce, name, (zobj->ce->__get != NULL), cache_slot, &prop_info, true);

if (EXPECTED(IS_VALID_PROPERTY_OFFSET(property_offset))) {
try_again:
Expand Down Expand Up @@ -1562,7 +1591,7 @@ ZEND_API void zend_std_unset_property(zend_object *zobj, zend_string *name, void
const zend_property_info *prop_info = NULL;
uint32_t *guard = NULL;

property_offset = zend_get_property_offset(zobj->ce, name, (zobj->ce->__unset != NULL), cache_slot, &prop_info);
property_offset = zend_get_property_offset(zobj->ce, name, (zobj->ce->__unset != NULL), cache_slot, &prop_info, true);

if (EXPECTED(IS_VALID_PROPERTY_OFFSET(property_offset))) {
zval *slot = OBJ_PROP(zobj, property_offset);
Expand Down Expand Up @@ -2372,7 +2401,7 @@ ZEND_API int zend_std_has_property(zend_object *zobj, zend_string *name, int has
uintptr_t property_offset;
const zend_property_info *prop_info = NULL;

property_offset = zend_get_property_offset(zobj->ce, name, 1, cache_slot, &prop_info);
property_offset = zend_get_property_offset(zobj->ce, name, 1, cache_slot, &prop_info, false);

if (EXPECTED(IS_VALID_PROPERTY_OFFSET(property_offset))) {
try_again:
Expand Down
4 changes: 3 additions & 1 deletion Zend/zend_vm_def.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2524,7 +2524,9 @@ 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);
/* Cache-hit path: set access was verified when the
* slot was populated. */
value = zend_assign_to_typed_prop_granted(prop_info, property_val, value, &garbage EXECUTE_DATA_CC);
ZEND_VM_C_GOTO(free_and_exit_assign_obj);
} else {
ZEND_VM_C_LABEL(fast_assign_obj):
Expand Down
Loading
Loading