Skip to content

Mark writes that are really reads if working on an object #5273

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

Closed
wants to merge 1 commit into from
Closed

Mark writes that are really reads if working on an object #5273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion Zend/tests/bug70083.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ try {
var_dump($foo);

?>
--EXPECT--
--EXPECTF--
Notice: Indirect modification of overloaded property foo::$i has no effect in %s on line %d
Cannot assign by reference to overloaded object
object(foo)#1 (1) {
["var":"foo":private]=>
Expand Down
1 change: 1 addition & 0 deletions Zend/tests/bug70288.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ $a = new A;
test($a->dummy);
?>
--EXPECTF--
Notice: Indirect modification of overloaded property A::$dummy has no effect in %s on line %d
object(stdClass)#%d (0) {
}
20 changes: 20 additions & 0 deletions Zend/zend_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2638,6 +2638,21 @@ static inline void zend_emit_assign_znode(zend_ast *var_ast, znode *value_node)
}
/* }}} */

static zend_bool is_prop_write_fetch(zend_op *opline, uint32_t type) {
if (type != BP_VAR_W && type != BP_VAR_RW && type != BP_VAR_UNSET) {
return 0;
}
if (!opline) {
return 0;
}
return opline->opcode == ZEND_FETCH_OBJ_W
|| opline->opcode == ZEND_FETCH_OBJ_RW
|| opline->opcode == ZEND_FETCH_OBJ_UNSET
|| opline->opcode == ZEND_FETCH_STATIC_PROP_W
|| opline->opcode == ZEND_FETCH_STATIC_PROP_RW
|| opline->opcode == ZEND_FETCH_STATIC_PROP_UNSET;
}

static zend_op *zend_delayed_compile_dim(znode *result, zend_ast *ast, uint32_t type) /* {{{ */
{
if (ast->attr == ZEND_DIM_ALTERNATIVE_SYNTAX) {
Expand All @@ -2653,6 +2668,8 @@ static zend_op *zend_delayed_compile_dim(znode *result, zend_ast *ast, uint32_t
opline = zend_delayed_compile_var(&var_node, var_ast, type, 0);
if (opline && type == BP_VAR_W && (opline->opcode == ZEND_FETCH_STATIC_PROP_W || opline->opcode == ZEND_FETCH_OBJ_W)) {
opline->extended_value |= ZEND_FETCH_DIM_WRITE;
} else if (is_prop_write_fetch(opline, type)) {
opline->extended_value |= ZEND_FETCH_OBJ_IS_R;
}

zend_separate_if_call_and_write(&var_node, var_ast, type);
Expand Down Expand Up @@ -2704,6 +2721,9 @@ static zend_op *zend_delayed_compile_prop(znode *result, zend_ast *ast, uint32_t
CG(active_op_array)->fn_flags |= ZEND_ACC_USES_THIS;
} else {
opline = zend_delayed_compile_var(&obj_node, obj_ast, type, 0);
if (is_prop_write_fetch(opline, type)) {
opline->extended_value |= ZEND_FETCH_OBJ_IS_R;
}
zend_separate_if_call_and_write(&obj_node, obj_ast, type);
}
zend_compile_expr(&prop_node, prop_ast);
Expand Down
16 changes: 10 additions & 6 deletions Zend/zend_compile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -877,12 +877,15 @@ ZEND_API zend_string *zend_type_to_string(zend_type type);
#define ZEND_ARRAY_SYNTAX_SHORT 3 /* [] */

/* var status for backpatching */
#define BP_VAR_R 0
#define BP_VAR_W 1
#define BP_VAR_RW 2
#define BP_VAR_IS 3
#define BP_VAR_FUNC_ARG 4
#define BP_VAR_UNSET 5
#define BP_VAR_R (1 << 0)
#define BP_VAR_W (1 << 1)
#define BP_VAR_RW (1 << 2)
#define BP_VAR_IS (1 << 3)
#define BP_VAR_FUNC_ARG (1 << 4)
#define BP_VAR_UNSET (1 << 5)
/* Combined with BP_VAR_W, BP_VAR_RW, BP_VAR_UNSET,
* indicates that it should be interpreted as a read if the value is an object. */
#define BP_VAR_OBJ_IS_R (1 << 6)

#define ZEND_INTERNAL_FUNCTION 1
#define ZEND_USER_FUNCTION 2
Expand All @@ -909,6 +912,7 @@ ZEND_API zend_string *zend_type_to_string(zend_type type);
/* Only one of these can ever be in use */
#define ZEND_FETCH_REF 1
#define ZEND_FETCH_DIM_WRITE 2
#define ZEND_FETCH_OBJ_IS_R 3
#define ZEND_FETCH_OBJ_FLAGS 3

#define ZEND_ISEMPTY (1<<0)
Expand Down
3 changes: 3 additions & 0 deletions Zend/zend_execute.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2717,6 +2717,9 @@ static zend_always_inline void zend_fetch_property_address(zval *result, zval *c
}
ptr = zobj->handlers->get_property_ptr_ptr(zobj, name, type, cache_slot);
if (NULL == ptr) {
if (flags == ZEND_FETCH_OBJ_IS_R || flags == ZEND_FETCH_DIM_WRITE) {
type |= BP_VAR_OBJ_IS_R;
}
ptr = zobj->handlers->read_property(zobj, name, type, cache_slot, result);
if (ptr == result) {
if (UNEXPECTED(Z_ISREF_P(ptr) && Z_REFCOUNT_P(ptr) == 1)) {
Expand Down
5 changes: 2 additions & 3 deletions Zend/zend_object_handlers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -750,9 +750,8 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int

if (Z_TYPE_P(rv) != IS_UNDEF) {
retval = rv;
if (!Z_ISREF_P(rv) &&
(type == BP_VAR_W || type == BP_VAR_RW || type == BP_VAR_UNSET)) {
if (UNEXPECTED(Z_TYPE_P(rv) != IS_OBJECT)) {
if (!Z_ISREF_P(rv) && (type & (BP_VAR_W|BP_VAR_RW|BP_VAR_UNSET))) {
if (Z_TYPE_P(rv) != IS_OBJECT || !(type & BP_VAR_OBJ_IS_R)) {
zend_error(E_NOTICE, "Indirect modification of overloaded property %s::$%s has no effect", ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
}
}
Expand Down
6 changes: 4 additions & 2 deletions Zend/zend_vm_def.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2279,7 +2279,7 @@ ZEND_VM_COLD_CONST_HANDLER(94, ZEND_FETCH_OBJ_FUNC_ARG, CONST|TMP|VAR|UNUSED|THI
}
}

ZEND_VM_HANDLER(97, ZEND_FETCH_OBJ_UNSET, VAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, CACHE_SLOT)
ZEND_VM_HANDLER(97, ZEND_FETCH_OBJ_UNSET, VAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, FETCH_REF|CACHE_SLOT)
{
USE_OPLINE
zval *container, *property, *result;
Expand All @@ -2288,7 +2288,9 @@ ZEND_VM_HANDLER(97, ZEND_FETCH_OBJ_UNSET, VAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, C
container = GET_OP1_OBJ_ZVAL_PTR_PTR_UNDEF(BP_VAR_UNSET);
property = GET_OP2_ZVAL_PTR(BP_VAR_R);
result = EX_VAR(opline->result.var);
zend_fetch_property_address(result, container, OP1_TYPE, property, OP2_TYPE, ((OP2_TYPE == IS_CONST) ? CACHE_ADDR(opline->extended_value) : NULL), BP_VAR_UNSET, 0, 1 OPLINE_CC EXECUTE_DATA_CC);
zend_fetch_property_address(result, container, OP1_TYPE, property, OP2_TYPE,
((OP2_TYPE == IS_CONST) ? CACHE_ADDR(opline->extended_value & ~ZEND_FETCH_OBJ_FLAGS) : NULL),
BP_VAR_UNSET, opline->extended_value & ZEND_FETCH_OBJ_FLAGS, 1 OPLINE_CC EXECUTE_DATA_CC);
FREE_OP2();
if (OP1_TYPE == IS_VAR) {
FREE_VAR_PTR_AND_EXTRACT_RESULT_IF_NECESSARY(opline->op1.var);
Expand Down
36 changes: 27 additions & 9 deletions Zend/zend_vm_execute.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21453,7 +21453,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_UNSET_SPEC_VAR_CONST
container = _get_zval_ptr_ptr_var(opline->op1.var EXECUTE_DATA_CC);
property = RT_CONSTANT(opline, opline->op2);
result = EX_VAR(opline->result.var);
zend_fetch_property_address(result, container, IS_VAR, property, IS_CONST, ((IS_CONST == IS_CONST) ? CACHE_ADDR(opline->extended_value) : NULL), BP_VAR_UNSET, 0, 1 OPLINE_CC EXECUTE_DATA_CC);
zend_fetch_property_address(result, container, IS_VAR, property, IS_CONST,
((IS_CONST == IS_CONST) ? CACHE_ADDR(opline->extended_value & ~ZEND_FETCH_OBJ_FLAGS) : NULL),
BP_VAR_UNSET, opline->extended_value & ZEND_FETCH_OBJ_FLAGS, 1 OPLINE_CC EXECUTE_DATA_CC);

if (IS_VAR == IS_VAR) {
FREE_VAR_PTR_AND_EXTRACT_RESULT_IF_NECESSARY(opline->op1.var);
Expand Down Expand Up @@ -23722,7 +23724,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_UNSET_SPEC_VAR_TMPVA
container = _get_zval_ptr_ptr_var(opline->op1.var EXECUTE_DATA_CC);
property = _get_zval_ptr_var(opline->op2.var EXECUTE_DATA_CC);
result = EX_VAR(opline->result.var);
zend_fetch_property_address(result, container, IS_VAR, property, (IS_TMP_VAR|IS_VAR), (((IS_TMP_VAR|IS_VAR) == IS_CONST) ? CACHE_ADDR(opline->extended_value) : NULL), BP_VAR_UNSET, 0, 1 OPLINE_CC EXECUTE_DATA_CC);
zend_fetch_property_address(result, container, IS_VAR, property, (IS_TMP_VAR|IS_VAR),
(((IS_TMP_VAR|IS_VAR) == IS_CONST) ? CACHE_ADDR(opline->extended_value & ~ZEND_FETCH_OBJ_FLAGS) : NULL),
BP_VAR_UNSET, opline->extended_value & ZEND_FETCH_OBJ_FLAGS, 1 OPLINE_CC EXECUTE_DATA_CC);
zval_ptr_dtor_nogc(EX_VAR(opline->op2.var));
if (IS_VAR == IS_VAR) {
FREE_VAR_PTR_AND_EXTRACT_RESULT_IF_NECESSARY(opline->op1.var);
Expand Down Expand Up @@ -27174,7 +27178,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_UNSET_SPEC_VAR_CV_HA
container = _get_zval_ptr_ptr_var(opline->op1.var EXECUTE_DATA_CC);
property = _get_zval_ptr_cv_BP_VAR_R(opline->op2.var EXECUTE_DATA_CC);
result = EX_VAR(opline->result.var);
zend_fetch_property_address(result, container, IS_VAR, property, IS_CV, ((IS_CV == IS_CONST) ? CACHE_ADDR(opline->extended_value) : NULL), BP_VAR_UNSET, 0, 1 OPLINE_CC EXECUTE_DATA_CC);
zend_fetch_property_address(result, container, IS_VAR, property, IS_CV,
((IS_CV == IS_CONST) ? CACHE_ADDR(opline->extended_value & ~ZEND_FETCH_OBJ_FLAGS) : NULL),
BP_VAR_UNSET, opline->extended_value & ZEND_FETCH_OBJ_FLAGS, 1 OPLINE_CC EXECUTE_DATA_CC);

if (IS_VAR == IS_VAR) {
FREE_VAR_PTR_AND_EXTRACT_RESULT_IF_NECESSARY(opline->op1.var);
Expand Down Expand Up @@ -29693,7 +29699,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_UNSET_SPEC_UNUSED_CO
container = &EX(This);
property = RT_CONSTANT(opline, opline->op2);
result = EX_VAR(opline->result.var);
zend_fetch_property_address(result, container, IS_UNUSED, property, IS_CONST, ((IS_CONST == IS_CONST) ? CACHE_ADDR(opline->extended_value) : NULL), BP_VAR_UNSET, 0, 1 OPLINE_CC EXECUTE_DATA_CC);
zend_fetch_property_address(result, container, IS_UNUSED, property, IS_CONST,
((IS_CONST == IS_CONST) ? CACHE_ADDR(opline->extended_value & ~ZEND_FETCH_OBJ_FLAGS) : NULL),
BP_VAR_UNSET, opline->extended_value & ZEND_FETCH_OBJ_FLAGS, 1 OPLINE_CC EXECUTE_DATA_CC);

if (IS_UNUSED == IS_VAR) {
FREE_VAR_PTR_AND_EXTRACT_RESULT_IF_NECESSARY(opline->op1.var);
Expand Down Expand Up @@ -31560,7 +31568,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_UNSET_SPEC_UNUSED_TM
container = &EX(This);
property = _get_zval_ptr_var(opline->op2.var EXECUTE_DATA_CC);
result = EX_VAR(opline->result.var);
zend_fetch_property_address(result, container, IS_UNUSED, property, (IS_TMP_VAR|IS_VAR), (((IS_TMP_VAR|IS_VAR) == IS_CONST) ? CACHE_ADDR(opline->extended_value) : NULL), BP_VAR_UNSET, 0, 1 OPLINE_CC EXECUTE_DATA_CC);
zend_fetch_property_address(result, container, IS_UNUSED, property, (IS_TMP_VAR|IS_VAR),
(((IS_TMP_VAR|IS_VAR) == IS_CONST) ? CACHE_ADDR(opline->extended_value & ~ZEND_FETCH_OBJ_FLAGS) : NULL),
BP_VAR_UNSET, opline->extended_value & ZEND_FETCH_OBJ_FLAGS, 1 OPLINE_CC EXECUTE_DATA_CC);
zval_ptr_dtor_nogc(EX_VAR(opline->op2.var));
if (IS_UNUSED == IS_VAR) {
FREE_VAR_PTR_AND_EXTRACT_RESULT_IF_NECESSARY(opline->op1.var);
Expand Down Expand Up @@ -33955,7 +33965,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_UNSET_SPEC_UNUSED_CV
container = &EX(This);
property = _get_zval_ptr_cv_BP_VAR_R(opline->op2.var EXECUTE_DATA_CC);
result = EX_VAR(opline->result.var);
zend_fetch_property_address(result, container, IS_UNUSED, property, IS_CV, ((IS_CV == IS_CONST) ? CACHE_ADDR(opline->extended_value) : NULL), BP_VAR_UNSET, 0, 1 OPLINE_CC EXECUTE_DATA_CC);
zend_fetch_property_address(result, container, IS_UNUSED, property, IS_CV,
((IS_CV == IS_CONST) ? CACHE_ADDR(opline->extended_value & ~ZEND_FETCH_OBJ_FLAGS) : NULL),
BP_VAR_UNSET, opline->extended_value & ZEND_FETCH_OBJ_FLAGS, 1 OPLINE_CC EXECUTE_DATA_CC);

if (IS_UNUSED == IS_VAR) {
FREE_VAR_PTR_AND_EXTRACT_RESULT_IF_NECESSARY(opline->op1.var);
Expand Down Expand Up @@ -38267,7 +38279,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_UNSET_SPEC_CV_CONST_
container = EX_VAR(opline->op1.var);
property = RT_CONSTANT(opline, opline->op2);
result = EX_VAR(opline->result.var);
zend_fetch_property_address(result, container, IS_CV, property, IS_CONST, ((IS_CONST == IS_CONST) ? CACHE_ADDR(opline->extended_value) : NULL), BP_VAR_UNSET, 0, 1 OPLINE_CC EXECUTE_DATA_CC);
zend_fetch_property_address(result, container, IS_CV, property, IS_CONST,
((IS_CONST == IS_CONST) ? CACHE_ADDR(opline->extended_value & ~ZEND_FETCH_OBJ_FLAGS) : NULL),
BP_VAR_UNSET, opline->extended_value & ZEND_FETCH_OBJ_FLAGS, 1 OPLINE_CC EXECUTE_DATA_CC);

if (IS_CV == IS_VAR) {
FREE_VAR_PTR_AND_EXTRACT_RESULT_IF_NECESSARY(opline->op1.var);
Expand Down Expand Up @@ -41745,7 +41759,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_UNSET_SPEC_CV_TMPVAR
container = EX_VAR(opline->op1.var);
property = _get_zval_ptr_var(opline->op2.var EXECUTE_DATA_CC);
result = EX_VAR(opline->result.var);
zend_fetch_property_address(result, container, IS_CV, property, (IS_TMP_VAR|IS_VAR), (((IS_TMP_VAR|IS_VAR) == IS_CONST) ? CACHE_ADDR(opline->extended_value) : NULL), BP_VAR_UNSET, 0, 1 OPLINE_CC EXECUTE_DATA_CC);
zend_fetch_property_address(result, container, IS_CV, property, (IS_TMP_VAR|IS_VAR),
(((IS_TMP_VAR|IS_VAR) == IS_CONST) ? CACHE_ADDR(opline->extended_value & ~ZEND_FETCH_OBJ_FLAGS) : NULL),
BP_VAR_UNSET, opline->extended_value & ZEND_FETCH_OBJ_FLAGS, 1 OPLINE_CC EXECUTE_DATA_CC);
zval_ptr_dtor_nogc(EX_VAR(opline->op2.var));
if (IS_CV == IS_VAR) {
FREE_VAR_PTR_AND_EXTRACT_RESULT_IF_NECESSARY(opline->op1.var);
Expand Down Expand Up @@ -46553,7 +46569,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_UNSET_SPEC_CV_CV_HAN
container = EX_VAR(opline->op1.var);
property = _get_zval_ptr_cv_BP_VAR_R(opline->op2.var EXECUTE_DATA_CC);
result = EX_VAR(opline->result.var);
zend_fetch_property_address(result, container, IS_CV, property, IS_CV, ((IS_CV == IS_CONST) ? CACHE_ADDR(opline->extended_value) : NULL), BP_VAR_UNSET, 0, 1 OPLINE_CC EXECUTE_DATA_CC);
zend_fetch_property_address(result, container, IS_CV, property, IS_CV,
((IS_CV == IS_CONST) ? CACHE_ADDR(opline->extended_value & ~ZEND_FETCH_OBJ_FLAGS) : NULL),
BP_VAR_UNSET, opline->extended_value & ZEND_FETCH_OBJ_FLAGS, 1 OPLINE_CC EXECUTE_DATA_CC);

if (IS_CV == IS_VAR) {
FREE_VAR_PTR_AND_EXTRACT_RESULT_IF_NECESSARY(opline->op1.var);
Expand Down
2 changes: 1 addition & 1 deletion Zend/zend_vm_opcodes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ static uint32_t zend_vm_opcodes_flags[195] = {
0x00240753,
0x00010107,
0x00000701,
0x00040751,
0x00240751,
0x0000070b,
0x00040391,
0x00001001,
Expand Down