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.
This commit is contained in:
parent
6921c13bbb
commit
efde146b0c
|
@ -267,8 +267,9 @@ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx*/
|
|||
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
|
||||
|
||||
/*
|
||||
|
|
|
@ -6,10 +6,12 @@ import pickle
|
|||
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 @@ except ImportError:
|
|||
# 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 @@ class CAPITest(unittest.TestCase):
|
|||
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):
|
||||
|
|
|
@ -3360,6 +3360,24 @@ pymarshal_read_object_from_file(PyObject* self, PyObject *args)
|
|||
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 @@ 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},
|
||||
{NULL, NULL} /* sentinel */
|
||||
};
|
||||
|
||||
|
|
|
@ -2074,10 +2074,12 @@ PyObject_CallObject(PyObject *o, PyObject *a)
|
|||
}
|
||||
|
||||
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 @@ _Py_CheckFunctionResult(PyObject *result, const char *func_name)
|
|||
|
||||
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_CheckFunctionResult(PyObject *result, const char *func_name)
|
|||
|
||||
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 @@ PyObject_Call(PyObject *func, PyObject *arg, PyObject *kw)
|
|||
|
||||
Py_LeaveRecursiveCall();
|
||||
|
||||
return _Py_CheckFunctionResult(result, "PyObject_Call");
|
||||
return _Py_CheckFunctionResult(func, result, NULL);
|
||||
}
|
||||
|
||||
static PyObject*
|
||||
|
|
|
@ -142,7 +142,7 @@ PyCFunction_Call(PyObject *func, PyObject *args, PyObject *kwds)
|
|||
}
|
||||
}
|
||||
|
||||
return _Py_CheckFunctionResult(res, "PyCFunction_Call");
|
||||
return _Py_CheckFunctionResult(func, res, NULL);
|
||||
}
|
||||
|
||||
/* Methods (the standard built-in methods, that is) */
|
||||
|
|
|
@ -3253,7 +3253,7 @@ exit_eval_frame:
|
|||
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 @@ call_function(PyObject ***pp_stack, int oparg
|
|||
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);
|
||||
|
|
Loading…
Reference in New Issue