Skip to content

gh-111178: fix UBSan failures in Modules/_zoneinfo.c #129798

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.

Sign up for GitHub

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 7 commits into from
Feb 26, 2025
Merged

gh-111178: fix UBSan failures in Modules/_zoneinfo.c #129798

Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
57237cf
fix UBSan failures for `PyZoneInfo_ZoneInfo`
picnixz Jan 25, 2025
8c2d236
Merge remote-tracking branch 'upstream/main' into fix/ubsan/zoneinfo-…
picnixz Feb 7, 2025
832a702
Do Do not use `_` + capital letter in new code as it is also UB.
picnixz Feb 8, 2025
d97c2fd
Merge branch 'main' into fix/ubsan/zoneinfo-111178
encukou Feb 21, 2025
29f6979
remove `PyZoneInfo_ZoneInfo_FAST_CAST` macro
picnixz Feb 25, 2025
d949061
reduce diff
picnixz Feb 25, 2025
9447bb4
fix semantic naming
picnixz Feb 25, 2025
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
68 changes: 33 additions & 35 deletions Modules/_zoneinfo.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ typedef struct {
unsigned char source;
} PyZoneInfo_ZoneInfo;

#define PyZoneInfo_ZoneInfo_FAST_CAST(op) ((PyZoneInfo_ZoneInfo *)(op))
#define PyZoneInfo_ZoneInfo_CAST(op) PyZoneInfo_ZoneInfo_FAST_CAST(op)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This deserves a comment.
Or replacing FAST_CAST uses with direct casts.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the fast cast macro but added a message in my commit saying that that raw casts don't need something else. I think I've only had fast cast macros when I was writing the date/time/tkinter PRs but I may have introduced that in other PR.


struct TransitionRuleType {
int64_t (*year_to_timestamp)(TransitionRuleType *, int);
};
Expand Down Expand Up @@ -238,7 +241,7 @@ zoneinfo_new_instance(zoneinfo_state *state, PyTypeObject *type, PyObject *key)
}
}

PyObject *self = (PyObject *)(type->tp_alloc(type, 0));
PyObject *self = type->tp_alloc(type, 0);
if (self == NULL) {
goto error;
}
Expand All @@ -251,7 +254,7 @@ zoneinfo_new_instance(zoneinfo_state *state, PyTypeObject *type, PyObject *key)
}
}

if (load_data(state, (PyZoneInfo_ZoneInfo *)self, file_obj)) {
if (load_data(state, PyZoneInfo_ZoneInfo_FAST_CAST(self), file_obj)) {
goto error;
}

Expand All @@ -262,7 +265,7 @@ zoneinfo_new_instance(zoneinfo_state *state, PyTypeObject *type, PyObject *key)
}
Py_DECREF(rv);

((PyZoneInfo_ZoneInfo *)self)->key = Py_NewRef(key);
PyZoneInfo_ZoneInfo_FAST_CAST(self)->key = Py_NewRef(key);

goto cleanup;
error:
Expand Down Expand Up @@ -338,24 +341,26 @@ zoneinfo_ZoneInfo_impl(PyTypeObject *type, PyObject *key)
if (instance == NULL) {
return NULL;
}
((PyZoneInfo_ZoneInfo *)instance)->source = SOURCE_CACHE;
PyZoneInfo_ZoneInfo_FAST_CAST(instance)->source = SOURCE_CACHE;
}

update_strong_cache(state, type, key, instance);
return instance;
}

static int
zoneinfo_traverse(PyZoneInfo_ZoneInfo *self, visitproc visit, void *arg)
zoneinfo_traverse(PyObject *op, visitproc visit, void *arg)
{
PyZoneInfo_ZoneInfo *self = PyZoneInfo_ZoneInfo_CAST(op);
Py_VISIT(Py_TYPE(self));
Py_VISIT(self->key);
return 0;
}

static int
zoneinfo_clear(PyZoneInfo_ZoneInfo *self)
zoneinfo_clear(PyObject *op)
{
PyZoneInfo_ZoneInfo *self = PyZoneInfo_ZoneInfo_CAST(op);
Py_CLEAR(self->key);
Py_CLEAR(self->file_repr);
return 0;
Expand All @@ -364,7 +369,7 @@ zoneinfo_clear(PyZoneInfo_ZoneInfo *self)
static void
zoneinfo_dealloc(PyObject *obj_self)
{
PyZoneInfo_ZoneInfo *self = (PyZoneInfo_ZoneInfo *)obj_self;
PyZoneInfo_ZoneInfo *self = PyZoneInfo_ZoneInfo_CAST(obj_self);
PyTypeObject *tp = Py_TYPE(self);
PyObject_GC_UnTrack(self);

Expand Down Expand Up @@ -395,7 +400,7 @@ zoneinfo_dealloc(PyObject *obj_self)

free_tzrule(&(self->tzrule_after));

zoneinfo_clear(self);
(void)zoneinfo_clear(obj_self);
tp->tp_free(obj_self);
Py_DECREF(tp);
}
Expand All @@ -420,11 +425,11 @@ zoneinfo_ZoneInfo_from_file_impl(PyTypeObject *type, PyTypeObject *cls,
PyObject *file_repr = NULL;
PyZoneInfo_ZoneInfo *self = NULL;

PyObject *obj_self = (PyObject *)(type->tp_alloc(type, 0));
self = (PyZoneInfo_ZoneInfo *)obj_self;
if (self == NULL) {
PyObject *obj_self = type->tp_alloc(type, 0);
if (obj_self == NULL) {
return NULL;
}
self = PyZoneInfo_ZoneInfo_FAST_CAST(obj_self);

file_repr = PyObject_Repr(file_obj);
if (file_repr == NULL) {
Expand Down Expand Up @@ -466,7 +471,7 @@ zoneinfo_ZoneInfo_no_cache_impl(PyTypeObject *type, PyTypeObject *cls,
zoneinfo_state *state = zoneinfo_get_state_by_cls(cls);
PyObject *out = zoneinfo_new_instance(state, type, key);
if (out != NULL) {
((PyZoneInfo_ZoneInfo *)out)->source = SOURCE_NOCACHE;
PyZoneInfo_ZoneInfo_CAST(out)->source = SOURCE_NOCACHE;
}

return out;
Expand Down Expand Up @@ -558,7 +563,7 @@ zoneinfo_ZoneInfo_utcoffset_impl(PyObject *self, PyTypeObject *cls,
/*[clinic end generated code: output=b71016c319ba1f91 input=2bb6c5364938f19c]*/
{
zoneinfo_state *state = zoneinfo_get_state_by_cls(cls);
_ttinfo *tti = find_ttinfo(state, (PyZoneInfo_ZoneInfo *)self, dt);
_ttinfo *tti = find_ttinfo(state, PyZoneInfo_ZoneInfo_CAST(self), dt);
if (tti == NULL) {
return NULL;
}
Expand All @@ -580,7 +585,7 @@ zoneinfo_ZoneInfo_dst_impl(PyObject *self, PyTypeObject *cls, PyObject *dt)
/*[clinic end generated code: output=cb6168d7723a6ae6 input=2167fb80cf8645c6]*/
{
zoneinfo_state *state = zoneinfo_get_state_by_cls(cls);
_ttinfo *tti = find_ttinfo(state, (PyZoneInfo_ZoneInfo *)self, dt);
_ttinfo *tti = find_ttinfo(state, PyZoneInfo_ZoneInfo_CAST(self), dt);
if (tti == NULL) {
return NULL;
}
Expand All @@ -603,7 +608,7 @@ zoneinfo_ZoneInfo_tzname_impl(PyObject *self, PyTypeObject *cls,
/*[clinic end generated code: output=3b6ae6c3053ea75a input=15a59a4f92ed1f1f]*/
{
zoneinfo_state *state = zoneinfo_get_state_by_cls(cls);
_ttinfo *tti = find_ttinfo(state, (PyZoneInfo_ZoneInfo *)self, dt);
_ttinfo *tti = find_ttinfo(state, PyZoneInfo_ZoneInfo_CAST(self), dt);
if (tti == NULL) {
return NULL;
}
Expand All @@ -627,7 +632,7 @@ zoneinfo_fromutc(PyObject *obj_self, PyObject *dt)
return NULL;
}

PyZoneInfo_ZoneInfo *self = (PyZoneInfo_ZoneInfo *)obj_self;
PyZoneInfo_ZoneInfo *self = PyZoneInfo_ZoneInfo_CAST(obj_self);

int64_t timestamp;
if (get_local_timestamp(dt, &timestamp)) {
Expand Down Expand Up @@ -736,31 +741,24 @@ zoneinfo_fromutc(PyObject *obj_self, PyObject *dt)
}

static PyObject *
zoneinfo_repr(PyZoneInfo_ZoneInfo *self)
zoneinfo_repr(PyObject *op)
{
PyObject *rv = NULL;
const char *type_name = Py_TYPE((PyObject *)self)->tp_name;
if (!(self->key == Py_None)) {
rv = PyUnicode_FromFormat("%s(key=%R)", type_name, self->key);
}
else {
assert(PyUnicode_Check(self->file_repr));
rv = PyUnicode_FromFormat("%s.from_file(%U)", type_name,
self->file_repr);
PyZoneInfo_ZoneInfo *self = PyZoneInfo_ZoneInfo_CAST(op);
if (self->key != Py_None) {
return PyUnicode_FromFormat("%T(key=%R)", self, self->key);
}

return rv;
assert(PyUnicode_Check(self->file_repr));
return PyUnicode_FromFormat("%T.from_file(%U)", self, self->file_repr);
}

static PyObject *
zoneinfo_str(PyZoneInfo_ZoneInfo *self)
zoneinfo_str(PyObject *op)
{
if (!(self->key == Py_None)) {
PyZoneInfo_ZoneInfo *self = PyZoneInfo_ZoneInfo_CAST(op);
if (self->key != Py_None) {
return Py_NewRef(self->key);
}
else {
return zoneinfo_repr(self);
}
return zoneinfo_repr(op);
}

/* Pickles the ZoneInfo object by key and source.
Expand All @@ -776,9 +774,9 @@ zoneinfo_str(PyZoneInfo_ZoneInfo *self)
* Objects constructed from ZoneInfo.from_file cannot be pickled.
*/
static PyObject *
zoneinfo_reduce(PyObject *obj_self, PyObject *unused)
zoneinfo_reduce(PyObject *obj_self, PyObject *Py_UNUSED(unused))
{
PyZoneInfo_ZoneInfo *self = (PyZoneInfo_ZoneInfo *)obj_self;
PyZoneInfo_ZoneInfo *self = PyZoneInfo_ZoneInfo_CAST(obj_self);
if (self->source == SOURCE_FILE) {
// Objects constructed from files cannot be pickled.
PyObject *pickle_error =
Expand Down
Loading