Skip to content

gh-111178: fix UBSan failures in Modules/_tkinter.c #129795

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 10 commits into from
Feb 26, 2025
118 changes: 68 additions & 50 deletions Modules/_tkinter.c
Original file line number Diff line number Diff line change
Expand Up @@ -290,12 +290,14 @@ static PyThreadState *tcl_tstate = NULL;
tcl_tstate = tstate; }

#define CHECK_TCL_APPARTMENT \
if (((TkappObject *)self)->threaded && \
((TkappObject *)self)->thread_id != Tcl_GetCurrentThread()) { \
PyErr_SetString(PyExc_RuntimeError, \
"Calling Tcl from different apartment"); \
return 0; \
}
do { \
if (TkappObject_CAST(self)->threaded && \
TkappObject_CAST(self)->thread_id != Tcl_GetCurrentThread()) { \
PyErr_SetString(PyExc_RuntimeError, \
"Calling Tcl from different apartment"); \
return 0; \
} \
} while (0)

#ifndef FREECAST
#define FREECAST (char *)
Expand Down Expand Up @@ -328,7 +330,8 @@ typedef struct {
const Tcl_ObjType *PixelType;
} TkappObject;

#define Tkapp_Interp(v) (((TkappObject *) (v))->interp)
#define TkappObject_CAST(op) ((TkappObject *)(op))
#define Tkapp_Interp(v) (TkappObject_CAST(v)->interp)


/**** Error Handling ****/
Expand Down Expand Up @@ -763,6 +766,8 @@ typedef struct {
PyObject *string; /* This cannot cause cycles. */
} PyTclObject;

#define PyTclObject_CAST(op) ((PyTclObject *)(op))

static PyObject *PyTclObject_Type;
#define PyTclObject_Check(v) Py_IS_TYPE(v, (PyTypeObject *) PyTclObject_Type)

Expand All @@ -782,7 +787,7 @@ newPyTclObject(Tcl_Obj *arg)
static void
PyTclObject_dealloc(PyObject *_self)
{
PyTclObject *self = (PyTclObject *)_self;
PyTclObject *self = PyTclObject_CAST(_self);
PyObject *tp = (PyObject *) Py_TYPE(self);
Tcl_DecrRefCount(self->value);
Py_XDECREF(self->string);
Expand All @@ -795,9 +800,9 @@ PyDoc_STRVAR(PyTclObject_string__doc__,
"the string representation of this object, either as str or bytes");

static PyObject *
PyTclObject_string(PyObject *_self, void *ignored)
PyTclObject_string(PyObject *_self, void *Py_UNUSED(closure))
{
PyTclObject *self = (PyTclObject *)_self;
PyTclObject *self = PyTclObject_CAST(_self);
if (!self->string) {
self->string = unicodeFromTclObj(NULL, self->value);
if (!self->string)
Expand All @@ -809,7 +814,7 @@ PyTclObject_string(PyObject *_self, void *ignored)
static PyObject *
PyTclObject_str(PyObject *_self)
{
PyTclObject *self = (PyTclObject *)_self;
PyTclObject *self = PyTclObject_CAST(_self);
if (self->string) {
return Py_NewRef(self->string);
}
Expand All @@ -820,7 +825,7 @@ PyTclObject_str(PyObject *_self)
static PyObject *
PyTclObject_repr(PyObject *_self)
{
PyTclObject *self = (PyTclObject *)_self;
PyTclObject *self = PyTclObject_CAST(_self);
PyObject *repr, *str = PyTclObject_str(_self);
if (str == NULL)
return NULL;
Expand All @@ -846,21 +851,25 @@ PyTclObject_richcompare(PyObject *self, PyObject *other, int op)
Py_RETURN_NOTIMPLEMENTED;
}

if (self == other)
if (self == other) {
/* fast path when self and other are identical */
result = 0;
else
}
else {
// 'self' and 'other' are known to be of correct type,
// so we can fast cast them (no need to use the macro)
result = strcmp(Tcl_GetString(((PyTclObject *)self)->value),
Tcl_GetString(((PyTclObject *)other)->value));
}
Py_RETURN_RICHCOMPARE(result, 0, op);
}

PyDoc_STRVAR(get_typename__doc__, "name of the Tcl type");

static PyObject*
get_typename(PyObject *self, void* ignored)
get_typename(PyObject *self, void *Py_UNUSED(closure))
{
PyTclObject *obj = (PyTclObject *)self;
PyTclObject *obj = PyTclObject_CAST(self);
return unicodeFromTclString(obj->value->typePtr->name);
}

Expand Down Expand Up @@ -1457,7 +1466,7 @@ Tkapp_Call(PyObject *selfptr, PyObject *args)
Tcl_Obj **objv = NULL;
Tcl_Size objc;
PyObject *res = NULL;
TkappObject *self = (TkappObject*)selfptr;
TkappObject *self = TkappObject_CAST(selfptr);
int flags = TCL_EVAL_DIRECT | TCL_EVAL_GLOBAL;

/* If args is a single tuple, replace with contents of tuple */
Expand Down Expand Up @@ -1705,7 +1714,7 @@ varname_converter(PyObject *in, void *_out)
return 1;
}
if (PyTclObject_Check(in)) {
*out = Tcl_GetString(((PyTclObject *)in)->value);
*out = Tcl_GetString(((PyTclObject *)in)->value); // safe fast cast
return 1;
}
PyErr_Format(PyExc_TypeError,
Expand Down Expand Up @@ -1742,7 +1751,7 @@ var_proc(Tcl_Event *evPtr, int flags)
static PyObject*
var_invoke(EventFunc func, PyObject *selfptr, PyObject *args, int flags)
{
TkappObject *self = (TkappObject*)selfptr;
TkappObject *self = TkappObject_CAST(selfptr);
if (self->threaded && self->thread_id != Tcl_GetCurrentThread()) {
VarEvent *ev;
// init 'res' and 'exc' to make static analyzers happy
Expand Down Expand Up @@ -1800,11 +1809,11 @@ SetVar(TkappObject *self, PyObject *args, int flags)
return NULL;

if (flags & TCL_GLOBAL_ONLY) {
TRACE((TkappObject *)self, ("((ssssO))", "uplevel", "#0", "set",
name1, newValue));
TRACE(self, ("((ssssO))", "uplevel", "#0", "set",
name1, newValue));
}
else {
TRACE((TkappObject *)self, ("((ssO))", "set", name1, newValue));
TRACE(self, ("((ssO))", "set", name1, newValue));
}

ENTER_TCL
Expand All @@ -1827,16 +1836,16 @@ SetVar(TkappObject *self, PyObject *args, int flags)

/* XXX must hold tcl lock already??? */
newval = AsObj(newValue);
if (((TkappObject *)self)->trace) {
if (self->trace) {
if (flags & TCL_GLOBAL_ONLY) {
TRACE((TkappObject *)self, ("((sssNO))", "uplevel", "#0", "set",
PyUnicode_FromFormat("%s(%s)", name1, name2),
newValue));
TRACE(self, ("((sssNO))", "uplevel", "#0", "set",
PyUnicode_FromFormat("%s(%s)", name1, name2),
newValue));
}
else {
TRACE((TkappObject *)self, ("((sNO))", "set",
PyUnicode_FromFormat("%s(%s)", name1, name2),
newValue));
TRACE(self, ("((sNO))", "set",
PyUnicode_FromFormat("%s(%s)", name1, name2),
newValue));
}
}

Expand Down Expand Up @@ -1921,29 +1930,30 @@ UnsetVar(TkappObject *self, PyObject *args, int flags)
int code;
PyObject *res = NULL;

if (!PyArg_ParseTuple(args, "s|s:unsetvar", &name1, &name2))
if (!PyArg_ParseTuple(args, "s|s:unsetvar", &name1, &name2)) {
return NULL;
}

CHECK_STRING_LENGTH(name1);
CHECK_STRING_LENGTH(name2);

if (((TkappObject *)self)->trace) {
if (self->trace) {
if (flags & TCL_GLOBAL_ONLY) {
if (name2) {
TRACE((TkappObject *)self, ("((sssN))", "uplevel", "#0", "unset",
PyUnicode_FromFormat("%s(%s)", name1, name2)));
TRACE(self, ("((sssN))", "uplevel", "#0", "unset",
PyUnicode_FromFormat("%s(%s)", name1, name2)));
}
else {
TRACE((TkappObject *)self, ("((ssss))", "uplevel", "#0", "unset", name1));
TRACE(self, ("((ssss))", "uplevel", "#0", "unset", name1));
}
}
else {
if (name2) {
TRACE((TkappObject *)self, ("((sN))", "unset",
PyUnicode_FromFormat("%s(%s)", name1, name2)));
TRACE(self, ("((sN))", "unset",
PyUnicode_FromFormat("%s(%s)", name1, name2)));
}
else {
TRACE((TkappObject *)self, ("((ss))", "unset", name1));
TRACE(self, ("((ss))", "unset", name1));
}
}
}
Expand Down Expand Up @@ -2679,13 +2689,19 @@ _tkinter_tkapp_deletefilehandler(TkappObject *self, PyObject *file)
/**** Tktt Object (timer token) ****/

static PyObject *Tktt_Type;
#define TkttObject_Check(op) \
PyObject_TypeCheck((op), (PyTypeObject *)Tktt_Type)

typedef struct {
PyObject_HEAD
Tcl_TimerToken token;
PyObject *func;
} TkttObject;

#define TkttObject_FAST_CAST(op) ((TkttObject *)(op))
#define TkttObject_CAST(op) \
(assert(TkttObject_Check(op)), TkttObject_FAST_CAST(op))

/*[clinic input]
_tkinter.tktimertoken.deletetimerhandler

Expand Down Expand Up @@ -2730,7 +2746,7 @@ Tktt_New(PyObject *func)
static void
Tktt_Dealloc(PyObject *self)
{
TkttObject *v = (TkttObject *)self;
TkttObject *v = TkttObject_CAST(self);
PyObject *func = v->func;
PyObject *tp = (PyObject *) Py_TYPE(self);

Expand All @@ -2743,7 +2759,7 @@ Tktt_Dealloc(PyObject *self)
static PyObject *
Tktt_Repr(PyObject *self)
{
TkttObject *v = (TkttObject *)self;
TkttObject *v = TkttObject_CAST(self);
return PyUnicode_FromFormat("<tktimertoken at %p%s>",
v,
v->func == NULL ? ", handler deleted" : "");
Expand All @@ -2754,7 +2770,7 @@ Tktt_Repr(PyObject *self)
static void
TimerHandler(ClientData clientData)
{
TkttObject *v = (TkttObject *)clientData;
TkttObject *v = TkttObject_CAST(clientData);
PyObject *func = v->func;
PyObject *res;

Expand Down Expand Up @@ -2958,16 +2974,17 @@ _tkinter_tkapp_loadtk_impl(TkappObject *self)
}

static PyObject *
Tkapp_WantObjects(PyObject *self, PyObject *args)
Tkapp_WantObjects(PyObject *op, PyObject *args)
{

TkappObject *self = TkappObject_CAST(op);
int wantobjects = -1;
if (!PyArg_ParseTuple(args, "|i:wantobjects", &wantobjects))
if (!PyArg_ParseTuple(args, "|i:wantobjects", &wantobjects)) {
return NULL;
if (wantobjects == -1)
return PyLong_FromLong(((TkappObject*)self)->wantobjects);
((TkappObject*)self)->wantobjects = wantobjects;

}
if (wantobjects == -1) {
return PyLong_FromLong(self->wantobjects);
}
self->wantobjects = wantobjects;
Py_RETURN_NONE;
}

Expand Down Expand Up @@ -3030,14 +3047,15 @@ _tkinter_tkapp_willdispatch_impl(TkappObject *self)
/**** Tkapp Type Methods ****/

static void
Tkapp_Dealloc(PyObject *self)
Tkapp_Dealloc(PyObject *op)
{
PyObject *tp = (PyObject *) Py_TYPE(self);
TkappObject *self = TkappObject_CAST(op);
PyTypeObject *tp = Py_TYPE(self);
/*CHECK_TCL_APPARTMENT;*/
ENTER_TCL
Tcl_DeleteInterp(Tkapp_Interp(self));
LEAVE_TCL
Py_XDECREF(((TkappObject *)self)->trace);
Py_XDECREF(self->trace);
PyObject_Free(self);
Py_DECREF(tp);
DisableEventHook();
Expand Down
Loading