bpo-36907: fix refcount bug in _PyStack_UnpackDict() (GH-13381)

jdemeyer · encukou · commit 77aa396bb941 · 2019-05-22T13:09:35.000+02:00
diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py
@@ -8,6 +8,7 @@
 import struct
 import collections
 import itertools
+import gc
 
 
 class FunctionCalls(unittest.TestCase):
@@ -457,6 +458,22 @@ def test_fastcall_keywords(self):
                 result = _testcapi.pyobject_fastcallkeywords(func, args, kwnames)
                 self.check_result(result, expected)
 
+    def test_fastcall_clearing_dict(self):
+        # Test bpo-36907: the point of the test is just checking that this
+        # does not crash.
+        class IntWithDict:
+            __slots__ = ["kwargs"]
+            def __init__(self, **kwargs):
+                self.kwargs = kwargs
+            def __index__(self):
+                self.kwargs.clear()
+                gc.collect()
+                return 0
+        x = IntWithDict(dont_inherit=IntWithDict())
+        # We test the argument handling of "compile" here, the compilation
+        # itself is not relevant. When we pass flags=x below, x.__index__() is
+        # called, which changes the keywords dict.
+        compile("pass", "", "exec", x, **x.kwargs)
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-05-17-12-28-24.bpo-36907.rk7kgp.rst b/Misc/NEWS.d/next/Core and Builtins/2019-05-17-12-28-24.bpo-36907.rk7kgp.rst
@@ -0,0 +1,2 @@
+Fix a crash when calling a C function with a keyword dict (``f(**kwargs)``)
+and changing the dict ``kwargs`` while that function is running.
diff --git a/Objects/call.c b/Objects/call.c
@@ -544,10 +544,14 @@ _PyMethodDef_RawFastCallDict(PyMethodDef *method, PyObject *self,
         }
 
         result = (*fastmeth) (self, stack, nargs, kwnames);
-        if (stack != args) {
+        if (kwnames != NULL) {
+            Py_ssize_t i, n = nargs + PyTuple_GET_SIZE(kwnames);
+            for (i = 0; i < n; i++) {
+                Py_DECREF(stack[i]);
+            }
             PyMem_Free((PyObject **)stack);
+            Py_DECREF(kwnames);
         }
-        Py_XDECREF(kwnames);
         break;
     }
 
@@ -1334,8 +1338,11 @@ _PyStack_UnpackDict(PyObject *const *args, Py_ssize_t nargs, PyObject *kwargs,
         return -1;
     }
 
-    /* Copy position arguments (borrowed references) */
-    memcpy(stack, args, nargs * sizeof(stack[0]));
+    /* Copy positional arguments */
+    for (i = 0; i < nargs; i++) {
+        Py_INCREF(args[i]);
+        stack[i] = args[i];
+    }
 
     kwstack = stack + nargs;
     pos = i = 0;
@@ -1344,8 +1351,8 @@ _PyStack_UnpackDict(PyObject *const *args, Py_ssize_t nargs, PyObject *kwargs,
        called in the performance critical hot code. */
     while (PyDict_Next(kwargs, &pos, &key, &value)) {
         Py_INCREF(key);
+        Py_INCREF(value);
         PyTuple_SET_ITEM(kwnames, i, key);
-        /* The stack contains borrowed references */
         kwstack[i] = value;
         i++;
     }

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	+Fix a crash when calling a C function with a keyword dict (``f(**kwargs)``)
	`2`	+and changing the dict ``kwargs`` while that function is running.