Hi David,

When I was just checking PHP 5.4 compatibility with Xdebug I ran into 
the case where zend_eval_string() no longer would bail out when you'd 
evalulate something that doesn't work (like a standalone 
"$this->property;" without being in a class scope).

Xdebug has code (simplified) like this:

    int                res = FAILURE;

    zend_first_try {
        res = zend_eval_string(eval_string, ret_zval, "xdebug://debug-eval" 
TSRMLS_CC);
    } zend_end_try();

    return res;

This means that when zend_eval_string would cause an engine exception, 
res would be FAILURE. It would only be true if zend_eval_string 
succesfully completed.

However, after your patch, my "res" would still be set to SUCCESS. This 
is because zend_execute's bailout is now caught by the zend_end_try that 
you added:

    int retval;

    new_op_array = zend_compile_string(&pv, string_name TSRMLS_CC);

    if (new_op_array) {

        zend_try { // new code
            zend_execute(new_op_array TSRMLS_CC);
        } zend_end_try(); // new code

        retval = SUCCESS;

Obvious, the return value should not be SUCCESS if something incorrect 
was tried to be eval'ed.

Your patch also changes the behaviour of zend_eval_string; which is 
probably not a very big issue for many people, but it can easily be 
addressed by the following patch:


Index: zend_execute_API.c
===================================================================
--- zend_execute_API.c  (revision 322905)
+++ zend_execute_API.c  (working copy)
@@ -1195,8 +1195,11 @@
                }
                CG(interactive) = 0;
 
+               retval = SUCCESS;
                zend_try {
                        zend_execute(new_op_array TSRMLS_CC);
+               } zend_catch {
+                       retval = FAILURE;
                } zend_end_try();
 
                CG(interactive) = orig_interactive;
@@ -1218,7 +1221,6 @@
                destroy_op_array(new_op_array TSRMLS_CC);
                efree(new_op_array);
                EG(return_value_ptr_ptr) = original_return_value_ptr_ptr;
-               retval = SUCCESS;
        } else {
                retval = FAILURE;
        }


The patch just changes the setting of the return value around, and I think we
should include this in the upcoming RC.

cheers,
Derick




On Sat, 12 Nov 2011, David Soria Parra wrote:

> dsp                                      Sat, 12 Nov 2011 17:05:08 +0000
> 
> Revision: http://svn.php.net/viewvc?view=revision&revision=319102
> 
> Log:
> Fix #60218 (instantiating unknown class leads to memory leak in cli)
> 
> Bug: https://bugs.php.net/60218 (error getting bug information)
>       
> Changed paths:
>     U   php/php-src/branches/PHP_5_4/Zend/zend_execute_API.c
>     U   php/php-src/trunk/Zend/zend_execute_API.c
> 
> Modified: php/php-src/branches/PHP_5_4/Zend/zend_execute_API.c
> ===================================================================
> --- php/php-src/branches/PHP_5_4/Zend/zend_execute_API.c      2011-11-12 
> 16:32:10 UTC (rev 319101)
> +++ php/php-src/branches/PHP_5_4/Zend/zend_execute_API.c      2011-11-12 
> 17:05:08 UTC (rev 319102)
> @@ -1195,7 +1195,9 @@
>               }
>               CG(interactive) = 0;
> 
> -             zend_execute(new_op_array TSRMLS_CC);
> +             zend_try {
> +                 zend_execute(new_op_array TSRMLS_CC);
> +             } zend_end_try();
> 
>               CG(interactive) = orig_interactive;
>               if (local_retval_ptr) {
> 
> Modified: php/php-src/trunk/Zend/zend_execute_API.c
> ===================================================================
> --- php/php-src/trunk/Zend/zend_execute_API.c 2011-11-12 16:32:10 UTC (rev 
> 319101)
> +++ php/php-src/trunk/Zend/zend_execute_API.c 2011-11-12 17:05:08 UTC (rev 
> 319102)
> @@ -1195,7 +1195,9 @@
>               }
>               CG(interactive) = 0;
> 
> -             zend_execute(new_op_array TSRMLS_CC);
> +             zend_try {
> +                     zend_execute(new_op_array TSRMLS_CC);
> +             } zend_end_try();
> 
>               CG(interactive) = orig_interactive;
>               if (local_retval_ptr) {
> 
> 

-- 
http://derickrethans.nl | http://xdebug.org
Like Xdebug? Consider a donation: http://xdebug.org/donate.php
twitter: @derickr and @xdebug

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to