gh-111178: fix UBSan failures in Modules/_threadmodule.c (GH-129794)

Fix UBSan failures for `PyThreadHandleObject`, `lockobject`, `rlockobject`, `localdummyobject`, `localobject`

Add safe casts

Clean up module functions

Use semantically correct parameter names
This commit is contained in:
Bénédikt Tran 2025-02-24 13:42:39 +01:00 committed by GitHub
parent 73bbaf33ae
commit f8eefc2f35
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -582,6 +582,8 @@ typedef struct {
ThreadHandle *handle;
} PyThreadHandleObject;
#define PyThreadHandleObject_CAST(op) ((PyThreadHandleObject *)(op))
static PyThreadHandleObject *
PyThreadHandleObject_new(PyTypeObject *type)
{
@ -609,8 +611,7 @@ PyThreadHandleObject_tp_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
}
static int
PyThreadHandleObject_traverse(PyThreadHandleObject *self, visitproc visit,
void *arg)
PyThreadHandleObject_traverse(PyObject *self, visitproc visit, void *arg)
{
Py_VISIT(Py_TYPE(self));
return 0;
@ -619,7 +620,7 @@ PyThreadHandleObject_traverse(PyThreadHandleObject *self, visitproc visit,
static void
PyThreadHandleObject_dealloc(PyObject *op)
{
PyThreadHandleObject *self = (PyThreadHandleObject*)op;
PyThreadHandleObject *self = PyThreadHandleObject_CAST(op);
PyObject_GC_UnTrack(self);
PyTypeObject *tp = Py_TYPE(self);
ThreadHandle_decref(self->handle);
@ -630,23 +631,23 @@ PyThreadHandleObject_dealloc(PyObject *op)
static PyObject *
PyThreadHandleObject_repr(PyObject *op)
{
PyThreadHandleObject *self = (PyThreadHandleObject*)op;
PyThreadHandleObject *self = PyThreadHandleObject_CAST(op);
PyThread_ident_t ident = ThreadHandle_ident(self->handle);
return PyUnicode_FromFormat("<%s object: ident=%" PY_FORMAT_THREAD_IDENT_T ">",
Py_TYPE(self)->tp_name, ident);
}
static PyObject *
PyThreadHandleObject_get_ident(PyObject *op, void *Py_UNUSED(ignored))
PyThreadHandleObject_get_ident(PyObject *op, void *Py_UNUSED(closure))
{
PyThreadHandleObject *self = (PyThreadHandleObject*)op;
PyThreadHandleObject *self = PyThreadHandleObject_CAST(op);
return PyLong_FromUnsignedLongLong(ThreadHandle_ident(self->handle));
}
static PyObject *
PyThreadHandleObject_join(PyObject *op, PyObject *args)
{
PyThreadHandleObject *self = (PyThreadHandleObject*)op;
PyThreadHandleObject *self = PyThreadHandleObject_CAST(op);
PyObject *timeout_obj = NULL;
if (!PyArg_ParseTuple(args, "|O:join", &timeout_obj)) {
@ -668,9 +669,9 @@ PyThreadHandleObject_join(PyObject *op, PyObject *args)
}
static PyObject *
PyThreadHandleObject_is_done(PyObject *op, PyObject *Py_UNUSED(ignored))
PyThreadHandleObject_is_done(PyObject *op, PyObject *Py_UNUSED(dummy))
{
PyThreadHandleObject *self = (PyThreadHandleObject*)op;
PyThreadHandleObject *self = PyThreadHandleObject_CAST(op);
if (_PyEvent_IsSet(&self->handle->thread_is_exiting)) {
Py_RETURN_TRUE;
}
@ -680,9 +681,9 @@ PyThreadHandleObject_is_done(PyObject *op, PyObject *Py_UNUSED(ignored))
}
static PyObject *
PyThreadHandleObject_set_done(PyObject *op, PyObject *Py_UNUSED(ignored))
PyThreadHandleObject_set_done(PyObject *op, PyObject *Py_UNUSED(dummy))
{
PyThreadHandleObject *self = (PyThreadHandleObject*)op;
PyThreadHandleObject *self = PyThreadHandleObject_CAST(op);
if (ThreadHandle_set_done(self->handle) < 0) {
return NULL;
}
@ -726,6 +727,8 @@ typedef struct {
PyMutex lock;
} lockobject;
#define lockobject_CAST(op) ((lockobject *)(op))
static int
lock_traverse(PyObject *self, visitproc visit, void *arg)
{
@ -734,13 +737,12 @@ lock_traverse(PyObject *self, visitproc visit, void *arg)
}
static void
lock_dealloc(PyObject *op)
lock_dealloc(PyObject *self)
{
lockobject *self = (lockobject*)op;
PyObject_GC_UnTrack(self);
PyObject_ClearWeakRefs((PyObject *) self);
PyObject_ClearWeakRefs(self);
PyTypeObject *tp = Py_TYPE(self);
tp->tp_free((PyObject*)self);
tp->tp_free(self);
Py_DECREF(tp);
}
@ -794,7 +796,7 @@ lock_acquire_parse_args(PyObject *args, PyObject *kwds,
static PyObject *
lock_PyThread_acquire_lock(PyObject *op, PyObject *args, PyObject *kwds)
{
lockobject *self = (lockobject*)op;
lockobject *self = lockobject_CAST(op);
PyTime_t timeout;
if (lock_acquire_parse_args(args, kwds, &timeout) < 0) {
@ -834,9 +836,9 @@ PyDoc_STRVAR(enter_doc,
Lock the lock.");
static PyObject *
lock_PyThread_release_lock(PyObject *op, PyObject *Py_UNUSED(ignored))
lock_PyThread_release_lock(PyObject *op, PyObject *Py_UNUSED(dummy))
{
lockobject *self = (lockobject*)op;
lockobject *self = lockobject_CAST(op);
/* Sanity check: the lock must be locked */
if (_PyMutex_TryUnlock(&self->lock) < 0) {
PyErr_SetString(ThreadError, "release unlocked lock");
@ -867,9 +869,9 @@ PyDoc_STRVAR(lock_exit_doc,
Release the lock.");
static PyObject *
lock_locked_lock(PyObject *op, PyObject *Py_UNUSED(ignored))
lock_locked_lock(PyObject *op, PyObject *Py_UNUSED(dummy))
{
lockobject *self = (lockobject*)op;
lockobject *self = lockobject_CAST(op);
return PyBool_FromLong(PyMutex_IsLocked(&self->lock));
}
@ -888,16 +890,16 @@ An obsolete synonym of locked().");
static PyObject *
lock_repr(PyObject *op)
{
lockobject *self = (lockobject*)op;
lockobject *self = lockobject_CAST(op);
return PyUnicode_FromFormat("<%s %s object at %p>",
PyMutex_IsLocked(&self->lock) ? "locked" : "unlocked", Py_TYPE(self)->tp_name, self);
}
#ifdef HAVE_FORK
static PyObject *
lock__at_fork_reinit(PyObject *op, PyObject *Py_UNUSED(args))
lock__at_fork_reinit(PyObject *op, PyObject *Py_UNUSED(dummy))
{
lockobject *self = (lockobject *)op;
lockobject *self = lockobject_CAST(op);
_PyMutex_at_fork_reinit(&self->lock);
Py_RETURN_NONE;
}
@ -989,8 +991,10 @@ typedef struct {
_PyRecursiveMutex lock;
} rlockobject;
#define rlockobject_CAST(op) ((rlockobject *)(op))
static int
rlock_traverse(rlockobject *self, visitproc visit, void *arg)
rlock_traverse(PyObject *self, visitproc visit, void *arg)
{
Py_VISIT(Py_TYPE(self));
return 0;
@ -998,11 +1002,10 @@ rlock_traverse(rlockobject *self, visitproc visit, void *arg)
static void
rlock_dealloc(PyObject *op)
rlock_dealloc(PyObject *self)
{
rlockobject *self = (rlockobject*)op;
PyObject_GC_UnTrack(self);
PyObject_ClearWeakRefs((PyObject *) self);
PyObject_ClearWeakRefs(self);
PyTypeObject *tp = Py_TYPE(self);
tp->tp_free(self);
Py_DECREF(tp);
@ -1012,7 +1015,7 @@ rlock_dealloc(PyObject *op)
static PyObject *
rlock_acquire(PyObject *op, PyObject *args, PyObject *kwds)
{
rlockobject *self = (rlockobject*)op;
rlockobject *self = rlockobject_CAST(op);
PyTime_t timeout;
if (lock_acquire_parse_args(args, kwds, &timeout) < 0) {
@ -1052,10 +1055,9 @@ PyDoc_STRVAR(rlock_enter_doc,
Lock the lock.");
static PyObject *
rlock_release(PyObject *op, PyObject *Py_UNUSED(ignored))
rlock_release(PyObject *op, PyObject *Py_UNUSED(dummy))
{
rlockobject *self = (rlockobject*)op;
rlockobject *self = rlockobject_CAST(op);
if (_PyRecursiveMutex_TryUnlock(&self->lock) < 0) {
PyErr_SetString(PyExc_RuntimeError,
"cannot release un-acquired lock");
@ -1086,7 +1088,7 @@ Release the lock.");
static PyObject *
rlock_acquire_restore(PyObject *op, PyObject *args)
{
rlockobject *self = (rlockobject*)op;
rlockobject *self = rlockobject_CAST(op);
PyThread_ident_t owner;
Py_ssize_t count;
@ -1107,9 +1109,9 @@ PyDoc_STRVAR(rlock_acquire_restore_doc,
For internal use by `threading.Condition`.");
static PyObject *
rlock_release_save(PyObject *op, PyObject *Py_UNUSED(ignored))
rlock_release_save(PyObject *op, PyObject *Py_UNUSED(dummy))
{
rlockobject *self = (rlockobject*)op;
rlockobject *self = rlockobject_CAST(op);
if (!_PyRecursiveMutex_IsLockedByCurrentThread(&self->lock)) {
PyErr_SetString(PyExc_RuntimeError,
@ -1131,9 +1133,9 @@ PyDoc_STRVAR(rlock_release_save_doc,
For internal use by `threading.Condition`.");
static PyObject *
rlock_recursion_count(PyObject *op, PyObject *Py_UNUSED(ignored))
rlock_recursion_count(PyObject *op, PyObject *Py_UNUSED(dummy))
{
rlockobject *self = (rlockobject*)op;
rlockobject *self = rlockobject_CAST(op);
if (_PyRecursiveMutex_IsLockedByCurrentThread(&self->lock)) {
return PyLong_FromSize_t(self->lock.level + 1);
}
@ -1147,9 +1149,9 @@ PyDoc_STRVAR(rlock_recursion_count_doc,
For internal use by reentrancy checks.");
static PyObject *
rlock_is_owned(PyObject *op, PyObject *Py_UNUSED(ignored))
rlock_is_owned(PyObject *op, PyObject *Py_UNUSED(dummy))
{
rlockobject *self = (rlockobject*)op;
rlockobject *self = rlockobject_CAST(op);
long owned = _PyRecursiveMutex_IsLockedByCurrentThread(&self->lock);
return PyBool_FromLong(owned);
}
@ -1174,7 +1176,7 @@ rlock_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
static PyObject *
rlock_repr(PyObject *op)
{
rlockobject *self = (rlockobject*)op;
rlockobject *self = rlockobject_CAST(op);
PyThread_ident_t owner = self->lock.thread;
size_t count = self->lock.level + 1;
return PyUnicode_FromFormat(
@ -1187,8 +1189,9 @@ rlock_repr(PyObject *op)
#ifdef HAVE_FORK
static PyObject *
rlock__at_fork_reinit(rlockobject *self, PyObject *Py_UNUSED(args))
rlock__at_fork_reinit(PyObject *op, PyObject *Py_UNUSED(dummy))
{
rlockobject *self = rlockobject_CAST(op);
self->lock = (_PyRecursiveMutex){0};
Py_RETURN_NONE;
}
@ -1213,7 +1216,7 @@ static PyMethodDef rlock_methods[] = {
{"__exit__", rlock_release,
METH_VARARGS, rlock_exit_doc},
#ifdef HAVE_FORK
{"_at_fork_reinit", (PyCFunction)rlock__at_fork_reinit,
{"_at_fork_reinit", rlock__at_fork_reinit,
METH_NOARGS, NULL},
#endif
{NULL, NULL} /* sentinel */
@ -1310,14 +1313,17 @@ typedef struct {
PyObject *weakreflist; /* List of weak references to self */
} localdummyobject;
#define localdummyobject_CAST(op) ((localdummyobject *)(op))
static void
localdummy_dealloc(PyObject *op)
{
localdummyobject *self = (localdummyobject*)op;
if (self->weakreflist != NULL)
PyObject_ClearWeakRefs((PyObject *) self);
localdummyobject *self = localdummyobject_CAST(op);
if (self->weakreflist != NULL) {
PyObject_ClearWeakRefs(op);
}
PyTypeObject *tp = Py_TYPE(self);
tp->tp_free((PyObject*)self);
tp->tp_free(self);
Py_DECREF(tp);
}
@ -1353,6 +1359,8 @@ typedef struct {
PyObject *thread_watchdogs;
} localobject;
#define localobject_CAST(op) ((localobject *)(op))
/* Forward declaration */
static int create_localsdict(localobject *self, thread_module_state *state,
PyObject **localsdict, PyObject **sentinel_wr);
@ -1363,7 +1371,7 @@ static PyObject *
create_sentinel_wr(localobject *self)
{
static PyMethodDef wr_callback_def = {
"clear_locals", (PyCFunction) clear_locals, METH_O
"clear_locals", clear_locals, METH_O
};
PyThreadState *tstate = PyThreadState_Get();
@ -1455,8 +1463,9 @@ local_new(PyTypeObject *type, PyObject *args, PyObject *kw)
}
static int
local_traverse(localobject *self, visitproc visit, void *arg)
local_traverse(PyObject *op, visitproc visit, void *arg)
{
localobject *self = localobject_CAST(op);
Py_VISIT(Py_TYPE(self));
Py_VISIT(self->args);
Py_VISIT(self->kw);
@ -1466,8 +1475,9 @@ local_traverse(localobject *self, visitproc visit, void *arg)
}
static int
local_clear(localobject *self)
local_clear(PyObject *op)
{
localobject *self = localobject_CAST(op);
Py_CLEAR(self->args);
Py_CLEAR(self->kw);
Py_CLEAR(self->localdicts);
@ -1476,20 +1486,18 @@ local_clear(localobject *self)
}
static void
local_dealloc(localobject *self)
local_dealloc(PyObject *op)
{
localobject *self = localobject_CAST(op);
/* Weakrefs must be invalidated right now, otherwise they can be used
from code called below, which is very dangerous since Py_REFCNT(self) == 0 */
if (self->weakreflist != NULL) {
PyObject_ClearWeakRefs((PyObject *) self);
PyObject_ClearWeakRefs(op);
}
PyObject_GC_UnTrack(self);
local_clear(self);
(void)local_clear(op);
PyTypeObject *tp = Py_TYPE(self);
tp->tp_free((PyObject*)self);
tp->tp_free(self);
Py_DECREF(tp);
}
@ -1636,8 +1644,9 @@ _ldict(localobject *self, thread_module_state *state)
}
static int
local_setattro(localobject *self, PyObject *name, PyObject *v)
local_setattro(PyObject *op, PyObject *name, PyObject *v)
{
localobject *self = localobject_CAST(op);
PyObject *module = PyType_GetModuleByDef(Py_TYPE(self), &thread_module);
assert(module != NULL);
thread_module_state *state = get_thread_state(module);
@ -1658,8 +1667,7 @@ local_setattro(localobject *self, PyObject *name, PyObject *v)
goto err;
}
int st =
_PyObject_GenericSetAttrWithDict((PyObject *)self, name, v, ldict);
int st = _PyObject_GenericSetAttrWithDict(op, name, v, ldict);
Py_DECREF(ldict);
return st;
@ -1668,7 +1676,7 @@ err:
return -1;
}
static PyObject *local_getattro(localobject *, PyObject *);
static PyObject *local_getattro(PyObject *, PyObject *);
static PyMemberDef local_type_members[] = {
{"__weaklistoffset__", Py_T_PYSSIZET, offsetof(localobject, weakreflist), Py_READONLY},
@ -1676,12 +1684,12 @@ static PyMemberDef local_type_members[] = {
};
static PyType_Slot local_type_slots[] = {
{Py_tp_dealloc, (destructor)local_dealloc},
{Py_tp_getattro, (getattrofunc)local_getattro},
{Py_tp_setattro, (setattrofunc)local_setattro},
{Py_tp_dealloc, local_dealloc},
{Py_tp_getattro, local_getattro},
{Py_tp_setattro, local_setattro},
{Py_tp_doc, "_local()\n--\n\nThread-local data"},
{Py_tp_traverse, (traverseproc)local_traverse},
{Py_tp_clear, (inquiry)local_clear},
{Py_tp_traverse, local_traverse},
{Py_tp_clear, local_clear},
{Py_tp_new, local_new},
{Py_tp_members, local_type_members},
{0, 0}
@ -1696,8 +1704,9 @@ static PyType_Spec local_type_spec = {
};
static PyObject *
local_getattro(localobject *self, PyObject *name)
local_getattro(PyObject *op, PyObject *name)
{
localobject *self = localobject_CAST(op);
PyObject *module = PyType_GetModuleByDef(Py_TYPE(self), &thread_module);
assert(module != NULL);
thread_module_state *state = get_thread_state(module);
@ -1717,8 +1726,7 @@ local_getattro(localobject *self, PyObject *name)
if (!Py_IS_TYPE(self, state->local_type)) {
/* use generic lookup for subtypes */
PyObject *res =
_PyObject_GenericGetAttrWithDict((PyObject *)self, name, ldict, 0);
PyObject *res = _PyObject_GenericGetAttrWithDict(op, name, ldict, 0);
Py_DECREF(ldict);
return res;
}
@ -1732,8 +1740,7 @@ local_getattro(localobject *self, PyObject *name)
}
/* Fall back on generic to get __class__ and __dict__ */
PyObject *res =
_PyObject_GenericGetAttrWithDict((PyObject *)self, name, ldict, 0);
PyObject *res = _PyObject_GenericGetAttrWithDict(op, name, ldict, 0);
Py_DECREF(ldict);
return res;
}
@ -1744,7 +1751,7 @@ static PyObject *
clear_locals(PyObject *locals_and_key, PyObject *dummyweakref)
{
PyObject *localweakref = PyTuple_GetItem(locals_and_key, 0);
localobject *self = (localobject *)_PyWeakref_GET_REF(localweakref);
localobject *self = localobject_CAST(_PyWeakref_GET_REF(localweakref));
if (self == NULL) {
Py_RETURN_NONE;
}
@ -2525,13 +2532,13 @@ _thread_set_name_impl(PyObject *module, PyObject *name_obj)
static PyMethodDef thread_methods[] = {
{"start_new_thread", (PyCFunction)thread_PyThread_start_new_thread,
{"start_new_thread", thread_PyThread_start_new_thread,
METH_VARARGS, start_new_thread_doc},
{"start_new", (PyCFunction)thread_PyThread_start_new_thread,
{"start_new", thread_PyThread_start_new_thread,
METH_VARARGS, start_new_doc},
{"start_joinable_thread", _PyCFunction_CAST(thread_PyThread_start_joinable_thread),
METH_VARARGS | METH_KEYWORDS, start_joinable_doc},
{"daemon_threads_allowed", (PyCFunction)thread_daemon_threads_allowed,
{"daemon_threads_allowed", thread_daemon_threads_allowed,
METH_NOARGS, daemon_threads_allowed_doc},
{"allocate_lock", thread_PyThread_allocate_lock,
METH_NOARGS, allocate_lock_doc},
@ -2541,7 +2548,7 @@ static PyMethodDef thread_methods[] = {
METH_NOARGS, exit_thread_doc},
{"exit", thread_PyThread_exit_thread,
METH_NOARGS, exit_doc},
{"interrupt_main", (PyCFunction)thread_PyThread_interrupt_main,
{"interrupt_main", thread_PyThread_interrupt_main,
METH_VARARGS, interrupt_doc},
{"get_ident", thread_get_ident,
METH_NOARGS, get_ident_doc},
@ -2551,7 +2558,7 @@ static PyMethodDef thread_methods[] = {
#endif
{"_count", thread__count,
METH_NOARGS, _count_doc},
{"stack_size", (PyCFunction)thread_stack_size,
{"stack_size", thread_stack_size,
METH_VARARGS, stack_size_doc},
{"_excepthook", thread_excepthook,
METH_O, excepthook_doc},
@ -2723,7 +2730,7 @@ thread_module_clear(PyObject *module)
static void
thread_module_free(void *module)
{
thread_module_clear((PyObject *)module);
(void)thread_module_clear((PyObject *)module);
}