bpo-39882: Add _Py_FatalErrorFormat() function (GH-19157)

15 files changed, 157 insertions, 109 deletions

diff --git a/Include/cpython/pyerrors.h b/Include/cpython/pyerrors.h
index ab2e74018b..cdd052026c 100644
--- a/Include/cpython/pyerrors.h
+++ b/Include/cpython/pyerrors.h
@@ -182,6 +182,11 @@ PyAPI_FUNC(void) _Py_NO_RETURN _Py_FatalErrorFunc(
     const char *func,
     const char *message);
 
+PyAPI_FUNC(void) _Py_NO_RETURN _Py_FatalErrorFormat(
+    const char *func,
+    const char *format,
+    ...);
+
 #define Py_FatalError(message) _Py_FatalErrorFunc(__func__, message)
 
 #ifdef __cplusplus
diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c
index f2d0467a3f..6e76db42c3 100644
--- a/Modules/_io/bufferedio.c
+++ b/Modules/_io/bufferedio.c
@@ -292,12 +292,11 @@ _enter_buffered_busy(buffered *self)
     }
     Py_END_ALLOW_THREADS
     if (relax_locking && st != PY_LOCK_ACQUIRED) {
-        PyObject *msgobj = PyUnicode_FromFormat(
-            "could not acquire lock for %A at interpreter "
+        PyObject *ascii = PyObject_ASCII((PyObject*)self);
+        _Py_FatalErrorFormat(__func__,
+            "could not acquire lock for %s at interpreter "
             "shutdown, possibly due to daemon threads",
-            (PyObject *) self);
-        const char *msg = PyUnicode_AsUTF8(msgobj);
-        Py_FatalError(msg);
+            ascii ? PyUnicode_AsUTF8(ascii) : "<ascii(self) failed>");
     }
     return 1;
 }
diff --git a/Objects/call.c b/Objects/call.c
index 37d079d169..0861414324 100644
--- a/Objects/call.c
+++ b/Objects/call.c
@@ -46,7 +46,8 @@ _Py_CheckFunctionResult(PyThreadState *tstate, PyObject *callable,
                               "%s returned NULL without setting an error",
                               where);
 #ifdef Py_DEBUG
-            /* Ensure that the bug is caught in debug mode */
+            /* Ensure that the bug is caught in debug mode.
+               Py_FatalError() logs the SystemError exception raised above. */
             Py_FatalError("a function returned NULL without setting an error");
 #endif
             return NULL;
@@ -67,7 +68,8 @@ _Py_CheckFunctionResult(PyThreadState *tstate, PyObject *callable,
                     "%s returned a result with an error set", where);
             }
 #ifdef Py_DEBUG
-            /* Ensure that the bug is caught in debug mode */
+            /* Ensure that the bug is caught in debug mode.
+               Py_FatalError() logs the SystemError exception raised above. */
             Py_FatalError("a function returned a result with an error set");
 #endif
             return NULL;
diff --git a/Objects/frameobject.c b/Objects/frameobject.c
index 64f5754fe2..340267bd47 100644
--- a/Objects/frameobject.c
+++ b/Objects/frameobject.c
@@ -957,8 +957,9 @@ void
 PyFrame_BlockSetup(PyFrameObject *f, int type, int handler, int level)
 {
     PyTryBlock *b;
-    if (f->f_iblock >= CO_MAXBLOCKS)
-        Py_FatalError("XXX block stack overflow");
+    if (f->f_iblock >= CO_MAXBLOCKS) {
+        Py_FatalError("block stack overflow");
+    }
     b = &f->f_blockstack[f->f_iblock++];
     b->b_type = type;
     b->b_level = level;
@@ -969,8 +970,9 @@ PyTryBlock *
 PyFrame_BlockPop(PyFrameObject *f)
 {
     PyTryBlock *b;
-    if (f->f_iblock <= 0)
-        Py_FatalError("XXX block stack underflow");
+    if (f->f_iblock <= 0) {
+        Py_FatalError("block stack underflow");
+    }
     b = &f->f_blockstack[--f->f_iblock];
     return b;
 }
diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c
index 3b574bffd8..3f6f9cf9ca 100644
--- a/Objects/obmalloc.c
+++ b/Objects/obmalloc.c
@@ -2361,26 +2361,22 @@ _PyMem_DebugRealloc(void *ctx, void *ptr, size_t nbytes)
 static void
 _PyMem_DebugCheckAddress(const char *func, char api, const void *p)
 {
+    assert(p != NULL);
+
     const uint8_t *q = (const uint8_t *)p;
-    char msgbuf[64];
-    const char *msg;
     size_t nbytes;
     const uint8_t *tail;
     int i;
     char id;
 
-    if (p == NULL) {
-        msg = "didn't expect a NULL pointer";
-        goto error;
-    }
-
     /* Check the API id */
     id = (char)q[-SST];
     if (id != api) {
-        msg = msgbuf;
-        snprintf(msgbuf, sizeof(msgbuf), "bad ID: Allocated using API '%c', verified using API '%c'", id, api);
-        msgbuf[sizeof(msgbuf)-1] = 0;
-        goto error;
+        _PyObject_DebugDumpAddress(p);
+        _Py_FatalErrorFormat(func,
+                             "bad ID: Allocated using API '%c', "
+                             "verified using API '%c'",
+                             id, api);
     }
 
     /* Check the stuff at the start of p first:  if there's underwrite
@@ -2389,8 +2385,8 @@ _PyMem_DebugCheckAddress(const char *func, char api, const void *p)
      */
     for (i = SST-1; i >= 1; --i) {
         if (*(q-i) != PYMEM_FORBIDDENBYTE) {
-            msg = "bad leading pad byte";
-            goto error;
+            _PyObject_DebugDumpAddress(p);
+            _Py_FatalErrorFunc(func, "bad leading pad byte");
         }
     }
 
@@ -2398,16 +2394,10 @@ _PyMem_DebugCheckAddress(const char *func, char api, const void *p)
     tail = q + nbytes;
     for (i = 0; i < SST; ++i) {
         if (tail[i] != PYMEM_FORBIDDENBYTE) {
-            msg = "bad trailing pad byte";
-            goto error;
+            _PyObject_DebugDumpAddress(p);
+            _Py_FatalErrorFunc(func, "bad trailing pad byte");
         }
     }
-
-    return;
-
-error:
-    _PyObject_DebugDumpAddress(p);
-    _Py_FatalErrorFunc(func, msg);
 }
 
 /* Display info to stderr about the memory block at p. */
diff --git a/Parser/grammar1.c b/Parser/grammar1.c
index e0b8fbb8b8..c702040efd 100644
--- a/Parser/grammar1.c
+++ b/Parser/grammar1.c
@@ -41,7 +41,7 @@ PyGrammar_LabelRepr(label *lb)
         }
     }
     else {
-        Py_FatalError("invalid label");
+        Py_FatalError("invalid grammar label");
         return NULL;
     }
 }
diff --git a/Parser/tokenizer.c b/Parser/tokenizer.c
index 75e8da4362..c650442a4a 100644
--- a/Parser/tokenizer.c
+++ b/Parser/tokenizer.c
@@ -1032,7 +1032,7 @@ tok_backup(struct tok_state *tok, int c)
 {
     if (c != EOF) {
         if (--tok->cur < tok->buf) {
-            Py_FatalError("beginning of buffer");
+            Py_FatalError("tokenizer beginning of buffer");
         }
         if (*tok->cur != c) {
             *tok->cur = c;
diff --git a/Python/ast.c b/Python/ast.c
index 2b74ed4dbd..fb23c026e8 100644
--- a/Python/ast.c
+++ b/Python/ast.c
@@ -727,11 +727,8 @@ num_stmts(const node *n)
                 return l;
             }
         default: {
-            char buf[128];
-
-            sprintf(buf, "Non-statement found: %d %d",
-                    TYPE(n), NCH(n));
-            Py_FatalError(buf);
+            _Py_FatalErrorFormat(__func__, "Non-statement found: %d %d",
+                                 TYPE(n), NCH(n));
         }
     }
     Py_UNREACHABLE();
@@ -1664,7 +1661,7 @@ ast_for_decorator(struct compiling *c, const node *n)
     REQ(n, decorator);
     REQ(CHILD(n, 0), AT);
     REQ(CHILD(n, 2), NEWLINE);
-    
+
     return ast_for_expr(c, CHILD(n, 1));
 }
 
diff --git a/Python/compile.c b/Python/compile.c
index 1175c8fa6f..0b3926c436 100644
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -1876,18 +1876,15 @@ get_ref_type(struct compiler *c, PyObject *name)
         return CELL;
     scope = PyST_GetScope(c->u->u_ste, name);
     if (scope == 0) {
-        char buf[350];
-        PyOS_snprintf(buf, sizeof(buf),
-                      "unknown scope for %.100s in %.100s(%s)\n"
-                      "symbols: %s\nlocals: %s\nglobals: %s",
-                      PyUnicode_AsUTF8(name),
-                      PyUnicode_AsUTF8(c->u->u_name),
-                      PyUnicode_AsUTF8(PyObject_Repr(c->u->u_ste->ste_id)),
-                      PyUnicode_AsUTF8(PyObject_Repr(c->u->u_ste->ste_symbols)),
-                      PyUnicode_AsUTF8(PyObject_Repr(c->u->u_varnames)),
-                      PyUnicode_AsUTF8(PyObject_Repr(c->u->u_names))
-        );
-        Py_FatalError(buf);
+        _Py_FatalErrorFormat(__func__,
+           "unknown scope for %.100s in %.100s(%s)\n"
+           "symbols: %s\nlocals: %s\nglobals: %s",
+           PyUnicode_AsUTF8(name),
+           PyUnicode_AsUTF8(c->u->u_name),
+           PyUnicode_AsUTF8(PyObject_Repr(c->u->u_ste->ste_id)),
+           PyUnicode_AsUTF8(PyObject_Repr(c->u->u_ste->ste_symbols)),
+           PyUnicode_AsUTF8(PyObject_Repr(c->u->u_varnames)),
+           PyUnicode_AsUTF8(PyObject_Repr(c->u->u_names)));
     }
 
     return scope;
@@ -1930,7 +1927,7 @@ compiler_make_closure(struct compiler *c, PyCodeObject *co, Py_ssize_t flags, Py
             else /* (reftype == FREE) */
                 arg = compiler_lookup_arg(c->u->u_freevars, name);
             if (arg == -1) {
-                fprintf(stderr,
+                _Py_FatalErrorFormat(__func__,
                     "lookup %s in %s %d %d\n"
                     "freevars of %s: %s\n",
                     PyUnicode_AsUTF8(PyObject_Repr(name)),
@@ -1938,7 +1935,6 @@ compiler_make_closure(struct compiler *c, PyCodeObject *co, Py_ssize_t flags, Py
                     reftype, arg,
                     PyUnicode_AsUTF8(co->co_name),
                     PyUnicode_AsUTF8(PyObject_Repr(co->co_freevars)));
-                Py_FatalError("compiler_make_closure()");
             }
             ADDOP_I(c, LOAD_CLOSURE, arg);
         }
@@ -5411,8 +5407,8 @@ stackdepth(struct compiler *c)
             struct instr *instr = &b->b_instr[i];
             int effect = stack_effect(instr->i_opcode, instr->i_oparg, 0);
             if (effect == PY_INVALID_STACK_EFFECT) {
-                fprintf(stderr, "opcode = %d\n", instr->i_opcode);
-                Py_FatalError("PyCompile_OpcodeStackEffect()");
+                _Py_FatalErrorFormat(__func__,
+                    "opcode = %d", instr->i_opcode);
             }
             int new_depth = depth + effect;
             if (new_depth > maxdepth) {
diff --git a/Python/frozenmain.c b/Python/frozenmain.c
index 7f9cc19317..508721b855 100644
--- a/Python/frozenmain.c
+++ b/Python/frozenmain.c
@@ -99,7 +99,7 @@ Py_FrozenMain(int argc, char **argv)
 
     n = PyImport_ImportFrozenModule("__main__");
     if (n == 0)
-        Py_FatalError("__main__ not frozen");
+        Py_FatalError("the __main__ module is not frozen");
     if (n < 0) {
         PyErr_Print();
         sts = 1;
diff --git a/Python/import.c b/Python/import.c
index 936ad1fd50..645ebdf7b3 100644
--- a/Python/import.c
+++ b/Python/import.c
@@ -209,7 +209,7 @@ _PyImport_ReInitLock(void)
     if (import_lock != NULL) {
         import_lock = PyThread_allocate_lock();
         if (import_lock == NULL) {
-            Py_FatalError("PyImport_ReInitLock failed to create a new lock");
+            _Py_FatalErrorFunc(__func__, "failed to create a new lock");
         }
     }
     if (import_lock_level > 1) {
@@ -310,7 +310,7 @@ PyImport_GetModuleDict(void)
 {
     PyInterpreterState *interp = _PyInterpreterState_GET_UNSAFE();
     if (interp->modules == NULL) {
-        Py_FatalError("no module dictionary");
+        Py_FatalError("interpreter has no modules dictionary");
     }
     return interp->modules;
 }
@@ -982,7 +982,7 @@ PyImport_ExecCodeModuleWithPathnames(const char *name, PyObject *co,
         _Py_IDENTIFIER(_get_sourcefile);
 
         if (interp == NULL) {
-            Py_FatalError("no interpreter!");
+            Py_FatalError("no current interpreter");
         }
 
         external= PyObject_GetAttrString(interp->importlib,
diff --git a/Python/mysnprintf.c b/Python/mysnprintf.c
index a08e249b42..945a81abb0 100644
--- a/Python/mysnprintf.c
+++ b/Python/mysnprintf.c
@@ -81,12 +81,12 @@ PyOS_vsnprintf(char *str, size_t size, const char  *format, va_list va)
     }
 
     len = vsprintf(buffer, format, va);
-    if (len < 0)
+    if (len < 0) {
         /* ignore the error */;
-
-    else if ((size_t)len >= size + _PyOS_vsnprintf_EXTRA_SPACE)
-        Py_FatalError("Buffer overflow in PyOS_snprintf/PyOS_vsnprintf");
-
+    }
+    else if ((size_t)len >= size + _PyOS_vsnprintf_EXTRA_SPACE) {
+        _Py_FatalErrorFunc(__func__, "Buffer overflow");
+    }
     else {
         const size_t to_copy = (size_t)len < size ?
                                 (size_t)len : size - 1;
diff --git a/Python/pathconfig.c b/Python/pathconfig.c
index 3756e3a9b8..aa1d6f8139 100644
--- a/Python/pathconfig.c
+++ b/Python/pathconfig.c
@@ -484,6 +484,12 @@ pathconfig_global_init(void)
 
 /* External interface */
 
+static void _Py_NO_RETURN
+path_out_of_memory(const char *func)
+{
+    _Py_FatalErrorFunc(func, "out of memory");
+}
+
 void
 Py_SetPath(const wchar_t *path)
 {
@@ -515,7 +521,7 @@ Py_SetPath(const wchar_t *path)
         || _Py_path_config.exec_prefix == NULL
         || _Py_path_config.module_search_path == NULL)
     {
-        Py_FatalError("out of memory");
+        path_out_of_memory(__func__);
     }
 }
 
@@ -536,7 +542,7 @@ Py_SetPythonHome(const wchar_t *home)
     PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc);
 
     if (_Py_path_config.home == NULL) {
-        Py_FatalError("out of memory");
+        path_out_of_memory(__func__);
     }
 }
 
@@ -557,7 +563,7 @@ Py_SetProgramName(const wchar_t *program_name)
     PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc);
 
     if (_Py_path_config.program_name == NULL) {
-        Py_FatalError("out of memory");
+        path_out_of_memory(__func__);
     }
 }
 
@@ -577,7 +583,7 @@ _Py_SetProgramFullPath(const wchar_t *program_full_path)
     PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc);
 
     if (_Py_path_config.program_full_path == NULL) {
-        Py_FatalError("out of memory");
+        path_out_of_memory(__func__);
     }
 }
 
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index b7019e3bb6..08a727fb29 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -2176,33 +2176,50 @@ fatal_error_dump_runtime(FILE *stream, _PyRuntimeState *runtime)
 }
 
 
+static inline void _Py_NO_RETURN
+fatal_error_exit(int status)
+{
+    if (status < 0) {
+#if defined(MS_WINDOWS) && defined(_DEBUG)
+        DebugBreak();
+#endif
+        abort();
+    }
+    else {
+        exit(status);
+    }
+}
+
+
 static void _Py_NO_RETURN
-fatal_error(const char *prefix, const char *msg, int status)
+fatal_error(FILE *stream, int header, const char *prefix, const char *msg,
+            int status)
 {
-    FILE *stream = stderr;
     const int fd = fileno(stream);
     static int reentrant = 0;
 
     if (reentrant) {
         /* Py_FatalError() caused a second fatal error.
            Example: flush_std_files() raises a recursion error. */
-        goto exit;
+        fatal_error_exit(status);
     }
     reentrant = 1;
 
-    fprintf(stream, "Fatal Python error: ");
-    if (prefix) {
-        fputs(prefix, stream);
-        fputs(": ", stream);
-    }
-    if (msg) {
-        fputs(msg, stream);
-    }
-    else {
-        fprintf(stream, "<message not set>");
+    if (header) {
+        fprintf(stream, "Fatal Python error: ");
+        if (prefix) {
+            fputs(prefix, stream);
+            fputs(": ", stream);
+        }
+        if (msg) {
+            fputs(msg, stream);
+        }
+        else {
+            fprintf(stream, "<message not set>");
+        }
+        fputs("\n", stream);
+        fflush(stream);
     }
-    fputs("\n", stream);
-    fflush(stream); /* it helps in Windows debug build */
 
     _PyRuntimeState *runtime = &_PyRuntime;
     fatal_error_dump_runtime(stream, runtime);
@@ -2250,32 +2267,60 @@ fatal_error(const char *prefix, const char *msg, int status)
     fatal_output_debug(msg);
 #endif /* MS_WINDOWS */
 
-exit:
-    if (status < 0) {
-#if defined(MS_WINDOWS) && defined(_DEBUG)
-        DebugBreak();
-#endif
-        abort();
-    }
-    else {
-        exit(status);
-    }
+    fatal_error_exit(status);
 }
 
+
 #undef Py_FatalError
 
 void _Py_NO_RETURN
 Py_FatalError(const char *msg)
 {
-    fatal_error(NULL, msg, -1);
+    fatal_error(stderr, 1, NULL, msg, -1);
 }
 
+
 void _Py_NO_RETURN
 _Py_FatalErrorFunc(const char *func, const char *msg)
 {
-    fatal_error(func, msg, -1);
+    fatal_error(stderr, 1, func, msg, -1);
+}
+
+
+void _Py_NO_RETURN
+_Py_FatalErrorFormat(const char *func, const char *format, ...)
+{
+    static int reentrant = 0;
+    if (reentrant) {
+        /* _Py_FatalErrorFormat() caused a second fatal error */
+        fatal_error_exit(-1);
+    }
+    reentrant = 1;
+
+    FILE *stream = stderr;
+    fprintf(stream, "Fatal Python error: ");
+    if (func) {
+        fputs(func, stream);
+        fputs(": ", stream);
+    }
+    fflush(stream);
+
+    va_list vargs;
+#ifdef HAVE_STDARG_PROTOTYPES
+    va_start(vargs, format);
+#else
+    va_start(vargs);
+#endif
+    vfprintf(stream, format, vargs);
+    va_end(vargs);
+
+    fputs("\n", stream);
+    fflush(stream);
+
+    fatal_error(stream, 0, NULL, NULL, -1);
 }
 
+
 void _Py_NO_RETURN
 Py_ExitStatusException(PyStatus status)
 {
@@ -2283,7 +2328,7 @@ Py_ExitStatusException(PyStatus status)
         exit(status.exitcode);
     }
     else if (_PyStatus_IS_ERROR(status)) {
-        fatal_error(status.func, status.err_msg, 1);
+        fatal_error(stderr, 1, status.func, status.err_msg, 1);
     }
     else {
         Py_FatalError("Py_ExitStatusException() must not be called on success");
diff --git a/Python/pystate.c b/Python/pystate.c
index bd2e44d7a0..c7154ea706 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -331,7 +331,7 @@ PyInterpreterState_Delete(PyInterpreterState *interp)
     PyInterpreterState **p;
     for (p = &interpreters->head; ; p = &(*p)->next) {
         if (*p == NULL) {
-            Py_FatalError("invalid interp");
+            Py_FatalError("NULL interpreter");
         }
         if (*p == interp) {
             break;
@@ -393,7 +393,7 @@ _PyInterpreterState_DeleteExceptMain(_PyRuntimeState *runtime)
     HEAD_UNLOCK(runtime);
 
     if (interpreters->head == NULL) {
-        Py_FatalError("missing main");
+        Py_FatalError("missing main interpreter");
     }
     _PyThreadState_Swap(gilstate, tstate);
 }
@@ -686,7 +686,7 @@ int
 PyState_AddModule(PyObject* module, struct PyModuleDef* def)
 {
     if (!def) {
-        Py_FatalError("Module Definition is NULL");
+        Py_FatalError("module definition is NULL");
         return -1;
     }
 
@@ -697,7 +697,7 @@ PyState_AddModule(PyObject* module, struct PyModuleDef* def)
         index < PyList_GET_SIZE(interp->modules_by_index) &&
         module == PyList_GET_ITEM(interp->modules_by_index, index))
     {
-        Py_FatalError("Module already added");
+        _Py_FatalErrorFormat(__func__, "module %p already added", module);
         return -1;
     }
     return _PyState_AddModule(tstate, module, def);
@@ -715,7 +715,7 @@ PyState_RemoveModule(struct PyModuleDef* def)
     }
     state = _PyInterpreterState_GET_UNSAFE();
     if (index == 0) {
-        Py_FatalError("Module index invalid");
+        Py_FatalError("invalid module index");
         return -1;
     }
     if (state->modules_by_index == NULL) {
@@ -816,19 +816,23 @@ static void
 tstate_delete_common(PyThreadState *tstate,
                      struct _gilstate_runtime_state *gilstate)
 {
-    _PyRuntimeState *runtime = tstate->interp->runtime;
     ensure_tstate_not_null(__func__, tstate);
     PyInterpreterState *interp = tstate->interp;
     if (interp == NULL) {
-        Py_FatalError("NULL interp");
+        Py_FatalError("NULL interpreter");
     }
+    _PyRuntimeState *runtime = interp->runtime;
+
     HEAD_LOCK(runtime);
-    if (tstate->prev)
+    if (tstate->prev) {
         tstate->prev->next = tstate->next;
-    else
+    }
+    else {
         interp->tstate_head = tstate->next;
-    if (tstate->next)
+    }
+    if (tstate->next) {
         tstate->next->prev = tstate->prev;
+    }
     HEAD_UNLOCK(runtime);
 
     if (gilstate->autoInterpreterState &&
@@ -845,7 +849,7 @@ _PyThreadState_Delete(PyThreadState *tstate, int check_current)
     struct _gilstate_runtime_state *gilstate = &tstate->interp->runtime->gilstate;
     if (check_current) {
         if (tstate == _PyRuntimeGILState_GetThreadState(gilstate)) {
-            Py_FatalError("tstate is still current");
+            _Py_FatalErrorFormat(__func__, "tstate %p is still current", tstate);
         }
     }
     tstate_delete_common(tstate, gilstate);
@@ -1355,7 +1359,9 @@ PyGILState_Release(PyGILState_STATE oldstate)
        by release-only users can't hurt.
     */
     if (!PyThreadState_IsCurrent(tstate)) {
-        Py_FatalError("This thread state must be current when releasing");
+        _Py_FatalErrorFormat(__func__,
+                             "thread state %p must be current when releasing",
+                             tstate);
     }
     assert(PyThreadState_IsCurrent(tstate));
     --tstate->gilstate_counter;