From 41c57b335330ff48af098d47e379e0f9ba09d233 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 1 Sep 2019 12:03:39 +0300 Subject: bpo-37994: Fix silencing all errors if an attribute lookup fails. (GH-15630) Only AttributeError should be silenced. --- .../2019-08-31-11-13-25.bpo-37994.Rj6S4j.rst | 2 + Modules/_csv.c | 5 +- Modules/_datetimemodule.c | 21 ++-- Modules/_pickle.c | 13 +- Modules/_threadmodule.c | 7 +- Modules/pyexpat.c | 4 +- Objects/bytearrayobject.c | 5 +- Objects/descrobject.c | 34 +++--- Objects/fileobject.c | 6 +- Objects/setobject.c | 5 +- Objects/typeobject.c | 131 +++++++++++---------- Objects/weakrefobject.c | 6 +- Python/bltinmodule.c | 8 +- Python/sysmodule.c | 14 +-- 14 files changed, 134 insertions(+), 127 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-08-31-11-13-25.bpo-37994.Rj6S4j.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-08-31-11-13-25.bpo-37994.Rj6S4j.rst b/Misc/NEWS.d/next/Core and Builtins/2019-08-31-11-13-25.bpo-37994.Rj6S4j.rst new file mode 100644 index 0000000..103ac5a --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-08-31-11-13-25.bpo-37994.Rj6S4j.rst @@ -0,0 +1,2 @@ +Fixed silencing arbitrary errors if an attribute lookup fails in several +sites. Only AttributeError should be silenced. diff --git a/Modules/_csv.c b/Modules/_csv.c index 9653ff9..aaf3776 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -1382,7 +1382,10 @@ csv_writer(PyObject *module, PyObject *args, PyObject *keyword_args) Py_DECREF(self); return NULL; } - self->write = _PyObject_GetAttrId(output_file, &PyId_write); + if (_PyObject_LookupAttrId(output_file, &PyId_write, &self->write) < 0) { + Py_DECREF(self); + return NULL; + } if (self->write == NULL || !PyCallable_Check(self->write)) { PyErr_SetString(PyExc_TypeError, "argument 1 must have a \"write\" method"); diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index 8f4fa21..c1b2407 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -3607,24 +3607,24 @@ tzinfo_reduce(PyObject *self, PyObject *Py_UNUSED(ignored)) _Py_IDENTIFIER(__getinitargs__); _Py_IDENTIFIER(__getstate__); - getinitargs = _PyObject_GetAttrId(self, &PyId___getinitargs__); + if (_PyObject_LookupAttrId(self, &PyId___getinitargs__, &getinitargs) < 0) { + return NULL; + } if (getinitargs != NULL) { args = PyObject_CallNoArgs(getinitargs); Py_DECREF(getinitargs); - if (args == NULL) { - return NULL; - } } else { - PyErr_Clear(); - args = PyTuple_New(0); - if (args == NULL) { - return NULL; - } + } + if (args == NULL) { + return NULL; } - getstate = _PyObject_GetAttrId(self, &PyId___getstate__); + if (_PyObject_LookupAttrId(self, &PyId___getstate__, &getstate) < 0) { + Py_DECREF(args); + return NULL; + } if (getstate != NULL) { state = PyObject_CallNoArgs(getstate); Py_DECREF(getstate); @@ -3635,7 +3635,6 @@ tzinfo_reduce(PyObject *self, PyObject *Py_UNUSED(ignored)) } else { PyObject **dictptr; - PyErr_Clear(); state = Py_None; dictptr = _PyObject_GetDictPtr(self); if (dictptr && *dictptr && PyDict_GET_SIZE(*dictptr)) { diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 0c53f2e..9e82d14 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -4383,7 +4383,6 @@ save(PicklerObject *self, PyObject *obj, int pers_save) _Py_IDENTIFIER(__reduce__); _Py_IDENTIFIER(__reduce_ex__); - /* XXX: If the __reduce__ method is defined, __reduce_ex__ is automatically defined as __reduce__. While this is convenient, this make it impossible to know which method was actually called. Of @@ -4404,14 +4403,15 @@ save(PicklerObject *self, PyObject *obj, int pers_save) } } else { - PickleState *st = _Pickle_GetGlobalState(); - /* Check for a __reduce__ method. */ - reduce_func = _PyObject_GetAttrId(obj, &PyId___reduce__); + if (_PyObject_LookupAttrId(obj, &PyId___reduce__, &reduce_func) < 0) { + goto error; + } if (reduce_func != NULL) { reduce_value = PyObject_CallNoArgs(reduce_func); } else { + PickleState *st = _Pickle_GetGlobalState(); PyErr_Format(st->PicklingError, "can't pickle '%.200s' object: %R", type->tp_name, obj); @@ -6446,7 +6446,9 @@ do_append(UnpicklerObject *self, Py_ssize_t x) PyObject *extend_func; _Py_IDENTIFIER(extend); - extend_func = _PyObject_GetAttrId(list, &PyId_extend); + if (_PyObject_LookupAttrId(list, &PyId_extend, &extend_func) < 0) { + return -1; + } if (extend_func != NULL) { slice = Pdata_poplist(self->stack, x); if (!slice) { @@ -6466,7 +6468,6 @@ do_append(UnpicklerObject *self, Py_ssize_t x) /* Even if the PEP 307 requires extend() and append() methods, fall back on append() if the object has no extend() method for backward compatibility. */ - PyErr_Clear(); append_func = _PyObject_GetAttrId(list, &PyId_append); if (append_func == NULL) return -1; diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 6665827..a3ecd2e 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1313,6 +1313,7 @@ static int thread_excepthook_file(PyObject *file, PyObject *exc_type, PyObject *exc_value, PyObject *exc_traceback, PyObject *thread) { + _Py_IDENTIFIER(name); /* print(f"Exception in thread {thread.name}:", file=file) */ if (PyFile_WriteString("Exception in thread ", file) < 0) { return -1; @@ -1320,7 +1321,9 @@ thread_excepthook_file(PyObject *file, PyObject *exc_type, PyObject *exc_value, PyObject *name = NULL; if (thread != Py_None) { - name = PyObject_GetAttrString(thread, "name"); + if (_PyObject_LookupAttrId(thread, &PyId_name, &name) < 0) { + return -1; + } } if (name != NULL) { if (PyFile_WriteObject(name, file, Py_PRINT_RAW) < 0) { @@ -1330,8 +1333,6 @@ thread_excepthook_file(PyObject *file, PyObject *exc_type, PyObject *exc_value, Py_DECREF(name); } else { - PyErr_Clear(); - unsigned long ident = PyThread_get_thread_ident(); PyObject *str = PyUnicode_FromFormat("%lu", ident); if (str != NULL) { diff --git a/Modules/pyexpat.c b/Modules/pyexpat.c index b0096e6..5d284cd 100644 --- a/Modules/pyexpat.c +++ b/Modules/pyexpat.c @@ -810,7 +810,9 @@ pyexpat_xmlparser_ParseFile(xmlparseobject *self, PyObject *file) PyObject *readmethod = NULL; _Py_IDENTIFIER(read); - readmethod = _PyObject_GetAttrId(file, &PyId_read); + if (_PyObject_LookupAttrId(file, &PyId_read, &readmethod) < 0) { + return NULL; + } if (readmethod == NULL) { PyErr_SetString(PyExc_TypeError, "argument must have 'read' attribute"); diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c index 9dd6712..8c16cc6 100644 --- a/Objects/bytearrayobject.c +++ b/Objects/bytearrayobject.c @@ -2061,9 +2061,10 @@ _common_reduce(PyByteArrayObject *self, int proto) _Py_IDENTIFIER(__dict__); char *buf; - dict = _PyObject_GetAttrId((PyObject *)self, &PyId___dict__); + if (_PyObject_LookupAttrId((PyObject *)self, &PyId___dict__, &dict) < 0) { + return NULL; + } if (dict == NULL) { - PyErr_Clear(); dict = Py_None; Py_INCREF(dict); } diff --git a/Objects/descrobject.c b/Objects/descrobject.c index 9e1b281..c50fe00 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1616,29 +1616,25 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset, /* if no docstring given and the getter has one, use that one */ if ((doc == NULL || doc == Py_None) && fget != NULL) { _Py_IDENTIFIER(__doc__); - PyObject *get_doc = _PyObject_GetAttrId(fget, &PyId___doc__); - if (get_doc) { - if (Py_TYPE(self) == &PyProperty_Type) { - Py_XSETREF(self->prop_doc, get_doc); - } - else { - /* If this is a property subclass, put __doc__ - in dict of the subclass instance instead, - otherwise it gets shadowed by __doc__ in the - class's dict. */ - int err = _PyObject_SetAttrId((PyObject *)self, &PyId___doc__, get_doc); - Py_DECREF(get_doc); - if (err < 0) - return -1; - } - self->getter_doc = 1; + PyObject *get_doc; + int rc = _PyObject_LookupAttrId(fget, &PyId___doc__, &get_doc); + if (rc <= 0) { + return rc; } - else if (PyErr_ExceptionMatches(PyExc_Exception)) { - PyErr_Clear(); + if (Py_TYPE(self) == &PyProperty_Type) { + Py_XSETREF(self->prop_doc, get_doc); } else { - return -1; + /* If this is a property subclass, put __doc__ + in dict of the subclass instance instead, + otherwise it gets shadowed by __doc__ in the + class's dict. */ + int err = _PyObject_SetAttrId((PyObject *)self, &PyId___doc__, get_doc); + Py_DECREF(get_doc); + if (err < 0) + return -1; } + self->getter_doc = 1; } return 0; diff --git a/Objects/fileobject.c b/Objects/fileobject.c index 0faf7e7..61c9428 100644 --- a/Objects/fileobject.c +++ b/Objects/fileobject.c @@ -185,8 +185,10 @@ PyObject_AsFileDescriptor(PyObject *o) if (PyLong_Check(o)) { fd = _PyLong_AsInt(o); } - else if ((meth = _PyObject_GetAttrId(o, &PyId_fileno)) != NULL) - { + else if (_PyObject_LookupAttrId(o, &PyId_fileno, &meth) < 0) { + return -1; + } + else if (meth != NULL) { PyObject *fno = _PyObject_CallNoArg(meth); Py_DECREF(meth); if (fno == NULL) diff --git a/Objects/setobject.c b/Objects/setobject.c index fafc2fa..924885d 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1970,9 +1970,10 @@ set_reduce(PySetObject *so, PyObject *Py_UNUSED(ignored)) args = PyTuple_Pack(1, keys); if (args == NULL) goto done; - dict = _PyObject_GetAttrId((PyObject *)so, &PyId___dict__); + if (_PyObject_LookupAttrId((PyObject *)so, &PyId___dict__, &dict) < 0) { + goto done; + } if (dict == NULL) { - PyErr_Clear(); dict = Py_None; Py_INCREF(dict); } diff --git a/Objects/typeobject.c b/Objects/typeobject.c index c816c3b..1d8216d 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -1551,17 +1551,10 @@ tail_contains(PyObject *tuple, int whence, PyObject *o) static PyObject * class_name(PyObject *cls) { - PyObject *name = _PyObject_GetAttrId(cls, &PyId___name__); - if (name == NULL) { - PyErr_Clear(); + PyObject *name; + if (_PyObject_LookupAttrId(cls, &PyId___name__, &name) == 0) { name = PyObject_Repr(cls); } - if (name == NULL) - return NULL; - if (!PyUnicode_Check(name)) { - Py_DECREF(name); - return NULL; - } return name; } @@ -1579,13 +1572,15 @@ check_duplicates(PyObject *tuple) if (PyTuple_GET_ITEM(tuple, j) == o) { o = class_name(o); if (o != NULL) { - PyErr_Format(PyExc_TypeError, - "duplicate base class %U", - o); + if (PyUnicode_Check(o)) { + PyErr_Format(PyExc_TypeError, + "duplicate base class %U", o); + } + else { + PyErr_SetString(PyExc_TypeError, + "duplicate base class"); + } Py_DECREF(o); - } else { - PyErr_SetString(PyExc_TypeError, - "duplicate base class"); } return -1; } @@ -1629,13 +1624,20 @@ consistent method resolution\norder (MRO) for bases"); i = 0; while (PyDict_Next(set, &i, &k, &v) && (size_t)off < sizeof(buf)) { PyObject *name = class_name(k); - const char *name_str; + const char *name_str = NULL; if (name != NULL) { - name_str = PyUnicode_AsUTF8(name); - if (name_str == NULL) + if (PyUnicode_Check(name)) { + name_str = PyUnicode_AsUTF8(name); + } + else { name_str = "?"; - } else - name_str = "?"; + } + } + if (name_str == NULL) { + Py_XDECREF(name); + Py_DECREF(set); + return; + } off += PyOS_snprintf(buf + off, sizeof(buf) - off, " %s", name_str); Py_XDECREF(name); if (--n && (size_t)(off+1) < sizeof(buf)) { @@ -3422,10 +3424,10 @@ merge_class_dict(PyObject *dict, PyObject *aclass) assert(aclass); /* Merge in the type's dict (if any). */ - classdict = _PyObject_GetAttrId(aclass, &PyId___dict__); - if (classdict == NULL) - PyErr_Clear(); - else { + if (_PyObject_LookupAttrId(aclass, &PyId___dict__, &classdict) < 0) { + return -1; + } + if (classdict != NULL) { int status = PyDict_Update(dict, classdict); Py_DECREF(classdict); if (status < 0) @@ -3433,15 +3435,17 @@ merge_class_dict(PyObject *dict, PyObject *aclass) } /* Recursively merge in the base types' (if any) dicts. */ - bases = _PyObject_GetAttrId(aclass, &PyId___bases__); - if (bases == NULL) - PyErr_Clear(); - else { + if (_PyObject_LookupAttrId(aclass, &PyId___bases__, &bases) < 0) { + return -1; + } + if (bases != NULL) { /* We have no guarantee that bases is a real tuple */ Py_ssize_t i, n; n = PySequence_Size(bases); /* This better be right */ - if (n < 0) - PyErr_Clear(); + if (n < 0) { + Py_DECREF(bases); + return -1; + } else { for (i = 0; i < n; i++) { int status; @@ -4730,9 +4734,10 @@ object___dir___impl(PyObject *self) PyObject *itsclass = NULL; /* Get __dict__ (which may or may not be a real dict...) */ - dict = _PyObject_GetAttrId(self, &PyId___dict__); + if (_PyObject_LookupAttrId(self, &PyId___dict__, &dict) < 0) { + return NULL; + } if (dict == NULL) { - PyErr_Clear(); dict = PyDict_New(); } else if (!PyDict_Check(dict)) { @@ -4750,12 +4755,12 @@ object___dir___impl(PyObject *self) goto error; /* Merge in attrs reachable from its class. */ - itsclass = _PyObject_GetAttrId(self, &PyId___class__); - if (itsclass == NULL) - /* XXX(tomer): Perhaps fall back to obj->ob_type if no - __class__ exists? */ - PyErr_Clear(); - else if (merge_class_dict(dict, itsclass) != 0) + if (_PyObject_LookupAttrId(self, &PyId___class__, &itsclass) < 0) { + goto error; + } + /* XXX(tomer): Perhaps fall back to obj->ob_type if no + __class__ exists? */ + if (itsclass != NULL && merge_class_dict(dict, itsclass) < 0) goto error; result = PyDict_Keys(dict); @@ -6111,16 +6116,19 @@ method_is_overloaded(PyObject *left, PyObject *right, struct _Py_Identifier *nam PyObject *a, *b; int ok; - b = _PyObject_GetAttrId((PyObject *)(Py_TYPE(right)), name); + if (_PyObject_LookupAttrId((PyObject *)(Py_TYPE(right)), name, &b) < 0) { + return -1; + } if (b == NULL) { - PyErr_Clear(); /* If right doesn't have it, it's not overloaded */ return 0; } - a = _PyObject_GetAttrId((PyObject *)(Py_TYPE(left)), name); + if (_PyObject_LookupAttrId((PyObject *)(Py_TYPE(left)), name, &a) < 0) { + Py_DECREF(b); + return -1; + } if (a == NULL) { - PyErr_Clear(); Py_DECREF(b); /* If right has it but left doesn't, it's overloaded */ return 1; @@ -6129,11 +6137,6 @@ method_is_overloaded(PyObject *left, PyObject *right, struct _Py_Identifier *nam ok = PyObject_RichCompareBool(a, b, Py_NE); Py_DECREF(a); Py_DECREF(b); - if (ok < 0) { - PyErr_Clear(); - return 0; - } - return ok; } @@ -6151,16 +6154,20 @@ FUNCNAME(PyObject *self, PyObject *other) \ if (Py_TYPE(self)->tp_as_number != NULL && \ Py_TYPE(self)->tp_as_number->SLOTNAME == TESTFUNC) { \ PyObject *r; \ - if (do_other && \ - PyType_IsSubtype(Py_TYPE(other), Py_TYPE(self)) && \ - method_is_overloaded(self, other, &rop_id)) { \ - stack[0] = other; \ - stack[1] = self; \ - r = vectorcall_maybe(&rop_id, stack, 2); \ - if (r != Py_NotImplemented) \ - return r; \ - Py_DECREF(r); \ - do_other = 0; \ + if (do_other && PyType_IsSubtype(Py_TYPE(other), Py_TYPE(self))) { \ + int ok = method_is_overloaded(self, other, &rop_id); \ + if (ok < 0) { \ + return NULL; \ + } \ + if (ok) { \ + stack[0] = other; \ + stack[1] = self; \ + r = vectorcall_maybe(&rop_id, stack, 2); \ + if (r != Py_NotImplemented) \ + return r; \ + Py_DECREF(r); \ + do_other = 0; \ + } \ } \ stack[0] = self; \ stack[1] = other; \ @@ -7753,7 +7760,9 @@ supercheck(PyTypeObject *type, PyObject *obj) /* Try the slow way */ PyObject *class_attr; - class_attr = _PyObject_GetAttrId(obj, &PyId___class__); + if (_PyObject_LookupAttrId(obj, &PyId___class__, &class_attr) < 0) { + return NULL; + } if (class_attr != NULL && PyType_Check(class_attr) && (PyTypeObject *)class_attr != Py_TYPE(obj)) @@ -7763,11 +7772,7 @@ supercheck(PyTypeObject *type, PyObject *obj) if (ok) return (PyTypeObject *)class_attr; } - - if (class_attr == NULL) - PyErr_Clear(); - else - Py_DECREF(class_attr); + Py_XDECREF(class_attr); } PyErr_SetString(PyExc_TypeError, diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c index e8a429a..daee476 100644 --- a/Objects/weakrefobject.c +++ b/Objects/weakrefobject.c @@ -163,10 +163,10 @@ weakref_repr(PyWeakReference *self) if (PyWeakref_GET_OBJECT(self) == Py_None) return PyUnicode_FromFormat("", self); - name = _PyObject_GetAttrId(PyWeakref_GET_OBJECT(self), &PyId___name__); + if (_PyObject_LookupAttrId(PyWeakref_GET_OBJECT(self), &PyId___name__, &name) < 0) { + return NULL; + } if (name == NULL || !PyUnicode_Check(name)) { - if (name == NULL) - PyErr_Clear(); repr = PyUnicode_FromFormat( "", self, diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index 63e5812..5053f7a 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -2255,16 +2255,12 @@ builtin_vars(PyObject *self, PyObject *args) return NULL; if (v == NULL) { d = PyEval_GetLocals(); - if (d == NULL) - return NULL; - Py_INCREF(d); + Py_XINCREF(d); } else { - d = _PyObject_GetAttrId(v, &PyId___dict__); - if (d == NULL) { + if (_PyObject_LookupAttrId(v, &PyId___dict__, &d) == 0) { PyErr_SetString(PyExc_TypeError, "vars() argument must have __dict__ attribute"); - return NULL; } } return d; diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 0635e9d..8509aaf 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -224,16 +224,12 @@ PySys_Audit(const char *event, const char *argFormat, ...) ts->tracing++; ts->use_tracing = 0; while ((hook = PyIter_Next(hooks)) != NULL) { + _Py_IDENTIFIER(__cantrace__); PyObject *o; - int canTrace = -1; - o = PyObject_GetAttrString(hook, "__cantrace__"); + int canTrace = _PyObject_LookupAttrId(hook, &PyId___cantrace__, &o); if (o) { canTrace = PyObject_IsTrue(o); Py_DECREF(o); - } else if (_PyErr_Occurred(ts) && - _PyErr_ExceptionMatches(ts, PyExc_AttributeError)) { - _PyErr_Clear(ts); - canTrace = 0; } if (canTrace < 0) { break; @@ -579,7 +575,10 @@ sys_displayhook_unencodable(PyThreadState *tstate, PyObject *outf, PyObject *o) if (encoded == NULL) goto error; - buffer = _PyObject_GetAttrId(outf, &PyId_buffer); + if (_PyObject_LookupAttrId(outf, &PyId_buffer, &buffer) < 0) { + Py_DECREF(encoded); + goto error; + } if (buffer) { result = _PyObject_CallMethodIdOneArg(buffer, &PyId_write, encoded); Py_DECREF(buffer); @@ -589,7 +588,6 @@ sys_displayhook_unencodable(PyThreadState *tstate, PyObject *outf, PyObject *o) Py_DECREF(result); } else { - _PyErr_Clear(tstate); escaped_str = PyUnicode_FromEncodedObject(encoded, stdout_encoding_str, "strict"); -- cgit v0.12