From 66f77caca39ba39ebe1e4a95dba6d19b20d51951 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 19 Jan 2021 23:35:27 +0100 Subject: bpo-42923: _Py_DumpExtensionModules() ignores stdlib ext (GH-24254) --- Include/internal/pycore_traceback.h | 2 +- Lib/test/test_capi.py | 23 +++++++++++------ Lib/test/test_faulthandler.py | 10 ++++---- Python/pylifecycle.c | 50 +++++++++++++++++++++++++++++-------- Python/traceback.c | 6 ++--- 5 files changed, 65 insertions(+), 26 deletions(-) diff --git a/Include/internal/pycore_traceback.h b/Include/internal/pycore_traceback.h index 169e99b..4d28230 100644 --- a/Include/internal/pycore_traceback.h +++ b/Include/internal/pycore_traceback.h @@ -74,7 +74,7 @@ PyAPI_FUNC(void) _Py_DumpASCII(int fd, PyObject *text); This function is signal safe. */ PyAPI_FUNC(void) _Py_DumpDecimal( int fd, - unsigned long value); + size_t value); /* Format an integer as hexadecimal with width digits into fd file descriptor. The function is signal safe. */ diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 5e72ba9..67175cd 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -547,8 +547,7 @@ class CAPITest(unittest.TestCase): self.assertRaises(TypeError, pynumber_tobase, '123', 10) self.assertRaises(SystemError, pynumber_tobase, 123, 0) - def test_fatal_error(self): - code = 'import _testcapi; _testcapi.fatal_error(b"MESSAGE")' + def check_fatal_error(self, code, expected, not_expected=()): with support.SuppressCrashReport(): rc, out, err = assert_python_failure('-sSI', '-c', code) @@ -556,15 +555,25 @@ class CAPITest(unittest.TestCase): self.assertIn('Fatal Python error: test_fatal_error: MESSAGE\n', err) - match = re.search('^Extension modules:(.*)$', err, re.MULTILINE) + match = re.search(r'^Extension modules:(.*) \(total: ([0-9]+)\)$', + err, re.MULTILINE) if not match: self.fail(f"Cannot find 'Extension modules:' in {err!r}") modules = set(match.group(1).strip().split(', ')) - # Test _PyModule_IsExtension(): the list doesn't have to - # be exhaustive. - for name in ('sys', 'builtins', '_imp', '_thread', '_weakref', - '_io', 'marshal', '_signal', '_abc', '_testcapi'): + total = int(match.group(2)) + + for name in expected: self.assertIn(name, modules) + for name in not_expected: + self.assertNotIn(name, modules) + self.assertEqual(len(modules), total) + + def test_fatal_error(self): + expected = ('_testcapi',) + not_expected = ('sys', 'builtins', '_imp', '_thread', '_weakref', + '_io', 'marshal', '_signal', '_abc') + code = 'import _testcapi; _testcapi.fatal_error(b"MESSAGE")' + self.check_fatal_error(code, expected, not_expected) class TestPendingCalls(unittest.TestCase): diff --git a/Lib/test/test_faulthandler.py b/Lib/test/test_faulthandler.py index c6b763a..b4a654f 100644 --- a/Lib/test/test_faulthandler.py +++ b/Lib/test/test_faulthandler.py @@ -334,19 +334,19 @@ class FaultHandlerTests(unittest.TestCase): def test_dump_ext_modules(self): code = """ import faulthandler + # _testcapi is a test module and not considered as a stdlib module + import _testcapi faulthandler.enable() faulthandler._sigsegv() """ stderr, exitcode = self.get_output(code) stderr = '\n'.join(stderr) - match = re.search('^Extension modules:(.*)$', stderr, re.MULTILINE) + match = re.search(r'^Extension modules:(.*) \(total: [0-9]+\)$', + stderr, re.MULTILINE) if not match: self.fail(f"Cannot find 'Extension modules:' in {stderr!r}") modules = set(match.group(1).strip().split(', ')) - # Only check for a few extensions, the list doesn't have to be - # exhaustive. - for ext in ('sys', 'builtins', '_io', 'faulthandler'): - self.assertIn(ext, modules) + self.assertIn('_testcapi', modules) def test_is_enabled(self): orig_stderr = sys.stderr diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index ee64b0f..e9df8fb 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -18,6 +18,8 @@ #include "pycore_sysmodule.h" // _PySys_ClearAuditHooks() #include "pycore_traceback.h" // _Py_DumpTracebackThreads() +#include "module_names.h" // _Py_module_names + #include // setlocale() #ifdef HAVE_SIGNAL_H @@ -2496,10 +2498,12 @@ fatal_error_exit(int status) } -// Dump the list of extension modules of sys.modules into fd file descriptor. +// Dump the list of extension modules of sys.modules, excluding stdlib modules +// (_Py_module_names), into fd file descriptor. +// // This function is called by a signal handler in faulthandler: avoid memory -// allocations and keep the implementation simple. For example, the list -// is not sorted on purpose. +// allocations and keep the implementation simple. For example, the list is not +// sorted on purpose. void _Py_DumpExtensionModules(int fd, PyInterpreterState *interp) { @@ -2507,15 +2511,14 @@ _Py_DumpExtensionModules(int fd, PyInterpreterState *interp) return; } PyObject *modules = interp->modules; - if (!PyDict_Check(modules)) { + if (modules == NULL || !PyDict_Check(modules)) { return; } - PUTS(fd, "\nExtension modules: "); - + int header = 1; + Py_ssize_t count = 0; Py_ssize_t pos = 0; PyObject *key, *value; - int comma = 0; while (PyDict_Next(modules, &pos, &key, &value)) { if (!PyUnicode_Check(key)) { continue; @@ -2524,14 +2527,41 @@ _Py_DumpExtensionModules(int fd, PyInterpreterState *interp) continue; } - if (comma) { + // Check if it is a stdlib extension module. + // Use the module name from the sys.modules key, + // don't attempt to get the module object name. + const Py_ssize_t names_len = Py_ARRAY_LENGTH(_Py_module_names); + int is_stdlib_mod = 0; + for (Py_ssize_t i=0; i < names_len; i++) { + const char *name = _Py_module_names[i]; + if (PyUnicode_CompareWithASCIIString(key, name) == 0) { + is_stdlib_mod = 1; + break; + } + } + if (is_stdlib_mod) { + // Ignore stdlib extension module. + continue; + } + + if (header) { + PUTS(fd, "\nExtension modules: "); + header = 0; + } + else { PUTS(fd, ", "); } - comma = 1; _Py_DumpASCII(fd, key); + count++; + } + + if (count) { + PUTS(fd, " (total: "); + _Py_DumpDecimal(fd, count); + PUTS(fd, ")"); + PUTS(fd, "\n"); } - PUTS(fd, "\n"); } diff --git a/Python/traceback.c b/Python/traceback.c index bd5fd35..9363d4e 100644 --- a/Python/traceback.c +++ b/Python/traceback.c @@ -628,12 +628,12 @@ PyTraceBack_Print(PyObject *v, PyObject *f) This function is signal safe. */ void -_Py_DumpDecimal(int fd, unsigned long value) +_Py_DumpDecimal(int fd, size_t value) { /* maximum number of characters required for output of %lld or %p. We need at most ceil(log10(256)*SIZEOF_LONG_LONG) digits, plus 1 for the null byte. 53/22 is an upper bound for log10(256). */ - char buffer[1 + (sizeof(unsigned long)*53-1) / 22 + 1]; + char buffer[1 + (sizeof(size_t)*53-1) / 22 + 1]; char *ptr, *end; end = &buffer[Py_ARRAY_LENGTH(buffer) - 1]; @@ -767,7 +767,7 @@ dump_frame(int fd, PyFrameObject *frame) int lineno = PyCode_Addr2Line(code, frame->f_lasti); PUTS(fd, ", line "); if (lineno >= 0) { - _Py_DumpDecimal(fd, (unsigned long)lineno); + _Py_DumpDecimal(fd, (size_t)lineno); } else { PUTS(fd, "???"); -- cgit v0.12