Skip to content

Use correct fetch mode for object modification #5250

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 4 commits into from
Closed

Use correct fetch mode for object modification #5250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
77b124d
Use correct fetch mode for object modification
nikic Mar 10, 2020
e2b06e7
Update SimpleXML tests
nikic Mar 10, 2020
eae98ec
Pass scratch register to SAVE_VALID_OPLINE
nikic Mar 18, 2020
adc1657
Use different scratch register in read_obj
nikic Mar 18, 2020
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
15 changes: 14 additions & 1 deletion Zend/tests/033.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ try {
echo $e->getMessage(), "\n";
}

/*
$arr[][] = 2;

try {
$arr[][]->bar = 2;
} catch (Error $e) {
echo $e->getMessage(), "\n";
}
*/

?>
--EXPECTF--
Expand Down Expand Up @@ -62,5 +64,16 @@ Warning: Trying to access array offset on value of type null in %s on line %d
Warning: Trying to access array offset on value of type null in %s on line %d

Warning: Trying to get property 'foo' of non-object in %s on line %d

Warning: Undefined variable: arr in %s on line %d
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's going to be hard to explain why $a[2]->x = 2; warns about undefined variable but $a[2][2] = 2; does not.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be hard to explain why arrays and objects have different semantics in the first place, but given that they do, this seems more consistent:

$a[0] = 42 where $a is undefined is considered legal and does not throw any notices or errors. $a->x = 42 is considered illegal and throws a warning in PHP 7 and exception in PHP 8. In this case, the undefined variable notice indicates the "root cause" that needs to be fixed, while the "Attempt to assign property 'b' of non-object" is a followup error caused by it.


Warning: Trying to access array offset on value of type null in %s on line %d

Warning: Trying to access array offset on value of type null in %s on line %d

Warning: Trying to access array offset on value of type null in %s on line %d

Warning: Trying to access array offset on value of type null in %s on line %d

Warning: Trying to access array offset on value of type null in %s on line %d
Attempt to assign property 'foo' of non-object
Attempt to assign property 'bar' of non-object
2 changes: 1 addition & 1 deletion Zend/tests/bug41813.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ $foo[0]->bar = "xyz";
echo "Done\n";
?>
--EXPECTF--
Fatal error: Uncaught Error: Cannot use string offset as an object in %s:%d
Fatal error: Uncaught Error: Attempt to assign property 'bar' of non-object in %s:%d
Stack trace:
#0 {main}
thrown in %s on line %d
4 changes: 3 additions & 1 deletion Zend/tests/bug41919.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ $foo[3]->bar[1] = "bang";
echo "ok\n";
?>
--EXPECTF--
Fatal error: Uncaught Error: Cannot use string offset as an object in %sbug41919.php:%d
Warning: Uninitialized string offset: 3 in %s on line %d

Fatal error: Uncaught Error: Attempt to modify property 'bar' of non-object in %s:%d
Stack trace:
#0 {main}
thrown in %sbug41919.php on line %d
2 changes: 1 addition & 1 deletion Zend/tests/bug47704.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ $s = "abd";
$s[0]->a += 1;
?>
--EXPECTF--
Fatal error: Uncaught Error: Cannot use string offset as an object in %sbug47704.php:%d
Fatal error: Uncaught Error: Attempt to assign property 'a' of non-object in %s:%d
Stack trace:
#0 {main}
thrown in %sbug47704.php on line %d
12 changes: 9 additions & 3 deletions Zend/tests/bug52041.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,25 @@ Warning: Undefined variable: x in %s on line %d
Attempt to assign property 'a' of non-object

Warning: Undefined variable: x in %s on line %d
Attempt to modify property 'a' of non-object

Warning: Trying to get property 'a' of non-object in %s on line %d
Attempt to assign property 'b' of non-object

Warning: Undefined variable: x in %s on line %d
Attempt to increment/decrement property 'a' of non-object

Warning: Undefined variable: x in %s on line %d
Attempt to modify property 'a' of non-object

Warning: Trying to get property 'a' of non-object in %s on line %d
Attempt to increment/decrement property 'b' of non-object

Warning: Undefined variable: x in %s on line %d
Attempt to assign property 'a' of non-object

Warning: Undefined variable: x in %s on line %d
Attempt to modify property 'a' of non-object

Warning: Trying to get property 'a' of non-object in %s on line %d
Attempt to assign property 'b' of non-object

Warning: Undefined variable: x in %s on line %d

Expand Down
20 changes: 17 additions & 3 deletions Zend/tests/bug75921.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,29 @@ Attempt to modify property 'a' of non-object

Warning: Undefined variable: null in %s on line %d
NULL
Attempt to modify property 'a' of non-object

Warning: Undefined variable: null in %s on line %d

Warning: Trying to get property 'a' of non-object in %s on line %d
Attempt to assign property 'b' of non-object

Warning: Undefined variable: null in %s on line %d
NULL
Attempt to modify property 'a' of non-object

Warning: Undefined variable: null in %s on line %d

Warning: Trying to get property 'a' of non-object in %s on line %d

Warning: Trying to access array offset on value of type null in %s on line %d
Attempt to assign property 'b' of non-object

Warning: Undefined variable: null in %s on line %d
NULL
Attempt to modify property 'a' of non-object

Warning: Undefined variable: null in %s on line %d

Warning: Trying to get property 'a' of non-object in %s on line %d
Attempt to modify property 'b' of non-object

Warning: Undefined variable: null in %s on line %d
NULL
4 changes: 3 additions & 1 deletion Zend/tests/bug78531.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,6 @@ Warning: Undefined variable: u3 in %s on line %d
Attempt to increment/decrement property 'a' of non-object

Warning: Undefined variable: u4 in %s on line %d
Attempt to modify property 'a' of non-object

Warning: Trying to get property 'a' of non-object in %s on line %d
Attempt to assign property 'a' of non-object
8 changes: 7 additions & 1 deletion Zend/zend_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2703,7 +2703,13 @@ 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);
/* In $a->b->c = $d, fetch $a->b for read and only ->c for write.
* We will never modify $a->b itself, only the object it holds. */
if (type == BP_VAR_W || type == BP_VAR_RW || type == BP_VAR_FUNC_ARG) {
opline = zend_delayed_compile_var(&obj_node, obj_ast, BP_VAR_R, 0);
} else {
opline = zend_delayed_compile_var(&obj_node, obj_ast, type, 0);
}
zend_separate_if_call_and_write(&obj_node, obj_ast, type);
}
zend_compile_expr(&prop_node, prop_ast);
Expand Down
44 changes: 22 additions & 22 deletions Zend/zend_vm_def.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -989,7 +989,7 @@ ZEND_VM_COLD_HELPER(zend_undefined_function_helper, ANY, ANY)
HANDLE_EXCEPTION();
}

ZEND_VM_HANDLER(28, ZEND_ASSIGN_OBJ_OP, VAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, OP)
ZEND_VM_HANDLER(28, ZEND_ASSIGN_OBJ_OP, TMPVAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, OP)
{
USE_OPLINE
zval *object;
Expand All @@ -1002,7 +1002,7 @@ ZEND_VM_HANDLER(28, ZEND_ASSIGN_OBJ_OP, VAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, OP)
zend_string *name, *tmp_name;

SAVE_OPLINE();
object = GET_OP1_OBJ_ZVAL_PTR_PTR_UNDEF(BP_VAR_RW);
object = GET_OP1_OBJ_ZVAL_PTR_UNDEF(BP_VAR_R);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may miss support for IS_INDIRECT.

property = GET_OP2_ZVAL_PTR(BP_VAR_R);

do {
Expand Down Expand Up @@ -1080,7 +1080,7 @@ ZEND_VM_C_LABEL(assign_op_object):

FREE_OP_DATA();
FREE_OP2();
FREE_OP1_VAR_PTR();
FREE_OP1();
/* assign_obj has two opcodes! */
ZEND_VM_NEXT_OPCODE_EX(1, 2);
}
Expand Down Expand Up @@ -1248,7 +1248,7 @@ ZEND_VM_HANDLER(26, ZEND_ASSIGN_OP, VAR|CV, CONST|TMPVAR|CV, OP)
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
}

ZEND_VM_HANDLER(132, ZEND_PRE_INC_OBJ, VAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, CACHE_SLOT)
ZEND_VM_HANDLER(132, ZEND_PRE_INC_OBJ, TMPVAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, CACHE_SLOT)
{
USE_OPLINE
zval *object;
Expand All @@ -1260,7 +1260,7 @@ ZEND_VM_HANDLER(132, ZEND_PRE_INC_OBJ, VAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, CACH
zend_string *name, *tmp_name;

SAVE_OPLINE();
object = GET_OP1_OBJ_ZVAL_PTR_PTR_UNDEF(BP_VAR_RW);
object = GET_OP1_OBJ_ZVAL_PTR_UNDEF(BP_VAR_R);
property = GET_OP2_ZVAL_PTR(BP_VAR_R);

do {
Expand Down Expand Up @@ -1312,7 +1312,7 @@ ZEND_VM_C_LABEL(pre_incdec_object):
} while (0);

FREE_OP2();
FREE_OP1_VAR_PTR();
FREE_OP1();
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
}

Expand All @@ -1333,7 +1333,7 @@ ZEND_VM_HANDLER(134, ZEND_POST_INC_OBJ, VAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, CAC
zend_string *name, *tmp_name;

SAVE_OPLINE();
object = GET_OP1_OBJ_ZVAL_PTR_PTR_UNDEF(BP_VAR_RW);
object = GET_OP1_OBJ_ZVAL_PTR_UNDEF(BP_VAR_R);
property = GET_OP2_ZVAL_PTR(BP_VAR_R);

do {
Expand Down Expand Up @@ -1384,7 +1384,7 @@ ZEND_VM_C_LABEL(post_incdec_object):
} while (0);

FREE_OP2();
FREE_OP1_VAR_PTR();
FREE_OP1();
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
}

Expand Down Expand Up @@ -2113,39 +2113,39 @@ ZEND_VM_C_LABEL(fetch_obj_r_finish):
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
}

ZEND_VM_HANDLER(85, ZEND_FETCH_OBJ_W, VAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, FETCH_REF|DIM_WRITE|CACHE_SLOT)
ZEND_VM_HANDLER(85, ZEND_FETCH_OBJ_W, TMPVAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, FETCH_REF|DIM_WRITE|CACHE_SLOT)
{
USE_OPLINE
zval *property, *container, *result;

SAVE_OPLINE();

container = GET_OP1_OBJ_ZVAL_PTR_PTR_UNDEF(BP_VAR_W);
container = GET_OP1_OBJ_ZVAL_PTR_UNDEF(BP_VAR_R);
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 & ~ZEND_FETCH_OBJ_FLAGS) : NULL),
BP_VAR_W, opline->extended_value & ZEND_FETCH_OBJ_FLAGS, 1 OPLINE_CC EXECUTE_DATA_CC);
FREE_OP2();
if (OP1_TYPE == IS_VAR) {
if (OP1_TYPE & (IS_VAR|IS_TMP_VAR)) {
FREE_VAR_PTR_AND_EXTRACT_RESULT_IF_NECESSARY(opline->op1.var);
}
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
}

ZEND_VM_HANDLER(88, ZEND_FETCH_OBJ_RW, VAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, CACHE_SLOT)
ZEND_VM_HANDLER(88, ZEND_FETCH_OBJ_RW, TMPVAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, CACHE_SLOT)
{
USE_OPLINE
zval *property, *container, *result;

SAVE_OPLINE();
container = GET_OP1_OBJ_ZVAL_PTR_PTR_UNDEF(BP_VAR_RW);
container = GET_OP1_OBJ_ZVAL_PTR_UNDEF(BP_VAR_R);
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_RW, 0, 1 OPLINE_CC EXECUTE_DATA_CC);
FREE_OP2();
if (OP1_TYPE == IS_VAR) {
if (OP1_TYPE & (IS_VAR|IS_TMP_VAR)) {
FREE_VAR_PTR_AND_EXTRACT_RESULT_IF_NECESSARY(opline->op1.var);
}
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
Expand Down Expand Up @@ -2262,15 +2262,15 @@ ZEND_VM_C_LABEL(fetch_obj_is_finish):
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
}

ZEND_VM_COLD_CONST_HANDLER(94, ZEND_FETCH_OBJ_FUNC_ARG, CONST|TMP|VAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, FETCH_REF|CACHE_SLOT)
ZEND_VM_COLD_CONST_HANDLER(94, ZEND_FETCH_OBJ_FUNC_ARG, CONST|TMPVAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, FETCH_REF|CACHE_SLOT)
{
#if !ZEND_VM_SPEC
USE_OPLINE
#endif

if (UNEXPECTED(ZEND_CALL_INFO(EX(call)) & ZEND_CALL_SEND_ARG_BY_REF)) {
/* Behave like FETCH_OBJ_W */
if ((OP1_TYPE & (IS_CONST|IS_TMP_VAR))) {
if (OP1_TYPE == IS_CONST) {
ZEND_VM_DISPATCH_TO_HELPER(zend_use_tmp_in_write_context_helper);
}
ZEND_VM_DISPATCH_TO_HANDLER(ZEND_FETCH_OBJ_W);
Expand Down Expand Up @@ -2331,15 +2331,15 @@ ZEND_VM_HANDLER(155, ZEND_FETCH_LIST_W, VAR, CONST|TMPVAR|CV)
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
}

ZEND_VM_HANDLER(24, ZEND_ASSIGN_OBJ, VAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, CACHE_SLOT, SPEC(OP_DATA=CONST|TMP|VAR|CV))
ZEND_VM_HANDLER(24, ZEND_ASSIGN_OBJ, TMPVAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, CACHE_SLOT, SPEC(OP_DATA=CONST|TMP|VAR|CV))
{
USE_OPLINE
zval *object, *property, *value, tmp;
zend_object *zobj;
zend_string *name, *tmp_name;

SAVE_OPLINE();
object = GET_OP1_OBJ_ZVAL_PTR_PTR_UNDEF(BP_VAR_W);
object = GET_OP1_OBJ_ZVAL_PTR_UNDEF(BP_VAR_R);
property = GET_OP2_ZVAL_PTR(BP_VAR_R);
value = GET_OP_DATA_ZVAL_PTR(BP_VAR_R);

Expand Down Expand Up @@ -2470,7 +2470,7 @@ ZEND_VM_C_LABEL(free_and_exit_assign_obj):
FREE_OP_DATA();
ZEND_VM_C_LABEL(exit_assign_obj):
FREE_OP2();
FREE_OP1_VAR_PTR();
FREE_OP1();
/* assign_obj has two opcodes! */
ZEND_VM_NEXT_OPCODE_EX(1, 2);
}
Expand Down Expand Up @@ -2676,14 +2676,14 @@ ZEND_VM_HANDLER(30, ZEND_ASSIGN_REF, VAR|CV, VAR|CV, SRC)
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
}

ZEND_VM_HANDLER(32, ZEND_ASSIGN_OBJ_REF, VAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, CACHE_SLOT|SRC, SPEC(OP_DATA=VAR|CV))
ZEND_VM_HANDLER(32, ZEND_ASSIGN_OBJ_REF, TMPVAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, CACHE_SLOT|SRC, SPEC(OP_DATA=VAR|CV))
{
USE_OPLINE
zval *property, *container, *value_ptr;

SAVE_OPLINE();

container = GET_OP1_OBJ_ZVAL_PTR_PTR_UNDEF(BP_VAR_W);
container = GET_OP1_OBJ_ZVAL_PTR_UNDEF(BP_VAR_R);
property = GET_OP2_ZVAL_PTR(BP_VAR_R);

value_ptr = GET_OP_DATA_ZVAL_PTR_PTR(BP_VAR_W);
Expand All @@ -2706,7 +2706,7 @@ ZEND_VM_HANDLER(32, ZEND_ASSIGN_OBJ_REF, VAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, CA
zend_assign_to_property_reference(container, OP1_TYPE, property, OP2_TYPE, value_ptr OPLINE_CC EXECUTE_DATA_CC);
}

FREE_OP1_VAR_PTR();
FREE_OP1();
FREE_OP2();
FREE_OP_DATA_VAR_PTR();
ZEND_VM_NEXT_OPCODE_EX(1, 2);
Expand Down
Loading