GeezerVison is showing github.com — gh-110481: Fix Py_SET_REFCNT() integer overflow by vstinner · Pull Request #112174 · python/cpython
Conversation
vstinner
commented
Nov 16, 2023
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
If Py_NOGIL is defined and Py_SET_REFCNT() is called with a reference count larger than UINT32_MAX, make the object immortal.
Set _Py_IMMORTAL_REFCNT constant type to Py_ssize_t to fix the following compiler warning:
Include/internal/pycore_global_objects_fini_generated.h:14:24: warning: comparison of integers of different signs: 'Py_ssize_t' (aka 'long') and 'unsigned int' [-Wsign-compare]
if (Py_REFCNT(obj) < _Py_IMMORTAL_REFCNT) {
~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
@vstinner I can do a thorough review this Saturday if no one gets to it before then!
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The biased reference counting changes lgtm.
This adds some C-style casts. What's the "rule" for using C-style casts vs. _Py_STATIC_CAST/_Py_CAST in this file?
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Oh, I forgot about _Py_CAST(). i introduced it to avoid C++ compiler warnings with the strict "no old C cast" warning. Sadly, my complicated C++ _Py_CAST() implementation had to be reverted. There are now some basic C++ tests in test_cppext, but it doesn't catch forgotten _Py_CAST().
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This adds some C-style casts. What's the "rule" for using C-style casts vs. _Py_STATIC_CAST/_Py_CAST in this file?
I added _Py_CAST() calls.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
I modified the PR that Py_SET_REFCNT() makes an object immortal if refcnt is larger than UINT32_MAX. PR based on my previous PR which adds "immortal" to the documentation glossary: #112180
I also added comments in Py_INCREF() to describe the behavior when the reference count in UINT32_MAX in Py_INCREF() (not in the 3rd code path which checks explicitly for _Py_IsImmortal()).
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
@eduardo-elizondo:
@vstinner I can do a thorough review this Saturday if no one gets to it before then!
Sure, I will wait for your review :-)
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
If Py_NOGIL is defined and Py_SET_REFCNT() is called with a reference
count larger than UINT32_MAX, make the object immortal.
Set _Py_IMMORTAL_REFCNT constant type to Py_ssize_t to fix the
following compiler warning:
Include/internal/pycore_global_objects_fini_generated.h:14:24:
warning: comparison of integers of different signs: 'Py_ssize_t'
(aka 'long') and 'unsigned int' [-Wsign-compare]
if (Py_REFCNT(obj) < _Py_IMMORTAL_REFCNT) {
~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~
|
I prepared the documentation change of this PR by adding the "immortal" term to the documentation glossary: #112180
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️
Hi! The buildbot AMD64 Ubuntu NoGIL 3.x has failed when building commit 5f6ac2d.
What do you need to do:
- Don't panic.
- Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
- Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/1225/builds/780) and take a look at the build logs.
- Check if the failure is related to this commit (5f6ac2d) or if it is a false positive.
- If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.
You can take a look at the buildbot page here:
https://buildbot.python.org/all/#builders/1225/builds/780
Summary of the results of the build (if available):
Click to see traceback logs
remote: Enumerating objects: 9, done.
remote: Counting objects: 16% (1/6)
remote: Counting objects: 33% (2/6)
remote: Counting objects: 50% (3/6)
remote: Counting objects: 66% (4/6)
remote: Counting objects: 83% (5/6)
remote: Counting objects: 100% (6/6)
remote: Counting objects: 100% (6/6), done.
remote: Compressing objects: 25% (1/4)
remote: Compressing objects: 50% (2/4)
remote: Compressing objects: 75% (3/4)
remote: Compressing objects: 100% (4/4)
remote: Compressing objects: 100% (4/4), done.
remote: Total 9 (delta 2), reused 2 (delta 2), pack-reused 3
From https://github.com/python/cpython
* branch main -> FETCH_HEAD
Note: switching to '5f6ac2d88a49b8a7c764691365cd41ee6226a8d0'.
You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.
If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:
git switch -c <new-branch-name>
Or undo this operation with:
git switch -
Turn off this advice by setting config variable advice.detachedHead to false
HEAD is now at 5f6ac2d88a gh-110481: Fix Py_SET_REFCNT() integer overflow (#112174)
Switched to and reset branch 'main'
In file included from ./Include/Python.h:58,
from ./Programs/python.c:3:
./Include/object.h: In function ‘Py_SET_REFCNT’:
In file included from ./Include/Python.h:58,
from Parser/pegen.c:1:
./Include/object.h: In function ‘Py_SET_REFCNT’:
In file included from ./Include/Python.h:58,
from Parser/pegen_errors.c:1:
./Include/object.h: In function ‘Py_SET_REFCNT’:
./Include/object.h:360:13: error: ‘op’ undeclared (first use in this function); did you mean ‘ob’?
360 | op->ob_tid = _Py_UNOWNED_TID;
| ^~
| ob
./Include/object.h:360:13: error: ‘op’ undeclared (first use in this function); did you mean ‘ob’?
360 | op->ob_tid = _Py_UNOWNED_TID;
| ^~
| ob
./Include/object.h:360:13: note: each undeclared identifier is reported only once for each function it appears in
./Include/object.h:360:13: note: each undeclared identifier is reported only once for each function it appears in
./Include/object.h:360:13: error: ‘op’ undeclared (first use in this function); did you mean ‘ob’?
360 | op->ob_tid = _Py_UNOWNED_TID;
| ^~
| ob
./Include/object.h:360:13: note: each undeclared identifier is reported only once for each function it appears in
In file included from ./Include/Python.h:58,
from Parser/pegen.h:4,
from Parser/parser.c:2:
./Include/object.h: In function ‘Py_SET_REFCNT’:
./Include/object.h:360:13: error: ‘op’ undeclared (first use in this function); did you mean ‘ob’?
360 | op->ob_tid = _Py_UNOWNED_TID;
| ^~
| ob
./Include/object.h:360:13: note: each undeclared identifier is reported only once for each function it appears in
In file included from ./Include/Python.h:58,
from Parser/action_helpers.c:1:
./Include/object.h: In function ‘Py_SET_REFCNT’:
./Include/object.h:360:13: error: ‘op’ undeclared (first use in this function); did you mean ‘ob’?
360 | op->ob_tid = _Py_UNOWNED_TID;
| ^~
| ob
./Include/object.h:360:13: note: each undeclared identifier is reported only once for each function it appears in
In file included from ./Include/Python.h:58,
from Parser/lexer/buffer.c:1:
./Include/object.h: In function ‘Py_SET_REFCNT’:
./Include/object.h:360:13: error: ‘op’ undeclared (first use in this function); did you mean ‘ob’?
360 | op->ob_tid = _Py_UNOWNED_TID;
| ^~
| ob
./Include/object.h:360:13: note: each undeclared identifier is reported only once for each function it appears in
make: *** [Makefile:1541: Programs/python.o] Error 1
make: *** Waiting for unfinished jobs....
make: *** [Makefile:2767: Parser/lexer/buffer.o] Error 1
make: *** [Makefile:2767: Parser/pegen_errors.o] Error 1
In file included from ./Include/Python.h:58,
from Parser/string_parser.c:3:
./Include/object.h: In function ‘Py_SET_REFCNT’:
./Include/object.h:360:13: error: ‘op’ undeclared (first use in this function); did you mean ‘ob’?
360 | op->ob_tid = _Py_UNOWNED_TID;
| ^~
| ob
./Include/object.h:360:13: note: each undeclared identifier is reported only once for each function it appears in
In file included from ./Include/Python.h:58,
from Parser/token.c:3:
./Include/object.h: In function ‘Py_SET_REFCNT’:
./Include/object.h:360:13: error: ‘op’ undeclared (first use in this function); did you mean ‘ob’?
360 | op->ob_tid = _Py_UNOWNED_TID;
| ^~
| ob
./Include/object.h:360:13: note: each undeclared identifier is reported only once for each function it appears in
In file included from ./Include/Python.h:58,
from Parser/peg_api.c:1:
./Include/object.h: In function ‘Py_SET_REFCNT’:
./Include/object.h:360:13: error: ‘op’ undeclared (first use in this function); did you mean ‘ob’?
360 | op->ob_tid = _Py_UNOWNED_TID;
| ^~
| ob
./Include/object.h:360:13: note: each undeclared identifier is reported only once for each function it appears in
In file included from ./Include/Python.h:58,
from Parser/lexer/lexer.c:1:
./Include/object.h: In function ‘Py_SET_REFCNT’:
./Include/object.h:360:13: error: ‘op’ undeclared (first use in this function); did you mean ‘ob’?
360 | op->ob_tid = _Py_UNOWNED_TID;
| ^~
| ob
./Include/object.h:360:13: note: each undeclared identifier is reported only once for each function it appears in
make: *** [Makefile:2767: Parser/pegen.o] Error 1
make: *** [Makefile:2767: Parser/token.o] Error 1
make: *** [Makefile:2767: Parser/action_helpers.o] Error 1
make: *** [Makefile:2767: Parser/string_parser.o] Error 1
make: *** [Makefile:2767: Parser/peg_api.o] Error 1
make: *** [Makefile:2767: Parser/lexer/lexer.o] Error 1
make: *** [Makefile:2767: Parser/parser.o] Error 1
find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
make: [Makefile:2853: clean-retain-profile] Error 1 (ignored)
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Oh, I missed an obvious typo :-( Sadly, the NoGIL jobs didn't run in the pre-commit CI. Maybe because I created the PR 2 weeks ago?
Anyway, PR #112595 should fix the buildbot.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️
Hi! The buildbot x86-64 MacOS Intel NoGIL 3.x has failed when building commit 5f6ac2d.
What do you need to do:
- Don't panic.
- Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
- Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/1258/builds/292) and take a look at the build logs.
- Check if the failure is related to this commit (5f6ac2d) or if it is a false positive.
- If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.
You can take a look at the buildbot page here:
https://buildbot.python.org/all/#builders/1258/builds/292
Summary of the results of the build (if available):
Click to see traceback logs
remote: Enumerating objects: 18, done.
remote: Counting objects: 7% (1/14)
remote: Counting objects: 14% (2/14)
remote: Counting objects: 21% (3/14)
remote: Counting objects: 28% (4/14)
remote: Counting objects: 35% (5/14)
remote: Counting objects: 42% (6/14)
remote: Counting objects: 50% (7/14)
remote: Counting objects: 57% (8/14)
remote: Counting objects: 64% (9/14)
remote: Counting objects: 71% (10/14)
remote: Counting objects: 78% (11/14)
remote: Counting objects: 85% (12/14)
remote: Counting objects: 92% (13/14)
remote: Counting objects: 100% (14/14)
remote: Counting objects: 100% (14/14), done.
remote: Compressing objects: 9% (1/11)
remote: Compressing objects: 18% (2/11)
remote: Compressing objects: 27% (3/11)
remote: Compressing objects: 36% (4/11)
remote: Compressing objects: 45% (5/11)
remote: Compressing objects: 54% (6/11)
remote: Compressing objects: 63% (7/11)
remote: Compressing objects: 72% (8/11)
remote: Compressing objects: 81% (9/11)
remote: Compressing objects: 90% (10/11)
remote: Compressing objects: 100% (11/11)
remote: Compressing objects: 100% (11/11), done.
remote: Total 18 (delta 4), reused 7 (delta 3), pack-reused 4
From https://github.com/python/cpython
* branch main -> FETCH_HEAD
Note: switching to '5f6ac2d88a49b8a7c764691365cd41ee6226a8d0'.
You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.
If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:
git switch -c <new-branch-name>
Or undo this operation with:
git switch -
Turn off this advice by setting config variable advice.detachedHead to false
HEAD is now at 5f6ac2d88a gh-110481: Fix Py_SET_REFCNT() integer overflow (#112174)
Switched to and reset branch 'main'
In file included from Parser/action_helpers.c:1:
In file included from ./Include/Python.h:58:
./Include/object.h:360:13: error: use of undeclared identifier 'op'
op->ob_tid = _Py_UNOWNED_TID;
^
./Include/object.h:361:13: error: use of undeclared identifier 'op'
op->ob_ref_local = _Py_IMMORTAL_REFCNT_LOCAL;
^
./Include/object.h:362:13: error: use of undeclared identifier 'op'
op->ob_ref_shared = 0;
^
In file included from Parser/token.c:3:
In file included from ./Include/Python.h:58:
./Include/object.h:360:13: error: use of undeclared identifier 'op'
op->ob_tid = _Py_UNOWNED_TID;
^
./Include/object.h:361:13: error: use of undeclared identifier 'op'
op->ob_ref_local = _Py_IMMORTAL_REFCNT_LOCAL;
^
./Include/object.h:362:13: error: use of undeclared identifier 'op'
op->ob_ref_shared = 0;
^
In file included from Parser/lexer/buffer.c:1:
In file included from ./Include/Python.h:58:
./Include/object.h:360:13: error: use of undeclared identifier 'op'
op->ob_tid = _Py_UNOWNED_TID;
^
./Include/object.h:361:13: error: use of undeclared identifier 'op'
op->ob_ref_local = _Py_IMMORTAL_REFCNT_LOCAL;
^
In file included from Parser/pegen.c:1:
In file included from ./Include/Python.h:58:
./Include/object.h:360:13: ./Include/object.h:error: use of undeclared identifier 'op'
362:13: error: use of undeclared identifier 'op'
op->ob_tid = _Py_UNOWNED_TID; op->ob_ref_shared = 0;
^ ^
./Include/object.h:361:13: error: use of undeclared identifier 'op'
op->ob_ref_local = _Py_IMMORTAL_REFCNT_LOCAL;
^
./Include/object.h:362:13: error: use of undeclared identifier 'op'
op->ob_ref_shared = 0;
^
In file included from Parser/parser.c:2:
In file included from Parser/pegen.h:4:
In file included from ./Include/Python.h:58:
./Include/object.h:360:13: error: use of undeclared identifier 'op'
op->ob_tid = _Py_UNOWNED_TID;
^
In file included from ./Programs/python.c:3:
In file included from ./Include/Python.h:58:
./Include/object.h:360:13: error: use of undeclared identifier 'op'
op->ob_tid = _Py_UNOWNED_TID;
^
./Include/object.h:361:13: error: use of undeclared identifier 'op'
op->ob_ref_local = _Py_IMMORTAL_REFCNT_LOCAL;
^
./Include/object.h:361:13: error: use of undeclared identifier 'op'
op->ob_ref_local = _Py_IMMORTAL_REFCNT_LOCAL;
^
./Include/object.h:362:13: error: use of undeclared identifier 'op'
op->ob_ref_shared = 0;
^
./Include/object.h:362:13: error: use of undeclared identifier 'op'
op->ob_ref_shared = 0;
^
In file included from Parser/string_parser.c:3:
In file included from ./Include/Python.h:58:
./Include/object.h:360:13: error: use of undeclared identifier 'op'
op->ob_tid = _Py_UNOWNED_TID;
^
./Include/object.h:361:13: error: use of undeclared identifier 'op'
op->ob_ref_local = _Py_IMMORTAL_REFCNT_LOCAL;
^
./Include/object.h:362:13: error: use of undeclared identifier 'op'
op->ob_ref_shared = 0;
^
In file included from Parser/peg_api.c:1:
In file included from ./Include/Python.h:58:
./Include/object.h:360:13: error: use of undeclared identifier 'op'
op->ob_tid = _Py_UNOWNED_TID;
^
./Include/object.h:361:13: error: use of undeclared identifier 'op'
op->ob_ref_local = _Py_IMMORTAL_REFCNT_LOCAL;
^
./Include/object.h:362:13: error: use of undeclared identifier 'op'
op->ob_ref_shared = 0;
^
In file included from Parser/pegen_errors.c:1:
In file included from ./Include/Python.h:58:
./Include/object.h:360:13: error: use of undeclared identifier 'op'
op->ob_tid = _Py_UNOWNED_TID;
^
./Include/object.h:361:13: error: use of undeclared identifier 'op'
op->ob_ref_local = _Py_IMMORTAL_REFCNT_LOCAL;
^
./Include/object.h:362:13: error: use of undeclared identifier 'op'
op->ob_ref_shared = 0;
^
In file included from Parser/lexer/lexer.c:1:
In file included from ./Include/Python.h:58:
./Include/object.h:360:13: error: use of undeclared identifier 'op'
op->ob_tid = _Py_UNOWNED_TID;
^
./Include/object.h:361:13: error: use of undeclared identifier 'op'
op->ob_ref_local = _Py_IMMORTAL_REFCNT_LOCAL;
^
./Include/object.h:362:13: error: use of undeclared identifier 'op'
op->ob_ref_shared = 0;
^
3 errors generated.
make: *** [Programs/python.o] Error 1
make: *** Waiting for unfinished jobs....
3 errors generated.
3 errors generated.
3 errors generated.
make: *** [Parser/lexer/buffer.o] Error 1
make: *** [Parser/token.o] Error 1
make: *** [Parser/string_parser.o] Error 1
3 errors generated.
3 errors generated.
make: *** [Parser/peg_api.o] Error 1
make: *** [Parser/pegen_errors.o] Error 1
3 errors generated.
make: *** [Parser/lexer/lexer.o] Error 1
3 errors generated.
3 errors generated.
make: *** [Parser/pegen.o] Error 1
make: *** [Parser/action_helpers.o] Error 1
3 errors generated.
make: *** [Parser/parser.o] Error 1
find: build: No such file or directory
find: build: No such file or directory
find: build: No such file or directory
find: build: No such file or directory
make: [clean-retain-profile] Error 1 (ignored)
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
aisk
pushed a commit
to aisk/cpython
that referenced
this pull request
Feb 11, 2024
If Py_NOGIL is defined and Py_SET_REFCNT() is called with a reference
count larger than UINT32_MAX, make the object immortal.
Set _Py_IMMORTAL_REFCNT constant type to Py_ssize_t to fix the
following compiler warning:
Include/internal/pycore_global_objects_fini_generated.h:14:24:
warning: comparison of integers of different signs: 'Py_ssize_t'
(aka 'long') and 'unsigned int' [-Wsign-compare]
if (Py_REFCNT(obj) < _Py_IMMORTAL_REFCNT) {
~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~
Glyphack
pushed a commit
to Glyphack/cpython
that referenced
this pull request
Sep 2, 2024
If Py_NOGIL is defined and Py_SET_REFCNT() is called with a reference
count larger than UINT32_MAX, make the object immortal.
Set _Py_IMMORTAL_REFCNT constant type to Py_ssize_t to fix the
following compiler warning:
Include/internal/pycore_global_objects_fini_generated.h:14:24:
warning: comparison of integers of different signs: 'Py_ssize_t'
(aka 'long') and 'unsigned int' [-Wsign-compare]
if (Py_REFCNT(obj) < _Py_IMMORTAL_REFCNT) {
~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|
colesbury
left a comment