[Python-checkins] cpython: Issue #23571: _Py_CheckFunctionResult() now gives the name of the function
victor.stinner
python-checkins at python.org
Sat Mar 21 15:06:37 CET 2015
https://hg.python.org/cpython/rev/f30a5f6a665c
changeset: 95115:f30a5f6a665c
user: Victor Stinner <victor.stinner at gmail.com>
date: Sat Mar 21 15:04:43 2015 +0100
summary:
Issue #23571: _Py_CheckFunctionResult() now gives the name of the function
which returned an invalid result (result+error or no result without error) in
the exception message.
Add also unit test to check that the exception contains the name of the
function.
Special case: the final _PyEval_EvalFrameEx() check doesn't mention the
function since it didn't execute a single function but a whole frame.
files:
Include/abstract.h | 5 +-
Lib/test/test_capi.py | 44 +++++++++++++++++++++++++++
Modules/_testcapimodule.c | 22 +++++++++++++
Objects/abstract.c | 26 ++++++++++++---
Objects/methodobject.c | 2 +-
Python/ceval.c | 6 +-
6 files changed, 93 insertions(+), 12 deletions(-)
diff --git a/Include/abstract.h b/Include/abstract.h
--- a/Include/abstract.h
+++ b/Include/abstract.h
@@ -267,8 +267,9 @@
PyObject *args, PyObject *kw);
#ifndef Py_LIMITED_API
- PyAPI_FUNC(PyObject *) _Py_CheckFunctionResult(PyObject *obj,
- const char *func_name);
+ PyAPI_FUNC(PyObject *) _Py_CheckFunctionResult(PyObject *func,
+ PyObject *result,
+ const char *where);
#endif
/*
diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py
--- a/Lib/test/test_capi.py
+++ b/Lib/test/test_capi.py
@@ -6,10 +6,12 @@
import random
import subprocess
import sys
+import textwrap
import time
import unittest
from test import support
from test.support import MISSING_C_DOCSTRINGS
+from test.script_helper import assert_python_failure
try:
import _posixsubprocess
except ImportError:
@@ -21,6 +23,9 @@
# Skip this test if the _testcapi module isn't available.
_testcapi = support.import_module('_testcapi')
+# Were we compiled --with-pydebug or with #define Py_DEBUG?
+Py_DEBUG = hasattr(sys, 'gettotalrefcount')
+
def testfunction(self):
"""some doc"""
@@ -167,6 +172,45 @@
o @= m1
self.assertEqual(o, ("matmul", 42, m1))
+ def test_return_null_without_error(self):
+ # Issue #23571: A function must not return NULL without setting an
+ # error
+ if Py_DEBUG:
+ code = textwrap.dedent("""
+ import _testcapi
+ from test import support
+
+ with support.SuppressCrashReport():
+ _testcapi.return_null_without_error()
+ """)
+ rc, out, err = assert_python_failure('-c', code)
+ self.assertIn(b'_Py_CheckFunctionResult: Assertion', err)
+ 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')
+
+ def test_return_result_with_error(self):
+ # Issue #23571: A function must not return a result with an error set
+ if Py_DEBUG:
+ code = textwrap.dedent("""
+ import _testcapi
+ from test import support
+
+ with support.SuppressCrashReport():
+ _testcapi.return_result_with_error()
+ """)
+ rc, out, err = assert_python_failure('-c', code)
+ self.assertIn(b'_Py_CheckFunctionResult: Assertion', err)
+ 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')
+
@unittest.skipUnless(threading, 'Threading required for this test.')
class TestPendingCalls(unittest.TestCase):
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -3360,6 +3360,24 @@
return Py_BuildValue("Nl", obj, pos);
}
+static PyObject*
+return_null_without_error(PyObject *self, PyObject *args)
+{
+ /* invalid call: return NULL without setting an error,
+ * _Py_CheckFunctionResult() must detect such bug at runtime. */
+ PyErr_Clear();
+ return NULL;
+}
+
+static PyObject*
+return_result_with_error(PyObject *self, PyObject *args)
+{
+ /* invalid call: return a result with an error set,
+ * _Py_CheckFunctionResult() must detect such bug at runtime. */
+ PyErr_SetNone(PyExc_ValueError);
+ Py_RETURN_NONE;
+}
+
static PyMethodDef TestMethods[] = {
{"raise_exception", raise_exception, METH_VARARGS},
@@ -3519,6 +3537,10 @@
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},
{NULL, NULL} /* sentinel */
};
diff --git a/Objects/abstract.c b/Objects/abstract.c
--- a/Objects/abstract.c
+++ b/Objects/abstract.c
@@ -2074,10 +2074,12 @@
}
PyObject*
-_Py_CheckFunctionResult(PyObject *result, const char *func_name)
+_Py_CheckFunctionResult(PyObject *func, PyObject *result, const char *where)
{
int err_occurred = (PyErr_Occurred() != NULL);
+ assert((func != NULL) ^ (where != NULL));
+
#ifndef NDEBUG
/* In debug mode: abort() with an assertion error. Use two different
assertions, so if an assertion fails, it's possible to know
@@ -2090,8 +2092,14 @@
if (result == NULL) {
if (!err_occurred) {
- PyErr_Format(PyExc_SystemError,
- "NULL result without error in %s", func_name);
+ if (func)
+ PyErr_Format(PyExc_SystemError,
+ "%R returned NULL without setting an error",
+ func);
+ else
+ PyErr_Format(PyExc_SystemError,
+ "%s returned NULL without setting an error",
+ where);
return NULL;
}
}
@@ -2102,8 +2110,14 @@
Py_DECREF(result);
- PyErr_Format(PyExc_SystemError,
- "result with error in %s", func_name);
+ if (func)
+ PyErr_Format(PyExc_SystemError,
+ "%R returned a result with an error set",
+ func);
+ else
+ PyErr_Format(PyExc_SystemError,
+ "%s returned a result with an error set",
+ where);
_PyErr_ChainExceptions(exc, val, tb);
return NULL;
}
@@ -2136,7 +2150,7 @@
Py_LeaveRecursiveCall();
- return _Py_CheckFunctionResult(result, "PyObject_Call");
+ return _Py_CheckFunctionResult(func, result, NULL);
}
static PyObject*
diff --git a/Objects/methodobject.c b/Objects/methodobject.c
--- a/Objects/methodobject.c
+++ b/Objects/methodobject.c
@@ -142,7 +142,7 @@
}
}
- return _Py_CheckFunctionResult(res, "PyCFunction_Call");
+ return _Py_CheckFunctionResult(func, res, NULL);
}
/* Methods (the standard built-in methods, that is) */
diff --git a/Python/ceval.c b/Python/ceval.c
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -3253,7 +3253,7 @@
f->f_executing = 0;
tstate->frame = f->f_back;
- return _Py_CheckFunctionResult(retval, "PyEval_EvalFrameEx");
+ return _Py_CheckFunctionResult(NULL, retval, "PyEval_EvalFrameEx");
}
static void
@@ -4251,14 +4251,14 @@
if (flags & METH_NOARGS && na == 0) {
C_TRACE(x, (*meth)(self,NULL));
- x = _Py_CheckFunctionResult(x, "call_function");
+ x = _Py_CheckFunctionResult(func, x, NULL);
}
else if (flags & METH_O && na == 1) {
PyObject *arg = EXT_POP(*pp_stack);
C_TRACE(x, (*meth)(self,arg));
Py_DECREF(arg);
- x = _Py_CheckFunctionResult(x, "call_function");
+ x = _Py_CheckFunctionResult(func, x, NULL);
}
else {
err_args(func, flags, na);
--
Repository URL: https://hg.python.org/cpython
More information about the Python-checkins
mailing list