Skip to content

bpo-42923: _Py_DumpExtensionModules() ignores stdlib ext #24254

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

Merged
merged 1 commit into from
Jan 19, 2021
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
2 changes: 1 addition & 1 deletion Include/internal/pycore_traceback.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
23 changes: 16 additions & 7 deletions Lib/test/test_capi.py
Original file line number Diff line number Diff line change
Expand Up @@ -547,24 +547,33 @@ def test_pynumber_tobase(self):
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)

err = err.replace(b'\r', b'').decode('ascii', 'replace')
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):
Expand Down
10 changes: 5 additions & 5 deletions Lib/test/test_faulthandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,19 +334,19 @@ def test_disable(self):
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
Expand Down
50 changes: 40 additions & 10 deletions Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
#include "pycore_sysmodule.h" // _PySys_ClearAuditHooks()
#include "pycore_traceback.h" // _Py_DumpTracebackThreads()

#include "module_names.h" // _Py_module_names

#include <locale.h> // setlocale()

#ifdef HAVE_SIGNAL_H
Expand Down Expand Up @@ -2496,26 +2498,27 @@ 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)
{
if (interp == NULL) {
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;
Expand All @@ -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");
}


Expand Down
6 changes: 3 additions & 3 deletions Python/traceback.c
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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, "???");
Expand Down