Skip to content

Convert ZEND_ECHO operand to string #5118

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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: 15 additions & 0 deletions ext/opcache/Optimizer/zend_optimizer.c
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,21 @@ int zend_optimizer_update_op1_const(zend_op_array *op_array,
case ZEND_FETCH_LIST_R:
case ZEND_COPY_TMP:
return 0;
case ZEND_ECHO:
{
zval zv;
if (Z_TYPE_P(val) != IS_STRING && zend_optimizer_eval_cast(&zv, IS_STRING, val) == SUCCESS) {
zval_ptr_dtor_nogc(val);
val = &zv;
}
Copy link
Member

Choose a reason for hiding this comment

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

I'd write this as:

			if (Z_TYPE_P(val) != IS_STRING && zend_optimizer_eval_cast(&zv, IS_STRING, val) == SUCCESS) {
				zval_ptr_dtor_nogc(val);
				val = &zv;
			}
			opline->op1.constant = zend_optimizer_add_literal(op_array, val);

Destroying val is technically not necessary because it so happens that the cast can only succeed if the original value wasn't refcounted, but I think this makes the logic of the code more obvious.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is IS_RESOURCE possible (e.g. STDOUT)? That is refcounted

Copy link
Member

Choose a reason for hiding this comment

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

I believe inside opcache it's not possible.

opline->op1.constant = zend_optimizer_add_literal(op_array, val);
if (Z_TYPE_P(val) == IS_STRING && Z_STRLEN_P(val) == 0) {
MAKE_NOP(opline);
}
/* TODO: In a subsequent pass, *after* this step and compacting nops, combine consecutive ZEND_ECHOs using the block information from ssa->cfg */
/* (e.g. for ext/opcache/tests/opt/sccp_010.phpt) */
break;
}
case ZEND_CONCAT:
case ZEND_FAST_CONCAT:
case ZEND_FETCH_R:
Expand Down
4 changes: 2 additions & 2 deletions ext/opcache/tests/opt/sccp_002.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ foo: ; (lines=4, args=1, vars=1, tmps=0)
; (after optimizer)
; %ssccp_002.php:2-12
L0 (2): CV0($x) = RECV 1
L1 (9): ECHO int(1)
L2 (11): ECHO int(1)
L1 (9): ECHO string("1")
L2 (11): ECHO string("1")
L3 (12): RETURN null
4 changes: 2 additions & 2 deletions ext/opcache/tests/opt/sccp_003.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ L0 (14): RETURN int(1)
foo: ; (lines=3, args=0, vars=0, tmps=0)
; (after optimizer)
; %ssccp_003.php:2-12
L0 (9): ECHO int(1)
L1 (11): ECHO int(1)
L0 (9): ECHO string("1")
L1 (11): ECHO string("1")
L2 (12): RETURN null
4 changes: 2 additions & 2 deletions ext/opcache/tests/opt/sccp_004.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,6 @@ foo: ; (lines=4, args=1, vars=1, tmps=0)
; (after optimizer)
; %ssccp_004.php:2-15
L0 (2): CV0($x) = RECV 1
L1 (11): ECHO bool(true)
L2 (14): ECHO int(1)
L1 (11): ECHO string("1")
L2 (14): ECHO string("1")
L3 (15): RETURN null
2 changes: 1 addition & 1 deletion ext/opcache/tests/opt/sccp_005.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@ foo: ; (lines=3, args=1, vars=1, tmps=0)
; (after optimizer)
; %ssccp_005.php:2-5
L0 (2): CV0($x) = RECV 1
L1 (4): ECHO int(2)
L1 (4): ECHO string("2")
L2 (5): RETURN null
2 changes: 1 addition & 1 deletion ext/opcache/tests/opt/sccp_007.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,5 @@ foo: ; (lines=3, args=1, vars=1, tmps=0)
; (after optimizer)
; %ssccp_007.php:2-9
L0 (2): CV0($x) = RECV 1
L1 (8): ECHO int(0)
L1 (8): ECHO string("0")
L2 (9): RETURN null
2 changes: 1 addition & 1 deletion ext/opcache/tests/opt/sccp_009.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,5 @@ foo: ; (lines=3, args=1, vars=1, tmps=0)
; (after optimizer)
; %ssccp_009.php:2-6
L0 (2): CV0($x) = RECV 1
L1 (5): ECHO int(2)
L1 (5): ECHO string("2")
L2 (6): RETURN null
4 changes: 2 additions & 2 deletions ext/opcache/tests/opt/sccp_010.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ L0 (15): RETURN int(1)
foo: ; (lines=3, args=0, vars=0, tmps=0)
; (after optimizer)
; %ssccp_010.php:2-13
L0 (10): ECHO int(1)
L1 (12): ECHO int(1)
L0 (10): ECHO string("1")
L1 (12): ECHO string("1")
L2 (13): RETURN null
2 changes: 1 addition & 1 deletion ext/opcache/tests/opt/sccp_011.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,5 @@ foo: ; (lines=3, args=1, vars=1, tmps=0)
; (after optimizer)
; %ssccp_011.php:2-12
L0 (2): CV0($x) = RECV 1
L1 (11): ECHO int(0)
L1 (11): ECHO string("0")
L2 (12): RETURN null
4 changes: 2 additions & 2 deletions ext/opcache/tests/opt/sccp_012.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ L0 (17): RETURN int(1)
foo: ; (lines=3, args=0, vars=0, tmps=0)
; (after optimizer)
; %ssccp_012.php:2-15
L0 (10): ECHO int(1)
L1 (14): ECHO int(4)
L0 (10): ECHO string("1")
L1 (14): ECHO string("4")
L2 (15): RETURN null
2 changes: 1 addition & 1 deletion ext/opcache/tests/opt/sccp_022.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ L1 (3): ASSIGN_DIM CV1($a) int(0)
L2 (3): OP_DATA CV0($x)
L3 (4): ASSIGN_DIM CV1($a) int(1)
L4 (4): OP_DATA int(5)
L5 (5): ECHO int(5)
L5 (5): ECHO string("5")
L6 (6): ASSIGN_OBJ CV1($a) string("foo")
L7 (6): OP_DATA int(5)
L8 (7): T2 = FETCH_DIM_R CV1($a) int(1)
Expand Down
36 changes: 36 additions & 0 deletions ext/opcache/tests/opt/sccp_031.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
--TEST--
SCCP 031: Echo optimizations
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.optimization_level=-1
opcache.opt_debug_level=0x20000
opcache.preload=
--SKIPIF--
<?php require_once('skipif.inc'); ?>
--FILE--
<?php
function foo() {
$k = 0;
$a = [null];
echo isset($a[$k]);
echo "b";
echo isset($a[$k+1]);
echo "c";
echo $a[$k];
echo $a; // Should not be optimized
}
?>
--EXPECTF--
$_main: ; (lines=1, args=0, vars=0, tmps=0)
; (after optimizer)
; %ssccp_031.php:1-13
L0 (13): RETURN int(1)

foo: ; (lines=4, args=0, vars=0, tmps=0)
; (after optimizer)
; %s_031.php:2-11
L0 (6): ECHO string("b")
L1 (8): ECHO string("c")
L2 (10): ECHO array(...)
L3 (11): RETURN null