GeezerVison is showing github.com — 3.13.2: cast in Py_XDECREF causes runtime failures with immortal objects · Issue #135746 · python/cpython
3.13.2: cast in Py_XDECREF causes runtime failures with immortal objects #135746Labels OS-windowsinterpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)topic-C-APItype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error DescriptionBug report
Bug description:
In Python 3.13.2 I use
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) <-- crashThe 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 Activitypicnixz commented on Jun 20, 2025 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. changed the title [-]Py_XDECREF | Run-Time Check Failure | VS2022 | Python 3.13.2[/-] [+]3.13.2: cast in Py_XDECREF causes build failures with immortal objects[/+] on Jun 20, 2025Jejos commented on Jun 20, 2025 Hi picnixz, I just updated to 3.13.5, but the run-time check failure still exists and my program throws an exception. picnixz commented on Jun 20, 2025 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 commented on Jun 20, 2025
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 commented on Jun 20, 2025 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 commented on Jun 20, 2025 Yeah, as @zooba said, we do not use any run-time error checks in any of our Windows builds. 1
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?
changed the title [-]3.13.2: cast in Py_XDECREF causes build failures with immortal objects[/-] [+]3.13.2: cast in Py_XDECREF causes runtime failures with immortal objects[/+] on Jun 20, 2025Jejos commented on Jun 20, 2025 @chris-eibl: Sorry, you're right. I slipped in the line. In my project both, /RTCc and /RTC1 are enabled.
I tried to make a PoC from scratch, but so far, it doesn't crash, although my development project reproducibly crashes.
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.
As far as I understand the code, you use Py_ssize_t ob_refcnt as well as PY_UINT32_T ob_refcnt_split[2] and as far as I can see ob_refcnt_split[0] contains the lower 32 bit. So maybe another solution would be:
_Py_CAST(PY_INT32_T, op->ob_refcnt_split[0]) ? Jejos commented on Jun 20, 2025 I made a small project that reproduces the problem.
As an additional dependency the Python library "opencv_python" is needed (see info.txt). ZeroIntensity commented on Jun 20, 2025 I might be losing my mind, but looking at the repro and report, reading ob_refcnt (for _Py_IsImmortal) shouldn't be causing a crash, regardless of what the actual reference count is, or what the bitmask is. It will only crash if the object is invalid, such as when it's been freed.
I think that's probably what's going on here--I'm not a C++ expert, but I suspect the nullptr that you pass to PyObject_CallFunction isn't actually defined as (void *)0 on Windows, so Python doesn't notice (because it checks NULL) and tries to read it as a string. If it does notice and correctly falls back to _PyObject_CallNoArgsTstate, then the nullptr in the variadic arg could also cause problems, because it's left on the stack without being popped off by _PyObject_CallFunctionVa. In both cases, there's bad things going on that could cause weird crashes like this.
Basically, my theory is that some memory corruption happens, and then the returned pointer is just some non-NULL junk. Does the problem go away when you use PyObject_CallNoArgs? Jejos commented on Jun 23, 2025 You're probably right and it's a problem in the external library "opencv". I just downloaded it and try to build it to find the bug. zooba commented on Jun 23, 2025 Using a union would be better than casting, for sure. I think there was opposition to that when I last proposed it though (IIRC some compilers didn't like it).
Passing nullptr should be interpreted as (void *)0 anywhere it crosses into C. It's just for type checking (which in C++ means overload resolution, so it's actually a very useful type).
If a native module is involved, it's possible that it isn't aware of immortalization and so has been decrementing the refcount of immortal objects - I think you only need to decrement once to get the top bit of the lower DWORD set, which would cause this runtime check to fail. Perhaps we just need to change the check to quickly determine the common case (immortal, unchanged) and also handle the uncommon case (legacy extension has decref'd an immortal) but slower? encukou commented on Jun 23, 2025
@encukou [...] should understand the refcounting support well enough to know what the intended result is.
Sorry, I'm afraid my understanding is out of date by several optimization passes. Jejos commented on Jun 23, 2025 The version of OpenCV, that can currently be downloaded at PyPI, is "opencv-python 4.11.0.86".
To find the bug, I just downloaded the newest source and compiled OpenCV 4.12 (cv2.cp313-win_amd64.pyd).
When I replace the file Python313/Lib/sitepackages/cv2.pyd with my compilation, the crash vanishes! I can switch between both versions and the older version reproducibly crashes, while the new one doesn't.
Thanks to the discussion above, I got the understanding, that the return value of the old library (0x0000000100000000) is invalid and should never appear at this position in the Python code and that the library was buggy. Indeed, the new OpenCV version returns 0x00000000ffffffff.
Thanks a lot for all your support. The bug seems to be fixed, at least it's no problem with Python 3.13. zooba commented on Jun 24, 2025
When I replace the file Python313/Lib/sitepackages/cv2.pyd with my compilation, the crash vanishes!
Yeah, I guess that cv2.pyd is probably using the limited ABI, which breaksmisbehaves if you use an old version against immortal objects. Rebuilding for the current version is the best/only way to avoid this. vstinner commented on Jul 1, 2025 Can we close this issue since using a recent OpenVC version works around the issue? zooba commented on Jul 1, 2025 It's not a "recent OpenCV version" - it's compiling without the stable ABI (or at least compiling with stable ABI from after we added immortal object support to the headers and/or made Py_DECREF opaque).
If you're happy to say that stable ABI pre-3.whenever-that-was is unsupported, fine. I'll quote you on that when we're discussing replacing the stable ABI next, but I'm not opposed to it. vstinner commented on Jul 8, 2025 Sadly, it seems like building a C extension for the stable ABI with Python 3.11 or older (before PEP 683 – Immortal Objects) and using it on Python 3.12 and newer doesn't work well because of changes in the reference counting (Immortal Objects).
MetadataMetadataOS-windowsinterpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)topic-C-APItype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error DevelopmentNo branches or pull requestsIssue actions
|