[Python-checkins] bpo-42979: Enhance abstract.c assertions checking slot result (GH-24352)
markshannon
webhook-mailer at python.org
Wed Jan 27 11:39:43 EST 2021
https://github.com/python/cpython/commit/c9b8e9c421b57acdcaf24fab0c93bc29b3ef7c67
commit: c9b8e9c421b57acdcaf24fab0c93bc29b3ef7c67
branch: master
author: Victor Stinner <vstinner at python.org>
committer: markshannon <mark at hotpy.org>
date: 2021-01-27T16:39:16Z
summary:
bpo-42979: Enhance abstract.c assertions checking slot result (GH-24352)
* bpo-42979: Enhance abstract.c assertions checking slot result
Add _Py_CheckSlotResult() function which fails with a fatal error if
a slot function succeeded with an exception set or failed with no
exception set: write the slot name, the type name and the current
exception (if an exception is set).
files:
M Include/cpython/abstract.h
M Include/internal/pycore_object.h
M Lib/test/test_capi.py
M Modules/_testcapimodule.c
M Objects/abstract.c
M Objects/call.c
diff --git a/Include/cpython/abstract.h b/Include/cpython/abstract.h
index 1083942c14929..7a4219c8b338b 100644
--- a/Include/cpython/abstract.h
+++ b/Include/cpython/abstract.h
@@ -376,4 +376,4 @@ PyAPI_FUNC(void) _Py_add_one_to_index_C(int nd, Py_ssize_t *index,
PyAPI_FUNC(int) _Py_convert_optional_to_ssize_t(PyObject *, void *);
/* Same as PyNumber_Index but can return an instance of a subclass of int. */
-PyAPI_FUNC(PyObject *) _PyNumber_Index(PyObject *o);
\ No newline at end of file
+PyAPI_FUNC(PyObject *) _PyNumber_Index(PyObject *o);
diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h
index 3975765a46cc4..3cd27b035c2c7 100644
--- a/Include/internal/pycore_object.h
+++ b/Include/internal/pycore_object.h
@@ -168,6 +168,12 @@ _PyObject_IS_GC(PyObject *obj)
// Fast inlined version of PyType_IS_GC()
#define _PyType_IS_GC(t) _PyType_HasFeature((t), Py_TPFLAGS_HAVE_GC)
+// Usage: assert(_Py_CheckSlotResult(obj, "__getitem__", result != NULL)));
+extern int _Py_CheckSlotResult(
+ PyObject *obj,
+ const char *slot_name,
+ int success);
+
#ifdef __cplusplus
}
#endif
diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py
index 8e92a50026c86..1b18bfad55300 100644
--- a/Lib/test/test_capi.py
+++ b/Lib/test/test_capi.py
@@ -2,6 +2,8 @@
# these are all functions _testcapi exports whose name begins with 'test_'.
from collections import OrderedDict
+import importlib.machinery
+import importlib.util
import os
import pickle
import random
@@ -13,8 +15,6 @@
import time
import unittest
import weakref
-import importlib.machinery
-import importlib.util
from test import support
from test.support import MISSING_C_DOCSTRINGS
from test.support import import_helper
@@ -35,6 +35,10 @@
Py_DEBUG = hasattr(sys, 'gettotalrefcount')
+def decode_stderr(err):
+ return err.decode('utf-8', 'replace').replace('\r', '')
+
+
def testfunction(self):
"""some doc"""
return self
@@ -207,23 +211,22 @@ def test_return_null_without_error(self):
_testcapi.return_null_without_error()
""")
rc, out, err = assert_python_failure('-c', code)
- self.assertRegex(err.replace(b'\r', b''),
- 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 '
- br'return_null_without_error> returned NULL '
- br'without setting an error\n'
- br'\n'
- br'Current thread.*:\n'
- br' File .*", line 6 in <module>')
+ err = decode_stderr(err)
+ self.assertRegex(err,
+ r'Fatal Python error: _Py_CheckFunctionResult: '
+ r'a function returned NULL without setting an exception\n'
+ r'Python runtime state: initialized\n'
+ r'SystemError: <built-in function return_null_without_error> '
+ r'returned NULL without setting an exception\n'
+ r'\n'
+ r'Current thread.*:\n'
+ r' File .*", line 6 in <module>\n')
else:
with self.assertRaises(SystemError) as cm:
_testcapi.return_null_without_error()
self.assertRegex(str(cm.exception),
'return_null_without_error.* '
- 'returned NULL without setting an error')
+ 'returned NULL without setting an exception')
def test_return_result_with_error(self):
# Issue #23571: A function must not return a result with an error set
@@ -236,28 +239,58 @@ def test_return_result_with_error(self):
_testcapi.return_result_with_error()
""")
rc, out, err = assert_python_failure('-c', code)
- self.assertRegex(err.replace(b'\r', b''),
- 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'
- br'The above exception was the direct cause '
- br'of the following exception:\n'
- br'\n'
- br'SystemError: <built-in '
- br'function return_result_with_error> '
- br'returned a result with an error set\n'
- br'\n'
- br'Current thread.*:\n'
- br' File .*, line 6 in <module>')
+ err = decode_stderr(err)
+ self.assertRegex(err,
+ r'Fatal Python error: _Py_CheckFunctionResult: '
+ r'a function returned a result with an exception set\n'
+ r'Python runtime state: initialized\n'
+ r'ValueError\n'
+ r'\n'
+ r'The above exception was the direct cause '
+ r'of the following exception:\n'
+ r'\n'
+ r'SystemError: <built-in '
+ r'function return_result_with_error> '
+ r'returned a result with an exception set\n'
+ r'\n'
+ r'Current thread.*:\n'
+ r' File .*, line 6 in <module>\n')
else:
with self.assertRaises(SystemError) as cm:
_testcapi.return_result_with_error()
self.assertRegex(str(cm.exception),
'return_result_with_error.* '
- 'returned a result with an error set')
+ 'returned a result with an exception set')
+
+ def test_getitem_with_error(self):
+ # Test _Py_CheckSlotResult(). Raise an exception and then calls
+ # PyObject_GetItem(): check that the assertion catchs the bug.
+ # PyObject_GetItem() must not be called with an exception set.
+ code = textwrap.dedent("""
+ import _testcapi
+ from test import support
+
+ with support.SuppressCrashReport():
+ _testcapi.getitem_with_error({1: 2}, 1)
+ """)
+ rc, out, err = assert_python_failure('-c', code)
+ err = decode_stderr(err)
+ if 'SystemError: ' not in err:
+ self.assertRegex(err,
+ r'Fatal Python error: _Py_CheckSlotResult: '
+ r'Slot __getitem__ of type dict succeeded '
+ r'with an exception set\n'
+ r'Python runtime state: initialized\n'
+ r'ValueError: bug\n'
+ r'\n'
+ r'Current thread .* \(most recent call first\):\n'
+ r' File .*, line 6 in <module>\n'
+ r'\n'
+ r'Extension modules: _testcapi \(total: 1\)\n')
+ else:
+ # Python built with NDEBUG macro defined:
+ # test _Py_CheckFunctionResult() instead.
+ self.assertIn('returned a result with an exception set', err)
def test_buildvalue_N(self):
_testcapi.test_buildvalue_N()
@@ -551,7 +584,7 @@ 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')
+ err = decode_stderr(err)
self.assertIn('Fatal Python error: test_fatal_error: MESSAGE\n',
err)
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index 2a5b3d9c50b62..ed59c32e9c5e7 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -4736,6 +4736,18 @@ return_result_with_error(PyObject *self, PyObject *args)
Py_RETURN_NONE;
}
+static PyObject*
+getitem_with_error(PyObject *self, PyObject *args)
+{
+ PyObject *map, *key;
+ if (!PyArg_ParseTuple(args, "OO", &map, &key)) {
+ return NULL;
+ }
+
+ PyErr_SetString(PyExc_ValueError, "bug");
+ return PyObject_GetItem(map, key);
+}
+
static PyObject *
test_pytime_fromseconds(PyObject *self, PyObject *args)
{
@@ -5901,10 +5913,9 @@ static PyMethodDef TestMethods[] = {
pymarshal_read_last_object_from_file, METH_VARARGS},
{"pymarshal_read_object_from_file",
pymarshal_read_object_from_file, METH_VARARGS},
- {"return_null_without_error",
- return_null_without_error, METH_NOARGS},
- {"return_result_with_error",
- return_result_with_error, METH_NOARGS},
+ {"return_null_without_error", return_null_without_error, METH_NOARGS},
+ {"return_result_with_error", return_result_with_error, METH_NOARGS},
+ {"getitem_with_error", getitem_with_error, METH_VARARGS},
{"PyTime_FromSeconds", test_pytime_fromseconds, METH_VARARGS},
{"PyTime_FromSecondsObject", test_pytime_fromsecondsobject, METH_VARARGS},
{"PyTime_AsSecondsDouble", test_pytime_assecondsdouble, METH_VARARGS},
diff --git a/Objects/abstract.c b/Objects/abstract.c
index 44ed5b3932bf2..cc452eaaf3daa 100644
--- a/Objects/abstract.c
+++ b/Objects/abstract.c
@@ -1,11 +1,12 @@
/* Abstract Object Interface (many thanks to Jim Fulton) */
#include "Python.h"
-#include "pycore_unionobject.h" // _Py_UnionType && _Py_Union()
#include "pycore_abstract.h" // _PyIndex_Check()
#include "pycore_ceval.h" // _Py_EnterRecursiveCall()
+#include "pycore_object.h" // _Py_CheckSlotResult()
#include "pycore_pyerrors.h" // _PyErr_Occurred()
#include "pycore_pystate.h" // _PyThreadState_GET()
+#include "pycore_unionobject.h" // _Py_UnionType && _Py_Union()
#include <ctype.h>
#include <stddef.h> // offsetof()
#include "longintrepr.h"
@@ -61,7 +62,7 @@ PyObject_Size(PyObject *o)
m = Py_TYPE(o)->tp_as_sequence;
if (m && m->sq_length) {
Py_ssize_t len = m->sq_length(o);
- assert(len >= 0 || PyErr_Occurred());
+ assert(_Py_CheckSlotResult(o, "__len__", len >= 0));
return len;
}
@@ -160,7 +161,7 @@ PyObject_GetItem(PyObject *o, PyObject *key)
m = Py_TYPE(o)->tp_as_mapping;
if (m && m->mp_subscript) {
PyObject *item = m->mp_subscript(o, key);
- assert((item != NULL) ^ (PyErr_Occurred() != NULL));
+ assert(_Py_CheckSlotResult(o, "__getitem__", item != NULL));
return item;
}
@@ -1564,7 +1565,7 @@ PySequence_Size(PyObject *s)
m = Py_TYPE(s)->tp_as_sequence;
if (m && m->sq_length) {
Py_ssize_t len = m->sq_length(s);
- assert(len >= 0 || PyErr_Occurred());
+ assert(_Py_CheckSlotResult(s, "__len__", len >= 0));
return len;
}
@@ -1708,8 +1709,8 @@ PySequence_GetItem(PyObject *s, Py_ssize_t i)
if (i < 0) {
if (m->sq_length) {
Py_ssize_t l = (*m->sq_length)(s);
+ assert(_Py_CheckSlotResult(s, "__len__", l >= 0));
if (l < 0) {
- assert(PyErr_Occurred());
return NULL;
}
i += l;
@@ -1762,8 +1763,8 @@ PySequence_SetItem(PyObject *s, Py_ssize_t i, PyObject *o)
if (i < 0) {
if (m->sq_length) {
Py_ssize_t l = (*m->sq_length)(s);
+ assert(_Py_CheckSlotResult(s, "__len__", l >= 0));
if (l < 0) {
- assert(PyErr_Occurred());
return -1;
}
i += l;
@@ -1795,8 +1796,8 @@ PySequence_DelItem(PyObject *s, Py_ssize_t i)
if (i < 0) {
if (m->sq_length) {
Py_ssize_t l = (*m->sq_length)(s);
+ assert(_Py_CheckSlotResult(s, "__len__", l >= 0));
if (l < 0) {
- assert(PyErr_Occurred());
return -1;
}
i += l;
@@ -2145,7 +2146,7 @@ PyMapping_Size(PyObject *o)
m = Py_TYPE(o)->tp_as_mapping;
if (m && m->mp_length) {
Py_ssize_t len = m->mp_length(o);
- assert(len >= 0 || PyErr_Occurred());
+ assert(_Py_CheckSlotResult(o, "__len__", len >= 0));
return len;
}
diff --git a/Objects/call.c b/Objects/call.c
index 35b06a9b9c4d7..1fb85efab6169 100644
--- a/Objects/call.c
+++ b/Objects/call.c
@@ -39,16 +39,16 @@ _Py_CheckFunctionResult(PyThreadState *tstate, PyObject *callable,
if (!_PyErr_Occurred(tstate)) {
if (callable)
_PyErr_Format(tstate, PyExc_SystemError,
- "%R returned NULL without setting an error",
+ "%R returned NULL without setting an exception",
callable);
else
_PyErr_Format(tstate, PyExc_SystemError,
- "%s returned NULL without setting an error",
+ "%s returned NULL without setting an exception",
where);
#ifdef Py_DEBUG
/* Ensure that the bug is caught in debug mode.
Py_FatalError() logs the SystemError exception raised above. */
- Py_FatalError("a function returned NULL without setting an error");
+ Py_FatalError("a function returned NULL without setting an exception");
#endif
return NULL;
}
@@ -60,17 +60,17 @@ _Py_CheckFunctionResult(PyThreadState *tstate, PyObject *callable,
if (callable) {
_PyErr_FormatFromCauseTstate(
tstate, PyExc_SystemError,
- "%R returned a result with an error set", callable);
+ "%R returned a result with an exception set", callable);
}
else {
_PyErr_FormatFromCauseTstate(
tstate, PyExc_SystemError,
- "%s returned a result with an error set", where);
+ "%s returned a result with an exception set", where);
}
#ifdef Py_DEBUG
/* Ensure that the bug is caught in debug mode.
Py_FatalError() logs the SystemError exception raised above. */
- Py_FatalError("a function returned a result with an error set");
+ Py_FatalError("a function returned a result with an exception set");
#endif
return NULL;
}
@@ -79,6 +79,30 @@ _Py_CheckFunctionResult(PyThreadState *tstate, PyObject *callable,
}
+int
+_Py_CheckSlotResult(PyObject *obj, const char *slot_name, int success)
+{
+ PyThreadState *tstate = _PyThreadState_GET();
+ if (!success) {
+ if (!_PyErr_Occurred(tstate)) {
+ _Py_FatalErrorFormat(__func__,
+ "Slot %s of type %s failed "
+ "without setting an exception",
+ slot_name, Py_TYPE(obj)->tp_name);
+ }
+ }
+ else {
+ if (_PyErr_Occurred(tstate)) {
+ _Py_FatalErrorFormat(__func__,
+ "Slot %s of type %s succeeded "
+ "with an exception set",
+ slot_name, Py_TYPE(obj)->tp_name);
+ }
+ }
+ return 1;
+}
+
+
/* --- Core PyObject call functions ------------------------------- */
/* Call a callable Python object without any arguments */
More information about the Python-checkins
mailing list