PyLongWriter_Finish() now raises SystemError

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

Author: vstinner

Lib/test/test_capi/test_long.py

@@ -803,6 +803,16 @@ def to_digits(num):
                 self.assertEqual(pylongwriter_create(negative, digits), num,
                                  (negative, digits))
 
+    @unittest.skipUnless(support.Py_DEBUG, "need a debug build (Py_DEBUG)")
+    def test_longwriter_finish(self):
+        # Test PyLongWriter_Create(0, 3, &digits) with PyLongWriter_Finish()
+        # where the last digit is left uninitialized
+        pylongwriter_finish_bug = _testcapi.pylongwriter_finish_bug
+        with self.assertRaises(SystemError) as cm:
+            pylongwriter_finish_bug()
+        self.assertEqual(str(cm.exception),
+                         'PyLongWriter_Finish: digit 2 is uninitialized')
+
     def test_bug_143050(self):
         with support.adjust_int_max_str_digits(0):
             # Bug coming from using _pylong.int_from_string(), that

Modules/_testcapi/long.c

@@ -254,6 +254,25 @@ pylongwriter_create(PyObject *module, PyObject *args)
 }
 
 
+static PyObject *
+pylongwriter_finish_bug(PyObject *module, PyObject *Py_UNUSED(args))
+{
+    void *writer_digits;
+    PyLongWriter *writer = PyLongWriter_Create(0, 3, &writer_digits);
+    if (writer == NULL) {
+        return NULL;
+    }
+
+    assert(PyLong_GetNativeLayout()->digit_size == sizeof(digit));
+    digit *digits = writer_digits;
+    digits[0] = 1;
+    digits[1] = 1;
+    // Oops, digits[2] is left uninitialized on purpose
+    // to test PyLongWriter_Finish()
+    return PyLongWriter_Finish(writer);
+}
+
+
 static PyObject *
 get_pylong_layout(PyObject *module, PyObject *Py_UNUSED(args))
 {
@@ -271,6 +290,7 @@ static PyMethodDef test_methods[] = {
     {"pylong_aspid",                pylong_aspid,               METH_O},
     {"pylong_export",               pylong_export,              METH_O},
     {"pylongwriter_create",         pylongwriter_create,        METH_VARARGS},
+    {"pylongwriter_finish_bug",     pylongwriter_finish_bug,    METH_NOARGS},
     {"get_pylong_layout",           get_pylong_layout,          METH_NOARGS},
     {"pylong_ispositive",           pylong_ispositive,          METH_O},
     {"pylong_isnegative",           pylong_isnegative,          METH_O},

Objects/longobject.c

@@ -6959,16 +6959,23 @@ PyLongWriter_Finish(PyLongWriter *writer)
     assert(Py_REFCNT(obj) == 1);
 
 #ifdef Py_DEBUG
-    // gh-147988: Detect uninitialized digits:
-    // long_alloc() fills digits with 0xFF byte pattern.
+    // gh-147988: Detect uninitialized digits: long_alloc() fills digits with
+    // 0xFF byte pattern. It's posssible because PyLong_BASE is smaller than
+    // the maximum value of the C digit type (uint32_t or unsigned short).
     Py_ssize_t ndigits = _PyLong_DigitCount(obj);
     if (ndigits == 0) {
         // Check ob_digit[0] digit for the number zero
         ndigits = 1;
     }
     for (Py_ssize_t i = 0; i < ndigits; i++) {
         digit d = obj->long_value.ob_digit[i];
-        assert(d < PyLong_BASE);
+        if (d >= PyLong_BASE) {
+            Py_DECREF(obj);
+            PyErr_Format(PyExc_SystemError,
+                         "PyLongWriter_Finish: digit %zd is uninitialized",
+                         i);
+            return NULL;
+        }
     }
 #endif