From 6455ebd7681e1bf75540d21051b28fe88b15383e 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, 12 Apr 2025 10:17:05 +0200 Subject: [PATCH 1/6] ensure correct alignment of PyInterpreterState when UBSan is on --- Include/pyport.h | 3 +++ Python/pystate.c | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/Include/pyport.h b/Include/pyport.h index 2a7192c2c55cdd..1bc678d01c32df 100644 --- a/Include/pyport.h +++ b/Include/pyport.h @@ -664,14 +664,17 @@ extern "C" { #if defined(__has_feature) # if __has_feature(undefined_behavior_sanitizer) # define _Py_NO_SANITIZE_UNDEFINED __attribute__((no_sanitize("undefined"))) +# define _Py_HAS_UNDEFINED_BEHAVIOR_SANITIZER # endif #endif #if !defined(_Py_NO_SANITIZE_UNDEFINED) && defined(__GNUC__) \ && ((__GNUC__ >= 5) || (__GNUC__ == 4) && (__GNUC_MINOR__ >= 9)) # define _Py_NO_SANITIZE_UNDEFINED __attribute__((no_sanitize_undefined)) +# define _Py_HAS_UNDEFINED_BEHAVIOR_SANITIZER #endif #ifndef _Py_NO_SANITIZE_UNDEFINED # define _Py_NO_SANITIZE_UNDEFINED +# undef _Py_HAS_UNDEFINED_BEHAVIOR_SANITIZER #endif diff --git a/Python/pystate.c b/Python/pystate.c index ee35f0fa945f8b..63a9ffd08f7aab 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -569,11 +569,28 @@ _PyInterpreterState_Enable(_PyRuntimeState *runtime) return _PyStatus_OK(); } +#ifdef _Py_HAS_UNDEFINED_BEHAVIOR_SANITIZER +#define RAW_ALIGN_PTR_OFFSET sizeof(void *) +#endif -static PyInterpreterState * +static inline PyInterpreterState * alloc_interpreter(void) { +#ifdef _Py_HAS_UNDEFINED_BEHAVIOR_SANITIZER + size_t statesize = sizeof(PyInterpreterState); + size_t alignment = _Alignof(PyInterpreterState); + size_t allocsize = statesize + alignment - 1 + RAW_ALIGN_PTR_OFFSET; + void *mem = PyMem_RawCalloc(1, allocsize); + if (mem == NULL) { + return NULL; + } + char *interp = _Py_ALIGN_UP((char *)mem + RAW_ALIGN_PTR_OFFSET, alignment); + *(void **)(interp - RAW_ALIGN_PTR_OFFSET) = mem; + assert(_Py_IS_ALIGNED(interp, alignment)); + return (PyInterpreterState *)interp; +#else return PyMem_RawCalloc(1, sizeof(PyInterpreterState)); +#endif } static void @@ -587,12 +604,27 @@ free_interpreter(PyInterpreterState *interp) PyMem_RawFree(interp->obmalloc); interp->obmalloc = NULL; } +#ifdef _Py_HAS_UNDEFINED_BEHAVIOR_SANITIZER + assert(_Py_IS_ALIGNED(interp, _Alignof(PyInterpreterState))); + char *mem_location = (char *)interp - RAW_ALIGN_PTR_OFFSET; + void *mem = *((void **)mem_location); + if (mem != NULL) { + PyMem_RawFree(mem); + } +#else PyMem_RawFree(interp); +#endif } } + +#ifdef _Py_HAS_UNDEFINED_BEHAVIOR_SANITIZER +#undef RAW_ALIGN_PTR_OFFSET +#endif + #ifndef NDEBUG static inline int check_interpreter_whence(long); #endif + /* Get the interpreter state to a minimal consistent state. Further init happens in pylifecycle.c before it can be used. All fields not initialized here are expected to be zeroed out, From 239bfbdf540f20f4c5eef4ba322047dbd04df1ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 13 Apr 2025 01:56:19 +0200 Subject: [PATCH 2/6] Update Python/pystate.c --- Python/pystate.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Python/pystate.c b/Python/pystate.c index 63a9ffd08f7aab..b143a1a4a74d05 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -608,9 +608,7 @@ free_interpreter(PyInterpreterState *interp) assert(_Py_IS_ALIGNED(interp, _Alignof(PyInterpreterState))); char *mem_location = (char *)interp - RAW_ALIGN_PTR_OFFSET; void *mem = *((void **)mem_location); - if (mem != NULL) { - PyMem_RawFree(mem); - } + PyMem_RawFree(mem); #else PyMem_RawFree(interp); #endif From abb1af791c8fbaa745b96cba277854c25c804f82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Thu, 17 Apr 2025 20:51:12 +0200 Subject: [PATCH 3/6] Remove redundant `inline` keyword --- Python/pystate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/pystate.c b/Python/pystate.c index b143a1a4a74d05..710f0f3bee3773 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -573,7 +573,7 @@ _PyInterpreterState_Enable(_PyRuntimeState *runtime) #define RAW_ALIGN_PTR_OFFSET sizeof(void *) #endif -static inline PyInterpreterState * +static PyInterpreterState * alloc_interpreter(void) { #ifdef _Py_HAS_UNDEFINED_BEHAVIOR_SANITIZER From 9cb710d51d6a629e4c9fc20a77a57d2e7c3d0e3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Fri, 18 Apr 2025 11:24:50 +0200 Subject: [PATCH 4/6] unconditionally align `PyInterpreterState` --- Include/internal/pycore_interp_structs.h | 5 ++++ Include/pyport.h | 3 --- Python/pystate.c | 29 ++++-------------------- 3 files changed, 10 insertions(+), 27 deletions(-) diff --git a/Include/internal/pycore_interp_structs.h b/Include/internal/pycore_interp_structs.h index 573b56a57e1d54..03e5c6426f1fa6 100644 --- a/Include/internal/pycore_interp_structs.h +++ b/Include/internal/pycore_interp_structs.h @@ -748,6 +748,11 @@ struct _Py_unique_id_pool { The PyInterpreterState typedef is in Include/pytypedefs.h. */ struct _is { + /* This structure is carefully allocated so that it's correctly aligned + * to avoid undefined behaviors during LOAD and STORE. The '_malloced' + * field stores the allocated pointer address that will later be freed. + */ + uintptr_t _malloced; /* This struct contains the eval_breaker, * which is by far the hottest field in this struct diff --git a/Include/pyport.h b/Include/pyport.h index 1bc678d01c32df..2a7192c2c55cdd 100644 --- a/Include/pyport.h +++ b/Include/pyport.h @@ -664,17 +664,14 @@ extern "C" { #if defined(__has_feature) # if __has_feature(undefined_behavior_sanitizer) # define _Py_NO_SANITIZE_UNDEFINED __attribute__((no_sanitize("undefined"))) -# define _Py_HAS_UNDEFINED_BEHAVIOR_SANITIZER # endif #endif #if !defined(_Py_NO_SANITIZE_UNDEFINED) && defined(__GNUC__) \ && ((__GNUC__ >= 5) || (__GNUC__ == 4) && (__GNUC_MINOR__ >= 9)) # define _Py_NO_SANITIZE_UNDEFINED __attribute__((no_sanitize_undefined)) -# define _Py_HAS_UNDEFINED_BEHAVIOR_SANITIZER #endif #ifndef _Py_NO_SANITIZE_UNDEFINED # define _Py_NO_SANITIZE_UNDEFINED -# undef _Py_HAS_UNDEFINED_BEHAVIOR_SANITIZER #endif diff --git a/Python/pystate.c b/Python/pystate.c index 710f0f3bee3773..1dd2c8249f7500 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -569,28 +569,19 @@ _PyInterpreterState_Enable(_PyRuntimeState *runtime) return _PyStatus_OK(); } -#ifdef _Py_HAS_UNDEFINED_BEHAVIOR_SANITIZER -#define RAW_ALIGN_PTR_OFFSET sizeof(void *) -#endif - static PyInterpreterState * alloc_interpreter(void) { -#ifdef _Py_HAS_UNDEFINED_BEHAVIOR_SANITIZER - size_t statesize = sizeof(PyInterpreterState); size_t alignment = _Alignof(PyInterpreterState); - size_t allocsize = statesize + alignment - 1 + RAW_ALIGN_PTR_OFFSET; + size_t allocsize = sizeof(PyInterpreterState) + alignment - 1; void *mem = PyMem_RawCalloc(1, allocsize); if (mem == NULL) { return NULL; } - char *interp = _Py_ALIGN_UP((char *)mem + RAW_ALIGN_PTR_OFFSET, alignment); - *(void **)(interp - RAW_ALIGN_PTR_OFFSET) = mem; + PyInterpreterState *interp = _Py_ALIGN_UP(mem, alignment); assert(_Py_IS_ALIGNED(interp, alignment)); - return (PyInterpreterState *)interp; -#else - return PyMem_RawCalloc(1, sizeof(PyInterpreterState)); -#endif + interp->_malloced = (uintptr_t)mem; + return interp; } static void @@ -604,21 +595,11 @@ free_interpreter(PyInterpreterState *interp) PyMem_RawFree(interp->obmalloc); interp->obmalloc = NULL; } -#ifdef _Py_HAS_UNDEFINED_BEHAVIOR_SANITIZER assert(_Py_IS_ALIGNED(interp, _Alignof(PyInterpreterState))); - char *mem_location = (char *)interp - RAW_ALIGN_PTR_OFFSET; - void *mem = *((void **)mem_location); - PyMem_RawFree(mem); -#else - PyMem_RawFree(interp); -#endif + PyMem_RawFree((void *)interp->_malloced); } } -#ifdef _Py_HAS_UNDEFINED_BEHAVIOR_SANITIZER -#undef RAW_ALIGN_PTR_OFFSET -#endif - #ifndef NDEBUG static inline int check_interpreter_whence(long); #endif From 8bd8f220584c052e725b4738e5d1f862e77718db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Fri, 18 Apr 2025 16:40:38 +0200 Subject: [PATCH 5/6] use `void *` instead of `uintptr_t` --- Include/internal/pycore_interp_structs.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Include/internal/pycore_interp_structs.h b/Include/internal/pycore_interp_structs.h index 03e5c6426f1fa6..9ac4b4630abd3f 100644 --- a/Include/internal/pycore_interp_structs.h +++ b/Include/internal/pycore_interp_structs.h @@ -748,17 +748,18 @@ struct _Py_unique_id_pool { The PyInterpreterState typedef is in Include/pytypedefs.h. */ struct _is { - /* This structure is carefully allocated so that it's correctly aligned - * to avoid undefined behaviors during LOAD and STORE. The '_malloced' - * field stores the allocated pointer address that will later be freed. - */ - uintptr_t _malloced; /* This struct contains the eval_breaker, * which is by far the hottest field in this struct * and should be placed at the beginning. */ struct _ceval_state ceval; + /* This structure is carefully allocated so that it's correctly aligned + * to avoid undefined behaviors during LOAD and STORE. The '_malloced' + * field stores the allocated pointer address that will later be freed. + */ + void *_malloced; + PyInterpreterState *next; int64_t id; From 12032dfbf7c156c23ac65dd39056c045eb41286f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Fri, 18 Apr 2025 16:46:27 +0200 Subject: [PATCH 6/6] Apply suggestions from code review --- Python/pystate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/pystate.c b/Python/pystate.c index 1dd2c8249f7500..aba558279a657d 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -580,7 +580,7 @@ alloc_interpreter(void) } PyInterpreterState *interp = _Py_ALIGN_UP(mem, alignment); assert(_Py_IS_ALIGNED(interp, alignment)); - interp->_malloced = (uintptr_t)mem; + interp->_malloced = mem; return interp; } @@ -596,7 +596,7 @@ free_interpreter(PyInterpreterState *interp) interp->obmalloc = NULL; } assert(_Py_IS_ALIGNED(interp, _Alignof(PyInterpreterState))); - PyMem_RawFree((void *)interp->_malloced); + PyMem_RawFree(interp->_malloced); } }