Re: [PHP-CVS] com php-src: Fixed bug #64135 (Exceptions from set_error_handler are not always propagated): NEWS Zend/tests/bug61767.phpt Zend/zend_vm_def.h Zend/zend_vm_execute.h
Hey: I fixed these issues, and extra one clone issue, committed here: https://github.com/php/php-src/commit/75742d57eb34c2c1c4d23d55f9b9ee44939b32c2 and if you can find more, please tell me, it will be appreciated. thanks On Sat, Feb 16, 2013 at 10:25 PM, Laruence wrote: > On Fri, Feb 8, 2013 at 8:36 PM, Nikita Popov wrote: >> On Fri, Feb 8, 2013 at 4:11 AM, Laruence wrote: >>> >>> On Fri, Feb 8, 2013 at 3:36 AM, Nikita Popov wrote: >>> > On Thu, Feb 7, 2013 at 4:44 PM, Xinchen Hui wrote: >>> >> >>> >> Commit:290509755ac4a3279b2b31b899aa9f2dd780f5f4 >>> >> Author:Xinchen Hui Thu, 7 Feb 2013 >>> >> 23:44:46 >>> >> +0800 >>> >> Parents: 0547a36e95ec36025a30e93e971d26b6b1ecf0e9 >>> >> Branches: PHP-5.5 >>> >> >>> >> Link: >>> >> >>> >> http://git.php.net/?p=php-src.git;a=commitdiff;h=290509755ac4a3279b2b31b899aa9f2dd780f5f4 >>> >> >>> >> Log: >>> >> Fixed bug #64135 (Exceptions from set_error_handler are not always >>> >> propagated) >>> > >>> > >>> > Is there any particular reason why the exception check was added only >>> > the >>> > that error branch? Can't it be done right after the GET_OP1_OBJ_ZVAL_PTR >>> > in >>> > http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2442? This way it >>> > would >>> > cover the other error conditions as well. >>> Hey: >>> >>> could you please example one, which will trigger a error(warning, >>> notice) out of that branch? >> >> >> Looking at the code a bit more, you are right, those branches can't be >> reached. The only warning that the GET_ZVAL_PTR can trigger is an undefined >> variable in which case the return value will be an uninited zval, which will >> always use the last error branch. >> >> What I did notice though is that this commit just fixes one out of very many >> cases where something like this or something similar could occur. E.g. just >> a few lines above it the same problem exists: >> http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2432 >> >> So if you change the test code from $undefinedObject->$definedMethod() to >> $definedObject->$undefinedMethod() or $undefinedObject->$undefinedMethod() >> it will still fail. >> >> A few lines higher FETCH_CLASS has the same problem >> (http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2398). Going down a >> bit INIT_STATIC_METHOD_CALL has this too: >> http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2548. >> INIT_FCALL_BY_NAME also has this: >> http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2753 So you can trigger >> it with just $undefined() too. Again the same thing in THROW: >> http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2963 (throw >> $undefined;). > > thanks for the notice. you are right , there are some similar issues > need to be fixed. > >> >> >> I could continue this list, but I think you get the point ^^ This issue >> exists pretty much everywhere where a ZVAL_PTR fetch is followed by some >> error condition that can happen when the zval is NULL. And we have a *lot* >> of those. So I'm not sure that it really makes sense to fix one of those >> cases and leave 50 others intact. Either this should be fixed generically >> (e.g. on the side of GET_ZVAL_PTR, though that might incur a performance >> penalty) or should be just left alone. > > or , we fix one when we found one. > > thanks >> >> Thanks, >> Nikita > > > > -- > Laruence Xinchen Hui > http://www.laruence.com/ -- Laruence Xinchen Hui http://www.laruence.com/ -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] com php-src: Fixed bug #64135 (Exceptions from set_error_handler are not always propagated): NEWS Zend/tests/bug61767.phpt Zend/zend_vm_def.h Zend/zend_vm_execute.h
On Fri, Feb 8, 2013 at 8:36 PM, Nikita Popov wrote: > On Fri, Feb 8, 2013 at 4:11 AM, Laruence wrote: >> >> On Fri, Feb 8, 2013 at 3:36 AM, Nikita Popov wrote: >> > On Thu, Feb 7, 2013 at 4:44 PM, Xinchen Hui wrote: >> >> >> >> Commit:290509755ac4a3279b2b31b899aa9f2dd780f5f4 >> >> Author:Xinchen Hui Thu, 7 Feb 2013 >> >> 23:44:46 >> >> +0800 >> >> Parents: 0547a36e95ec36025a30e93e971d26b6b1ecf0e9 >> >> Branches: PHP-5.5 >> >> >> >> Link: >> >> >> >> http://git.php.net/?p=php-src.git;a=commitdiff;h=290509755ac4a3279b2b31b899aa9f2dd780f5f4 >> >> >> >> Log: >> >> Fixed bug #64135 (Exceptions from set_error_handler are not always >> >> propagated) >> > >> > >> > Is there any particular reason why the exception check was added only >> > the >> > that error branch? Can't it be done right after the GET_OP1_OBJ_ZVAL_PTR >> > in >> > http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2442? This way it >> > would >> > cover the other error conditions as well. >> Hey: >> >> could you please example one, which will trigger a error(warning, >> notice) out of that branch? > > > Looking at the code a bit more, you are right, those branches can't be > reached. The only warning that the GET_ZVAL_PTR can trigger is an undefined > variable in which case the return value will be an uninited zval, which will > always use the last error branch. > > What I did notice though is that this commit just fixes one out of very many > cases where something like this or something similar could occur. E.g. just > a few lines above it the same problem exists: > http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2432 > > So if you change the test code from $undefinedObject->$definedMethod() to > $definedObject->$undefinedMethod() or $undefinedObject->$undefinedMethod() > it will still fail. > > A few lines higher FETCH_CLASS has the same problem > (http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2398). Going down a > bit INIT_STATIC_METHOD_CALL has this too: > http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2548. > INIT_FCALL_BY_NAME also has this: > http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2753 So you can trigger > it with just $undefined() too. Again the same thing in THROW: > http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2963 (throw > $undefined;). thanks for the notice. you are right , there are some similar issues need to be fixed. > > > I could continue this list, but I think you get the point ^^ This issue > exists pretty much everywhere where a ZVAL_PTR fetch is followed by some > error condition that can happen when the zval is NULL. And we have a *lot* > of those. So I'm not sure that it really makes sense to fix one of those > cases and leave 50 others intact. Either this should be fixed generically > (e.g. on the side of GET_ZVAL_PTR, though that might incur a performance > penalty) or should be just left alone. or , we fix one when we found one. thanks > > Thanks, > Nikita -- Laruence Xinchen Hui http://www.laruence.com/ -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] com php-src: Fixed bug #64135 (Exceptions from set_error_handler are not always propagated): NEWS Zend/tests/bug61767.phpt Zend/zend_vm_def.h Zend/zend_vm_execute.h
On Fri, Feb 8, 2013 at 4:11 AM, Laruence wrote: > On Fri, Feb 8, 2013 at 3:36 AM, Nikita Popov wrote: > > On Thu, Feb 7, 2013 at 4:44 PM, Xinchen Hui wrote: > >> > >> Commit:290509755ac4a3279b2b31b899aa9f2dd780f5f4 > >> Author:Xinchen Hui Thu, 7 Feb 2013 > 23:44:46 > >> +0800 > >> Parents: 0547a36e95ec36025a30e93e971d26b6b1ecf0e9 > >> Branches: PHP-5.5 > >> > >> Link: > >> > http://git.php.net/?p=php-src.git;a=commitdiff;h=290509755ac4a3279b2b31b899aa9f2dd780f5f4 > >> > >> Log: > >> Fixed bug #64135 (Exceptions from set_error_handler are not always > >> propagated) > > > > > > Is there any particular reason why the exception check was added only the > > that error branch? Can't it be done right after the GET_OP1_OBJ_ZVAL_PTR > in > > http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2442? This way it > would > > cover the other error conditions as well. > Hey: > > could you please example one, which will trigger a error(warning, > notice) out of that branch? > Looking at the code a bit more, you are right, those branches can't be reached. The only warning that the GET_ZVAL_PTR can trigger is an undefined variable in which case the return value will be an uninited zval, which will always use the last error branch. What I did notice though is that this commit just fixes one out of very many cases where something like this or something similar could occur. E.g. just a few lines above it the same problem exists: http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2432 So if you change the test code from $undefinedObject->$definedMethod() to $definedObject->$undefinedMethod() or $undefinedObject->$undefinedMethod() it will still fail. A few lines higher FETCH_CLASS has the same problem ( http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2398). Going down a bit INIT_STATIC_METHOD_CALL has this too: http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2548. INIT_FCALL_BY_NAME also has this: http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2753 So you can trigger it with just $undefined() too. Again the same thing in THROW: http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2963 (throw $undefined;). I could continue this list, but I think you get the point ^^ This issue exists pretty much everywhere where a ZVAL_PTR fetch is followed by some error condition that can happen when the zval is NULL. And we have a *lot* of those. So I'm not sure that it really makes sense to fix one of those cases and leave 50 others intact. Either this should be fixed generically (e.g. on the side of GET_ZVAL_PTR, though that might incur a performance penalty) or should be just left alone. Thanks, Nikita
Re: [PHP-CVS] com php-src: Fixed bug #64135 (Exceptions from set_error_handler are not always propagated): NEWS Zend/tests/bug61767.phpt Zend/zend_vm_def.h Zend/zend_vm_execute.h
On Fri, Feb 8, 2013 at 3:36 AM, Nikita Popov wrote: > On Thu, Feb 7, 2013 at 4:44 PM, Xinchen Hui wrote: >> >> Commit:290509755ac4a3279b2b31b899aa9f2dd780f5f4 >> Author:Xinchen Hui Thu, 7 Feb 2013 23:44:46 >> +0800 >> Parents: 0547a36e95ec36025a30e93e971d26b6b1ecf0e9 >> Branches: PHP-5.5 >> >> Link: >> http://git.php.net/?p=php-src.git;a=commitdiff;h=290509755ac4a3279b2b31b899aa9f2dd780f5f4 >> >> Log: >> Fixed bug #64135 (Exceptions from set_error_handler are not always >> propagated) > > > Is there any particular reason why the exception check was added only the > that error branch? Can't it be done right after the GET_OP1_OBJ_ZVAL_PTR in > http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2442? This way it would > cover the other error conditions as well. Hey: could you please example one, which will trigger a error(warning, notice) out of that branch? thanks > > Thanks, > Nikita -- Laruence Xinchen Hui http://www.laruence.com/ -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] com php-src: Fixed bug #64135 (Exceptions from set_error_handler are not always propagated): NEWS Zend/tests/bug61767.phpt Zend/zend_vm_def.h Zend/zend_vm_execute.h
On Thu, Feb 7, 2013 at 4:44 PM, Xinchen Hui wrote: > Commit:290509755ac4a3279b2b31b899aa9f2dd780f5f4 > Author:Xinchen Hui Thu, 7 Feb 2013 > 23:44:46 +0800 > Parents: 0547a36e95ec36025a30e93e971d26b6b1ecf0e9 > Branches: PHP-5.5 > > Link: > http://git.php.net/?p=php-src.git;a=commitdiff;h=290509755ac4a3279b2b31b899aa9f2dd780f5f4 > > Log: > Fixed bug #64135 (Exceptions from set_error_handler are not always > propagated) > Is there any particular reason why the exception check was added only the that error branch? Can't it be done right after the GET_OP1_OBJ_ZVAL_PTR in http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#2442? This way it would cover the other error conditions as well. Thanks, Nikita
