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.
diff --git a/Doc/c-api/sys.rst b/Doc/c-api/sys.rst
index c851ff6..9ac9179 100644
--- a/Doc/c-api/sys.rst
+++ b/Doc/c-api/sys.rst
@@ -388,6 +388,13 @@
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 f8480fb..ab2e740 100644
--- a/Include/cpython/pyerrors.h
+++ b/Include/cpython/pyerrors.h
@@ -178,6 +178,12 @@
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 3fd133c..399bb7c 100644
--- a/Include/pyerrors.h
+++ b/Include/pyerrors.h
@@ -21,7 +21,11 @@
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 e65973c..ff18a21 100644
--- a/Lib/test/test_capi.py
+++ b/Lib/test/test_capi.py
@@ -61,8 +61,8 @@
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 @@
""")
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: <built-in function '
@@ -225,8 +226,9 @@
""")
rc, out, err = assert_python_failure('-c', code)
self.assertRegex(err.replace(b'\r', b''),
- br'Fatal Python error: a function returned a '
- br'result with an error set\n'
+ br'Fatal Python error: _Py_CheckFunctionResult: '
+ br'a function returned a result '
+ br'with an error set\n'
br'Python runtime state: initialized\n'
br'ValueError\n'
br'\n'
@@ -668,7 +670,7 @@
r"\n"
r"Enable tracemalloc to get the memory block allocation traceback\n"
r"\n"
- r"Fatal Python error: bad trailing pad byte")
+ r"Fatal Python error: _PyMem_DebugRawFree: bad trailing pad byte")
regex = regex.format(ptr=self.PTR_REGEX)
regex = re.compile(regex, flags=re.DOTALL)
self.assertRegex(out, regex)
@@ -684,14 +686,14 @@
r"\n"
r"Enable tracemalloc to get the memory block allocation traceback\n"
r"\n"
- r"Fatal Python error: bad ID: Allocated using API 'm', verified using API 'r'\n")
+ r"Fatal Python error: _PyMem_DebugRawFree: bad ID: Allocated using API 'm', verified using API 'r'\n")
regex = regex.format(ptr=self.PTR_REGEX)
self.assertRegex(out, regex)
def check_malloc_without_gil(self, code):
out = self.check(code)
- expected = ('Fatal Python error: Python memory allocator called '
- 'without holding the GIL')
+ expected = ('Fatal Python error: _PyMem_DebugMalloc: '
+ 'Python memory allocator called without holding the GIL')
self.assertIn(expected, out)
def test_pymem_malloc_without_gil(self):
diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py
index 22a2236..2d3a13a 100644
--- a/Lib/test/test_exceptions.py
+++ b/Lib/test/test_exceptions.py
@@ -1078,8 +1078,9 @@
"""
with SuppressCrashReport():
rc, out, err = script_helper.assert_python_failure("-c", code)
- self.assertIn(b'Fatal Python error: Cannot recover from '
- b'MemoryErrors while normalizing exceptions.', err)
+ self.assertIn(b'Fatal Python error: _PyErr_NormalizeException: '
+ b'Cannot recover from MemoryErrors while '
+ b'normalizing exceptions.', err)
@cpython_only
def test_MemoryError(self):
diff --git a/Lib/test/test_faulthandler.py b/Lib/test/test_faulthandler.py
index ac8cf46..c64afe8 100644
--- a/Lib/test/test_faulthandler.py
+++ b/Lib/test/test_faulthandler.py
@@ -123,7 +123,9 @@
self.assertRegex(output, regex)
self.assertNotEqual(exitcode, 0)
- def check_fatal_error(self, code, line_number, name_regex, **kw):
+ def check_fatal_error(self, code, line_number, name_regex, func=None, **kw):
+ if func:
+ name_regex = '%s: %s' % (func, name_regex)
fatal_error = 'Fatal Python error: %s' % name_regex
self.check_error(code, line_number, fatal_error, **kw)
@@ -173,6 +175,7 @@
3,
'in new thread',
know_current_thread=False,
+ func='faulthandler_fatal_error_thread',
py_fatal_error=True)
def test_sigabrt(self):
@@ -230,6 +233,7 @@
""",
2,
'xyz',
+ func='faulthandler_fatal_error_py',
py_fatal_error=True)
def test_fatal_error_without_gil(self):
@@ -239,6 +243,7 @@
""",
2,
'xyz',
+ func='faulthandler_fatal_error_py',
py_fatal_error=True)
@unittest.skipIf(sys.platform.startswith('openbsd'),
diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py
index 4a7cbe5..fe07b56 100644
--- a/Lib/test/test_io.py
+++ b/Lib/test/test_io.py
@@ -4247,7 +4247,8 @@
err = res.err.decode()
if res.rc != 0:
# Failure: should be a fatal error
- pattern = (r"Fatal Python error: could not acquire lock "
+ pattern = (r"Fatal Python error: _enter_buffered_busy: "
+ r"could not acquire lock "
r"for <(_io\.)?BufferedWriter name='<{stream_name}>'> "
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 c5bd8a4..027f87e 100644
--- a/Lib/test/test_sys.py
+++ b/Lib/test/test_sys.py
@@ -269,6 +269,8 @@
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 @@
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 0000000..b5eae7a
--- /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 3881ff1..3b574bf 100644
--- a/Objects/obmalloc.c
+++ b/Objects/obmalloc.c
@@ -25,7 +25,7 @@
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 @@
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 @@
#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 @@
}
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 @@
static void
_PyMem_DebugFree(void *ctx, void *ptr)
{
- _PyMem_DebugCheckGIL();
+ _PyMem_DebugCheckGIL(__func__);
_PyMem_DebugRawFree(ctx, ptr);
}
@@ -2348,7 +2349,7 @@
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 @@
* 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 @@
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 227b918..a61b2f5 100644
--- a/Parser/parser.c
+++ b/Parser/parser.c
@@ -54,8 +54,9 @@
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 f82b102..75e8da4 100644
--- a/Parser/tokenizer.c
+++ b/Parser/tokenizer.c
@@ -1031,10 +1031,12 @@
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 20e32e2..04e0824 100644
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -283,7 +283,7 @@
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 @@
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 @@
_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 @@
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 @@
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 @@
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 392d711..c4a19bc 100644
--- a/Python/import.c
+++ b/Python/import.c
@@ -310,7 +310,7 @@
{
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 @@
_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 e37b561..3756e3a 100644
--- a/Python/pathconfig.c
+++ b/Python/pathconfig.c
@@ -515,7 +515,7 @@
|| _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 @@
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 @@
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 @@
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 23d74ee..9e3b257 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -1611,10 +1611,10 @@
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 @@
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,6 +2241,8 @@
}
}
+#undef Py_FatalError
+
void _Py_NO_RETURN
Py_FatalError(const char *msg)
{
@@ -2248,6 +2250,12 @@
}
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)
{
if (_PyStatus_IS_EXIT(status)) {
diff --git a/Python/pystate.c b/Python/pystate.c
index 9002669..a1eb523 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -327,20 +327,20 @@
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 @@
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 @@
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 @@
{
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 @@
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 @@
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 @@
}
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 @@
{
_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 @@
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 @@
{
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_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;
}