Skip to content

Make ASSIGN, ASSIGN_OP, INC and DEC opcodes to return IS_TMP_VAR instead of IS_VAR #5158

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

Make ASSIGN, ASSIGN_OP, INC and DEC opcodes to return IS_TMP_VAR instead of IS_VAR #5158

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
291f1fb
Make ASSIGN, ASSIGN_OP, INC and DEC opcodes to return IS_TMP_VAR inst…
dstogov Feb 6, 2020
0824c3d
Reflect changes in UPGRADING
dstogov Feb 7, 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
5 changes: 5 additions & 0 deletions UPGRADING
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,11 @@ PHP 8.0 UPGRADE NOTES
warning.
. Uncaught exceptions now go through "clean shutdown", which means that
destructors will be called after an uncaught exception.
. Compile time fatal error "Only variables can be passed by reference" has been
delayed until run-time and converted to "Cannot pass parameter by reference"
exception.
. Some "Only variables should be passed by reference" notices have been converted
to "Cannot pass parameter by reference" exception.

- COM:
. Removed the ability to import case-insensitive constants from type
Expand Down
9 changes: 9 additions & 0 deletions UPGRADING.INTERNALS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ PHP 8.0 INTERNALS UPGRADE NOTES
h. zend_value_error()
i. get_closure() object handler
j. compare_objects() and compare() object handlers
k. The 'I' length modifier
l. Some VM instructions switched to IS_TMP_VAR result insted of IS_VAR

2. Build system changes
a. Abstract
Expand Down Expand Up @@ -89,6 +91,13 @@ PHP 8.0 INTERNALS UPGRADE NOTES
The 'v' format from the custom snprintf and spprintf implementations has
been removed. Use the standard 's' format instead.

l. Some VM instructions switched to IS_TMP_VAR result insted of IS_VAR.
Actually, all assignments (ZEND_ASSIGN, ZEND_ASSIGN_DIM, ZEND_ASSIGN_OBJ,
ZEND_ASSIGN_STATIC_PROP), all compound assignments (ZEND_ASSIGN_OP,
ZEND_ASSIGN_DIM_OP, ZEND_ASSIGN_OBJ_OP, ZEND_ASSIGN_STATIC_PROP_OP) and all
pre increments/decremets (ZEND_PRE_INC, ZEND_PRE_DEC, ZEND_PRE_INC_OBJ
Copy link
Member

Choose a reason for hiding this comment

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

decremets -> decrements

ZEND_PRE_DEC_OBJ, ZEND_PRE_INC_STATIC_PROP ZEND_PRE_DEC_STATIC_PROP).

========================
2. Build system changes
========================
Expand Down
34 changes: 21 additions & 13 deletions Zend/tests/bug72038.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,31 @@ Bug #72038 (Function calls with values to a by-ref parameter don't always throw
--FILE--
<?php

test($foo = new stdClass);
var_dump($foo);
test($bar = 2);
var_dump($bar);
test($baz = &$bar);
var_dump($baz);
try {
test($foo = new stdClass);
var_dump($foo);
} catch (Throwable $e) {
echo "Exception: " . $e->getMessage() . "\n";
}
try {
test($bar = 2);
var_dump($bar);
} catch (Throwable $e) {
echo "Exception: " . $e->getMessage() . "\n";
}
try {
test($baz = &$bar);
var_dump($baz);
} catch (Throwable $e) {
echo "Exception: " . $e->getMessage() . "\n";
}

function test(&$param) {
$param = 1;
}

?>
--EXPECTF--
Notice: Only variables should be passed by reference in %s on line %d
object(stdClass)#1 (0) {
}

Notice: Only variables should be passed by reference in %s on line %d
int(2)
--EXPECT--
Exception: Cannot pass parameter 1 by reference
Exception: Cannot pass parameter 1 by reference
int(1)
5 changes: 4 additions & 1 deletion Zend/tests/bug73663_2.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@ change(list($val) = $array);
var_dump($array);
?>
--EXPECTF--
Fatal error: Only variables can be passed by reference in %s on line %d
Fatal error: Uncaught Error: Cannot pass parameter 1 by reference in %s:%d
Stack trace:
#0 {main}
thrown in %s on line %d
27 changes: 15 additions & 12 deletions Zend/tests/bug78154.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,23 @@ Bug #78154: SEND_VAR_NO_REF does not always send reference
<?php

namespace {
var_dump(similar_text('a', 'a', $c=0x44444444));
var_dump($c);
try {
var_dump(similar_text('a', 'a', $c=0x44444444));
var_dump($c);
} catch (Throwable $e) {
echo "Exception: " . $e->getMessage() . "\n";
}
}
namespace Foo {
var_dump(similar_text('a', 'a', $d=0x44444444));
var_dump($d);
try {
var_dump(similar_text('a', 'a', $d=0x44444444));
var_dump($d);
} catch (\Throwable $e) {
echo "Exception: " . $e->getMessage() . "\n";
}
}

?>
--EXPECTF--
Notice: Only variables should be passed by reference in %s on line %d
int(1)
int(1145324612)

Notice: Only variables should be passed by reference in %s on line %d
int(1)
int(1145324612)
--EXPECT--
Exception: Cannot pass parameter 3 by reference
Exception: Cannot pass parameter 3 by reference
5 changes: 4 additions & 1 deletion Zend/tests/errmsg_022.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,7 @@ foo(1);
echo "Done\n";
?>
--EXPECTF--
Fatal error: Only variables can be passed by reference in %s on line %d
Fatal error: Uncaught Error: Cannot pass parameter 1 by reference in %s:%d
Stack trace:
#0 {main}
thrown in %s on line %d
5 changes: 4 additions & 1 deletion Zend/tests/variadic/by_ref_error.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,7 @@ test(1);

?>
--EXPECTF--
Fatal error: Only variables can be passed by reference in %s on line %d
Fatal error: Uncaught Error: Cannot pass parameter 1 by reference in %s:%d
Stack trace:
#0 {main}
thrown in %s on line %d
55 changes: 46 additions & 9 deletions Zend/zend_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,8 @@ void zend_do_free(znode *op1) /* {{{ */
if (op1->op_type == IS_TMP_VAR) {
zend_op *opline = &CG(active_op_array)->opcodes[CG(active_op_array)->last-1];

while (opline->opcode == ZEND_END_SILENCE) {
while (opline->opcode == ZEND_END_SILENCE ||
opline->opcode == ZEND_OP_DATA) {
opline--;
}

Expand All @@ -738,6 +739,22 @@ void zend_do_free(znode *op1) /* {{{ */
opline->opcode -= 2;
opline->result_type = IS_UNUSED;
return;
case ZEND_ASSIGN:
case ZEND_ASSIGN_DIM:
case ZEND_ASSIGN_OBJ:
case ZEND_ASSIGN_STATIC_PROP:
case ZEND_ASSIGN_OP:
case ZEND_ASSIGN_DIM_OP:
case ZEND_ASSIGN_OBJ_OP:
case ZEND_ASSIGN_STATIC_PROP_OP:
case ZEND_PRE_INC_STATIC_PROP:
case ZEND_PRE_DEC_STATIC_PROP:
case ZEND_PRE_INC_OBJ:
case ZEND_PRE_DEC_OBJ:
case ZEND_PRE_INC:
case ZEND_PRE_DEC:
opline->result_type = IS_UNUSED;
return;
}
}

Expand Down Expand Up @@ -2921,7 +2938,7 @@ void zend_compile_assign(znode *result, zend_ast *ast) /* {{{ */
zend_delayed_compile_var(&var_node, var_ast, BP_VAR_W, 0);
zend_compile_expr(&expr_node, expr_ast);
zend_delayed_compile_end(offset);
zend_emit_op(result, ZEND_ASSIGN, &var_node, &expr_node);
zend_emit_op_tmp(result, ZEND_ASSIGN, &var_node, &expr_node);
return;
case ZEND_AST_STATIC_PROP:
offset = zend_delayed_compile_begin();
Expand All @@ -2930,6 +2947,8 @@ void zend_compile_assign(znode *result, zend_ast *ast) /* {{{ */

opline = zend_delayed_compile_end(offset);
opline->opcode = ZEND_ASSIGN_STATIC_PROP;
opline->result_type = IS_TMP_VAR;
result->op_type = IS_TMP_VAR;

zend_emit_op_data(&expr_node);
return;
Expand All @@ -2953,6 +2972,8 @@ void zend_compile_assign(znode *result, zend_ast *ast) /* {{{ */

opline = zend_delayed_compile_end(offset);
opline->opcode = ZEND_ASSIGN_DIM;
opline->result_type = IS_TMP_VAR;
result->op_type = IS_TMP_VAR;

opline = zend_emit_op_data(&expr_node);
return;
Expand All @@ -2963,6 +2984,8 @@ void zend_compile_assign(znode *result, zend_ast *ast) /* {{{ */

opline = zend_delayed_compile_end(offset);
opline->opcode = ZEND_ASSIGN_OBJ;
opline->result_type = IS_TMP_VAR;
result->op_type = IS_TMP_VAR;

zend_emit_op_data(&expr_node);
return;
Expand Down Expand Up @@ -3086,7 +3109,7 @@ void zend_compile_compound_assign(znode *result, zend_ast *ast) /* {{{ */
zend_delayed_compile_var(&var_node, var_ast, BP_VAR_RW, 0);
zend_compile_expr(&expr_node, expr_ast);
zend_delayed_compile_end(offset);
opline = zend_emit_op(result, ZEND_ASSIGN_OP, &var_node, &expr_node);
opline = zend_emit_op_tmp(result, ZEND_ASSIGN_OP, &var_node, &expr_node);
opline->extended_value = opcode;
return;
case ZEND_AST_STATIC_PROP:
Expand All @@ -3098,6 +3121,8 @@ void zend_compile_compound_assign(znode *result, zend_ast *ast) /* {{{ */
cache_slot = opline->extended_value;
opline->opcode = ZEND_ASSIGN_STATIC_PROP_OP;
opline->extended_value = opcode;
opline->result_type = IS_TMP_VAR;
result->op_type = IS_TMP_VAR;

opline = zend_emit_op_data(&expr_node);
opline->extended_value = cache_slot;
Expand All @@ -3110,6 +3135,8 @@ void zend_compile_compound_assign(znode *result, zend_ast *ast) /* {{{ */
opline = zend_delayed_compile_end(offset);
opline->opcode = ZEND_ASSIGN_DIM_OP;
opline->extended_value = opcode;
opline->result_type = IS_TMP_VAR;
result->op_type = IS_TMP_VAR;

zend_emit_op_data(&expr_node);
return;
Expand All @@ -3122,6 +3149,8 @@ void zend_compile_compound_assign(znode *result, zend_ast *ast) /* {{{ */
cache_slot = opline->extended_value;
opline->opcode = ZEND_ASSIGN_OBJ_OP;
opline->extended_value = opcode;
opline->result_type = IS_TMP_VAR;
result->op_type = IS_TMP_VAR;

opline = zend_emit_op_data(&expr_node);
opline->extended_value = cache_slot;
Expand Down Expand Up @@ -3236,11 +3265,9 @@ uint32_t zend_compile_args(zend_ast *ast, zend_function *fbc) /* {{{ */
opcode = ZEND_SEND_VAR_EX;
}
} else {
if (fbc) {
/* Delay "Only variables can be passed by reference" error to execution */
if (fbc && !ARG_MUST_BE_SENT_BY_REF(fbc, arg_num)) {
opcode = ZEND_SEND_VAL;
if (ARG_MUST_BE_SENT_BY_REF(fbc, arg_num)) {
zend_error_noreturn(E_COMPILE_ERROR, "Only variables can be passed by reference");
}
} else {
opcode = ZEND_SEND_VAL_EX;
}
Expand Down Expand Up @@ -7577,13 +7604,17 @@ void zend_compile_pre_incdec(znode *result, zend_ast *ast) /* {{{ */
if (var_ast->kind == ZEND_AST_PROP) {
zend_op *opline = zend_compile_prop(result, var_ast, BP_VAR_RW, 0);
opline->opcode = ast->kind == ZEND_AST_PRE_INC ? ZEND_PRE_INC_OBJ : ZEND_PRE_DEC_OBJ;
opline->result_type = IS_TMP_VAR;
result->op_type = IS_TMP_VAR;
} else if (var_ast->kind == ZEND_AST_STATIC_PROP) {
zend_op *opline = zend_compile_static_prop(result, var_ast, BP_VAR_RW, 0, 0);
opline->opcode = ast->kind == ZEND_AST_PRE_INC ? ZEND_PRE_INC_STATIC_PROP : ZEND_PRE_DEC_STATIC_PROP;
opline->result_type = IS_TMP_VAR;
result->op_type = IS_TMP_VAR;
} else {
znode var_node;
zend_compile_var(&var_node, var_ast, BP_VAR_RW, 0);
zend_emit_op(result, ast->kind == ZEND_AST_PRE_INC ? ZEND_PRE_INC : ZEND_PRE_DEC,
zend_emit_op_tmp(result, ast->kind == ZEND_AST_PRE_INC ? ZEND_PRE_INC : ZEND_PRE_DEC,
&var_node, NULL);
}
}
Expand Down Expand Up @@ -7764,20 +7795,26 @@ void zend_compile_assign_coalesce(znode *result, zend_ast *ast) /* {{{ */
opline = &CG(active_op_array)->opcodes[CG(active_op_array)->last-1];
switch (var_ast->kind) {
case ZEND_AST_VAR:
zend_emit_op(&assign_node, ZEND_ASSIGN, &var_node_w, &default_node);
zend_emit_op_tmp(&assign_node, ZEND_ASSIGN, &var_node_w, &default_node);
break;
case ZEND_AST_STATIC_PROP:
opline->opcode = ZEND_ASSIGN_STATIC_PROP;
opline->result_type = IS_TMP_VAR;
var_node_w.op_type = IS_TMP_VAR;
zend_emit_op_data(&default_node);
assign_node = var_node_w;
break;
case ZEND_AST_DIM:
opline->opcode = ZEND_ASSIGN_DIM;
opline->result_type = IS_TMP_VAR;
var_node_w.op_type = IS_TMP_VAR;
zend_emit_op_data(&default_node);
assign_node = var_node_w;
break;
case ZEND_AST_PROP:
opline->opcode = ZEND_ASSIGN_OBJ;
opline->result_type = IS_TMP_VAR;
var_node_w.op_type = IS_TMP_VAR;
zend_emit_op_data(&default_node);
assign_node = var_node_w;
break;
Expand Down
48 changes: 33 additions & 15 deletions ext/opcache/Optimizer/block_pass.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,13 +250,38 @@ static void zend_optimize_block(zend_basic_block *block, zend_op_array *op_array
case ZEND_FREE:
if (opline->op1_type == IS_TMP_VAR) {
src = VAR_SOURCE(opline->op1);
if (src &&
(src->opcode == ZEND_BOOL || src->opcode == ZEND_BOOL_NOT)) {
/* T = BOOL(X), FREE(T) => T = BOOL(X) */
/* The remaining BOOL is removed by a separate optimization */
VAR_SOURCE(opline->op1) = NULL;
MAKE_NOP(opline);
++(*opt_count);
if (src) {
switch (src->opcode) {
case ZEND_BOOL:
case ZEND_BOOL_NOT:
/* T = BOOL(X), FREE(T) => T = BOOL(X) */
/* The remaining BOOL is removed by a separate optimization */
VAR_SOURCE(opline->op1) = NULL;
MAKE_NOP(opline);
++(*opt_count);
break;
case ZEND_ASSIGN:
case ZEND_ASSIGN_DIM:
case ZEND_ASSIGN_OBJ:
case ZEND_ASSIGN_STATIC_PROP:
case ZEND_ASSIGN_OP:
case ZEND_ASSIGN_DIM_OP:
case ZEND_ASSIGN_OBJ_OP:
case ZEND_ASSIGN_STATIC_PROP_OP:
case ZEND_PRE_INC:
case ZEND_PRE_DEC:
case ZEND_PRE_INC_OBJ:
case ZEND_PRE_DEC_OBJ:
case ZEND_PRE_INC_STATIC_PROP:
case ZEND_PRE_DEC_STATIC_PROP:
src->result_type = IS_UNUSED;
VAR_SOURCE(opline->op1) = NULL;
MAKE_NOP(opline);
++(*opt_count);
break;
default:
break;
}
}
} else if (opline->op1_type == IS_VAR) {
src = VAR_SOURCE(opline->op1);
Expand Down Expand Up @@ -1649,7 +1674,7 @@ static void zend_t_usage(zend_cfg *cfg, zend_op_array *op_array, zend_bitset use

while (opline >= end) {
/* usage checks */
if (opline->result_type == IS_VAR) {
if (opline->result_type & (IS_VAR|IS_TMP_VAR)) {
if (!zend_bitset_in(usage, VAR_NUM(opline->result.var))) {
switch (opline->opcode) {
case ZEND_ASSIGN_OP:
Expand All @@ -1666,13 +1691,6 @@ static void zend_t_usage(zend_cfg *cfg, zend_op_array *op_array, zend_bitset use
case ZEND_DO_FCALL_BY_NAME:
opline->result_type = IS_UNUSED;
break;
}
} else {
zend_bitset_excl(usage, VAR_NUM(opline->result.var));
}
} else if (opline->result_type == IS_TMP_VAR) {
if (!zend_bitset_in(usage, VAR_NUM(opline->result.var))) {
switch (opline->opcode) {
case ZEND_POST_INC:
case ZEND_POST_DEC:
case ZEND_POST_INC_OBJ:
Expand Down
2 changes: 1 addition & 1 deletion ext/opcache/Optimizer/zend_ssa.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ static void place_essa_pis(
pi_range_not_equals(pi, -1, 1);
}
}
} else if (opline->op1_type == IS_VAR &&
} else if (opline->op1_type == IS_TMP_VAR &&
((opline-1)->opcode == ZEND_PRE_INC ||
(opline-1)->opcode == ZEND_PRE_DEC) &&
opline->op1.var == (opline-1)->result.var &&
Expand Down
7 changes: 6 additions & 1 deletion ext/pcre/tests/preg_match_all_error3.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,9 @@ var_dump(preg_match_all($regex, $subject, 'test'));
echo "Done";
?>
--EXPECTF--
Fatal error: Only variables can be passed by reference in %spreg_match_all_error3.php on line %d
*** Testing preg_match_all() : error conditions ***

Fatal error: Uncaught Error: Cannot pass parameter 3 by reference in %s:%d
Stack trace:
#0 {main}
thrown in %s on line %d
Loading