gh-147988: Initialize digits in long_alloc() in debug mode by vstinner · Pull Request #147989 · python/cpython

vstinner · 2026-04-01T20:17:33Z

When Python is built in debug mode, long_alloc() now initializes digits with a pattern to detect usage of uninitialized digits.

_PyLong_CompactValue() makes sure that the digit is zero when the sign is zero.

Issue: Detect usage of uninitialized digits in Objects/longobject.c (_PyLong_New()) #147988

When Python is built in debug mode, long_alloc() now initializes digits with a pattern to detect usage of uninitialized digits. _PyLong_CompactValue() makes sure that the digit is zero when the sign is zero.

vstinner · 2026-04-01T20:27:14Z

cc @serhiy-storchaka @skirpichev

vstinner · 2026-04-01T20:55:10Z

I also modified PyLongWriter_Finish() to detect uninitialized digits. It's a good place for such checks :-)

serhiy-storchaka · 2026-04-01T21:35:51Z

Include/cpython/longintrepr.h

    assert(PyType_HasFeature(op->ob_base.ob_type, Py_TPFLAGS_LONG_SUBCLASS));
    assert(PyUnstable_Long_IsCompact(op));
    sign = 1 - (op->long_value.lv_tag & _PyLong_SIGN_MASK);
+    if (sign == 0) {


Does not it emit a warning or generate inefficient code in non-debug build?

You can write assert(size != 0 || ...).

I think that sane compiler should recognize here no-op. I've tested assembly code for a toy program with gcc -O2.

In release mode, assertions are removed and so the code becomes if (size == 0) {}. C compilers are good to remove such dead code. I looked at the assembly code of Python built in release mode, and I cannot see any instruction related to if (size == 0).

gcc -O3 doesn't emit a warning on such code.

serhiy-storchaka · 2026-04-01T21:38:03Z

Objects/longobject.c

    int sign = is_signed ? -1: 1;
    if (idigit == 0) {
        sign = 0;
+        v->long_value.ob_digit[0] = 0;


Is it needed in non-debug build?

As described in the issue, MSAN generates a memory error without this line, since _PyLong_CompactValue() uses uninitialized memory (ob_digit[0]).

If you ignore MSAN (and other sanitizers), maybe_small_long() returns the zero singleton, and _PyLong_CompactValue() returns 0 because it computes sign * digit where sign is 0 and digit is a random number.

This PR moves v->long_value.ob_digit[0] = 0; from long_alloc() to _PyLong_FromByteArray(), and make it conditional: only write ob_digit[0] if (idigit == 0) is true. So in the common case, the assignment is no longer done.

Technically, we could live without all this. But I think it's a good idea to set digit[0] for zero.

Historically, we set number of digits to zero for zero. I think it should be 1 to reflect actual memory allocation. Then zero representation will make sense in both "big int" format (sign + magnitude as 1-element array) and as a "small int" (sign*digit[0])

Objects/longobject.c

serhiy-storchaka · 2026-04-01T21:46:06Z

Objects/longobject.c

        Py_UNREACHABLE();
    }

+    if ((size_z + negz) == 0) {


This is equivalent to size_z == 0 & !negz, right? Why write the condition in such form? Why add this if? Is it needed in non-debug build?

This is equivalent to size_z == 0 & !negz, right? Why write the condition in such form?

My intent is to add a special case for long_alloc(0) on long_alloc(size_z + negz) below.

Why add this if?

The alternative is to add z->long_value.ob_digit[0] = 0; after long_alloc() below.

Do you prefer adding z->long_value.ob_digit[0] = 0; ?

Is it needed in non-debug build?

See my reply on _PyLong_FromByteArray() change.

Well, it does not matter.

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

serhiy-storchaka

LGTM. 👍

serhiy-storchaka · 2026-04-02T10:31:47Z

Objects/longobject.c

+    // long_alloc() fills digits with 0xFF byte pattern.
+    Py_ssize_t ndigits = _PyLong_DigitCount(obj);
+    if (ndigits == 0) {
+        // Check ob_digit[0] digit for the number zero


Should not we simply assert obj->long_value.ob_digit[0] == 0?

I modified the code to raise SystemError, so there is now more complex code below.

serhiy-storchaka · 2026-04-02T10:33:22Z

Objects/longobject.c

        Py_UNREACHABLE();
    }

+    if ((size_z + negz) == 0) {


Well, it does not matter.

PyLongWriter_Finish() now raises SystemError instead of stopping the process with abort(). Add test on PyLongWriter_Finish() bug.

vstinner · 2026-04-02T11:13:12Z

I modified PyLongWriter_Finish() to raise SystemError, instead of stopping the process with a fatal error. It may be a more pleasant developer experience. I also added a PyLongWriter test on uninitialized digits.

vstinner · 2026-04-02T14:29:00Z

Merged. Thanks for reviews.

pythongh-147988: Initialize digits in long_alloc() in debug mode

a14685f

When Python is built in debug mode, long_alloc() now initializes digits with a pattern to detect usage of uninitialized digits. _PyLong_CompactValue() makes sure that the digit is zero when the sign is zero.

vstinner added the skip news label Apr 1, 2026

bedevere-app bot added the awaiting core review label Apr 1, 2026

bedevere-app bot mentioned this pull request Apr 1, 2026

Detect usage of uninitialized digits in Objects/longobject.c (_PyLong_New()) #147988

Closed

vstinner mentioned this pull request Apr 1, 2026

gh-143050: add helper _PyLong_InitTag() #147956

Merged

PyLongWriter_Finish() detects uninitialized digits

91ace76

serhiy-storchaka reviewed Apr 1, 2026

View reviewed changes

Apply suggestion from @serhiy-storchaka

bb7e393

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

serhiy-storchaka approved these changes Apr 2, 2026

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting core review labels Apr 2, 2026

PyLongWriter_Finish() now raises SystemError

4be289e

PyLongWriter_Finish() now raises SystemError instead of stopping the process with abort(). Add test on PyLongWriter_Finish() bug.

PyLongWriter_Finish() checks most significant bits

d3293fd

vstinner enabled auto-merge (squash) April 2, 2026 11:39

serhiy-storchaka approved these changes Apr 2, 2026

View reviewed changes

vstinner merged commit c1a4112 into python:main Apr 2, 2026
51 checks passed

vstinner deleted the long_uninit branch April 2, 2026 11:55

bedevere-app bot removed the awaiting merge label Apr 2, 2026

Uh oh!

Conversation

vstinner commented Apr 1, 2026 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner commented Apr 1, 2026

Uh oh!

vstinner commented Apr 1, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

serhiy-storchaka left a comment

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!

vstinner commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

vstinner commented Apr 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vstinner commented Apr 1, 2026 •

edited by bedevere-app bot

Loading

vstinner commented Apr 2, 2026 •

edited

Loading