Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 35.2k
gh-140815: Fix faulthandler for invalid/freed frame#140921
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
1cab1e540598379b261877d9a4b8c208539c273f9937645486239435File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -24,6 +24,36 @@ static inline PyCodeObject *_PyFrame_GetCode(_PyInterpreterFrame *f) { | ||
| return (PyCodeObject *)executable; | ||
| } | ||
| // Similar to _PyFrame_GetCode(), but return NULL if the frame is invalid or | ||
| // freed. Used by dump_frame() in Python/traceback.c. The function uses | ||
| // heuristics to detect freed memory, it's not 100% reliable. | ||
| static inline PyCodeObject* | ||
| _PyFrame_SafeGetCode(_PyInterpreterFrame *f) | ||
| { | ||
| // globals and builtins may be NULL on a legit frame, but it's unlikely. | ||
| // It's more likely that it's a sign of an invalid frame. | ||
| if (f->f_globals == NULL || f->f_builtins == NULL) { | ||
| return NULL; | ||
| } | ||
| if (PyStackRef_IsNull(f->f_executable)) { | ||
| return NULL; | ||
| } | ||
| void *ptr; | ||
| memcpy(&ptr, &f->f_executable, sizeof(f->f_executable)); | ||
| if (_PyMem_IsPtrFreed(ptr)) { | ||
| return NULL; | ||
| } | ||
| PyObject *executable = PyStackRef_AsPyObjectBorrow(f->f_executable); | ||
| if (_PyObject_IsFreed(executable)) { | ||
Comment on lines
+44
to
+48
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this patch work on release builds? I thought MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh. I didn't try on a release build! _PyMem_IsPtrFreed() and _PyObject_IsFreed() work better with debug allocators, you're correct. But they detect freed (reused) memory in some cases in a release build as well. They are just less reliable in release mode. I pushed more changes to make the heuristics stricter. | ||
| return NULL; | ||
| } | ||
| if (!PyCode_Check(executable)) { | ||
| return NULL; | ||
| } | ||
| return (PyCodeObject *)executable; | ||
| } | ||
| static inline _Py_CODEUNIT * | ||
| _PyFrame_GetBytecode(_PyInterpreterFrame *f) | ||
| { | ||
| @@ -37,6 +67,31 @@ _PyFrame_GetBytecode(_PyInterpreterFrame *f) | ||
| #endif | ||
| } | ||
| // Similar to PyUnstable_InterpreterFrame_GetLasti(), but return NULL if the | ||
| // frame is invalid or freed. Used by dump_frame() in Python/traceback.c. The | ||
| // function uses heuristics to detect freed memory, it's not 100% reliable. | ||
| static inline int | ||
| _PyFrame_SafeGetLasti(struct _PyInterpreterFrame *f) | ||
| { | ||
| // Code based on _PyFrame_GetBytecode() but replace _PyFrame_GetCode() | ||
| // with _PyFrame_SafeGetCode(). | ||
| PyCodeObject *co = _PyFrame_SafeGetCode(f); | ||
| if (co == NULL) { | ||
| return -1; | ||
| } | ||
vstinner marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| _Py_CODEUNIT *bytecode; | ||
| #ifdef Py_GIL_DISABLED | ||
| _PyCodeArray *tlbc = _PyCode_GetTLBCArray(co); | ||
| assert(f->tlbc_index >= 0 && f->tlbc_index < tlbc->size); | ||
| bytecode = (_Py_CODEUNIT *)tlbc->entries[f->tlbc_index]; | ||
| #else | ||
| bytecode = _PyCode_CODE(co); | ||
| #endif | ||
| return (int)(f->instr_ptr - bytecode) * sizeof(_Py_CODEUNIT); | ||
| } | ||
| static inline PyFunctionObject *_PyFrame_GetFunction(_PyInterpreterFrame *f) { | ||
| PyObject *func = PyStackRef_AsPyObjectBorrow(f->f_funcobj); | ||
| assert(PyFunction_Check(func)); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| :mod:`faulthandler` now detects if a frame or a code object is invalid or | ||
| freed. Patch by Victor Stinner. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1028,44 +1028,61 @@ _Py_DumpWideString(int fd, wchar_t *str) | ||
| /* Write a frame into the file fd: "File "xxx", line xxx in xxx". | ||
| This function is signal safe. */ | ||
| This function is signal safe. | ||
| static void | ||
| Return 0 on success. Return -1 if the frame is invalid. */ | ||
| static int | ||
| dump_frame(int fd, _PyInterpreterFrame *frame) | ||
| { | ||
| assert(frame->owner < FRAME_OWNED_BY_INTERPRETER); | ||
| if (frame->owner == FRAME_OWNED_BY_INTERPRETER) { | ||
| /* Ignore trampoline frame */ | ||
| return 0; | ||
| } | ||
| PyCodeObject *code =_PyFrame_GetCode(frame); | ||
| PyCodeObject *code = _PyFrame_SafeGetCode(frame); | ||
| if (code == NULL) { | ||
| return -1; | ||
| } | ||
| int res = 0; | ||
| PUTS(fd, " File "); | ||
| if (code->co_filename != NULL | ||
| && PyUnicode_Check(code->co_filename)) | ||
| { | ||
| PUTS(fd, "\""); | ||
| _Py_DumpASCII(fd, code->co_filename); | ||
| PUTS(fd, "\""); | ||
| } else { | ||
| } | ||
| else { | ||
| PUTS(fd, "???"); | ||
| res = -1; | ||
| } | ||
| int lasti = PyUnstable_InterpreterFrame_GetLasti(frame); | ||
| int lineno = _PyCode_Addr2LineNoTstate(code, lasti); | ||
| PUTS(fd, ", line "); | ||
| int lasti = _PyFrame_SafeGetLasti(frame); | ||
| int lineno = -1; | ||
| if (lasti >= 0) { | ||
| lineno = _PyCode_SafeAddr2Line(code, lasti); | ||
| } | ||
| if (lineno >= 0) { | ||
| _Py_DumpDecimal(fd, (size_t)lineno); | ||
| } | ||
| else { | ||
| PUTS(fd, "???"); | ||
| res = -1; | ||
| } | ||
| PUTS(fd, " in "); | ||
| if (code->co_name != NULL | ||
| && PyUnicode_Check(code->co_name)) { | ||
| PUTS(fd, " in "); | ||
| if (code->co_name != NULL && PyUnicode_Check(code->co_name)) { | ||
| _Py_DumpASCII(fd, code->co_name); | ||
| } | ||
| else { | ||
| PUTS(fd, "???"); | ||
| res = -1; | ||
| } | ||
| PUTS(fd, "\n"); | ||
| return res; | ||
| } | ||
| static int | ||
| @@ -1108,17 +1125,6 @@ dump_traceback(int fd, PyThreadState *tstate, int write_header) | ||
| unsigned int depth = 0; | ||
| while (1) { | ||
| if (frame->owner == FRAME_OWNED_BY_INTERPRETER) { | ||
| /* Trampoline frame */ | ||
| frame = frame->previous; | ||
| if (frame == NULL) { | ||
| break; | ||
| } | ||
| /* Can't have more than one shim frame in a row */ | ||
| assert(frame->owner != FRAME_OWNED_BY_INTERPRETER); | ||
vstinner marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| if (MAX_FRAME_DEPTH <= depth) { | ||
| if (MAX_FRAME_DEPTH < depth) { | ||
| PUTS(fd, "plus "); | ||
| @@ -1128,7 +1134,15 @@ dump_traceback(int fd, PyThreadState *tstate, int write_header) | ||
| break; | ||
| } | ||
| dump_frame(fd, frame); | ||
| if (_PyMem_IsPtrFreed(frame)) { | ||
| PUTS(fd, " <freed frame>\n"); | ||
| break; | ||
| } | ||
| if (dump_frame(fd, frame) < 0) { | ||
| PUTS(fd, " <invalid frame>\n"); | ||
| break; | ||
| } | ||
| frame = frame->previous; | ||
| if (frame == NULL) { | ||
| break; | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.