Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-111744: Support opcode events in bdb#111834

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

gaogaotiantian
Copy link
Member

@gaogaotiantiangaogaotiantian commentedNov 7, 2023•
edited by bedevere-appbot
Loading

This is the first step to solve#111744 . After some discussion in discord, most of the developers like the idea to havebreakpoint() 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() anduser_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 newbreakpoint(), we can keep quiet about it. If we decide to make it available to public, I can write the docs.

@gaogaotiantian
Copy link
MemberAuthor

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 abreakpoint 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.

Copy link
Member

@brandtbucherbrandtbucher left a 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:

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
Copy link
Member

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 whereset_trace was called (these are the same frames wheref_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 seepdb.Pdb has acurframe attribute, so maybe it's to avoid confusion with that?

Copy link
MemberAuthor

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 withpdb.Pdb.curframe. And yes, we only want to settrace_opcodes in the "user frames", which means frames above (including) the caller frame into the pdb. So we need to record the entering frame.

Thecurframe attribute ofpdb means the current frame being debugged. Do you prefer to change the the name__curframe to something likeentering_frame?

Copy link
Member

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 havestopframe,returnframe, andbotframe.

Co-authored-by: Brandt Bucher <brandtbucher@gmail.com>
@brandtbucher
Copy link
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 thebreakpoint() change is feasible for 3.13? I'm available for review (I promise).

@brandtbucher
Copy link
Member

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

@gaogaotiantian
Copy link
MemberAuthor

#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

@brandtbucher
Copy link
Member

brandtbucher commentedMay 4, 2024•
edited
Loading

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.

@brandtbucherbrandtbucher self-assigned thisMay 4, 2024
@gaogaotiantian
Copy link
MemberAuthor

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 afterbreakpoint() - it might be the same line. I'll see if it causes issues.

@brandtbucher
Copy link
Member

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

@gaogaotiantian
Copy link
MemberAuthor

I usedenterframe to match the style to others.

Copy link
Member

@brandtbucherbrandtbucher left a 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:

@bedevere-app
Copy link

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 phraseI 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.

@bedevere-bot
Copy link

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

Hi! The buildbotiOS ARM64 Simulator 3.x has failed when building commitf34e965.

What do you need to do:

  1. Don't panic.
  2. Checkthe 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 0From https://github.com/python/cpython * branch                  main       -> FETCH_HEADNote:switching to 'f34e965e52b9bdf157b829371870edfde45b80bf'.You are in 'detached HEAD' state. You can look around, make experimentalchanges and commit them, and you can discard any commits you make in thisstate without impacting any branches by switching back to a branch.If you want to create a new branch to retain commits you create, you maydo 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 falseHEAD 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 hereif (key!=NULL) {^~~../../Python/import.c:1057:9: note: remove the 'if' if its condition is always falseif (_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 hereif (key!=NULL) {^~~../../Python/import.c:1057:9: note: remove the 'if' if its condition is always falseif (_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.0SQLITE_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.0SQLITE_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>", line1, 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

@gaogaotiantiangaogaotiantian deleted the bdb-support-opcode branchMay 6, 2024 19:43
SonicField pushed a commit to SonicField/cpython that referenced this pull requestMay 8, 2024
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@brandtbucherbrandtbucherbrandtbucher approved these changes

Assignees

@brandtbucherbrandtbucher

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@gaogaotiantian@brandtbucher@bedevere-bot
[8]ページ先頭
©2009-2025 Movatter.jp