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

Step Into not listing targets properly #576

Closed
fabioz opened this issue Mar 29, 2021 · 4 comments
Closed

Step Into not listing targets properly #576

fabioz opened this issue Mar 29, 2021 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@fabioz
Copy link
Collaborator

fabioz commented Mar 29, 2021

Doing some testing I found an issue in how the targets are computed.

A construct such as:

    def function():
        yield sys._getframe()
        bbb = foo.bar(
            Something(param1, param2=xxx.yyy), {}
        )
        call()

Is not finding bar properly.

The main issue is that a stack is created and later when a CALL_FUNCTION bytecode is found items are popped from the stack, but the stack is incomplete, so, either the way to deal with the stack becomes much more complete (as we need to deal with much more bytecode to have a complete picture on how the stack would look -- which may not be worth it) or go with a simpler approach to just list the names without validating if those would actually be function calls (which may be reasonable since cases such as accessing properties would also work -- those don't work right now with the current approach).

I'm pending towards implementing the 2nd approach...

@fabioz fabioz self-assigned this Mar 29, 2021
@int19h int19h added the bug Something isn't working label Mar 31, 2021
@fabioz
Copy link
Collaborator Author

fabioz commented Apr 1, 2021

@int19h @karthiknadig the approach I had in mind doesn't work because the bytecode offset won't match.

i.e.: What the step into target currently does is check in a called frame that the back frame was the initial frame where the step into was requested and the bytecode offset matches the expected bytecode for the call. The LOAD_XXX instructions have the wrong offset to check that, so, the only way I can see how to fix this is actually properly implementing the stack operations to know which name matches a given call operation (or possibly a LOAD_ATTR to deal with @property).

It'd be comparable to something as: https://github.com/nedbat/byterun/blob/master/byterun/pyvm2.py but only working at the metadata level, not really interpreting code.

So, I think this is going to take a while to implement (my guess is it'll take around 10-15 working days (which at my current time allocated means something around 2 months) as it needs to support many versions of python, which means lots of bytecodes to test). My plan is starting to work at that right now, but possibly pausing for smaller (or more critical) issues over time.

-- I'm also open to other approaches you may envision here.... another approach I thought of would be using the actual source code and creating an AST and then trying to do a match from which name loaded matches which called name to match the bytecode -- I think it'd be faster to implement, but I'm not 100% sure we can cover all cases this way and it wouldn't work on cases where sources aren't available.

@fabioz
Copy link
Collaborator Author

fabioz commented Apr 1, 2021

@int19h @karthiknadig I think that if we could support only Python 3.5 onwards for this feature it'd be nicer to implement.

That would enable using https://github.com/MatthieuDartiailh/bytecode (which is already integrated into debugpy) as well as ignoring a bunch of Python 2.7 specific bytecode...

Do you think that's ok?

@karthiknadig
Copy link
Member

karthiknadig commented Apr 1, 2021

3.5+ for this feature is fine.

@fabioz
Copy link
Collaborator Author

fabioz commented Apr 16, 2021

Note: I'm still working on this (it's a big feature and it'll still take a while to be complete).

In a proof of concept the new approach seems to be working reasonably well...

I also found an interesting corner case (which I'm also working on as a part of this issue), which is stepping into a function which is inside a generator -- in this case an intermediary frame such as <listcomp> or <genexpr> is created, so, we need to stop based on 2 bytecode offsets, not a single one as the back frame is not immediately what'd be expected in the regular case.

fabioz added a commit to fabioz/debugpy that referenced this issue Apr 29, 2021
Description:
- Changes the structure to create a stack from the bytecode information
  so that function call names are correctly computed (this is still a work
  in progress as not all bytecode instructions are currently supported).
- Support to step into function calls inside comprehensions/generators.
- Updates the bytecode dependency to 0.12.0.
  Note: changes were done to the bytecode library to keep the bytecode offset.
fabioz added a commit to fabioz/debugpy that referenced this issue Apr 30, 2021
Description:
- Changes the structure to create a stack from the bytecode information
  so that function call names are correctly computed (this is still a work
  in progress as not all bytecode instructions are currently supported).
- Support to step into function calls inside comprehensions/generators.
- Updates the bytecode dependency to 0.12.0.
  Note: changes were done to the bytecode library to keep the bytecode offset.
fabioz added a commit to fabioz/debugpy that referenced this issue Apr 30, 2021
Description:
- Changes the structure to create a stack from the bytecode information
  so that function call names are correctly computed (this is still a work
  in progress as not all bytecode instructions are currently supported).
- Support to step into function calls inside comprehensions/generators.
- Updates the bytecode dependency to 0.12.0.
  Note: changes were done to the bytecode library to keep the bytecode offset.
int19h pushed a commit that referenced this issue May 4, 2021
Description:
- Changes the structure to create a stack from the bytecode information
  so that function call names are correctly computed (this is still a work
  in progress as not all bytecode instructions are currently supported).
- Support to step into function calls inside comprehensions/generators.
- Updates the bytecode dependency to 0.12.0.
  Note: changes were done to the bytecode library to keep the bytecode offset.
fabioz added a commit to fabioz/debugpy that referenced this issue May 21, 2021
fabioz added a commit to fabioz/debugpy that referenced this issue May 21, 2021
@fabioz fabioz closed this as completed in ab4b969 May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants