From 57237cf34e57bbf008b5827c226ca785d2341943 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 25 Jan 2025 17:09:17 +0100 Subject: [PATCH 1/5] fix UBSan failures for `PyZoneInfo_ZoneInfo` --- Modules/_zoneinfo.c | 68 ++++++++++++++++++++++----------------------- 1 file changed, 33 insertions(+), 35 deletions(-) diff --git a/Modules/_zoneinfo.c b/Modules/_zoneinfo.c index c5292575c22f23..519bec9904d411 100644 --- a/Modules/_zoneinfo.c +++ b/Modules/_zoneinfo.c @@ -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) + struct TransitionRuleType { int64_t (*year_to_timestamp)(TransitionRuleType *, int); }; @@ -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; } @@ -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; } @@ -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: @@ -338,7 +341,7 @@ 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); @@ -346,16 +349,18 @@ zoneinfo_ZoneInfo_impl(PyTypeObject *type, PyObject *key) } 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; @@ -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); @@ -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); } @@ -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) { @@ -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; @@ -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; } @@ -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; } @@ -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; } @@ -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, ×tamp)) { @@ -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. @@ -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 = From 832a7021a664f49118ae37d6f4b78ee762b2f281 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 8 Feb 2025 10:22:51 +0100 Subject: [PATCH 2/5] Do Do not use `_` + capital letter in new code as it is also UB. --- Modules/_zoneinfo.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/Modules/_zoneinfo.c b/Modules/_zoneinfo.c index fb73ebaffac248..5a38d8651c0c2a 100644 --- a/Modules/_zoneinfo.c +++ b/Modules/_zoneinfo.c @@ -56,8 +56,8 @@ 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) +#define PyZoneInfo_ZoneInfo_FAST_CAST(op) ((PyZoneInfo_ZoneInfo *)(op)) +#define PyZoneInfo_ZoneInfo_CAST(op) PyZoneInfo_ZoneInfo_FAST_CAST(op) struct TransitionRuleType { int64_t (*year_to_timestamp)(TransitionRuleType *, int); @@ -254,7 +254,7 @@ zoneinfo_new_instance(zoneinfo_state *state, PyTypeObject *type, PyObject *key) } } - if (load_data(state, _PyZoneInfo_ZoneInfo_FAST_CAST(self), file_obj)) { + if (load_data(state, PyZoneInfo_ZoneInfo_FAST_CAST(self), file_obj)) { goto error; } @@ -265,7 +265,7 @@ zoneinfo_new_instance(zoneinfo_state *state, PyTypeObject *type, PyObject *key) } Py_DECREF(rv); - _PyZoneInfo_ZoneInfo_FAST_CAST(self)->key = Py_NewRef(key); + PyZoneInfo_ZoneInfo_FAST_CAST(self)->key = Py_NewRef(key); goto cleanup; error: @@ -341,7 +341,7 @@ zoneinfo_ZoneInfo_impl(PyTypeObject *type, PyObject *key) if (instance == NULL) { return NULL; } - _PyZoneInfo_ZoneInfo_FAST_CAST(instance)->source = SOURCE_CACHE; + PyZoneInfo_ZoneInfo_FAST_CAST(instance)->source = SOURCE_CACHE; } update_strong_cache(state, type, key, instance); @@ -351,7 +351,7 @@ zoneinfo_ZoneInfo_impl(PyTypeObject *type, PyObject *key) static int zoneinfo_traverse(PyObject *op, visitproc visit, void *arg) { - PyZoneInfo_ZoneInfo *self = _PyZoneInfo_ZoneInfo_CAST(op); + PyZoneInfo_ZoneInfo *self = PyZoneInfo_ZoneInfo_CAST(op); Py_VISIT(Py_TYPE(self)); Py_VISIT(self->key); return 0; @@ -360,7 +360,7 @@ zoneinfo_traverse(PyObject *op, visitproc visit, void *arg) static int zoneinfo_clear(PyObject *op) { - PyZoneInfo_ZoneInfo *self = _PyZoneInfo_ZoneInfo_CAST(op); + PyZoneInfo_ZoneInfo *self = PyZoneInfo_ZoneInfo_CAST(op); Py_CLEAR(self->key); Py_CLEAR(self->file_repr); return 0; @@ -369,7 +369,7 @@ zoneinfo_clear(PyObject *op) static void zoneinfo_dealloc(PyObject *obj_self) { - PyZoneInfo_ZoneInfo *self = _PyZoneInfo_ZoneInfo_CAST(obj_self); + PyZoneInfo_ZoneInfo *self = PyZoneInfo_ZoneInfo_CAST(obj_self); PyTypeObject *tp = Py_TYPE(self); PyObject_GC_UnTrack(self); @@ -429,7 +429,7 @@ zoneinfo_ZoneInfo_from_file_impl(PyTypeObject *type, PyTypeObject *cls, if (obj_self == NULL) { return NULL; } - self = _PyZoneInfo_ZoneInfo_FAST_CAST(obj_self); + self = PyZoneInfo_ZoneInfo_FAST_CAST(obj_self); file_repr = PyObject_Repr(file_obj); if (file_repr == NULL) { @@ -471,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_CAST(out)->source = SOURCE_NOCACHE; + PyZoneInfo_ZoneInfo_CAST(out)->source = SOURCE_NOCACHE; } return out; @@ -563,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_CAST(self), dt); + _ttinfo *tti = find_ttinfo(state, PyZoneInfo_ZoneInfo_CAST(self), dt); if (tti == NULL) { return NULL; } @@ -585,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_CAST(self), dt); + _ttinfo *tti = find_ttinfo(state, PyZoneInfo_ZoneInfo_CAST(self), dt); if (tti == NULL) { return NULL; } @@ -608,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_CAST(self), dt); + _ttinfo *tti = find_ttinfo(state, PyZoneInfo_ZoneInfo_CAST(self), dt); if (tti == NULL) { return NULL; } @@ -632,7 +632,7 @@ zoneinfo_fromutc(PyObject *obj_self, PyObject *dt) return NULL; } - PyZoneInfo_ZoneInfo *self = _PyZoneInfo_ZoneInfo_CAST(obj_self); + PyZoneInfo_ZoneInfo *self = PyZoneInfo_ZoneInfo_CAST(obj_self); int64_t timestamp; if (get_local_timestamp(dt, ×tamp)) { @@ -743,7 +743,7 @@ zoneinfo_fromutc(PyObject *obj_self, PyObject *dt) static PyObject * zoneinfo_repr(PyObject *op) { - PyZoneInfo_ZoneInfo *self = _PyZoneInfo_ZoneInfo_CAST(op); + PyZoneInfo_ZoneInfo *self = PyZoneInfo_ZoneInfo_CAST(op); if (self->key != Py_None) { return PyUnicode_FromFormat("%T(key=%R)", self, self->key); } @@ -754,7 +754,7 @@ zoneinfo_repr(PyObject *op) static PyObject * zoneinfo_str(PyObject *op) { - PyZoneInfo_ZoneInfo *self = _PyZoneInfo_ZoneInfo_CAST(op); + PyZoneInfo_ZoneInfo *self = PyZoneInfo_ZoneInfo_CAST(op); if (self->key != Py_None) { return Py_NewRef(self->key); } @@ -776,7 +776,7 @@ zoneinfo_str(PyObject *op) static PyObject * zoneinfo_reduce(PyObject *obj_self, PyObject *Py_UNUSED(unused)) { - PyZoneInfo_ZoneInfo *self = _PyZoneInfo_ZoneInfo_CAST(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 = From 29f69792207b997c5ac0d8bc355ec4018d37f0d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 25 Feb 2025 13:46:58 +0100 Subject: [PATCH 3/5] remove `PyZoneInfo_ZoneInfo_FAST_CAST` macro Raw casts were restored when they could be left as is --- Modules/_zoneinfo.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/Modules/_zoneinfo.c b/Modules/_zoneinfo.c index 5a38d8651c0c2a..4e5fd62a5d9dde 100644 --- a/Modules/_zoneinfo.c +++ b/Modules/_zoneinfo.c @@ -56,8 +56,7 @@ 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) +#define PyZoneInfo_ZoneInfo_CAST(op) ((PyZoneInfo_ZoneInfo *)(op)) struct TransitionRuleType { int64_t (*year_to_timestamp)(TransitionRuleType *, int); @@ -254,7 +253,8 @@ zoneinfo_new_instance(zoneinfo_state *state, PyTypeObject *type, PyObject *key) } } - if (load_data(state, PyZoneInfo_ZoneInfo_FAST_CAST(self), file_obj)) { + PyZoneInfo_ZoneInfo *self_zinfo = (PyZoneInfo_ZoneInfo *)self; + if (load_data(state, self_zinfo, file_obj)) { goto error; } @@ -265,7 +265,7 @@ zoneinfo_new_instance(zoneinfo_state *state, PyTypeObject *type, PyObject *key) } Py_DECREF(rv); - PyZoneInfo_ZoneInfo_FAST_CAST(self)->key = Py_NewRef(key); + self_zinfo->key = Py_NewRef(key); goto cleanup; error: @@ -341,7 +341,7 @@ zoneinfo_ZoneInfo_impl(PyTypeObject *type, PyObject *key) if (instance == NULL) { return NULL; } - PyZoneInfo_ZoneInfo_FAST_CAST(instance)->source = SOURCE_CACHE; + ((PyZoneInfo_ZoneInfo *)(instance))->source = SOURCE_CACHE; } update_strong_cache(state, type, key, instance); @@ -425,11 +425,10 @@ zoneinfo_ZoneInfo_from_file_impl(PyTypeObject *type, PyTypeObject *cls, PyObject *file_repr = NULL; PyZoneInfo_ZoneInfo *self = NULL; - PyObject *obj_self = type->tp_alloc(type, 0); - if (obj_self == NULL) { + self = (PyZoneInfo_ZoneInfo *)type->tp_alloc(type, 0); + if (self == NULL) { return NULL; } - self = PyZoneInfo_ZoneInfo_FAST_CAST(obj_self); file_repr = PyObject_Repr(file_obj); if (file_repr == NULL) { @@ -444,7 +443,7 @@ zoneinfo_ZoneInfo_from_file_impl(PyTypeObject *type, PyTypeObject *cls, self->source = SOURCE_FILE; self->file_repr = file_repr; self->key = Py_NewRef(key); - return obj_self; + return (PyObject *)self; error: Py_XDECREF(file_repr); From d9490615be8d77fc35390e073ae5d40cc970d055 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 25 Feb 2025 13:51:05 +0100 Subject: [PATCH 4/5] reduce diff --- Modules/_zoneinfo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_zoneinfo.c b/Modules/_zoneinfo.c index 4e5fd62a5d9dde..50c55f961f6372 100644 --- a/Modules/_zoneinfo.c +++ b/Modules/_zoneinfo.c @@ -341,7 +341,7 @@ zoneinfo_ZoneInfo_impl(PyTypeObject *type, PyObject *key) if (instance == NULL) { return NULL; } - ((PyZoneInfo_ZoneInfo *)(instance))->source = SOURCE_CACHE; + ((PyZoneInfo_ZoneInfo *)instance)->source = SOURCE_CACHE; } update_strong_cache(state, type, key, instance); From 9447bb49c02c68cafd27167ceb0f116bcc08e1e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 25 Feb 2025 13:51:16 +0100 Subject: [PATCH 5/5] fix semantic naming --- Modules/_zoneinfo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_zoneinfo.c b/Modules/_zoneinfo.c index 50c55f961f6372..8c882bef24867e 100644 --- a/Modules/_zoneinfo.c +++ b/Modules/_zoneinfo.c @@ -773,7 +773,7 @@ zoneinfo_str(PyObject *op) * Objects constructed from ZoneInfo.from_file cannot be pickled. */ static PyObject * -zoneinfo_reduce(PyObject *obj_self, PyObject *Py_UNUSED(unused)) +zoneinfo_reduce(PyObject *obj_self, PyObject *Py_UNUSED(dummy)) { PyZoneInfo_ZoneInfo *self = PyZoneInfo_ZoneInfo_CAST(obj_self); if (self->source == SOURCE_FILE) {