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

Jump to bottom

gh-99110: Initialize frame->previous in init_frame to fix segmentation fault #100182

Merged
merged 10 commits into from Dec 23, 2022
Merged

gh-99110: Initialize frame->previous in init_frame to fix segmentation fault #100182

merged 10 commits into from Dec 23, 2022
+34 −1

Conversation

byllyfish
Copy link
Contributor

@byllyfish byllyfish commented Dec 12, 2022 •
edited by bedevere-bot

This PR fixes a segmentation fault caused by an uninitialized variable. One way to trigger the seg. fault is to use the PyFrame_New() C API. I've also run across this segmentation fault by calling asyncio.Task.get_stack() on cython based asyncio Task (in the grpcio module).

The fix for #99110 needs to be back-ported to 3.11.

@bedevere-bot bedevere-bot mentioned this pull request Dec 12, 2022
Closed
@bedevere-bot
Copy link

bedevere-bot commented Dec 12, 2022

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Dec 12, 2022 •
edited

All commit authors signed the Contributor License Agreement.
CLA signed

@markshannon
Copy link
Member

markshannon commented Dec 13, 2022

_PyInterpreterFrame.previous gets initialized when the frame is linked into the frame stack.
I'd rather not add extra code to _PyFrame_InitializeSpecials, as it is performance critical.

_PyInterpreterFrame.previous could be set to NULL when initializing the frame object

@python python deleted a comment from netlify bot Dec 13, 2022
@byllyfish
Move initialization of frame->previous to init_frame.
d679783 
Use self.assertEquals instead of assert.
@byllyfish byllyfish changed the title gh-99110: Initialize frame->previous in _PyFrame_InitializeSpecials gh-99110: Initialize frame->previous in init_frame to fix segmentation fault Dec 13, 2022
@gnossen gnossen mentioned this pull request Dec 16, 2022
Closed
markshannon
markshannon requested changes Dec 16, 2022
View changes
Include/internal/pycore_frame.h Outdated Show resolved Hide resolved
Lib/test/test_ctypes/test_python_api.py Outdated Show resolved Hide resolved
@bedevere-bot
Copy link

bedevere-bot commented Dec 16, 2022

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.

@byllyfish
Copy link
Contributor Author

byllyfish commented Dec 18, 2022

I moved the test code into the TestCAPI unit test class in test_frame.py.

I have made the requested changes; please review again.

@bedevere-bot
Copy link

bedevere-bot commented Dec 18, 2022

Thanks for making the requested changes!

@markshannon: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from markshannon Dec 18, 2022
@kumaraditya303 kumaraditya303 added the type-crash A hard crash of the interpreter, possibly with a core dump label Dec 23, 2022
kumaraditya303
kumaraditya303 reviewed Dec 23, 2022
View changes
Lib/test/test_frame.py Outdated Show resolved Hide resolved
markshannon
markshannon approved these changes Dec 23, 2022
View changes
Copy link
Member

@markshannon markshannon left a comment

Looks good now. Thanks.

@kumaraditya303 kumaraditya303 linked an issue Dec 23, 2022 that may be closed by this pull request
Segfault on frame.f_back when frame is created with PyFrame_New() #99110
Closed
@kumaraditya303 kumaraditya303 merged commit 88d565f into python:main Dec 23, 2022
15 of 16 checks passed
@miss-islington
Copy link
Contributor

miss-islington commented Dec 23, 2022

Thanks @byllyfish for the PR, and @kumaraditya303 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

miss-islington commented Dec 23, 2022

Sorry, @byllyfish and @kumaraditya303, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 88d565f32a709140664444c6dea20ecd35a10e94 3.11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kumaraditya303 kumaraditya303

@markshannon markshannon

@brandtbucher brandtbucher

Assignees

@kumaraditya303 kumaraditya303

Labels
needs backport to 3.11 type-crash A hard crash of the interpreter, possibly with a core dump
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Segfault on frame.f_back when frame is created with PyFrame_New()
5 participants
@byllyfish @bedevere-bot @markshannon @miss-islington @kumaraditya303