Skip to content

gh-64783: Fix signal.NSIG value on FreeBSD #91929

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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