From 87d3b9db4ade1aa100ee6f065082cb7e85b8992f Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 25 Mar 2020 19:27:36 +0100 Subject: bpo-39882: Add _Py_FatalErrorFormat() function (GH-19157) --- Include/cpython/pyerrors.h | 5 +++ Modules/_io/bufferedio.c | 9 ++-- Objects/call.c | 6 ++- Objects/frameobject.c | 10 +++-- Objects/obmalloc.c | 32 +++++--------- Parser/grammar1.c | 2 +- Parser/tokenizer.c | 2 +- Python/ast.c | 9 ++-- Python/compile.c | 28 ++++++------- Python/frozenmain.c | 2 +- Python/import.c | 6 +-- Python/mysnprintf.c | 10 ++--- Python/pathconfig.c | 14 +++++-- Python/pylifecycle.c | 101 ++++++++++++++++++++++++++++++++------------- Python/pystate.c | 30 ++++++++------ 15 files changed, 157 insertions(+), 109 deletions(-) diff --git a/Include/cpython/pyerrors.h b/Include/cpython/pyerrors.h index ab2e740..cdd0520 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 f2d0467..6e76db4 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) : ""); } return 1; } diff --git a/Objects/call.c b/Objects/call.c index 37d079d..0861414 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 64f5754..340267b 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 3b574bf..3f6f9cf 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 e0b8fbb..c702040 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 75e8da4..c650442 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 2b74ed4..fb23c02 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 1175c8f..0b3926c 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 7f9cc19..508721b 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 936ad1f..645ebdf 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 a08e249..945a81a 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 3756e3a..aa1d6f8 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 b7019e3..08a727f 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, ""); + if (header) { + fprintf(stream, "Fatal Python error: "); + if (prefix) { + fputs(prefix, stream); + fputs(": ", stream); + } + if (msg) { + fputs(msg, stream); + } + else { + fprintf(stream, ""); + } + 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 bd2e44d..c7154ea 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; -- cgit v0.12