Skip to content

Commit

Permalink
pythongh-64783: Fix signal.NSIG value on FreeBSD (python#91929)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
vstinner committed Apr 25, 2022
1 parent 61381d7 commit 20cc695
Show file tree
Hide file tree
Showing 11 changed files with 81 additions and 51 deletions.
1 change: 1 addition & 0 deletions Doc/library/signal.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 0 additions & 16 deletions Include/internal/pycore_pylifecycle.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,8 @@ extern "C" {
# error "this header requires Py_BUILD_CORE define"
#endif

#ifdef HAVE_SIGNAL_H
#include <signal.h>
#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;
Expand Down
35 changes: 35 additions & 0 deletions Include/internal/pycore_signal.h
Original file line number Diff line number Diff line change
@@ -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 <signal.h> // NSIG

#ifdef _SIG_MAXSIG
// gh-91145: On FreeBSD, <signal.h> 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
10 changes: 10 additions & 0 deletions Lib/test/test_signal.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,16 @@ def test_valid_signals(self):
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):
Expand Down
1 change: 1 addition & 0 deletions Makefile.pre.in
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
22 changes: 5 additions & 17 deletions Modules/faulthandler.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 */

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions Modules/posixmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
34 changes: 18 additions & 16 deletions Modules/signalmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}
}
Expand Down
1 change: 1 addition & 0 deletions PCbuild/pythoncore.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@
<ClInclude Include="..\Include\internal\pycore_pystate.h" />
<ClInclude Include="..\Include\internal\pycore_runtime.h" />
<ClInclude Include="..\Include\internal\pycore_runtime_init.h" />
<ClInclude Include="..\Include\internal\pycore_signal.h" />
<ClInclude Include="..\Include\internal\pycore_sliceobject.h" />
<ClInclude Include="..\Include\internal\pycore_strhex.h" />
<ClInclude Include="..\Include\internal\pycore_structseq.h" />
Expand Down
3 changes: 3 additions & 0 deletions PCbuild/pythoncore.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,9 @@
<ClInclude Include="..\Include\internal\pycore_runtime_init.h">
<Filter>Include\internal</Filter>
</ClInclude>
<ClInclude Include="..\Include\internal\pycore_signal.h">
<Filter>Include\internal</Filter>
</ClInclude>
<ClInclude Include="..\Include\internal\pycore_sliceobject.h">
<Filter>Include\internal</Filter>
</ClInclude>
Expand Down

0 comments on commit 20cc695

Please sign in to comment.