Bug report

Bug description:

According to Python docs, exec and eval take as their globals argument an exact dict, and not a subclass of it.

If only globals is provided, it must be a dictionary (and not a subclass of dictionary), which will be used for both the global and the local variables. If globals and locals are given, they are used for the global and local variables, respectively. If provided, locals can be any mapping object.

However, builtin_exec_impl allows subclasses of dict because it checks the argument with PyDict_Check instead of PyDict_CheckExact (see here).

builtin_eval_impl makes the same, wrong, validation, with a slightly different error message. (see here).

This is a problem because it allows developers to pass a dictionary with a custom __getitem__ method which will never actually get called. It will not get called because when opcode LOAD_NAME is executed, it assumes globals is a dict, and accesses it using PyDict_GetItemRef instead of PyMapping_GetOptionalItem (see here).

Furthermore, opcode LOAD_GLOBAL behaves slightly different, first checking if globals is an exact dict and then choosing how to access it.

Here is an example of the inconsistent and unexpected behavior:

class mydict(dict):
  def __getitem__(self, key): 
    print('accessing', key)
    if key == 'f': return "hi!"
    return super().__getitem__(key)

exec('global f; print(f)', mydict(), {}) # This works, it finds f
exec('print(f)', mydict(), {}) # This, surprisingly, does not work
exec('print(f)', mydict()) # But this does work, because locals is a copy of globals and it supports the custom __getitem__

There even is a test (test_builtin.py, see here) that says "custom globals or builtins can raise errors on item access". The test passes only because a locals is not provided to exec, so exec copies it from globals. A custom globals cannot raise errors on item access.

If one applies the change I think must be made (Changing PyDict_Check to PyDict_CheckExact in exec and eval's implementations) 4 more tests break.

• test_getpath passes custom dictionaries to exec (which, remember, the documentation forbids), and should be rewritten but does not seem to be very complex to do.
• test_descrtut should change exec("x = 3; print(x)", a) to exec("x = 3; print(x)", {}, a), and then not expect 'builtins' to appear in a.keys()
• test_dynamic's test_load_global_specialization_failure_keeps_oparg should be rewritten to, instead of creating a class with __missing__, create a dictionary with data (for i in range(variables): my_globals[f"_number_{i}"] = i)
• test_type_aliases. test_exec_with_unusual_globals fails, but I do not understand how to fix it.

Alternatively, _PyEval_LoadName could be modified (and the documentation updated) so subclasses of dictionaries are supported as globals, but this is a hot path and I have not measured any possible performance implications. Modifying exec to comply with the docs makes more sense, IMHO.

CPython versions tested on:

CPython main branch, 3.12, 3.10, 3.9

Operating systems tested on:

Linux