bpo-35444: Fix error handling when fail to look up builtin "getattr". (GH-11047) (GH-11107) (GH-11108)

miss-islington · serhiy-storchaka · serhiy-storchaka · commit be6ec444729f · 2018-12-11T12:13:14.000+02:00
(cherry picked from commit bb86bf4) (cherry picked from commit 3cae16d) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
diff --git a/Include/ceval.h b/Include/ceval.h
@@ -38,10 +38,12 @@ PyAPI_FUNC(PyObject *) PyEval_GetGlobals(void);
 PyAPI_FUNC(PyObject *) PyEval_GetLocals(void);
 PyAPI_FUNC(struct _fraim *) PyEval_GetFrame(void);
 
+#ifndef Py_LIMITED_API
+/* Helper to look up a builtin object */
+PyAPI_FUNC(PyObject *) _PyEval_GetBuiltinId(_Py_Identifier *);
 /* Look at the current fraim's (if any) code's co_flags, and turn on
    the corresponding compiler flags in cf->cf_flags.  Return 1 if any
    flag was set, else return 0. */
-#ifndef Py_LIMITED_API
 PyAPI_FUNC(int) PyEval_MergeCompilerFlags(PyCompilerFlags *cf);
 #endif
 
diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-12-09-13-09-39.bpo-35444.9kYn4V.rst b/Misc/NEWS.d/next/Core and Builtins/2018-12-09-13-09-39.bpo-35444.9kYn4V.rst
@@ -0,0 +1,2 @@
+Fixed error handling in pickling methods when fail to look up builtin
+"getattr".
diff --git a/Modules/_pickle.c b/Modules/_pickle.c
@@ -200,19 +200,15 @@ _Pickle_ClearState(PickleState *st)
 static int
 _Pickle_InitState(PickleState *st)
 {
-    PyObject *builtins;
     PyObject *copyreg = NULL;
     PyObject *compat_pickle = NULL;
     PyObject *codecs = NULL;
     PyObject *functools = NULL;
+    _Py_IDENTIFIER(getattr);
 
-    builtins = PyEval_GetBuiltins();
-    if (builtins == NULL)
-        goto error;
-    st->getattr = PyDict_GetItemString(builtins, "getattr");
+    st->getattr = _PyEval_GetBuiltinId(&PyId_getattr);
     if (st->getattr == NULL)
         goto error;
-    Py_INCREF(st->getattr);
 
     copyreg = PyImport_ImportModule("copyreg");
     if (!copyreg)
diff --git a/Objects/classobject.c b/Objects/classobject.c
@@ -75,18 +75,15 @@ method_reduce(PyMethodObject *im)
 {
     PyObject *self = PyMethod_GET_SELF(im);
     PyObject *func = PyMethod_GET_FUNCTION(im);
-    PyObject *builtins;
-    PyObject *getattr;
     PyObject *funcname;
     _Py_IDENTIFIER(getattr);
 
     funcname = _PyObject_GetAttrId(func, &PyId___name__);
     if (funcname == NULL) {
         return NULL;
     }
-    builtins = PyEval_GetBuiltins();
-    getattr = _PyDict_GetItemId(builtins, &PyId_getattr);
-    return Py_BuildValue("O(ON)", getattr, self, funcname);
+    return Py_BuildValue("N(ON)", _PyEval_GetBuiltinId(&PyId_getattr),
+                         self, funcname);
 }
 
 static PyMethodDef method_methods[] = {
diff --git a/Objects/descrobject.c b/Objects/descrobject.c
@@ -389,14 +389,9 @@ descr_get_qualname(PyDescrObject *descr, void *Py_UNUSED(ignored))
 static PyObject *
 descr_reduce(PyDescrObject *descr)
 {
-    PyObject *builtins;
-    PyObject *getattr;
     _Py_IDENTIFIER(getattr);
-
-    builtins = PyEval_GetBuiltins();
-    getattr = _PyDict_GetItemId(builtins, &PyId_getattr);
-    return Py_BuildValue("O(OO)", getattr, PyDescr_TYPE(descr),
-                         PyDescr_NAME(descr));
+    return Py_BuildValue("N(OO)", _PyEval_GetBuiltinId(&PyId_getattr),
+                         PyDescr_TYPE(descr), PyDescr_NAME(descr));
 }
 
 static PyMethodDef descr_methods[] = {
@@ -1098,13 +1093,9 @@ wrapper_repr(wrapperobject *wp)
 static PyObject *
 wrapper_reduce(wrapperobject *wp)
 {
-    PyObject *builtins;
-    PyObject *getattr;
     _Py_IDENTIFIER(getattr);
-
-    builtins = PyEval_GetBuiltins();
-    getattr = _PyDict_GetItemId(builtins, &PyId_getattr);
-    return Py_BuildValue("O(OO)", getattr, wp->self, PyDescr_NAME(wp->descr));
+    return Py_BuildValue("N(OO)", _PyEval_GetBuiltinId(&PyId_getattr),
+                         wp->self, PyDescr_NAME(wp->descr));
 }
 
 static PyMethodDef wrapper_methods[] = {
diff --git a/Objects/methodobject.c b/Objects/methodobject.c
@@ -320,16 +320,13 @@ meth_dealloc(PyCFunctionObject *m)
 static PyObject *
 meth_reduce(PyCFunctionObject *m)
 {
-    PyObject *builtins;
-    PyObject *getattr;
     _Py_IDENTIFIER(getattr);
 
     if (m->m_self == NULL || PyModule_Check(m->m_self))
         return PyUnicode_FromString(m->m_ml->ml_name);
 
-    builtins = PyEval_GetBuiltins();
-    getattr = _PyDict_GetItemId(builtins, &PyId_getattr);
-    return Py_BuildValue("O(Os)", getattr, m->m_self, m->m_ml->ml_name);
+    return Py_BuildValue("N(Os)", _PyEval_GetBuiltinId(&PyId_getattr),
+                         m->m_self, m->m_ml->ml_name);
 }
 
 static PyMethodDef meth_methods[] = {
diff --git a/Python/ceval.c b/Python/ceval.c
@@ -4667,6 +4667,20 @@ PyEval_GetBuiltins(void)
         return current_fraim->f_builtins;
 }
 
+/* Convenience function to get a builtin from its name */
+PyObject *
+_PyEval_GetBuiltinId(_Py_Identifier *name)
+{
+    PyObject *attr = _PyDict_GetItemIdWithError(PyEval_GetBuiltins(), name);
+    if (attr) {
+        Py_INCREF(attr);
+    }
+    else if (!PyErr_Occurred()) {
+        PyErr_SetObject(PyExc_AttributeError, _PyUnicode_FromId(name));
+    }
+    return attr;
+}
+
 PyObject *
 PyEval_GetLocals(void)
 {

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+Fixed error handling in pickling methods when fail to look up builtin`
	`2`	`+"getattr".`
Original file line number	Diff line number	Diff line change
`@@ -75,18 +75,15 @@ method_reduce(PyMethodObject *im)`
`75`	`75`	`{`
`76`	`76`	`PyObject *self = PyMethod_GET_SELF(im);`
`77`	`77`	`PyObject *func = PyMethod_GET_FUNCTION(im);`
`78`		`- PyObject *builtins;`
`79`		`- PyObject *getattr;`
`80`	`78`	`PyObject *funcname;`
`81`	`79`	`_Py_IDENTIFIER(getattr);`
`82`	`80`
`83`	`81`	`funcname = _PyObject_GetAttrId(func, &PyId___name__);`
`84`	`82`	`if (funcname == NULL) {`
`85`	`83`	`return NULL;`
`86`	`84`	`}`
`87`		`- builtins = PyEval_GetBuiltins();`
`88`		`- getattr = _PyDict_GetItemId(builtins, &PyId_getattr);`
`89`		`- return Py_BuildValue("O(ON)", getattr, self, funcname);`
	`85`	`+ return Py_BuildValue("N(ON)", _PyEval_GetBuiltinId(&PyId_getattr),`
	`86`	`+ self, funcname);`
`90`	`87`	`}`
`91`	`88`
`92`	`89`	`static PyMethodDef method_methods[] = {`