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

TMP_VAR is not only used once #183

Closed
laruence opened this Issue May 24, 2014 · 2 comments

Comments

Projects
None yet
2 participants
@laruence
Member

laruence commented May 24, 2014

This is the reason for issue #176 , thanks for the reporter provided a vps for me to debugging.

the new repleace_tmp_by_const assume TMP_VAR is used only once

static void replace_tmp_by_const(zend_op_array *op_array,
                                 zend_op       *opline,
                                 zend_uint      var,
                                 zval          *val
                                 TSRMLS_DC)
{
    zend_op *end = op_array->opcodes + op_array->last;

    while (opline < end) {
        if (ZEND_OP1_TYPE(opline) == IS_TMP_VAR &&
            ZEND_OP1(opline).var == var) {

            update_op1_const(op_array, opline, val TSRMLS_CC);
            /* TMP_VAR my be used only once */
            break;
        }

unfortunately it's not, for a example:

<?php
switch(PHP_OS)
{
        case "Linux":
        break;
        case "Linux":
        break;
        case "Darwin":
        break;
}

the FETCH_CONSTENT result will be used multiply times. include a implicit ZEND_FREE

thanks

@laruence

This comment has been minimized.

Member

laruence commented May 24, 2014

A fix could be:

diff --git a/ext/opcache/Optimizer/zend_optimizer.c b/ext/opcache/Optimizer/zend_optimizer.c
index fd0f77d..ce61728 100644
--- a/ext/opcache/Optimizer/zend_optimizer.c
+++ b/ext/opcache/Optimizer/zend_optimizer.c
@@ -279,21 +279,37 @@ static void replace_tmp_by_const(zend_op_array *op_array,
                                  TSRMLS_DC)
 {
    zend_op *end = op_array->opcodes + op_array->last;
+   zend_bool used = 0;

    while (opline < end) {
        if (ZEND_OP1_TYPE(opline) == IS_TMP_VAR &&
            ZEND_OP1(opline).var == var) {

-           update_op1_const(op_array, opline, val TSRMLS_CC);
-           /* TMP_VAR my be used only once */
-           break;
+           if (opline->opcode == ZEND_FREE) {
+               MAKE_NOP(opline);
+               if (!used) {
+                   zval_dtor(val);
+               }
+               break;
+           } else {
+               if (used) {
+                   zval_copy_ctor(val);
+               } else {
+                   used = 1;
+               }
+               update_op1_const(op_array, opline, val TSRMLS_CC);
+           }
        }

        if (ZEND_OP2_TYPE(opline) == IS_TMP_VAR &&
            ZEND_OP2(opline).var == var) {

+           if (used) {
+               zval_copy_ctor(val);
+           } else {
+               used = 1;
+           }
            update_op2_const(op_array, opline, val TSRMLS_CC);
-           /* TMP_VAR my be used only once */
            break;
        }
        opline++;

@dstogov

This comment has been minimized.

Member

dstogov commented May 26, 2014

It must be fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment