Skip to content
Open
15 changes: 14 additions & 1 deletion Include/internal/pycore_long.h
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,17 @@ _PyLong_IsPositive(const PyLongObject *op)
return (op->long_value.lv_tag & SIGN_MASK) == 0;
}

/* Return true if the argument is a small int */
static inline bool
_PyLong_HasImmortalTag(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));
assert(_Py_IsImmortal(op) || (!is_small_int));
return is_small_int;
}

static inline Py_ssize_t
_PyLong_DigitCount(const PyLongObject *op)
{
Expand Down Expand Up @@ -292,7 +303,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_HasImmortalTag(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
10 changes: 10 additions & 0 deletions Lib/test/test_capi/test_long.py
Original file line number Diff line number Diff line change
Expand Up @@ -803,6 +803,16 @@ 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):
# Bug coming from using _pylong.int_from_string(), that
# currently requires > 6000 decimal digits.
int('-' + '0' * 7000, 10)
_testcapi.test_immortal_small_ints()
# Test also nonzero small int
int('-' + '0' * 7000 + '123', 10)
_testcapi.test_immortal_small_ints()
Copy link
Copy Markdown
Member Author

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?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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



if __name__ == "__main__":
unittest.main()
4 changes: 2 additions & 2 deletions Modules/_testcapi/immortal.c
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_HasImmortalTag((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_HasImmortalTag((PyLongObject *)obj);
assert(!has_int_immortal_bit);
Py_DECREF(obj);
}
Expand Down
20 changes: 5 additions & 15 deletions Objects/longobject.c
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
Copy Markdown
Contributor

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
Copy Markdown
Member Author

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
Copy Markdown
Member Author

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_HasImmortalTag((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_HasImmortalTag((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