Skip to content

gh-143050: add helper _PyLong_InitTag()#147956

Open
skirpichev wants to merge 6 commits intopython:mainfrom
skirpichev:add-_PyLong_Init
Open

gh-143050: add helper _PyLong_InitTag()#147956
skirpichev wants to merge 6 commits intopython:mainfrom
skirpichev:add-_PyLong_Init

Conversation

@skirpichev
Copy link
Copy Markdown
Member

@skirpichev skirpichev commented Apr 1, 2026

With this we can assume, that _PyLong_Set*() operate on non-immortal integers.

With this we can assume, that _PyLong_Set*() operate on
non-immortal integers.
{
assert(PyLong_Check(op));
op->long_value.lv_tag = 1; /* non-immortal zero */
op->long_value.ob_digit[0] = 0;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would prefer to only initialize the tag and so rename the function to _PyLong_InitTag().

op->long_value.ob_digit[0] = 0; is only used (needed) by long_alloc(). All other functions using _PyLong_Init() will rewrite ob_digit[0] immediately and so it's inefficient.

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 think this may trigger a compiler warning. I did renaming, lets do another change in a second commit.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lets do another change in a second commit.

You didn't make the second commit yet.

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.

@skirpichev skirpichev changed the title gh-143050: add helper _PyLong_Init() gh-143050: add helper _PyLong_InitTag() Apr 1, 2026
@vstinner
Copy link
Copy Markdown
Member

vstinner commented Apr 1, 2026

"Tests / Android (x86_64)" failed with: Error on ZipFile unknown archive. It's unrelated to this change.

  > Task :app:maxVersionSetup
  "Install Android Emulator v.36.4.10" ready.
  Installing Android Emulator in /usr/local/lib/android/sdk/emulator
  "Install Android Emulator v.36.4.10" complete.
  "Install Android Emulator v.36.4.10" finished.
  Checking the license for package AOSP ATD Intel x86_64 Atom System Image in /usr/local/lib/android/sdk/licenses
  License for package AOSP ATD Intel x86_64 Atom System Image accepted.
  Preparing "Install AOSP ATD Intel x86_64 Atom System Image API 32 (revision 1)".
  Warning: An error occurred while preparing SDK package AOSP ATD Intel x86_64 Atom System Image: Error on ZipFile unknown archive.:
  java.io.IOException: Error on ZipFile unknown archive
  	at org.apache.commons.compress.archivers.zip.ZipFile.<init>(ZipFile.java:383)
  	at org.apache.commons.compress.archivers.zip.ZipFile.<init>(ZipFile.java:323)
  	at org.apache.commons.compress.archivers.zip.ZipFile.<init>(ZipFile.java:279)
  	at com.android.repository.util.InstallerUtil.unzip(InstallerUtil.java:101)
  	at com.android.repository.impl.installer.BasicInstaller.doPrepare(BasicInstaller.java:91)

@skirpichev
Copy link
Copy Markdown
Member Author

skirpichev commented Apr 1, 2026 via email

@vstinner
Copy link
Copy Markdown
Member

vstinner commented Apr 1, 2026

"Tests / CIFuzz / cpython3 (memory)" CI logs an use-of-uninitialized-value:

==45==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x55af57984f54 in _PyLong_IsSmallInt /src/cpython3/Include/internal/pycore_long.h:243:5
    #1 0x55af57984f54 in _PyLong_SetSignAndDigitCount /src/cpython3/Include/internal/pycore_long.h:305:5
    #2 0x55af579844c6 in long_alloc /src/cpython3/Objects/longobject.c:190:5
    #3 0x55af5798ef9a in PyLong_FromUnsignedLong /src/cpython3/Objects/longobject.c:442:5
    #4 0x55af5798ef9a in PyLong_FromVoidPtr /src/cpython3/Objects/longobject.c:1505:12

pycore_long.h:243 is the following assertion in _PyLong_IsSmallInt():

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

Oh right, op->long_value.ob_digit[0] = 0; is needed before calling _PyLong_IsSmallInt() to have initialized memory (_PyLong_CompactValue()).

Copy link
Copy Markdown
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

What do you think of changing _PyLong_IsSmallInt() implementation like that?

static inline bool
_PyLong_IsSmallInt(const PyLongObject *op)
{
    assert(PyLong_Check(op));
    bool is_small_int = (op->long_value.lv_tag & IMMORTALITY_BIT_MASK) != 0;
    if (is_small_int) {
        assert(PyLong_CheckExact(op));
        assert(_Py_IsImmortal(op));
        assert((_PyLong_IsCompact(op)
                && _PY_IS_SMALL_INT(_PyLong_CompactValue(op))));
    }
    return is_small_int;
}

The current code runs many checks when is_small_int is false which are not needed.

@skirpichev
Copy link
Copy Markdown
Member Author

What do you think of changing _PyLong_IsSmallInt() implementation like that?

Lets try, but I suspect it might fall in other place.

Comment on lines +289 to +295
/* Initialize a freshly-allocated int.
*
* Fast operations for single digit integers (including zero)
* assume that there is always at least one digit present.
* The digit has to be initialized explicitly to avoid
* use-of-uninitialized-value.
*/
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This function doesn't initialize the first digit, so I'm not sure why you reintroduce the "The digit has to be initialized explicitly" comment:

Suggested change
/* Initialize a freshly-allocated int.
*
* Fast operations for single digit integers (including zero)
* assume that there is always at least one digit present.
* The digit has to be initialized explicitly to avoid
* use-of-uninitialized-value.
*/
/* Initialize the tag of a freshly-allocated int. */

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants