Skip to content

Conversation

@mdickinson
Copy link
Member

@mdickinsonmdickinson commented Jan 14, 2023

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
Copy link
MemberAuthor

@corona10 Would you be willing to review?

@mdickinson
Copy link
MemberAuthor

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.

@mdickinsonmdickinson 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-botbedevere-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

@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 

Copy link
Member

@corona10corona10 left a comment

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)

@corona10corona10force-pushed the fix-int-subtype-underallocation branch from 2ddd5c1 to 877067cCompareJanuary 19, 2023 13:51
@corona10corona10 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-botbedevere-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
MemberAuthor

@corona10 No problem; thanks for reviewing!

@mdickinsonmdickinson merged commit 401fdf9 into python:mainJan 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
…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. FixespythonGH-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-botbedevere-bot removed the needs backport to 3.11 only security fixes label Jan 21, 2023
@mdickinsonmdickinson deleted the fix-int-subtype-underallocation branch January 21, 2023 10:24
mdickinson added a commit that referenced this pull request Jan 21, 2023
…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. FixesGH-101037. (cherry picked from commit 401fdf9) Co-authored-by: Mark Dickinson <dickinsm@gmail.com>
Sign up for freeto 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.

long_subtype_new underallocates for zero

4 participants

@mdickinson@bedevere-bot@corona10@miss-islington