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 nikita@gmail.com wrote: On Fri, Feb 8, 2013 at 4:11 AM, Laruence larue...@php.net wrote: On Fri, Feb 8, 2013 at 3:36 AM, Nikita Popov nikita@gmail.com wrote: On Thu, Feb 7, 2013 at 4:44 PM, Xinchen Hui larue...@php.net wrote: Commit:290509755ac4a3279b2b31b899aa9f2dd780f5f4 Author:Xinchen Hui larue...@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/ -- 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
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 larue...@php.net wrote: On Fri, Feb 8, 2013 at 8:36 PM, Nikita Popov nikita@gmail.com wrote: On Fri, Feb 8, 2013 at 4:11 AM, Laruence larue...@php.net wrote: On Fri, Feb 8, 2013 at 3:36 AM, Nikita Popov nikita@gmail.com wrote: On Thu, Feb 7, 2013 at 4:44 PM, Xinchen Hui larue...@php.net wrote: Commit:290509755ac4a3279b2b31b899aa9f2dd780f5f4 Author:Xinchen Hui larue...@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/ -- 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 larue...@php.net wrote: On Fri, Feb 8, 2013 at 3:36 AM, Nikita Popov nikita@gmail.com wrote: On Thu, Feb 7, 2013 at 4:44 PM, Xinchen Hui larue...@php.net wrote: Commit:290509755ac4a3279b2b31b899aa9f2dd780f5f4 Author:Xinchen Hui larue...@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;). 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
[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
Commit:290509755ac4a3279b2b31b899aa9f2dd780f5f4
Author:Xinchen Hui larue...@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)
Bugs:
https://bugs.php.net/64135
Changed paths:
M NEWS
M Zend/tests/bug61767.phpt
M Zend/zend_vm_def.h
M Zend/zend_vm_execute.h
Diff:
diff --git a/NEWS b/NEWS
index c343f43..3bb6ace 100644
--- a/NEWS
+++ b/NEWS
@@ -3,11 +3,13 @@ PHP
NEWS
?? ??? 201?, PHP 5.5.0 Beta 1
- Core:
+ . Fixed bug #64135 (Exceptions from set_error_handler are not always
+propagated). (Laruence)
+ . Fixed bug #63830 (Segfault on undefined function call in nested generator).
+(Nikita Popov)
. Fixed bug #60833 (self, parent, static behave inconsistently
case-sensitive). (Stas, mario at include-once dot org)
. Implemented FR #60524 (specify temp dir by php.ini). (ALeX Kazik).
- . Fixed bug #63830 (Segfault on undefined function call in nested generator).
-(Nikita Popov)
- CLI server:
. Fixed bug #64128 (buit-in web server is broken on ppc64). (Remi)
diff --git a/Zend/tests/bug61767.phpt b/Zend/tests/bug61767.phpt
index 5270872..9bd9c90 100644
--- a/Zend/tests/bug61767.phpt
+++ b/Zend/tests/bug61767.phpt
@@ -17,18 +17,16 @@ $undefined-foo();
--EXPECTF--
Error handler called (Undefined variable: undefined)
-Warning: Uncaught exception 'ErrorException' with message 'Undefined variable:
undefined' in %sbug61767.php:13
+Fatal error: Uncaught exception 'ErrorException' with message 'Undefined
variable: undefined' in %sbug61767.php:%d
Stack trace:
-#0 %sbug61767.php(13): {closure}(8, 'Undefined varia...', '%s', 13, Array)
+#0 %sbug61767.php(%d): {closure}(%s, 'Undefined varia...', '%s', %d, Array)
#1 {main}
- thrown in %sbug61767.php on line 13
-
-Fatal error: Call to a member function foo() on a non-object in %sbug61767.php
on line 13
+ thrown in %sbug61767.php on line %d
Shutting down
Array
(
[type] = 1
-[message] = Call to a member function foo() on a non-object
+[message] = %a
[file] = %sbug61767.php
-[line] = 13
+[line] = %d
)
diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h
index 4dd544e..cbd3810 100644
--- a/Zend/zend_vm_def.h
+++ b/Zend/zend_vm_def.h
@@ -2462,6 +2462,10 @@ ZEND_VM_HANDLER(112, ZEND_INIT_METHOD_CALL,
TMP|VAR|UNUSED|CV, CONST|TMP|VAR|CV)
}
}
} else {
+ if (UNEXPECTED(EG(exception) != NULL)) {
+ FREE_OP2();
+ HANDLE_EXCEPTION();
+ }
zend_error_noreturn(E_ERROR, Call to a member function %s() on
a non-object, function_name_strval);
}
diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h
index 25ac1ea..7b874de 100644
--- a/Zend/zend_vm_execute.h
+++ b/Zend/zend_vm_execute.h
@@ -8947,6 +8947,10 @@ static int ZEND_FASTCALL
ZEND_INIT_METHOD_CALL_SPEC_TMP_CONST_HANDLER(ZEND_OPCO
}
}
} else {
+ if (UNEXPECTED(EG(exception) != NULL)) {
+
+ HANDLE_EXCEPTION();
+ }
zend_error_noreturn(E_ERROR, Call to a member function %s() on
a non-object, function_name_strval);
}
@@ -9801,6 +9805,10 @@ static int ZEND_FASTCALL
ZEND_INIT_METHOD_CALL_SPEC_TMP_TMP_HANDLER(ZEND_OPCODE
}
}
} else {
+ if (UNEXPECTED(EG(exception) != NULL)) {
+ zval_dtor(free_op2.var);
+ HANDLE_EXCEPTION();
+ }
zend_error_noreturn(E_ERROR, Call to a member function %s() on
a non-object, function_name_strval);
}
@@ -10660,6 +10668,10 @@ static int ZEND_FASTCALL
ZEND_INIT_METHOD_CALL_SPEC_TMP_VAR_HANDLER(ZEND_OPCODE
}
}
} else {
+ if (UNEXPECTED(EG(exception) != NULL)) {
+ if (free_op2.var) {zval_ptr_dtor(free_op2.var);};
+ HANDLE_EXCEPTION();
+ }
zend_error_noreturn(E_ERROR, Call to a member function %s() on
a non-object, function_name_strval);
}
@@ -12094,6 +12106,10 @@ static int ZEND_FASTCALL
ZEND_INIT_METHOD_CALL_SPEC_TMP_CV_HANDLER(ZEND_OPCODE_
}
}
} else {
+ if (UNEXPECTED(EG(exception) != NULL)) {
+
+ HANDLE_EXCEPTION();
+ }
zend_error_noreturn(E_ERROR, Call to a member function %s() on
a non-object, function_name_strval);
}
@@ -15319,6 +15335,10 @@ static int ZEND_FASTCALL
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 larue...@php.net wrote: Commit:290509755ac4a3279b2b31b899aa9f2dd780f5f4 Author:Xinchen Hui larue...@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. 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 nikita@gmail.com wrote: On Thu, Feb 7, 2013 at 4:44 PM, Xinchen Hui larue...@php.net wrote: Commit:290509755ac4a3279b2b31b899aa9f2dd780f5f4 Author:Xinchen Hui larue...@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? 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
