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-36027: Fix a potential refcount bug in long_pow #26690

Merged

Conversation

mdickinson
Copy link
Member

@mdickinson mdickinson commented Jun 12, 2021

This PR fixes a reference counting bug introduced in #13266 and identified by @tim-one in #26662: if we don't set temp to NULL, and if the value it has at this point is retained until the end of the function, then we end up doing an extra decref. I spent some time trying to identify a test-case that exercises this bug, and failed - I couldn't find a code path where temp isn't re-used at some point. Nevertheless, it's a potential point of fragility for maintenance of this code.

https://bugs.python.org/issue36027

@mdickinson mdickinson added skip news 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 Jun 12, 2021
@mdickinson mdickinson added type-bug An unexpected behavior, bug, or error and removed needs backport to 3.8 only security fixes labels Jun 12, 2021
@mdickinson
Copy link
Member Author

3.8 is in security-fix-only mode; adjusting the backport labels.

@tim-one
Copy link
Member

tim-one commented Jun 12, 2021

Yes, this showed up for me because I introduced a new path that, for exponent 1 and no modulus, invoked REDUCE but not MULT. Without a modulus, REDUCE didn't overwrite temp, and without MULT nothing else did either. Nasty 😉.

Copy link
Member

@tim-one tim-one left a comment

Choose a reason for hiding this comment

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

Ship it!

@mdickinson mdickinson merged commit 5924243 into python:main Jun 13, 2021
@miss-islington
Copy link
Contributor

Thanks @mdickinson for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.9.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington
Copy link
Contributor

Sorry @mdickinson, I had trouble checking out the 3.10 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 59242431991794064824cf2ab70886367613f29e 3.10

@bedevere-bot
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 13, 2021
(cherry picked from commit 5924243)

Co-authored-by: Mark Dickinson <mdickinson@enthought.com>
mdickinson added a commit to mdickinson/cpython that referenced this pull request Jun 13, 2021
…6690)

(cherry picked from commit 5924243)

Co-authored-by: Mark Dickinson <mdickinson@enthought.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jun 13, 2021
@bedevere-bot
Copy link

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

mdickinson added a commit that referenced this pull request Jun 13, 2021
(cherry picked from commit 5924243)

Co-authored-by: Mark Dickinson <mdickinson@enthought.com>

Co-authored-by: Mark Dickinson <mdickinson@enthought.com>
@mdickinson mdickinson deleted the fix/potential-refcount-error-in-long-pow branch June 13, 2021 07:46
mdickinson added a commit that referenced this pull request Jun 13, 2021
…H-26703)

(cherry picked from commit 5924243)

Co-authored-by: Mark Dickinson <mdickinson@enthought.com>
jdevries3133 pushed a commit to jdevries3133/cpython that referenced this pull request Jun 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants