From 9e5d30cc99e34f4c3e7b2cd851de20816c9d1927 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Sat, 7 Mar 2020 00:54:20 +0100 Subject: [PATCH] bpo-39882: Py_FatalError() logs the function name (GH-18819) The Py_FatalError() function is replaced with a macro which logs automatically the name of the current function, unless the Py_LIMITED_API macro is defined. Changes: * Add _Py_FatalErrorFunc() function. * Remove the function name from the message of Py_FatalError() calls which included the function name. * Update tests. --- Doc/c-api/sys.rst | 7 ++++ Include/cpython/pyerrors.h | 6 +++ Include/pyerrors.h | 6 ++- Lib/test/test_capi.py | 20 +++++---- Lib/test/test_exceptions.py | 5 ++- Lib/test/test_faulthandler.py | 7 +++- Lib/test/test_io.py | 3 +- Lib/test/test_sys.py | 5 ++- .../2020-03-06-23-56-04.bpo-39882.Iqhcqm.rst | 3 ++ Objects/obmalloc.c | 25 +++++------ Parser/parser.c | 5 ++- Parser/tokenizer.c | 8 ++-- Python/ceval.c | 12 +++--- Python/import.c | 5 +-- Python/pathconfig.c | 8 ++-- Python/pylifecycle.c | 14 +++++-- Python/pystate.c | 42 +++++++++---------- 17 files changed, 112 insertions(+), 69 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2020-03-06-23-56-04.bpo-39882.Iqhcqm.rst diff --git a/Doc/c-api/sys.rst b/Doc/c-api/sys.rst index c851ff66487..9ac91790978 100644 --- a/Doc/c-api/sys.rst +++ b/Doc/c-api/sys.rst @@ -388,6 +388,13 @@ Process Control function :c:func:`abort` is called which will attempt to produce a :file:`core` file. + The ``Py_FatalError()`` function is replaced with a macro which logs + automatically the name of the current function, unless the + ``Py_LIMITED_API`` macro is defined. + + .. versionchanged:: 3.9 + Log the function name automatically. + .. c:function:: void Py_Exit(int status) diff --git a/Include/cpython/pyerrors.h b/Include/cpython/pyerrors.h index f8480fb79e5..ab2e74018b4 100644 --- a/Include/cpython/pyerrors.h +++ b/Include/cpython/pyerrors.h @@ -178,6 +178,12 @@ PyAPI_FUNC(void) _PyErr_WriteUnraisableMsg( const char *err_msg, PyObject *obj); +PyAPI_FUNC(void) _Py_NO_RETURN _Py_FatalErrorFunc( + const char *func, + const char *message); + +#define Py_FatalError(message) _Py_FatalErrorFunc(__func__, message) + #ifdef __cplusplus } #endif diff --git a/Include/pyerrors.h b/Include/pyerrors.h index 3fd133c57de..399bb7c3a6f 100644 --- a/Include/pyerrors.h +++ b/Include/pyerrors.h @@ -21,7 +21,11 @@ PyAPI_FUNC(void) PyErr_GetExcInfo(PyObject **, PyObject **, PyObject **); PyAPI_FUNC(void) PyErr_SetExcInfo(PyObject *, PyObject *, PyObject *); #endif -/* Defined in Python/pylifecycle.c */ +/* Defined in Python/pylifecycle.c + + The Py_FatalError() function is replaced with a macro which logs + automatically the name of the current function, unless the Py_LIMITED_API + macro is defined. */ PyAPI_FUNC(void) _Py_NO_RETURN Py_FatalError(const char *message); #if defined(Py_DEBUG) || defined(Py_LIMITED_API) diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index e65973c4646..ff18a211f8b 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -61,8 +61,8 @@ def test_no_FatalError_infinite_loop(self): self.assertEqual(out, b'') # This used to cause an infinite loop. self.assertTrue(err.rstrip().startswith( - b'Fatal Python error:' - b' PyThreadState_Get: no current thread')) + b'Fatal Python error: ' + b'PyThreadState_Get: no current thread')) def test_memoryview_from_NULL_pointer(self): self.assertRaises(ValueError, _testcapi.make_memoryview_from_NULL_pointer) @@ -197,7 +197,8 @@ def test_return_null_without_error(self): """) rc, out, err = assert_python_failure('-c', code) self.assertRegex(err.replace(b'\r', b''), - br'Fatal Python error: a function returned NULL ' + br'Fatal Python error: _Py_CheckFunctionResult: ' + br'a function returned NULL ' br'without setting an error\n' br'Python runtime state: initialized\n' br'SystemError: " r"at interpreter shutdown, possibly due to " r"daemon threads".format_map(locals())) diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index c5bd8a4b1ff..027f87e0d4c 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -269,6 +269,8 @@ def set_recursion_limit_at_depth(depth, limit): finally: sys.setrecursionlimit(oldlimit) + # The error message is specific to CPython + @test.support.cpython_only def test_recursionlimit_fatalerror(self): # A fatal error occurs if a second recursion limit is hit when recovering # from a first one. @@ -290,7 +292,8 @@ def f(): err = sub.communicate()[1] self.assertTrue(sub.returncode, sub.returncode) self.assertIn( - b"Fatal Python error: Cannot recover from stack overflow", + b"Fatal Python error: _Py_CheckRecursiveCall: " + b"Cannot recover from stack overflow", err) def test_getwindowsversion(self): diff --git a/Misc/NEWS.d/next/C API/2020-03-06-23-56-04.bpo-39882.Iqhcqm.rst b/Misc/NEWS.d/next/C API/2020-03-06-23-56-04.bpo-39882.Iqhcqm.rst new file mode 100644 index 00000000000..b5eae7af721 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2020-03-06-23-56-04.bpo-39882.Iqhcqm.rst @@ -0,0 +1,3 @@ +The :c:func:`Py_FatalError` function is replaced with a macro which logs +automatically the name of the current function, unless the ``Py_LIMITED_API`` +macro is defined. diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 3881ff17e06..3b574bffd82 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -25,7 +25,7 @@ static void* _PyMem_DebugRealloc(void *ctx, void *ptr, size_t size); static void _PyMem_DebugFree(void *ctx, void *p); static void _PyObject_DebugDumpAddress(const void *p); -static void _PyMem_DebugCheckAddress(char api_id, const void *p); +static void _PyMem_DebugCheckAddress(const char *func, char api_id, const void *p); static void _PyMem_SetupDebugHooksDomain(PyMemAllocatorDomain domain); @@ -2205,7 +2205,7 @@ _PyMem_DebugRawFree(void *ctx, void *p) uint8_t *q = (uint8_t *)p - 2*SST; /* address returned from malloc */ size_t nbytes; - _PyMem_DebugCheckAddress(api->api_id, p); + _PyMem_DebugCheckAddress(__func__, api->api_id, p); nbytes = read_size_t(q); nbytes += PYMEM_DEBUG_EXTRA_BYTES; memset(q, PYMEM_DEADBYTE, nbytes); @@ -2230,7 +2230,7 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) #define ERASED_SIZE 64 uint8_t save[2*ERASED_SIZE]; /* A copy of erased bytes. */ - _PyMem_DebugCheckAddress(api->api_id, p); + _PyMem_DebugCheckAddress(__func__, api->api_id, p); data = (uint8_t *)p; head = data - 2*SST; @@ -2314,25 +2314,26 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) } static inline void -_PyMem_DebugCheckGIL(void) +_PyMem_DebugCheckGIL(const char *func) { if (!PyGILState_Check()) { - Py_FatalError("Python memory allocator called " - "without holding the GIL"); + _Py_FatalErrorFunc(func, + "Python memory allocator called " + "without holding the GIL"); } } static void * _PyMem_DebugMalloc(void *ctx, size_t nbytes) { - _PyMem_DebugCheckGIL(); + _PyMem_DebugCheckGIL(__func__); return _PyMem_DebugRawMalloc(ctx, nbytes); } static void * _PyMem_DebugCalloc(void *ctx, size_t nelem, size_t elsize) { - _PyMem_DebugCheckGIL(); + _PyMem_DebugCheckGIL(__func__); return _PyMem_DebugRawCalloc(ctx, nelem, elsize); } @@ -2340,7 +2341,7 @@ _PyMem_DebugCalloc(void *ctx, size_t nelem, size_t elsize) static void _PyMem_DebugFree(void *ctx, void *ptr) { - _PyMem_DebugCheckGIL(); + _PyMem_DebugCheckGIL(__func__); _PyMem_DebugRawFree(ctx, ptr); } @@ -2348,7 +2349,7 @@ _PyMem_DebugFree(void *ctx, void *ptr) static void * _PyMem_DebugRealloc(void *ctx, void *ptr, size_t nbytes) { - _PyMem_DebugCheckGIL(); + _PyMem_DebugCheckGIL(__func__); return _PyMem_DebugRawRealloc(ctx, ptr, nbytes); } @@ -2358,7 +2359,7 @@ _PyMem_DebugRealloc(void *ctx, void *ptr, size_t nbytes) * The API id, is also checked. */ static void -_PyMem_DebugCheckAddress(char api, const void *p) +_PyMem_DebugCheckAddress(const char *func, char api, const void *p) { const uint8_t *q = (const uint8_t *)p; char msgbuf[64]; @@ -2406,7 +2407,7 @@ _PyMem_DebugCheckAddress(char api, const void *p) error: _PyObject_DebugDumpAddress(p); - Py_FatalError(msg); + _Py_FatalErrorFunc(func, msg); } /* Display info to stderr about the memory block at p. */ diff --git a/Parser/parser.c b/Parser/parser.c index 227b9184f47..a61b2f5ebf7 100644 --- a/Parser/parser.c +++ b/Parser/parser.c @@ -54,8 +54,9 @@ s_push(stack *s, const dfa *d, node *parent) static void s_pop(stack *s) { - if (s_empty(s)) - Py_FatalError("s_pop: parser stack underflow -- FATAL"); + if (s_empty(s)) { + Py_FatalError("parser stack underflow"); + } s->s_top++; } diff --git a/Parser/tokenizer.c b/Parser/tokenizer.c index f82b1029981..75e8da43622 100644 --- a/Parser/tokenizer.c +++ b/Parser/tokenizer.c @@ -1031,10 +1031,12 @@ static void tok_backup(struct tok_state *tok, int c) { if (c != EOF) { - if (--tok->cur < tok->buf) - Py_FatalError("tok_backup: beginning of buffer"); - if (*tok->cur != c) + if (--tok->cur < tok->buf) { + Py_FatalError("beginning of buffer"); + } + if (*tok->cur != c) { *tok->cur = c; + } } } diff --git a/Python/ceval.c b/Python/ceval.c index 20e32e224e7..04e0824727e 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -283,7 +283,7 @@ PyEval_AcquireLock(void) struct _ceval_runtime_state *ceval = &runtime->ceval; PyThreadState *tstate = _PyRuntimeState_GetThreadState(runtime); if (tstate == NULL) { - Py_FatalError("PyEval_AcquireLock: current thread state is NULL"); + Py_FatalError("current thread state is NULL"); } take_gil(ceval, tstate); exit_thread_if_finalizing(tstate); @@ -314,7 +314,7 @@ PyEval_AcquireThread(PyThreadState *tstate) take_gil(ceval, tstate); exit_thread_if_finalizing(tstate); if (_PyThreadState_Swap(&runtime->gilstate, tstate) != NULL) { - Py_FatalError("PyEval_AcquireThread: non-NULL old thread state"); + Py_FatalError("non-NULL old thread state"); } } @@ -326,7 +326,7 @@ PyEval_ReleaseThread(PyThreadState *tstate) _PyRuntimeState *runtime = tstate->interp->runtime; PyThreadState *new_tstate = _PyThreadState_Swap(&runtime->gilstate, NULL); if (new_tstate != tstate) { - Py_FatalError("PyEval_ReleaseThread: wrong thread state"); + Py_FatalError("wrong thread state"); } drop_gil(&runtime->ceval, tstate); } @@ -373,7 +373,7 @@ PyEval_SaveThread(void) struct _ceval_runtime_state *ceval = &runtime->ceval; PyThreadState *tstate = _PyThreadState_Swap(&runtime->gilstate, NULL); if (tstate == NULL) { - Py_FatalError("PyEval_SaveThread: NULL tstate"); + Py_FatalError("NULL tstate"); } assert(gil_created(&ceval->gil)); drop_gil(ceval, tstate); @@ -1236,7 +1236,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) if (_Py_atomic_load_relaxed(&ceval->gil_drop_request)) { /* Give another thread a chance */ if (_PyThreadState_Swap(&runtime->gilstate, NULL) != tstate) { - Py_FatalError("ceval: tstate mix-up"); + Py_FatalError("tstate mix-up"); } drop_gil(ceval, tstate); @@ -1248,7 +1248,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) exit_thread_if_finalizing(tstate); if (_PyThreadState_Swap(&runtime->gilstate, tstate) != NULL) { - Py_FatalError("ceval: orphan tstate"); + Py_FatalError("orphan tstate"); } } /* Check for asynchronous exceptions. */ diff --git a/Python/import.c b/Python/import.c index 392d711299e..c4a19bc229e 100644 --- a/Python/import.c +++ b/Python/import.c @@ -310,7 +310,7 @@ PyImport_GetModuleDict(void) { PyInterpreterState *interp = _PyInterpreterState_GET_UNSAFE(); if (interp->modules == NULL) { - Py_FatalError("PyImport_GetModuleDict: no module dictionary!"); + Py_FatalError("no module dictionary"); } return interp->modules; } @@ -982,8 +982,7 @@ PyImport_ExecCodeModuleWithPathnames(const char *name, PyObject *co, _Py_IDENTIFIER(_get_sourcefile); if (interp == NULL) { - Py_FatalError("PyImport_ExecCodeModuleWithPathnames: " - "no interpreter!"); + Py_FatalError("no interpreter!"); } external= PyObject_GetAttrString(interp->importlib, diff --git a/Python/pathconfig.c b/Python/pathconfig.c index e37b5612366..3756e3a9b83 100644 --- a/Python/pathconfig.c +++ b/Python/pathconfig.c @@ -515,7 +515,7 @@ Py_SetPath(const wchar_t *path) || _Py_path_config.exec_prefix == NULL || _Py_path_config.module_search_path == NULL) { - Py_FatalError("Py_SetPath() failed: out of memory"); + Py_FatalError("out of memory"); } } @@ -536,7 +536,7 @@ Py_SetPythonHome(const wchar_t *home) PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); if (_Py_path_config.home == NULL) { - Py_FatalError("Py_SetPythonHome() failed: out of memory"); + Py_FatalError("out of memory"); } } @@ -557,7 +557,7 @@ Py_SetProgramName(const wchar_t *program_name) PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); if (_Py_path_config.program_name == NULL) { - Py_FatalError("Py_SetProgramName() failed: out of memory"); + Py_FatalError("out of memory"); } } @@ -577,7 +577,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("_Py_SetProgramFullPath() failed: out of memory"); + Py_FatalError("out of memory"); } } diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 23d74ee9503..9e3b2572794 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1611,10 +1611,10 @@ Py_EndInterpreter(PyThreadState *tstate) PyInterpreterState *interp = tstate->interp; if (tstate != _PyThreadState_GET()) { - Py_FatalError("Py_EndInterpreter: thread is not current"); + Py_FatalError("thread is not current"); } if (tstate->frame != NULL) { - Py_FatalError("Py_EndInterpreter: thread still has a frame"); + Py_FatalError("thread still has a frame"); } interp->finalizing = 1; @@ -1624,7 +1624,7 @@ Py_EndInterpreter(PyThreadState *tstate) call_py_exitfuncs(tstate); if (tstate != interp->tstate_head || tstate->next != NULL) { - Py_FatalError("Py_EndInterpreter: not the last thread"); + Py_FatalError("not the last thread"); } _PyImport_Cleanup(tstate); @@ -2241,12 +2241,20 @@ fatal_error(const char *prefix, const char *msg, int status) } } +#undef Py_FatalError + void _Py_NO_RETURN Py_FatalError(const char *msg) { fatal_error(NULL, msg, -1); } +void _Py_NO_RETURN +_Py_FatalErrorFunc(const char *func, const char *msg) +{ + fatal_error(func, msg, -1); +} + void _Py_NO_RETURN Py_ExitStatusException(PyStatus status) { diff --git a/Python/pystate.c b/Python/pystate.c index 900266919df..a1eb5239f7f 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -327,20 +327,20 @@ PyInterpreterState_Delete(PyInterpreterState *interp) PyInterpreterState **p; for (p = &interpreters->head; ; p = &(*p)->next) { if (*p == NULL) { - Py_FatalError("PyInterpreterState_Delete: invalid interp"); + Py_FatalError("invalid interp"); } if (*p == interp) { break; } } if (interp->tstate_head != NULL) { - Py_FatalError("PyInterpreterState_Delete: remaining threads"); + Py_FatalError("remaining threads"); } *p = interp->next; if (interpreters->main == interp) { interpreters->main = NULL; if (interpreters->head != NULL) { - Py_FatalError("PyInterpreterState_Delete: remaining subinterpreters"); + Py_FatalError("remaining subinterpreters"); } } HEAD_UNLOCK(runtime); @@ -363,7 +363,7 @@ _PyInterpreterState_DeleteExceptMain(_PyRuntimeState *runtime) PyThreadState *tstate = _PyThreadState_Swap(gilstate, NULL); if (tstate != NULL && tstate->interp != interpreters->main) { - Py_FatalError("PyInterpreterState_DeleteExceptMain: not main interpreter"); + Py_FatalError("not main interpreter"); } HEAD_LOCK(runtime); @@ -389,7 +389,7 @@ _PyInterpreterState_DeleteExceptMain(_PyRuntimeState *runtime) HEAD_UNLOCK(runtime); if (interpreters->head == NULL) { - Py_FatalError("PyInterpreterState_DeleteExceptMain: missing main"); + Py_FatalError("missing main"); } _PyThreadState_Swap(gilstate, tstate); } @@ -400,11 +400,11 @@ _PyInterpreterState_Get(void) { PyThreadState *tstate = _PyThreadState_GET(); if (tstate == NULL) { - Py_FatalError("_PyInterpreterState_Get(): no current thread state"); + Py_FatalError("no current thread state"); } PyInterpreterState *interp = tstate->interp; if (interp == NULL) { - Py_FatalError("_PyInterpreterState_Get(): no current interpreter"); + Py_FatalError("no current interpreter"); } return interp; } @@ -695,7 +695,7 @@ int PyState_AddModule(PyObject* module, struct PyModuleDef* def) { if (!def) { - Py_FatalError("PyState_AddModule: Module Definition is NULL"); + Py_FatalError("Module Definition is NULL"); return -1; } @@ -706,7 +706,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("PyState_AddModule: Module already added!"); + Py_FatalError("Module already added"); return -1; } return _PyState_AddModule(tstate, module, def); @@ -724,15 +724,15 @@ PyState_RemoveModule(struct PyModuleDef* def) } state = _PyInterpreterState_GET_UNSAFE(); if (index == 0) { - Py_FatalError("PyState_RemoveModule: Module index invalid."); + Py_FatalError("Module index invalid"); return -1; } if (state->modules_by_index == NULL) { - Py_FatalError("PyState_RemoveModule: Interpreters module-list not accessible."); + Py_FatalError("Interpreters module-list not accessible."); return -1; } if (index > PyList_GET_SIZE(state->modules_by_index)) { - Py_FatalError("PyState_RemoveModule: Module index out of bounds."); + Py_FatalError("Module index out of bounds."); return -1; } Py_INCREF(Py_None); @@ -819,11 +819,11 @@ tstate_delete_common(PyThreadState *tstate, { _PyRuntimeState *runtime = tstate->interp->runtime; if (tstate == NULL) { - Py_FatalError("PyThreadState_Delete: NULL tstate"); + Py_FatalError("NULL tstate"); } PyInterpreterState *interp = tstate->interp; if (interp == NULL) { - Py_FatalError("PyThreadState_Delete: NULL interp"); + Py_FatalError("NULL interp"); } HEAD_LOCK(runtime); if (tstate->prev) @@ -850,7 +850,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("PyThreadState_Delete: tstate is still current"); + Py_FatalError("tstate is still current"); } } tstate_delete_common(tstate, gilstate); @@ -869,9 +869,9 @@ _PyThreadState_DeleteCurrent(_PyRuntimeState *runtime) { struct _gilstate_runtime_state *gilstate = &runtime->gilstate; PyThreadState *tstate = _PyRuntimeGILState_GetThreadState(gilstate); - if (tstate == NULL) - Py_FatalError( - "PyThreadState_DeleteCurrent: no current tstate"); + if (tstate == NULL) { + Py_FatalError("no current tstate"); + } tstate_delete_common(tstate, gilstate); _PyRuntimeGILState_SetThreadState(gilstate, NULL); PyEval_ReleaseLock(); @@ -932,9 +932,9 @@ PyThreadState * PyThreadState_Get(void) { PyThreadState *tstate = _PyThreadState_GET(); - if (tstate == NULL) - Py_FatalError("PyThreadState_Get: no current thread"); - + if (tstate == NULL) { + Py_FatalError("no current thread"); + } return tstate; }