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

bpo-39091: Fix segfault when Exception constructor returns non-exception for gen.throw #17658

Merged
merged 8 commits into from Aug 3, 2021

Conversation

coolreader18
Copy link
Contributor

@coolreader18 coolreader18 commented Dec 19, 2019

I added a type check in _PyErr_CreateException as @serhiy-storchaka suggested.

I'm not sure if I should replace the check in do_raise in ceval.c with a call to _PyErr_CreateException somehow, as it's effectively the same functionality?

https://bugs.python.org/issue39091

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@coolreader18

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@coolreader18 coolreader18 changed the title bpo-39091: Fix segfault when Exception constructor returns non-exception for gen.throw (and probably some other places) bpo-39091: Fix segfault when Exception constructor returns non-exception for gen.throw Dec 19, 2019
Python/errors.c Outdated Show resolved Hide resolved
Python/errors.c Outdated Show resolved Hide resolved
Python/errors.c Outdated Show resolved Hide resolved
@serhiy-storchaka
Copy link
Member

do_raise has a complex code. Unless it contains similar bug I suggest to left refactoring to other issue.

@coolreader18
Copy link
Contributor Author

Any updates on this?

Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

@serhiy-storchaka @vstinner
This PR is waiting for review for a long time.
Can you please take a look?

@codecov
Copy link

codecov bot commented Feb 16, 2020

Codecov Report

Merging #17658 into master will increase coverage by 1.08%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #17658       +/-   ##
===========================================
+ Coverage   82.11%   83.20%    +1.08%     
===========================================
  Files        1956     1571      -385     
  Lines      589364   414752   -174612     
  Branches    44457    44457               
===========================================
- Hits       483963   345084   -138879     
+ Misses      95748    60021    -35727     
+ Partials     9653     9647        -6     
Impacted Files Coverage Δ
Lib/distutils/tests/test_bdist_rpm.py 30.00% <0.00%> (-65.00%) ⬇️
Lib/distutils/command/bdist_rpm.py 7.63% <0.00%> (-56.88%) ⬇️
Lib/test/test_urllib2net.py 76.92% <0.00%> (-13.85%) ⬇️
Lib/test/test_smtpnet.py 78.57% <0.00%> (-7.15%) ⬇️
Lib/ftplib.py 63.85% <0.00%> (-6.06%) ⬇️
Lib/test/test_ftplib.py 87.11% <0.00%> (-4.72%) ⬇️
Tools/scripts/db2pickle.py 17.82% <0.00%> (-3.97%) ⬇️
Tools/scripts/pickle2db.py 16.98% <0.00%> (-3.78%) ⬇️
Lib/test/test_socket.py 71.94% <0.00%> (-3.77%) ⬇️
Lib/test/test_asyncio/test_base_events.py 91.84% <0.00%> (-3.30%) ⬇️
... and 434 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c33bdbb...95254b4. Read the comment docs.

@bl-ue
Copy link

bl-ue commented Jun 17, 2021

ping @serhiy-storchaka @vstinner this PR needs to be merged.

@benjaminp benjaminp added needs backport to 3.8 only security fixes needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels Aug 3, 2021
@benjaminp benjaminp merged commit 83ca46b into python:main Aug 3, 2021
@bedevere-bot
Copy link

@benjaminp: Please replace # with GH- in the commit message next time. Thanks!

@miss-islington
Copy link
Contributor

Thanks @coolreader18 for the PR, and @benjaminp for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.8, 3.9.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-27572 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed needs backport to 3.10 only security fixes needs backport to 3.9 only security fixes labels Aug 3, 2021
@bedevere-bot
Copy link

GH-27573 is a backport of this pull request to the 3.9 branch.

@miss-islington
Copy link
Contributor

Sorry, @coolreader18 and @benjaminp, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 83ca46b7784b7357d82ec47b33295e09ed7380cb 3.8

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 3, 2021
…-exception for gen.throw. (pythonGH-17658)

Co-authored-by: Benjamin Peterson <benjamin@python.org>
(cherry picked from commit 83ca46b)

Co-authored-by: Noah <33094578+coolreader18@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 3, 2021
…-exception for gen.throw. (pythonGH-17658)

Co-authored-by: Benjamin Peterson <benjamin@python.org>
(cherry picked from commit 83ca46b)

Co-authored-by: Noah <33094578+coolreader18@users.noreply.github.com>
@benjaminp benjaminp removed the needs backport to 3.8 only security fixes label Aug 3, 2021
ambv pushed a commit that referenced this pull request Aug 3, 2021
…ion for gen.throw. (GH-17658) (GH-27572)

Co-authored-by: Benjamin Peterson <benjamin@python.org>
(cherry picked from commit 83ca46b)

Co-authored-by: Noah <33094578+coolreader18@users.noreply.github.com>
ambv pushed a commit that referenced this pull request Aug 3, 2021
…ion for gen.throw. (GH-17658) (GH-27573)

Co-authored-by: Benjamin Peterson <benjamin@python.org>
(cherry picked from commit 83ca46b)

Co-authored-by: Noah <33094578+coolreader18@users.noreply.github.com>
@vstinner
Copy link
Member

vstinner commented Aug 6, 2021

Thanks for the fix!

@nocturn9x
Copy link

Took 2 years but it's finally getting fixed! 😂

@vstinner
Copy link
Member

Better late than never.

@coolreader18 coolreader18 deleted the gen-throw-bpo-39091 branch August 31, 2021 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet