From 1b161377be460955b46e7d632ed804ef54c266c7 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 16 Dec 2023 02:22:05 +0900 Subject: [PATCH 1/4] gh-112535: Implement fallback implementation of _Py_ThreadId() --- Include/object.h | 9 +++++++++ Python/pystate.c | 10 ++++++++++ 2 files changed, 19 insertions(+) diff --git a/Include/object.h b/Include/object.h index d22e5c2b8be2a9..6afdc1688ee352 100644 --- a/Include/object.h +++ b/Include/object.h @@ -239,6 +239,8 @@ PyAPI_FUNC(int) Py_Is(PyObject *x, PyObject *y); #define Py_Is(x, y) ((x) == (y)) #if defined(Py_GIL_DISABLED) && !defined(Py_LIMITED_API) +PyAPI_FUNC(uintptr_t) _Py_TSS_Tstate_Addr(void); + static inline uintptr_t _Py_ThreadId(void) { @@ -290,6 +292,13 @@ _Py_ThreadId(void) // tp is Thread Pointer provided by the RISC-V ABI. __asm__ ("mv %0, tp" : "=r" (tid)); #endif +#elif defined(thread_local) || defined(__GNUC__) + // gh-112535: Using characteristics of TLS address mapping. + // The address of the thread-local variable is not equal with the actual thread pointer, + // However, it has the property of being determined by loader at runtime, with no duplication of values + // between different threads. So it can be used as the similar role of __builtin_thread_pointer(). + // But since it requires offset calculation, this hack is more expensive than __builtin_thread_pointer(). + tid = _Py_TSS_Tstate_Addr(); #else # error "define _Py_ThreadId for this platform" #endif diff --git a/Python/pystate.c b/Python/pystate.c index e18eb0186d0010..fadf76873d6daa 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1951,6 +1951,16 @@ _PyThreadState_Bind(PyThreadState *tstate) } } +#if defined(Py_GIL_DISABLED) +uintptr_t +_Py_TSS_Tstate_Addr(void) { +#ifdef HAVE_THREAD_LOCAL + return (uintptr_t)&_Py_tss_tstate; +#else +# error "no supported thread-local variable storage classifier" +#endif +} +#endif /***********************************/ /* routines for advanced debuggers */ From 8cb8f2772afbe8ee0a3dadbffaee99803f70cdb3 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 16 Dec 2023 03:11:43 +0900 Subject: [PATCH 2/4] Rename --- Include/object.h | 4 ++-- Python/pystate.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Include/object.h b/Include/object.h index 6afdc1688ee352..0c4d9e8275cd6f 100644 --- a/Include/object.h +++ b/Include/object.h @@ -239,7 +239,7 @@ PyAPI_FUNC(int) Py_Is(PyObject *x, PyObject *y); #define Py_Is(x, y) ((x) == (y)) #if defined(Py_GIL_DISABLED) && !defined(Py_LIMITED_API) -PyAPI_FUNC(uintptr_t) _Py_TSS_Tstate_Addr(void); +PyAPI_FUNC(uintptr_t) _Py_GetThreadLocal_Addr(void); static inline uintptr_t _Py_ThreadId(void) @@ -298,7 +298,7 @@ _Py_ThreadId(void) // However, it has the property of being determined by loader at runtime, with no duplication of values // between different threads. So it can be used as the similar role of __builtin_thread_pointer(). // But since it requires offset calculation, this hack is more expensive than __builtin_thread_pointer(). - tid = _Py_TSS_Tstate_Addr(); + tid = _Py_GetThreadLocal_Addr(); #else # error "define _Py_ThreadId for this platform" #endif diff --git a/Python/pystate.c b/Python/pystate.c index fadf76873d6daa..795edb3a7cb9f2 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1951,9 +1951,9 @@ _PyThreadState_Bind(PyThreadState *tstate) } } -#if defined(Py_GIL_DISABLED) +#if defined(Py_GIL_DISABLED) && !defined(Py_LIMITED_API) uintptr_t -_Py_TSS_Tstate_Addr(void) { +_Py_GetThreadLocal_Addr(void) { #ifdef HAVE_THREAD_LOCAL return (uintptr_t)&_Py_tss_tstate; #else From 05cac38bc4fef1bc9478700725aae58cc16367f2 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 19 Dec 2023 01:27:20 +0900 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: Sam Gross --- Include/object.h | 7 ++----- Python/pystate.c | 6 +++++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Include/object.h b/Include/object.h index 0c4d9e8275cd6f..41301760eaeb53 100644 --- a/Include/object.h +++ b/Include/object.h @@ -293,11 +293,8 @@ _Py_ThreadId(void) __asm__ ("mv %0, tp" : "=r" (tid)); #endif #elif defined(thread_local) || defined(__GNUC__) - // gh-112535: Using characteristics of TLS address mapping. - // The address of the thread-local variable is not equal with the actual thread pointer, - // However, it has the property of being determined by loader at runtime, with no duplication of values - // between different threads. So it can be used as the similar role of __builtin_thread_pointer(). - // But since it requires offset calculation, this hack is more expensive than __builtin_thread_pointer(). + // Fallback to a portable implementation if we do not have a faster + // platform-specific implementation. tid = _Py_GetThreadLocal_Addr(); #else # error "define _Py_ThreadId for this platform" diff --git a/Python/pystate.c b/Python/pystate.c index 795edb3a7cb9f2..632a119ea6d4f8 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1953,8 +1953,12 @@ _PyThreadState_Bind(PyThreadState *tstate) #if defined(Py_GIL_DISABLED) && !defined(Py_LIMITED_API) uintptr_t -_Py_GetThreadLocal_Addr(void) { +_Py_GetThreadLocal_Addr(void) +{ #ifdef HAVE_THREAD_LOCAL + // gh-112535: Use the address of the thread-local PyThreadState variable as + // a unique identifier for the current thread. Each thread has a unique + // _Py_tss_tstate variable with a unique address. return (uintptr_t)&_Py_tss_tstate; #else # error "no supported thread-local variable storage classifier" From a1551d35df0890ae84356f028d828bc7ab52f077 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 19 Dec 2023 01:29:42 +0900 Subject: [PATCH 4/4] Address code review --- Include/object.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Include/object.h b/Include/object.h index 41301760eaeb53..48f1ddf7510887 100644 --- a/Include/object.h +++ b/Include/object.h @@ -292,12 +292,10 @@ _Py_ThreadId(void) // tp is Thread Pointer provided by the RISC-V ABI. __asm__ ("mv %0, tp" : "=r" (tid)); #endif -#elif defined(thread_local) || defined(__GNUC__) +#else // Fallback to a portable implementation if we do not have a faster // platform-specific implementation. tid = _Py_GetThreadLocal_Addr(); -#else - # error "define _Py_ThreadId for this platform" #endif return tid; }