See https://bugs.python.org/issue45210#msg416849 for a manual test on this PR.

@serhiy-storchaka @methane @pablogsal: is it this change acceptable for a debug build? The intent is to detect bugs in C extensions. It's similar to _Py_CheckFunctionResult() and _Py_CheckSlotResult(). For example, if a deallocator function clears the current exception, it can be very problematic for code like:

PyErr_SetString(...);
Py_DECREF(obj);  // Call PyErr_Clear() !!!
return NULL;  // return NULL with no exception raised!?

old_exc_type is a borrowed reference. If the exception type is deallocated, comparing a pointer is an UB.

Well, I made the assumption that a tp_dealloc function is not going to allocate new types which would get he same memory address. Are you suggesting to store a strong reference to the exception type? I tried to minimize _Py_Dealloc() overhead, since it's called often. I tried to avoid INCREF/DECREF on the type, but IMO the error message would be useless if we cannot dump the freed object and we cannot at least mention the type name.

What if the deallocator raised an exception with the same popular type?

If the old and new exception types are the same, the test doesn't notice that tp_dealloc raised a new exception. Do you mean that I should store the pointer to the exception instance ("exc value") and compare it to the new value as well?

What if the exception was not normalized and the deallocator normalized it? Normalization of OSError changes the type.

The deallocator must not normalize the current exception: it must not change the current exception. IMO the test must fail in this case.

Either it doesn't touch exceptions at all, or it must save and restore the current exception. See GH #28358 and https://bugs.python.org/issue45210.

old_exc_type is a borrowed reference. If the exception type is deallocated, comparing a pointer is an UB.

If tp_dealloc deallocates the old exception type, allocates a new new exception type which gets the same address and tp_dealloc raises a new exception (bug!) with the new type: my test fails to detect the bug. Well. It's unfortunate. But at least, the test cannot crash Python (fatal error) with a false positive: it cannot fail even if the code is correct.

If you prefer, I can told strong references to the old exception type and to the old exception value.

Are you suggesting to store a strong reference to the exception type?

Practically, it perhaps does not cause problems on common platforms. Not sure about WebAssembly. And UB is UB -- the result is unpredictable, it can be not only crash.

@tiran @mdickinson

Py_Debug builds don't work on WASM yet. The default stack size is too small.

🤖 New build scheduled with the buildbot fleet by @gpshead for commit 3c57ddbd91ebdadc472749e97d9411269ac6d102 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

Updates:

• I rebased my PR
• I replaced bpo-45210 with gh-89373
• I added Py_XINCREF(old_exc_type) to avoid any risk of undefined behavior
• Mention the new feature in the documentation of Python Debug Mode

@gpshead:

I'm generally in favor of these kinds of checks in debug builds. They help ferret out issues in our code. If possible for this check, consider keying off of #ifndef NDEBUG rather than #ifdef Py_DEBUG. More builds use NDEBUG than full --with-pydebug.

I'm not comfortable with replacing Py_DEBUG with NDEBUG. I would prefer to have a guidelines saying what's acceptable or not acceptable for Py_DEBUG, NDEBUG and release builds. There is also the Python Development Mode which can be turned on "at runtime" (on the command line). My worry here is that the two Py_INCREF() might have subtle behavior changes. For me, NDEBUG is specific to assertions, but here is no call to assert(...) but an if with a call to Py_FatalError() on error.

Since Python 3.8, debug and release builds are ABI compatible. Where is the line between checking parameters at runtime and performance? Last time I proposed to drop these checks at runtime, the idea was rejected. The main reason is that popular CIs only provide release builds of Python, not debug builds.