pythongh-140815: Fix faulthandler for invalid/freed frame

faulthandler now detects if a frame or a code object is invalid or
freed.

Add helper functions:

* _PyCode_SafeAddr2Line()
* _PyFrame_SafeGetCode()
* _PyFrame_SafeGetBytecode()
* _PyFrame_SafeGetLasti()

_PyMem_IsPtrFreed() now also considers the pointer 0x1 as freed.

Author: vstinner

Include/internal/pycore_code.h

@@ -274,8 +274,11 @@ extern void _PyLineTable_InitAddressRange(
 /** API for traversing the line number table. */
 PyAPI_FUNC(int) _PyLineTable_NextAddressRange(PyCodeAddressRange *range);
 extern int _PyLineTable_PreviousAddressRange(PyCodeAddressRange *range);
-// This is used in dump_frame() in traceback.c without an attached tstate.
-extern int _PyCode_Addr2LineNoTstate(PyCodeObject *co, int addr);
+
+// Similar to PyCode_Addr2Line(), but return -1 if the code object is invalid.
+// Used by dump_frame() in Python/traceback.c.
+extern int _PyCode_SafeAddr2Line(PyCodeObject *co, int addr);
+
 
 /** API for executors */
 extern void _PyCode_Clear_Executors(PyCodeObject *code);

Include/internal/pycore_interpframe.h

@@ -24,6 +24,27 @@ 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.
+static inline PyCodeObject* _PyFrame_SafeGetCode(_PyInterpreterFrame *f) {
+    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)) {
+        return NULL;
+    }
+    if (!PyCode_Check(executable)) {
+        return NULL;
+    }
+    return (PyCodeObject *)executable;
+}
+
 static inline _Py_CODEUNIT *
 _PyFrame_GetBytecode(_PyInterpreterFrame *f)
 {
@@ -37,6 +58,41 @@ _PyFrame_GetBytecode(_PyInterpreterFrame *f)
 #endif
 }
 
+// Similar to _PyFrame_GetCode(), but return NULL if the code object is invalid
+// or freed. Used by _PyFrame_SafeGetLasti().
+static inline _Py_CODEUNIT *
+_PyFrame_SafeGetBytecode(_PyInterpreterFrame *f)
+{
+#ifdef Py_GIL_DISABLED
+    PyCodeObject *co = _PyFrame_SafeGetCode(f);
+    if (co == NULL) {
+        return NULL;
+    }
+    _PyCodeArray *tlbc = _PyCode_GetTLBCArray(co);
+    assert(f->tlbc_index >= 0 && f->tlbc_index < tlbc->size);
+    return (_Py_CODEUNIT *)tlbc->entries[f->tlbc_index];
+#else
+    PyCodeObject *co = _PyFrame_SafeGetCode(f);
+    if (co == NULL) {
+        return NULL;
+    }
+    return _PyCode_CODE(co);
+#endif
+}
+
+// Similar to _PyFrame_GetCode(), but return NULL if the frame is invalid or
+// freed. Used by dump_frame() in Python/traceback.c.
+static inline int
+_PyFrame_SafeGetLasti(struct _PyInterpreterFrame *frame)
+{
+    _Py_CODEUNIT *bytecode = _PyFrame_SafeGetBytecode(frame);
+    if (bytecode == NULL) {
+        return -1;
+    }
+    int lasti = ((int)(frame->instr_ptr - bytecode));
+    return lasti * sizeof(_Py_CODEUNIT);
+}
+
 static inline PyFunctionObject *_PyFrame_GetFunction(_PyInterpreterFrame *f) {
     PyObject *func = PyStackRef_AsPyObjectBorrow(f->f_funcobj);
     assert(PyFunction_Check(func));

Include/internal/pycore_pymem.h

@@ -54,12 +54,12 @@ static inline int _PyMem_IsPtrFreed(const void *ptr)
 {
     uintptr_t value = (uintptr_t)ptr;
 #if SIZEOF_VOID_P == 8
-    return (value == 0
+    return (value < 256  // NULL, 0x1, 0x2, ...
             || value == (uintptr_t)0xCDCDCDCDCDCDCDCD
             || value == (uintptr_t)0xDDDDDDDDDDDDDDDD
             || value == (uintptr_t)0xFDFDFDFDFDFDFDFD);
 #elif SIZEOF_VOID_P == 4
-    return (value == 0
+    return (value < 256
             || value == (uintptr_t)0xCDCDCDCD
             || value == (uintptr_t)0xDDDDDDDD
             || value == (uintptr_t)0xFDFDFDFD);

Objects/codeobject.c

@@ -1005,8 +1005,8 @@ PyCode_NewEmpty(const char *filename, const char *funcname, int firstlineno)
  * source location tracking (co_lines/co_positions)
  ******************/
 
-int
-_PyCode_Addr2LineNoTstate(PyCodeObject *co, int addrq)
+static int
+_PyCode_Addr2Line(PyCodeObject *co, int addrq)
 {
     if (addrq < 0) {
         return co->co_firstlineno;
@@ -1020,12 +1020,29 @@ _PyCode_Addr2LineNoTstate(PyCodeObject *co, int addrq)
     return _PyCode_CheckLineNumber(addrq, &bounds);
 }
 
+int
+_PyCode_SafeAddr2Line(PyCodeObject *co, int addrq)
+{
+    if (addrq < 0) {
+        return co->co_firstlineno;
+    }
+    if (co->_co_monitoring && co->_co_monitoring->lines) {
+        return _Py_Instrumentation_GetLine(co, addrq/sizeof(_Py_CODEUNIT));
+    }
+    if (!(addrq >= 0 && addrq < _PyCode_NBYTES(co))) {
+        return -1;
+    }
+    PyCodeAddressRange bounds;
+    _PyCode_InitAddressRange(co, &bounds);
+    return _PyCode_CheckLineNumber(addrq, &bounds);
+}
+
 int
 PyCode_Addr2Line(PyCodeObject *co, int addrq)
 {
     int lineno;
     Py_BEGIN_CRITICAL_SECTION(co);
-    lineno = _PyCode_Addr2LineNoTstate(co, addrq);
+    lineno = _PyCode_Addr2Line(co, addrq);
     Py_END_CRITICAL_SECTION();
     return lineno;
 }

Python/traceback.c

@@ -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 (0 <= lasti) {
+        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);
-        }
-
         if (MAX_FRAME_DEPTH <= depth) {
             if (MAX_FRAME_DEPTH < depth) {
                 PUTS(fd, "plus ");
@@ -1128,11 +1134,19 @@ dump_traceback(int fd, PyThreadState *tstate, int write_header)
             break;
         }
 
-        dump_frame(fd, frame);
+        if (dump_frame(fd, frame) < 0) {
+            PUTS(fd, "  <invalid frame>\n");
+            break;
+        }
+
         frame = frame->previous;
         if (frame == NULL) {
             break;
         }
+        if (_PyMem_IsPtrFreed(frame)) {
+            PUTS(fd, "  <freed frame>\n");
+            break;
+        }
         depth++;
     }
 }