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 2d042e2..47f492d 100644 --- a/googlecloudprofiler/src/populate_frames.cc +++ b/googlecloudprofiler/src/populate_frames.cc @@ -1,6 +1,11 @@ #include "populate_frames.h" #include +#include +#include +#include + +#include #include "stacktraces.h" @@ -9,102 +14,73 @@ #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 - * - Must use _PyFrame_GetCode() helper to access code object - * + * - 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. * - * Since this code runs as part of the SIGPROF handler, it cannot modify Python - * objects (including their refcounts) and standard getters can't be used. - * We expose the internal _PyInterpreterFrame and use that directly. + * 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 CPython 3.13 source for async-signal-safe access -// Python 3.13 flattened cframe->current_frame to just current_frame -// -// IMPORTANT: This can be called from a signal handler (SIGPROF), so we must -// be defensive about race conditions where the interpreter is in the middle -// of setting up frames. The current_frame pointer might be NULL or partially -// initialized if we interrupt during _PyEval_EvalFrameDefault setup. -static inline _PyInterpreterFrame *unsafe_PyThreadState_GetInterpreterFrame( - PyThreadState *tstate) { - if (tstate == NULL) { - return NULL; - } - - _PyInterpreterFrame *f = tstate->current_frame; - - // Handle race condition: current_frame might be NULL or uninitialized - // if we interrupted during frame setup - if (f == NULL) { - return NULL; - } - - while (f && _PyFrame_IsIncomplete(f)) { - f = f->previous; - } - return f; +// 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); } -// In Python 3.13, f_code became f_executable and is now a PyObject* -// This helper safely extracts the code object -static inline PyCodeObject *unsafe_PyInterpreterFrame_GetCode( - _PyInterpreterFrame *frame) { - if (frame == NULL || _PyFrame_IsIncomplete(frame)) { - return NULL; - } - - PyObject *executable = frame->f_executable; - if (executable == NULL) { - return NULL; +// 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; } - - // f_executable can be a code object or other types; verify it's a code object - // PyCode_Check uses type pointer, which should be safe to check in signal handler - if (!PyCode_Check(executable)) { - return NULL; - } - - return (PyCodeObject *)executable; -} - -static inline _PyInterpreterFrame *unsafe_PyInterpreterFrame_GetBack( - _PyInterpreterFrame *frame) { - if (frame == NULL || _PyFrame_IsIncomplete(frame)) { - return NULL; - } - - _PyInterpreterFrame *prev = frame->previous; - while (prev && _PyFrame_IsIncomplete(prev)) { - prev = prev->previous; - } - return prev; -} - -// Python 3.13 uses instr_ptr instead of prev_instr -int _PyInterpreterFrame_GetLine(_PyInterpreterFrame *frame) { - if (frame == NULL) { - return -1; + if (fr->owner == FRAME_OWNED_BY_GENERATOR) { + return false; } - - PyCodeObject *code = unsafe_PyInterpreterFrame_GetCode(frame); - if (code == NULL) { - return -1; + if (code == nullptr) { + return true; } - - int addr = (int)(frame->instr_ptr - _PyCode_CODE(code)) * sizeof(_Py_CODEUNIT); - return PyCode_Addr2Line(code, addr); + return fr->instr_ptr < _PyCode_CODE(code) + code_copy->_co_firsttraceable; } int PopulateFrames(CallFrame *frames, PyThreadState *ts) { @@ -114,22 +90,25 @@ int PopulateFrames(CallFrame *frames, PyThreadState *ts) { return 1; } - _PyInterpreterFrame *frame = unsafe_PyThreadState_GetInterpreterFrame(ts); + _PyInterpreterFrame *faddr = ts->current_frame; int num_frames = 0; - while (frame != nullptr && num_frames < kMaxFramesToCapture) { - // Get code object and line number - might be NULL/-1 if we hit a race condition - PyCodeObject *code = unsafe_PyInterpreterFrame_GetCode(frame); - int lineno = _PyInterpreterFrame_GetLine(frame); - - // Only record frames where we successfully got valid data - // This handles race conditions where frame is partially initialized - if (code != NULL && lineno >= 0) { - frames[num_frames].lineno = lineno; + 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++; } - - frame = unsafe_PyInterpreterFrame_GetBack(frame); + faddr = fr.previous; } return num_frames; } @@ -140,99 +119,38 @@ int PopulateFrames(CallFrame *frames, PyThreadState *ts) { * 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 + * - 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. * - * Since this code runs as part of the SIGPROF handler, it cannot modify Python - * objects (including their refcounts) and standard getters can't be used. - * We expose the internal _PyInterpreterFrame and use that directly. + * 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 CPython 3.12 source for async-signal-safe access -// -// IMPORTANT: This can be called from a signal handler (SIGPROF), which can -// interrupt the Python interpreter at ANY point, including during frame setup -// in _PyEval_EvalFrameDefault. Specifically, the signal can fire after: -// tstate->cframe = &cframe; -// but before: -// cframe.current_frame = frame; -// This creates a race condition where cframe is set but current_frame is -// uninitialized, causing segfaults when we dereference it. We must check -// for NULL at every step. -static inline _PyInterpreterFrame *unsafe_PyThreadState_GetInterpreterFrame( - PyThreadState *tstate) { - if (tstate == NULL) { - return NULL; - } - - // Check if cframe is set - might be NULL during initialization - _PyCFrame *cframe = tstate->cframe; - if (cframe == NULL) { - return NULL; - } - - // CRITICAL: Check if current_frame is set - might be uninitialized - // if we interrupted during _PyEval_EvalFrameDefault setup - _PyInterpreterFrame *f = cframe->current_frame; - if (f == NULL) { - return NULL; - } - - while (f && _PyFrame_IsIncomplete(f)) { - f = f->previous; - } - return f; -} - -// In Python 3.12, f_code is still PyCodeObject* but moved to first position -static inline PyCodeObject *unsafe_PyInterpreterFrame_GetCode( - _PyInterpreterFrame *frame) { - if (frame == NULL || _PyFrame_IsIncomplete(frame)) { - return NULL; +// 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; } - - PyCodeObject *code = frame->f_code; - if (code == NULL) { - return NULL; - } - - return code; -} - -static inline _PyInterpreterFrame *unsafe_PyInterpreterFrame_GetBack( - _PyInterpreterFrame *frame) { - if (frame == NULL || _PyFrame_IsIncomplete(frame)) { - return NULL; - } - - _PyInterpreterFrame *prev = frame->previous; - while (prev && _PyFrame_IsIncomplete(prev)) { - prev = prev->previous; - } - return prev; -} - -// Python 3.12 still uses prev_instr (not renamed yet) -int _PyInterpreterFrame_GetLine(_PyInterpreterFrame *frame) { - if (frame == NULL) { - return -1; + if (fr->owner == FRAME_OWNED_BY_GENERATOR) { + return false; } - - PyCodeObject *code = frame->f_code; - if (code == NULL) { - return -1; + PyCodeObject code; + if (!SafeCopy(&code, fr->f_code, sizeof(code))) { + return true; // unreadable code object: treat as incomplete (skip) } - - int addr = _PyInterpreterFrame_LASTI(frame) * sizeof(_Py_CODEUNIT); - return PyCode_Addr2Line(code, addr); + return fr->prev_instr < _PyCode_CODE(fr->f_code) + code._co_firsttraceable; } int PopulateFrames(CallFrame *frames, PyThreadState *ts) { @@ -242,22 +160,35 @@ int PopulateFrames(CallFrame *frames, PyThreadState *ts) { return 1; } - _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) { - // Get code object and line number - might be NULL/-1 if we hit a race condition - PyCodeObject *code = unsafe_PyInterpreterFrame_GetCode(frame); - int lineno = _PyInterpreterFrame_GetLine(frame); - - // Only record frames where we successfully got valid data - // This handles race conditions where frame is partially initialized - if (code != NULL && lineno >= 0) { - frames[num_frames].lineno = lineno; - frames[num_frames].py_code = code; + 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++; } - - frame = unsafe_PyInterpreterFrame_GetBack(frame); + faddr = fr.previous; } return num_frames; } @@ -266,100 +197,34 @@ int PopulateFrames(CallFrame *frames, PyThreadState *ts) { /** * 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. * - * Since this code runs as part of the SIGPROF handler, it cannot modify Python - * objects (including their refcounts) and standard getters can't be used. - * We expose the internal _PyInterpreterFrame and use that directly. + * 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 -// -// IMPORTANT: This can be called from a signal handler (SIGPROF), which can -// interrupt the Python interpreter during frame setup, creating race conditions. -// See Python 3.12 comments above for details on the race condition in -// _PyEval_EvalFrameDefault where cframe is set before current_frame. -static inline _PyInterpreterFrame *unsafe_PyThreadState_GetInterpreterFrame( - PyThreadState *tstate) { - if (tstate == NULL) { - return NULL; - } - - // Check if cframe is set - might be NULL during initialization - _PyCFrame *cframe = tstate->cframe; - if (cframe == NULL) { - return NULL; - } - - // Check if current_frame is set - might be uninitialized - _PyInterpreterFrame *f = cframe->current_frame; - if (f == NULL) { - return NULL; - } - - while (f && _PyFrame_IsIncomplete(f)) { - f = f->previous; - } - return f; -} - -// Modified from -// https://github.com/python/cpython/blob/v3.11.4/Objects/frameobject.c#L1310-L1315 -// with refcounting removed and additional NULL checks for signal safety -static inline PyCodeObject *unsafe_PyInterpreterFrame_GetCode( - _PyInterpreterFrame *frame) { - if (frame == NULL || _PyFrame_IsIncomplete(frame)) { - return NULL; - } - - PyCodeObject *code = frame->f_code; - if (code == NULL) { - return NULL; - } - - return code; -} - -// Modified from -// https://github.com/python/cpython/blob/v3.11.4/Objects/frameobject.c#L1326-L1329 -// with refcounting removed and additional NULL checks for signal safety -static inline _PyInterpreterFrame *unsafe_PyInterpreterFrame_GetBack( - _PyInterpreterFrame *frame) { - if (frame == NULL || _PyFrame_IsIncomplete(frame)) { - return NULL; - } - - _PyInterpreterFrame *prev = frame->previous; - while (prev && _PyFrame_IsIncomplete(prev)) { - prev = prev->previous; - } - return prev; -} - -// Copied from -// https://github.com/python/cpython/blob/v3.11.4/Python/frame.c#L165-L170 as -// this function is not available in libpython -// Added NULL checks for signal safety -int _PyInterpreterFrame_GetLine(_PyInterpreterFrame *frame) { - if (frame == NULL) { - return -1; +// 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 = frame->f_code; - if (code == NULL) { - return -1; + PyCodeObject code; + if (!SafeCopy(&code, fr->f_code, sizeof(code))) { + return true; // unreadable code object: treat as incomplete (skip) } - - int addr = _PyInterpreterFrame_LASTI(frame) * sizeof(_Py_CODEUNIT); - return PyCode_Addr2Line(code, addr); + return fr->prev_instr < _PyCode_CODE(fr->f_code) + code._co_firsttraceable; } int PopulateFrames(CallFrame *frames, PyThreadState *ts) { @@ -369,26 +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) { - // Get code object and line number - might be NULL/-1 if we hit a race condition - PyCodeObject *code = unsafe_PyInterpreterFrame_GetCode(frame); - int lineno = _PyInterpreterFrame_GetLine(frame); - - // Only record frames where we successfully got valid data - // This handles race conditions where frame is partially initialized - if (code != NULL && lineno >= 0) { - frames[num_frames].lineno = lineno; - frames[num_frames].py_code = code; + 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++; } - - frame = unsafe_PyInterpreterFrame_GetBack(frame); + faddr = fr.previous; } return num_frames; } @@ -416,4 +290,3 @@ int PopulateFrames(CallFrame *frames, PyThreadState *ts) { } #endif // PY_VERSION_HEX >= PY_311 - 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;