bpo-42990: Functions inherit current builtins

[vstinner]

The types.FunctionType constructor now inherits the current builtins
if the globals parameter is used and the globals dictionary has no
"builtins" key, rather than rather than using {"None": None} as
builtins: same behavior than 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().

https://bugs.python.org/issue42990

[vstinner]

@markshannon: Does it sounds like a reasonable default behavior according to you?

If someone wants to run a function in a builtins namespace different than the current builtins, it should be set explicitly in globals["__builtins__"].

Once this PR will be merged, I plan to write a 3rd PR to add an optional builtins keyword-only parameter to the function constructor (FunctionType).

[markshannon]

This would allow code to access the builtins by creating a new function, side stepping any sand boxing in eval.
eval(code, {}) should deny access to the builtins. This PR would make it trivial to get the builtins.

[vstinner]

This would allow code to access the builtins by creating a new function, side stepping any sand boxing in eval.
eval(code, {}) should deny access to the builtins. This PR would make it trivial to get the builtins.

ns={}; eval(code, ns) sets ns["__builtins__"] to the current builtins namespace in Python 3.9 and in 3.10. My PR doesn't change that.

static PyObject *
builtin_eval_impl(PyObject *module, PyObject *source, PyObject *globals,
                  PyObject *locals)
{
    ...
    int r = _PyDict_ContainsId(globals, &PyId___builtins__);
    if (r == 0) {
        r = _PyDict_SetItemId(globals, &PyId___builtins__,
                              PyEval_GetBuiltins());
    }
    ...
}

I added an unit test on the exec(code, {}) use case.

By the way, Python doesn't support sandboxes. There are many ways to access builtins. It has been proved by a very long list of "exploits" on the various Python sandbox attempts in the past.

[markshannon]

I had assumed that eval behaved the same as functions. Evidently I was wrong. In which case I am happy with the general idea.

Would you mind putting the relevant changes in one PR and refactor in another?
Mixing them makes it hard to review the code and if the changes need to be reverted, it is impossible to do so without reverting the refactoring.

Also, do we really need yet another PyThreadState *tstate parameter?

[vstinner]

Without this PR, func_builtins2.py of https://bugs.python.org/issue43228 currently fails on master: "NameError: name 'len' is not defined".

With this PR, func_builtins2.py works as expected (no exception).

[vstinner]

I had assumed that eval behaved the same as functions. Evidently I was wrong. In which case I am happy with the general idea.

To be honest, I also "re-discovered" that exec() has this special behavior.

Would you mind putting the relevant changes in one PR and refactor in another?

Right, sorry. I created PR #24566. I will merge it, and then rebase this one on top of it.

[vstinner]

Also, do we really need yet another PyThreadState *tstate parameter?

The rationale for passing explicitly tstate is that getting the current Python Thread State may become slower when https://bugs.python.org/issue40522 will be fixed (to run multiple interpreters in parallel).

I also expect that the compiler will be able to emit faster machine code, especially when using LTO, if tstate is passed explicitly. For example, I expect _PyErr_Occurred() to be inlined.

[vstinner]

I rebased my PR to restrict this change to the FunctionType constructor change. I merged the refactoring changes in a separated PR.

[vstinner]

I completed the documentation to clarify that this change makes types.FunctionType constructor more consistent with other Python functions like eval() and exec().

[vstinner]

@gvanrossum @pablogsal: Does this behavior change sound reasonable to you?

[gvanrossum]

LGTM, just two doc nits.

[vstinner]

@gvanrossum: Thanks for the review, I merged my PR.