Re: svn: /php/php-src/ branches/PHP_5_4/Zend/zend_execute_API.c trunk/Zend/zend_execute_API.c

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