Re: 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 <nikita.ppv@gmail.com> wrote: > On Fri, Feb 8, 2013 at 4:11 AM, Laruence <laruence@php.net> wrote: >> >> On Fri, Feb 8, 2013 at 3:36 AM, Nikita Popov <nikita.ppv@gmail.com> wrote: >> > On Thu, Feb 7, 2013 at 4:44 PM, Xinchen Hui <laruence@php.net> wrote: >> >> >> >> Commit: 290509755ac4a3279b2b31b899aa9f2dd780f5f4 >> >> Author: Xinchen Hui <laruence@php.net> 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/