Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C API] Add PyFrame_GetVar(frame, name) function #91248

Closed
vstinner opened this issue Mar 22, 2022 · 12 comments
Closed

[C API] Add PyFrame_GetVar(frame, name) function #91248

vstinner opened this issue Mar 22, 2022 · 12 comments
Labels
3.12 bugs and security fixes topic-C-API

Comments

@vstinner
Copy link
Member

vstinner commented Mar 22, 2022

BPO 47092
Nosy @ncoghlan, @vstinner, @markshannon

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2022-03-22.14:41:38.157>
labels = ['expert-C-API', '3.11']
title = '[C API] Add PyFrame_GetVar(frame, name) function'
updated_at = <Date 2022-03-30.15:00:53.282>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2022-03-30.15:00:53.282>
actor = 'ncoghlan'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['C API']
creation = <Date 2022-03-22.14:41:38.157>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 47092
keywords = []
message_count = 8.0
messages = ['415776', '415779', '415780', '415784', '415790', '415792', '415802', '416371']
nosy_count = 3.0
nosy_names = ['ncoghlan', 'vstinner', 'Mark.Shannon']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue47092'
versions = ['Python 3.11']

@vstinner
Copy link
Member Author

In Python 3.10, it's possible to call PyFrame_FastToLocalsWithError() on a frame to get all variables as a dictionary. In Python, getting frame.f_locals calls PyFrame_FastToLocalsWithError(). It's used by the pdb module for example:

    self.curframe_locals = self.curframe.f_locals

The PyFrame_FastToLocalsWithError() function has multiple issues:

  • It's inefficient.
  • It may create a Python dictionary.
  • It can fail to add items to the dictionary.
  • Allocating memory can fail with memory allocation failure.

The problem is that a debugger or profiler should not modify a process when it inspects it. It should avoid allocation memory for example. I propose adding a new API to prevent that.

In Python 3.11, the PyFrameObject structure became opaque and changed deeply. There are differend kinds of variables stored differently:

  • Free variables: maybe in frame->f_func->func_closure[index], maybe in frame->localsplus[index]
  • Fast variable: frame->f_frame->localsplus[index]
  • Cell variable: also need to call PyCell_GET()

Well... Look at _PyFrame_FastToLocalsWithError(), it's quite complicated ;-)

I propose to add a new public C API just to get a single variable value:

  PyObject* PyFrame_GetVar(PyFrameObject *frame, PyObject *name)

I prefer to use a PyObject* for the name. You can use PyUnicode_FromString() to get a PyObject* from a const char*.

This function would get the value where the variable is stored, it doesn't have a to create a dictionary. Return NULL if the variable doesn't exist.

If I understand correctly, it should be possible to ensure that this function would never raise an exception (never "fail"). So it should be possible to call it even if an exception is raised, which is convenient for a debugger.

--

I plan to implement this API, but first I would like to make sure that there is an agreement that such API is needed and helpful ;-)

--

See also draft PEP-558 and PEP-667 which propose API to *modify* variables and make sure that they remain consistent when they are set and then get. The scope of these PEPs is way wider than the simple propose PyFrame_GetVar() which would be simpler implementation than PyFrame_FastToLocalsWithError().

@vstinner vstinner added 3.11 only security fixes topic-C-API labels Mar 22, 2022
@vstinner
Copy link
Member Author

In Python 3.10, it's possible to call PyFrame_FastToLocalsWithError() on a frame to get all variables as a dictionary.

In 2018, it was decided to *not* document this function: see bpo-19431.

In C, It is possible to call PyObject_GetAttrString(frame, "f_locals") to call indirectly _PyFrame_FastToLocalsWithError() and get the dictionary.

@vstinner
Copy link
Member Author

Currently, Tools/gdb/libpython.py uses PyFramePtr.iter_locals() which iterates on PyFrameObject.f_frame.localsplus.

There is a PyFramePtr.get_var_by_name() function which only checks for frame variables in PyFrameObject.f_frame.localsplus, or look up in globals and builtins. So it only supports some kinds of variables.

@markshannon
Copy link
Member

I'm looking into adding two new APIs.
One to round out the getters for FrameObject and one to introspect the internal frame stack.

It would probably make more sense to add this capability to the frame stack API, as it would avoid creating the frame object as well as the locals dictionary.

E.g. PyFrameStack_GetVar(int depth, PyObject *name)

@vstinner
Copy link
Member Author

See also:

@vstinner
Copy link
Member Author

If PyFrameStack_GetVar(depth, name) is added, would it make PyFrame_GetVar() irrelevant?

@vstinner
Copy link
Member Author

See also bpo-42197: Disable automatic update of frame locals during tracing.

@ncoghlan
Copy link
Contributor

Mark's suggested frame stack API makes sense to me, but being able to get/set specific values through the full frame API seems like it would be useful even if the lower level API existed - if we don't get one of the proxy PEPs in shape to land in 3.11, trace functions written in C could still use this to avoid materialising the locals dict if they only needed to manipulate specific values.

Even after a fast locals proxy is defined, there would still be some saving in skipping creating the proxy object when only accessing known keys.

We'd need the name-to-index mapping on the code objects to implement this API efficiently, but that isn't a PEP level change in its own right (the proxy PEPs only mention it because they need it)

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@kumaraditya303 kumaraditya303 added 3.12 bugs and security fixes and removed 3.11 only security fixes labels Jul 14, 2022
@vstinner
Copy link
Member Author

vstinner commented Aug 1, 2022

A function like PyFrame_GetVar() may help for such use case: https://discuss.python.org/t/getting-the-class-name-of-a-code-frame-object-in-cpython-3-11-c-api/17396

@vstinner
Copy link
Member Author

vstinner commented Aug 5, 2022

I propose a concrete implementation of my idea: PR #95712.

[PyFrameStack_GetVar] would probably make more sense to add this capability to the frame stack API, as it would avoid creating the frame object as well as the locals dictionary.

The problem is about porting existing Python 3.10 code to Python 3.11 and Python 3.12. Existing code works directly on Python frame objects. But yeah, in the future, there is likely room for enhancements, to design an even more efficient API which avoid creating Python frame objects.

vstinner added a commit that referenced this issue Nov 8, 2022
Add PyFrame_GetVar() and PyFrame_GetVarString() functions to get a
frame variable by its name.

Move PyFrameObject C API tests from test_capi to test_frame.
@vstinner
Copy link
Member Author

vstinner commented Nov 8, 2022

Fixed by commit 4d5fcca

@vstinner vstinner closed this as completed Nov 8, 2022
vstinner added a commit that referenced this issue Nov 13, 2022
PyFrame_GetVar() no longer creates a temporary dictionary to get a
variable.
@vstinner
Copy link
Member Author

Implementation optimized by 6788303 to avoid the creation of a temporary dictionary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes topic-C-API
Projects
None yet
Development

No branches or pull requests

4 participants