Skip to content

Commit 6cc800d

Browse files
bpo-43693: Clean up the PyCodeObject fields. (GH-26364)
* Move up the comment about fields using in hashing/comparision. * Group the fields more clearly. * Add co_ncellvars and co_nfreevars. * Raise ValueError if nlocals != len(varnames), rather than aborting.
1 parent e6c815d commit 6cc800d

File tree

7 files changed

+146
-81
lines changed

7 files changed

+146
-81
lines changed

Include/cpython/code.h

+44-13
Original file line numberDiff line numberDiff line change
@@ -17,31 +17,62 @@ typedef struct _PyOpcache _PyOpcache;
1717
/* Bytecode object */
1818
struct PyCodeObject {
1919
PyObject_HEAD
20+
21+
/* Note only the following fields are used in hash and/or comparisons
22+
*
23+
* - co_name
24+
* - co_argcount
25+
* - co_posonlyargcount
26+
* - co_kwonlyargcount
27+
* - co_nlocals
28+
* - co_stacksize
29+
* - co_flags
30+
* - co_firstlineno
31+
* - co_code
32+
* - co_consts
33+
* - co_names
34+
* - co_varnames
35+
* - co_freevars
36+
* - co_cellvars
37+
*
38+
* This is done to preserve the name and line number for tracebacks
39+
* and debuggers; otherwise, constant de-duplication would collapse
40+
* identical functions/lambdas defined on different lines.
41+
*/
42+
43+
/* These fields are set with provided values on new code objects. */
44+
45+
// The hottest fields (in the eval loop) are grouped here at the top.
46+
PyObject *co_code; /* instruction opcodes */
47+
PyObject *co_consts; /* list (constants used) */
48+
PyObject *co_names; /* list of strings (names used) */
49+
int co_flags; /* CO_..., see below */
50+
// The rest are not so impactful on performance.
2051
int co_argcount; /* #arguments, except *args */
2152
int co_posonlyargcount; /* #positional only arguments */
2253
int co_kwonlyargcount; /* #keyword only arguments */
23-
int co_nlocals; /* #local variables */
2454
int co_stacksize; /* #entries needed for evaluation stack */
25-
int co_flags; /* CO_..., see below */
2655
int co_firstlineno; /* first source line number */
27-
PyObject *co_code; /* instruction opcodes */
28-
PyObject *co_consts; /* list (constants used) */
29-
PyObject *co_names; /* list of strings (names used) */
3056
PyObject *co_varnames; /* tuple of strings (local variable names) */
31-
PyObject *co_freevars; /* tuple of strings (free variable names) */
3257
PyObject *co_cellvars; /* tuple of strings (cell variable names) */
33-
/* The rest aren't used in either hash or comparisons, except for co_name,
34-
used in both. This is done to preserve the name and line number
35-
for tracebacks and debuggers; otherwise, constant de-duplication
36-
would collapse identical functions/lambdas defined on different lines.
37-
*/
38-
Py_ssize_t *co_cell2arg; /* Maps cell vars which are arguments. */
58+
PyObject *co_freevars; /* tuple of strings (free variable names) */
3959
PyObject *co_filename; /* unicode (where it was loaded from) */
4060
PyObject *co_name; /* unicode (name, for reference) */
4161
PyObject *co_linetable; /* string (encoding addr<->lineno mapping) See
4262
Objects/lnotab_notes.txt for details. */
43-
int co_nlocalsplus; /* Number of locals + free + cell variables */
4463
PyObject *co_exceptiontable; /* Byte string encoding exception handling table */
64+
65+
/* These fields are set with computed values on new code objects. */
66+
67+
Py_ssize_t *co_cell2arg; /* Maps cell vars which are arguments. */
68+
// These are redundant but offer some performance benefit.
69+
int co_nlocalsplus; /* number of local + cell + free variables */
70+
int co_nlocals; /* number of local variables */
71+
int co_ncellvars; /* number of cell variables */
72+
int co_nfreevars; /* number of free variables */
73+
74+
/* The remaining fields are zeroed out on new code objects. */
75+
4576
PyObject *co_weakreflist; /* to support weakrefs to code objects */
4677
/* Scratch space for extra data relating to the code object.
4778
Type is a void* to keep the format private in codeobject.c to force

Lib/test/test_code.py

+44-2
Original file line numberDiff line numberDiff line change
@@ -240,21 +240,22 @@ def func():
240240
# different co_name, co_varnames, co_consts
241241
def func2():
242242
y = 2
243+
z = 3
243244
return y
244245
code2 = func2.__code__
245246

246247
for attr, value in (
247248
("co_argcount", 0),
248249
("co_posonlyargcount", 0),
249250
("co_kwonlyargcount", 0),
250-
("co_nlocals", 0),
251+
("co_nlocals", 1),
251252
("co_stacksize", 0),
252253
("co_flags", code.co_flags | inspect.CO_COROUTINE),
253254
("co_firstlineno", 100),
254255
("co_code", code2.co_code),
255256
("co_consts", code2.co_consts),
256257
("co_names", ("myname",)),
257-
("co_varnames", code2.co_varnames),
258+
("co_varnames", ('spam',)),
258259
("co_freevars", ("freevar",)),
259260
("co_cellvars", ("cellvar",)),
260261
("co_filename", "newfilename"),
@@ -265,6 +266,47 @@ def func2():
265266
new_code = code.replace(**{attr: value})
266267
self.assertEqual(getattr(new_code, attr), value)
267268

269+
new_code = code.replace(co_varnames=code2.co_varnames,
270+
co_nlocals=code2.co_nlocals)
271+
self.assertEqual(new_code.co_varnames, code2.co_varnames)
272+
self.assertEqual(new_code.co_nlocals, code2.co_nlocals)
273+
274+
def test_nlocals_mismatch(self):
275+
def func():
276+
x = 1
277+
return x
278+
co = func.__code__
279+
assert co.co_nlocals > 0;
280+
281+
# First we try the constructor.
282+
CodeType = type(co)
283+
for diff in (-1, 1):
284+
with self.assertRaises(ValueError):
285+
CodeType(co.co_argcount,
286+
co.co_posonlyargcount,
287+
co.co_kwonlyargcount,
288+
# This is the only change.
289+
co.co_nlocals + diff,
290+
co.co_stacksize,
291+
co.co_flags,
292+
co.co_code,
293+
co.co_consts,
294+
co.co_names,
295+
co.co_varnames,
296+
co.co_filename,
297+
co.co_name,
298+
co.co_firstlineno,
299+
co.co_lnotab,
300+
co.co_exceptiontable,
301+
co.co_freevars,
302+
co.co_cellvars,
303+
)
304+
# Then we try the replace method.
305+
with self.assertRaises(ValueError):
306+
co.replace(co_nlocals=co.co_nlocals - 1)
307+
with self.assertRaises(ValueError):
308+
co.replace(co_nlocals=co.co_nlocals + 1)
309+
268310
def test_empty_linetable(self):
269311
def func():
270312
pass

Objects/codeobject.c

+30-21
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,8 @@ PyCode_NewWithPosOnlyArgs(int argcount, int posonlyargcount, int kwonlyargcount,
164164
{
165165
PyCodeObject *co;
166166
Py_ssize_t *cell2arg = NULL;
167-
Py_ssize_t i, n_cellvars, n_varnames, total_args;
167+
Py_ssize_t i, total_args;
168+
int ncellvars, nfreevars;
168169

169170
/* Check argument types */
170171
if (argcount < posonlyargcount || posonlyargcount < 0 ||
@@ -184,6 +185,19 @@ PyCode_NewWithPosOnlyArgs(int argcount, int posonlyargcount, int kwonlyargcount,
184185
return NULL;
185186
}
186187

188+
/* Make sure that code is indexable with an int, this is
189+
a long running assumption in ceval.c and many parts of
190+
the interpreter. */
191+
if (PyBytes_GET_SIZE(code) > INT_MAX) {
192+
PyErr_SetString(PyExc_OverflowError, "co_code larger than INT_MAX");
193+
return NULL;
194+
}
195+
196+
if (nlocals != PyTuple_GET_SIZE(varnames)) {
197+
PyErr_SetString(PyExc_ValueError, "co_nlocals != len(co_varnames)");
198+
return NULL;
199+
}
200+
187201
/* Ensure that strings are ready Unicode string */
188202
if (PyUnicode_READY(name) < 0) {
189203
return NULL;
@@ -208,46 +222,40 @@ PyCode_NewWithPosOnlyArgs(int argcount, int posonlyargcount, int kwonlyargcount,
208222
return NULL;
209223
}
210224

211-
/* Make sure that code is indexable with an int, this is
212-
a long running assumption in ceval.c and many parts of
213-
the interpreter. */
214-
if (PyBytes_GET_SIZE(code) > INT_MAX) {
215-
PyErr_SetString(PyExc_OverflowError, "co_code larger than INT_MAX");
216-
return NULL;
217-
}
218-
219225
/* Check for any inner or outer closure references */
220-
n_cellvars = PyTuple_GET_SIZE(cellvars);
221-
if (!n_cellvars && !PyTuple_GET_SIZE(freevars)) {
226+
assert(PyTuple_GET_SIZE(cellvars) < INT_MAX);
227+
ncellvars = (int)PyTuple_GET_SIZE(cellvars);
228+
assert(PyTuple_GET_SIZE(freevars) < INT_MAX);
229+
nfreevars = (int)PyTuple_GET_SIZE(freevars);
230+
if (!ncellvars && !nfreevars) {
222231
flags |= CO_NOFREE;
223232
} else {
224233
flags &= ~CO_NOFREE;
225234
}
226235

227-
n_varnames = PyTuple_GET_SIZE(varnames);
228-
if (argcount <= n_varnames && kwonlyargcount <= n_varnames) {
236+
if (argcount <= nlocals && kwonlyargcount <= nlocals) {
229237
/* Never overflows. */
230238
total_args = (Py_ssize_t)argcount + (Py_ssize_t)kwonlyargcount +
231239
((flags & CO_VARARGS) != 0) + ((flags & CO_VARKEYWORDS) != 0);
232240
}
233241
else {
234-
total_args = n_varnames + 1;
242+
total_args = nlocals + 1;
235243
}
236-
if (total_args > n_varnames) {
244+
if (total_args > nlocals) {
237245
PyErr_SetString(PyExc_ValueError, "code: varnames is too small");
238246
return NULL;
239247
}
240248

241249
/* Create mapping between cells and arguments if needed. */
242-
if (n_cellvars) {
250+
if (ncellvars) {
243251
bool used_cell2arg = false;
244-
cell2arg = PyMem_NEW(Py_ssize_t, n_cellvars);
252+
cell2arg = PyMem_NEW(Py_ssize_t, ncellvars);
245253
if (cell2arg == NULL) {
246254
PyErr_NoMemory();
247255
return NULL;
248256
}
249257
/* Find cells which are also arguments. */
250-
for (i = 0; i < n_cellvars; i++) {
258+
for (i = 0; i < ncellvars; i++) {
251259
Py_ssize_t j;
252260
PyObject *cell = PyTuple_GET_ITEM(cellvars, i);
253261
cell2arg[i] = CO_CELL_NOT_AN_ARG;
@@ -279,9 +287,10 @@ PyCode_NewWithPosOnlyArgs(int argcount, int posonlyargcount, int kwonlyargcount,
279287
co->co_argcount = argcount;
280288
co->co_posonlyargcount = posonlyargcount;
281289
co->co_kwonlyargcount = kwonlyargcount;
290+
co->co_nlocalsplus = nlocals + ncellvars + nfreevars;
282291
co->co_nlocals = nlocals;
283-
co->co_nlocalsplus = nlocals +
284-
(int)PyTuple_GET_SIZE(freevars) + (int)PyTuple_GET_SIZE(cellvars);
292+
co->co_ncellvars = ncellvars;
293+
co->co_nfreevars = nfreevars;
285294
co->co_stacksize = stacksize;
286295
co->co_flags = flags;
287296
Py_INCREF(code);
@@ -1139,7 +1148,7 @@ code_sizeof(PyCodeObject *co, PyObject *Py_UNUSED(args))
11391148
_PyCodeObjectExtra *co_extra = (_PyCodeObjectExtra*) co->co_extra;
11401149

11411150
if (co->co_cell2arg != NULL && co->co_cellvars != NULL) {
1142-
res += PyTuple_GET_SIZE(co->co_cellvars) * sizeof(Py_ssize_t);
1151+
res += co->co_ncellvars * sizeof(Py_ssize_t);
11431152
}
11441153
if (co_extra != NULL) {
11451154
res += sizeof(_PyCodeObjectExtra) +

Objects/frameobject.c

+9-15
Original file line numberDiff line numberDiff line change
@@ -1023,7 +1023,6 @@ PyFrame_FastToLocalsWithError(PyFrameObject *f)
10231023
PyObject **fast;
10241024
PyCodeObject *co;
10251025
Py_ssize_t j;
1026-
Py_ssize_t ncells, nfreevars;
10271026

10281027
if (f == NULL) {
10291028
PyErr_BadInternalCall();
@@ -1051,10 +1050,8 @@ PyFrame_FastToLocalsWithError(PyFrameObject *f)
10511050
if (map_to_dict(map, j, locals, fast, 0) < 0)
10521051
return -1;
10531052
}
1054-
ncells = PyTuple_GET_SIZE(co->co_cellvars);
1055-
nfreevars = PyTuple_GET_SIZE(co->co_freevars);
1056-
if (ncells || nfreevars) {
1057-
if (map_to_dict(co->co_cellvars, ncells,
1053+
if (co->co_ncellvars || co->co_nfreevars) {
1054+
if (map_to_dict(co->co_cellvars, co->co_ncellvars,
10581055
locals, fast + co->co_nlocals, 1))
10591056
return -1;
10601057

@@ -1067,8 +1064,8 @@ PyFrame_FastToLocalsWithError(PyFrameObject *f)
10671064
into the locals dict used by the class.
10681065
*/
10691066
if (co->co_flags & CO_OPTIMIZED) {
1070-
if (map_to_dict(co->co_freevars, nfreevars,
1071-
locals, fast + co->co_nlocals + ncells, 1) < 0)
1067+
if (map_to_dict(co->co_freevars, co->co_nfreevars, locals,
1068+
fast + co->co_nlocals + co->co_ncellvars, 1) < 0)
10721069
return -1;
10731070
}
10741071
}
@@ -1096,7 +1093,6 @@ PyFrame_LocalsToFast(PyFrameObject *f, int clear)
10961093
PyObject *error_type, *error_value, *error_traceback;
10971094
PyCodeObject *co;
10981095
Py_ssize_t j;
1099-
Py_ssize_t ncells, nfreevars;
11001096
if (f == NULL)
11011097
return;
11021098
locals = _PyFrame_Specials(f)[FRAME_SPECIALS_LOCALS_OFFSET];
@@ -1113,16 +1109,14 @@ PyFrame_LocalsToFast(PyFrameObject *f, int clear)
11131109
j = co->co_nlocals;
11141110
if (co->co_nlocals)
11151111
dict_to_map(co->co_varnames, j, locals, fast, 0, clear);
1116-
ncells = PyTuple_GET_SIZE(co->co_cellvars);
1117-
nfreevars = PyTuple_GET_SIZE(co->co_freevars);
1118-
if (ncells || nfreevars) {
1119-
dict_to_map(co->co_cellvars, ncells,
1112+
if (co->co_ncellvars || co->co_nfreevars) {
1113+
dict_to_map(co->co_cellvars, co->co_ncellvars,
11201114
locals, fast + co->co_nlocals, 1, clear);
11211115
/* Same test as in PyFrame_FastToLocals() above. */
11221116
if (co->co_flags & CO_OPTIMIZED) {
1123-
dict_to_map(co->co_freevars, nfreevars,
1124-
locals, fast + co->co_nlocals + ncells, 1,
1125-
clear);
1117+
dict_to_map(co->co_freevars, co->co_nfreevars, locals,
1118+
fast + co->co_nlocals + co->co_ncellvars, 1,
1119+
clear);
11261120
}
11271121
}
11281122
PyErr_Restore(error_type, error_value, error_traceback);

Objects/funcobject.c

+7-7
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,8 @@ func_get_code(PyFunctionObject *op, void *Py_UNUSED(ignored))
279279
static int
280280
func_set_code(PyFunctionObject *op, PyObject *value, void *Py_UNUSED(ignored))
281281
{
282-
Py_ssize_t nfree, nclosure;
282+
Py_ssize_t nclosure;
283+
int nfree;
283284

284285
/* Not legal to del f.func_code or to set it to anything
285286
* other than a code object. */
@@ -294,7 +295,7 @@ func_set_code(PyFunctionObject *op, PyObject *value, void *Py_UNUSED(ignored))
294295
return -1;
295296
}
296297

297-
nfree = PyCode_GetNumFree((PyCodeObject *)value);
298+
nfree = ((PyCodeObject *)value)->co_nfreevars;
298299
nclosure = (op->func_closure == NULL ? 0 :
299300
PyTuple_GET_SIZE(op->func_closure));
300301
if (nclosure != nfree) {
@@ -538,7 +539,7 @@ func_new_impl(PyTypeObject *type, PyCodeObject *code, PyObject *globals,
538539
/*[clinic end generated code: output=99c6d9da3a24e3be input=93611752fc2daf11]*/
539540
{
540541
PyFunctionObject *newfunc;
541-
Py_ssize_t nfree, nclosure;
542+
Py_ssize_t nclosure;
542543

543544
if (name != Py_None && !PyUnicode_Check(name)) {
544545
PyErr_SetString(PyExc_TypeError,
@@ -550,9 +551,8 @@ func_new_impl(PyTypeObject *type, PyCodeObject *code, PyObject *globals,
550551
"arg 4 (defaults) must be None or tuple");
551552
return NULL;
552553
}
553-
nfree = PyTuple_GET_SIZE(code->co_freevars);
554554
if (!PyTuple_Check(closure)) {
555-
if (nfree && closure == Py_None) {
555+
if (code->co_nfreevars && closure == Py_None) {
556556
PyErr_SetString(PyExc_TypeError,
557557
"arg 5 (closure) must be tuple");
558558
return NULL;
@@ -566,10 +566,10 @@ func_new_impl(PyTypeObject *type, PyCodeObject *code, PyObject *globals,
566566

567567
/* check that the closure is well-formed */
568568
nclosure = closure == Py_None ? 0 : PyTuple_GET_SIZE(closure);
569-
if (nfree != nclosure)
569+
if (code->co_nfreevars != nclosure)
570570
return PyErr_Format(PyExc_ValueError,
571571
"%U requires closure of length %zd, not %zd",
572-
code->co_name, nfree, nclosure);
572+
code->co_name, code->co_nfreevars, nclosure);
573573
if (nclosure) {
574574
Py_ssize_t i;
575575
for (i = 0; i < nclosure; i++) {

0 commit comments

Comments
 (0)