Hi Derick:
On Sun, Jan 29, 2012 at 1:03 AM, David Soria Parra <dso...@gmx.net> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Thank you for catching that. Please go ahead and commit!.
>
> Thanks
>
> On 01/28/2012 04:00 PM, Derick Rethans wrote:
>> 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:
there comes a new bug relate to this change, #60978, in php_cli.c
case PHP_MODE_CLI_DIRECT:
cli_register_file_handles(TSRMLS_C);
if (zend_eval_string_ex(exec_direct, NULL, "Command line
code", 1 TSRMLS_CC) == FAILURE) {
exit_status=254;
}
return failure in zend_eval_stringl will cause exit_status be overrided.
and I think a appropriate way to fix these issues (memleak one, xdebug
one, and exit code one), is fix it in the old style way , which is :
catch -> free -> throw agian.
which I have made a patch(see blow),
what do you guys think? and also if this patch is okey, I will also
ask for a permission to ci to 5.4.
thanks
Index: Zend/zend_execute_API.c
===================================================================
--- Zend/zend_execute_API.c (revision 323082)
+++ Zend/zend_execute_API.c (working copy)
@@ -1195,11 +1195,12 @@
}
CG(interactive) = 0;
- retval = SUCCESS;
zend_try {
zend_execute(new_op_array TSRMLS_CC);
} zend_catch {
- retval = FAILURE;
+ destroy_op_array(new_op_array TSRMLS_CC);
+ efree(new_op_array);
+ zend_bailout();
} zend_end_try();
CG(interactive) = orig_interactive;
@@ -1221,6 +1222,7 @@
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;
}
>>
>>
>> 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) {
>>>
>>>
>>
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iQIcBAEBAgAGBQJPJCpeAAoJEAT0aMuPE7Z1v1oP+wV/jLdUNwhLu5rlWgpGbSrP
> 31HdRF1OzwTA7WfkCh0Vj6J59iGFbwF/odZatXxMHvX++KPuPScveGtFsCXon3zv
> 4N2AfvulZrlaSgaxyB2KAII+JhqEH5fX1mqooz0Xqz8zeZXqCKSdCO0FJVWUIm+1
> 6qWc7WIhsB4nwRBNT/uCqMx2btFgcPLqW72iwfPw7uWTSUQfPW8vOpwsv+5LXMKd
> 2wHG9+6C+dQXQEBL+bWXetCKIPzL1Q28LnDr2iQKtS0qLA3p0fbRkpCt4VVUkQ6t
> fO8tfZ26XIX9Ms3kQVFtQcwfNuQ47j8zmOSxVd14u2d2suYuXIYN8xpimgKU6xSM
> q2INXZVtTjYenSAPLfLwFdQXb4RqTRk4Gtv2exTCZPqJDSw9WQgZ5KX6VoYilcN6
> HkxT7i++13+RlUFr58RjE7DKSC7SQ3ZnHagxz9INm9wDl5CJGJMTkwWrTUhdh412
> C5wHgLnxLJvAmhvFZnMfkgZF8YPEs82+cN7M6PFz7xVvmq++zmavb1icAvysvb1d
> VSQ+V8N3WGPaprOzSbqQW8QFl2dUAOKKBJIrS8aPIsDHIlIv9UHuwiXCx4Y/51W8
> llVGBzYTY9RLf9tZYsIobMw7WseH3N52K0KcmAsjzEu7RJeBuk/WFQf/9z+ufoMl
> L/HF6eiz0eA+0F2pYA84
> =EzEH
> -----END PGP SIGNATURE-----
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
--
Laruence Xinchen Hui
http://www.laruence.com/
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
