Skip to content

gh-143050: correct PyLong_FromString() to use _PyLong_Negate() #145901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
skirpichev wants to merge 4 commits into
base: main
Choose a base branch
from
+26 −18
Open

gh-143050: correct PyLong_FromString() to use _PyLong_Negate() #145901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fbcec5a
gh-143050: correct PyLong_FromString() to use _PyLong_Negate()
skirpichev Mar 13, 2026
b8333ba
cleanup, use _testcapi.test_immortal_small_ints()
skirpichev Mar 13, 2026
20e79c6
+ add also test with nonzero small int
skirpichev Mar 14, 2026
413e56d
_long_is_small_int() -> _PyLong_IsImmortal() helper
skirpichev Mar 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion Include/internal/pycore_long.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,15 @@ _PyLong_IsPositive(const PyLongObject *op)
return (op->long_value.lv_tag & SIGN_MASK) == 0;
}

static inline bool
_PyLong_IsImmortal(const PyLongObject *op)
{
assert(PyLong_Check(op));
bool is_small_int = (op->long_value.lv_tag & IMMORTALITY_BIT_MASK) != 0;
assert(PyLong_CheckExact(op) || (!is_small_int));
return is_small_int;
}

static inline Py_ssize_t
_PyLong_DigitCount(const PyLongObject *op)
{
Expand Down Expand Up @@ -292,7 +301,9 @@ _PyLong_SetDigitCount(PyLongObject *op, Py_ssize_t size)
#define NON_SIZE_MASK ~(uintptr_t)((1 << NON_SIZE_BITS) - 1)

static inline void
_PyLong_FlipSign(PyLongObject *op) {
_PyLong_FlipSign(PyLongObject *op)
{
assert(!_PyLong_IsImmortal(op));
unsigned int flipped_sign = 2 - (op->long_value.lv_tag & SIGN_MASK);
op->long_value.lv_tag &= NON_SIZE_MASK;
op->long_value.lv_tag |= flipped_sign;
Expand Down
7 changes: 7 additions & 0 deletions Lib/test/test_capi/test_long.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -803,6 +803,13 @@ def to_digits(num):
self.assertEqual(pylongwriter_create(negative, digits), num,
(negative, digits))

def test_bug_143050(self):
with support.adjust_int_max_str_digits(0):
int('-' + '0' * 7000, 10)
_testcapi.test_immortal_small_ints()
int('-' + '0' * 7000 + '123', 10)
_testcapi.test_immortal_small_ints()
Copy link
Member Author

@skirpichev skirpichev Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?



if __name__ == "__main__":
unittest.main()
4 changes: 2 additions & 2 deletions Modules/_testcapi/immortal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ test_immortal_small_ints(PyObject *self, PyObject *Py_UNUSED(ignored))
for (int i = -5; i <= 1024; i++) {
PyObject *obj = PyLong_FromLong(i);
assert(verify_immortality(obj));
int has_int_immortal_bit = ((PyLongObject *)obj)->long_value.lv_tag & IMMORTALITY_BIT_MASK;
int has_int_immortal_bit = _PyLong_IsImmortal((PyLongObject *)obj);
assert(has_int_immortal_bit);
}
for (int i = 1025; i <= 1030; i++) {
PyObject *obj = PyLong_FromLong(i);
assert(obj);
int has_int_immortal_bit = ((PyLongObject *)obj)->long_value.lv_tag & IMMORTALITY_BIT_MASK;
int has_int_immortal_bit = _PyLong_IsImmortal((PyLongObject *)obj);
assert(!has_int_immortal_bit);
Py_DECREF(obj);
}
Expand Down
20 changes: 5 additions & 15 deletions Objects/longobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3118,11 +3118,11 @@ PyLong_FromString(const char *str, char **pend, int base)
}

/* Set sign and normalize */
if (sign < 0) {
_PyLong_FlipSign(z);
Copy link
Contributor

@eendebakpt eendebakpt Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling _PyLong_FlipSign this on the 0 small int caused the problems. Can we add an assert in _PyLong_FlipSign:

static inline void
_PyLong_FlipSign(PyLongObject *op) {
    assert(!__PyLong_IsSmallInt((PyObject *)op));
    unsigned int flipped_sign = 2 - (op->long_value.lv_tag & SIGN_MASK);
    op->long_value.lv_tag &= NON_SIZE_MASK;
    op->long_value.lv_tag |= flipped_sign;
}

The __PyLong_IsSmallInt does not exist yet, I used the following in testing:

static inline int
/// Return 1 if the object is one of the immortal small ints
_PyLong_IsSmallInt(PyObject *op)
{
    // slow, but safe version, for use in assertions and debugging.
    for(int i = 0; i < _PY_NSMALLPOSINTS + _PY_NSMALLNEGINTS; i++) {
        if (op == (PyObject *)&_PyLong_SMALL_INTS[i]) {
            return 1;
        }
    }
    return 0;
}

In a similar way we could add an assert to _PyLong_SetSignAndDigitCount.

Copy link
Member Author

@skirpichev skirpichev Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling _PyLong_FlipSign this on the 0 small int caused the problems.

Yes, but any small int will trigger this. We can add test case for -1, for example.

Can we add an assert in _PyLong_FlipSign

My suggestion would be using assert(!_PyLong_IsImmortal((PyObject *)op));: as the "small int" bit will be cleared by the _PyLong_FlipSign(). (_PyLong_IsImmortal() is current _long_is_small_int().)

In a similar way we could add an assert to _PyLong_SetSignAndDigitCount.

This will affect all "setters" in Include/internal/pycore_long.h (i.e. also _PyLong_SetDigitCount).

Copy link
Member Author

@skirpichev skirpichev Mar 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added _PyLong_IsImmortal() helper.

In a similar way we could add an assert to _PyLong_SetSignAndDigitCount.

I tried to add assert here and in _PyLong_SetDigitCount(), but got:

$ make -s
_freeze_module: ./Include/internal/pycore_long.h:291: _PyLong_SetSignAndDigitCount: Assertion `!_PyLong_IsImmortal(op)' failed.
Aborted
make: *** [Makefile:1952: Python/frozen_modules/getpath.h] Error 134

}
long_normalize(z);
z = maybe_small_long(z);
if (sign < 0) {
_PyLong_Negate(&z);
}

if (pend != NULL) {
*pend = (char *)str;
Expand Down Expand Up @@ -3622,21 +3622,11 @@ long_richcompare(PyObject *self, PyObject *other, int op)
Py_RETURN_RICHCOMPARE(result, 0, op);
}

static inline int
/// Return 1 if the object is one of the immortal small ints
_long_is_small_int(PyObject *op)
{
PyLongObject *long_object = (PyLongObject *)op;
int is_small_int = (long_object->long_value.lv_tag & IMMORTALITY_BIT_MASK) != 0;
assert((!is_small_int) || PyLong_CheckExact(op));
return is_small_int;
}

void
_PyLong_ExactDealloc(PyObject *self)
{
assert(PyLong_CheckExact(self));
if (_long_is_small_int(self)) {
if (_PyLong_IsImmortal((PyLongObject *)self)) {
// See PEP 683, section Accidental De-Immortalizing for details
_Py_SetImmortal(self);
return;
Expand All @@ -3651,7 +3641,7 @@ _PyLong_ExactDealloc(PyObject *self)
static void
long_dealloc(PyObject *self)
{
if (_long_is_small_int(self)) {
if (_PyLong_IsImmortal((PyLongObject *)self)) {
/* This should never get called, but we also don't want to SEGV if
* we accidentally decref small Ints out of existence. Instead,
* since small Ints are immortal, re-set the reference count.
Expand Down
Loading