From 20cc69528677b3e5191139d1cb587531f4893b55 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 26 Apr 2022 00:13:31 +0200 Subject: [PATCH] gh-64783: Fix signal.NSIG value on FreeBSD (#91929) Fix signal.NSIG value on FreeBSD to accept signal numbers greater than 32, like signal.SIGRTMIN and signal.SIGRTMAX. * Add Py_NSIG constant. * Add pycore_signal.h internal header file. * _Py_Sigset_Converter() now includes the range of valid signals in the error message. --- Doc/library/signal.rst | 1 + Include/internal/pycore_pylifecycle.h | 16 --------- Include/internal/pycore_signal.h | 35 +++++++++++++++++++ Lib/test/test_signal.py | 10 ++++++ Makefile.pre.in | 1 + ...2-04-25-18-30-20.gh-issue-64783.HFtERN.rst | 3 ++ Modules/faulthandler.c | 22 +++--------- Modules/posixmodule.c | 6 ++-- Modules/signalmodule.c | 34 +++++++++--------- PCbuild/pythoncore.vcxproj | 1 + PCbuild/pythoncore.vcxproj.filters | 3 ++ 11 files changed, 81 insertions(+), 51 deletions(-) create mode 100644 Include/internal/pycore_signal.h create mode 100644 Misc/NEWS.d/next/Library/2022-04-25-18-30-20.gh-issue-64783.HFtERN.rst diff --git a/Doc/library/signal.rst b/Doc/library/signal.rst index fdc9846f664..678411d4f17 100644 --- a/Doc/library/signal.rst +++ b/Doc/library/signal.rst @@ -266,6 +266,7 @@ The variables defined in the :mod:`signal` module are: .. data:: NSIG One more than the number of the highest signal number. + Use :func:`valid_signals` to get valid signal numbers. .. data:: ITIMER_REAL diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h index 295505f1f37..b4718b8ade2 100644 --- a/Include/internal/pycore_pylifecycle.h +++ b/Include/internal/pycore_pylifecycle.h @@ -8,24 +8,8 @@ extern "C" { # error "this header requires Py_BUILD_CORE define" #endif -#ifdef HAVE_SIGNAL_H -#include -#endif - #include "pycore_runtime.h" // _PyRuntimeState -#ifndef NSIG -# if defined(_NSIG) -# define NSIG _NSIG /* For BSD/SysV */ -# elif defined(_SIGMAX) -# define NSIG (_SIGMAX + 1) /* For QNX */ -# elif defined(SIGMAX) -# define NSIG (SIGMAX + 1) /* For djgpp */ -# else -# define NSIG 64 /* Use a reasonable default value */ -# endif -#endif - /* Forward declarations */ struct _PyArgv; struct pyruntimestate; diff --git a/Include/internal/pycore_signal.h b/Include/internal/pycore_signal.h new file mode 100644 index 00000000000..b921dd170e9 --- /dev/null +++ b/Include/internal/pycore_signal.h @@ -0,0 +1,35 @@ +// Define Py_NSIG constant for signal handling. + +#ifndef Py_INTERNAL_SIGNAL_H +#define Py_INTERNAL_SIGNAL_H +#ifdef __cplusplus +extern "C" { +#endif + +#ifndef Py_BUILD_CORE +# error "this header requires Py_BUILD_CORE define" +#endif + +#include // NSIG + +#ifdef _SIG_MAXSIG + // gh-91145: On FreeBSD, defines NSIG as 32: it doesn't include + // realtime signals: [SIGRTMIN,SIGRTMAX]. Use _SIG_MAXSIG instead. For + // example on x86-64 FreeBSD 13, SIGRTMAX is 126 and _SIG_MAXSIG is 128. +# define Py_NSIG _SIG_MAXSIG +#elif defined(NSIG) +# define Py_NSIG NSIG +#elif defined(_NSIG) +# define Py_NSIG _NSIG // BSD/SysV +#elif defined(_SIGMAX) +# define Py_NSIG (_SIGMAX + 1) // QNX +#elif defined(SIGMAX) +# define Py_NSIG (SIGMAX + 1) // djgpp +#else +# define Py_NSIG 64 // Use a reasonable default value +#endif + +#ifdef __cplusplus +} +#endif +#endif // !Py_INTERNAL_SIGNAL_H diff --git a/Lib/test/test_signal.py b/Lib/test/test_signal.py index ea13c59ec71..d38992db7b8 100644 --- a/Lib/test/test_signal.py +++ b/Lib/test/test_signal.py @@ -116,6 +116,16 @@ class PosixTests(unittest.TestCase): self.assertNotIn(signal.NSIG, s) self.assertLess(len(s), signal.NSIG) + # gh-91145: Make sure that all SIGxxx constants exposed by the Python + # signal module have a number in the [0; signal.NSIG-1] range. + for name in dir(signal): + if not name.startswith("SIG"): + continue + with self.subTest(name=name): + signum = getattr(signal, name) + self.assertGreaterEqual(signum, 0) + self.assertLess(signum, signal.NSIG) + @unittest.skipUnless(sys.executable, "sys.executable required.") @support.requires_subprocess() def test_keyboard_interrupt_exit_code(self): diff --git a/Makefile.pre.in b/Makefile.pre.in index d9f821dd14e..3952f5b542b 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -1622,6 +1622,7 @@ PYTHON_HEADERS= \ $(srcdir)/Include/internal/pycore_pystate.h \ $(srcdir)/Include/internal/pycore_runtime.h \ $(srcdir)/Include/internal/pycore_runtime_init.h \ + $(srcdir)/Include/internal/pycore_signal.h \ $(srcdir)/Include/internal/pycore_sliceobject.h \ $(srcdir)/Include/internal/pycore_strhex.h \ $(srcdir)/Include/internal/pycore_structseq.h \ diff --git a/Misc/NEWS.d/next/Library/2022-04-25-18-30-20.gh-issue-64783.HFtERN.rst b/Misc/NEWS.d/next/Library/2022-04-25-18-30-20.gh-issue-64783.HFtERN.rst new file mode 100644 index 00000000000..41814a6eb76 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-04-25-18-30-20.gh-issue-64783.HFtERN.rst @@ -0,0 +1,3 @@ +Fix :data:`signal.NSIG` value on FreeBSD to accept signal numbers greater than +32, like :data:`signal.SIGRTMIN` and :data:`signal.SIGRTMAX`. Patch by Victor +Stinner. diff --git a/Modules/faulthandler.c b/Modules/faulthandler.c index 91f96665a4a..4f709edb479 100644 --- a/Modules/faulthandler.c +++ b/Modules/faulthandler.c @@ -2,6 +2,7 @@ #include "pycore_initconfig.h" // _PyStatus_ERR #include "pycore_pyerrors.h" // _Py_DumpExtensionModules #include "pycore_pystate.h" // _PyThreadState_GET() +#include "pycore_signal.h" // Py_NSIG #include "pycore_traceback.h" // _Py_DumpTracebackThreads #include "frameobject.h" @@ -115,19 +116,6 @@ typedef struct { static user_signal_t *user_signals; -/* the following macros come from Python: Modules/signalmodule.c */ -#ifndef NSIG -# if defined(_NSIG) -# define NSIG _NSIG /* For BSD/SysV */ -# elif defined(_SIGMAX) -# define NSIG (_SIGMAX + 1) /* For QNX */ -# elif defined(SIGMAX) -# define NSIG (SIGMAX + 1) /* For djgpp */ -# else -# define NSIG 64 /* Use a reasonable default value */ -# endif -#endif - static void faulthandler_user(int signum); #endif /* FAULTHANDLER_USER */ @@ -896,7 +884,7 @@ check_signum(int signum) return 0; } } - if (signum < 1 || NSIG <= signum) { + if (signum < 1 || Py_NSIG <= signum) { PyErr_SetString(PyExc_ValueError, "signal number out of range"); return 0; } @@ -935,7 +923,7 @@ faulthandler_register_py(PyObject *self, return NULL; if (user_signals == NULL) { - user_signals = PyMem_Calloc(NSIG, sizeof(user_signal_t)); + user_signals = PyMem_Calloc(Py_NSIG, sizeof(user_signal_t)); if (user_signals == NULL) return PyErr_NoMemory(); } @@ -1215,7 +1203,7 @@ faulthandler_traverse(PyObject *module, visitproc visit, void *arg) Py_VISIT(thread.file); #ifdef FAULTHANDLER_USER if (user_signals != NULL) { - for (size_t signum=0; signum < NSIG; signum++) + for (size_t signum=0; signum < Py_NSIG; signum++) Py_VISIT(user_signals[signum].file); } #endif @@ -1416,7 +1404,7 @@ void _PyFaulthandler_Fini(void) #ifdef FAULTHANDLER_USER /* user */ if (user_signals != NULL) { - for (size_t signum=0; signum < NSIG; signum++) { + for (size_t signum=0; signum < Py_NSIG; signum++) { faulthandler_unregister(&user_signals[signum], signum); } PyMem_Free(user_signals); diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index a9132a78994..a2ea5079969 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -29,6 +29,7 @@ #include "pycore_moduleobject.h" // _PyModule_GetState() #include "pycore_object.h" // _PyObject_LookupSpecial() #include "pycore_pystate.h" // _PyInterpreterState_GET() +#include "pycore_signal.h" // Py_NSIG #include "structmember.h" // PyMemberDef #ifndef MS_WINDOWS @@ -1503,10 +1504,11 @@ _Py_Sigset_Converter(PyObject *obj, void *addr) while ((item = PyIter_Next(iterator)) != NULL) { signum = PyLong_AsLongAndOverflow(item, &overflow); Py_DECREF(item); - if (signum <= 0 || signum >= NSIG) { + if (signum <= 0 || signum >= Py_NSIG) { if (overflow || signum != -1 || !PyErr_Occurred()) { PyErr_Format(PyExc_ValueError, - "signal number %ld out of range", signum); + "signal number %ld out of range [1; %i]", + signum, Py_NSIG - 1); } goto error; } diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index 02c58ff538e..b602338e4f9 100644 --- a/Modules/signalmodule.c +++ b/Modules/signalmodule.c @@ -7,13 +7,13 @@ #include "pycore_atomic.h" // _Py_atomic_int #include "pycore_call.h" // _PyObject_Call() #include "pycore_ceval.h" // _PyEval_SignalReceived() +#include "pycore_emscripten_signal.h" // _Py_CHECK_EMSCRIPTEN_SIGNALS #include "pycore_fileutils.h" // _Py_BEGIN_SUPPRESS_IPH #include "pycore_frame.h" // _PyInterpreterFrame #include "pycore_moduleobject.h" // _PyModule_GetState() #include "pycore_pyerrors.h" // _PyErr_SetString() -#include "pycore_pylifecycle.h" // NSIG #include "pycore_pystate.h" // _PyThreadState_GET() -#include "pycore_emscripten_signal.h" // _Py_CHECK_EMSCRIPTEN_SIGNALS +#include "pycore_signal.h" // Py_NSIG #ifndef MS_WINDOWS # include "posixmodule.h" @@ -106,7 +106,7 @@ static volatile struct { * (even though it would probably be otherwise, anyway). */ _Py_atomic_address func; -} Handlers[NSIG]; +} Handlers[Py_NSIG]; #ifdef MS_WINDOWS #define INVALID_FD ((SOCKET_T)-1) @@ -542,7 +542,7 @@ signal_signal_impl(PyObject *module, int signalnum, PyObject *handler) "of the main interpreter"); return NULL; } - if (signalnum < 1 || signalnum >= NSIG) { + if (signalnum < 1 || signalnum >= Py_NSIG) { _PyErr_SetString(tstate, PyExc_ValueError, "signal number out of range"); return NULL; @@ -601,7 +601,7 @@ signal_getsignal_impl(PyObject *module, int signalnum) /*[clinic end generated code: output=35b3e0e796fd555e input=ac23a00f19dfa509]*/ { PyObject *old_handler; - if (signalnum < 1 || signalnum >= NSIG) { + if (signalnum < 1 || signalnum >= Py_NSIG) { PyErr_SetString(PyExc_ValueError, "signal number out of range"); return NULL; @@ -634,7 +634,7 @@ signal_strsignal_impl(PyObject *module, int signalnum) { const char *res; - if (signalnum < 1 || signalnum >= NSIG) { + if (signalnum < 1 || signalnum >= Py_NSIG) { PyErr_SetString(PyExc_ValueError, "signal number out of range"); return NULL; @@ -712,7 +712,7 @@ static PyObject * signal_siginterrupt_impl(PyObject *module, int signalnum, int flag) /*[clinic end generated code: output=063816243d85dd19 input=4160acacca3e2099]*/ { - if (signalnum < 1 || signalnum >= NSIG) { + if (signalnum < 1 || signalnum >= Py_NSIG) { PyErr_SetString(PyExc_ValueError, "signal number out of range"); return NULL; @@ -964,7 +964,7 @@ sigset_to_set(sigset_t mask) if (result == NULL) return NULL; - for (sig = 1; sig < NSIG; sig++) { + for (sig = 1; sig < Py_NSIG; sig++) { if (sigismember(&mask, sig) != 1) continue; @@ -1439,13 +1439,15 @@ the first is the signal number, the second is the interrupted stack frame."); static int signal_add_constants(PyObject *module) { + if (PyModule_AddIntConstant(module, "NSIG", Py_NSIG) < 0) { + return -1; + } + #define ADD_INT_MACRO(macro) \ if (PyModule_AddIntConstant(module, #macro, macro) < 0) { \ return -1; \ } - ADD_INT_MACRO(NSIG); - // SIG_xxx pthread_sigmask() constants #ifdef SIG_BLOCK ADD_INT_MACRO(SIG_BLOCK); @@ -1605,7 +1607,7 @@ static int signal_get_set_handlers(signal_state_t *state, PyObject *mod_dict) { // Get signal handlers - for (int signum = 1; signum < NSIG; signum++) { + for (int signum = 1; signum < Py_NSIG; signum++) { void (*c_handler)(int) = PyOS_getsig(signum); PyObject *func; if (c_handler == SIG_DFL) { @@ -1762,7 +1764,7 @@ _PySignal_Fini(void) signal_state_t *state = &signal_global_state; // Restore default signals and clear handlers - for (int signum = 1; signum < NSIG; signum++) { + for (int signum = 1; signum < Py_NSIG; signum++) { PyObject *func = get_handler(signum); _Py_atomic_store_relaxed(&Handlers[signum].tripped, 0); set_handler(signum, NULL); @@ -1828,7 +1830,7 @@ _PyErr_CheckSignalsTstate(PyThreadState *tstate) _PyInterpreterFrame *frame = tstate->cframe->current_frame; signal_state_t *state = &signal_global_state; - for (int i = 1; i < NSIG; i++) { + for (int i = 1; i < Py_NSIG; i++) { if (!_Py_atomic_load_relaxed(&Handlers[i].tripped)) { continue; } @@ -1905,7 +1907,7 @@ _PyErr_CheckSignals(void) int PyErr_SetInterruptEx(int signum) { - if (signum < 1 || signum >= NSIG) { + if (signum < 1 || signum >= Py_NSIG) { return -1; } @@ -1995,7 +1997,7 @@ _PySignal_Init(int install_signal_handlers) } #endif - for (int signum = 1; signum < NSIG; signum++) { + for (int signum = 1; signum < Py_NSIG; signum++) { _Py_atomic_store_relaxed(&Handlers[signum].tripped, 0); } @@ -2045,7 +2047,7 @@ _clear_pending_signals(void) } _Py_atomic_store(&is_tripped, 0); - for (int i = 1; i < NSIG; ++i) { + for (int i = 1; i < Py_NSIG; ++i) { _Py_atomic_store_relaxed(&Handlers[i].tripped, 0); } } diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj index 78bbec1e104..3ce116d2bab 100644 --- a/PCbuild/pythoncore.vcxproj +++ b/PCbuild/pythoncore.vcxproj @@ -238,6 +238,7 @@ + diff --git a/PCbuild/pythoncore.vcxproj.filters b/PCbuild/pythoncore.vcxproj.filters index 0a9d5454ba9..542d5510456 100644 --- a/PCbuild/pythoncore.vcxproj.filters +++ b/PCbuild/pythoncore.vcxproj.filters @@ -618,6 +618,9 @@ Include\internal + + Include\internal + Include\internal