gh-110481: Fix Py_SET_REFCNT() integer overflow by vstinner · Pull Request #112174 · python/cpython · GitHub

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

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

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

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?

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

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.

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

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

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

⚠️⚠️⚠️ 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)

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.

⚠️⚠️⚠️ 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)

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

vstinner added the skip news label Nov 16, 2023

bedevere-app Bot added the awaiting core review label Nov 16, 2023

bedevere-app Bot mentioned this pull request Nov 16, 2023

Implement biased reference counting in --disable-gil builds #110481

Closed

vstinner force-pushed the signed_refcnt branch from 1d1bc93 to 7d64093 Compare November 16, 2023 18:33

colesbury reviewed Nov 16, 2023

View reviewed changes

vstinner force-pushed the signed_refcnt branch from 4a34e88 to 56fc4e1 Compare November 17, 2023 14:16

vstinner requested review from corona10 and erlend-aasland as code owners November 17, 2023 14:16

Eclips4 reviewed Nov 18, 2023

View reviewed changes

Comment thread Doc/using/configure.rst Outdated

vstinner added 6 commits November 27, 2023 12:17

Use _Py_CAST()

3a53d9b

Document the behavior

53df2c1

Add comment about _Py_IMMORTAL_REFCNT in Py_INCREF()

3bb41e5

fix sphinx syntax in the doc

11f91f3

fix typo in the doc link

d873b98

vstinner force-pushed the signed_refcnt branch from fc4349e to d873b98 Compare November 27, 2023 11:18

vstinner merged commit 5f6ac2d into python:main Dec 1, 2023

vstinner deleted the signed_refcnt branch December 1, 2023 14:50

bedevere-app Bot removed the awaiting core review label Dec 1, 2023

vstinner mentioned this pull request Jul 13, 2024

[3.13] _PyObject_Init() assertion fails with the stable ABI on Python built with assertions (extension built with Python 3.11) #121528

Closed

gh-110481: Fix Py_SET_REFCNT() integer overflow by vstinner · Pull Request #112174 · python/cpython · GitHub

Repository navigation

gh-110481: Fix Py_SET_REFCNT() integer overflow - #112174

gh-110481: Fix Py_SET_REFCNT() integer overflow#112174
vstinner merged 6 commits into
python:mainfrom
vstinner:signed_refcnt

vstinner commented Nov 16, 2023 •
edited
Loading

Uh oh!

Uh oh!

vstinner commented Nov 16, 2023

Uh oh!

eduardo-elizondo commented Nov 16, 2023

Uh oh!

colesbury left a comment

Uh oh!

vstinner commented Nov 16, 2023

Uh oh!

vstinner commented Nov 16, 2023

Uh oh!

vstinner commented Nov 17, 2023

Uh oh!

vstinner commented Nov 17, 2023

Uh oh!

Uh oh!

vstinner commented Nov 27, 2023

Uh oh!

bedevere-bot commented Dec 1, 2023

Uh oh!

vstinner commented Dec 1, 2023

Uh oh!

bedevere-bot commented Dec 1, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gh-110481: Fix Py_SET_REFCNT() integer overflow by vstinner · Pull Request #112174 · python/cpython · GitHub

Repository navigation

gh-110481: Fix Py_SET_REFCNT() integer overflow - #112174

gh-110481: Fix Py_SET_REFCNT() integer overflow#112174vstinner merged 6 commits intopython:mainpython/cpython:mainfrom vstinner:signed_refcntvstinner/cpython:signed_refcntCopy head branch name to clipboard

Conversation

vstinner commented Nov 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner commented Nov 16, 2023

Uh oh!

eduardo-elizondo commented Nov 16, 2023

Uh oh!

colesbury left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner commented Nov 16, 2023

Uh oh!

vstinner commented Nov 16, 2023

Uh oh!

vstinner commented Nov 17, 2023

Uh oh!

vstinner commented Nov 17, 2023

Uh oh!

Uh oh!

vstinner commented Nov 27, 2023

Uh oh!

bedevere-bot commented Dec 1, 2023

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

Uh oh!

vstinner commented Dec 1, 2023

Uh oh!

bedevere-bot commented Dec 1, 2023

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

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gh-110481: Fix Py_SET_REFCNT() integer overflow#112174
vstinner merged 6 commits into
python:mainfrom
vstinner:signed_refcnt

vstinner commented Nov 16, 2023 •
edited
Loading