Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-39882: Py_FatalError() logs the function name #18819

Merged
merged 3 commits into from
Mar 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Doc/c-api/sys.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
6 changes: 6 additions & 0 deletions Include/cpython/pyerrors.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 5 additions & 1 deletion Include/pyerrors.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 11 additions & 9 deletions Lib/test/test_capi.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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: <built-in function '
Expand Down Expand Up @@ -225,8 +226,9 @@ def test_return_result_with_error(self):
""")
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'
Expand Down Expand Up @@ -668,7 +670,7 @@ def test_buffer_overflow(self):
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)
Expand All @@ -684,14 +686,14 @@ def test_api_misuse(self):
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):
Expand Down
5 changes: 3 additions & 2 deletions Lib/test/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1078,8 +1078,9 @@ def recurse(cnt):
"""
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):
Expand Down
7 changes: 6 additions & 1 deletion Lib/test/test_faulthandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,9 @@ def check_error(self, code, line_number, fatal_error, *,
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)

Expand Down Expand Up @@ -173,6 +175,7 @@ def test_fatal_error_c_thread(self):
3,
'in new thread',
know_current_thread=False,
func='faulthandler_fatal_error_thread',
py_fatal_error=True)

def test_sigabrt(self):
Expand Down Expand Up @@ -230,6 +233,7 @@ def test_fatal_error(self):
""",
2,
'xyz',
func='faulthandler_fatal_error_py',
py_fatal_error=True)

def test_fatal_error_without_gil(self):
Expand All @@ -239,6 +243,7 @@ def test_fatal_error_without_gil(self):
""",
2,
'xyz',
func='faulthandler_fatal_error_py',
py_fatal_error=True)

@unittest.skipIf(sys.platform.startswith('openbsd'),
Expand Down
3 changes: 2 additions & 1 deletion Lib/test/test_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -4247,7 +4247,8 @@ def run():
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()))
Expand Down
5 changes: 4 additions & 1 deletion Lib/test/test_sys.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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):
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
25 changes: 13 additions & 12 deletions Objects/obmalloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -2314,41 +2314,42 @@ _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);
}


static void
_PyMem_DebugFree(void *ctx, void *ptr)
{
_PyMem_DebugCheckGIL();
_PyMem_DebugCheckGIL(__func__);
_PyMem_DebugRawFree(ctx, ptr);
}


static void *
_PyMem_DebugRealloc(void *ctx, void *ptr, size_t nbytes)
{
_PyMem_DebugCheckGIL();
_PyMem_DebugCheckGIL(__func__);
return _PyMem_DebugRawRealloc(ctx, ptr, nbytes);
}

Expand All @@ -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];
Expand Down Expand Up @@ -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. */
Expand Down
5 changes: 3 additions & 2 deletions Parser/parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
}

Expand Down
8 changes: 5 additions & 3 deletions Parser/tokenizer.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}

Expand Down
12 changes: 6 additions & 6 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,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);
Expand Down Expand Up @@ -313,7 +313,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");
}
}

Expand All @@ -325,7 +325,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);
}
Expand Down Expand Up @@ -372,7 +372,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);
Expand Down Expand Up @@ -1235,7 +1235,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);

Expand All @@ -1247,7 +1247,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. */
Expand Down
5 changes: 2 additions & 3 deletions Python/import.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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,
Expand Down
Loading