3.13.2: cast in Py_XDECREF causes runtime failures with immortal objects

[Jejos]

Bug report

Bug description:

In Python 3.13.2 I use

Py_XDECREF(PyObject *op) 

It is called at the end of a function-call to load an image (cv2.imread) which fails, resulting in a None value (not NULL!). For Python 3.9 Py_XDECREF works just fine, without crash. For Python 3.13.2 the program, compiled with VS2022 (64 Bit), raises a "Run-Time Check Failure #1 - A cast to a smaller data type has caused a loss of data." and my application crashes ("/RTC1 = Basic Runtime Checks enabled").

I followed up the calls:

    Py_DECREF
        _Py_IsImmortal
            #elif SIZEOF_VOID_P > 4
                _Py_CAST(PY_INT32_T, op->ob_refcnt)     <-- crash

The value is

op->ob_refcnt == 0x0000000100000000

To fix this problem, I propose, that the type cast to 32 bit should be changed to

_Py_CAST(PY_INT32_T, op->ob_refcnt & 0xFFFFFFFF)

CPython versions tested on:

3.13

Operating systems tested on:

Windows

[picnixz]

Can you check with Python 3.13.5? I think we changed something concerning checks on immortal objects. Also, this build failure hasn't been caught by GHA so I'm surprised.

[Jejos]

Hi picnixz, I just updated to 3.13.5, but the run-time check failure still exists and my program throws an exception.

[picnixz]

Can you provide a PoC please? also, I believe that it's because of warnings treated as errors (I'm actually surprised that Windows complains here because ob_count is a uint32_t so the cast to int32_t should be safe).

cc @chris-eibl as a Windows expert

[Jejos]

I believe that it's because of warnings treated as errors

Yes, that's true for my project.

On my system, VS2022 (64 Bit) says: op->ob_recnt is __int64.

[zooba]

The runtime checks add some overhead, so performance-focused people turn them off. Having a test run that validates with them on would be a good thing for us to have, even if we do eventually release with them off.

uint32_t to int32_t isn't any more safe than any other cast without a check (which is why Python checks, and runtime checks can be enabled in MSVC).

Changing it to _Py_CAST(PY_INT32_T, op->ob_refcnt & 0x7FFFFFFF) (assuming we don't want negative results) would make sense for avoiding the warning, but I admit I'm not up on exactly what is being calculated here.

@encukou and/or @vstinner should understand the refcounting support well enough to know what the intended result is.

[chris-eibl]

Yeah, as @zooba said, we do not use any run-time error checks in any of our Windows builds. ¹

You mentioned that you are using

"/RTC1 = Basic Runtime Checks enabled"

What I do not understand, according to the documentation

/RTC1 Equivalent to /RTCsu

this should only enable stack frame run-time error checking (/RTCs) and checking for unitialized variables (/RTCu), but not checking for data losses due to assigning to a smaller data type (/RTCc).

For /RTCc, it is mentioned, that

This option can report situations in which you intend to truncate.

and the suggested workaround would be to

first mask off the information you need to avoid the run-time error

as you and @zooba proposed.

But here in _Py_IsImmortal, this "masking by just casting" is intentional to get the lower bits, and introducing any "real" masking like & 0xFFFFFFFF can result in a performance penalty in potentially hot code.

@ezio-melotti and later @markshannon tried very hard to keep the performance impact of immortal objects as low as possible.

Maybe the runtime_checks pragma could be used to turn it off for _Py_IsImmortal?

Footnotes

1. Furthermore, it cannot be used when compiling with optimizations enabled ↩