8000 gh-101037: Fix potential memory underallocation for zeros of int subtypes by mdickinson · Pull Request #101038 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-101037: Fix potential memory underallocation for zeros of int subtypes #101038

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.

Sign up for GitHub

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
Merged
merged 2 commits into from
Jan 21, 2023
Merged

gh-101037: Fix potential memory underallocation for zeros of int subtypes #101038

merged 2 commits into from
Jan 21, 2023

Conversation

mdickinson
Copy link
Member
@mdickinson mdickinson commented Jan 14, 2023 •
edited by bedevere-bot
Loading

This PR fixes object allocation in long_subtype_new to ensure that there's at least one digit in all cases, and makes sure that the value of that digit is copied over from the source long.

Needs backport to 3.11, but not any further: the change to require at least one digit was only introduced for Python 3.11.

Fixes #101037.

@mdickinson mdickinson added the needs backport to 3.11 only security fixes label Jan 14, 2023
@bedevere-bot bedevere-bot mentioned this pull request Jan 14, 2023
Closed
@mdickinson
Copy link
Member Author

@corona10 Would you be willing to review?

@mdickinson mdickinson requested a review from corona10 January 14, 2023 17:06
@mdickinson mdickinson mentioned this pull request Jan 14, 2023
Closed
@mdickinson
Copy link
Member Author

and makes sure that the value of that digit is copied over from the source long.

Just a comment: this seems fairly harmless in this case, but I'd note that we don't in general do anything special to make sure that ob_digit[0] contains anything meaningful in the case of zero being represented. (E.g., _PyLong_Copy won't copy over the zero digit.) I've added a note to longintrepr.h in this PR to clarify this.

@mdickinson mdickinson added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 14, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @mdickinson for commit 2ddd5c1 🤖

If you want to schedule another build, you need to add the :hammer: test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 14, 2023
@corona10
Copy link
Member
corona10 commented Jan 15, 2023 •
edited
Loading

@mdickinson I will take a look. Thanks

@corona10
Copy link
Member

For refleak:


➜  cpython git:(pr/101038) ✗ ./python.exe -m test test_capi.test_misc -R 3:3
Raised RLIMIT_NOFILE: 256 -> 1024
0:00:00 load avg: 1.76 Run tests sequentially
0:00:00 load avg: 1.76 [1/1] test_capi.test_misc
beginning 6 repetitions
123456
......
test_capi.test_misc leaked [200, 200, 200] references, sum=600
test_capi.test_misc failed (reference leak)

== Tests result: FAILURE ==

1 test failed:
    test_capi.test_misc

Total duration: 21.3 sec
Tests result: FAILURE

@corona10 corona10 mentioned this pull request Jan 15, 2023
Merged
corona10
corona10 approved these changes Jan 15, 2023
View reviewed changes
Copy link
Member
@corona10 corona10 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM

The refleak is unrelated to this PR :)
see: #100998 (comment)

@corona10 corona10 force-pushed the fix-int-subtype-underallocation branch from 2ddd5c1 to 877067c Compare January 19, 2023 13:51
@corona10 corona10 added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Jan 20, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @corona10 for commit 877067c 🤖

If you want to schedule another build, you need to add the :hammer: test-with-refleak-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Jan 20, 2023
@corona10
Copy link
Member

@mdickinson Oops, sorry, I tried to rebase the branch for refleak test, but it messed up the commit history

@mdickinson
Copy link
Member Author

@corona10 No problem; thanks for reviewing!

@mdickinson mdickinson merged commit 401fdf9 into python:main Jan 21, 2023
@miss-islington
Copy link
Contributor

Thanks @mdickinson for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 21, 2023
@mdickinson @miss-islington
pythongh-101037: Fix potential memory underallocation for zeros of in…
c1a9d27 
…t subtypes (pythonGH-101038)

This PR fixes object allocation in long_subtype_new to ensure that there's at least one digit in all cases, and makes sure that the value of that digit is copied over from the source long.

Needs backport to 3.11, but not any further: the change to require at least one digit was only introduced for Python 3.11.

Fixes pythonGH-101037.
(cherry picked from commit 401fdf9)

Co-authored-by: Mark Dickinson <dickinsm@gmail.com>
@bedevere-bot
Copy link

GH-101219 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Jan 21, 2023
@mdickinson mdickinson deleted the fix-int-subtype-underallocation branch January 21, 2023 10:24
mdickinson added a commit that referenced this pull request Jan 21, 2023
@miss-islington @mdickinson
[3.11] gh-101037: Fix potential memory underallocation for zeros of i…
d2aaf81 
…nt subtypes (GH-101038) (#101219)

gh-101037: Fix potential memory underallocation for zeros of int subtypes (GH-101038)

This PR fixes object allocation in long_subtype_new to ensure that there's at least one digit in all cases, and makes sure that the value of that digit is copied over from the source long.

Needs backport to 3.11, but not any further: the change to require at least one digit was only introduced for Python 3.11.

Fixes GH-101037.
(cherry picked from commit 401fdf9)

Co-authored-by: Mark Dickinson <dickinsm@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@corona10 corona10 corona10 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

long_subtype_new underallocates for zero
4 participants
@mdickinson @bedevere-bot @corona10 @miss-islington
0