From 80d607259897724bebfe31ab68e4b86dffd324e4 Mon Sep 17 00:00:00 2001 From: "Gabriele N. Tornetta" Date: Sun, 20 Aug 2023 16:10:40 +0100 Subject: [PATCH] fix GIL holder inference for memory attributions --- src/argparse.c | 2 +- src/py_proc.c | 16 +++++++++++++-- src/py_thread.c | 4 +++- src/py_thread.h | 2 ++ src/python/gil.h | 11 ++-------- src/python/interp.h | 10 ++++++++- src/python/thread.h | 50 +++++++++++++++++++++++---------------------- src/version.h | 25 +++++++++++++++++++++-- test/test_cli.py | 2 +- 9 files changed, 81 insertions(+), 41 deletions(-) diff --git a/src/argparse.c b/src/argparse.c index ba49810b..9f090f1b 100644 --- a/src/argparse.c +++ b/src/argparse.c @@ -45,7 +45,7 @@ #else #define DEFAULT_SAMPLING_INTERVAL 100 #endif -#define DEFAULT_INIT_TIMEOUT_MS 500 // 0.5 seconds +#define DEFAULT_INIT_TIMEOUT_MS 1000 // 1 second #define DEFAULT_HEAP_SIZE 0 const char SAMPLE_FORMAT_NORMAL[] = ";%s:%s:%d"; diff --git a/src/py_proc.c b/src/py_proc.c index 43b9070a..bcccfa79 100644 --- a/src/py_proc.c +++ b/src/py_proc.c @@ -1215,13 +1215,25 @@ py_proc__sample(py_proc_t * self) { if (pargs.memory) { // Use the current thread to determine which thread is manipulating memory - current_thread = _py_proc__get_current_thread_state_raddr(self); + if (V_MIN(3, 12)) { + void * gil_state_raddr = V_FIELD(void *, is, py_is, o_gil_state); + if (!isvalid(gil_state_raddr)) + SUCCESS; + gil_state_t gil_state; + if (fail(copy_datatype(self->proc_ref, gil_state_raddr, gil_state))) { + log_ie("Failed to copy GIL state"); + FAIL; + } + current_thread = (void *) gil_state.last_holder._value; + } + else + current_thread = _py_proc__get_current_thread_state_raddr(self); } do { if (pargs.memory) { mem_delta = 0; - if (self->symbols[DYNSYM_RUNTIME] != NULL && current_thread == (void *) -1) { + if (V_MAX(3, 11) && self->symbols[DYNSYM_RUNTIME] != NULL && current_thread == (void *) -1) { if (_py_proc__find_current_thread_offset(self, py_thread.raddr.addr)) continue; else diff --git a/src/py_thread.c b/src/py_thread.c index 4a364ad2..7feb6dd0 100644 --- a/src/py_thread.c +++ b/src/py_thread.c @@ -779,7 +779,9 @@ py_thread__fill_from_raddr(py_thread_t * self, raddr_t * raddr, py_proc_t * proc self->raddr = *raddr; self->top_frame = V_FIELD(void*, ts, py_thread, o_frame); - + + self->status = V_FIELD(tstate_status_t, ts, py_thread, o_status); + self->next_raddr = (raddr_t) { raddr->pref, V_FIELD(void*, ts, py_thread, o_next) == raddr->addr \ diff --git a/src/py_thread.h b/src/py_thread.h index 1d2d5187..48b3b807 100644 --- a/src/py_thread.h +++ b/src/py_thread.h @@ -61,6 +61,8 @@ typedef struct thread { /* The per-thread datastack was introduced in Python 3.11 */ void * stack; size_t stack_size; + + tstate_status_t status; } py_thread_t; diff --git a/src/python/gil.h b/src/python/gil.h index f9d8d317..3be5e8a5 100644 --- a/src/python/gil.h +++ b/src/python/gil.h @@ -42,15 +42,8 @@ struct _gil_runtime_state3_11 { unsigned long interval; _Py_atomic_address last_holder; _Py_atomic_int locked; - unsigned long switch_number; - PyCOND_T cond; - PyMUTEX_T mutex; -#ifdef FORCE_SWITCHING - /* This condition variable helps the GIL-releasing thread wait for - a GIL-awaiting thread to be scheduled and take the GIL. */ - PyCOND_T switch_cond; - PyMUTEX_T switch_mutex; -#endif }; +typedef struct _gil_runtime_state3_11 gil_state_t; + #endif diff --git a/src/python/interp.h b/src/python/interp.h index e24a9848..4454b1ef 100644 --- a/src/python/interp.h +++ b/src/python/interp.h @@ -149,7 +149,7 @@ typedef struct { size_t stacksize; } threads; - struct pyruntimestate *runtime; + void *runtime; _Py_atomic_address _finalizing; @@ -160,6 +160,14 @@ typedef struct { // Dictionary of the builtins module PyObject *builtins; + + struct { + _Py_atomic_int eval_breaker; + _Py_atomic_int gil_drop_request; + int recursion_limit; + void *gil; + // ... + } ceval; } PyInterpreterState3_12; diff --git a/src/python/thread.h b/src/python/thread.h index f5f2ac36..c548557e 100644 --- a/src/python/thread.h +++ b/src/python/thread.h @@ -148,6 +148,31 @@ typedef struct _ts3_11 { } PyThreadState3_11; +typedef struct { + /* Has been initialized to a safe state. + + In order to be effective, this must be set to 0 during or right + after allocation. */ + unsigned int initialized:1; + + /* Has been bound to an OS thread. */ + unsigned int bound:1; + /* Has been unbound from its OS thread. */ + unsigned int unbound:1; + /* Has been bound aa current for the GILState API. */ + unsigned int bound_gilstate:1; + /* Currently in use (maybe holds the GIL). */ + unsigned int active:1; + + /* various stages of finalization */ + unsigned int finalizing:1; + unsigned int cleared:1; + unsigned int finalized:1; + + /* padding to align to 4 bytes */ + unsigned int :24; +} tstate_status_t; + typedef struct { /* See Python/ceval.c for comments explaining most fields */ @@ -155,30 +180,7 @@ typedef struct { void *next; void *interp; - struct { - /* Has been initialized to a safe state. - - In order to be effective, this must be set to 0 during or right - after allocation. */ - unsigned int initialized:1; - - /* Has been bound to an OS thread. */ - unsigned int bound:1; - /* Has been unbound from its OS thread. */ - unsigned int unbound:1; - /* Has been bound aa current for the GILState API. */ - unsigned int bound_gilstate:1; - /* Currently in use (maybe holds the GIL). */ - unsigned int active:1; - - /* various stages of finalization */ - unsigned int finalizing:1; - unsigned int cleared:1; - unsigned int finalized:1; - - /* padding to align to 4 bytes */ - unsigned int :24; - } _status; + tstate_status_t _status; int py_recursion_remaining; int py_recursion_limit; diff --git a/src/version.h b/src/version.h index 69fbeae2..5610978a 100644 --- a/src/version.h +++ b/src/version.h @@ -135,6 +135,7 @@ typedef struct { offset_t o_thread_id; offset_t o_native_thread_id; offset_t o_stack; + offset_t o_status; } py_thread_v; @@ -162,6 +163,7 @@ typedef struct { offset_t o_next; offset_t o_tstate_head; offset_t o_gc; + offset_t o_gil_state; } py_is_v; @@ -264,6 +266,18 @@ typedef struct { offsetof(s, datastack_chunk), \ } +#define PY_THREAD_312(s) { \ + sizeof(s), \ + offsetof(s, prev), \ + offsetof(s, next), \ + offsetof(s, interp), \ + offsetof(s, cframe), \ + offsetof(s, thread_id), \ + offsetof(s, native_thread_id), \ + offsetof(s, datastack_chunk), \ + offsetof(s, _status) \ +} + #define PY_UNICODE(n) { \ n \ } @@ -298,6 +312,13 @@ typedef struct { offsetof(s, gc), \ } +#define PY_IS_312(s) { \ + sizeof(s), \ + offsetof(s, next), \ + offsetof(s, threads.head), \ + offsetof(s, gc), \ + offsetof(s, ceval.gil), \ +} #define PY_GC(s) { \ sizeof(s), \ @@ -356,8 +377,8 @@ python_v python_v3_11 = { python_v python_v3_12 = { PY_CODE_311 (PyCodeObject3_12), PY_FRAME (PyFrameObject3_10), // Irrelevant - PY_THREAD_311 (PyThreadState3_12), - PY_IS_311 (PyInterpreterState3_12), + PY_THREAD_312 (PyThreadState3_12), + PY_IS_312 (PyInterpreterState3_12), PY_RUNTIME_311 (_PyRuntimeState3_12), PY_GC (struct _gc_runtime_state3_12), PY_CFRAME_311 (_PyCFrame3_12), diff --git a/test/test_cli.py b/test/test_cli.py index 40067dde..6b4ea8e6 100644 --- a/test/test_cli.py +++ b/test/test_cli.py @@ -41,7 +41,7 @@ def test_cli_no_python(): "-C", "powershell" if platform.system() == "Windows" else "bash", "-c", - "sleep 1", + "sleep 2", ) assert result.returncode == 39 assert "not a Python" in result.stderr or "Cannot launch" in result.stderr