Skip to content

Commit

Permalink
GH-128682: Stronger checking of PyStackRef_CLOSE and DEAD. (GH-12…
Browse files Browse the repository at this point in the history 
…8683)
  • Loading branch information
@markshannon
markshannon authored Jan 13, 2025
1 parent 6ff8f82 commit 517dc65
Show file tree
Hide file tree
Showing 7 changed files with 220 additions and 123 deletions.
6 changes: 3 additions & 3 deletions Include/internal/pycore_opcode_metadata.h
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions Include/internal/pycore_uop_metadata.h
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions Lib/test/test_generated_cases.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1739,6 +1739,18 @@ def test_no_escaping_calls_in_branching_macros(self):
with self.assertRaises(SyntaxError):
self.run_cases_test(input, "")

def test_kill_in_wrong_order(self):
input = """
inst(OP, (a, b -- c)) {
c = b;
PyStackRef_CLOSE(a);
PyStackRef_CLOSE(b);
}
"""
with self.assertRaises(SyntaxError):
self.run_cases_test(input, "")


class TestGeneratedAbstractCases(unittest.TestCase):
def setUp(self) -> None:
super().setUp()
Expand Down
74 changes: 38 additions & 36 deletions Python/bytecodes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ dummy_func(
(void)receiver;
val = value;
DEAD(value);
PyStackRef_CLOSE(receiver);
DECREF_INPUTS();
}

tier1 inst(INSTRUMENTED_END_SEND, (receiver, value -- val)) {
Expand Down Expand Up @@ -681,8 +681,8 @@ dummy_func(

STAT_INC(BINARY_OP, hit);
PyObject *res_o = PyUnicode_Concat(left_o, right_o);
PyStackRef_CLOSE_SPECIALIZED(left, _PyUnicode_ExactDealloc);
PyStackRef_CLOSE_SPECIALIZED(right, _PyUnicode_ExactDealloc);
PyStackRef_CLOSE_SPECIALIZED(left, _PyUnicode_ExactDealloc);
INPUTS_DEAD();
ERROR_IF(res_o == NULL, error);
res = PyStackRef_FromPyObjectSteal(res_o);
Expand Down Expand Up @@ -725,7 +725,7 @@ dummy_func(
* that the string is safe to mutate.
*/
assert(Py_REFCNT(left_o) >= 2);
PyStackRef_CLOSE(left);
PyStackRef_CLOSE_SPECIALIZED(left, _PyUnicode_ExactDealloc);
DEAD(left);
PyObject *temp = PyStackRef_AsPyObjectSteal(*target_local);
PyUnicode_Append(&temp, right_o);
Expand Down Expand Up @@ -822,8 +822,7 @@ dummy_func(
err = PyObject_SetItem(PyStackRef_AsPyObjectBorrow(container), slice, PyStackRef_AsPyObjectBorrow(v));
Py_DECREF(slice);
}
PyStackRef_CLOSE(v);
PyStackRef_CLOSE(container);
DECREF_INPUTS();
ERROR_IF(err, error);
}

Expand Down Expand Up @@ -2082,11 +2081,8 @@ dummy_func(
int method_found = 0;
PyObject *attr_o = _PySuper_Lookup(cls, self, name,
Py_TYPE(self)->tp_getattro == PyObject_GenericGetAttr ? &method_found : NULL);
PyStackRef_CLOSE(global_super_st);
PyStackRef_CLOSE(class_st);
if (attr_o == NULL) {
PyStackRef_CLOSE(self_st);
ERROR_IF(true, error);
ERROR_NO_POP();
}
if (method_found) {
self_or_null = self_st; // transfer ownership
Expand All @@ -2095,6 +2091,8 @@ dummy_func(
PyStackRef_CLOSE(self_st);
self_or_null = PyStackRef_NULL;
}
PyStackRef_CLOSE(class_st);
PyStackRef_CLOSE(global_super_st);

attr = PyStackRef_FromPyObjectSteal(attr_o);
}
Expand Down Expand Up @@ -2920,7 +2918,6 @@ dummy_func(
else {
/* `iterable` is not a generator. */
PyObject *iter_o = PyObject_GetIter(iterable_o);
DEAD(iterable);
if (iter_o == NULL) {
ERROR_NO_POP();
}
Expand Down Expand Up @@ -3466,11 +3463,11 @@ dummy_func(
/* Callable is not a normal Python function */
STACKREFS_TO_PYOBJECTS(args, total_args, args_o);
if (CONVERSION_FAILED(args_o)) {
PyStackRef_CLOSE(callable[0]);
for (int i = 0; i < total_args; i++) {
PyStackRef_CLOSE(args[i]);
}
DEAD(self_or_null);
PyStackRef_CLOSE(callable[0]);
ERROR_IF(true, error);
}
PyObject *res_o = PyObject_Vectorcall(
Expand All @@ -3496,11 +3493,11 @@ dummy_func(
}
}
assert((res_o != NULL) ^ (_PyErr_Occurred(tstate) != NULL));
PyStackRef_CLOSE(callable[0]);
for (int i = 0; i < total_args; i++) {
PyStackRef_CLOSE(args[i]);
}
DEAD(self_or_null);
PyStackRef_CLOSE(callable[0]);
ERROR_IF(res_o == NULL, error);
res = PyStackRef_FromPyObjectSteal(res_o);
}
Expand Down Expand Up @@ -3636,11 +3633,11 @@ dummy_func(
NULL);
STACKREFS_TO_PYOBJECTS_CLEANUP(args_o);
assert((res_o != NULL) ^ (_PyErr_Occurred(tstate) != NULL));
PyStackRef_CLOSE(callable[0]);
for (int i = 0; i < total_args; i++) {
PyStackRef_CLOSE(args[i]);
}
DEAD(self_or_null);
PyStackRef_CLOSE(callable[0]);
ERROR_IF(res_o == NULL, error);
res = PyStackRef_FromPyObjectSteal(res_o);
}
Expand Down Expand Up @@ -3865,28 +3862,29 @@ dummy_func(

op(_CALL_BUILTIN_CLASS, (callable[1], self_or_null[1], args[oparg] -- res)) {
PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable[0]);

DEOPT_IF(!PyType_Check(callable_o));
PyTypeObject *tp = (PyTypeObject *)callable_o;
int total_args = oparg;
_PyStackRef *arguments = args;
if (!PyStackRef_IsNull(self_or_null[0])) {
args--;
arguments--;
total_args++;
}
DEAD(self_or_null);
DEOPT_IF(!PyType_Check(callable_o));
PyTypeObject *tp = (PyTypeObject *)callable_o;
DEOPT_IF(tp->tp_vectorcall == NULL);
STAT_INC(CALL, hit);
STACKREFS_TO_PYOBJECTS(args, total_args, args_o);
STACKREFS_TO_PYOBJECTS(arguments, total_args, args_o);
if (CONVERSION_FAILED(args_o)) {
DECREF_INPUTS();
ERROR_IF(true, error);
}
DEAD(self_or_null);
PyObject *res_o = tp->tp_vectorcall((PyObject *)tp, args_o, total_args, NULL);
STACKREFS_TO_PYOBJECTS_CLEANUP(args_o);
/* Free the arguments. */
for (int i = 0; i < total_args; i++) {
PyStackRef_CLOSE(args[i]);
PyStackRef_CLOSE(arguments[i]);
}
DEAD(args);
PyStackRef_CLOSE(callable[0]);
ERROR_IF(res_o == NULL, error);
res = PyStackRef_FromPyObjectSteal(res_o);
Expand Down Expand Up @@ -3939,21 +3937,22 @@ dummy_func(
PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable[0]);

int total_args = oparg;
_PyStackRef *arguments = args;
if (!PyStackRef_IsNull(self_or_null[0])) {
args--;
arguments--;
total_args++;
}
DEAD(self_or_null);
DEOPT_IF(!PyCFunction_CheckExact(callable_o));
DEOPT_IF(PyCFunction_GET_FLAGS(callable_o) != METH_FASTCALL);
STAT_INC(CALL, hit);
PyCFunction cfunc = PyCFunction_GET_FUNCTION(callable_o);
/* res = func(self, args, nargs) */
STACKREFS_TO_PYOBJECTS(args, total_args, args_o);
STACKREFS_TO_PYOBJECTS(arguments, total_args, args_o);
if (CONVERSION_FAILED(args_o)) {
DECREF_INPUTS();
ERROR_IF(true, error);
}
DEAD(self_or_null);
PyObject *res_o = ((PyCFunctionFast)(void(*)(void))cfunc)(
PyCFunction_GET_SELF(callable_o),
args_o,
Expand All @@ -3963,8 +3962,9 @@ dummy_func(

/* Free the arguments. */
for (int i = 0; i < total_args; i++) {
PyStackRef_CLOSE(args[i]);
PyStackRef_CLOSE(arguments[i]);
}
DEAD(args);
PyStackRef_CLOSE(callable[0]);
ERROR_IF(res_o == NULL, error);
res = PyStackRef_FromPyObjectSteal(res_o);
Expand Down Expand Up @@ -4043,8 +4043,10 @@ dummy_func(
if (res_o == NULL) {
GOTO_ERROR(error);
}
PyStackRef_CLOSE(callable[0]);
PyStackRef_CLOSE(arg_stackref);
DEAD(args);
DEAD(self_or_null);
PyStackRef_CLOSE(callable[0]);
res = PyStackRef_FromPyObjectSteal(res_o);
}

Expand All @@ -4053,25 +4055,24 @@ dummy_func(
PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable[0]);

int total_args = oparg;
_PyStackRef *arguments = args;
if (!PyStackRef_IsNull(self_or_null[0])) {
args--;
arguments--;
total_args++;
}
DEOPT_IF(total_args != 2);
PyInterpreterState *interp = tstate->interp;
DEOPT_IF(callable_o != interp->callable_cache.isinstance);
STAT_INC(CALL, hit);
_PyStackRef cls_stackref = args[1];
_PyStackRef inst_stackref = args[0];
_PyStackRef cls_stackref = arguments[1];
_PyStackRef inst_stackref = arguments[0];
int retval = PyObject_IsInstance(PyStackRef_AsPyObjectBorrow(inst_stackref), PyStackRef_AsPyObjectBorrow(cls_stackref));
if (retval < 0) {
ERROR_NO_POP();
}
res = retval ? PyStackRef_True : PyStackRef_False;
assert((!PyStackRef_IsNull(res)) ^ (_PyErr_Occurred(tstate) != NULL));
PyStackRef_CLOSE(inst_stackref);
PyStackRef_CLOSE(cls_stackref);
PyStackRef_CLOSE(callable[0]);
DECREF_INPUTS();
}

// This is secretly a super-instruction
Expand Down Expand Up @@ -4370,11 +4371,11 @@ dummy_func(
}
PyStackRef_CLOSE(kwnames);
assert((res_o != NULL) ^ (_PyErr_Occurred(tstate) != NULL));
PyStackRef_CLOSE(callable[0]);
for (int i = 0; i < total_args; i++) {
PyStackRef_CLOSE(args[i]);
}
DEAD(self_or_null);
PyStackRef_CLOSE(callable[0]);
ERROR_IF(res_o == NULL, error);
res = PyStackRef_FromPyObjectSteal(res_o);
}
Expand Down Expand Up @@ -4529,6 +4530,8 @@ dummy_func(
PyObject *callargs_o = PyStackRef_AsPyObjectBorrow(callargs);
if (PyTuple_CheckExact(callargs_o)) {
tuple = callargs;
kwargs_out = kwargs_in;
DEAD(kwargs_in);
DEAD(callargs);
}
else {
Expand All @@ -4540,11 +4543,11 @@ dummy_func(
if (tuple_o == NULL) {
ERROR_NO_POP();
}
kwargs_out = kwargs_in;
DEAD(kwargs_in);
PyStackRef_CLOSE(callargs);
tuple = PyStackRef_FromPyObjectSteal(tuple_o);
}
kwargs_out = kwargs_in;
DEAD(kwargs_in);
}

op(_DO_CALL_FUNCTION_EX, (func_st, unused, callargs_st, kwargs_st if (oparg & 1) -- result)) {
Expand Down Expand Up @@ -4720,8 +4723,7 @@ dummy_func(

inst(FORMAT_WITH_SPEC, (value, fmt_spec -- res)) {
PyObject *res_o = PyObject_Format(PyStackRef_AsPyObjectBorrow(value), PyStackRef_AsPyObjectBorrow(fmt_spec));
PyStackRef_CLOSE(value);
PyStackRef_CLOSE(fmt_spec);
DECREF_INPUTS();
ERROR_IF(res_o == NULL, error);
res = PyStackRef_FromPyObjectSteal(res_o);
}
Expand Down
Loading

0 comments on commit 517dc65

Please sign in to comment.