From 1f3c39531ebdab80c1acd2a3e81890f0e8e7bfd1 Mon Sep 17 00:00:00 2001 From: Alex Dowad Date: Sat, 13 Jun 2020 10:48:10 +0200 Subject: [PATCH 1/2] Remove redundant global variable: SIGG(reset) This global variable controls whether the Zend deferred signal handlers are installed at the beginning of each request or not. Interestingly, it is set to 1 on initialization, and there is only *one* place where the value is modified. This is in OPCache, where the global is temporarily set to 0 while preloading. Why is this? Discussion on GitHub revealed that the purpose was so that what happens during preloading would not affect subsequent requests... but the fact is, as long as Zend signal handling is enabled, the Zend signal handlers will always be installed on each subsequent request, so installing them during preloading will not affect anything. --- Zend/zend_signal.c | 7 ++----- Zend/zend_signal.h | 1 - ext/opcache/ZendAccelerator.c | 15 --------------- 3 files changed, 2 insertions(+), 21 deletions(-) diff --git a/Zend/zend_signal.c b/Zend/zend_signal.c index 96870f8d5cc49..421b027cfcd94 100644 --- a/Zend/zend_signal.c +++ b/Zend/zend_signal.c @@ -320,10 +320,8 @@ void zend_signal_activate(void) memcpy(&SIGG(handlers), &global_orig_handlers, sizeof(global_orig_handlers)); - if (SIGG(reset)) { - for (x = 0; x < sizeof(zend_sigs) / sizeof(*zend_sigs); x++) { - zend_signal_register(zend_sigs[x], zend_signal_handler_defer); - } + for (x = 0; x < sizeof(zend_sigs) / sizeof(*zend_sigs); x++) { + zend_signal_register(zend_sigs[x], zend_signal_handler_defer); } SIGG(active) = 1; @@ -376,7 +374,6 @@ static void zend_signal_globals_ctor(zend_signal_globals_t *zend_signal_globals) size_t x; memset(zend_signal_globals, 0, sizeof(*zend_signal_globals)); - zend_signal_globals->reset = 1; for (x = 0; x < sizeof(zend_signal_globals->pstorage) / sizeof(*zend_signal_globals->pstorage); ++x) { zend_signal_queue_t *queue = &zend_signal_globals->pstorage[x]; diff --git a/Zend/zend_signal.h b/Zend/zend_signal.h index 0bb191db73245..1a2b3960d37d4 100644 --- a/Zend/zend_signal.h +++ b/Zend/zend_signal.h @@ -57,7 +57,6 @@ typedef struct _zend_signal_globals_t { int running; /* in signal handler execution */ int active; /* internal signal handling is enabled */ zend_bool check; /* check for replaced handlers on shutdown */ - zend_bool reset; /* reset signal handlers on each request */ zend_signal_entry_t handlers[NSIG]; zend_signal_queue_t pstorage[ZEND_SIGNAL_QUEUE_SIZE], *phead, *ptail, *pavail; /* pending queue */ } zend_signal_globals_t; diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index aa81410c82f1e..5fbb25ffac1d3 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -4802,9 +4802,6 @@ static int accel_finish_startup(void) size_t (*orig_ub_write)(const char *str, size_t str_length) = sapi_module.ub_write; void (*orig_flush)(void *server_context) = sapi_module.flush; uint32_t orig_compiler_options = CG(compiler_options); -#ifdef ZEND_SIGNALS - zend_bool old_reset_signals = SIGG(reset); -#endif if (UNEXPECTED(file_cache_only)) { zend_accel_error(ACCEL_LOG_WARNING, "Preloading doesn't work in \"file_cache_only\" mode"); @@ -4907,10 +4904,6 @@ static int accel_finish_startup(void) zend_interned_strings_switch_storage(1); -#ifdef ZEND_SIGNALS - SIGG(reset) = 0; -#endif - orig_error_reporting = EG(error_reporting); EG(error_reporting) = 0; @@ -4943,20 +4936,12 @@ static int accel_finish_startup(void) orig_report_memleaks = PG(report_memleaks); PG(report_memleaks) = 0; -#ifdef ZEND_SIGNALS - /* We may not have registered signal handlers due to SIGG(reset)=0, so - * also disable the check that they are registered. */ - SIGG(check) = 0; -#endif php_request_shutdown(NULL); /* calls zend_shared_alloc_unlock(); */ PG(report_memleaks) = orig_report_memleaks; } else { zend_shared_alloc_unlock(); ret = FAILURE; } -#ifdef ZEND_SIGNALS - SIGG(reset) = old_reset_signals; -#endif CG(compiler_options) = orig_compiler_options; From 1fc5f3dda116fb5a2b9363aedeff31190c01e155 Mon Sep 17 00:00:00 2001 From: Alex Dowad Date: Sat, 13 Jun 2020 10:54:18 +0200 Subject: [PATCH 2/2] Always use Zend signal handling Being able to choose whether Zend signal handling should be used or not does not benefit users (or packagers) of PHP in any way. The use of Zend SH is a purely internal implementation detail, so there is no need for the build config parameters --{disable,enable}-zend-signals. By just choosing one way to do things (in this case, always using Zend signal handling), we can simplify the codebase a bit. Of course, some platforms do not have the concept of signals and signal handlers at all. That is why previously, Zend signal handling was automatically disabled if the platform did not have `sigaction`. In harmony with that, we now guard the implementation of Zend signal handling with `#ifdef HAVE_SIGACTION`. --- Zend/Zend.m4 | 17 ----------------- Zend/zend.c | 2 +- Zend/zend_execute_API.c | 17 ----------------- Zend/zend_signal.c | 8 ++++++-- Zend/zend_signal.h | 12 +++++++++--- configure.ac | 1 + ext/standard/info.c | 6 ------ .../tests/general_functions/phpinfo.phpt | 1 - main/main.c | 9 +-------- sapi/apache2handler/sapi_apache2.c | 4 ---- sapi/fuzzer/fuzzer-sapi.c | 4 +--- sapi/litespeed/lsapi_main.c | 4 ---- sapi/phpdbg/phpdbg.h | 2 +- 13 files changed, 20 insertions(+), 67 deletions(-) diff --git a/Zend/Zend.m4 b/Zend/Zend.m4 index 7c715e853bf5b..de77b22208d9c 100644 --- a/Zend/Zend.m4 +++ b/Zend/Zend.m4 @@ -303,23 +303,6 @@ AC_MSG_RESULT(done) AC_CHECK_FUNCS(mremap) -AC_ARG_ENABLE([zend-signals], - [AS_HELP_STRING([--disable-zend-signals], - [whether to enable zend signal handling])], - [ZEND_SIGNALS=$enableval], - [ZEND_SIGNALS=yes]) - -AC_CHECK_FUNCS([sigaction], [], [ - ZEND_SIGNALS=no -]) -if test "$ZEND_SIGNALS" = "yes"; then - AC_DEFINE(ZEND_SIGNALS, 1, [Use zend signal handling]) - CFLAGS="$CFLAGS -DZEND_SIGNALS" -fi - -AC_MSG_CHECKING(whether to enable zend signal handling) -AC_MSG_RESULT($ZEND_SIGNALS) - ]) AC_MSG_CHECKING(whether /dev/urandom exists) diff --git a/Zend/zend.c b/Zend/zend.c index b72184e726847..9eceaa3849c3c 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -165,7 +165,7 @@ ZEND_INI_BEGIN() STD_ZEND_INI_BOOLEAN("zend.multibyte", "0", ZEND_INI_PERDIR, OnUpdateBool, multibyte, zend_compiler_globals, compiler_globals) ZEND_INI_ENTRY("zend.script_encoding", NULL, ZEND_INI_ALL, OnUpdateScriptEncoding) STD_ZEND_INI_BOOLEAN("zend.detect_unicode", "1", ZEND_INI_ALL, OnUpdateBool, detect_unicode, zend_compiler_globals, compiler_globals) -#ifdef ZEND_SIGNALS +#ifdef HAVE_SIGACTION STD_ZEND_INI_BOOLEAN("zend.signal_check", "0", ZEND_INI_SYSTEM, OnUpdateBool, check, zend_signal_globals_t, zend_signal_globals) #endif STD_ZEND_INI_BOOLEAN("zend.exception_ignore_args", "0", ZEND_INI_ALL, OnUpdateBool, exception_ignore_args, zend_executor_globals, executor_globals) diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c index f1babc3d021ef..8005095eb2357 100644 --- a/Zend/zend_execute_API.c +++ b/Zend/zend_execute_API.c @@ -1302,24 +1302,7 @@ static void zend_set_timeout_ex(zend_long seconds, int reset_signals) /* {{{ */ # endif if (reset_signals) { -# ifdef ZEND_SIGNALS zend_signal(signo, zend_timeout_handler); -# else - sigset_t sigset; -# ifdef HAVE_SIGACTION - struct sigaction act; - - act.sa_handler = zend_timeout_handler; - sigemptyset(&act.sa_mask); - act.sa_flags = SA_RESETHAND | SA_NODEFER; - sigaction(signo, &act, NULL); -# else - signal(signo, zend_timeout_handler); -# endif /* HAVE_SIGACTION */ - sigemptyset(&sigset); - sigaddset(&sigset, signo); - sigprocmask(SIG_UNBLOCK, &sigset, NULL); -# endif /* ZEND_SIGNALS */ } } #endif /* HAVE_SETITIMER */ diff --git a/Zend/zend_signal.c b/Zend/zend_signal.c index 421b027cfcd94..ae2ea31589d2c 100644 --- a/Zend/zend_signal.c +++ b/Zend/zend_signal.c @@ -36,7 +36,7 @@ #include #endif -#ifdef ZEND_SIGNALS +#ifdef HAVE_SIGACTION #include "zend_signal.h" @@ -441,5 +441,9 @@ ZEND_API void zend_signal_startup(void) } /* }}} */ +ZEND_API void zend_signal_disable_debug(void) +{ + SIGG(check) = 0; +} -#endif /* ZEND_SIGNALS */ +#endif /* HAVE_SIGACTION */ diff --git a/Zend/zend_signal.h b/Zend/zend_signal.h index 1a2b3960d37d4..271871fc1ae28 100644 --- a/Zend/zend_signal.h +++ b/Zend/zend_signal.h @@ -21,7 +21,7 @@ #ifndef ZEND_SIGNAL_H #define ZEND_SIGNAL_H -#ifdef ZEND_SIGNALS +#ifdef HAVE_SIGACTION #include @@ -61,6 +61,8 @@ typedef struct _zend_signal_globals_t { zend_signal_queue_t pstorage[ZEND_SIGNAL_QUEUE_SIZE], *phead, *ptail, *pavail; /* pending queue */ } zend_signal_globals_t; +# define SIZEOF_ZEND_SIGNAL_GLOBALS sizeof(zend_signal_globals_t) + # ifdef ZTS # define SIGG(v) ZEND_TSRMG_FAST(zend_signal_globals_offset, zend_signal_globals_t *, v) BEGIN_EXTERN_C() @@ -89,11 +91,14 @@ BEGIN_EXTERN_C() ZEND_API void zend_signal_startup(void); END_EXTERN_C() void zend_signal_init(void); +ZEND_API void zend_signal_disable_debug(void); ZEND_API int zend_signal(int signo, void (*handler)(int)); ZEND_API int zend_sigaction(int signo, const struct sigaction *act, struct sigaction *oldact); -#else /* ZEND_SIGNALS */ +#else /* HAVE_SIGACTION */ + +# define SIZEOF_ZEND_SIGNAL_GLOBALS 0 # define ZEND_SIGNAL_BLOCK_INTERRUPTIONS() # define ZEND_SIGNAL_UNBLOCK_INTERRUPTIONS() @@ -102,10 +107,11 @@ ZEND_API int zend_sigaction(int signo, const struct sigaction *act, struct sigac # define zend_signal_deactivate() # define zend_signal_startup() # define zend_signal_init() +# define zend_signal_disable_debug() # define zend_signal(signo, handler) signal(signo, handler) # define zend_sigaction(signo, act, oldact) sigaction(signo, act, oldact) -#endif /* ZEND_SIGNALS */ +#endif /* HAVE_SIGACTION */ #endif /* ZEND_SIGNAL_H */ diff --git a/configure.ac b/configure.ac index 4f291b45d6282..274007f770890 100644 --- a/configure.ac +++ b/configure.ac @@ -577,6 +577,7 @@ scandir \ setitimer \ setenv \ shutdown \ +sigaction \ sigprocmask \ statfs \ statvfs \ diff --git a/ext/standard/info.c b/ext/standard/info.c index 262e95ae2731f..b3c88c2309322 100644 --- a/ext/standard/info.c +++ b/ext/standard/info.c @@ -848,12 +848,6 @@ PHPAPI ZEND_COLD void php_print_info(int flag) php_info_print_table_row(2, "Thread Safety", "disabled" ); #endif -#ifdef ZEND_SIGNALS - php_info_print_table_row(2, "Zend Signal Handling", "enabled" ); -#else - php_info_print_table_row(2, "Zend Signal Handling", "disabled" ); -#endif - php_info_print_table_row(2, "Zend Memory Manager", is_zend_mm() ? "enabled" : "disabled" ); { diff --git a/ext/standard/tests/general_functions/phpinfo.phpt b/ext/standard/tests/general_functions/phpinfo.phpt index 5f4d99ffbaf2e..56c76090376aa 100644 --- a/ext/standard/tests/general_functions/phpinfo.phpt +++ b/ext/standard/tests/general_functions/phpinfo.phpt @@ -31,7 +31,6 @@ Zend Extension Build => API%s PHP Extension Build => API%s Debug Build => %s Thread Safety => %s%A -Zend Signal Handling => %s Zend Memory Manager => %s Zend Multibyte Support => %s IPv6 Support => %s diff --git a/main/main.c b/main/main.c index ae6b539ba2588..624183ae0970f 100644 --- a/main/main.c +++ b/main/main.c @@ -1751,10 +1751,7 @@ int php_request_startup(void) zend_activate(); sapi_activate(); - -#ifdef ZEND_SIGNALS zend_signal_activate(); -#endif if (PG(max_input_time) == -1) { zend_set_timeout(EG(timeout_seconds), 1); @@ -1903,9 +1900,7 @@ void php_request_shutdown(void *dummy) } zend_end_try(); /* 16. Deactivate Zend signals */ -#ifdef ZEND_SIGNALS zend_signal_deactivate(); -#endif #ifdef PHP_WIN32 if (PG(com_initialized)) { @@ -2729,9 +2724,7 @@ PHPAPI void php_reserve_tsrm_memory(void) TSRM_ALIGNED_SIZE(sizeof(zend_php_scanner_globals)) + TSRM_ALIGNED_SIZE(sizeof(zend_ini_scanner_globals)) + TSRM_ALIGNED_SIZE(sizeof(virtual_cwd_globals)) + -#ifdef ZEND_SIGNALS - TSRM_ALIGNED_SIZE(sizeof(zend_signal_globals_t)) + -#endif + TSRM_ALIGNED_SIZE(SIZEOF_ZEND_SIGNAL_GLOBALS) + TSRM_ALIGNED_SIZE(zend_mm_globals_size()) + TSRM_ALIGNED_SIZE(zend_gc_globals_size()) + TSRM_ALIGNED_SIZE(sizeof(php_core_globals)) + diff --git a/sapi/apache2handler/sapi_apache2.c b/sapi/apache2handler/sapi_apache2.c index 126d09b1c6a50..779638c6072a3 100644 --- a/sapi/apache2handler/sapi_apache2.c +++ b/sapi/apache2handler/sapi_apache2.c @@ -739,20 +739,16 @@ static void php_apache_child_init(apr_pool_t *pchild, server_rec *s) apr_pool_cleanup_register(pchild, NULL, php_apache_child_shutdown, apr_pool_cleanup_null); } -#ifdef ZEND_SIGNALS static void php_apache_signal_init(apr_pool_t *pchild, server_rec *s) { zend_signal_init(); } -#endif void php_ap2_register_hook(apr_pool_t *p) { ap_hook_pre_config(php_pre_config, NULL, NULL, APR_HOOK_MIDDLE); ap_hook_post_config(php_apache_server_startup, NULL, NULL, APR_HOOK_MIDDLE); ap_hook_handler(php_handler, NULL, NULL, APR_HOOK_MIDDLE); -#ifdef ZEND_SIGNALS ap_hook_child_init(php_apache_signal_init, NULL, NULL, APR_HOOK_MIDDLE); -#endif ap_hook_child_init(php_apache_child_init, NULL, NULL, APR_HOOK_MIDDLE); } diff --git a/sapi/fuzzer/fuzzer-sapi.c b/sapi/fuzzer/fuzzer-sapi.c index ca474af1ee80f..2f7a838d2958d 100644 --- a/sapi/fuzzer/fuzzer-sapi.c +++ b/sapi/fuzzer/fuzzer-sapi.c @@ -147,11 +147,9 @@ int fuzzer_request_startup() return FAILURE; } -#ifdef ZEND_SIGNALS /* Some signal handlers will be overridden, * don't complain about them during shutdown. */ - SIGG(check) = 0; -#endif + zend_signal_disable_debug(); return SUCCESS; } diff --git a/sapi/litespeed/lsapi_main.c b/sapi/litespeed/lsapi_main.c index 6fec0c2b54d2e..765cbbcd7d956 100644 --- a/sapi/litespeed/lsapi_main.c +++ b/sapi/litespeed/lsapi_main.c @@ -1483,11 +1483,7 @@ int main( int argc, char * argv[] ) php_tsrm_startup(); #endif -#if PHP_MAJOR_VERSION >= 7 -#if defined(ZEND_SIGNALS) || PHP_MINOR_VERSION > 0 zend_signal_startup(); -#endif -#endif if (argc > 1 ) { if ( parse_opt( argc, argv, &climode, diff --git a/sapi/phpdbg/phpdbg.h b/sapi/phpdbg/phpdbg.h index 5405155833cbe..e4ae2ebd455d7 100644 --- a/sapi/phpdbg/phpdbg.h +++ b/sapi/phpdbg/phpdbg.h @@ -44,7 +44,7 @@ #include "zend_ini_scanner.h" #include "zend_stream.h" #include "zend_signal.h" -#if !defined(_WIN32) && !defined(ZEND_SIGNALS) +#if !defined(_WIN32) # include #elif defined(PHP_WIN32) # include "win32/signal.h"