Re: [PHP-CVS] cvs: php-src /build order_by_dep.awk /ext/date config.m4 config0.m4 php_date.c ZendEngine2 zend.c
Jani Taskinen wrote: Dmitry Stogov kirjoitti: Hi Jani, Unfortunately this patch doesn't fix the bug. I'm not sure if it fixes module dependency issues (let me know how to check it), but the original bug has the same two problems. 1) Possible infinity recursion in php_log_err() I did not even try to fix that problem, I wanted to fix the dependancy issues first. Now ext/date is the first to init (after Core) and last to shutdown. Preventing infinite recursion is for someone else to fix how they want. :) 2) Uninitialized DATEG(tzcache. It's initialized only during RINIT() so any error messages befor this may crash PHP. Eh..I did fix that until I found out it didn't help at all without THIS stuff being fixed first. :) To test it I just inserted a warning message into main.c around line 2130, right before module_initialized assignment. +zend_error(E_WARNING, test); module_initialized = 1; Yes, of course, ext/date needs the stuff fixed now that it's actually possible. But what about the part #1 of this..? Can I MFH to PHP_5_* ? In case you tested it enough, you can. Thanks. Dmitry. -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-CVS] cvs: php-src /build order_by_dep.awk
janiThu May 14 21:00:07 2009 UTC Modified files: /php-src/build order_by_dep.awk Log: - Fix harmless extra for() loop iteration http://cvs.php.net/viewvc.cgi/php-src/build/order_by_dep.awk?r1=1.2r2=1.3diff_format=u Index: php-src/build/order_by_dep.awk diff -u php-src/build/order_by_dep.awk:1.2 php-src/build/order_by_dep.awk:1.3 --- php-src/build/order_by_dep.awk:1.2 Wed May 13 00:45:57 2009 +++ php-src/build/order_by_dep.awk Thu May 14 21:00:07 2009 @@ -51,8 +51,6 @@ do_deps(depidx); } } - - #printf( phpext_%s_ptr,\n, module_name); printf(phpext_%s_ptr,@NEWLINE@, module_name); delete mods[mod_idx]; } @@ -79,7 +77,7 @@ out_count = 0; while (count(mods)) { - for (i = 0; i = mod_count; i++) { + for (i = 0; i mod_count - 1; i++) { if (i in mods) { do_deps(i); } -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-CVS] cvs: php-src /build order_by_dep.awk
janiThu May 14 21:23:44 2009 UTC Modified files: /php-src/build order_by_dep.awk Log: - Fix the harmless patch that caused harm :) http://cvs.php.net/viewvc.cgi/php-src/build/order_by_dep.awk?r1=1.3r2=1.4diff_format=u Index: php-src/build/order_by_dep.awk diff -u php-src/build/order_by_dep.awk:1.3 php-src/build/order_by_dep.awk:1.4 --- php-src/build/order_by_dep.awk:1.3 Thu May 14 21:00:07 2009 +++ php-src/build/order_by_dep.awk Thu May 14 21:23:44 2009 @@ -77,7 +77,7 @@ out_count = 0; while (count(mods)) { - for (i = 0; i mod_count - 1; i++) { + for (i = 0; i = mod_count - 1; i++) { if (i in mods) { do_deps(i); } -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] cvs: php-src /build order_by_dep.awk /ext/date config.m4 config0.m4 php_date.c ZendEngine2 zend.c
Hi Jani, Unfortunately this patch doesn't fix the bug. I'm not sure if it fixes module dependency issues (let me know how to check it), but the original bug has the same two problems. 1) Possible infinity recursion in php_log_err() 2) Uninitialized DATEG(tzcache. It's initialized only during RINIT() so any error messages befor this may crash PHP. To test it I just inserted a warning message into main.c around line 2130, right before module_initialized assignment. + zend_error(E_WARNING, test); module_initialized = 1; Then run something like this $ sapi/cli/php -n -ddate.timezone= -derror_log=/tmp/php.log -r 'echo ok\n;' /home/dmitry/php/php6/Zend/zend_hash.c(91) : Bailed out without a bailout address! It may bailout or crash. Thanks. Dmitry. Jani Taskinen wrote: janiWed May 13 00:45:57 2009 UTC Added files: /php-src/ext/date config0.m4 Removed files: /php-src/ext/date config.m4 Modified files: /ZendEngine2 zend.c /php-src/build order_by_dep.awk /php-src/ext/date php_date.c Log: - Fixed bug #48247 (PHP crashes on errors during startup) # # This was sum of many little things: # 1. Wrong assumption made in order_by_dep.awk on the ordering #(that script was done prior to adding runtime deps..?) # 2. request shutdown calls weren't done in reverse order like all other #shutdown funcs are called. # 3. config0.m4 rename is necessary for keeping things simple. # http://cvs.php.net/viewvc.cgi/ZendEngine2/zend.c?r1=1.432r2=1.433diff_format=u Index: ZendEngine2/zend.c diff -u ZendEngine2/zend.c:1.432 ZendEngine2/zend.c:1.433 --- ZendEngine2/zend.c:1.432Thu Mar 26 20:01:37 2009 +++ ZendEngine2/zend.c Wed May 13 00:45:57 2009 @@ -17,7 +17,7 @@ +--+ */ -/* $Id: zend.c,v 1.432 2009/03/26 20:01:37 felipe Exp $ */ +/* $Id: zend.c,v 1.433 2009/05/13 00:45:57 jani Exp $ */ #include zend.h #include zend_extensions.h @@ -1418,7 +1418,7 @@ EG(opline_ptr) = NULL; /* we're no longer executing anything */ zend_try { - zend_hash_apply(module_registry, (apply_func_t) module_registry_cleanup TSRMLS_CC); + zend_hash_reverse_apply(module_registry, (apply_func_t) module_registry_cleanup TSRMLS_CC); } zend_end_try(); } /* }}} */ http://cvs.php.net/viewvc.cgi/php-src/build/order_by_dep.awk?r1=1.1r2=1.2diff_format=u Index: php-src/build/order_by_dep.awk diff -u php-src/build/order_by_dep.awk:1.1 php-src/build/order_by_dep.awk:1.2 --- php-src/build/order_by_dep.awk:1.1 Sun Jul 18 12:03:51 2004 +++ php-src/build/order_by_dep.awk Wed May 13 00:45:57 2009 @@ -79,8 +79,7 @@ out_count = 0; while (count(mods)) { - # count down, since we need to assemble it in reverse order - for (i = mod_count-1; i = 0; --i) { + for (i = 0; i = mod_count; i++) { if (i in mods) { do_deps(i); } http://cvs.php.net/viewvc.cgi/php-src/ext/date/php_date.c?r1=1.230r2=1.231diff_format=u Index: php-src/ext/date/php_date.c diff -u php-src/ext/date/php_date.c:1.230 php-src/ext/date/php_date.c:1.231 --- php-src/ext/date/php_date.c:1.230 Tue May 5 12:34:03 2009 +++ php-src/ext/date/php_date.c Wed May 13 00:45:57 2009 @@ -16,7 +16,7 @@ +--+ */ -/* $Id: php_date.c,v 1.230 2009/05/05 12:34:03 iliaa Exp $ */ +/* $Id: php_date.c,v 1.231 2009/05/13 00:45:57 jani Exp $ */ #define _ISOC9X_SOURCE @@ -570,17 +570,11 @@ zval *date_interval_read_property(zval *object, zval *member, int type TSRMLS_DC); void date_interval_write_property(zval *object, zval *member, zval *value TSRMLS_DC); -/* This is need to ensure that session extension request shutdown occurs 1st, because it uses the date extension */ -static const zend_module_dep date_deps[] = { - ZEND_MOD_OPTIONAL(session) - {NULL, NULL, NULL} -}; - /* {{{ Module struct */ zend_module_entry date_module_entry = { STANDARD_MODULE_HEADER_EX, NULL, - date_deps, + NULL, date, /* extension name */ date_functions, /* function list */ PHP_MINIT(date),/* process startup */ http://cvs.php.net/viewvc.cgi/php-src/ext/date/config0.m4?view=markuprev=1.1 Index: php-src/ext/date/config0.m4 +++ php-src/ext/date/config0.m4 dnl $Id: config0.m4,v 1.1 2009/05/13 00:45:57 jani Exp $ dnl config.m4 for date extension sinclude(ext/date/lib/timelib.m4) sinclude(lib/timelib.m4) PHP_DATE_CFLAGS=-...@ext_builddir@/lib timelib_sources=lib/astro.c lib/dow.c lib/parse_date.c lib/parse_tz.c lib/timelib.c lib/tm2unixtime.c lib/unixtime2tm.c lib/parse_iso_intervals.c lib/interval.c
Re: [PHP-CVS] cvs: php-src /build order_by_dep.awk /ext/date config.m4 config0.m4 php_date.c ZendEngine2 zend.c
Hi! 2) Uninitialized DATEG(tzcache. It's initialized only during RINIT() so any error messages befor this may crash PHP. This isn't the first time we see extension being used before initialization. Maybe we need some infrastructure to prevent that. -- Stanislav Malyshev, Zend Software Architect s...@zend.com http://www.zend.com/ (408)253-8829 MSN: s...@zend.com -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] cvs: php-src /build order_by_dep.awk /ext/date config.m4 config0.m4 php_date.c ZendEngine2 zend.c
On Wed, 13 May 2009, Stanislav Malyshev wrote: 2) Uninitialized DATEG(tzcache. It's initialized only during RINIT() so any error messages befor this may crash PHP. This isn't the first time we see extension being used before initialization. Maybe we need some infrastructure to prevent that. In this case, I don't see why I can't init the tzcache in MINIT() though. But would that solve things? regards, Derick -- http://derickrethans.nl | http://ezcomponents.org | http://xdebug.org twitter: @derickr -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] cvs: php-src /build order_by_dep.awk /ext/date config.m4 config0.m4 php_date.c ZendEngine2 zend.c
Derick Rethans wrote: On Wed, 13 May 2009, Stanislav Malyshev wrote: 2) Uninitialized DATEG(tzcache. It's initialized only during RINIT() so any error messages befor this may crash PHP. This isn't the first time we see extension being used before initialization. Maybe we need some infrastructure to prevent that. In this case, I don't see why I can't init the tzcache in MINIT() though. But would that solve things? Not in general, but it'll fix error logging for all extensions initialized after ext/date. Thanks. Dmitry. regards, Derick -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] cvs: php-src /build order_by_dep.awk /ext/date config.m4 config0.m4 php_date.c ZendEngine2 zend.c
Dmitry Stogov kirjoitti: Hi Jani, Unfortunately this patch doesn't fix the bug. I'm not sure if it fixes module dependency issues (let me know how to check it), but the original bug has the same two problems. 1) Possible infinity recursion in php_log_err() I did not even try to fix that problem, I wanted to fix the dependancy issues first. Now ext/date is the first to init (after Core) and last to shutdown. Preventing infinite recursion is for someone else to fix how they want. :) 2) Uninitialized DATEG(tzcache. It's initialized only during RINIT() so any error messages befor this may crash PHP. Eh..I did fix that until I found out it didn't help at all without THIS stuff being fixed first. :) To test it I just inserted a warning message into main.c around line 2130, right before module_initialized assignment. +zend_error(E_WARNING, test); module_initialized = 1; Yes, of course, ext/date needs the stuff fixed now that it's actually possible. But what about the part #1 of this..? Can I MFH to PHP_5_* ? --Jani -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] cvs: php-src /build order_by_dep.awk /ext/date config.m4 config0.m4 php_date.c ZendEngine2 zend.c
Stanislav Malyshev kirjoitti: Hi! 2) Uninitialized DATEG(tzcache. It's initialized only during RINIT() so any error messages befor this may crash PHP. This isn't the first time we see extension being used before initialization. Maybe we need some infrastructure to prevent that. Yes, we have that, the loading order stuff + dependancies. There just was a bug in all of it which I fixed now.. ;) --Jani -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] cvs: php-src /build order_by_dep.awk /ext/date config.m4 config0.m4 php_date.c ZendEngine2 zend.c
Dmitry Stogov kirjoitti: Derick Rethans wrote: On Wed, 13 May 2009, Stanislav Malyshev wrote: 2) Uninitialized DATEG(tzcache. It's initialized only during RINIT() so any error messages befor this may crash PHP. This isn't the first time we see extension being used before initialization. Maybe we need some infrastructure to prevent that. In this case, I don't see why I can't init the tzcache in MINIT() though. But would that solve things? Not in general, but it'll fix error logging for all extensions initialized after ext/date. I had a patch first which added the init (also) in GINIT along with that RINIT one but that was useless without fixing the core issues first. And then I lost it. :D In case of ext/date: It should be moved to core. --Jani -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] cvs: php-src /build order_by_dep.awk /ext/date config.m4 config0.m4 php_date.c ZendEngine2 zend.c
On Wed, 2009-05-13 at 00:45 +, Jani Taskinen wrote: Log: - Fixed bug #48247 (PHP crashes on errors during startup) Looks good for 5.3 to me. One comment though: cat $ext_builddir/lib/timelib_config.h EOF #ifdef PHP_WIN32 # include config.w32.h #else # include php_config.h #endif EOF Any issue with this file being committed instead of being generated? - Just to reduce the complexity and number of things that might break. People using the timelib for other purpose should still be able to overwrite this file ... johannes -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] cvs: php-src /build order_by_dep.awk /ext/date config.m4 config0.m4 php_date.c ZendEngine2 zend.c
Johannes Schlüter kirjoitti: On Wed, 2009-05-13 at 00:45 +, Jani Taskinen wrote: Log: - Fixed bug #48247 (PHP crashes on errors during startup) Looks good for 5.3 to me. Committed. One comment though: cat $ext_builddir/lib/timelib_config.h EOF #ifdef PHP_WIN32 # include config.w32.h #else # include php_config.h #endif EOF Any issue with this file being committed instead of being generated? - Just to reduce the complexity and number of things that might break. People using the timelib for other purpose should still be able to overwrite this file ... I only renamed that file, I did not change anything in it. :) So this is up to Derick to fix/change. --Jani -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-CVS] cvs: php-src /build order_by_dep.awk /ext/date config.m4 config0.m4 php_date.c ZendEngine2 zend.c
janiWed May 13 00:45:57 2009 UTC Added files: /php-src/ext/date config0.m4 Removed files: /php-src/ext/date config.m4 Modified files: /ZendEngine2zend.c /php-src/build order_by_dep.awk /php-src/ext/date php_date.c Log: - Fixed bug #48247 (PHP crashes on errors during startup) # # This was sum of many little things: # 1. Wrong assumption made in order_by_dep.awk on the ordering #(that script was done prior to adding runtime deps..?) # 2. request shutdown calls weren't done in reverse order like all other #shutdown funcs are called. # 3. config0.m4 rename is necessary for keeping things simple. # http://cvs.php.net/viewvc.cgi/ZendEngine2/zend.c?r1=1.432r2=1.433diff_format=u Index: ZendEngine2/zend.c diff -u ZendEngine2/zend.c:1.432 ZendEngine2/zend.c:1.433 --- ZendEngine2/zend.c:1.432Thu Mar 26 20:01:37 2009 +++ ZendEngine2/zend.c Wed May 13 00:45:57 2009 @@ -17,7 +17,7 @@ +--+ */ -/* $Id: zend.c,v 1.432 2009/03/26 20:01:37 felipe Exp $ */ +/* $Id: zend.c,v 1.433 2009/05/13 00:45:57 jani Exp $ */ #include zend.h #include zend_extensions.h @@ -1418,7 +1418,7 @@ EG(opline_ptr) = NULL; /* we're no longer executing anything */ zend_try { - zend_hash_apply(module_registry, (apply_func_t) module_registry_cleanup TSRMLS_CC); + zend_hash_reverse_apply(module_registry, (apply_func_t) module_registry_cleanup TSRMLS_CC); } zend_end_try(); } /* }}} */ http://cvs.php.net/viewvc.cgi/php-src/build/order_by_dep.awk?r1=1.1r2=1.2diff_format=u Index: php-src/build/order_by_dep.awk diff -u php-src/build/order_by_dep.awk:1.1 php-src/build/order_by_dep.awk:1.2 --- php-src/build/order_by_dep.awk:1.1 Sun Jul 18 12:03:51 2004 +++ php-src/build/order_by_dep.awk Wed May 13 00:45:57 2009 @@ -79,8 +79,7 @@ out_count = 0; while (count(mods)) { - # count down, since we need to assemble it in reverse order - for (i = mod_count-1; i = 0; --i) { + for (i = 0; i = mod_count; i++) { if (i in mods) { do_deps(i); } http://cvs.php.net/viewvc.cgi/php-src/ext/date/php_date.c?r1=1.230r2=1.231diff_format=u Index: php-src/ext/date/php_date.c diff -u php-src/ext/date/php_date.c:1.230 php-src/ext/date/php_date.c:1.231 --- php-src/ext/date/php_date.c:1.230 Tue May 5 12:34:03 2009 +++ php-src/ext/date/php_date.c Wed May 13 00:45:57 2009 @@ -16,7 +16,7 @@ +--+ */ -/* $Id: php_date.c,v 1.230 2009/05/05 12:34:03 iliaa Exp $ */ +/* $Id: php_date.c,v 1.231 2009/05/13 00:45:57 jani Exp $ */ #define _ISOC9X_SOURCE @@ -570,17 +570,11 @@ zval *date_interval_read_property(zval *object, zval *member, int type TSRMLS_DC); void date_interval_write_property(zval *object, zval *member, zval *value TSRMLS_DC); -/* This is need to ensure that session extension request shutdown occurs 1st, because it uses the date extension */ -static const zend_module_dep date_deps[] = { - ZEND_MOD_OPTIONAL(session) - {NULL, NULL, NULL} -}; - /* {{{ Module struct */ zend_module_entry date_module_entry = { STANDARD_MODULE_HEADER_EX, NULL, - date_deps, + NULL, date, /* extension name */ date_functions, /* function list */ PHP_MINIT(date),/* process startup */ http://cvs.php.net/viewvc.cgi/php-src/ext/date/config0.m4?view=markuprev=1.1 Index: php-src/ext/date/config0.m4 +++ php-src/ext/date/config0.m4 dnl $Id: config0.m4,v 1.1 2009/05/13 00:45:57 jani Exp $ dnl config.m4 for date extension sinclude(ext/date/lib/timelib.m4) sinclude(lib/timelib.m4) PHP_DATE_CFLAGS=-...@ext_builddir@/lib timelib_sources=lib/astro.c lib/dow.c lib/parse_date.c lib/parse_tz.c lib/timelib.c lib/tm2unixtime.c lib/unixtime2tm.c lib/parse_iso_intervals.c lib/interval.c PHP_NEW_EXTENSION(date, php_date.c $timelib_sources, no,, $PHP_DATE_CFLAGS) PHP_ADD_BUILD_DIR([$ext_builddir/lib], 1) PHP_ADD_INCLUDE([$ext_builddir/lib]) PHP_ADD_INCLUDE([$ext_srcdir/lib]) PHP_INSTALL_HEADERS([ext/date], [php_date.h lib/timelib.h lib/timelib_structs.h lib/timelib_config.h]) cat $ext_builddir/lib/timelib_config.h EOF #ifdef PHP_WIN32 # include config.w32.h #else # include php_config.h #endif EOF -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php