diff --git a/README.md b/README.md index 4f83071..83172be 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ information specific to Linux Alpine kernels, see ## Supported Python Versions -Python >= 3.7 and <= 3.11 +Python >= 3.7 and <= 3.13 ## Installation & usage @@ -104,3 +104,4 @@ Exception ignored when trying to write to the signal wakeup fd see https://cloud.google.com/profiler/docs/troubleshooting#python-blocking for the cause and the workaround. + diff --git a/googlecloudprofiler/__version__.py b/googlecloudprofiler/__version__.py index 8e58090..02d34d4 100644 --- a/googlecloudprofiler/__version__.py +++ b/googlecloudprofiler/__version__.py @@ -16,4 +16,7 @@ """Version of Python Cloud Profiler module.""" # setup.py reads the version information from here to set package version -__version__ = '4.1.0' +# Glean fork: upstream 4.1.0 + 3.12/3.13 support (vendored as 4.2.0) + the +# SIGPROF frame-walk crash fix (copy-the-chain). Patch bump over the synthetic +# 4.2.0 -- the fix is a backward-compatible bug fix. Upstream has no 4.2.x. +__version__ = '4.2.1' diff --git a/googlecloudprofiler/src/populate_frames.cc b/googlecloudprofiler/src/populate_frames.cc index 4ff10a0..47f492d 100644 --- a/googlecloudprofiler/src/populate_frames.cc +++ b/googlecloudprofiler/src/populate_frames.cc @@ -1,76 +1,230 @@ #include "populate_frames.h" #include +#include +#include +#include + +#include #include "stacktraces.h" -// 0x030B0000 is 3.11. -#define PY_311 0x030B0000 -#if PY_VERSION_HEX >= PY_311 +// Python version definitions +#define PY_311 0x030B0000 // 3.11 +#define PY_312 0x030C0000 // 3.12 +#define PY_313 0x030D0000 // 3.13 + +// Reads the current process's own memory through the kernel instead of +// dereferencing the pointer directly. process_vm_readv reports an unmapped or +// otherwise invalid source range as EFAULT (a failed return), never a SIGSEGV, +// so the SIGPROF frame walk can follow interpreter pointers that may have been +// torn down or partially written by the interrupted thread without crashing the +// process. Reading our own pid is always permitted and the raw syscall touches +// no libc state, so this is safe to call from the signal handler. +bool SafeCopy(void *dst, const void *src, size_t n) { + if (src == nullptr) { + return false; + } + struct iovec local = {dst, n}; + struct iovec remote = {const_cast(src), n}; + long got = syscall(SYS_process_vm_readv, static_cast(getpid()), &local, + 1UL, &remote, 1UL, 0UL); + return got == static_cast(n); +} + +#if PY_VERSION_HEX >= PY_313 /** + * Python 3.13 introduced significant changes to the frame structure: + * - f_code renamed to f_executable (now PyObject* instead of PyCodeObject*) + * - prev_instr renamed to instr_ptr + * - cframe->current_frame flattened to tstate->current_frame + * * The PyFrameObject structure members have been removed from the public C API * in 3.11: -https://docs.python.org/3/whatsnew/3.11.html#pyframeobject-3-11-hiding. - * - * Instead, getters are provided which participate in reference counting; since - * this code runs as part of the SIGPROF handler, it cannot modify Python - * objects (including their refcounts) and the getters can't be used. Instead, - * we expose the internal _PyInterpreterFrame and use that directly. + * https://docs.python.org/3/whatsnew/3.11.html#pyframeobject-3-11-hiding. * + * The walk runs in the SIGPROF handler, which can interrupt the interpreter at + * any instruction -- including while a frame or its code object is being set up + * or torn down. Rather than dereferencing the chain (which faults on a stale or + * half-written pointer), every interpreter pointer is read with SafeCopy and + * the walk operates only on the local copies. */ #define Py_BUILD_CORE #include "internal/pycore_frame.h" #undef Py_BUILD_CORE -// Modified from -// https://github.com/python/cpython/blob/v3.11.4/Python/pystate.c#L1278-L1285 -static inline _PyInterpreterFrame *unsafe_PyThreadState_GetInterpreterFrame( - PyThreadState *tstate) { - assert(tstate != NULL); - _PyInterpreterFrame *f = tstate->cframe->current_frame; - while (f && _PyFrame_IsIncomplete(f)) { - f = f->previous; +// Reads and validates the code object behind a 3.13 frame's f_executable into +// *code_copy. Returns the (live) code pointer, or nullptr if the executable is +// unreadable or is not a code object (f_executable may hold other types). +static PyCodeObject *FrameCode(const _PyInterpreterFrame *fr, + PyCodeObject *code_copy) { + if (fr->f_executable == nullptr || + !SafeCopy(code_copy, fr->f_executable, sizeof(*code_copy)) || + Py_TYPE(reinterpret_cast(code_copy)) != &PyCode_Type) { + return nullptr; + } + return reinterpret_cast(fr->f_executable); +} + +// CPython 3.13 _PyFrame_IsIncomplete, reimplemented over copies. +static bool FrameIsIncomplete(const _PyInterpreterFrame *fr, PyCodeObject *code, + const PyCodeObject *code_copy) { + if (fr->owner == FRAME_OWNED_BY_CSTACK) { + return true; + } + if (fr->owner == FRAME_OWNED_BY_GENERATOR) { + return false; + } + if (code == nullptr) { + return true; + } + return fr->instr_ptr < _PyCode_CODE(code) + code_copy->_co_firsttraceable; +} + +int PopulateFrames(CallFrame *frames, PyThreadState *ts) { + if (ts == nullptr) { + frames[0].lineno = kNoPyState; + frames[0].py_code = nullptr; + return 1; } - if (f == NULL) { - return NULL; + + _PyInterpreterFrame *faddr = ts->current_frame; + int num_frames = 0; + while (faddr != nullptr && num_frames < kMaxFramesToCapture) { + _PyInterpreterFrame fr; + if (!SafeCopy(&fr, faddr, sizeof(fr))) { + break; // unreadable frame: stop, keep the frames gathered so far + } + PyCodeObject code_copy; + PyCodeObject *code = FrameCode(&fr, &code_copy); + if (code != nullptr && !FrameIsIncomplete(&fr, code, &code_copy)) { + // Defer line and name/filename resolution to PythonTraces (GIL held). + // lineno temporarily carries the instruction byte offset; PythonTraces + // turns it into a source line with PyCode_Addr2Line on the live object. + frames[num_frames].py_code = code; + frames[num_frames].lineno = static_cast( + (fr.instr_ptr - _PyCode_CODE(code)) * sizeof(_Py_CODEUNIT)); + num_frames++; + } + faddr = fr.previous; } - return f; + return num_frames; } -// Modified from -// https://github.com/python/cpython/blob/v3.11.4/Objects/frameobject.c#L1310-L1315 -// with refcounting removed -static inline PyCodeObject *unsafe_PyInterpreterFrame_GetCode( - _PyInterpreterFrame *frame) { - assert(frame != NULL); - assert(!_PyFrame_IsIncomplete(frame)); - PyCodeObject *code = frame->f_code; - assert(code != NULL); - return code; +#elif PY_VERSION_HEX >= PY_312 + +/** + * Python 3.12 changes to the frame structure: + * - f_code moved to first position in the struct + * - f_func renamed to f_funcobj + * - is_entry field removed, return_offset field added + * + * The PyFrameObject structure members have been removed from the public C API + * in 3.11: + * https://docs.python.org/3/whatsnew/3.11.html#pyframeobject-3-11-hiding. + * + * The walk runs in the SIGPROF handler, which can interrupt the interpreter at + * any instruction -- including while a frame or its code object is being set up + * or torn down. Rather than dereferencing the chain (which faults on a stale or + * half-written pointer), every interpreter pointer is read with SafeCopy and + * the walk operates only on the local copies. + */ + +#define Py_BUILD_CORE +#include "internal/pycore_frame.h" +#undef Py_BUILD_CORE + +// CPython 3.12 _PyFrame_IsIncomplete, reimplemented over a frame copy so the +// prologue check reads the code object via SafeCopy instead of dereferencing a +// possibly-invalid pointer. +static bool FrameIsIncomplete(const _PyInterpreterFrame *fr) { + if (fr->owner == FRAME_OWNED_BY_CSTACK) { + return true; + } + if (fr->owner == FRAME_OWNED_BY_GENERATOR) { + return false; + } + PyCodeObject code; + if (!SafeCopy(&code, fr->f_code, sizeof(code))) { + return true; // unreadable code object: treat as incomplete (skip) + } + return fr->prev_instr < _PyCode_CODE(fr->f_code) + code._co_firsttraceable; } -// Modified from -// https://github.com/python/cpython/blob/v3.11.4/Objects/frameobject.c#L1326-L1329 -// with refcounting removed -static inline _PyInterpreterFrame *unsafe_PyInterpreterFrame_GetBack( - _PyInterpreterFrame *frame) { - assert(frame != NULL); - assert(!_PyFrame_IsIncomplete(frame)); - _PyInterpreterFrame *prev = frame->previous; - while (prev && _PyFrame_IsIncomplete(prev)) { - prev = prev->previous; - } - return prev; +int PopulateFrames(CallFrame *frames, PyThreadState *ts) { + if (ts == nullptr) { + frames[0].lineno = kNoPyState; + frames[0].py_code = nullptr; + return 1; + } + + // ts is the live thread state and safe to read directly. current_frame and + // the frame chain it links, however, can be torn/stale/unmapped if SIGPROF + // lands mid frame setup or teardown, so each is read with SafeCopy: a bad + // pointer aborts the walk instead of faulting. + _PyCFrame *cframe = ts->cframe; + if (cframe == nullptr) { + return 0; + } + _PyInterpreterFrame *faddr = nullptr; + if (!SafeCopy(&faddr, &cframe->current_frame, sizeof(faddr))) { + return 0; + } + + int num_frames = 0; + while (faddr != nullptr && num_frames < kMaxFramesToCapture) { + _PyInterpreterFrame fr; + if (!SafeCopy(&fr, faddr, sizeof(fr))) { + break; // unreadable frame: stop, keep the frames gathered so far + } + if (fr.f_code != nullptr && !FrameIsIncomplete(&fr)) { + // Defer line and name/filename resolution to PythonTraces (GIL held). + // lineno temporarily carries the instruction byte offset; PythonTraces + // turns it into a source line with PyCode_Addr2Line on the live object. + frames[num_frames].py_code = fr.f_code; + frames[num_frames].lineno = static_cast( + (fr.prev_instr - _PyCode_CODE(fr.f_code)) * sizeof(_Py_CODEUNIT)); + num_frames++; + } + faddr = fr.previous; + } + return num_frames; } -// Copied from -// https://github.com/python/cpython/blob/v3.11.4/Python/frame.c#L165-L170 as -// this function is not available in libpython -int _PyInterpreterFrame_GetLine(_PyInterpreterFrame *frame) { - int addr = _PyInterpreterFrame_LASTI(frame) * sizeof(_Py_CODEUNIT); - return PyCode_Addr2Line(frame->f_code, addr); +#elif PY_VERSION_HEX >= PY_311 + +/** + * Python 3.11 frame structure baseline. + * + * The PyFrameObject structure members have been removed from the public C API + * in 3.11: + * https://docs.python.org/3/whatsnew/3.11.html#pyframeobject-3-11-hiding. + * + * The walk runs in the SIGPROF handler, which can interrupt the interpreter at + * any instruction -- including while a frame or its code object is being set up + * or torn down. Rather than dereferencing the chain (which faults on a stale or + * half-written pointer), every interpreter pointer is read with SafeCopy and + * the walk operates only on the local copies. + */ + +#define Py_BUILD_CORE +#include "internal/pycore_frame.h" +#undef Py_BUILD_CORE + +// CPython 3.11 _PyFrame_IsIncomplete, reimplemented over a frame copy so the +// prologue check reads the code object via SafeCopy instead of dereferencing a +// possibly-invalid pointer. (3.11 has no FRAME_OWNED_BY_CSTACK.) +static bool FrameIsIncomplete(const _PyInterpreterFrame *fr) { + if (fr->owner == FRAME_OWNED_BY_GENERATOR) { + return false; + } + PyCodeObject code; + if (!SafeCopy(&code, fr->f_code, sizeof(code))) { + return true; // unreadable code object: treat as incomplete (skip) + } + return fr->prev_instr < _PyCode_CODE(fr->f_code) + code._co_firsttraceable; } int PopulateFrames(CallFrame *frames, PyThreadState *ts) { @@ -80,17 +234,35 @@ int PopulateFrames(CallFrame *frames, PyThreadState *ts) { return 1; } - // We are running in the context of the thread interrupted by the signal - // so the frame object for the current thread is stable. - // Unfortunately, we can't use PyFrameObjects because they are initialized - // lazily and will not have the info we need directly. - _PyInterpreterFrame *frame = unsafe_PyThreadState_GetInterpreterFrame(ts); + // ts is the live thread state and safe to read directly. current_frame and + // the frame chain it links, however, can be torn/stale/unmapped if SIGPROF + // lands mid frame setup or teardown, so each is read with SafeCopy: a bad + // pointer aborts the walk instead of faulting. + _PyCFrame *cframe = ts->cframe; + if (cframe == nullptr) { + return 0; + } + _PyInterpreterFrame *faddr = nullptr; + if (!SafeCopy(&faddr, &cframe->current_frame, sizeof(faddr))) { + return 0; + } + int num_frames = 0; - while (frame != nullptr && num_frames < kMaxFramesToCapture) { - frames[num_frames].lineno = _PyInterpreterFrame_GetLine(frame); - frames[num_frames].py_code = unsafe_PyInterpreterFrame_GetCode(frame); - num_frames++; - frame = unsafe_PyInterpreterFrame_GetBack(frame); + while (faddr != nullptr && num_frames < kMaxFramesToCapture) { + _PyInterpreterFrame fr; + if (!SafeCopy(&fr, faddr, sizeof(fr))) { + break; // unreadable frame: stop, keep the frames gathered so far + } + if (fr.f_code != nullptr && !FrameIsIncomplete(&fr)) { + // Defer line and name/filename resolution to PythonTraces (GIL held). + // lineno temporarily carries the instruction byte offset; PythonTraces + // turns it into a source line with PyCode_Addr2Line on the live object. + frames[num_frames].py_code = fr.f_code; + frames[num_frames].lineno = static_cast( + (fr.prev_instr - _PyCode_CODE(fr.f_code)) * sizeof(_Py_CODEUNIT)); + num_frames++; + } + faddr = fr.previous; } return num_frames; } diff --git a/googlecloudprofiler/src/populate_frames.h b/googlecloudprofiler/src/populate_frames.h index 63bbe25..88a5ee2 100644 --- a/googlecloudprofiler/src/populate_frames.h +++ b/googlecloudprofiler/src/populate_frames.h @@ -3,8 +3,20 @@ #include +#include + #include "stacktraces.h" +/** + * Async-signal-safe read of `n` bytes from `src` (in this process's own address + * space) into `dst`, via process_vm_readv. Returns false if the source range is + * not fully readable -- e.g. a torn-down, stale, or otherwise invalid pointer -- + * instead of faulting the process. Used to walk interpreter frames from the + * SIGPROF handler, and to validate code objects on the collection thread, + * without dereferencing pointers that may have been invalidated by a race. + */ +bool SafeCopy(void* dst, const void* src, size_t n); + /** * Populates the CallFrame array with at-most kMaxFramesToCapture python frames * from the provided PyThreadState. Returns the number of frames populated. diff --git a/googlecloudprofiler/src/profiler.cc b/googlecloudprofiler/src/profiler.cc index 8d95870..6d210dc 100644 --- a/googlecloudprofiler/src/profiler.cc +++ b/googlecloudprofiler/src/profiler.cc @@ -159,10 +159,20 @@ void Profiler::Handle(int signum, siginfo_t *info, void *context) { } void GetFuncLoc(PyCodeObject *code_object, FuncLoc *func_loc) { + // The code-object pointer was captured in the signal handler and may be stale + // (freed, or its address reused). Validate it is a readable code object via + // SafeCopy before dereferencing its name/filename. + PyCodeObject code; + if (code_object == nullptr || !SafeCopy(&code, code_object, sizeof(code)) || + Py_TYPE(reinterpret_cast(&code)) != &PyCode_Type) { + func_loc->name = "unknown"; + func_loc->filename = "unknown"; + return; + } // Note that PyUnicode_AsUTF8 caches the char array in the unicodeobject // and the memory is released when the unicodeobject is deallocated. - const char *name = PyUnicode_AsUTF8(code_object->co_name); - const char *filename = PyUnicode_AsUTF8(code_object->co_filename); + const char *name = PyUnicode_AsUTF8(code.co_name); + const char *filename = PyUnicode_AsUTF8(code.co_filename); func_loc->name = name != nullptr ? name : "unknown"; func_loc->filename = filename != nullptr ? filename : "unknown"; } @@ -204,26 +214,36 @@ PyObject *Profiler::PythonTraces() { const auto &frame = trace.first[i]; FuncLoc func_loc; PyCodeObject *pointer = frame.py_code; + // For real frames, frame.lineno carries the instruction byte offset + // captured in the signal handler; it is resolved to a source line here, + // with the GIL held, via PyCode_Addr2Line on the live code object. + int lineno = frame.lineno; if (pointer == nullptr) { func_loc = { CallTraceErrorToName(static_cast(frame.lineno)), ""}; + } else if (CodeDeallocHook::Find(pointer, &func_loc)) { + // The code object was deallocated during profiling (its name/filename + // were recorded by the hook), so its line table is no longer available. + // TODO: If multiple code objects are allocated at the same address, the + // func_loc stored by CodeDeallocHook may not belong to the sampled + // frame. At least we should mark the func_loc as invalid if we see an + // address is reused, probably by hooking PyCode_Type.tp_alloc. + lineno = 0; } else { - // All PyCodeObjects deallocated during profiling should be recorded - // by CodeDeallocHook. As we are holding GIL, no deallocation can happen - // elsewhere now. It's safe to assume that a PyCodeObject pointer not - // recorded by CodeDeallocHook points to a live object. - // TODO: If multiple code objects are allocated at the same - // address, the func_loc stored by CodeDeallocHook may not belong to the - // sampled frame. At least we should mark the func_loc as invalid if we - // see an address is reused, probably by hooking PyCode_Type.tp_alloc. - if (!CodeDeallocHook::Find(pointer, &func_loc)) { - GetFuncLoc(pointer, &func_loc); - } + // Not recorded by CodeDeallocHook: assume live (GIL held, no concurrent + // dealloc). Validate readability before resolving the line. + GetFuncLoc(pointer, &func_loc); + PyCodeObject code_check; + lineno = + (SafeCopy(&code_check, pointer, sizeof(code_check)) && + Py_TYPE(reinterpret_cast(&code_check)) == &PyCode_Type) + ? PyCode_Addr2Line(pointer, frame.lineno) + : 0; } PyObject *py_frame = Py_BuildValue("(ssi)", func_loc.name.c_str(), - func_loc.filename.c_str(), frame.lineno); + func_loc.filename.c_str(), lineno); if (py_frame == nullptr) { return nullptr; } diff --git a/googlecloudprofiler/src/stacktraces.h b/googlecloudprofiler/src/stacktraces.h index 8f6a96d..7a66bdf 100644 --- a/googlecloudprofiler/src/stacktraces.h +++ b/googlecloudprofiler/src/stacktraces.h @@ -24,6 +24,11 @@ #include typedef struct { + // For a real frame on 3.11+, lineno carries the instruction byte offset + // captured in the SIGPROF handler (resolved to a source line later, with the + // GIL held, by PyCode_Addr2Line). For the pre-3.11 path it is the source + // line directly; for error/sentinel frames (py_code == nullptr) it is a + // CallTraceErrors code. int lineno; PyCodeObject *py_code; } CallFrame; diff --git a/setup.py b/setup.py index cfa99b7..199b9f2 100644 --- a/setup.py +++ b/setup.py @@ -110,5 +110,8 @@ def get_version(): 'Programming Language :: Python :: 3.9', 'Programming Language :: Python :: 3.10', 'Programming Language :: Python :: 3.11', + 'Programming Language :: Python :: 3.12', + 'Programming Language :: Python :: 3.13', ], ) +