GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-111744: Support opcode events in bdb by gaogaotiantian · Pull Request #111834 · python/cpython

Conversation

@gaogaotiantian

@gaogaotiantian gaogaotiantian commented Nov 7, 2023 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

This is the first step to solve #111744 . After some discussion in discord, most of the developers like the idea to have breakpoint() break immediately, rather than on next event. In order to achieve that, we need to use the opcode events.

This PR is just to lay the ground for opcode events. In general, the users can use opcode events now with a new set of API - set_stepinstr() and user_opcode(). The documentation is not updated on purpose - if we prefer to keep it internal now and just use it as an infra to support new breakpoint(), we can keep quiet about it. If we decide to make it available to public, I can write the docs.

  • Issue: breakpoint does not enter pdb until the "next event" happens #111744

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
gaogaotiantian added 2 commits November 7, 2023 14:21
@gaogaotiantian
Support opcode events in bdb
c313480
@gaogaotiantian
Add basic test for stepinstr
2606580
@bedevere-app bedevere-app Bot added the awaiting review label Nov 7, 2023
@gaogaotiantian
gaogaotiantian requested a review from brandtbucher November 7, 2023 22:40
@bedevere-app bedevere-app Bot mentioned this pull request Nov 7, 2023
breakpoint does not enter pdb until the "next event" happens #111744
Closed
@blurb-it
📜🤖 Added by blurb_it.
99ec1e1
@gaogaotiantian

gaogaotiantian commented Nov 29, 2023

Copy link
Copy Markdown
Member Author

Hi @brandtbucher , I know you've been busy recently. When you have some time, could you take a look at this? This is the prerequisite for a breakpoint that breaks immediately. That change might need more reviews as it's a breaking behavior so I kind of hope I can lay the foundation soon.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

brandtbucher
brandtbucher approved these changes May 4, 2024
View reviewed changes

@brandtbucher brandtbucher left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a ton for your patience (I really dropped the ball on reviewing this)! I'd love to get your thoughts on a couple of things before merging:

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Lib/bdb.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/bdb.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/bdb.py Outdated
def _set_trace_opcodes(self, trace_opcodes):
if trace_opcodes != self.trace_opcodes:
self.trace_opcodes = trace_opcodes
frame = self.__curframe

@brandtbucher brandtbucher May 4, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And just to jog my memory: self.__curframe is needed because we only want to change events at or above the frame where set_trace was called (these are the same frames where f_trace_lines was set), but we may be much deeper at this point. Right?

But why is it double-underscored again? Do we need the name-mangling? I see pdb.Pdb has a curframe attribute, so maybe it's to avoid confusion with that?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@gaogaotiantian gaogaotiantian May 4, 2024

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the reason for the double-underscore is to avoid the conflict with pdb.Pdb.curframe. And yes, we only want to set trace_opcodes in the "user frames", which means frames above (including) the caller frame into the pdb. So we need to record the entering frame.

The curframe attribute of pdb means the current frame being debugged. Do you prefer to change the the name __curframe to something like entering_frame?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@brandtbucher brandtbucher May 4, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that feels a lot less cryptic and scary. :) Especially considering we already have stopframe, returnframe, and botframe.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels May 4, 2024
@gaogaotiantian @brandtbucher
Update Lib/bdb.py
a91fe16
Co-authored-by: Brandt Bucher <[email protected]>
@brandtbucher

brandtbucher commented May 4, 2024

Copy link
Copy Markdown
Member

Also, I played with this a bit while reviewing, and it's pretty neat. I know time's running out: do you think the rest of the breakpoint() change is feasible for 3.13? I'm available for review (I promise).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gaogaotiantian
Combine the two dicts into one
10ba295
@brandtbucher

brandtbucher commented May 4, 2024

Copy link
Copy Markdown
Member

Also, what's the relationship between this PR and GH-103050?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gaogaotiantian

gaogaotiantian commented May 4, 2024

Copy link
Copy Markdown
Member Author

#103050 was the bigger ambition to directly support instruction level debugging back then. It was not easy to push something that big at that time when I did not have some basic reputation of "this person knows what he's doing". That one was kind of abandoned and the most critical part (supporting opcode event in general) was adopted into this PR.

I will try to pursue it again in 3.14, alongwith some other more ambitious targets, but that's after the language summit :)

And to your question, I will finish the breakpoint change by the end of tomorrow and see if we can make it in 3.13.

brandtbucher reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@brandtbucher

brandtbucher commented May 4, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

And to your question, I will finish the breakpoint change by the end of tomorrow and see if we can make it in 3.13.

Let me know if there's anything I can do to help, besides being available for review. I think it's a great change.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@brandtbucher brandtbucher self-assigned this May 4, 2024
@gaogaotiantian

gaogaotiantian commented May 4, 2024

Copy link
Copy Markdown
Member Author

I had the prototype working once it should not be too difficult. Although this might break some tests - now that it stopped after the instruction after breakpoint() - it might be the same line. I'll see if it causes issues.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@brandtbucher

brandtbucher commented May 4, 2024

Copy link
Copy Markdown
Member

I'm ready to land this once __curframe is renamed. :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gaogaotiantian added 2 commits May 3, 2024 22:14
@gaogaotiantian
Merge branch 'main' into bdb-support-opcode
36fba4c
@gaogaotiantian
Rename __curframe to enterframe
65a3a76
@gaogaotiantian

gaogaotiantian commented May 4, 2024

Copy link
Copy Markdown
Member Author

I used enterframe to match the style to others.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

brandtbucher
brandtbucher requested changes May 4, 2024
View reviewed changes

@brandtbucher brandtbucher left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, spotted one more thing:

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Lib/bdb.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot added awaiting changes and removed awaiting merge labels May 4, 2024
@bedevere-app

bedevere-app Bot commented May 4, 2024

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gaogaotiantian
Fix a missing piece
05072f4
@gaogaotiantian
gaogaotiantian requested a review from brandtbucher May 4, 2024 06:44
brandtbucher
brandtbucher approved these changes May 4, 2024
View reviewed changes
@bedevere-app bedevere-app Bot removed the awaiting changes label May 4, 2024
@bedevere-app bedevere-app Bot added the awaiting merge label May 4, 2024
@brandtbucher
brandtbucher merged commit f34e965 into python:main May 4, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label May 4, 2024
@bedevere-bot

bedevere-bot commented May 4, 2024

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot iOS ARM64 Simulator 3.x has failed when building commit f34e965.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/1380/builds/199) and take a look at the build logs.
  4. Check if the failure is related to this commit (f34e965) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/1380/builds/199

Summary of the results of the build (if available):

Click to see traceback logs
remote: Enumerating objects: 22, done.        
remote: Counting objects:   4% (1/22)        
remote: Counting objects:   9% (2/22)        
remote: Counting objects:  13% (3/22)        
remote: Counting objects:  18% (4/22)        
remote: Counting objects:  22% (5/22)        
remote: Counting objects:  27% (6/22)        
remote: Counting objects:  31% (7/22)        
remote: Counting objects:  36% (8/22)        
remote: Counting objects:  40% (9/22)        
remote: Counting objects:  45% (10/22)        
remote: Counting objects:  50% (11/22)        
remote: Counting objects:  54% (12/22)        
remote: Counting objects:  59% (13/22)        
remote: Counting objects:  63% (14/22)        
remote: Counting objects:  68% (15/22)        
remote: Counting objects:  72% (16/22)        
remote: Counting objects:  77% (17/22)        
remote: Counting objects:  81% (18/22)        
remote: Counting objects:  86% (19/22)        
remote: Counting objects:  90% (20/22)        
remote: Counting objects:  95% (21/22)        
remote: Counting objects: 100% (22/22)        
remote: Counting objects: 100% (22/22), done.        
remote: Compressing objects:  11% (1/9)        
remote: Compressing objects:  22% (2/9)        
remote: Compressing objects:  33% (3/9)        
remote: Compressing objects:  44% (4/9)        
remote: Compressing objects:  55% (5/9)        
remote: Compressing objects:  66% (6/9)        
remote: Compressing objects:  77% (7/9)        
remote: Compressing objects:  88% (8/9)        
remote: Compressing objects: 100% (9/9)        
remote: Compressing objects: 100% (9/9), done.        
remote: Total 12 (delta 10), reused 3 (delta 2), pack-reused 0        
From https://github.com/python/cpython
 * branch                  main       -> FETCH_HEAD
Note: switching to 'f34e965e52b9bdf157b829371870edfde45b80bf'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at f34e965e52 GH-111744: Support opcode events in bdb (GH-111834)
Switched to and reset branch 'main'

configure: WARNING: pkg-config is missing. Some dependencies may not be detected correctly.

../../Objects/frameobject.c:240:57: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
framelocalsproxy_keys(PyObject *self, PyObject *__unused)
                                                        ^
../../Objects/frameobject.c:391:59: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
framelocalsproxy_values(PyObject *self, PyObject *__unused)
                                                          ^
../../Objects/frameobject.c:419:58: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
framelocalsproxy_items(PyObject *self, PyObject *__unused)
                                                         ^
../../Objects/frameobject.c:568:61: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
framelocalsproxy_reversed(PyObject *self, PyObject *__unused)
                                                            ^
4 warnings generated.
../../Python/import.c:1057:13: warning: variable 'key' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
        if (_extensions_cache_init() < 0) {
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Python/import.c:1095:9: note: uninitialized use occurs here
    if (key != NULL) {
        ^~~
../../Python/import.c:1057:9: note: remove the 'if' if its condition is always false
        if (_extensions_cache_init() < 0) {
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Python/import.c:1063:5: note: variable 'key' is declared here
    void *key = NULL;
    ^
../../Python/import.c:1195:1: warning: unused function 'is_core_module' [-Wunused-function]
is_core_module(PyInterpreterState *interp, PyObject *name, PyObject *path)
^
2 warnings generated.
../../Python/pystate.c:1125:1: warning: unused function 'check_interpreter_whence' [-Wunused-function]
check_interpreter_whence(long whence)
^
1 warning generated.

configure: WARNING: pkg-config is missing. Some dependencies may not be detected correctly.

../../Objects/frameobject.c:240:57: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
framelocalsproxy_keys(PyObject *self, PyObject *__unused)
                                                        ^
../../Objects/frameobject.c:391:59: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
framelocalsproxy_values(PyObject *self, PyObject *__unused)
                                                          ^
../../Objects/frameobject.c:419:58: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
framelocalsproxy_items(PyObject *self, PyObject *__unused)
                                                         ^
../../Objects/frameobject.c:568:61: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
framelocalsproxy_reversed(PyObject *self, PyObject *__unused)
                                                            ^
4 warnings generated.
../../Python/import.c:1057:13: warning: variable 'key' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
        if (_extensions_cache_init() < 0) {
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Python/import.c:1095:9: note: uninitialized use occurs here
    if (key != NULL) {
        ^~~
../../Python/import.c:1057:9: note: remove the 'if' if its condition is always false
        if (_extensions_cache_init() < 0) {
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Python/import.c:1063:5: note: variable 'key' is declared here
    void *key = NULL;
    ^
1 warning generated.
../../Modules/_sqlite/connection.c:1307:14: warning: 'sqlite3_create_window_function' is only available on iOS 13.0 or newer [-Wunguarded-availability-new]
        rc = sqlite3_create_window_function(self->db, name, num_params, flags,
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator17.4.sdk/usr/include/sqlite3.h:5533:16: note: 'sqlite3_create_window_function' has been marked as being introduced in iOS 13.0 here, but the deployment target is iOS 12.0.0
SQLITE_API int sqlite3_create_window_function(
               ^
../../Modules/_sqlite/connection.c:1307:14: note: enclose 'sqlite3_create_window_function' in a __builtin_available check to silence this warning
        rc = sqlite3_create_window_function(self->db, name, num_params, flags,
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Modules/_sqlite/connection.c:1315:14: warning: 'sqlite3_create_window_function' is only available on iOS 13.0 or newer [-Wunguarded-availability-new]
        rc = sqlite3_create_window_function(self->db, name, num_params, flags,
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator17.4.sdk/usr/include/sqlite3.h:5533:16: note: 'sqlite3_create_window_function' has been marked as being introduced in iOS 13.0 here, but the deployment target is iOS 12.0.0
SQLITE_API int sqlite3_create_window_function(
               ^
../../Modules/_sqlite/connection.c:1315:14: note: enclose 'sqlite3_create_window_function' in a __builtin_available check to silence this warning
        rc = sqlite3_create_window_function(self->db, name, num_params, flags,
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.

xcodebuild: error: 'iOSTestbed.arm64-iphonesimulator.1714834279/iOSTestbed.xcodeproj' does not exist.
Traceback (most recent call last):
  File "<string>", line 1, in <module>
    import sys, json; result = json.load(sys.stdin); print(result['subsections']['_values'][1]['subsections']['_values'][0]['emittedOutput']['_value'])
                                                           ~~~~~~^^^^^^^^^^^^^^^
KeyError: 'subsections'
make: *** [testios] Error 1
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gaogaotiantian
gaogaotiantian deleted the bdb-support-opcode branch May 6, 2024 19:43
SonicField pushed a commit to SonicField/cpython that referenced this pull request May 8, 2024
@gaogaotiantian @SonicField
pythonGH-111744: Support opcode events in bdb (pythonGH-111834)
f087d87
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brandtbucher brandtbucher brandtbucher approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@brandtbucher brandtbucher

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@gaogaotiantian @brandtbucher @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.