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

GH-100987: Allow non python frames in frame stack. #103010

Closed

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Mar 24, 2023

Inserting frames for builtin methods, method descriptors and other similar C callables into the frame stack can improve profiling and debugging.
Better still it can be done at very low cost.

Python/ceval.c Show resolved Hide resolved
Tools/gdb/libpython.py Show resolved Hide resolved
@@ -1076,7 +1080,10 @@ def _f_builtins(self):
return self._f_special("f_builtins")

def _f_code(self):
return self._f_special("f_code", PyCodeObjectPtr.from_pyobject_ptr)
return self._f_special("f_executable", PyCodeObjectPtr.from_pyobject_ptr)
Copy link
Member

Choose a reason for hiding this comment

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

Can we instead use self._f_executable here, test the type, and raise an explicit exception (or return None?) if it is not a code object?

Copy link
Member Author

Choose a reason for hiding this comment

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

All uses of this function should know that self._f_executable or be guarded against it not being.

pnames = self.co.field('co_localsplusnames')
self.co_localsplusnames = PyTupleObjectPtr.from_pyobject_ptr(pnames)
try:
self.co = self._f_code()
Copy link
Member

Choose a reason for hiding this comment

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

It would be better to get a clear signal right here and bail out as needed, than to have a blanket try/except around this entire section.

Copy link
Member Author

Choose a reason for hiding this comment

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

Because this code runs inside gdb and there seems to be no way to see into the embedded Python interpreter, it is really hard to debug and experiment with. I'm reluctant to make any changes beyond the minimum to make it work.

Comment on lines +286 to +287
#define CONSTS() ((PyCodeObject *)frame->base.f_executable)->co_consts
#define NAMES() ((PyCodeObject *)frame->base.f_executable)->co_names
Copy link
Member

Choose a reason for hiding this comment

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

any particular reason not to use _PyFrame_GetCode here?

Python/ceval.c Show resolved Hide resolved
@markshannon markshannon added the 3.13 bugs and security fixes label May 9, 2023
@markshannon
Copy link
Member Author

Doing this enables us to remove then need for fake code objects for shims. We can just set the f_executable field to None and point prev_instr into a static array of (immutable) instructions.

@markshannon markshannon closed this May 9, 2023
@markshannon
Copy link
Member Author

I'm closing this for now.
First we want to allow any "executable" instead of code objects. Once we that is done, we can allow smaller frames consisting of just the "executable" and previous pointer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes awaiting core review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants