From 44085a3fc9a150478aec1872dd1079c60dcc42f6 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 18 Feb 2021 19:20:16 +0100 Subject: bpo-42990: Refactor _PyFrame_New_NoTrack() (GH-24566) * Refactor _PyFrame_New_NoTrack() and PyFunction_NewWithQualName() code. * PyFrame_New() checks for _PyEval_BuiltinsFromGlobals() failure. * Fix a ref leak in _PyEval_BuiltinsFromGlobals() error path. * Complete PyFunction_GetModule() documentation: it returns a borrowed reference and it can return NULL. * Move _PyEval_BuiltinsFromGlobals() definition to the internal C API. * PyFunction_NewWithQualName() uses _Py_IDENTIFIER() API for the "__name__" string to make it compatible with subinterpreters. --- Doc/c-api/function.rst | 8 +-- Include/cpython/frameobject.h | 2 - Include/internal/pycore_ceval.h | 5 +- Objects/frameobject.c | 94 ++++++++++++++++------------------ Objects/funcobject.c | 111 +++++++++++++++++++++------------------- Python/ceval.c | 6 +-- 6 files changed, 114 insertions(+), 112 deletions(-) diff --git a/Doc/c-api/function.rst b/Doc/c-api/function.rst index 2096882..ad00842 100644 --- a/Doc/c-api/function.rst +++ b/Doc/c-api/function.rst @@ -61,9 +61,11 @@ There are a few functions specific to Python functions. .. c:function:: PyObject* PyFunction_GetModule(PyObject *op) - Return the *__module__* attribute of the function object *op*. This is normally - a string containing the module name, but can be set to any other object by - Python code. + Return a :term:`borrowed reference` to the *__module__* attribute of the + function object *op*. It can be *NULL*. + + This is normally a string containing the module name, but can be set to any + other object by Python code. .. c:function:: PyObject* PyFunction_GetDefaults(PyObject *op) diff --git a/Include/cpython/frameobject.h b/Include/cpython/frameobject.h index 5a19c00..5122ec4 100644 --- a/Include/cpython/frameobject.h +++ b/Include/cpython/frameobject.h @@ -92,5 +92,3 @@ PyAPI_FUNC(void) PyFrame_FastToLocals(PyFrameObject *); PyAPI_FUNC(void) _PyFrame_DebugMallocStats(FILE *out); PyAPI_FUNC(PyFrameObject *) PyFrame_GetBack(PyFrameObject *frame); - -PyObject *_PyEval_BuiltinsFromGlobals(PyObject *globals); diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index 0491d48..bb22322 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -31,9 +31,12 @@ PyAPI_FUNC(void) _PyEval_SetCoroutineOriginTrackingDepth( PyThreadState *tstate, int new_depth); -/* Private function */ void _PyEval_Fini(void); + +extern PyObject *_PyEval_BuiltinsFromGlobals(PyObject *globals); + + static inline PyObject* _PyEval_EvalFrame(PyThreadState *tstate, PyFrameObject *f, int throwflag) { diff --git a/Objects/frameobject.c b/Objects/frameobject.c index 57105e1..5f7fa40 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -1,12 +1,11 @@ /* Frame object implementation */ #include "Python.h" -#include "pycore_object.h" -#include "pycore_gc.h" // _PyObject_GC_IS_TRACKED() +#include "pycore_ceval.h" // _PyEval_BuiltinsFromGlobals() +#include "pycore_object.h" // _PyObject_GC_UNTRACK() -#include "code.h" -#include "frameobject.h" -#include "opcode.h" +#include "frameobject.h" // PyFrameObject +#include "opcode.h" // EXTENDED_ARG #include "structmember.h" // PyMemberDef #define OFF(x) offsetof(PyFrameObject, x) @@ -762,9 +761,7 @@ _Py_IDENTIFIER(__builtins__); static inline PyFrameObject* frame_alloc(PyCodeObject *code) { - PyFrameObject *f; - - f = code->co_zombieframe; + PyFrameObject *f = code->co_zombieframe; if (f != NULL) { code->co_zombieframe = NULL; _Py_NewReference((PyObject *)f); @@ -803,14 +800,11 @@ frame_alloc(PyCodeObject *code) _Py_NewReference((PyObject *)f); } - f->f_code = code; extras = code->co_nlocals + ncells + nfrees; f->f_valuestack = f->f_localsplus + extras; - for (Py_ssize_t i=0; if_localsplus[i] = NULL; } - f->f_locals = NULL; - f->f_trace = NULL; return f; } @@ -818,42 +812,33 @@ frame_alloc(PyCodeObject *code) PyFrameObject* _Py_HOT_FUNCTION _PyFrame_New_NoTrack(PyThreadState *tstate, PyFrameConstructor *con, PyObject *locals) { -#ifdef Py_DEBUG - if (con == NULL || con->fc_code == NULL || - (locals != NULL && !PyMapping_Check(locals))) { - PyErr_BadInternalCall(); - return NULL; - } -#endif - - PyFrameObject *back = tstate->frame; + assert(con != NULL); + assert(con->fc_globals != NULL); + assert(con->fc_builtins != NULL); + assert(con->fc_code != NULL); + assert(locals == NULL || PyMapping_Check(locals)); PyFrameObject *f = frame_alloc((PyCodeObject *)con->fc_code); if (f == NULL) { return NULL; } + f->f_back = (PyFrameObject*)Py_XNewRef(tstate->frame); + f->f_code = (PyCodeObject *)Py_NewRef(con->fc_code); + f->f_builtins = Py_NewRef(con->fc_builtins); + f->f_globals = Py_NewRef(con->fc_globals); + f->f_locals = Py_XNewRef(locals); + // f_valuestack initialized by frame_alloc() + f->f_trace = NULL; f->f_stackdepth = 0; - Py_INCREF(con->fc_builtins); - f->f_builtins = con->fc_builtins; - Py_XINCREF(back); - f->f_back = back; - Py_INCREF(con->fc_code); - Py_INCREF(con->fc_globals); - f->f_globals = con->fc_globals; - Py_XINCREF(locals); - f->f_locals = locals; - + f->f_trace_lines = 1; + f->f_trace_opcodes = 0; + f->f_gen = NULL; f->f_lasti = -1; f->f_lineno = 0; f->f_iblock = 0; f->f_state = FRAME_CREATED; - f->f_gen = NULL; - f->f_trace_opcodes = 0; - f->f_trace_lines = 1; - - assert(f->f_code != NULL); - + // f_blockstack and f_localsplus initialized by frame_alloc() return f; } @@ -863,6 +848,9 @@ PyFrame_New(PyThreadState *tstate, PyCodeObject *code, PyObject *globals, PyObject *locals) { PyObject *builtins = _PyEval_BuiltinsFromGlobals(globals); + if (builtins == NULL) { + return NULL; + } PyFrameConstructor desc = { .fc_globals = globals, .fc_builtins = builtins, @@ -875,8 +863,9 @@ PyFrame_New(PyThreadState *tstate, PyCodeObject *code, }; PyFrameObject *f = _PyFrame_New_NoTrack(tstate, &desc, locals); Py_DECREF(builtins); - if (f) + if (f) { _PyObject_GC_TRACK(f); + } return f; } @@ -1173,27 +1162,30 @@ PyFrame_GetBack(PyFrameObject *frame) return back; } -PyObject *_PyEval_BuiltinsFromGlobals(PyObject *globals) { +PyObject* +_PyEval_BuiltinsFromGlobals(PyObject *globals) +{ PyObject *builtins = _PyDict_GetItemIdWithError(globals, &PyId___builtins__); if (builtins) { if (PyModule_Check(builtins)) { builtins = PyModule_GetDict(builtins); assert(builtins != NULL); } + return Py_NewRef(builtins); } + + if (PyErr_Occurred()) { + return NULL; + } + + /* No builtins! Make up a minimal one. Give them 'None', at least. */ + builtins = PyDict_New(); if (builtins == NULL) { - if (PyErr_Occurred()) { - return NULL; - } - /* No builtins! Make up a minimal one - Give them 'None', at least. */ - builtins = PyDict_New(); - if (builtins == NULL || - PyDict_SetItemString( - builtins, "None", Py_None) < 0) - return NULL; + return NULL; + } + if (PyDict_SetItemString(builtins, "None", Py_None) < 0) { + Py_DECREF(builtins); + return NULL; } - else - Py_INCREF(builtins); return builtins; } diff --git a/Objects/funcobject.c b/Objects/funcobject.c index 523930d..4b92f6c 100644 --- a/Objects/funcobject.c +++ b/Objects/funcobject.c @@ -2,83 +2,90 @@ /* Function object implementation */ #include "Python.h" -#include "pycore_object.h" -#include "frameobject.h" -#include "code.h" +#include "pycore_ceval.h" // _PyEval_BuiltinsFromGlobals() +#include "pycore_object.h" // _PyObject_GC_UNTRACK() #include "structmember.h" // PyMemberDef PyObject * PyFunction_NewWithQualName(PyObject *code, PyObject *globals, PyObject *qualname) { - PyFunctionObject *op; - PyObject *doc, *consts, *module; - static PyObject *__name__ = NULL; + assert(globals != NULL); + assert(PyDict_Check(globals)); + Py_INCREF(globals); - if (__name__ == NULL) { - __name__ = PyUnicode_InternFromString("__name__"); - if (__name__ == NULL) - return NULL; + PyCodeObject *code_obj = (PyCodeObject *)code; + Py_INCREF(code_obj); + + PyObject *name = code_obj->co_name; + assert(name != NULL); + Py_INCREF(name); + if (!qualname) { + qualname = name; } + Py_INCREF(qualname); - /* __module__: If module name is in globals, use it. - Otherwise, use None. */ - module = PyDict_GetItemWithError(globals, __name__); - if (module) { - Py_INCREF(module); + PyObject *consts = code_obj->co_consts; + assert(PyTuple_Check(consts)); + PyObject *doc; + if (PyTuple_Size(consts) >= 1) { + doc = PyTuple_GetItem(consts, 0); + if (!PyUnicode_Check(doc)) { + doc = Py_None; + } } - else if (PyErr_Occurred()) { - return NULL; + else { + doc = Py_None; + } + Py_INCREF(doc); + + // __module__: Use globals['__name__'] if it exists, or NULL. + _Py_IDENTIFIER(__name__); + PyObject *module = _PyDict_GetItemIdWithError(globals, &PyId___name__); + PyObject *builtins = NULL; + if (module == NULL && PyErr_Occurred()) { + goto error; + } + Py_XINCREF(module); + + builtins = _PyEval_BuiltinsFromGlobals(globals); + if (builtins == NULL) { + goto error; } - op = PyObject_GC_New(PyFunctionObject, &PyFunction_Type); + PyFunctionObject *op = PyObject_GC_New(PyFunctionObject, &PyFunction_Type); if (op == NULL) { - Py_XDECREF(module); - return NULL; + goto error; } /* Note: No failures from this point on, since func_dealloc() does not expect a partially-created object. */ - op->func_weakreflist = NULL; - Py_INCREF(code); - op->func_code = code; - assert(globals != NULL); - Py_INCREF(globals); op->func_globals = globals; - PyObject *builtins = _PyEval_BuiltinsFromGlobals(globals); - if (builtins == NULL) { - return NULL; - } op->func_builtins = builtins; - op->func_name = ((PyCodeObject *)code)->co_name; - Py_INCREF(op->func_name); - op->func_defaults = NULL; /* No default arguments */ - op->func_kwdefaults = NULL; /* No keyword only defaults */ + op->func_name = name; + op->func_qualname = qualname; + op->func_code = (PyObject*)code_obj; + op->func_defaults = NULL; // No default positional arguments + op->func_kwdefaults = NULL; // No default keyword arguments op->func_closure = NULL; - op->vectorcall = _PyFunction_Vectorcall; - op->func_module = module; - - consts = ((PyCodeObject *)code)->co_consts; - if (PyTuple_Size(consts) >= 1) { - doc = PyTuple_GetItem(consts, 0); - if (!PyUnicode_Check(doc)) - doc = Py_None; - } - else - doc = Py_None; - Py_INCREF(doc); op->func_doc = doc; - op->func_dict = NULL; + op->func_weakreflist = NULL; + op->func_module = module; op->func_annotations = NULL; - - if (qualname) - op->func_qualname = qualname; - else - op->func_qualname = op->func_name; - Py_INCREF(op->func_qualname); + op->vectorcall = _PyFunction_Vectorcall; _PyObject_GC_TRACK(op); return (PyObject *)op; + +error: + Py_DECREF(globals); + Py_DECREF(code_obj); + Py_DECREF(name); + Py_DECREF(qualname); + Py_DECREF(doc); + Py_XDECREF(module); + Py_XDECREF(builtins); + return NULL; } PyObject * diff --git a/Python/ceval.c b/Python/ceval.c index 9e4c266..0b74003 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -908,7 +908,7 @@ PyEval_EvalCode(PyObject *co, PyObject *globals, PyObject *locals) .fc_closure = NULL }; PyThreadState *tstate = PyThreadState_GET(); - PyObject *res =_PyEval_Vector(tstate, &desc, locals, NULL, 0, NULL); + PyObject *res = _PyEval_Vector(tstate, &desc, locals, NULL, 0, NULL); Py_DECREF(builtins); return res; } @@ -4800,8 +4800,8 @@ PyEval_EvalCodeEx(PyObject *_co, PyObject *globals, PyObject *locals, }; PyThreadState *tstate = _PyThreadState_GET(); res = _PyEval_Vector(tstate, &constr, locals, - allargs, argcount, - kwnames); + allargs, argcount, + kwnames); if (kwcount) { Py_DECREF(kwnames); PyMem_Free(newargs); -- cgit v0.12