bpo-42990: Functions inherit current builtins (GH-24564)

The types.FunctionType constructor now inherits the current builtins
if the globals dictionary has no "__builtins__" key, rather than
using {"None": None} as builtins: same behavior as eval() and exec()
functions.

Defining a function with "def function(...): ..." in Python is not
affected, globals cannot be overriden with this syntax: it also
inherits the current builtins.

PyFrame_New(), PyEval_EvalCode(), PyEval_EvalCodeEx(),
PyFunction_New() and PyFunction_NewWithQualName() now inherits the
current builtins namespace if the globals dictionary has no
"__builtins__" key.

* Add _PyEval_GetBuiltins() function.
* _PyEval_BuiltinsFromGlobals() now uses _PyEval_GetBuiltins() if
  builtins cannot be found in globals.
* Add tstate parameter to _PyEval_BuiltinsFromGlobals().

Doc/whatsnew/3.10.rst

@@ -282,7 +282,8 @@ Other Language Changes
 
 * Functions have a new ``__builtins__`` attribute which is used to look for
   builtin symbols when a function is executed, instead of looking into
-  ``__globals__['__builtins__']``.
+  ``__globals__['__builtins__']``. The attribute is initialized from
+  ``__globals__["__builtins__"]`` if it exists, else from the current builtins.
   (Contributed by Mark Shannon in :issue:`42990`.)
 
 
@@ -789,6 +790,14 @@ Changes in the Python API
   (Contributed by Yurii Karabas, Andrew Svetlov, Yury Selivanov and Kyle Stanley
   in :issue:`42392`.)
 
+* The :data:`types.FunctionType` constructor now inherits the current builtins
+  if the *globals* dictionary has no ``"__builtins__"`` key, rather than using
+  ``{"None": None}`` as builtins: same behavior as :func:`eval` and
+  :func:`exec` functions.  Defining a function with ``def function(...): ...``
+  in Python is not affected, globals cannot be overriden with this syntax: it
+  also inherits the current builtins.
+  (Contributed by Victor Stinner in :issue:`42990`.)
+
 CPython bytecode changes
 ========================
 

Include/internal/pycore_ceval.h

@@ -34,7 +34,10 @@ PyAPI_FUNC(void) _PyEval_SetCoroutineOriginTrackingDepth(
 void _PyEval_Fini(void);
 
 
-extern PyObject *_PyEval_BuiltinsFromGlobals(PyObject *globals);
+extern PyObject* _PyEval_GetBuiltins(PyThreadState *tstate);
+extern PyObject *_PyEval_BuiltinsFromGlobals(
+    PyThreadState *tstate,
+    PyObject *globals);
 
 
 static inline PyObject*

Lib/test/test_funcattrs.py

@@ -1,3 +1,4 @@
+import textwrap
 import types
 import unittest
 
@@ -78,6 +79,32 @@ def test___builtins__(self):
         self.cannot_set_attr(self.b, '__builtins__', 2,
                              (AttributeError, TypeError))
 
+        # bpo-42990: If globals is specified and has no "__builtins__" key,
+        # a function inherits the current builtins namespace.
+        def func(s): return len(s)
+        ns = {}
+        func2 = type(func)(func.__code__, ns)
+        self.assertIs(func2.__globals__, ns)
+        self.assertIs(func2.__builtins__, __builtins__)
+
+        # Make sure that the function actually works.
+        self.assertEqual(func2("abc"), 3)
+        self.assertEqual(ns, {})
+
+        # Define functions using exec() with different builtins,
+        # and test inheritance when globals has no "__builtins__" key
+        code = textwrap.dedent("""
+            def func3(s): pass
+            func4 = type(func3)(func3.__code__, {})
+        """)
+        safe_builtins = {'None': None}
+        ns = {'type': type, '__builtins__': safe_builtins}
+        exec(code, ns)
+        self.assertIs(ns['func3'].__builtins__, safe_builtins)
+        self.assertIs(ns['func4'].__builtins__, safe_builtins)
+        self.assertIs(ns['func3'].__globals__['__builtins__'], safe_builtins)
+        self.assertNotIn('__builtins__', ns['func4'].__globals__)
+
     def test___closure__(self):
         a = 12
         def f(): print(a)

Misc/NEWS.d/next/Core and Builtins/2021-02-18-15-12-30.bpo-42990.toAqBH.rst

@@ -0,0 +1,7 @@
+The :data:`types.FunctionType` constructor now inherits the current builtins if
+the *globals* dictionary has no ``"__builtins__"`` key, rather than using
+``{"None": None}`` as builtins: same behavior as :func:`eval` and :func:`exec`
+functions. Defining a function with ``def function(...): ...`` in Python is
+not affected, globals cannot be overriden with this syntax: it also inherits
+the current builtins.
+Patch by Victor Stinner.

Objects/frameobject.c

@@ -847,7 +847,7 @@ PyFrameObject*
 PyFrame_New(PyThreadState *tstate, PyCodeObject *code,
             PyObject *globals, PyObject *locals)
 {
-    PyObject *builtins = _PyEval_BuiltinsFromGlobals(globals);
+    PyObject *builtins = _PyEval_BuiltinsFromGlobals(tstate, globals);
     if (builtins == NULL) {
         return NULL;
     }
@@ -862,7 +862,6 @@ PyFrame_New(PyThreadState *tstate, PyCodeObject *code,
         .fc_closure = NULL
     };
     PyFrameObject *f = _PyFrame_New_NoTrack(tstate, &desc, locals);
-    Py_DECREF(builtins);
     if (f) {
         _PyObject_GC_TRACK(f);
     }
@@ -1163,7 +1162,7 @@ PyFrame_GetBack(PyFrameObject *frame)
 }
 
 PyObject*
-_PyEval_BuiltinsFromGlobals(PyObject *globals)
+_PyEval_BuiltinsFromGlobals(PyThreadState *tstate, PyObject *globals)
 {
     PyObject *builtins = _PyDict_GetItemIdWithError(globals, &PyId___builtins__);
     if (builtins) {
@@ -1171,21 +1170,11 @@ _PyEval_BuiltinsFromGlobals(PyObject *globals)
             builtins = PyModule_GetDict(builtins);
             assert(builtins != NULL);
         }
-        return Py_NewRef(builtins);
+        return builtins;
     }
-
     if (PyErr_Occurred()) {
         return NULL;
     }
 
-    /* No builtins! Make up a minimal one. Give them 'None', at least. */
-    builtins = PyDict_New();
-    if (builtins == NULL) {
-        return NULL;
-    }
-    if (PyDict_SetItemString(builtins, "None", Py_None) < 0) {
-        Py_DECREF(builtins);
-        return NULL;
-    }
-    return builtins;
+    return _PyEval_GetBuiltins(tstate);
 }

Objects/funcobject.c

@@ -4,6 +4,7 @@
 #include "Python.h"
 #include "pycore_ceval.h"         // _PyEval_BuiltinsFromGlobals()
 #include "pycore_object.h"        // _PyObject_GC_UNTRACK()
+#include "pycore_pyerrors.h"      // _PyErr_Occurred()
 #include "structmember.h"         // PyMemberDef
 
 PyObject *
@@ -13,6 +14,8 @@ PyFunction_NewWithQualName(PyObject *code, PyObject *globals, PyObject *qualname
     assert(PyDict_Check(globals));
     Py_INCREF(globals);
 
+    PyThreadState *tstate = _PyThreadState_GET();
+
     PyCodeObject *code_obj = (PyCodeObject *)code;
     Py_INCREF(code_obj);
 
@@ -42,15 +45,16 @@ PyFunction_NewWithQualName(PyObject *code, PyObject *globals, PyObject *qualname
     _Py_IDENTIFIER(__name__);
     PyObject *module = _PyDict_GetItemIdWithError(globals, &PyId___name__);
     PyObject *builtins = NULL;
-    if (module == NULL && PyErr_Occurred()) {
+    if (module == NULL && _PyErr_Occurred(tstate)) {
         goto error;
     }
     Py_XINCREF(module);
 
-    builtins = _PyEval_BuiltinsFromGlobals(globals);
+    builtins = _PyEval_BuiltinsFromGlobals(tstate, globals);
     if (builtins == NULL) {
         goto error;
     }
+    Py_INCREF(builtins);
 
     PyFunctionObject *op = PyObject_GC_New(PyFunctionObject, &PyFunction_Type);
     if (op == NULL) {

Python/ceval.c

@@ -889,10 +889,11 @@ static int unpack_iterable(PyThreadState *, PyObject *, int, int, PyObject **);
 PyObject *
 PyEval_EvalCode(PyObject *co, PyObject *globals, PyObject *locals)
 {
+    PyThreadState *tstate = PyThreadState_GET();
     if (locals == NULL) {
         locals = globals;
     }
-    PyObject *builtins = _PyEval_BuiltinsFromGlobals(globals);
+    PyObject *builtins = _PyEval_BuiltinsFromGlobals(tstate, globals);
     if (builtins == NULL) {
         return NULL;
     }
@@ -906,10 +907,7 @@ PyEval_EvalCode(PyObject *co, PyObject *globals, PyObject *locals)
         .fc_kwdefaults = NULL,
         .fc_closure = NULL
     };
-    PyThreadState *tstate = PyThreadState_GET();
-    PyObject *res = _PyEval_Vector(tstate, &desc, locals, NULL, 0, NULL);
-    Py_DECREF(builtins);
-    return res;
+    return _PyEval_Vector(tstate, &desc, locals, NULL, 0, NULL);
 }
 
 
@@ -4733,12 +4731,13 @@ PyEval_EvalCodeEx(PyObject *_co, PyObject *globals, PyObject *locals,
                   PyObject *const *defs, int defcount,
                   PyObject *kwdefs, PyObject *closure)
 {
+    PyThreadState *tstate = _PyThreadState_GET();
     PyObject *res;
     PyObject *defaults = _PyTuple_FromArray(defs, defcount);
     if (defaults == NULL) {
         return NULL;
     }
-    PyObject *builtins = _PyEval_BuiltinsFromGlobals(globals);
+    PyObject *builtins = _PyEval_BuiltinsFromGlobals(tstate, globals);
     if (builtins == NULL) {
         Py_DECREF(defaults);
         return NULL;
@@ -4797,7 +4796,6 @@ PyEval_EvalCodeEx(PyObject *_co, PyObject *globals, PyObject *locals,
         .fc_kwdefaults = kwdefs,
         .fc_closure = closure
     };
-    PyThreadState *tstate = _PyThreadState_GET();
     res = _PyEval_Vector(tstate, &constr, locals,
                          allargs, argcount,
                          kwnames);
@@ -5315,15 +5313,21 @@ PyEval_GetFrame(void)
     return tstate->frame;
 }
 
+PyObject *
+_PyEval_GetBuiltins(PyThreadState *tstate)
+{
+    PyFrameObject *frame = tstate->frame;
+    if (frame != NULL) {
+        return frame->f_builtins;
+    }
+    return tstate->interp->builtins;
+}
+
 PyObject *
 PyEval_GetBuiltins(void)
 {
     PyThreadState *tstate = _PyThreadState_GET();
-    PyFrameObject *current_frame = tstate->frame;
-    if (current_frame == NULL)
-        return tstate->interp->builtins;
-    else
-        return current_frame->f_builtins;
+    return _PyEval_GetBuiltins(tstate);
 }
 
 /* Convenience function to get a builtin from its name */