gh-143050: correct PyLong_FromString() to use _PyLong_Negate()#145901
gh-143050: correct PyLong_FromString() to use _PyLong_Negate()#145901skirpichev wants to merge 2 commits intopython:mainfrom
Conversation
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().
|
Shouldn't affect users, so - no news. |
|
|
| with support.adjust_int_max_str_digits(0): | ||
| a = int('-' + '0' * 7000, 10) | ||
| del a | ||
| _testcapi.test_immortal_small_ints() |
|
|
||
| /* Set sign and normalize */ | ||
| if (sign < 0) { | ||
| _PyLong_FlipSign(z); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
|
|
||
| def test_bug_143050(self): | ||
| with support.adjust_int_max_str_digits(0): | ||
| a = int('-' + '0' * 7000, 10) |
There was a problem hiding this comment.
Also add the test case int('-' + '0' * 7000 + '123', 10)? That case was already fine on the main branch, but it would not hurt to add it I think.
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