Skip to content

Commit 72a089b

Browse files
committed
Simplify implementation
1 parent ecf55f0 commit 72a089b

5 files changed

Lines changed: 87 additions & 256 deletions

File tree

‎Zend/Optimizer/compact_literals.c‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ void zend_optimizer_compact_literals(zend_op_array *op_array, zend_optimizer_ctx
119119
HashTable hash;
120120
zend_string *key = NULL;
121121
void *checkpoint = zend_arena_checkpoint(ctx->arena);
122-
int *const_slot, *class_slot, *func_slot, *bind_var_slot, *property_slot, *method_slot, *jmp_slot, *assign_property_slot;
122+
int *const_slot, *class_slot, *func_slot, *bind_var_slot, *property_slot, *method_slot, *jmp_slot, *assign_obj_slots;
123123

124124
if (op_array->last_literal) {
125125
uint32_t j;
@@ -446,7 +446,7 @@ void zend_optimizer_compact_literals(zend_op_array *op_array, zend_optimizer_ctx
446446
property_slot = bind_var_slot + j;
447447
method_slot = property_slot + j;
448448
jmp_slot = method_slot + j;
449-
assign_property_slot = jmp_slot + j;
449+
assign_obj_slots = jmp_slot + j;
450450

451451
/* Update opcodes to use new literals table */
452452
cache_size = zend_op_array_extension_handles * sizeof(void*);
@@ -500,22 +500,15 @@ void zend_optimizer_compact_literals(zend_op_array *op_array, zend_optimizer_ctx
500500
}
501501
break;
502502
case ZEND_ASSIGN_OBJ:
503-
/* ASSIGN_OBJ must not share its cache slot with other
504-
* property opcodes: the asymmetric set-visibility check
505-
* runs when the slot is populated by the write handler,
506-
* and the cached direct-assign fast path relies on that
507-
* (see zend_assign_to_typed_prop_granted()). Slots are
508-
* shared among ASSIGN_OBJ oplines only, which all
509-
* populate through the set-checked write handler. */
510503
if (opline->op2_type == IS_CONST) {
511504
if (opline->op1_type == IS_UNUSED &&
512-
assign_property_slot[opline->op2.constant] >= 0) {
513-
opline->extended_value = assign_property_slot[opline->op2.constant];
505+
assign_obj_slots[opline->op2.constant] >= 0) {
506+
opline->extended_value = assign_obj_slots[opline->op2.constant];
514507
} else {
515508
opline->extended_value = cache_size;
516509
cache_size += 3 * sizeof(void *);
517510
if (opline->op1_type == IS_UNUSED) {
518-
assign_property_slot[opline->op2.constant] = opline->extended_value;
511+
assign_obj_slots[opline->op2.constant] = opline->extended_value;
519512
}
520513
}
521514
}

‎Zend/zend_execute.c‎

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1070,20 +1070,16 @@ ZEND_API bool zend_never_inline zend_verify_property_type(const zend_property_in
10701070
return i_zend_verify_property_type(info, property, strict);
10711071
}
10721072

1073-
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)
1073+
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)
10741074
{
10751075
zval tmp;
1076-
const uint32_t guard_mask =
1077-
ZEND_ACC_READONLY | (check_set_access ? ZEND_ACC_PPP_SET_MASK : 0);
10781076

1079-
if (UNEXPECTED(info->flags & guard_mask)) {
1077+
if (UNEXPECTED(info->flags & (ZEND_ACC_READONLY|ZEND_ACC_PPP_SET_MASK))) {
10801078
if ((info->flags & ZEND_ACC_READONLY) && !(Z_PROP_FLAG_P(property_val) & IS_PROP_REINITABLE)) {
10811079
zend_readonly_property_modification_error(info);
10821080
return &EG(uninitialized_zval);
10831081
}
1084-
if (check_set_access
1085-
&& (info->flags & ZEND_ACC_PPP_SET_MASK)
1086-
&& !zend_asymmetric_property_has_set_access(info)) {
1082+
if (check_writable && (info->flags & ZEND_ACC_PPP_SET_MASK) && !zend_asymmetric_property_has_set_access(info)) {
10871083
zend_asymmetric_visibility_property_modification_error(info, "modify");
10881084
return &EG(uninitialized_zval);
10891085
}
@@ -1102,24 +1098,6 @@ static zend_always_inline zval* zend_assign_to_typed_prop_ex(const zend_property
11021098
return zend_assign_to_variable_ex(property_val, &tmp, IS_TMP_VAR, EX_USES_STRICT_TYPES(), garbage_ptr);
11031099
}
11041100

1105-
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)
1106-
{
1107-
return zend_assign_to_typed_prop_ex(info, property_val, value, garbage_ptr, true EXECUTE_DATA_CC);
1108-
}
1109-
1110-
/* For write sites whose runtime cache slot produced `info`: the slot is only
1111-
* populated when set access was verified at resolution time
1112-
* (zend_get_property_offset()), so the per-write asymmetric visibility check
1113-
* is redundant. */
1114-
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)
1115-
{
1116-
ZEND_ASSERT(!(info->flags & ZEND_ACC_PPP_SET_MASK)
1117-
|| (info->flags & ZEND_ACC_PUBLIC_SET)
1118-
|| zend_asymmetric_property_has_set_access(info));
1119-
1120-
return zend_assign_to_typed_prop_ex(info, property_val, value, garbage_ptr, false EXECUTE_DATA_CC);
1121-
}
1122-
11231101
static zend_always_inline bool zend_value_instanceof_static(const zval *zv) {
11241102
if (Z_TYPE_P(zv) != IS_OBJECT) {
11251103
return 0;

‎Zend/zend_object_handlers.c‎

Lines changed: 17 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -362,21 +362,18 @@ static zend_always_inline const zend_class_entry *get_fake_or_executed_scope(voi
362362
}
363363
}
364364

365-
/* Resolve a property for reading (write_access == false) or for
366-
* writing/unsetting (write_access == true). Write-kind resolution verifies
367-
* asymmetric set visibility at population time: when the running scope lacks
368-
* set access, the result is still returned (the caller's state-dependent
369-
* slow path decides between an error and the __set fallback), but the
370-
* runtime cache is NOT populated. A populated write-site cache slot
371-
* therefore guarantees set access, which lets the VM's cached direct-assign
372-
* fast path skip the per-write asymmetric visibility check. */
373-
static zend_never_inline uintptr_t zend_get_property_offset_slow(zend_class_entry *ce, zend_string *member, int silent, void **cache_slot, const zend_property_info **info_ptr, bool write_access) /* {{{ */
365+
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) /* {{{ */
374366
{
375367
zval *zv;
376368
zend_property_info *property_info;
377369
uint32_t flags;
378370
uintptr_t offset;
379371

372+
if (cache_slot && EXPECTED(ce == CACHED_PTR_EX(cache_slot))) {
373+
*info_ptr = CACHED_PTR_EX(cache_slot + 2);
374+
return (uintptr_t)CACHED_PTR_EX(cache_slot + 1);
375+
}
376+
380377
if (UNEXPECTED(zend_hash_num_elements(&ce->properties_info) == 0)
381378
|| UNEXPECTED((zv = zend_hash_find(&ce->properties_info, member)) == NULL)) {
382379
if (UNEXPECTED(ZSTR_VAL(member)[0] == '\0') && ZSTR_LEN(member) != 0) {
@@ -443,26 +440,6 @@ static zend_never_inline uintptr_t zend_get_property_offset_slow(zend_class_entr
443440
return ZEND_DYNAMIC_PROPERTY_OFFSET;
444441
}
445442

446-
if (cache_slot
447-
&& write_access
448-
&& UNEXPECTED(flags & ZEND_ACC_PPP_SET_MASK)
449-
&& !(flags & ZEND_ACC_PUBLIC_SET)) {
450-
const zend_class_entry *scope = get_fake_or_executed_scope();
451-
452-
if (property_info->ce != scope
453-
&& (!(flags & ZEND_ACC_PROTECTED_SET)
454-
|| !is_protected_compatible_scope(property_info->prototype->ce, scope))) {
455-
/* Set access denied for this scope: resolve as usual, but keep
456-
* the cache slot empty so every such write stays on the slow
457-
* path with its state-dependent handling. Invalidate the key
458-
* too: downstream code (e.g. the hooked simple-write marking)
459-
* assumes resolution populated the slot and would otherwise
460-
* flag a stale polymorphic entry. */
461-
CACHE_PTR_EX(cache_slot, NULL);
462-
cache_slot = NULL;
463-
}
464-
}
465-
466443
if (property_info->hooks) {
467444
*info_ptr = property_info;
468445
if (cache_slot) {
@@ -486,24 +463,12 @@ static zend_never_inline uintptr_t zend_get_property_offset_slow(zend_class_entr
486463
}
487464
/* }}} */
488465

489-
/* Keep the inlined body limited to the cache-hit fast path; resolution
490-
* (visibility, modules, surfaces, hooks) stays out of line so callers'
491-
* hot paths remain compact. */
492-
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)
493-
{
494-
if (cache_slot && EXPECTED(ce == CACHED_PTR_EX(cache_slot))) {
495-
*info_ptr = CACHED_PTR_EX(cache_slot + 2);
496-
return (uintptr_t)CACHED_PTR_EX(cache_slot + 1);
497-
}
498-
return zend_get_property_offset_slow(ce, member, silent, cache_slot, info_ptr, write_access);
499-
}
500-
501466
static ZEND_COLD void zend_wrong_offset(zend_class_entry *ce, zend_string *member) /* {{{ */
502467
{
503468
const zend_property_info *dummy;
504469

505470
/* Trigger the correct error */
506-
zend_get_property_offset(ce, member, 0, NULL, &dummy, false);
471+
zend_get_property_offset(ce, member, 0, NULL, &dummy);
507472
}
508473
/* }}} */
509474

@@ -784,7 +749,7 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int
784749
#endif
785750

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

789754
if (EXPECTED(IS_VALID_PROPERTY_OFFSET(property_offset))) {
790755
try_again:
@@ -1107,7 +1072,7 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva
11071072
uint32_t *guard = NULL;
11081073
ZEND_ASSERT(!Z_ISREF_P(value));
11091074

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

11121077
if (EXPECTED(IS_VALID_PROPERTY_OFFSET(property_offset))) {
11131078
try_again:
@@ -1132,6 +1097,11 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva
11321097
if ((prop_info->flags & ZEND_ACC_PPP_SET_MASK) && !zend_asymmetric_property_has_set_access(prop_info)) {
11331098
zend_asymmetric_visibility_property_modification_error(prop_info, "modify");
11341099
variable_ptr = &EG(error_zval);
1100+
if (cache_slot) {
1101+
/* Reset cache slot to dodge fast path in next execution. */
1102+
CACHE_POLYMORPHIC_PTR_EX(cache_slot, NULL, NULL);
1103+
CACHE_PTR_EX(cache_slot + 2, NULL);
1104+
}
11351105
goto exit;
11361106
}
11371107
}
@@ -1469,7 +1439,7 @@ ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *zobj, zend_string *nam
14691439
fprintf(stderr, "Ptr object #%d property: %s\n", zobj->handle, ZSTR_VAL(name));
14701440
#endif
14711441

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

14741444
if (EXPECTED(IS_VALID_PROPERTY_OFFSET(property_offset))) {
14751445
try_again:
@@ -1597,7 +1567,7 @@ ZEND_API void zend_std_unset_property(zend_object *zobj, zend_string *name, void
15971567
const zend_property_info *prop_info = NULL;
15981568
uint32_t *guard = NULL;
15991569

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

16021572
if (EXPECTED(IS_VALID_PROPERTY_OFFSET(property_offset))) {
16031573
zval *slot = OBJ_PROP(zobj, property_offset);
@@ -2407,7 +2377,7 @@ ZEND_API int zend_std_has_property(zend_object *zobj, zend_string *name, int has
24072377
uintptr_t property_offset;
24082378
const zend_property_info *prop_info = NULL;
24092379

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

24122382
if (EXPECTED(IS_VALID_PROPERTY_OFFSET(property_offset))) {
24132383
try_again:

‎Zend/zend_vm_def.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2524,9 +2524,7 @@ ZEND_VM_C_LABEL(assign_obj_simple):
25242524
property_val = OBJ_PROP(zobj, prop_offset);
25252525
if (Z_TYPE_P(property_val) != IS_UNDEF) {
25262526
if (prop_info != NULL) {
2527-
/* Cache-hit path: set access was verified when the
2528-
* slot was populated. */
2529-
value = zend_assign_to_typed_prop_granted(prop_info, property_val, value, &garbage EXECUTE_DATA_CC);
2527+
value = zend_assign_to_typed_prop(prop_info, property_val, value, &garbage, /* check_writable */ false EXECUTE_DATA_CC);
25302528
ZEND_VM_C_GOTO(free_and_exit_assign_obj);
25312529
} else {
25322530
ZEND_VM_C_LABEL(fast_assign_obj):
@@ -2661,7 +2659,7 @@ ZEND_VM_HANDLER(25, ZEND_ASSIGN_STATIC_PROP, ANY, ANY, CACHE_SLOT, SPEC(OP_DATA=
26612659
value = GET_OP_DATA_ZVAL_PTR(BP_VAR_R);
26622660

26632661
if (ZEND_TYPE_IS_SET(prop_info->type)) {
2664-
value = zend_assign_to_typed_prop(prop_info, prop, value, &garbage EXECUTE_DATA_CC);
2662+
value = zend_assign_to_typed_prop(prop_info, prop, value, &garbage, /* check_writable */ true EXECUTE_DATA_CC);
26652663
FREE_OP_DATA();
26662664
} else {
26672665
value = zend_assign_to_variable_ex(prop, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES(), &garbage);

0 commit comments

Comments
 (0)