From 41bb43a71e1caa4e6993c6571544d415c9e178b9 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 29 Oct 2013 01:19:37 +0100 Subject: [PATCH] Issue #18408: Add a new PyFrame_FastToLocalsWithError() function to handle exceptions when merging fast locals into f_locals of a frame. PyEval_GetLocals() now raises an exception and return NULL on failure. --- Include/frameobject.h | 2 ++ Misc/NEWS | 4 +++ Objects/frameobject.c | 77 ++++++++++++++++++++++++++++--------------- Objects/object.c | 7 ++-- Python/bltinmodule.c | 17 +++++----- Python/ceval.c | 19 ++++++++--- Python/sysmodule.c | 7 ++-- 7 files changed, 87 insertions(+), 46 deletions(-) diff --git a/Include/frameobject.h b/Include/frameobject.h index 18419bf1816..b41bea684a5 100644 --- a/Include/frameobject.h +++ b/Include/frameobject.h @@ -78,6 +78,8 @@ PyAPI_FUNC(PyObject **) PyFrame_ExtendStack(PyFrameObject *, int, int); /* Conversions between "fast locals" and locals in dictionary */ PyAPI_FUNC(void) PyFrame_LocalsToFast(PyFrameObject *, int); + +PyAPI_FUNC(int) PyFrame_FastToLocalsWithError(PyFrameObject *f); PyAPI_FUNC(void) PyFrame_FastToLocals(PyFrameObject *); PyAPI_FUNC(int) PyFrame_ClearFreeList(void); diff --git a/Misc/NEWS b/Misc/NEWS index 5524efa6e90..28ab6047e84 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,10 @@ Projected release date: 2013-11-24 Core and Builtins ----------------- +- Issue #18408: Add a new PyFrame_FastToLocalsWithError() function to handle + exceptions when merging fast locals into f_locals of a frame. + PyEval_GetLocals() now raises an exception and return NULL on failure. + - Issue #19369: Optimized the usage of __length_hint__(). - Issue #18603: Ensure that PyOS_mystricmp and PyOS_mystrnicmp are in the diff --git a/Objects/frameobject.c b/Objects/frameobject.c index a62a45e1f60..76e77b8a9b4 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -21,7 +21,8 @@ static PyMemberDef frame_memberlist[] = { static PyObject * frame_getlocals(PyFrameObject *f, void *closure) { - PyFrame_FastToLocals(f); + if (PyFrame_FastToLocalsWithError(f) < 0) + return NULL; Py_INCREF(f->f_locals); return f->f_locals; } @@ -772,12 +773,9 @@ PyFrame_BlockPop(PyFrameObject *f) 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 +static int map_to_dict(PyObject *map, Py_ssize_t nmap, PyObject *dict, PyObject **values, int deref) { @@ -794,14 +792,19 @@ map_to_dict(PyObject *map, Py_ssize_t nmap, PyObject *dict, PyObject **values, value = PyCell_GET(value); } if (value == NULL) { - if (PyObject_DelItem(dict, key) != 0) - PyErr_Clear(); + if (PyObject_DelItem(dict, key) != 0) { + if (PyErr_ExceptionMatches(PyExc_KeyError)) + PyErr_Clear(); + else + return -1; + } } else { if (PyObject_SetItem(dict, key, value) != 0) - PyErr_Clear(); + return -1; } } + return 0; } /* Copy values from the "locals" dict into the fast locals. @@ -858,42 +861,49 @@ dict_to_map(PyObject *map, Py_ssize_t nmap, PyObject *dict, PyObject **values, } } -void -PyFrame_FastToLocals(PyFrameObject *f) +int +PyFrame_FastToLocalsWithError(PyFrameObject *f) { /* Merge fast locals into f->f_locals */ PyObject *locals, *map; PyObject **fast; - PyObject *error_type, *error_value, *error_traceback; PyCodeObject *co; Py_ssize_t j; Py_ssize_t ncells, nfreevars; - if (f == NULL) - return; + + if (f == NULL) { + PyErr_BadInternalCall(); + return -1; + } locals = f->f_locals; if (locals == NULL) { locals = f->f_locals = PyDict_New(); - if (locals == NULL) { - PyErr_Clear(); /* Can't report it :-( */ - return; - } + if (locals == NULL) + return -1; } co = f->f_code; map = co->co_varnames; - if (!PyTuple_Check(map)) - return; - PyErr_Fetch(&error_type, &error_value, &error_traceback); + if (!PyTuple_Check(map)) { + PyErr_Format(PyExc_SystemError, + "co_varnames must be a tuple, not %s", + Py_TYPE(map)->tp_name); + return -1; + } fast = f->f_localsplus; j = PyTuple_GET_SIZE(map); if (j > co->co_nlocals) j = co->co_nlocals; - if (co->co_nlocals) - map_to_dict(map, j, locals, fast, 0); + if (co->co_nlocals) { + if (map_to_dict(map, j, locals, fast, 0) < 0) + return -1; + } ncells = PyTuple_GET_SIZE(co->co_cellvars); nfreevars = PyTuple_GET_SIZE(co->co_freevars); if (ncells || nfreevars) { - map_to_dict(co->co_cellvars, ncells, - locals, fast + co->co_nlocals, 1); + if (map_to_dict(co->co_cellvars, ncells, + locals, fast + co->co_nlocals, 1)) + return -1; + /* If the namespace is unoptimized, then one of the following cases applies: 1. It does not contain free variables, because it @@ -903,11 +913,24 @@ PyFrame_FastToLocals(PyFrameObject *f) 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); + if (map_to_dict(co->co_freevars, nfreevars, + locals, fast + co->co_nlocals + ncells, 1) < 0) + return -1; } } - PyErr_Restore(error_type, error_value, error_traceback); + return 0; +} + +void +PyFrame_FastToLocals(PyFrameObject *f) +{ + int res; + + assert(!PyErr_Occurred()); + + res = PyFrame_FastToLocalsWithError(f); + if (res < 0) + PyErr_Clear(); } void diff --git a/Objects/object.c b/Objects/object.c index 8018c6a58c4..95a5334d8bf 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1407,12 +1407,11 @@ static PyObject * _dir_locals(void) { PyObject *names; - PyObject *locals = PyEval_GetLocals(); + PyObject *locals; - if (locals == NULL) { - PyErr_SetString(PyExc_SystemError, "frame does not exist"); + locals = PyEval_GetLocals(); + if (locals == NULL) return NULL; - } names = PyMapping_Keys(locals); if (!names) diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index 47138744577..b0be6715e7c 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -755,8 +755,11 @@ builtin_eval(PyObject *self, PyObject *args) } if (globals == Py_None) { globals = PyEval_GetGlobals(); - if (locals == Py_None) + if (locals == Py_None) { locals = PyEval_GetLocals(); + if (locals == NULL) + return NULL; + } } else if (locals == Py_None) locals = globals; @@ -820,6 +823,8 @@ builtin_exec(PyObject *self, PyObject *args) globals = PyEval_GetGlobals(); if (locals == Py_None) { locals = PyEval_GetLocals(); + if (locals == NULL) + return NULL; } if (!globals || !locals) { PyErr_SetString(PyExc_SystemError, @@ -1926,13 +1931,9 @@ builtin_vars(PyObject *self, PyObject *args) return NULL; if (v == NULL) { d = PyEval_GetLocals(); - if (d == NULL) { - if (!PyErr_Occurred()) - PyErr_SetString(PyExc_SystemError, - "vars(): no locals!?"); - } - else - Py_INCREF(d); + if (d == NULL) + return NULL; + Py_INCREF(d); } else { _Py_IDENTIFIER(__dict__); diff --git a/Python/ceval.c b/Python/ceval.c index 2d52862a84f..5f49615872b 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -2472,7 +2472,9 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag) TARGET(IMPORT_STAR) { PyObject *from = POP(), *locals; int err; - PyFrame_FastToLocals(f); + if (PyFrame_FastToLocalsWithError(f) < 0) + goto error; + locals = f->f_locals; if (locals == NULL) { PyErr_SetString(PyExc_SystemError, @@ -4005,9 +4007,15 @@ PyObject * PyEval_GetLocals(void) { PyFrameObject *current_frame = PyEval_GetFrame(); - if (current_frame == NULL) + if (current_frame == NULL) { + PyErr_SetString(PyExc_SystemError, "frame does not exist"); return NULL; - PyFrame_FastToLocals(current_frame); + } + + if (PyFrame_FastToLocalsWithError(current_frame) < 0) + return NULL; + + assert(current_frame->f_locals != NULL); return current_frame->f_locals; } @@ -4017,8 +4025,9 @@ PyEval_GetGlobals(void) PyFrameObject *current_frame = PyEval_GetFrame(); if (current_frame == NULL) return NULL; - else - return current_frame->f_globals; + + assert(current_frame->f_globals != NULL); + return current_frame->f_globals; } PyFrameObject * diff --git a/Python/sysmodule.c b/Python/sysmodule.c index d8848ae977f..97ce0594b55 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -332,12 +332,16 @@ static PyObject * call_trampoline(PyThreadState *tstate, PyObject* callback, PyFrameObject *frame, int what, PyObject *arg) { - PyObject *args = PyTuple_New(3); + PyObject *args; PyObject *whatstr; PyObject *result; + args = PyTuple_New(3); if (args == NULL) return NULL; + if (PyFrame_FastToLocalsWithError(frame) < 0) + return NULL; + Py_INCREF(frame); whatstr = whatstrings[what]; Py_INCREF(whatstr); @@ -349,7 +353,6 @@ call_trampoline(PyThreadState *tstate, PyObject* callback, PyTuple_SET_ITEM(args, 2, arg); /* call the Python-level function */ - PyFrame_FastToLocals(frame); result = PyEval_CallObject(callback, args); PyFrame_LocalsToFast(frame, 1); if (result == NULL)