gh-143050: correct PyLong_FromString() to use _PyLong_Negate() by skirpichev · Pull Request #145901 · python/cpython

skirpichev · 2026-03-13T07:53:50Z

The long_from_string_base() might return a small integer, when the _pylong.py is used to do conversion. Hence, we must be careful here to not smash it "small int" bit by using the _PyLong_FlipSign().

Co-authored-by: Sam Gross colesbury@gmail.com

Issue: Remove redundant decref from _PyLong_Negate #143050

The long_from_string_base() might return a small integer, when the _pylong.py is used to do conversion. Hence, we must be careful here to not smash it "small int" bit by using the _PyLong_FlipSign().

skirpichev · 2026-03-13T07:55:06Z

Shouldn't affect users, so - no news.

CC @colesbury, @eendebakpt

skirpichev · 2026-03-13T10:11:02Z

_long_is_small_int() logic used also in Modules/_testcapi/immortal.c. What about moving this inlined function to the Include/internal/pycore_long.h as _PyLong_IsImmortal()?

skirpichev · 2026-03-13T10:19:57Z

Lib/test/test_capi/test_long.py

+        with support.adjust_int_max_str_digits(0):
+            a = int('-' + '0' * 7000, 10)
+            del a
+            _testcapi.test_immortal_small_ints()


I'm slightly worry that this pass will non-debug builds.

@corona10 (author of #103962), are you sure there is no other way to implement this helper? Return a different value, raise an error?

That's okay as long as it catches the regression in a debug build.

_testcapi is always built with assertions, even in release mode.

I built Python in release mode without PyLong_FromString() fix. _PyLong_FlipSign() assertion didn't catch the bug since Python was built without assertions. But at least, test_immortal_small_ints() was able to detect that small integers were corrupted:

test_bug_143050 (test.test_capi.test_long.LongTests.test_bug_143050) ... python: ./Modules/_testcapi/immortal.c:35: test_immortal_small_ints: Assertion `has_int_immortal_bit' failed. Fatal Python error: Aborted Current thread 0x00007f617b91b780 [python] (most recent call first): File "/home/vstinner/python/main/Lib/test/test_capi/test_long.py", line 811 in test_bug_143050

Objects/longobject.c

Lib/test/test_capi/test_long.py

eendebakpt

Minor comment on the naming. But the PR fixes a bug, adds a test and cleans up along the way, so good enough for me!

Include/internal/pycore_long.h

colesbury

Looks good to me. Small comment about the function name below.

colesbury · 2026-03-17T20:02:24Z

Lib/test/test_capi/test_long.py

+        with support.adjust_int_max_str_digits(0):
+            a = int('-' + '0' * 7000, 10)
+            del a
+            _testcapi.test_immortal_small_ints()


That's okay as long as it catches the regression in a debug build.

Include/internal/pycore_long.h

colesbury

Let me know if you change the function name or keep it as is.

skirpichev · 2026-03-17T23:32:42Z

@colesbury, I did renaming.

~~I've tried also to add an assert(_Py_IsImmortal(op)), but this fails build like check in the _PyLong_SetSignAndDigitCount (see #145901 (comment)):~~

make: *** [Makefile:1957: Python/frozen_modules/importlib._bootstrap.h] Error 134
./Programs/_freeze_module importlib._bootstrap_external ../cpython-ro-srcdir/Lib/importlib/_bootstrap_external.py Python/frozen_modules/importlib._bootstrap_external.h
_freeze_module: ../cpython-ro-srcdir/Include/internal/pycore_long.h:238: _PyLong_IsImmortal: Assertion `_Py_IsImmortal(op)' failed.
Aborted (core dumped)
make: *** [Makefile:1960: Python/frozen_modules/importlib._bootstrap_external.h] Error 134
Error: Process completed with exit code 2.

~~I believe it's a bug, but lets fix this separately.~~

Include/internal/pycore_long.h

skirpichev · 2026-03-18T06:36:43Z

For failure in #145901 (comment).

I think we can set required asserts (operand: not a small int) for _PyLong_SetDigitCount and _PyLong_SetSignAndDigitCount. In most cases such assumption is true. Few exceptions coming from directly malloc'ed (not coming from the freelist) PyLongObject's: here we need to properly initialize the lv_tag, e.g. set it to 1 (zero). Following patch seems to be working:

diff --git a/Include/internal/pycore_long.h b/Include/internal/pycore_long.h
index 672e53a9427..f53dcbad2fc 100644
--- a/Include/internal/pycore_long.h
+++ b/Include/internal/pycore_long.h
@@ -289,6 +289,7 @@ _PyLong_SetSignAndDigitCount(PyLongObject *op, int sign, Py_ssize_t size)
     assert(size >= 0);
     assert(-1 <= sign && sign <= 1);
     assert(sign != 0 || size == 0);
+    assert(!_PyLong_HasImmortalTag(op));
     op->long_value.lv_tag = TAG_FROM_SIGN_AND_SIZE(sign, size);
 }
 
@@ -296,6 +297,7 @@ static inline void
 _PyLong_SetDigitCount(PyLongObject *op, Py_ssize_t size)
 {
     assert(size >= 0);
+    assert(!_PyLong_HasImmortalTag(op));
     op->long_value.lv_tag = (((size_t)size) << NON_SIZE_BITS) | (op->long_value.lv_tag & SIGN_MASK);
 }
 
diff --git a/Objects/longobject.c b/Objects/longobject.c
index b126efca0ef..73a051c3926 100644
--- a/Objects/longobject.c
+++ b/Objects/longobject.c
@@ -184,6 +184,7 @@ long_alloc(Py_ssize_t size)
             return NULL;
         }
         _PyObject_Init((PyObject*)result, &PyLong_Type);
+        result->long_value.lv_tag = 1;
     }
     _PyLong_SetSignAndDigitCount(result, size != 0, size);
     /* The digit has to be initialized explicitly to avoid
@@ -257,6 +258,7 @@ _PyLong_FromMedium(sdigit x)
             return NULL;
         }
         _PyObject_Init((PyObject*)v, &PyLong_Type);
+        v->long_value.lv_tag = 1;
     }
     digit abs_x = x < 0 ? -x : x;
     _PyLong_SetSignAndDigitCount(v, x<0?-1:1, 1);
@@ -336,6 +338,7 @@ medium_from_stwodigits(stwodigits x)
             return PyStackRef_NULL;
         }
         _PyObject_Init((PyObject*)v, &PyLong_Type);
+        v->long_value.lv_tag = 1;
     }
     digit abs_x = x < 0 ? (digit)(-x) : (digit)x;
     _PyLong_SetSignAndDigitCount(v, x<0?-1:1, 1);

Does it make sense here?

Maybe we need some helper for initialization of the PyLongObject struct, e.g.:

static inline void
_PyLong_Init(PyLongObject *op, Py_ssize_t size)
{
    assert(size >= 1);
    op->long_value.lv_tag = TAG_FROM_SIGN_AND_SIZE(0, size);
    op->long_value.digits[0] = 0;
}

Lib/test/test_capi/test_long.py

Include/internal/pycore_long.h

vstinner · 2026-03-30T12:38:20Z

Include/internal/pycore_long.h


+/* Return true if the argument is a small int */
+static inline bool
+_PyLong_HasImmortalTag(const PyLongObject *op)


IMO the intent here is to check if an integer is a small integer, rather than checking for a tag. So I would prefer to rename the function to _PyLong_IsSmallInt().

I wanted to suggest referring to _PY_IS_SMALL_INT() but I noticed the macro is wrong. I wrote #146631 to fix the macro.

There are two notions for "small int"'s. One - by value and other is - this. If @colesbury is ok - I'm fine with renaming..

_PyLong_HasImmortalTag() and _PY_IS_SMALL_INT() should be true or false for the same values.

For example, you can add the following check for _PyLong_HasImmortalTag():

+ assert((_PyLong_IsCompact(op) && _PY_IS_SMALL_INT(_PyLong_CompactValue(op))) + || (!is_small_int));

(You need to update your branch to retrieve my _PY_IS_SMALL_INT() change.)

For me, testing IMMORTALITY_BIT_MASK is just an implementation detail to check if an integer is "a small int".

I've added an assert. BTW, what do you think on #145901 (comment)?

I don't think we should mention _PY_IS_SMALL_INT in the description of the function, assert is readable enough.

For me, testing IMMORTALITY_BIT_MASK is just an implementation detail to check if an integer is "a small int".

For now, we use _PY_IS_SMALL_INT() to check this. But eventually this could be just testing a flag.

I would prefer to rename the function to _PyLong_IsSmallInt().

I've added suggestions.

Include/internal/pycore_long.h

Modules/_testcapi/immortal.c

Objects/longobject.c

Modules/_testcapi/immortal.c

Objects/longobject.c

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>

vstinner

LGTM

vstinner · 2026-03-31T12:55:57Z

test_opcache crashed on "Tests / Sanitizers / TSan (free-threading)" CI:

0:13:12 load avg: 4.44 [61/77/1] test_opcache worker non-zero exit code (Exit code -11 (SIGSEGV)) -- running (1): test_io.test_textio (34.1 sec)
Fatal Python error: Segmentation fault

<Cannot show all threads while the GIL is disabled>
Stack (most recent call first):
  File "/home/runner/work/cpython/cpython/Lib/test/test_opcache.py", line 1019 in read
  File "/home/runner/work/cpython/cpython/Lib/test/test_opcache.py", line 659 in assert_races_do_not_crash
  File "/home/runner/work/cpython/cpython/Lib/test/test_opcache.py", line 1032 in test_load_attr_property

It looks unrelated to this change. I just re-ran the CI.

vstinner · 2026-03-31T12:58:41Z

To backport this change to 3.14, _PY_IS_SMALL_INT() macro should be fixed first. So I prepared #147187 for that.

skirpichev · 2026-03-31T13:01:38Z

Why we need a backport?

vstinner · 2026-03-31T13:12:11Z

Why we need a backport?

This change uses _PY_IS_SMALL_INT(). I prefer to backport the change as it is, so _PY_IS_SMALL_INT() has to be fixed in 3.14.

miss-islington-app · 2026-03-31T13:31:45Z

Thanks @skirpichev for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

bedevere-app · 2026-03-31T13:31:59Z

GH-147331 is a backport of this pull request to the 3.14 branch.

…ythonGH-145901) The long_from_string_base() might return a small integer, when the _pylong.py is used to do conversion. Hence, we must be careful here to not smash it "small int" bit by using the _PyLong_FlipSign(). (cherry picked from commit db5936c) Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org>

bedevere-app · 2026-03-31T13:55:36Z

GH-147437 is a backport of this pull request to the 3.13 branch.

…GH-145901) (#147331) gh-143050: Correct PyLong_FromString() to use _PyLong_Negate() (GH-145901) The long_from_string_base() might return a small integer, when the _pylong.py is used to do conversion. Hence, we must be careful here to not smash it "small int" bit by using the _PyLong_FlipSign(). (cherry picked from commit db5936c) Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org>

…#145901) (#147437) The long_from_string_base() might return a small integer, when the _pylong.py is used to do conversion. Hence, we must be careful here to not smash it "small int" bit by using the _PyLong_FlipSign(). Co-authored-by: Victor Stinner <vstinner@python.org> (cherry picked from commit db5936c) Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>

pythongh-143050: correct PyLong_FromString() to use _PyLong_Negate()

fbcec5a

The long_from_string_base() might return a small integer, when the _pylong.py is used to do conversion. Hence, we must be careful here to not smash it "small int" bit by using the _PyLong_FlipSign().

skirpichev requested a review from a team as a code owner March 13, 2026 07:53

skirpichev added the skip news label Mar 13, 2026

skirpichev requested review from AA-Turner, emmatyping and erlend-aasland as code owners March 13, 2026 07:53

bedevere-app bot added the awaiting review label Mar 13, 2026

bedevere-app bot mentioned this pull request Mar 13, 2026

Remove redundant decref from _PyLong_Negate #143050

Closed

cleanup, use _testcapi.test_immortal_small_ints()

b8333ba

skirpichev commented Mar 13, 2026

View reviewed changes

eendebakpt reviewed Mar 13, 2026

View reviewed changes

Objects/longobject.c Show resolved Hide resolved

eendebakpt reviewed Mar 13, 2026

View reviewed changes

Lib/test/test_capi/test_long.py Outdated Show resolved Hide resolved

skirpichev added 2 commits March 14, 2026 03:02

+ add also test with nonzero small int

20e79c6

_long_is_small_int() -> _PyLong_IsImmortal() helper

413e56d

skirpichev requested review from colesbury and eendebakpt March 15, 2026 04:13

eendebakpt approved these changes Mar 17, 2026

View reviewed changes

Include/internal/pycore_long.h Outdated Show resolved Hide resolved

bedevere-app bot added awaiting core review and removed awaiting review labels Mar 17, 2026

colesbury reviewed Mar 17, 2026

View reviewed changes

colesbury approved these changes Mar 17, 2026

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting core review labels Mar 17, 2026

skirpichev added 2 commits March 18, 2026 02:12

Merge branch 'master' into fix-PyLong_FromString/143050

b892216

address review: rename _PyLong_IsImmortal -> _PyLong_HasImmortalTag

78c5b4b

skirpichev force-pushed the fix-PyLong_FromString/143050 branch from e28e94c to 78c5b4b Compare March 17, 2026 23:27

add assert

69d35bd

colesbury reviewed Mar 18, 2026

View reviewed changes

Include/internal/pycore_long.h Outdated Show resolved Hide resolved

address review: simplify assert

2f927e0

vstinner reviewed Mar 27, 2026

View reviewed changes

Lib/test/test_capi/test_long.py Show resolved Hide resolved

Lib/test/test_capi/test_long.py Show resolved Hide resolved

Include/internal/pycore_long.h Show resolved Hide resolved

skirpichev added 2 commits March 28, 2026 02:56

Merge branch 'master' into fix-PyLong_FromString/143050

53523de

address review: some comments

1f363f4

skirpichev requested a review from vstinner March 28, 2026 00:11

vstinner reviewed Mar 30, 2026

View reviewed changes

skirpichev added 2 commits March 31, 2026 02:41

Merge branch 'main' into fix-PyLong_FromString/143050

41961ed

address review: add assert on _PY_IS_SMALL_INT

0b29e28

skirpichev requested a review from vstinner March 31, 2026 00:22

skirpichev commented Mar 31, 2026

View reviewed changes

Rename to _PyLong_IsSmallInt()

45b3b04

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>

vstinner approved these changes Mar 31, 2026

View reviewed changes

vstinner enabled auto-merge (squash) March 31, 2026 12:59

vstinner merged commit db5936c into python:main Mar 31, 2026
92 of 94 checks passed

bedevere-app bot removed the awaiting merge label Mar 31, 2026

vstinner added the needs backport to 3.14 bugs and security fixes label Mar 31, 2026

bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Mar 31, 2026

skirpichev deleted the fix-PyLong_FromString/143050 branch March 31, 2026 14:06

Uh oh!

Conversation

skirpichev commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skirpichev commented Mar 13, 2026

Uh oh!

skirpichev commented Mar 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

eendebakpt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

colesbury left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

colesbury left a comment

Choose a reason for hiding this comment

Uh oh!

skirpichev commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

skirpichev commented Mar 18, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner commented Mar 31, 2026

Uh oh!

vstinner commented Mar 31, 2026

Uh oh!

skirpichev commented Mar 31, 2026

Uh oh!

vstinner commented Mar 31, 2026

Uh oh!

Uh oh!

miss-islington-app bot commented Mar 31, 2026

Uh oh!

bedevere-app bot commented Mar 31, 2026

Uh oh!

bedevere-app bot commented Mar 31, 2026

Uh oh!

Reviewers

Assignees

skirpichev commented Mar 13, 2026 •

edited

Loading

skirpichev commented Mar 17, 2026 •

edited

Loading