From f76d2c50c171242a1321f699cb448369052c1251 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 14 Apr 2023 10:12:45 -0700 Subject: [PATCH 01/56] Update osmodule.h --- Include/osmodule.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Include/osmodule.h b/Include/osmodule.h index 9095c2fdd3d638..e1ee42919af2a6 100644 --- a/Include/osmodule.h +++ b/Include/osmodule.h @@ -10,7 +10,18 @@ extern "C" { #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03060000 PyAPI_FUNC(PyObject *) PyOS_FSPath(PyObject *path); #endif + +typedef struct { + FILE* perf_map; + PyThread_type_lock map_lock; +} PerfMapState; +PyAPI_FUNC(void *) PyOS_PerfMapState_Init(void); + +PyAPI_FUNC(int) PyOS_WritePerfMapEntry(const void *code_addr, unsigned int code_size, const char *entry_name); + +PyAPI_FUNC(void) PyOS_PerfMapState_Fini(void); + #ifdef __cplusplus } #endif From de6b63ee3cd604cb35cc8cea714993ca4294f2c4 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 14 Apr 2023 10:15:31 -0700 Subject: [PATCH 02/56] Update posixmodule.c --- Modules/posixmodule.c | 50 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index dd150107e4a9de..9e8154006a51c1 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -16733,6 +16733,56 @@ INITFUNC(void) return PyModuleDef_Init(&posixmodule); } +static PerfMapState perf_map_state; + +PyAPI_FUNC(void *) PyOS_PerfMapState_Init(void) { + char filename[100]; + pid_t pid = getpid(); + // Use nofollow flag to prevent symlink attacks. + int flags = O_WRONLY | O_CREAT | O_APPEND | O_NOFOLLOW | O_CLOEXEC; + snprintf(filename, sizeof(filename) - 1, "/tmp/perf-%jd.map", + (intmax_t)pid); + int fd = open(filename, flags, 0600); + if (fd == -1) { + PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); + return NULL; + } + else{ + perf_map_state.perf_map = fdopen(fd, "a"); + if (perf_map_state.perf_map == NULL) { + PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); + return NULL; + } + } + perf_map_state.map_lock = PyThread_allocate_lock(); + return &perf_map_state; +} + +PyAPI_FUNC(int) PyOS_WritePerfMapEntry( + const void *code_addr, + unsigned int code_size, + const char *entry_name +) { + if (perf_map_state.perf_map == NULL) { + if(PyOS_PerfMapState_Init() == NULL){ + return -1; + } + } + PyThread_acquire_lock(perf_map_state.map_lock, 1); + fprintf(perf_map_state.perf_map, "%p %x %s\n", code_addr, code_size, entry_name); + fflush(perf_map_state.perf_map); + PyThread_release_lock(perf_map_state.map_lock); + return 0; +} + +PyAPI_FUNC(void) PyOS_PerfMapState_Fini(void) { + if (perf_map_state.perf_map != NULL) { + PyThread_free_lock(perf_map_state.map_lock); + fclose(perf_map_state.perf_map); + perf_map_state.perf_map = NULL; + } +} + #ifdef __cplusplus } #endif From 4f55b2d0e6bb52f4bfc12e5a3903f2fa795635e3 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 14 Apr 2023 10:45:33 -0700 Subject: [PATCH 03/56] Update pycore_ceval_state.h --- Include/internal/pycore_ceval_state.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/internal/pycore_ceval_state.h b/Include/internal/pycore_ceval_state.h index 9ba42eb03b2676..4050ed48ed9fc3 100644 --- a/Include/internal/pycore_ceval_state.h +++ b/Include/internal/pycore_ceval_state.h @@ -27,7 +27,7 @@ struct trampoline_api_st { void* (*init_state)(void); void (*write_state)(void* state, const void *code_addr, unsigned int code_size, PyCodeObject* code); - int (*free_state)(void* state); + void (*free_state)(void); void *state; }; #endif From 03fccfbc5b254f412892433f7fc66b0eb45f65c6 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 14 Apr 2023 10:53:58 -0700 Subject: [PATCH 04/56] Update perf_trampoline.c --- Python/perf_trampoline.c | 65 ++++++++++------------------------------ 1 file changed, 15 insertions(+), 50 deletions(-) diff --git a/Python/perf_trampoline.c b/Python/perf_trampoline.c index 1957ab82c33951..b6025563466913 100644 --- a/Python/perf_trampoline.c +++ b/Python/perf_trampoline.c @@ -196,66 +196,31 @@ typedef struct trampoline_api_st trampoline_api_t; static void * perf_map_get_file(void) { - if (perf_map_file) { - return perf_map_file; - } - char filename[100]; - pid_t pid = getpid(); - // Location and file name of perf map is hard-coded in perf tool. - // Use exclusive create flag wit nofollow to prevent symlink attacks. - int flags = O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC; - snprintf(filename, sizeof(filename) - 1, "/tmp/perf-%jd.map", - (intmax_t)pid); - int fd = open(filename, flags, 0600); - if (fd == -1) { - perf_status = PERF_STATUS_FAILED; - PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); - return NULL; - } - perf_map_file = fdopen(fd, "w"); - if (!perf_map_file) { - perf_status = PERF_STATUS_FAILED; - PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); - close(fd); - return NULL; - } - return perf_map_file; + return PyOS_PerfMapState_Init();; } -static int -perf_map_close(void *state) +static void +perf_map_close(void) { - FILE *fp = (FILE *)state; - int ret = 0; - if (fp) { - ret = fclose(fp); - } - perf_map_file = NULL; - perf_status = PERF_STATUS_NO_INIT; - return ret; + PyOS_PerfMapState_Fini(); } static void perf_map_write_entry(void *state, const void *code_addr, unsigned int code_size, PyCodeObject *co) { - assert(state != NULL); - FILE *method_file = (FILE *)state; - const char *entry = PyUnicode_AsUTF8(co->co_qualname); - if (entry == NULL) { - _PyErr_WriteUnraisableMsg("Failed to get qualname from code object", - NULL); - return; + assert(state != NULL); + const char *entry = ""; + if (co->co_qualname != NULL) { + entry = PyUnicode_AsUTF8(co->co_qualname); } - const char *filename = PyUnicode_AsUTF8(co->co_filename); - if (filename == NULL) { - _PyErr_WriteUnraisableMsg("Failed to get filename from code object", - NULL); - return; + const char *filename = ""; + if (co->co_filename != NULL) { + filename = PyUnicode_AsUTF8(co->co_filename); } - fprintf(method_file, "%p %x py::%s:%s\n", code_addr, code_size, entry, - filename); - fflush(method_file); + char perf_map_entry[500]; + snprintf(perf_map_entry, sizeof(perf_map_entry), "py::%s:%s", entry, filename); + PyOS_WritePerfMapEntry(code_addr, code_size, perf_map_entry); } _PyPerf_Callbacks _Py_perfmap_callbacks = { @@ -492,7 +457,7 @@ _PyPerfTrampoline_Fini(void) } free_code_arenas(); if (trampoline_api.state != NULL) { - trampoline_api.free_state(trampoline_api.state); + trampoline_api.free_state(); trampoline_api.state = NULL; } extra_code_index = -1; From 78d543adbc58877c27edf92631ab7a4b479e3926 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 14 Apr 2023 10:57:05 -0700 Subject: [PATCH 05/56] Update pylifecycle.c --- Python/pylifecycle.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index d6627bc6b7e86b..0846accf1062cf 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1778,6 +1778,7 @@ Py_FinalizeEx(void) */ _PyAtExit_Call(tstate->interp); + PyOS_PerfMapState_Fini(); /* Copy the core config, PyInterpreterState_Delete() free the core config memory */ From e8c321d854da5826f08baadd70781df725d1c91b Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 14 Apr 2023 10:58:41 -0700 Subject: [PATCH 06/56] Update pycore_ceval.h --- Include/internal/pycore_ceval.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index deda070a6dea79..1d91081aabca51 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -67,7 +67,7 @@ typedef struct { void (*write_state)(void* state, const void *code_addr, unsigned int code_size, PyCodeObject* code); // Callback to free the trampoline state - int (*free_state)(void* state); + void (*free_state)(void); } _PyPerf_Callbacks; extern int _PyPerfTrampoline_SetCallbacks(_PyPerf_Callbacks *); From 3a979338092be323d2423244889d9ee65a569b9a Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 14 Apr 2023 11:02:27 -0700 Subject: [PATCH 07/56] Update _testinternalcapi.c --- Modules/_testinternalcapi.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 632fac2de0c419..31c4b8c79f0e7d 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -684,6 +684,23 @@ clear_extension(PyObject *self, PyObject *args) Py_RETURN_NONE; } +static PyObject * +write_perf_map_entry(PyObject *self, PyObject *args) +{ + const void *code_addr; + unsigned int code_size; + const char *entry_name; + + if (!PyArg_ParseTuple(args, "KIs", &code_addr, &code_size, &entry_name)) + return NULL; + + int ret = PyOS_WritePerfMapEntry(code_addr, code_size, entry_name); + if (ret == -1) { + PyErr_SetString(PyExc_OSError, "Failed to write performance map entry"); + return NULL; + } + return Py_BuildValue("i", ret); +} static PyMethodDef module_functions[] = { {"get_configs", get_configs, METH_NOARGS}, @@ -707,6 +724,7 @@ static PyMethodDef module_functions[] = { _TESTINTERNALCAPI_OPTIMIZE_CFG_METHODDEF {"get_interp_settings", get_interp_settings, METH_VARARGS, NULL}, {"clear_extension", clear_extension, METH_VARARGS, NULL}, + {"write_perf_map_entry", write_perf_map_entry, METH_VARARGS}, {NULL, NULL} /* sentinel */ }; From 94441c45b737de8118066d5432ccff41665ef9df Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 14 Apr 2023 11:04:24 -0700 Subject: [PATCH 08/56] Create test_perfmaps.py --- Lib/test/test_perfmaps.py | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 Lib/test/test_perfmaps.py diff --git a/Lib/test/test_perfmaps.py b/Lib/test/test_perfmaps.py new file mode 100644 index 00000000000000..e9198c3fa62db3 --- /dev/null +++ b/Lib/test/test_perfmaps.py @@ -0,0 +1,11 @@ +import os +import unittest + + +class TestPerfMapWriting(unittest.TestCase): + def test_write_perf_map_entry(self): + from _testinternalcapi import write_perf_map_entry + self.assertEqual(write_perf_map_entry(0x1234, 0x5678, "entry1"), 0) + self.assertEqual(write_perf_map_entry(0x2345, 0x6789, "entry2"), 0) + with open(f"/tmp/perf-{os.getpid()}.map") as f: + self.assertEqual(f.read().strip(), "0x1234 5678 entry1\n0x2345 6789 entry2") From a221fd80417b8729a5b46d4dab510f05c23f3390 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 14 Apr 2023 12:26:30 -0700 Subject: [PATCH 09/56] Update posixmodule.c --- Modules/posixmodule.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 9e8154006a51c1..5a5af19b91ee20 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -16736,6 +16736,11 @@ INITFUNC(void) static PerfMapState perf_map_state; PyAPI_FUNC(void *) PyOS_PerfMapState_Init(void) { + #ifdef MS_WINDOWS + PyErr_SetString(PyExc_RuntimeError, "perf map is not supported on Windows"); + return NULL; + #endif + char filename[100]; pid_t pid = getpid(); // Use nofollow flag to prevent symlink attacks. From 392e842852430da7e92323f84c41e56cf6ddd2a6 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 14 Apr 2023 12:27:20 -0700 Subject: [PATCH 10/56] Update osmodule.h --- Include/osmodule.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Include/osmodule.h b/Include/osmodule.h index e1ee42919af2a6..b509dc55e46e60 100644 --- a/Include/osmodule.h +++ b/Include/osmodule.h @@ -9,8 +9,7 @@ extern "C" { #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03060000 PyAPI_FUNC(PyObject *) PyOS_FSPath(PyObject *path); -#endif - + typedef struct { FILE* perf_map; PyThread_type_lock map_lock; @@ -21,6 +20,7 @@ PyAPI_FUNC(void *) PyOS_PerfMapState_Init(void); PyAPI_FUNC(int) PyOS_WritePerfMapEntry(const void *code_addr, unsigned int code_size, const char *entry_name); PyAPI_FUNC(void) PyOS_PerfMapState_Fini(void); +#endif #ifdef __cplusplus } From 42232413ea4cb9a14020819e2f7e184c6eb9cc61 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 14 Apr 2023 12:37:56 -0700 Subject: [PATCH 11/56] Update osmodule.h --- Include/osmodule.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/osmodule.h b/Include/osmodule.h index b509dc55e46e60..8e10c239465c74 100644 --- a/Include/osmodule.h +++ b/Include/osmodule.h @@ -9,6 +9,7 @@ extern "C" { #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03060000 PyAPI_FUNC(PyObject *) PyOS_FSPath(PyObject *path); +#endif typedef struct { FILE* perf_map; @@ -20,7 +21,6 @@ PyAPI_FUNC(void *) PyOS_PerfMapState_Init(void); PyAPI_FUNC(int) PyOS_WritePerfMapEntry(const void *code_addr, unsigned int code_size, const char *entry_name); PyAPI_FUNC(void) PyOS_PerfMapState_Fini(void); -#endif #ifdef __cplusplus } From 804153a3d519289cf6659e7372dd25d3638c9592 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 14 Apr 2023 12:39:17 -0700 Subject: [PATCH 12/56] Update osmodule.h --- Include/osmodule.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Include/osmodule.h b/Include/osmodule.h index 8e10c239465c74..25397a7651f073 100644 --- a/Include/osmodule.h +++ b/Include/osmodule.h @@ -9,7 +9,6 @@ extern "C" { #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03060000 PyAPI_FUNC(PyObject *) PyOS_FSPath(PyObject *path); -#endif typedef struct { FILE* perf_map; @@ -22,6 +21,8 @@ PyAPI_FUNC(int) PyOS_WritePerfMapEntry(const void *code_addr, unsigned int code_ PyAPI_FUNC(void) PyOS_PerfMapState_Fini(void); +#endif + #ifdef __cplusplus } #endif From ce54c6c655fb420ffa5fce5cadd181f7befd3432 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 14 Apr 2023 15:44:04 -0700 Subject: [PATCH 13/56] Update posixmodule.c --- Modules/posixmodule.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 5a5af19b91ee20..dee7165a680067 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -16739,28 +16739,28 @@ PyAPI_FUNC(void *) PyOS_PerfMapState_Init(void) { #ifdef MS_WINDOWS PyErr_SetString(PyExc_RuntimeError, "perf map is not supported on Windows"); return NULL; - #endif - - char filename[100]; - pid_t pid = getpid(); - // Use nofollow flag to prevent symlink attacks. - int flags = O_WRONLY | O_CREAT | O_APPEND | O_NOFOLLOW | O_CLOEXEC; - snprintf(filename, sizeof(filename) - 1, "/tmp/perf-%jd.map", - (intmax_t)pid); - int fd = open(filename, flags, 0600); - if (fd == -1) { - PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); - return NULL; - } - else{ - perf_map_state.perf_map = fdopen(fd, "a"); - if (perf_map_state.perf_map == NULL) { + #else + char filename[100]; + pid_t pid = getpid(); + // Use nofollow flag to prevent symlink attacks. + int flags = O_WRONLY | O_CREAT | O_APPEND | O_NOFOLLOW | O_CLOEXEC; + snprintf(filename, sizeof(filename) - 1, "/tmp/perf-%jd.map", + (intmax_t)pid); + int fd = open(filename, flags, 0600); + if (fd == -1) { PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); return NULL; } - } - perf_map_state.map_lock = PyThread_allocate_lock(); - return &perf_map_state; + else{ + perf_map_state.perf_map = fdopen(fd, "a"); + if (perf_map_state.perf_map == NULL) { + PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); + return NULL; + } + } + perf_map_state.map_lock = PyThread_allocate_lock(); + return &perf_map_state; + #endif } PyAPI_FUNC(int) PyOS_WritePerfMapEntry( From 6c3f233f197d34d0dee528c2ddfb9c8fc5539883 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Fri, 14 Apr 2023 23:05:53 +0000 Subject: [PATCH 14/56] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst diff --git a/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst b/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst new file mode 100644 index 00000000000000..1eaec5494fc569 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst @@ -0,0 +1 @@ +The PyOS_WritePerfMapEntry function has been added to write entries to a perf map file that can be used for profiling Python programs. This function can be used to write the code address, code size, and entry name for a given section of code to the perf map file. The PyOS_PerfMapState_Init function is used to initialize the perf map state and create a perf map file in /tmp, and PyOS_PerfMapState_Fini is used to clean up the state and close the file. Note that perf map is not supported on Windows. From 71db36157abff2f0f8ffeba10eddd8b34adcc3f3 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 14 Apr 2023 18:24:17 -0700 Subject: [PATCH 15/56] Update osmodule.h --- Include/osmodule.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Include/osmodule.h b/Include/osmodule.h index 25397a7651f073..c835a287bdea82 100644 --- a/Include/osmodule.h +++ b/Include/osmodule.h @@ -9,7 +9,9 @@ extern "C" { #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03060000 PyAPI_FUNC(PyObject *) PyOS_FSPath(PyObject *path); - +#endif + +#if !defined(Py_LIMITED_API) typedef struct { FILE* perf_map; PyThread_type_lock map_lock; @@ -19,8 +21,7 @@ PyAPI_FUNC(void *) PyOS_PerfMapState_Init(void); PyAPI_FUNC(int) PyOS_WritePerfMapEntry(const void *code_addr, unsigned int code_size, const char *entry_name); -PyAPI_FUNC(void) PyOS_PerfMapState_Fini(void); - +PyAPI_FUNC(void) PyOS_PerfMapState_Fini(void); #endif #ifdef __cplusplus From 020c872bee880f668a79104a8b966b9771aec049 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 14 Apr 2023 18:26:26 -0700 Subject: [PATCH 16/56] Update test_perfmaps.py --- Lib/test/test_perfmaps.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Lib/test/test_perfmaps.py b/Lib/test/test_perfmaps.py index e9198c3fa62db3..f6298ae72be4c3 100644 --- a/Lib/test/test_perfmaps.py +++ b/Lib/test/test_perfmaps.py @@ -1,6 +1,10 @@ import os +import sys import unittest +if sys.platform in ('win32', 'darwin'): + raise unittest.SkipTest('Linux only') + class TestPerfMapWriting(unittest.TestCase): def test_write_perf_map_entry(self): From e33b45df76cc7e8c805ace6bdc95f6415a471d92 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Sat, 15 Apr 2023 12:50:14 -0700 Subject: [PATCH 17/56] Update ignored.tsv --- Tools/c-analyzer/cpython/ignored.tsv | 1 + 1 file changed, 1 insertion(+) diff --git a/Tools/c-analyzer/cpython/ignored.tsv b/Tools/c-analyzer/cpython/ignored.tsv index a8ba88efc732fb..d06c53636833ae 100644 --- a/Tools/c-analyzer/cpython/ignored.tsv +++ b/Tools/c-analyzer/cpython/ignored.tsv @@ -290,6 +290,7 @@ Modules/posixmodule.c os_listxattr_impl buffer_sizes - Modules/posixmodule.c - posix_constants_confstr - Modules/posixmodule.c - posix_constants_pathconf - Modules/posixmodule.c - posix_constants_sysconf - +Modules/posixmodule.c - perf_map_state - Modules/pyexpat.c - ExpatMemoryHandler - Modules/pyexpat.c - error_info_of - Modules/pyexpat.c - handler_info - From 04e9716241fef590ef9776e624b036e8c843ffdf Mon Sep 17 00:00:00 2001 From: Aniket Panse Date: Mon, 17 Apr 2023 20:56:48 -0700 Subject: [PATCH 18/56] Fix trampoline APIs on windows, return int instead of void* from the init function --- Doc/c-api/perfmaps.rst | 44 +++++++++++ Include/osmodule.h | 6 +- ...-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst | 4 +- Modules/posixmodule.c | 73 ++++++++++++------- Python/perf_trampoline.c | 24 ++---- Python/pylifecycle.c | 2 +- Python/sysmodule.c | 2 +- 7 files changed, 103 insertions(+), 52 deletions(-) create mode 100644 Doc/c-api/perfmaps.rst diff --git a/Doc/c-api/perfmaps.rst b/Doc/c-api/perfmaps.rst new file mode 100644 index 00000000000000..8c74e25f918159 --- /dev/null +++ b/Doc/c-api/perfmaps.rst @@ -0,0 +1,44 @@ +.. highlight:: c + +.. _perfmaps: + +Support for Perf Maps +---------------------- + +On supported platforms (as of this writing, only Linux), the runtime can take +advantage of *perf map files* to make Python functions visible to an external +profiling tool (such as `perf `_). +A running process may create a file in the `/tmp` directory, which contains entries +that can map a section of executable code to a name. This interface is described in the +`documentation of the Linux Perf tool `_. + +In Python, these helper APIs can be used by libraries and features that rely +on generating machine code on the fly. + +.. c:function:: int _PyOS_PerfMapState_Init(void) + + Open the `/tmp/perf-$pid.map` file, unless it's already opened, and create + a lock to ensure thread-safe writes to the file (provided the writes are + done through :c:func:`PyOS_WritePerfMapEntry`). Normally, there's no need + to call this explicitly, and it is safe to directly use :c:func:`PyOS_WritePerfMapEntry` + in your code. If the state isn't already initialized, it will be created on + the first call. + +.. c:function:: int PyOS_WritePerfMapEntry(const void *code_addr, unsigned int code_size, const char *entry_name) + + Write one single entry to the `/tmp/perf-$pid.map` file. This function is + thread safe. Here is what an example entry looks like:: + + # address size name + 0x7f3529fcf759 b py::bar:/run/t.py + + Extensions are encouraged to directly call this API when needed, instead of + separately initializing the state by calling :c:func:`_PyOS_PerfMapState_Init`. + +.. c:function:: int _PyOS_PerfMapState_Fini(void) + + Close the perf map file, which was opened in `_PyOS_PerfMapState_Init`. This + API is called by the runtime itself, during interpreter shut-down. In general, + there shouldn't be a reason to explicitly call this, except to handle specific + scenarios such as forking. diff --git a/Include/osmodule.h b/Include/osmodule.h index c835a287bdea82..52bc2bb75b1dfe 100644 --- a/Include/osmodule.h +++ b/Include/osmodule.h @@ -10,18 +10,18 @@ extern "C" { #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03060000 PyAPI_FUNC(PyObject *) PyOS_FSPath(PyObject *path); #endif - + #if !defined(Py_LIMITED_API) typedef struct { FILE* perf_map; PyThread_type_lock map_lock; } PerfMapState; -PyAPI_FUNC(void *) PyOS_PerfMapState_Init(void); +PyAPI_FUNC(int) _PyOS_PerfMapState_Init(void); PyAPI_FUNC(int) PyOS_WritePerfMapEntry(const void *code_addr, unsigned int code_size, const char *entry_name); -PyAPI_FUNC(void) PyOS_PerfMapState_Fini(void); +PyAPI_FUNC(void) _PyOS_PerfMapState_Fini(void); #endif #ifdef __cplusplus diff --git a/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst b/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst index 1eaec5494fc569..bfc23ed733b2e2 100644 --- a/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst +++ b/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst @@ -1 +1,3 @@ -The PyOS_WritePerfMapEntry function has been added to write entries to a perf map file that can be used for profiling Python programs. This function can be used to write the code address, code size, and entry name for a given section of code to the perf map file. The PyOS_PerfMapState_Init function is used to initialize the perf map state and create a perf map file in /tmp, and PyOS_PerfMapState_Fini is used to clean up the state and close the file. Note that perf map is not supported on Windows. +Introduced :c:func:`PyOS_WritePerfMapEntry`, :c:func:`_PyOS_PerfMapState_Init` and +:c:func:`_PyOS_PerfMapState_Fini`. These allow extension modules (JIT compilers in +particular) to write to perf-map files in a thread safe manner. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index dee7165a680067..b6fbbe0ad9b5b2 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -16735,32 +16735,36 @@ INITFUNC(void) static PerfMapState perf_map_state; -PyAPI_FUNC(void *) PyOS_PerfMapState_Init(void) { - #ifdef MS_WINDOWS - PyErr_SetString(PyExc_RuntimeError, "perf map is not supported on Windows"); - return NULL; - #else - char filename[100]; - pid_t pid = getpid(); - // Use nofollow flag to prevent symlink attacks. - int flags = O_WRONLY | O_CREAT | O_APPEND | O_NOFOLLOW | O_CLOEXEC; - snprintf(filename, sizeof(filename) - 1, "/tmp/perf-%jd.map", - (intmax_t)pid); - int fd = open(filename, flags, 0600); - if (fd == -1) { +PyAPI_FUNC(int) _PyOS_PerfMapState_Init(void) { +#ifdef MS_WINDOWS + PyErr_SetString(PyExc_RuntimeError, "perf map is not supported on Windows"); + return -1; +#else + char filename[100]; + pid_t pid = getpid(); + // Use nofollow flag to prevent symlink attacks. + int flags = O_WRONLY | O_CREAT | O_APPEND | O_NOFOLLOW | O_CLOEXEC; + snprintf(filename, sizeof(filename) - 1, "/tmp/perf-%jd.map", + (intmax_t)pid); + int fd = open(filename, flags, 0600); + if (fd == -1) { + PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); + return -1; + } + else{ + perf_map_state.perf_map = fdopen(fd, "a"); + if (perf_map_state.perf_map == NULL) { PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); - return NULL; - } - else{ - perf_map_state.perf_map = fdopen(fd, "a"); - if (perf_map_state.perf_map == NULL) { - PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); - return NULL; - } + return -1; } - perf_map_state.map_lock = PyThread_allocate_lock(); - return &perf_map_state; - #endif + } + perf_map_state.map_lock = PyThread_allocate_lock(); + if (perf_map_state.map_lock == NULL) { + PyErr_SetString(PyExc_RuntimeError, "failed to create a lock for perf-maps"); + return -1; + } + return 0; +#endif } PyAPI_FUNC(int) PyOS_WritePerfMapEntry( @@ -16768,8 +16772,12 @@ PyAPI_FUNC(int) PyOS_WritePerfMapEntry( unsigned int code_size, const char *entry_name ) { +#ifdef MS_WINDOWS + PyErr_SetString(PyExc_RuntimeError, "perf map is not supported on Windows"); + return -1; +#else if (perf_map_state.perf_map == NULL) { - if(PyOS_PerfMapState_Init() == NULL){ + if(_PyOS_PerfMapState_Init() == -1){ return -1; } } @@ -16778,14 +16786,25 @@ PyAPI_FUNC(int) PyOS_WritePerfMapEntry( fflush(perf_map_state.perf_map); PyThread_release_lock(perf_map_state.map_lock); return 0; +#endif } -PyAPI_FUNC(void) PyOS_PerfMapState_Fini(void) { +PyAPI_FUNC(void) _PyOS_PerfMapState_Fini(void) { +#ifdef MS_WINDOWS + PyErr_SetString(PyExc_RuntimeError, "perf map is not supported on Windows"); + return -1; +#else if (perf_map_state.perf_map != NULL) { - PyThread_free_lock(perf_map_state.map_lock); + // close the file + PyThread_acquire_lock(perf_map_state.map_lock, 1); fclose(perf_map_state.perf_map); + PyThread_release_lock(perf_map_state.map_lock); + + // clean up the lock and state + PyThread_free_lock(perf_map_state.map_lock); perf_map_state.perf_map = NULL; } +#endif } #ifdef __cplusplus diff --git a/Python/perf_trampoline.c b/Python/perf_trampoline.c index b6025563466913..b722a73f2a6a9c 100644 --- a/Python/perf_trampoline.c +++ b/Python/perf_trampoline.c @@ -193,23 +193,12 @@ typedef struct trampoline_api_st trampoline_api_t; #define trampoline_api _PyRuntime.ceval.perf.trampoline_api #define perf_map_file _PyRuntime.ceval.perf.map_file -static void * -perf_map_get_file(void) -{ - return PyOS_PerfMapState_Init();; -} - -static void -perf_map_close(void) -{ - PyOS_PerfMapState_Fini(); -} static void perf_map_write_entry(void *state, const void *code_addr, unsigned int code_size, PyCodeObject *co) { - assert(state != NULL); + assert(state != NULL); const char *entry = ""; if (co->co_qualname != NULL) { entry = PyUnicode_AsUTF8(co->co_qualname); @@ -224,9 +213,9 @@ perf_map_write_entry(void *state, const void *code_addr, } _PyPerf_Callbacks _Py_perfmap_callbacks = { - &perf_map_get_file, + NULL, &perf_map_write_entry, - &perf_map_close + NULL, }; static int @@ -430,7 +419,7 @@ _PyPerfTrampoline_Init(int activate) if (new_code_arena() < 0) { return -1; } - if (trampoline_api.state == NULL) { + if (trampoline_api.state == NULL && trampoline_api.init_state != NULL) { void *state = trampoline_api.init_state(); if (state == NULL) { return -1; @@ -456,10 +445,6 @@ _PyPerfTrampoline_Fini(void) tstate->interp->eval_frame = NULL; } free_code_arenas(); - if (trampoline_api.state != NULL) { - trampoline_api.free_state(); - trampoline_api.state = NULL; - } extra_code_index = -1; #endif return 0; @@ -472,6 +457,7 @@ _PyPerfTrampoline_AfterFork_Child(void) // Restart trampoline in file in child. int was_active = _PyIsPerfTrampolineActive(); _PyPerfTrampoline_Fini(); + _PyOS_PerfMapState_Fini(); if (was_active) { _PyPerfTrampoline_Init(1); } diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 0846accf1062cf..aade7e2c06c0f2 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1778,7 +1778,7 @@ Py_FinalizeEx(void) */ _PyAtExit_Call(tstate->interp); - PyOS_PerfMapState_Fini(); + _PyOS_PerfMapState_Fini(); /* Copy the core config, PyInterpreterState_Delete() free the core config memory */ diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 4d693a1be1f89e..3832b8970209d8 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2130,7 +2130,7 @@ sys_activate_stack_trampoline_impl(PyObject *module, const char *backend) if (strcmp(backend, "perf") == 0) { _PyPerf_Callbacks cur_cb; _PyPerfTrampoline_GetCallbacks(&cur_cb); - if (cur_cb.init_state != _Py_perfmap_callbacks.init_state) { + if (cur_cb.write_state != _Py_perfmap_callbacks.write_state) { if (_PyPerfTrampoline_SetCallbacks(&_Py_perfmap_callbacks) < 0 ) { PyErr_SetString(PyExc_ValueError, "can't activate perf trampoline"); return NULL; From 4b8cd2691e7a12c00d90c7d7d5612e6937abf234 Mon Sep 17 00:00:00 2001 From: Aniket Panse Date: Tue, 18 Apr 2023 23:14:52 -0700 Subject: [PATCH 19/56] fix restructuredtext literals --- Doc/c-api/perfmaps.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Doc/c-api/perfmaps.rst b/Doc/c-api/perfmaps.rst index 8c74e25f918159..8db31852326b64 100644 --- a/Doc/c-api/perfmaps.rst +++ b/Doc/c-api/perfmaps.rst @@ -8,7 +8,7 @@ Support for Perf Maps On supported platforms (as of this writing, only Linux), the runtime can take advantage of *perf map files* to make Python functions visible to an external profiling tool (such as `perf `_). -A running process may create a file in the `/tmp` directory, which contains entries +A running process may create a file in the ``/tmp`` directory, which contains entries that can map a section of executable code to a name. This interface is described in the `documentation of the Linux Perf tool `_. @@ -18,7 +18,7 @@ on generating machine code on the fly. .. c:function:: int _PyOS_PerfMapState_Init(void) - Open the `/tmp/perf-$pid.map` file, unless it's already opened, and create + Open the ``/tmp/perf-$pid.map`` file, unless it's already opened, and create a lock to ensure thread-safe writes to the file (provided the writes are done through :c:func:`PyOS_WritePerfMapEntry`). Normally, there's no need to call this explicitly, and it is safe to directly use :c:func:`PyOS_WritePerfMapEntry` @@ -27,7 +27,7 @@ on generating machine code on the fly. .. c:function:: int PyOS_WritePerfMapEntry(const void *code_addr, unsigned int code_size, const char *entry_name) - Write one single entry to the `/tmp/perf-$pid.map` file. This function is + Write one single entry to the ``/tmp/perf-$pid.map`` file. This function is thread safe. Here is what an example entry looks like:: # address size name @@ -38,7 +38,7 @@ on generating machine code on the fly. .. c:function:: int _PyOS_PerfMapState_Fini(void) - Close the perf map file, which was opened in `_PyOS_PerfMapState_Init`. This + Close the perf map file, which was opened in ``_PyOS_PerfMapState_Init``. This API is called by the runtime itself, during interpreter shut-down. In general, there shouldn't be a reason to explicitly call this, except to handle specific scenarios such as forking. From ccc2d5b037a44dfcd5e6f1fa0cea5c27bd19d07f Mon Sep 17 00:00:00 2001 From: Aniket Panse Date: Tue, 18 Apr 2023 23:20:37 -0700 Subject: [PATCH 20/56] add perfmaps docs under utilities --- Doc/c-api/utilities.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/Doc/c-api/utilities.rst b/Doc/c-api/utilities.rst index a805b564763c40..ccbf14e1850f68 100644 --- a/Doc/c-api/utilities.rst +++ b/Doc/c-api/utilities.rst @@ -19,3 +19,4 @@ and parsing function arguments and constructing Python values from C values. conversion.rst reflection.rst codec.rst + perfmaps.rst From 71a75786164b52b79af55b7598b56403e09109e1 Mon Sep 17 00:00:00 2001 From: Aniket Panse Date: Wed, 19 Apr 2023 16:30:56 -0700 Subject: [PATCH 21/56] make "state" a non-required part of trampoline --- Python/perf_trampoline.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Python/perf_trampoline.c b/Python/perf_trampoline.c index b722a73f2a6a9c..4efa23cb312a04 100644 --- a/Python/perf_trampoline.c +++ b/Python/perf_trampoline.c @@ -198,7 +198,6 @@ static void perf_map_write_entry(void *state, const void *code_addr, unsigned int code_size, PyCodeObject *co) { - assert(state != NULL); const char *entry = ""; if (co->co_qualname != NULL) { entry = PyUnicode_AsUTF8(co->co_qualname); From 0ede03999717d0c924169aa1df7f710e029eeaf3 Mon Sep 17 00:00:00 2001 From: Aniket Panse Date: Wed, 19 Apr 2023 22:47:37 -0700 Subject: [PATCH 22/56] make the API no-op on windows, instead of raising errors --- Modules/posixmodule.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index b6fbbe0ad9b5b2..e23d740c4b4b6e 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -16736,10 +16736,7 @@ INITFUNC(void) static PerfMapState perf_map_state; PyAPI_FUNC(int) _PyOS_PerfMapState_Init(void) { -#ifdef MS_WINDOWS - PyErr_SetString(PyExc_RuntimeError, "perf map is not supported on Windows"); - return -1; -#else +#ifndef MS_WINDOWS char filename[100]; pid_t pid = getpid(); // Use nofollow flag to prevent symlink attacks. @@ -16763,8 +16760,8 @@ PyAPI_FUNC(int) _PyOS_PerfMapState_Init(void) { PyErr_SetString(PyExc_RuntimeError, "failed to create a lock for perf-maps"); return -1; } - return 0; #endif + return 0; } PyAPI_FUNC(int) PyOS_WritePerfMapEntry( @@ -16772,10 +16769,7 @@ PyAPI_FUNC(int) PyOS_WritePerfMapEntry( unsigned int code_size, const char *entry_name ) { -#ifdef MS_WINDOWS - PyErr_SetString(PyExc_RuntimeError, "perf map is not supported on Windows"); - return -1; -#else +#ifndef MS_WINDOWS if (perf_map_state.perf_map == NULL) { if(_PyOS_PerfMapState_Init() == -1){ return -1; @@ -16785,15 +16779,12 @@ PyAPI_FUNC(int) PyOS_WritePerfMapEntry( fprintf(perf_map_state.perf_map, "%p %x %s\n", code_addr, code_size, entry_name); fflush(perf_map_state.perf_map); PyThread_release_lock(perf_map_state.map_lock); - return 0; #endif + return 0; } PyAPI_FUNC(void) _PyOS_PerfMapState_Fini(void) { -#ifdef MS_WINDOWS - PyErr_SetString(PyExc_RuntimeError, "perf map is not supported on Windows"); - return -1; -#else +#ifndef MS_WINDOWS if (perf_map_state.perf_map != NULL) { // close the file PyThread_acquire_lock(perf_map_state.map_lock, 1); From 7356a9ba1c3da63bb613fded2bb9c9b769c595e7 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Mon, 8 May 2023 13:57:10 -0700 Subject: [PATCH 23/56] use PyUnstable as a prefix --- Include/osmodule.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Include/osmodule.h b/Include/osmodule.h index 52bc2bb75b1dfe..a682e05e214931 100644 --- a/Include/osmodule.h +++ b/Include/osmodule.h @@ -17,11 +17,11 @@ typedef struct { PyThread_type_lock map_lock; } PerfMapState; -PyAPI_FUNC(int) _PyOS_PerfMapState_Init(void); +PyAPI_FUNC(int) PyUnstable_PerfMapState_Init(void); -PyAPI_FUNC(int) PyOS_WritePerfMapEntry(const void *code_addr, unsigned int code_size, const char *entry_name); +PyAPI_FUNC(int) PyUnstable_WritePerfMapEntry(const void *code_addr, unsigned int code_size, const char *entry_name); -PyAPI_FUNC(void) _PyOS_PerfMapState_Fini(void); +PyAPI_FUNC(void) PyUnstable_PerfMapState_Fini(void); #endif #ifdef __cplusplus From b504e9abd353466b8db080adbc5a87c4ee0a058c Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Mon, 8 May 2023 14:03:17 -0700 Subject: [PATCH 24/56] move the implementation of the perf-map api to the sys module --- Python/sysmodule.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 3832b8970209d8..8361651a5d0cad 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -52,6 +52,10 @@ extern const char *PyWin_DLLVersionString; #include #endif +#ifdef HAVE_FCNTL_H +#include +#endif + /*[clinic input] module sys [clinic start generated code]*/ @@ -2226,6 +2230,80 @@ sys__getframemodulename_impl(PyObject *module, int depth) } +#ifdef __cplusplus +extern "C" { +#endif + +static PerfMapState perf_map_state; + +PyAPI_FUNC(int) PyUnstable_PerfMapState_Init(void) { +#ifndef MS_WINDOWS + char filename[100]; + pid_t pid = getpid(); + // Use nofollow flag to prevent symlink attacks. + int flags = O_WRONLY | O_CREAT | O_APPEND | O_NOFOLLOW | O_CLOEXEC; + snprintf(filename, sizeof(filename) - 1, "/tmp/perf-%jd.map", + (intmax_t)pid); + int fd = open(filename, flags, 0600); + if (fd == -1) { + PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); + return -1; + } + else{ + perf_map_state.perf_map = fdopen(fd, "a"); + if (perf_map_state.perf_map == NULL) { + PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); + return -1; + } + } + perf_map_state.map_lock = PyThread_allocate_lock(); + if (perf_map_state.map_lock == NULL) { + PyErr_SetString(PyExc_RuntimeError, "failed to create a lock for perf-maps"); + return -1; + } +#endif + return 0; +} + +PyAPI_FUNC(int) PyUnstable_WritePerfMapEntry( + const void *code_addr, + unsigned int code_size, + const char *entry_name +) { +#ifndef MS_WINDOWS + if (perf_map_state.perf_map == NULL) { + if(PyUnstable_PerfMapState_Init() == -1){ + return -1; + } + } + PyThread_acquire_lock(perf_map_state.map_lock, 1); + fprintf(perf_map_state.perf_map, "%p %u %s\n", code_addr, code_size, entry_name); + fflush(perf_map_state.perf_map); + PyThread_release_lock(perf_map_state.map_lock); +#endif + return 0; +} + +PyAPI_FUNC(void) PyUnstable_PerfMapState_Fini(void) { +#ifndef MS_WINDOWS + if (perf_map_state.perf_map != NULL) { + // close the file + PyThread_acquire_lock(perf_map_state.map_lock, 1); + fclose(perf_map_state.perf_map); + PyThread_release_lock(perf_map_state.map_lock); + + // clean up the lock and state + PyThread_free_lock(perf_map_state.map_lock); + perf_map_state.perf_map = NULL; + } +#endif +} + +#ifdef __cplusplus +} +#endif + + static PyMethodDef sys_methods[] = { /* Might as well keep this in alphabetic order */ SYS_ADDAUDITHOOK_METHODDEF From 99dab1ad540625177edc4f5f9d26fca49f6f5f34 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Mon, 8 May 2023 14:05:02 -0700 Subject: [PATCH 25/56] Remove the perf-map API from the posixmodule.c --- Modules/posixmodule.c | 65 ------------------------------------------- 1 file changed, 65 deletions(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index e23d740c4b4b6e..dd150107e4a9de 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -16733,71 +16733,6 @@ INITFUNC(void) return PyModuleDef_Init(&posixmodule); } -static PerfMapState perf_map_state; - -PyAPI_FUNC(int) _PyOS_PerfMapState_Init(void) { -#ifndef MS_WINDOWS - char filename[100]; - pid_t pid = getpid(); - // Use nofollow flag to prevent symlink attacks. - int flags = O_WRONLY | O_CREAT | O_APPEND | O_NOFOLLOW | O_CLOEXEC; - snprintf(filename, sizeof(filename) - 1, "/tmp/perf-%jd.map", - (intmax_t)pid); - int fd = open(filename, flags, 0600); - if (fd == -1) { - PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); - return -1; - } - else{ - perf_map_state.perf_map = fdopen(fd, "a"); - if (perf_map_state.perf_map == NULL) { - PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); - return -1; - } - } - perf_map_state.map_lock = PyThread_allocate_lock(); - if (perf_map_state.map_lock == NULL) { - PyErr_SetString(PyExc_RuntimeError, "failed to create a lock for perf-maps"); - return -1; - } -#endif - return 0; -} - -PyAPI_FUNC(int) PyOS_WritePerfMapEntry( - const void *code_addr, - unsigned int code_size, - const char *entry_name -) { -#ifndef MS_WINDOWS - if (perf_map_state.perf_map == NULL) { - if(_PyOS_PerfMapState_Init() == -1){ - return -1; - } - } - PyThread_acquire_lock(perf_map_state.map_lock, 1); - fprintf(perf_map_state.perf_map, "%p %x %s\n", code_addr, code_size, entry_name); - fflush(perf_map_state.perf_map); - PyThread_release_lock(perf_map_state.map_lock); -#endif - return 0; -} - -PyAPI_FUNC(void) _PyOS_PerfMapState_Fini(void) { -#ifndef MS_WINDOWS - if (perf_map_state.perf_map != NULL) { - // close the file - PyThread_acquire_lock(perf_map_state.map_lock, 1); - fclose(perf_map_state.perf_map); - PyThread_release_lock(perf_map_state.map_lock); - - // clean up the lock and state - PyThread_free_lock(perf_map_state.map_lock); - perf_map_state.perf_map = NULL; - } -#endif -} - #ifdef __cplusplus } #endif From 6b95cbc9a25cce982e91aae27c8f5111f22174a9 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Mon, 8 May 2023 14:07:14 -0700 Subject: [PATCH 26/56] Update perfmaps.rst to reflect the PyUnstable prefix --- Doc/c-api/perfmaps.rst | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Doc/c-api/perfmaps.rst b/Doc/c-api/perfmaps.rst index 8db31852326b64..e69579edd223e1 100644 --- a/Doc/c-api/perfmaps.rst +++ b/Doc/c-api/perfmaps.rst @@ -16,29 +16,29 @@ kernel/git/torvalds/linux.git/tree/tools/perf/Documentation/jit-interface.txt>`_ In Python, these helper APIs can be used by libraries and features that rely on generating machine code on the fly. -.. c:function:: int _PyOS_PerfMapState_Init(void) +.. c:function:: int PyUnstable_PerfMapState_Init(void) Open the ``/tmp/perf-$pid.map`` file, unless it's already opened, and create a lock to ensure thread-safe writes to the file (provided the writes are - done through :c:func:`PyOS_WritePerfMapEntry`). Normally, there's no need - to call this explicitly, and it is safe to directly use :c:func:`PyOS_WritePerfMapEntry` + done through :c:func:`PyUnstable_WritePerfMapEntry`). Normally, there's no need + to call this explicitly, and it is safe to directly use :c:func:`PyUnstable_WritePerfMapEntry` in your code. If the state isn't already initialized, it will be created on the first call. -.. c:function:: int PyOS_WritePerfMapEntry(const void *code_addr, unsigned int code_size, const char *entry_name) +.. c:function:: int PyUnstable_WritePerfMapEntry(const void *code_addr, unsigned int code_size, const char *entry_name) Write one single entry to the ``/tmp/perf-$pid.map`` file. This function is thread safe. Here is what an example entry looks like:: # address size name - 0x7f3529fcf759 b py::bar:/run/t.py + 0x7f3529fcf759 11 py::bar:/run/t.py Extensions are encouraged to directly call this API when needed, instead of - separately initializing the state by calling :c:func:`_PyOS_PerfMapState_Init`. + separately initializing the state by calling :c:func:`PyUnstable_PerfMapState_Init`. -.. c:function:: int _PyOS_PerfMapState_Fini(void) +.. c:function:: int PyUnstable_PerfMapState_Fini(void) - Close the perf map file, which was opened in ``_PyOS_PerfMapState_Init``. This + Close the perf map file, which was opened in ``PyUnstable_PerfMapState_Init``. This API is called by the runtime itself, during interpreter shut-down. In general, there shouldn't be a reason to explicitly call this, except to handle specific scenarios such as forking. From d0ef0a1c77a5867868e3ff6a5bd8426fddc13db8 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Mon, 8 May 2023 14:08:48 -0700 Subject: [PATCH 27/56] Update perf_trampoline.c to use PyUnstable_WritePerfMapEntry --- Python/perf_trampoline.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/perf_trampoline.c b/Python/perf_trampoline.c index 4efa23cb312a04..a8d5c4ae72a8c9 100644 --- a/Python/perf_trampoline.c +++ b/Python/perf_trampoline.c @@ -208,7 +208,7 @@ perf_map_write_entry(void *state, const void *code_addr, } char perf_map_entry[500]; snprintf(perf_map_entry, sizeof(perf_map_entry), "py::%s:%s", entry, filename); - PyOS_WritePerfMapEntry(code_addr, code_size, perf_map_entry); + PyUnstable_WritePerfMapEntry(code_addr, code_size, perf_map_entry); } _PyPerf_Callbacks _Py_perfmap_callbacks = { From c5e766bbcf58d7f69fff19b6ccb66778539c8932 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Mon, 8 May 2023 14:11:06 -0700 Subject: [PATCH 28/56] Update pylifecycle.c to use PyUnstable_PerfMapState_Fini --- Python/pylifecycle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index aade7e2c06c0f2..de3ca27ac50462 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1778,7 +1778,7 @@ Py_FinalizeEx(void) */ _PyAtExit_Call(tstate->interp); - _PyOS_PerfMapState_Fini(); + PyUnstable_PerfMapState_Fini(); /* Copy the core config, PyInterpreterState_Delete() free the core config memory */ From b0641a794ce541560ef5eedff12f367535ca76fe Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Mon, 8 May 2023 14:14:25 -0700 Subject: [PATCH 29/56] expose PyUnstable_PerfMapState_Fini --- Modules/_testinternalcapi.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 31c4b8c79f0e7d..383c378a46dbb9 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -694,7 +694,7 @@ write_perf_map_entry(PyObject *self, PyObject *args) if (!PyArg_ParseTuple(args, "KIs", &code_addr, &code_size, &entry_name)) return NULL; - int ret = PyOS_WritePerfMapEntry(code_addr, code_size, entry_name); + int ret = PyUnstable_WritePerfMapEntry(code_addr, code_size, entry_name); if (ret == -1) { PyErr_SetString(PyExc_OSError, "Failed to write performance map entry"); return NULL; @@ -702,6 +702,14 @@ write_perf_map_entry(PyObject *self, PyObject *args) return Py_BuildValue("i", ret); } + +static PyObject * +perf_map_state_teardown(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(ignored)) +{ + PyUnstable_PerfMapState_Fini(); + Py_RETURN_NONE; +} + static PyMethodDef module_functions[] = { {"get_configs", get_configs, METH_NOARGS}, {"get_recursion_depth", get_recursion_depth, METH_NOARGS}, @@ -725,6 +733,7 @@ static PyMethodDef module_functions[] = { {"get_interp_settings", get_interp_settings, METH_VARARGS, NULL}, {"clear_extension", clear_extension, METH_VARARGS, NULL}, {"write_perf_map_entry", write_perf_map_entry, METH_VARARGS}, + {"perf_map_state_teardown", perf_map_state_teardown, METH_NOARGS}, {NULL, NULL} /* sentinel */ }; From c392b2419b41de6d860768c4bd621b336598631e Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Mon, 8 May 2023 14:17:48 -0700 Subject: [PATCH 30/56] use assertIn instead of assertEqual and tear down the perf_map_state at the end of the test --- Lib/test/test_perfmaps.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_perfmaps.py b/Lib/test/test_perfmaps.py index f6298ae72be4c3..fe03950a5f70cc 100644 --- a/Lib/test/test_perfmaps.py +++ b/Lib/test/test_perfmaps.py @@ -2,14 +2,18 @@ import sys import unittest -if sys.platform in ('win32', 'darwin'): +from _testinternalcapi import perf_map_state_teardown, write_perf_map_entry + +if sys.platform != 'linux': raise unittest.SkipTest('Linux only') class TestPerfMapWriting(unittest.TestCase): def test_write_perf_map_entry(self): - from _testinternalcapi import write_perf_map_entry - self.assertEqual(write_perf_map_entry(0x1234, 0x5678, "entry1"), 0) - self.assertEqual(write_perf_map_entry(0x2345, 0x6789, "entry2"), 0) + self.assertEqual(write_perf_map_entry(0x1234, 5678, "entry1"), 0) + self.assertEqual(write_perf_map_entry(0x2345, 6789, "entry2"), 0) with open(f"/tmp/perf-{os.getpid()}.map") as f: - self.assertEqual(f.read().strip(), "0x1234 5678 entry1\n0x2345 6789 entry2") + perf_file_contents = f.read() + self.assertIn("0x1234 5678 entry1", perf_file_contents) + self.assertIn("0x2345 6789 entry2", perf_file_contents) + perf_map_state_teardown() From 262a38b37ac8b8920fcf2bd32fd4440d93cbf7ba Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Mon, 8 May 2023 14:23:11 -0700 Subject: [PATCH 31/56] use malloc instead of fixed buffer size --- Python/perf_trampoline.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Python/perf_trampoline.c b/Python/perf_trampoline.c index a8d5c4ae72a8c9..374cedc204674a 100644 --- a/Python/perf_trampoline.c +++ b/Python/perf_trampoline.c @@ -206,9 +206,11 @@ perf_map_write_entry(void *state, const void *code_addr, if (co->co_filename != NULL) { filename = PyUnicode_AsUTF8(co->co_filename); } - char perf_map_entry[500]; - snprintf(perf_map_entry, sizeof(perf_map_entry), "py::%s:%s", entry, filename); + size_t perf_map_entry_size = snprintf(NULL, 0, "py::%s:%s", entry, filename) + 1; + char* perf_map_entry = (char*) malloc(perf_map_entry_size); + snprintf(perf_map_entry, perf_map_entry_size, "py::%s:%s", entry, filename); PyUnstable_WritePerfMapEntry(code_addr, code_size, perf_map_entry); + free(perf_map_entry); } _PyPerf_Callbacks _Py_perfmap_callbacks = { From a52a25a0f4a1ff388f7c6f535f01e675d70151a4 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Mon, 8 May 2023 15:23:06 -0700 Subject: [PATCH 32/56] Fix perf map entry to have the START and SIZE as hex numbers without 0x --- Python/sysmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 8361651a5d0cad..09d94bf9013f11 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2277,7 +2277,7 @@ PyAPI_FUNC(int) PyUnstable_WritePerfMapEntry( } } PyThread_acquire_lock(perf_map_state.map_lock, 1); - fprintf(perf_map_state.perf_map, "%p %u %s\n", code_addr, code_size, entry_name); + fprintf(perf_map_state.perf_map, "%" PRIxPTR " %x %s\n", code_addr, code_size, entry_name); fflush(perf_map_state.perf_map); PyThread_release_lock(perf_map_state.map_lock); #endif From 0932e30e46f0975e8dcb38f52ccfc93d2f5f4669 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Mon, 8 May 2023 15:39:57 -0700 Subject: [PATCH 33/56] Add (uintptr_t) casting for the code_addr --- Python/sysmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 09d94bf9013f11..e2422e1697588d 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2277,7 +2277,7 @@ PyAPI_FUNC(int) PyUnstable_WritePerfMapEntry( } } PyThread_acquire_lock(perf_map_state.map_lock, 1); - fprintf(perf_map_state.perf_map, "%" PRIxPTR " %x %s\n", code_addr, code_size, entry_name); + fprintf(perf_map_state.perf_map, "%" PRIxPTR " %x %s\n", (uintptr_t) code_addr, code_size, entry_name); fflush(perf_map_state.perf_map); PyThread_release_lock(perf_map_state.map_lock); #endif From 5aeb74ce3d9d51f92144fcffbd56a736fc22c004 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Mon, 8 May 2023 15:42:13 -0700 Subject: [PATCH 34/56] update the perf map entry example to be hex without 0x --- Doc/c-api/perfmaps.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/c-api/perfmaps.rst b/Doc/c-api/perfmaps.rst index e69579edd223e1..d1faf3568518ff 100644 --- a/Doc/c-api/perfmaps.rst +++ b/Doc/c-api/perfmaps.rst @@ -31,7 +31,7 @@ on generating machine code on the fly. thread safe. Here is what an example entry looks like:: # address size name - 0x7f3529fcf759 11 py::bar:/run/t.py + 7f3529fcf759 b py::bar:/run/t.py Extensions are encouraged to directly call this API when needed, instead of separately initializing the state by calling :c:func:`PyUnstable_PerfMapState_Init`. From b0682e05a5adc5ba5809e9ee3b33fe51c3b1b156 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Mon, 8 May 2023 15:50:53 -0700 Subject: [PATCH 35/56] Update the doc with the PyUnstable prefix --- .../next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst b/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst index bfc23ed733b2e2..c574f4f12cf6b9 100644 --- a/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst +++ b/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst @@ -1,3 +1,3 @@ -Introduced :c:func:`PyOS_WritePerfMapEntry`, :c:func:`_PyOS_PerfMapState_Init` and -:c:func:`_PyOS_PerfMapState_Fini`. These allow extension modules (JIT compilers in +Introduced :c:func:`PyUnstable_WritePerfMapEntry`, :c:func:`PyUnstable_PerfMapState_Init` and +:c:func:`PyUnstable_PerfMapState_Fini`. These allow extension modules (JIT compilers in particular) to write to perf-map files in a thread safe manner. From e581026e36a268cad3fc6f85edde293eac9c1a03 Mon Sep 17 00:00:00 2001 From: Aniket Panse Date: Mon, 8 May 2023 16:27:43 -0700 Subject: [PATCH 36/56] change function name to the new one --- Python/perf_trampoline.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/perf_trampoline.c b/Python/perf_trampoline.c index 374cedc204674a..6258e6fff93ae7 100644 --- a/Python/perf_trampoline.c +++ b/Python/perf_trampoline.c @@ -458,7 +458,7 @@ _PyPerfTrampoline_AfterFork_Child(void) // Restart trampoline in file in child. int was_active = _PyIsPerfTrampolineActive(); _PyPerfTrampoline_Fini(); - _PyOS_PerfMapState_Fini(); + PyUnstable_PerfMapState_Fini(); if (was_active) { _PyPerfTrampoline_Init(1); } From e1850921cce8090f46c52b8ea65bf3ced4c8888e Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Mon, 8 May 2023 17:00:13 -0700 Subject: [PATCH 37/56] fix tests_perfmaps --- Lib/test/test_perfmaps.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_perfmaps.py b/Lib/test/test_perfmaps.py index fe03950a5f70cc..a17adb89f55360 100644 --- a/Lib/test/test_perfmaps.py +++ b/Lib/test/test_perfmaps.py @@ -14,6 +14,6 @@ def test_write_perf_map_entry(self): self.assertEqual(write_perf_map_entry(0x2345, 6789, "entry2"), 0) with open(f"/tmp/perf-{os.getpid()}.map") as f: perf_file_contents = f.read() - self.assertIn("0x1234 5678 entry1", perf_file_contents) - self.assertIn("0x2345 6789 entry2", perf_file_contents) + self.assertIn("1234 162e entry1", perf_file_contents) + self.assertIn("2345 1a85 entry2", perf_file_contents) perf_map_state_teardown() From c97137bbd487159c16ed3bd6b689ad06b49a4ad2 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Mon, 8 May 2023 17:02:04 -0700 Subject: [PATCH 38/56] Update ignored.tsv --- Tools/c-analyzer/cpython/ignored.tsv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/c-analyzer/cpython/ignored.tsv b/Tools/c-analyzer/cpython/ignored.tsv index d56326b346ab03..e9056aed5dc0b2 100644 --- a/Tools/c-analyzer/cpython/ignored.tsv +++ b/Tools/c-analyzer/cpython/ignored.tsv @@ -291,7 +291,6 @@ Modules/posixmodule.c os_listxattr_impl buffer_sizes - Modules/posixmodule.c - posix_constants_confstr - Modules/posixmodule.c - posix_constants_pathconf - Modules/posixmodule.c - posix_constants_sysconf - -Modules/posixmodule.c - perf_map_state - Modules/pyexpat.c - ExpatMemoryHandler - Modules/pyexpat.c - error_info_of - Modules/pyexpat.c - handler_info - @@ -357,6 +356,7 @@ Python/pystate.c - initial - Python/specialize.c - adaptive_opcodes - Python/specialize.c - cache_requirements - Python/stdlib_module_names.h - _Py_stdlib_module_names - +Python/sysmodule.c - perf_map_state - Python/sysmodule.c - _PySys_ImplCacheTag - Python/sysmodule.c - _PySys_ImplName - Python/sysmodule.c - whatstrings - From 344782bdc9cf00fc2f217476941d1b1e499b2e83 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Mon, 8 May 2023 17:03:22 -0700 Subject: [PATCH 39/56] remove extra new line --- Modules/_testinternalcapi.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index c0444c3e641a00..9518540f3e1ac9 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -773,7 +773,6 @@ write_perf_map_entry(PyObject *self, PyObject *args) return Py_BuildValue("i", ret); } - static PyObject * perf_map_state_teardown(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(ignored)) { From 1ff695a845f8a0e0693f5ac09656105c165e4d28 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Mon, 8 May 2023 17:04:42 -0700 Subject: [PATCH 40/56] remove extra trailing white space --- Python/sysmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/sysmodule.c b/Python/sysmodule.c index f7bdb3878821ed..f9ea981f617c20 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2247,7 +2247,7 @@ sys__getframemodulename_impl(PyObject *module, int depth) #ifdef __cplusplus extern "C" { #endif - + static PerfMapState perf_map_state; PyAPI_FUNC(int) PyUnstable_PerfMapState_Init(void) { From 2f13a4edc370c939189149f2e04e566195650c9b Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Mon, 8 May 2023 17:29:14 -0700 Subject: [PATCH 41/56] check perf_map_entry for NULL. nothing we can do but skip writing an entry, but at least _we_ won't be the cause of a crash and something that might generate an appropriate error real soon can be instead. --- Python/perf_trampoline.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Python/perf_trampoline.c b/Python/perf_trampoline.c index 6258e6fff93ae7..713adc73d3ac38 100644 --- a/Python/perf_trampoline.c +++ b/Python/perf_trampoline.c @@ -208,6 +208,9 @@ perf_map_write_entry(void *state, const void *code_addr, } size_t perf_map_entry_size = snprintf(NULL, 0, "py::%s:%s", entry, filename) + 1; char* perf_map_entry = (char*) malloc(perf_map_entry_size); + if (perf_map_entry == NULL) { + return; + } snprintf(perf_map_entry, perf_map_entry_size, "py::%s:%s", entry, filename); PyUnstable_WritePerfMapEntry(code_addr, code_size, perf_map_entry); free(perf_map_entry); From 50d27e1c2e38e658dc5269c652468f33645b2836 Mon Sep 17 00:00:00 2001 From: Aniket Panse Date: Tue, 9 May 2023 10:42:06 -0700 Subject: [PATCH 42/56] update docs --- Doc/howto/perf_profiling.rst | 3 ++- .../next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Doc/howto/perf_profiling.rst b/Doc/howto/perf_profiling.rst index 6af5536166f58a..aec99afe3ca6a0 100644 --- a/Doc/howto/perf_profiling.rst +++ b/Doc/howto/perf_profiling.rst @@ -24,7 +24,8 @@ functions to appear in the output of the ``perf`` profiler. When this mode is enabled, the interpreter will interpose a small piece of code compiled on the fly before the execution of every Python function and it will teach ``perf`` the relationship between this piece of code and the associated Python function using -`perf map files`_. +`perf map files`_. If you're an extension author interested in having your extension +write to the perf map files, refer to :doc:`the C-API <../c-api/perfmaps.rst>`. .. note:: diff --git a/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst b/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst index c574f4f12cf6b9..a08d05eb6b2826 100644 --- a/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst +++ b/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst @@ -1,3 +1,5 @@ Introduced :c:func:`PyUnstable_WritePerfMapEntry`, :c:func:`PyUnstable_PerfMapState_Init` and :c:func:`PyUnstable_PerfMapState_Fini`. These allow extension modules (JIT compilers in -particular) to write to perf-map files in a thread safe manner. +particular) to write to perf-map files in a thread safe manner. The +:doc:`perf profiling feature <../howto/perf_profiling.rst>` also uses these APIs to write +entries in the perf-map file. From 717c4e4887820e64006296340e23f04182230138 Mon Sep 17 00:00:00 2001 From: Aniket Panse Date: Tue, 9 May 2023 11:05:19 -0700 Subject: [PATCH 43/56] remove rst extension from inline doc links --- Doc/howto/perf_profiling.rst | 2 +- .../next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/howto/perf_profiling.rst b/Doc/howto/perf_profiling.rst index aec99afe3ca6a0..ad44e26519896f 100644 --- a/Doc/howto/perf_profiling.rst +++ b/Doc/howto/perf_profiling.rst @@ -25,7 +25,7 @@ enabled, the interpreter will interpose a small piece of code compiled on the fly before the execution of every Python function and it will teach ``perf`` the relationship between this piece of code and the associated Python function using `perf map files`_. If you're an extension author interested in having your extension -write to the perf map files, refer to :doc:`the C-API <../c-api/perfmaps.rst>`. +write to the perf map files, refer to :doc:`the C-API <../c-api/perfmaps>`. .. note:: diff --git a/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst b/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst index a08d05eb6b2826..88222239858bc3 100644 --- a/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst +++ b/Misc/NEWS.d/next/C API/2023-04-14-23-05-52.gh-issue-103295.GRHY1Z.rst @@ -1,5 +1,5 @@ Introduced :c:func:`PyUnstable_WritePerfMapEntry`, :c:func:`PyUnstable_PerfMapState_Init` and :c:func:`PyUnstable_PerfMapState_Fini`. These allow extension modules (JIT compilers in particular) to write to perf-map files in a thread safe manner. The -:doc:`perf profiling feature <../howto/perf_profiling.rst>` also uses these APIs to write +:doc:`../howto/perf_profiling` also uses these APIs to write entries in the perf-map file. From 428dfc367d30323859dbd580a072030c039d68da Mon Sep 17 00:00:00 2001 From: Aniket Panse Date: Thu, 18 May 2023 09:44:59 -0700 Subject: [PATCH 44/56] fix the free_state function signature --- Include/internal/pycore_ceval.h | 2 +- Include/internal/pycore_ceval_state.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index 3137cdb19249c1..3c8b368bd2af4e 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -66,7 +66,7 @@ typedef struct { void (*write_state)(void* state, const void *code_addr, unsigned int code_size, PyCodeObject* code); // Callback to free the trampoline state - void (*free_state)(void); + int (*free_state)(void* state); } _PyPerf_Callbacks; extern int _PyPerfTrampoline_SetCallbacks(_PyPerf_Callbacks *); diff --git a/Include/internal/pycore_ceval_state.h b/Include/internal/pycore_ceval_state.h index c8d6ff88d762f7..b352801673c40a 100644 --- a/Include/internal/pycore_ceval_state.h +++ b/Include/internal/pycore_ceval_state.h @@ -27,7 +27,7 @@ struct trampoline_api_st { void* (*init_state)(void); void (*write_state)(void* state, const void *code_addr, unsigned int code_size, PyCodeObject* code); - void (*free_state)(void); + int (*free_state)(void* state); void *state; }; #endif From e386ef86fe3a10bf498088265d8252a85448a07d Mon Sep 17 00:00:00 2001 From: Aniket Panse Date: Thu, 18 May 2023 10:05:35 -0700 Subject: [PATCH 45/56] trim extra whitespace --- Modules/_testinternalcapi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index e30b6f9037cc58..2015bed2a44d97 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -782,7 +782,7 @@ perf_map_state_teardown(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(ignored)) PyUnstable_PerfMapState_Fini(); Py_RETURN_NONE; } - + static PyObject * iframe_getcode(PyObject *self, PyObject *frame) { From d245fbf74dc503def5b67862ef439c06bfa05137 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 19 May 2023 17:37:05 -0700 Subject: [PATCH 46/56] Return error codes instead of setting exceptions in PyUnstable_PerfMapState_Init --- Python/sysmodule.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 38e95812b96a91..84c704fd926d63 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2264,20 +2264,19 @@ PyAPI_FUNC(int) PyUnstable_PerfMapState_Init(void) { (intmax_t)pid); int fd = open(filename, flags, 0600); if (fd == -1) { - PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); return -1; } else{ perf_map_state.perf_map = fdopen(fd, "a"); if (perf_map_state.perf_map == NULL) { - PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); + close(fd); return -1; } } perf_map_state.map_lock = PyThread_allocate_lock(); if (perf_map_state.map_lock == NULL) { - PyErr_SetString(PyExc_RuntimeError, "failed to create a lock for perf-maps"); - return -1; + fclose(perf_map_state.perf_map); + return -2; } #endif return 0; @@ -2290,7 +2289,7 @@ PyAPI_FUNC(int) PyUnstable_WritePerfMapEntry( ) { #ifndef MS_WINDOWS if (perf_map_state.perf_map == NULL) { - if(PyUnstable_PerfMapState_Init() == -1){ + if(PyUnstable_PerfMapState_Init() != 0){ return -1; } } From 2027de67b093f081f5eeb5af73c8a0bf44be4f5a Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 19 May 2023 17:45:10 -0700 Subject: [PATCH 47/56] Add a note that holding the GIL is not required for these APIs --- Doc/c-api/perfmaps.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Doc/c-api/perfmaps.rst b/Doc/c-api/perfmaps.rst index d1faf3568518ff..e7d75bcbe69fce 100644 --- a/Doc/c-api/perfmaps.rst +++ b/Doc/c-api/perfmaps.rst @@ -16,6 +16,8 @@ kernel/git/torvalds/linux.git/tree/tools/perf/Documentation/jit-interface.txt>`_ In Python, these helper APIs can be used by libraries and features that rely on generating machine code on the fly. +Note that holding the Global Interpreter Lock (GIL) is not required for these APIs. + .. c:function:: int PyUnstable_PerfMapState_Init(void) Open the ``/tmp/perf-$pid.map`` file, unless it's already opened, and create @@ -23,7 +25,7 @@ on generating machine code on the fly. done through :c:func:`PyUnstable_WritePerfMapEntry`). Normally, there's no need to call this explicitly, and it is safe to directly use :c:func:`PyUnstable_WritePerfMapEntry` in your code. If the state isn't already initialized, it will be created on - the first call. + the first call. .. c:function:: int PyUnstable_WritePerfMapEntry(const void *code_addr, unsigned int code_size, const char *entry_name) From 0383b808c6d5548eee54d14fd9d2d5a03b062444 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 19 May 2023 17:54:03 -0700 Subject: [PATCH 48/56] Remove the init_state call since it is always NULL for the perf backend --- Python/perf_trampoline.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/Python/perf_trampoline.c b/Python/perf_trampoline.c index 713adc73d3ac38..461e999931271b 100644 --- a/Python/perf_trampoline.c +++ b/Python/perf_trampoline.c @@ -423,13 +423,6 @@ _PyPerfTrampoline_Init(int activate) if (new_code_arena() < 0) { return -1; } - if (trampoline_api.state == NULL && trampoline_api.init_state != NULL) { - void *state = trampoline_api.init_state(); - if (state == NULL) { - return -1; - } - trampoline_api.state = state; - } extra_code_index = _PyEval_RequestCodeExtraIndex(NULL); if (extra_code_index == -1) { return -1; From 59f3c1502573c90474fa98bbe1b56e3e049e5c40 Mon Sep 17 00:00:00 2001 From: gsallam <123525874+gsallam@users.noreply.github.com> Date: Fri, 19 May 2023 18:07:27 -0700 Subject: [PATCH 49/56] trim extra whitespaces --- Doc/c-api/perfmaps.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/c-api/perfmaps.rst b/Doc/c-api/perfmaps.rst index e7d75bcbe69fce..4096a730b8dabe 100644 --- a/Doc/c-api/perfmaps.rst +++ b/Doc/c-api/perfmaps.rst @@ -16,7 +16,7 @@ kernel/git/torvalds/linux.git/tree/tools/perf/Documentation/jit-interface.txt>`_ In Python, these helper APIs can be used by libraries and features that rely on generating machine code on the fly. -Note that holding the Global Interpreter Lock (GIL) is not required for these APIs. +Note that holding the Global Interpreter Lock (GIL) is not required for these APIs. .. c:function:: int PyUnstable_PerfMapState_Init(void) @@ -25,7 +25,7 @@ Note that holding the Global Interpreter Lock (GIL) is not required for these AP done through :c:func:`PyUnstable_WritePerfMapEntry`). Normally, there's no need to call this explicitly, and it is safe to directly use :c:func:`PyUnstable_WritePerfMapEntry` in your code. If the state isn't already initialized, it will be created on - the first call. + the first call. .. c:function:: int PyUnstable_WritePerfMapEntry(const void *code_addr, unsigned int code_size, const char *entry_name) From 8bc6150647440eae5f9be763e97f592953d2750a Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Fri, 19 May 2023 20:39:15 -0700 Subject: [PATCH 50/56] simplify link to perf map docs from perf profiling --- Doc/howto/perf_profiling.rst | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Doc/howto/perf_profiling.rst b/Doc/howto/perf_profiling.rst index ad44e26519896f..61812c19ae6ca9 100644 --- a/Doc/howto/perf_profiling.rst +++ b/Doc/howto/perf_profiling.rst @@ -24,8 +24,7 @@ functions to appear in the output of the ``perf`` profiler. When this mode is enabled, the interpreter will interpose a small piece of code compiled on the fly before the execution of every Python function and it will teach ``perf`` the relationship between this piece of code and the associated Python function using -`perf map files`_. If you're an extension author interested in having your extension -write to the perf map files, refer to :doc:`the C-API <../c-api/perfmaps>`. +:doc:`perf map files <../c-api/perfmaps>`. .. note:: @@ -207,5 +206,3 @@ You can check if your system has been compiled with this flag by running:: If you don't see any output it means that your interpreter has not been compiled with frame pointers and therefore it may not be able to show Python functions in the output of ``perf``. - -.. _perf map files: https://github.com/torvalds/linux/blob/0513e464f9007b70b96740271a948ca5ab6e7dd7/tools/perf/Documentation/jit-interface.txt From c7a49b4da6b4923b8306bcd7c79335f032cbe761 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Fri, 19 May 2023 20:39:34 -0700 Subject: [PATCH 51/56] document return values, other doc tweaks --- Doc/c-api/perfmaps.rst | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/Doc/c-api/perfmaps.rst b/Doc/c-api/perfmaps.rst index 4096a730b8dabe..6f40af3d1db972 100644 --- a/Doc/c-api/perfmaps.rst +++ b/Doc/c-api/perfmaps.rst @@ -23,9 +23,12 @@ Note that holding the Global Interpreter Lock (GIL) is not required for these AP Open the ``/tmp/perf-$pid.map`` file, unless it's already opened, and create a lock to ensure thread-safe writes to the file (provided the writes are done through :c:func:`PyUnstable_WritePerfMapEntry`). Normally, there's no need - to call this explicitly, and it is safe to directly use :c:func:`PyUnstable_WritePerfMapEntry` - in your code. If the state isn't already initialized, it will be created on - the first call. + to call this explicitly; just use :c:func:`PyUnstable_WritePerfMapEntry` + and it will initialize the state on first call. + + Returns ``0`` on success, ``-1`` on failure to create/open the perf map file, + or ``-2`` on failure to create a lock. Check :c:data:`errno` for more + information about the cause of a failure. .. c:function:: int PyUnstable_WritePerfMapEntry(const void *code_addr, unsigned int code_size, const char *entry_name) @@ -35,12 +38,13 @@ Note that holding the Global Interpreter Lock (GIL) is not required for these AP # address size name 7f3529fcf759 b py::bar:/run/t.py - Extensions are encouraged to directly call this API when needed, instead of - separately initializing the state by calling :c:func:`PyUnstable_PerfMapState_Init`. + Will call :c:func:`PyUnstable_PerfMapState_Init` before writing the entry, if + the perf map file is not already opened. Returns ``0`` on success, or the + same error codes as :c:func:`PyUnstable_PerfMapState_Init` on failure. -.. c:function:: int PyUnstable_PerfMapState_Fini(void) +.. c:function:: void PyUnstable_PerfMapState_Fini(void) - Close the perf map file, which was opened in ``PyUnstable_PerfMapState_Init``. This - API is called by the runtime itself, during interpreter shut-down. In general, - there shouldn't be a reason to explicitly call this, except to handle specific - scenarios such as forking. + Close the perf map file opened by :c:func:`PyUnstable_PerfMapState_Init`. + This is called by the runtime itself during interpreter shut-down. In + general, there shouldn't be a reason to explicitly call this, except to + handle specific scenarios such as forking. From 125513affcaea3fd410f22680d157d3956a59c4c Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Fri, 19 May 2023 20:39:48 -0700 Subject: [PATCH 52/56] move headers to sysmodule.h --- Include/osmodule.h | 13 ------------- Include/sysmodule.h | 13 +++++++++++++ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Include/osmodule.h b/Include/osmodule.h index a682e05e214931..9095c2fdd3d638 100644 --- a/Include/osmodule.h +++ b/Include/osmodule.h @@ -11,19 +11,6 @@ extern "C" { PyAPI_FUNC(PyObject *) PyOS_FSPath(PyObject *path); #endif -#if !defined(Py_LIMITED_API) -typedef struct { - FILE* perf_map; - PyThread_type_lock map_lock; -} PerfMapState; - -PyAPI_FUNC(int) PyUnstable_PerfMapState_Init(void); - -PyAPI_FUNC(int) PyUnstable_WritePerfMapEntry(const void *code_addr, unsigned int code_size, const char *entry_name); - -PyAPI_FUNC(void) PyUnstable_PerfMapState_Fini(void); -#endif - #ifdef __cplusplus } #endif diff --git a/Include/sysmodule.h b/Include/sysmodule.h index b5087119b1cae7..96f883870b34dc 100644 --- a/Include/sysmodule.h +++ b/Include/sysmodule.h @@ -29,6 +29,19 @@ Py_DEPRECATED(3.11) PyAPI_FUNC(int) PySys_HasWarnOptions(void); Py_DEPRECATED(3.11) PyAPI_FUNC(void) PySys_AddXOption(const wchar_t *); PyAPI_FUNC(PyObject *) PySys_GetXOptions(void); +#if !defined(Py_LIMITED_API) +typedef struct { + FILE* perf_map; + PyThread_type_lock map_lock; +} PerfMapState; + +PyAPI_FUNC(int) PyUnstable_PerfMapState_Init(void); + +PyAPI_FUNC(int) PyUnstable_WritePerfMapEntry(const void *code_addr, unsigned int code_size, const char *entry_name); + +PyAPI_FUNC(void) PyUnstable_PerfMapState_Fini(void); +#endif + #ifndef Py_LIMITED_API # define Py_CPYTHON_SYSMODULE_H # include "cpython/sysmodule.h" From 10630e80d583b697afe066d050673bd92193b439 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Fri, 19 May 2023 20:40:04 -0700 Subject: [PATCH 53/56] pass through init return value from write-entry --- Python/sysmodule.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 84c704fd926d63..8b28d58ec411cb 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2289,8 +2289,9 @@ PyAPI_FUNC(int) PyUnstable_WritePerfMapEntry( ) { #ifndef MS_WINDOWS if (perf_map_state.perf_map == NULL) { - if(PyUnstable_PerfMapState_Init() != 0){ - return -1; + int ret = PyUnstable_PerfMapState_Init(); + if(ret != 0){ + return ret; } } PyThread_acquire_lock(perf_map_state.map_lock, 1); From 386dd42da0c75c4a0e83e5e031eb97239a6ff1a8 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Fri, 19 May 2023 21:41:34 -0700 Subject: [PATCH 54/56] apparently :c:data:`errno` doesn't exist --- Doc/c-api/perfmaps.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/c-api/perfmaps.rst b/Doc/c-api/perfmaps.rst index 6f40af3d1db972..3d44d2eb6bf41d 100644 --- a/Doc/c-api/perfmaps.rst +++ b/Doc/c-api/perfmaps.rst @@ -27,8 +27,8 @@ Note that holding the Global Interpreter Lock (GIL) is not required for these AP and it will initialize the state on first call. Returns ``0`` on success, ``-1`` on failure to create/open the perf map file, - or ``-2`` on failure to create a lock. Check :c:data:`errno` for more - information about the cause of a failure. + or ``-2`` on failure to create a lock. Check ``errno`` for more information + about the cause of a failure. .. c:function:: int PyUnstable_WritePerfMapEntry(const void *code_addr, unsigned int code_size, const char *entry_name) From 6585f325b90f7ab029d079b61b3848a7c0b3dc31 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Fri, 19 May 2023 21:44:07 -0700 Subject: [PATCH 55/56] use PyMem_Raw* instead of malloc/free --- Python/perf_trampoline.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/perf_trampoline.c b/Python/perf_trampoline.c index 461e999931271b..ec08623c687452 100644 --- a/Python/perf_trampoline.c +++ b/Python/perf_trampoline.c @@ -207,13 +207,13 @@ perf_map_write_entry(void *state, const void *code_addr, filename = PyUnicode_AsUTF8(co->co_filename); } size_t perf_map_entry_size = snprintf(NULL, 0, "py::%s:%s", entry, filename) + 1; - char* perf_map_entry = (char*) malloc(perf_map_entry_size); + char* perf_map_entry = (char*) PyMem_RawMalloc(perf_map_entry_size); if (perf_map_entry == NULL) { return; } snprintf(perf_map_entry, perf_map_entry_size, "py::%s:%s", entry, filename); PyUnstable_WritePerfMapEntry(code_addr, code_size, perf_map_entry); - free(perf_map_entry); + PyMem_Free(perf_map_entry); } _PyPerf_Callbacks _Py_perfmap_callbacks = { From e6be2a788b5185df05d351ec199ccd96e5dcf248 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Sat, 20 May 2023 08:21:23 -0700 Subject: [PATCH 56/56] fix PyMem_Free to PyMem_RawFree --- Python/perf_trampoline.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/perf_trampoline.c b/Python/perf_trampoline.c index ec08623c687452..6a6f223d095d3b 100644 --- a/Python/perf_trampoline.c +++ b/Python/perf_trampoline.c @@ -213,7 +213,7 @@ perf_map_write_entry(void *state, const void *code_addr, } snprintf(perf_map_entry, perf_map_entry_size, "py::%s:%s", entry, filename); PyUnstable_WritePerfMapEntry(code_addr, code_size, perf_map_entry); - PyMem_Free(perf_map_entry); + PyMem_RawFree(perf_map_entry); } _PyPerf_Callbacks _Py_perfmap_callbacks = {