gh-110481: Fix Py_SET_REFCNT() integer overflow

[vstinner]

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){~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~ 

• Issue: Implement biased reference counting in --disable-gil builds #110481

[vstinner]

@colesbury@eduardo-elizondo@corona10: Would you mind to review these changes?

[eduardo-elizondo]

@vstinner I can do a thorough review this Saturday if no one gets to it before then!

[colesbury]

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?

[vstinner]

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().

[vstinner]

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.

[vstinner]

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()).

[vstinner]

@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 :-)

[vstinner]

I prepared the documentation change of this PR by adding the "immortal" term to the documentation glossary: #112180

[bedevere-bot]

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Ubuntu NoGIL 3.x has failed when building commit 5f6ac2d.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. 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.
4. Check if the failure is related to this commit (5f6ac2d) or if it is a false positive.
5. 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 1make: *** Waiting for unfinished jobs....make: *** [Makefile:2767: Parser/lexer/buffer.o] Error 1make: *** [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 1make: *** [Makefile:2767: Parser/token.o] Error 1make: *** [Makefile:2767: Parser/action_helpers.o] Error 1make: *** [Makefile:2767: Parser/string_parser.o] Error 1make: *** [Makefile:2767: Parser/peg_api.o] Error 1make: *** [Makefile:2767: Parser/lexer/lexer.o] Error 1make: *** [Makefile:2767: Parser/parser.o] Error 1find: ‘build’: No such file or directoryfind: ‘build’: No such file or directoryfind: ‘build’: No such file or directoryfind: ‘build’: No such file or directorymake: [Makefile:2853: clean-retain-profile] Error 1 (ignored)

[vstinner]

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.

[bedevere-bot]

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot x86-64 MacOS Intel NoGIL 3.x has failed when building commit 5f6ac2d.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. 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.
4. Check if the failure is related to this commit (5f6ac2d) or if it is a false positive.
5. 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 1make: *** Waiting for unfinished jobs.... 3 errors generated. 3 errors generated. 3 errors generated. make: *** [Parser/lexer/buffer.o] Error 1make: *** [Parser/token.o] Error 1make: *** [Parser/string_parser.o] Error 1 3 errors generated. 3 errors generated. make: *** [Parser/peg_api.o] Error 1make: *** [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 1make: *** [Parser/action_helpers.o] Error 1 3 errors generated. make: *** [Parser/parser.o] Error 1find: build: No such file or directoryfind: build: No such file or directoryfind: build: No such file or directoryfind: build: No such file or directorymake: [clean-retain-profile] Error 1 (ignored)