Do not copy free variables to locals in class namespaces.

Fixes bug 1569356, but at the cost of a minor incompatibility in
locals().  Add test that verifies that the class namespace is not
polluted.  Also clarify the behavior in the library docs.

Along the way, cleaned up the dict_to_map and map_to_dict
implementations and added some comments that explain what they do.

diff --git a/Doc/lib/libfuncs.tex b/Doc/lib/libfuncs.tex
index 26bffcca0ca9d35bef94fd4a1b9a6e1bd4b9cea8..8125963b43a68a252671d29d49da686cc7694503 100644 (file)
--- a/Doc/lib/libfuncs.tex
+++ b/Doc/lib/libfuncs.tex
@@ -607,6 +607,11 @@ class C:
   \warning{The contents of this dictionary should not be modified;
   changes may not affect the values of local variables used by the
   interpreter.}
+
+  Free variables are returned by \var{locals} when it is called in
+  a function block.  Modifications of free variables may not affect
+  the values used by the interpreter.  Free variables are not
+  returned in class blocks.
 \end{funcdesc}
 
 \begin{funcdesc}{long}{\optional{x\optional{, radix}}}

diff --git a/Lib/test/test_scope.py b/Lib/test/test_scope.py
index a53b30fa675642cd7cda545d9edc77fa33c0c30f..db88dbd5130da8a5087240ca754ee8818e76325b 100644 (file)
--- a/Lib/test/test_scope.py
+++ b/Lib/test/test_scope.py
@@ -486,6 +486,39 @@ self.assert_(X.passed)
         del d['h']
         self.assertEqual(d, {'x': 2, 'y': 7, 'w': 6})
 
+    def testLocalsClass(self):
+        # This test verifies that calling locals() does not pollute
+        # the local namespace of the class with free variables.  Old
+        # versions of Python had a bug, where a free variable being
+        # passed through a class namespace would be inserted into
+        # locals() by locals() or exec or a trace function.
+        #
+        # The real bug lies in frame code that copies variables
+        # between fast locals and the locals dict, e.g. when executing
+        # a trace function.
+
+        def f(x):
+            class C:
+                x = 12
+                def m(self):
+                    return x
+                locals()
+            return C
+
+        self.assertEqual(f(1).x, 12)
+
+        def f(x):
+            class C:
+                y = x
+                def m(self):
+                    return x
+                z = list(locals())
+            return C
+
+        varnames = f(1).z
+        self.assert_("x" not in varnames)
+        self.assert_("y" in varnames)
+
     def testBoundAndFree(self):
         # var is bound and free in class
 

diff --git a/Objects/frameobject.c b/Objects/frameobject.c
index 3a073b6fa02a2e1f6ec37b9d9e60dbe1e74d801e..642d77c6062d800c86236dcf65b4dce79cce248a 100644 (file)
--- a/Objects/frameobject.c
+++ b/Objects/frameobject.c
@@ -701,18 +701,38 @@ PyFrame_BlockPop(PyFrameObject *f)
        return b;
 }
 
-/* Convert between "fast" version of locals and dictionary version */
+/* Convert between "fast" version of locals and dictionary version.
+   
+   map and values are input arguments.  map is a tuple of strings.
+   values is an array of PyObject*.  At index i, map[i] is the name of
+   the variable with value values[i].  The function copies the first
+   nmap variable from map/values into dict.  If values[i] is NULL,
+   the variable is deleted from dict.
+
+   If deref is true, then the values being copied are cell variables
+   and the value is extracted from the cell variable before being put
+   in dict.
+
+   Exceptions raised while modifying the dict are silently ignored,
+   because there is no good way to report them.
+ */
 
 static void
 map_to_dict(PyObject *map, Py_ssize_t nmap, PyObject *dict, PyObject **values,
-           Py_ssize_t deref)
+           int deref)
 {
        Py_ssize_t j;
+        assert(PyTuple_Check(map));
+        assert(PyDict_Check(dict));
+        assert(PyTuple_Size(map) > nmap);
        for (j = nmap; --j >= 0; ) {
                PyObject *key = PyTuple_GET_ITEM(map, j);
                PyObject *value = values[j];
-               if (deref)
+                assert(PyString_Check(key));
+               if (deref) {
+                        assert(PyCell_Check(value));
                        value = PyCell_GET(value);
+                }
                if (value == NULL) {
                        if (PyObject_DelItem(dict, key) != 0)
                                PyErr_Clear();
@@ -724,29 +744,55 @@ map_to_dict(PyObject *map, Py_ssize_t nmap, PyObject *dict, PyObject **values,
        }
 }
 
+/* Copy values from the "locals" dict into the fast locals.
+
+   dict is an input argument containing string keys representing
+   variables names and arbitrary PyObject* as values.
+
+   map and values are input arguments.  map is a tuple of strings.
+   values is an array of PyObject*.  At index i, map[i] is the name of
+   the variable with value values[i].  The function copies the first
+   nmap variable from map/values into dict.  If values[i] is NULL,
+   the variable is deleted from dict.
+
+   If deref is true, then the values being copied are cell variables
+   and the value is extracted from the cell variable before being put
+   in dict.  If clear is true, then variables in map but not in dict
+   are set to NULL in map; if clear is false, variables missing in
+   dict are ignored.
+
+   Exceptions raised while modifying the dict are silently ignored,
+   because there is no good way to report them.
+*/
+
 static void
 dict_to_map(PyObject *map, Py_ssize_t nmap, PyObject *dict, PyObject **values,
-           Py_ssize_t deref, int clear)
+           int deref, int clear)
 {
        Py_ssize_t j;
+        assert(PyTuple_Check(map));
+        assert(PyDict_Check(dict));
+        assert(PyTuple_Size(map) > nmap);
        for (j = nmap; --j >= 0; ) {
                PyObject *key = PyTuple_GET_ITEM(map, j);
                PyObject *value = PyObject_GetItem(dict, key);
-               if (value == NULL)
+                assert(PyString_Check(key));
+                /* We only care about NULLs if clear is true. */
+               if (value == NULL) {
                        PyErr_Clear();
+                        if (!clear)
+                                continue;
+                }
                if (deref) {
-                       if (value || clear) {
-                               if (PyCell_GET(values[j]) != value) {
-                                       if (PyCell_Set(values[j], value) < 0)
-                                               PyErr_Clear();
-                               }
-                       }
-               } else if (value != NULL || clear) {
-                       if (values[j] != value) {
-                               Py_XINCREF(value);
-                               Py_XDECREF(values[j]);
-                               values[j] = value;
-                       }
+                        assert(PyCell_Check(values[j]));
+                        if (PyCell_GET(values[j]) != value) {
+                                if (PyCell_Set(values[j], value) < 0)
+                                        PyErr_Clear();
+                        }
+               } else if (values[j] != value) {
+                        Py_XINCREF(value);
+                        Py_XDECREF(values[j]);
+                        values[j] = value;
                }
                Py_XDECREF(value);
        }
@@ -788,8 +834,18 @@ PyFrame_FastToLocals(PyFrameObject *f)
        if (ncells || nfreevars) {
                map_to_dict(co->co_cellvars, ncells,
                            locals, fast + co->co_nlocals, 1);
-               map_to_dict(co->co_freevars, nfreevars,
-                           locals, fast + co->co_nlocals + ncells, 1);
+                /* If the namespace is unoptimized, then one of the 
+                   following cases applies:
+                   1. It does not contain free variables, because it
+                      uses import * or is a top-level namespace.
+                   2. It is a class namespace.
+                   We don't want to accidentally copy free variables
+                   into the locals dict used by the class.
+                */
+                if (co->co_flags & CO_OPTIMIZED) {
+                        map_to_dict(co->co_freevars, nfreevars,
+                                    locals, fast + co->co_nlocals + ncells, 1);
+                }
        }
        PyErr_Restore(error_type, error_value, error_traceback);
 }