From 759410b372915be0de2a225d7fc6b71dcfae6db3 Mon Sep 17 00:00:00 2001 From: Jeremy Hylton Date: Mon, 26 Feb 2007 18:41:18 +0000 Subject: [PATCH] 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. --- Doc/lib/libfuncs.tex | 5 +++ Lib/test/test_scope.py | 33 +++++++++++++++ Objects/frameobject.c | 94 +++++++++++++++++++++++++++++++++--------- 3 files changed, 113 insertions(+), 19 deletions(-) diff --git a/Doc/lib/libfuncs.tex b/Doc/lib/libfuncs.tex index 26bffcca0ca..8125963b43a 100644 --- 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 a53b30fa675..db88dbd5130 100644 --- 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 3a073b6fa02..642d77c6062 100644 --- 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); }