zendtech / ZendOptimizerPlus

Other
914 stars 142 forks source link

TMP_VAR is not only used once #183

Closed laruence closed 10 years ago

laruence commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

It must be fixed now.