From 568db400ff07240a5ed6f263af281405ccaec716 Mon Sep 17 00:00:00 2001 From: Collin Funk Date: Thu, 20 Feb 2025 08:02:33 -0800 Subject: [PATCH 1/4] gh-129838: Don't redefine _Py_NO_SANITIZE_UNDEFINED (#129839) Newer GCC versions accept both __attribute__((no_sanitize("undefined"))) and __attribute__((no_sanitize_undefined)) so check that the macro is not already defined. --- .../Build/2025-02-07-21-20-21.gh-issue-129838.fkuiEc.rst | 2 ++ Modules/faulthandler.c | 9 +++++---- 2 files changed, 7 insertions(+), 4 deletions(-) create mode 100644 Misc/NEWS.d/next/Build/2025-02-07-21-20-21.gh-issue-129838.fkuiEc.rst diff --git a/Misc/NEWS.d/next/Build/2025-02-07-21-20-21.gh-issue-129838.fkuiEc.rst b/Misc/NEWS.d/next/Build/2025-02-07-21-20-21.gh-issue-129838.fkuiEc.rst new file mode 100644 index 00000000000000..958460249a65b6 --- /dev/null +++ b/Misc/NEWS.d/next/Build/2025-02-07-21-20-21.gh-issue-129838.fkuiEc.rst @@ -0,0 +1,2 @@ +Don't redefine ``_Py_NO_SANITIZE_UNDEFINED`` when compiling with a recent +GCC version and undefined sanitizer enabled. diff --git a/Modules/faulthandler.c b/Modules/faulthandler.c index a15ced22677ab7..4bfb63e66d2b62 100644 --- a/Modules/faulthandler.c +++ b/Modules/faulthandler.c @@ -37,14 +37,15 @@ #define PUTS(fd, str) (void)_Py_write_noraise(fd, str, strlen(str)) -// clang uses __attribute__((no_sanitize("undefined"))) -// GCC 4.9+ uses __attribute__((no_sanitize_undefined)) -#if defined(__has_feature) // Clang +// Clang and GCC 9.0+ use __attribute__((no_sanitize("undefined"))) +#if defined(__has_feature) # if __has_feature(undefined_behavior_sanitizer) # define _Py_NO_SANITIZE_UNDEFINED __attribute__((no_sanitize("undefined"))) # endif #endif -#if defined(__GNUC__) \ + +// GCC 4.9+ uses __attribute__((no_sanitize_undefined)) +#if !defined(_Py_NO_SANITIZE_UNDEFINED) && defined(__GNUC__) \ && ((__GNUC__ >= 5) || (__GNUC__ == 4) && (__GNUC_MINOR__ >= 9)) # define _Py_NO_SANITIZE_UNDEFINED __attribute__((no_sanitize_undefined)) #endif From ca22147547413229a933e3d9cac21cbecf1183fe Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 20 Feb 2025 11:31:15 -0500 Subject: [PATCH 2/4] gh-111924: Fix data races when swapping allocators (gh-130287) CPython current temporarily changes `PYMEM_DOMAIN_RAW` to the default allocator during initialization and shutdown. The motivation is to ensure that core runtime structures are allocated and freed using the same allocator. However, modifying the current allocator changes global state and is not thread-safe even with the GIL. Other threads may be allocating or freeing objects use PYMEM_DOMAIN_RAW; they are not required to hold the GIL to call PyMem_RawMalloc/PyMem_RawFree. This adds new internal-only functions like `_PyMem_DefaultRawMalloc` that aren't affected by calls to `PyMem_SetAllocator()`, so they're appropriate for Python runtime initialization and finalization. Use these calls in places where we previously swapped to the default raw allocator. --- Include/internal/pycore_pymem.h | 15 ++--- Objects/obmalloc.c | 73 +++++++++++++++++---- Python/import.c | 43 ++----------- Python/initconfig.c | 74 ++++++++++++++-------- Python/pathconfig.c | 71 +++++++-------------- Python/pystate.c | 2 +- Python/sysmodule.c | 22 ++----- Tools/tsan/suppressions.txt | 2 - Tools/tsan/suppressions_free_threading.txt | 7 -- 9 files changed, 155 insertions(+), 154 deletions(-) diff --git a/Include/internal/pycore_pymem.h b/Include/internal/pycore_pymem.h index 5bb34001aab1b4..defe143e9dcc3c 100644 --- a/Include/internal/pycore_pymem.h +++ b/Include/internal/pycore_pymem.h @@ -55,13 +55,6 @@ struct _Py_mem_interp_free_queue { struct llist_node head; // queue of _mem_work_chunk items }; -/* Set the memory allocator of the specified domain to the default. - Save the old allocator into *old_alloc if it's non-NULL. - Return on success, or return -1 if the domain is unknown. */ -extern int _PyMem_SetDefaultAllocator( - PyMemAllocatorDomain domain, - PyMemAllocatorEx *old_alloc); - /* Special bytes broadcast into debug memory blocks at appropriate times. Strings of these are unlikely to be valid addresses, floats, ints or 7-bit ASCII. @@ -113,6 +106,13 @@ extern int _PyMem_GetAllocatorName( PYMEM_ALLOCATOR_NOT_SET does nothing. */ extern int _PyMem_SetupAllocators(PyMemAllocatorName allocator); +// Default raw memory allocator that is not affected by PyMem_SetAllocator() +extern void *_PyMem_DefaultRawMalloc(size_t); +extern void *_PyMem_DefaultRawCalloc(size_t, size_t); +extern void *_PyMem_DefaultRawRealloc(void *, size_t); +extern void _PyMem_DefaultRawFree(void *); +extern wchar_t *_PyMem_DefaultRawWcsdup(const wchar_t *str); + /* Is the debug allocator enabled? */ extern int _PyMem_DebugEnabled(void); @@ -132,7 +132,6 @@ static inline void _PyObject_XDecRefDelayed(PyObject *obj) // Periodically process delayed free requests. extern void _PyMem_ProcessDelayed(PyThreadState *tstate); - // Periodically process delayed free requests when the world is stopped. // Notify of any objects whic should be freeed. typedef void (*delayed_dealloc_cb)(PyObject *, void *); diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 6341251007aa8e..5e70e06b9e3171 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -344,6 +344,68 @@ void _PyMem_DebugFree(void *ctx, void *p); #define PYDBGOBJ_ALLOC \ {&_PyRuntime.allocators.debug.obj, _PyMem_DebugMalloc, _PyMem_DebugCalloc, _PyMem_DebugRealloc, _PyMem_DebugFree} +/* default raw allocator (not swappable) */ + +void * +_PyMem_DefaultRawMalloc(size_t size) +{ +#ifdef Py_DEBUG + return _PyMem_DebugRawMalloc(&_PyRuntime.allocators.debug.raw, size); +#else + return _PyMem_RawMalloc(NULL, size); +#endif +} + +void * +_PyMem_DefaultRawCalloc(size_t nelem, size_t elsize) +{ +#ifdef Py_DEBUG + return _PyMem_DebugRawCalloc(&_PyRuntime.allocators.debug.raw, nelem, elsize); +#else + return _PyMem_RawCalloc(NULL, nelem, elsize); +#endif +} + +void * +_PyMem_DefaultRawRealloc(void *ptr, size_t size) +{ +#ifdef Py_DEBUG + return _PyMem_DebugRawRealloc(&_PyRuntime.allocators.debug.raw, ptr, size); +#else + return _PyMem_RawRealloc(NULL, ptr, size); +#endif +} + +void +_PyMem_DefaultRawFree(void *ptr) +{ +#ifdef Py_DEBUG + _PyMem_DebugRawFree(&_PyRuntime.allocators.debug.raw, ptr); +#else + _PyMem_RawFree(NULL, ptr); +#endif +} + +wchar_t* +_PyMem_DefaultRawWcsdup(const wchar_t *str) +{ + assert(str != NULL); + + size_t len = wcslen(str); + if (len > (size_t)PY_SSIZE_T_MAX / sizeof(wchar_t) - 1) { + return NULL; + } + + size_t size = (len + 1) * sizeof(wchar_t); + wchar_t *str2 = _PyMem_DefaultRawMalloc(size); + if (str2 == NULL) { + return NULL; + } + + memcpy(str2, str, size); + return str2; +} + /* the low-level virtual memory allocator */ #ifdef WITH_PYMALLOC @@ -492,17 +554,6 @@ static const int pydebug = 1; static const int pydebug = 0; #endif -int -_PyMem_SetDefaultAllocator(PyMemAllocatorDomain domain, - PyMemAllocatorEx *old_alloc) -{ - PyMutex_Lock(&ALLOCATORS_MUTEX); - int res = set_default_allocator_unlocked(domain, pydebug, old_alloc); - PyMutex_Unlock(&ALLOCATORS_MUTEX); - return res; -} - - int _PyMem_GetAllocatorName(const char *name, PyMemAllocatorName *allocator) { diff --git a/Python/import.c b/Python/import.c index 8cc8d3a503bffa..6d3b42941bdc37 100644 --- a/Python/import.c +++ b/Python/import.c @@ -13,7 +13,7 @@ #include "pycore_pyerrors.h" // _PyErr_SetString() #include "pycore_pyhash.h" // _Py_KeyedHash() #include "pycore_pylifecycle.h" -#include "pycore_pymem.h" // _PyMem_SetDefaultAllocator() +#include "pycore_pymem.h" // _PyMem_DefaultRawFree() #include "pycore_pystate.h" // _PyInterpreterState_GET() #include "pycore_sysmodule.h" // _PySys_ClearAttrString() #include "pycore_time.h" // _PyTime_AsMicroseconds() @@ -2387,14 +2387,11 @@ PyImport_ExtendInittab(struct _inittab *newtab) /* Force default raw memory allocator to get a known allocator to be able to release the memory in _PyImport_Fini2() */ - PyMemAllocatorEx old_alloc; - _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc); - /* Allocate new memory for the combined table */ p = NULL; if (i + n <= SIZE_MAX / sizeof(struct _inittab) - 1) { size_t size = sizeof(struct _inittab) * (i + n + 1); - p = PyMem_RawRealloc(inittab_copy, size); + p = _PyMem_DefaultRawRealloc(inittab_copy, size); } if (p == NULL) { res = -1; @@ -2408,9 +2405,7 @@ PyImport_ExtendInittab(struct _inittab *newtab) } memcpy(p + i, newtab, (n + 1) * sizeof(struct _inittab)); PyImport_Inittab = inittab_copy = p; - done: - PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); return res; } @@ -2445,7 +2440,7 @@ init_builtin_modules_table(void) size++; /* Make the copy. */ - struct _inittab *copied = PyMem_RawMalloc(size * sizeof(struct _inittab)); + struct _inittab *copied = _PyMem_DefaultRawMalloc(size * sizeof(struct _inittab)); if (copied == NULL) { return -1; } @@ -2459,7 +2454,7 @@ fini_builtin_modules_table(void) { struct _inittab *inittab = INITTAB; INITTAB = NULL; - PyMem_RawFree(inittab); + _PyMem_DefaultRawFree(inittab); } PyObject * @@ -3977,22 +3972,10 @@ _PyImport_Init(void) if (INITTAB != NULL) { return _PyStatus_ERR("global import state already initialized"); } - - PyStatus status = _PyStatus_OK(); - - /* Force default raw memory allocator to get a known allocator to be able - to release the memory in _PyImport_Fini() */ - PyMemAllocatorEx old_alloc; - _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc); - if (init_builtin_modules_table() != 0) { - status = PyStatus_NoMemory(); - goto done; + return PyStatus_NoMemory(); } - -done: - PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); - return status; + return _PyStatus_OK(); } void @@ -4003,31 +3986,19 @@ _PyImport_Fini(void) // ever dlclose() the module files? _extensions_cache_clear_all(); - /* Use the same memory allocator as _PyImport_Init(). */ - PyMemAllocatorEx old_alloc; - _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc); - /* Free memory allocated by _PyImport_Init() */ fini_builtin_modules_table(); - - PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); } void _PyImport_Fini2(void) { - /* Use the same memory allocator than PyImport_ExtendInittab(). */ - PyMemAllocatorEx old_alloc; - _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc); - // Reset PyImport_Inittab PyImport_Inittab = _PyImport_Inittab; /* Free memory allocated by PyImport_ExtendInittab() */ - PyMem_RawFree(inittab_copy); + _PyMem_DefaultRawFree(inittab_copy); inittab_copy = NULL; - - PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); } diff --git a/Python/initconfig.c b/Python/initconfig.c index 1733ed05a3e8d9..8fc36b1c0bcc22 100644 --- a/Python/initconfig.c +++ b/Python/initconfig.c @@ -7,7 +7,7 @@ #include "pycore_pathconfig.h" // _Py_path_config #include "pycore_pyerrors.h" // _PyErr_GetRaisedException() #include "pycore_pylifecycle.h" // _Py_PreInitializeFromConfig() -#include "pycore_pymem.h" // _PyMem_SetDefaultAllocator() +#include "pycore_pymem.h" // _PyMem_DefaultRawMalloc() #include "pycore_pystate.h" // _PyThreadState_GET() #include "pycore_pystats.h" // _Py_StatsOn() #include "pycore_sysmodule.h" // _PySys_SetIntMaxStrDigits() @@ -619,53 +619,87 @@ _PyWideStringList_CheckConsistency(const PyWideStringList *list) #endif /* Py_DEBUG */ -void -_PyWideStringList_Clear(PyWideStringList *list) +static void +_PyWideStringList_ClearEx(PyWideStringList *list, + bool use_default_allocator) { assert(_PyWideStringList_CheckConsistency(list)); for (Py_ssize_t i=0; i < list->length; i++) { - PyMem_RawFree(list->items[i]); + if (use_default_allocator) { + _PyMem_DefaultRawFree(list->items[i]); + } + else { + PyMem_RawFree(list->items[i]); + } + } + if (use_default_allocator) { + _PyMem_DefaultRawFree(list->items); + } + else { + PyMem_RawFree(list->items); } - PyMem_RawFree(list->items); list->length = 0; list->items = NULL; } +void +_PyWideStringList_Clear(PyWideStringList *list) +{ + _PyWideStringList_ClearEx(list, false); +} -int -_PyWideStringList_Copy(PyWideStringList *list, const PyWideStringList *list2) +static int +_PyWideStringList_CopyEx(PyWideStringList *list, + const PyWideStringList *list2, + bool use_default_allocator) { assert(_PyWideStringList_CheckConsistency(list)); assert(_PyWideStringList_CheckConsistency(list2)); if (list2->length == 0) { - _PyWideStringList_Clear(list); + _PyWideStringList_ClearEx(list, use_default_allocator); return 0; } PyWideStringList copy = _PyWideStringList_INIT; size_t size = list2->length * sizeof(list2->items[0]); - copy.items = PyMem_RawMalloc(size); + if (use_default_allocator) { + copy.items = _PyMem_DefaultRawMalloc(size); + } + else { + copy.items = PyMem_RawMalloc(size); + } if (copy.items == NULL) { return -1; } for (Py_ssize_t i=0; i < list2->length; i++) { - wchar_t *item = _PyMem_RawWcsdup(list2->items[i]); + wchar_t *item; + if (use_default_allocator) { + item = _PyMem_DefaultRawWcsdup(list2->items[i]); + } + else { + item = _PyMem_RawWcsdup(list2->items[i]); + } if (item == NULL) { - _PyWideStringList_Clear(©); + _PyWideStringList_ClearEx(©, use_default_allocator); return -1; } copy.items[i] = item; copy.length = i + 1; } - _PyWideStringList_Clear(list); + _PyWideStringList_ClearEx(list, use_default_allocator); *list = copy; return 0; } +int +_PyWideStringList_Copy(PyWideStringList *list, const PyWideStringList *list2) +{ + return _PyWideStringList_CopyEx(list, list2, false); +} PyStatus PyWideStringList_Insert(PyWideStringList *list, @@ -789,12 +823,7 @@ _PyWideStringList_AsTuple(const PyWideStringList *list) void _Py_ClearArgcArgv(void) { - PyMemAllocatorEx old_alloc; - _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc); - - _PyWideStringList_Clear(&_PyRuntime.orig_argv); - - PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); + _PyWideStringList_ClearEx(&_PyRuntime.orig_argv, true); } @@ -802,17 +831,10 @@ static int _Py_SetArgcArgv(Py_ssize_t argc, wchar_t * const *argv) { const PyWideStringList argv_list = {.length = argc, .items = (wchar_t **)argv}; - int res; - - PyMemAllocatorEx old_alloc; - _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc); // XXX _PyRuntime.orig_argv only gets cleared by Py_Main(), // so it currently leaks for embedders. - res = _PyWideStringList_Copy(&_PyRuntime.orig_argv, &argv_list); - - PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); - return res; + return _PyWideStringList_CopyEx(&_PyRuntime.orig_argv, &argv_list, true); } diff --git a/Python/pathconfig.c b/Python/pathconfig.c index 33abaddc1b5df4..92360c1bb02420 100644 --- a/Python/pathconfig.c +++ b/Python/pathconfig.c @@ -4,7 +4,7 @@ #include "pycore_initconfig.h" // _PyStatus_OK() #include "pycore_fileutils.h" // _Py_wgetcwd() #include "pycore_pathconfig.h" -#include "pycore_pymem.h" // _PyMem_SetDefaultAllocator() +#include "pycore_pymem.h" // _PyMem_DefaultRawFree() #include #include "marshal.h" // PyMarshal_ReadObjectFromString @@ -54,12 +54,9 @@ _PyPathConfig_GetGlobalModuleSearchPath(void) void _PyPathConfig_ClearGlobal(void) { - PyMemAllocatorEx old_alloc; - _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc); - #define CLEAR(ATTR) \ do { \ - PyMem_RawFree(_Py_path_config.ATTR); \ + _PyMem_DefaultRawFree(_Py_path_config.ATTR); \ _Py_path_config.ATTR = NULL; \ } while (0) @@ -74,8 +71,6 @@ _PyPathConfig_ClearGlobal(void) _Py_path_config._is_python_build = 0; #undef CLEAR - - PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); } PyStatus @@ -126,14 +121,11 @@ _PyPathConfig_ReadGlobal(PyConfig *config) PyStatus _PyPathConfig_UpdateGlobal(const PyConfig *config) { - PyMemAllocatorEx old_alloc; - _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc); - #define COPY(ATTR) \ do { \ if (config->ATTR) { \ - PyMem_RawFree(_Py_path_config.ATTR); \ - _Py_path_config.ATTR = _PyMem_RawWcsdup(config->ATTR); \ + _PyMem_DefaultRawFree(_Py_path_config.ATTR); \ + _Py_path_config.ATTR = _PyMem_DefaultRawWcsdup(config->ATTR); \ if (!_Py_path_config.ATTR) goto error; \ } \ } while (0) @@ -141,8 +133,8 @@ _PyPathConfig_UpdateGlobal(const PyConfig *config) #define COPY2(ATTR, SRCATTR) \ do { \ if (config->SRCATTR) { \ - PyMem_RawFree(_Py_path_config.ATTR); \ - _Py_path_config.ATTR = _PyMem_RawWcsdup(config->SRCATTR); \ + _PyMem_DefaultRawFree(_Py_path_config.ATTR); \ + _Py_path_config.ATTR = _PyMem_DefaultRawWcsdup(config->SRCATTR); \ if (!_Py_path_config.ATTR) goto error; \ } \ } while (0) @@ -165,9 +157,9 @@ _PyPathConfig_UpdateGlobal(const PyConfig *config) #undef COPY2 #undef COPY_INT - PyMem_RawFree(_Py_path_config.module_search_path); + _PyMem_DefaultRawFree(_Py_path_config.module_search_path); _Py_path_config.module_search_path = NULL; - PyMem_RawFree(_Py_path_config.calculated_module_search_path); + _PyMem_DefaultRawFree(_Py_path_config.calculated_module_search_path); _Py_path_config.calculated_module_search_path = NULL; do { @@ -176,7 +168,7 @@ _PyPathConfig_UpdateGlobal(const PyConfig *config) cch += 1 + wcslen(config->module_search_paths.items[i]); } - wchar_t *path = (wchar_t*)PyMem_RawMalloc(sizeof(wchar_t) * cch); + wchar_t *path = (wchar_t*)_PyMem_DefaultRawMalloc(sizeof(wchar_t) * cch); if (!path) { goto error; } @@ -194,11 +186,9 @@ _PyPathConfig_UpdateGlobal(const PyConfig *config) _Py_path_config.calculated_module_search_path = path; } while (0); - PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); return _PyStatus_OK(); error: - PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); return _PyStatus_NO_MEMORY(); } @@ -218,29 +208,24 @@ Py_SetPath(const wchar_t *path) return; } - PyMemAllocatorEx old_alloc; - _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc); + _PyMem_DefaultRawFree(_Py_path_config.prefix); + _PyMem_DefaultRawFree(_Py_path_config.exec_prefix); + _PyMem_DefaultRawFree(_Py_path_config.stdlib_dir); + _PyMem_DefaultRawFree(_Py_path_config.module_search_path); + _PyMem_DefaultRawFree(_Py_path_config.calculated_module_search_path); - PyMem_RawFree(_Py_path_config.prefix); - PyMem_RawFree(_Py_path_config.exec_prefix); - PyMem_RawFree(_Py_path_config.stdlib_dir); - PyMem_RawFree(_Py_path_config.module_search_path); - PyMem_RawFree(_Py_path_config.calculated_module_search_path); - - _Py_path_config.prefix = _PyMem_RawWcsdup(L""); - _Py_path_config.exec_prefix = _PyMem_RawWcsdup(L""); + _Py_path_config.prefix = _PyMem_DefaultRawWcsdup(L""); + _Py_path_config.exec_prefix = _PyMem_DefaultRawWcsdup(L""); // XXX Copy this from the new module_search_path? if (_Py_path_config.home != NULL) { - _Py_path_config.stdlib_dir = _PyMem_RawWcsdup(_Py_path_config.home); + _Py_path_config.stdlib_dir = _PyMem_DefaultRawWcsdup(_Py_path_config.home); } else { - _Py_path_config.stdlib_dir = _PyMem_RawWcsdup(L""); + _Py_path_config.stdlib_dir = _PyMem_DefaultRawWcsdup(L""); } - _Py_path_config.module_search_path = _PyMem_RawWcsdup(path); + _Py_path_config.module_search_path = _PyMem_DefaultRawWcsdup(path); _Py_path_config.calculated_module_search_path = NULL; - PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); - if (_Py_path_config.prefix == NULL || _Py_path_config.exec_prefix == NULL || _Py_path_config.stdlib_dir == NULL @@ -256,18 +241,13 @@ Py_SetPythonHome(const wchar_t *home) { int has_value = home && home[0]; - PyMemAllocatorEx old_alloc; - _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc); - - PyMem_RawFree(_Py_path_config.home); + _PyMem_DefaultRawFree(_Py_path_config.home); _Py_path_config.home = NULL; if (has_value) { - _Py_path_config.home = _PyMem_RawWcsdup(home); + _Py_path_config.home = _PyMem_DefaultRawWcsdup(home); } - PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); - if (has_value && _Py_path_config.home == NULL) { path_out_of_memory(__func__); } @@ -279,18 +259,13 @@ Py_SetProgramName(const wchar_t *program_name) { int has_value = program_name && program_name[0]; - PyMemAllocatorEx old_alloc; - _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc); - - PyMem_RawFree(_Py_path_config.program_name); + _PyMem_DefaultRawFree(_Py_path_config.program_name); _Py_path_config.program_name = NULL; if (has_value) { - _Py_path_config.program_name = _PyMem_RawWcsdup(program_name); + _Py_path_config.program_name = _PyMem_DefaultRawWcsdup(program_name); } - PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); - if (has_value && _Py_path_config.program_name == NULL) { path_out_of_memory(__func__); } diff --git a/Python/pystate.c b/Python/pystate.c index 4caef2260ae58e..09b83cdeb1f42d 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -16,7 +16,7 @@ #include "pycore_parking_lot.h" // _PyParkingLot_AfterFork() #include "pycore_pyerrors.h" // _PyErr_Clear() #include "pycore_pylifecycle.h" // _PyAST_Fini() -#include "pycore_pymem.h" // _PyMem_SetDefaultAllocator() +#include "pycore_pymem.h" // _PyMem_DebugEnabled() #include "pycore_pystate.h" #include "pycore_runtime_init.h" // _PyRuntimeState_INIT #include "pycore_stackref.h" // Py_STACKREF_DEBUG diff --git a/Python/sysmodule.c b/Python/sysmodule.c index d5cb448eb618e8..50b027ab56ef4d 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -29,7 +29,7 @@ Data members: #include "pycore_pyerrors.h" // _PyErr_GetRaisedException() #include "pycore_pylifecycle.h" // _PyErr_WriteUnraisableDefaultHook() #include "pycore_pymath.h" // _PY_SHORT_FLOAT_REPR -#include "pycore_pymem.h" // _PyMem_SetDefaultAllocator() +#include "pycore_pymem.h" // _PyMem_DefaultRawFree() #include "pycore_pystate.h" // _PyThreadState_GET() #include "pycore_pystats.h" // _Py_PrintSpecializationStats() #include "pycore_structseq.h" // _PyStructSequence_InitBuiltinWithFlags() @@ -2724,22 +2724,17 @@ _alloc_preinit_entry(const wchar_t *value) /* To get this to work, we have to initialize the runtime implicitly */ _PyRuntime_Initialize(); - /* Force default allocator, so we can ensure that it also gets used to + /* Use the default allocator, so we can ensure that it also gets used to * destroy the linked list in _clear_preinit_entries. */ - PyMemAllocatorEx old_alloc; - _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc); - - _Py_PreInitEntry node = PyMem_RawCalloc(1, sizeof(*node)); + _Py_PreInitEntry node = _PyMem_DefaultRawCalloc(1, sizeof(*node)); if (node != NULL) { - node->value = _PyMem_RawWcsdup(value); + node->value = _PyMem_DefaultRawWcsdup(value); if (node->value == NULL) { - PyMem_RawFree(node); + _PyMem_DefaultRawFree(node); node = NULL; }; }; - - PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); return node; } @@ -2771,15 +2766,12 @@ _clear_preinit_entries(_Py_PreInitEntry *optionlist) _Py_PreInitEntry current = *optionlist; *optionlist = NULL; /* Deallocate the nodes and their contents using the default allocator */ - PyMemAllocatorEx old_alloc; - _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc); while (current != NULL) { _Py_PreInitEntry next = current->next; - PyMem_RawFree(current->value); - PyMem_RawFree(current); + _PyMem_DefaultRawFree(current->value); + _PyMem_DefaultRawFree(current); current = next; } - PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); } diff --git a/Tools/tsan/suppressions.txt b/Tools/tsan/suppressions.txt index 22ba9d6ba2ab4d..6bda5ecd570889 100644 --- a/Tools/tsan/suppressions.txt +++ b/Tools/tsan/suppressions.txt @@ -1,7 +1,5 @@ # This file contains suppressions for the default (with GIL) build. # reference: https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions -race:get_allocator_unlocked -race:set_allocator_unlocked # https://gist.github.com/mpage/daaf32b39180c1989572957b943eb665 thread:pthread_create diff --git a/Tools/tsan/suppressions_free_threading.txt b/Tools/tsan/suppressions_free_threading.txt index 1802473fea3fcc..3354b5756811c9 100644 --- a/Tools/tsan/suppressions_free_threading.txt +++ b/Tools/tsan/suppressions_free_threading.txt @@ -4,11 +4,6 @@ # # reference: https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions -## Default build suppresssions - -race:get_allocator_unlocked -race:set_allocator_unlocked - ## Free-threaded suppressions @@ -38,8 +33,6 @@ race_top:tstate_is_freed race_top:type_modified_unlocked race_top:write_thread_id race_top:PyThreadState_Clear -# Only seen on macOS, sample: https://gist.github.com/aisk/dda53f5d494a4556c35dde1fce03259c -race_top:set_default_allocator_unlocked # gh-129068: race on shared range iterators (test_free_threading.test_zip.ZipThreading.test_threading) race_top:rangeiter_next From 69426fcee7fcecbe34be66d2c5b58b6d0ffe2809 Mon Sep 17 00:00:00 2001 From: Sergey Miryanov Date: Thu, 20 Feb 2025 22:05:39 +0500 Subject: [PATCH 3/4] gh-130052: Fix some exceptions on error paths in _testexternalinspection (#130053) Co-authored-by: Victor Stinner --- Modules/_testexternalinspection.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/Modules/_testexternalinspection.c b/Modules/_testexternalinspection.c index 77984460400c5d..fcb18aeef08c39 100644 --- a/Modules/_testexternalinspection.c +++ b/Modules/_testexternalinspection.c @@ -133,6 +133,10 @@ return_section_address( cmd = (struct segment_command_64*)((void*)cmd + cmd->cmdsize); } + + // We should not be here, but if we are there, we should say about this + PyErr_SetString( + PyExc_RuntimeError, "Cannot find section address.\n"); return 0; } @@ -188,6 +192,8 @@ search_section_in_file( munmap(map, fs.st_size); if (close(fd) != 0) { + // This might hide one of the above exceptions, maybe we + // should chain them? PyErr_SetFromErrno(PyExc_OSError); } return result; @@ -217,7 +223,6 @@ search_map_for_section(pid_t pid, const char* secname, const char* substr) { mach_port_t proc_ref = pid_to_task(pid); if (proc_ref == 0) { - PyErr_SetString(PyExc_PermissionError, "Cannot get task for PID"); return 0; } @@ -260,6 +265,9 @@ search_map_for_section(pid_t pid, const char* secname, const char* substr) { address += size; } + + PyErr_SetString(PyExc_RuntimeError, + "mach_vm_region failed to find the section"); return 0; } @@ -306,6 +314,8 @@ find_map_start_address(pid_t pid, char* result_filename, const char* map) if (!match_found) { map_filename[0] = '\0'; + PyErr_Format(PyExc_RuntimeError, + "Cannot find map start address for map: %s", map); } return result_address; @@ -401,6 +411,8 @@ search_map_for_section(pid_t pid, const char* secname, const char* map) static uintptr_t search_map_for_section(pid_t pid, const char* secname, const char* map) { + PyErr_SetString(PyExc_NotImplementedError, + "Not supported on this platform"); return 0; } #endif @@ -419,7 +431,8 @@ get_py_runtime(pid_t pid) static uintptr_t get_async_debug(pid_t pid) { - uintptr_t result = search_map_for_section(pid, "AsyncioDebug", "_asyncio.cpython"); + uintptr_t result = search_map_for_section(pid, "AsyncioDebug", + "_asyncio.cpython"); if (result == 0 && !PyErr_Occurred()) { PyErr_SetString(PyExc_RuntimeError, "Cannot find AsyncioDebug section"); } @@ -482,6 +495,9 @@ read_memory(pid_t pid, uintptr_t remote_address, size_t len, void* dst) } total_bytes_read = len; #else + PyErr_SetString( + PyExc_RuntimeError, + "Memory reading is not supported on this platform"); return -1; #endif return total_bytes_read; @@ -789,6 +805,9 @@ parse_coro_chain( pid, coro_address + offsets->gen_object.gi_frame_state, &gi_frame_state); + if (err) { + return -1; + } if (gi_frame_state == FRAME_SUSPENDED_YIELD_FROM) { char owner; From 6c450f44c283c61d0e1ada05ead9524a1fe97962 Mon Sep 17 00:00:00 2001 From: Dino Viehland Date: Thu, 20 Feb 2025 13:32:57 -0800 Subject: [PATCH 4/4] gh-130313: Avoid locking when clearing objects (#130126) Avoid locking when clearing objects in the free-threaded build --- Objects/dictobject.c | 117 +++++++++++++++++++++++++++---------------- 1 file changed, 73 insertions(+), 44 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 900d001d4dd56a..e30d626439e7f8 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -7163,6 +7163,17 @@ PyObject_VisitManagedDict(PyObject *obj, visitproc visit, void *arg) return 0; } +static void +clear_inline_values(PyDictValues *values) +{ + if (values->valid) { + FT_ATOMIC_STORE_UINT8(values->valid, 0); + for (Py_ssize_t i = 0; i < values->capacity; i++) { + Py_CLEAR(values->values[i]); + } + } +} + static void set_dict_inline_values(PyObject *obj, PyDictObject *new_dict) { @@ -7173,12 +7184,7 @@ set_dict_inline_values(PyObject *obj, PyDictObject *new_dict) Py_XINCREF(new_dict); FT_ATOMIC_STORE_PTR(_PyObject_ManagedDictPointer(obj)->dict, new_dict); - if (values->valid) { - FT_ATOMIC_STORE_UINT8(values->valid, 0); - for (Py_ssize_t i = 0; i < values->capacity; i++) { - Py_CLEAR(values->values[i]); - } - } + clear_inline_values(values); } #ifdef Py_GIL_DISABLED @@ -7256,8 +7262,8 @@ decref_maybe_delay(PyObject *obj, bool delay) } } -static int -set_or_clear_managed_dict(PyObject *obj, PyObject *new_dict, bool clear) +int +_PyObject_SetManagedDict(PyObject *obj, PyObject *new_dict) { assert(Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT); #ifndef NDEBUG @@ -7292,8 +7298,7 @@ set_or_clear_managed_dict(PyObject *obj, PyObject *new_dict, bool clear) // Decref for the dictionary we incref'd in try_set_dict_inline_only_or_other_dict // while the object was locked - decref_maybe_delay((PyObject *)prev_dict, - !clear && prev_dict != cur_dict); + decref_maybe_delay((PyObject *)prev_dict, prev_dict != cur_dict); if (err != 0) { return err; } @@ -7303,7 +7308,7 @@ set_or_clear_managed_dict(PyObject *obj, PyObject *new_dict, bool clear) if (prev_dict != NULL) { // decref for the dictionary that we replaced - decref_maybe_delay((PyObject *)prev_dict, !clear); + decref_maybe_delay((PyObject *)prev_dict, true); } return 0; @@ -7333,45 +7338,15 @@ set_or_clear_managed_dict(PyObject *obj, PyObject *new_dict, bool clear) (PyDictObject *)Py_XNewRef(new_dict)); Py_END_CRITICAL_SECTION(); - decref_maybe_delay((PyObject *)dict, !clear); + decref_maybe_delay((PyObject *)dict, true); } assert(_PyObject_InlineValuesConsistencyCheck(obj)); return err; } -int -_PyObject_SetManagedDict(PyObject *obj, PyObject *new_dict) -{ - return set_or_clear_managed_dict(obj, new_dict, false); -} - -void -PyObject_ClearManagedDict(PyObject *obj) -{ - if (set_or_clear_managed_dict(obj, NULL, true) < 0) { - /* Must be out of memory */ - assert(PyErr_Occurred() == PyExc_MemoryError); - PyErr_FormatUnraisable("Exception ignored while " - "clearing an object managed dict"); - /* Clear the dict */ - PyDictObject *dict = _PyObject_GetManagedDict(obj); - Py_BEGIN_CRITICAL_SECTION2(dict, obj); - dict = _PyObject_ManagedDictPointer(obj)->dict; - PyInterpreterState *interp = _PyInterpreterState_GET(); - PyDictKeysObject *oldkeys = dict->ma_keys; - set_keys(dict, Py_EMPTY_KEYS); - dict->ma_values = NULL; - dictkeys_decref(interp, oldkeys, IS_DICT_SHARED(dict)); - STORE_USED(dict, 0); - set_dict_inline_values(obj, NULL); - Py_END_CRITICAL_SECTION2(); - } -} - -int -_PyDict_DetachFromObject(PyDictObject *mp, PyObject *obj) +static int +detach_dict_from_object(PyDictObject *mp, PyObject *obj) { - ASSERT_WORLD_STOPPED_OR_OBJ_LOCKED(obj); assert(_PyObject_ManagedDictPointer(obj)->dict == mp); assert(_PyObject_InlineValuesConsistencyCheck(obj)); @@ -7401,6 +7376,60 @@ _PyDict_DetachFromObject(PyDictObject *mp, PyObject *obj) return 0; } + +void +PyObject_ClearManagedDict(PyObject *obj) +{ + // This is called when the object is being freed or cleared + // by the GC and therefore known to have no references. + if (Py_TYPE(obj)->tp_flags & Py_TPFLAGS_INLINE_VALUES) { + PyDictObject *dict = _PyObject_GetManagedDict(obj); + if (dict == NULL) { + // We have no materialized dictionary and inline values + // that just need to be cleared. + // No dict to clear, we're done + clear_inline_values(_PyObject_InlineValues(obj)); + return; + } + else if (FT_ATOMIC_LOAD_PTR_RELAXED(dict->ma_values) == + _PyObject_InlineValues(obj)) { + // We have a materialized object which points at the inline + // values. We need to materialize the keys. Nothing can modify + // this object, but we need to lock the dictionary. + int err; + Py_BEGIN_CRITICAL_SECTION(dict); + err = detach_dict_from_object(dict, obj); + Py_END_CRITICAL_SECTION(); + + if (err) { + /* Must be out of memory */ + assert(PyErr_Occurred() == PyExc_MemoryError); + PyErr_FormatUnraisable("Exception ignored while " + "clearing an object managed dict"); + /* Clear the dict */ + Py_BEGIN_CRITICAL_SECTION(dict); + PyInterpreterState *interp = _PyInterpreterState_GET(); + PyDictKeysObject *oldkeys = dict->ma_keys; + set_keys(dict, Py_EMPTY_KEYS); + dict->ma_values = NULL; + dictkeys_decref(interp, oldkeys, IS_DICT_SHARED(dict)); + STORE_USED(dict, 0); + clear_inline_values(_PyObject_InlineValues(obj)); + Py_END_CRITICAL_SECTION(); + } + } + } + Py_CLEAR(_PyObject_ManagedDictPointer(obj)->dict); +} + +int +_PyDict_DetachFromObject(PyDictObject *mp, PyObject *obj) +{ + ASSERT_WORLD_STOPPED_OR_OBJ_LOCKED(obj); + + return detach_dict_from_object(mp, obj); +} + static inline PyObject * ensure_managed_dict(PyObject *obj) {