gh-98724: Fix Py_CLEAR() macro side effects (#99100)

[vstinner]

The Py_CLEAR(), Py_SETREF() and Py_XSETREF() macros now only evaluate their arguments once. If an argument has side effects, these side effects are no longer duplicated.

Use temporary variables to avoid duplicating side effects of macro arguments side effects. Use memcpy() for assignment to prevent a miscompilation with strict aliasing caused by type punning.

Add test_py_clear() and test_py_setref() unit tests to _testcapi.

• Issue: Macro Py_CLEAR references argument two times. #98724

[netlify]

✅ Deploy Preview for python-cpython-preview ready!

Name	Link
🔨 Latest commit	cffd9d4
🔍 Latest deploy log	https://app.netlify.com/sites/python-cpython-preview/deploys/639094347616260008a4e69e
😎 Deploy Preview	https://deploy-preview-100070--python-cpython-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[vstinner]

cc @serge-sans-paille @erlend-aasland: This version should no longer miscompile Python: it avoids the type punning issue thanks to memcpy(). I wrote a comment in Py_CLEAR() to explain the rationale for memcpy().

[vstinner]

This PR no longer reintroduces the miscompilation bug: issue #99701. I tested manually.

I tested this PR with this local patch (otherwise the output is just too verbose!):

diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index 8209132ebc..cf243c8ef9 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -1891,9 +1891,6 @@ Py_FinalizeEx(void)
     /* Destroy all modules */
     finalize_modules(tstate);
 
-    /* Print debug stats if any */
-    _PyEval_Fini();
-
     /* Flush sys.stdout and sys.stderr (again, in case more was printed) */
     if (flush_std_files() < 0) {
         status = -1;

Test command:

rm ~/pyprefix -rf; git clean -fdx
./configure --enable-pystats --prefix ~/pyprefix CC=gcc CFLAGS="-O3 -fstrict-aliasing"
make && make install
./python -m test test_capi test_itertools

The test succeeded with CC=gcc CFLAGS="-O3 -fstrict-aliasing" and CC=clang LD=clang CFLAGS="-O3 -fstrict-aliasing".

[vstinner]

I updated my PR to use __typeof__() if available to still emit compiler warnings in Py_SETREF(dst, src) if src has not the same type than dst. On Windows, the memcpy() implementation is used since __typeof__() is not available: memcpy() avoids the type punning issue, but it does not emit a compiler warning if src has the same type.

My previous attempt PR #99739 always used __typeof__(), but this function is not available on Windows with MSVC.

[vstinner]

I ran my test for 4 cases and all succeeded:

• GCC with __typeof__()
• GCC with memcpy()
• clang with __typeof__()
• clang with memcpy()

[vstinner]

To also emit a warning in the memcpy() implementation, @serge-sans-paille suggested to me using sizeof(dst=src). It works because sizeof() does not evalute its argument at runtime, it only infers the argument type. MSVC emits a warning with sizeof(dst=src).

I'm not sure about this one. Maybe this enhancement can be made later, once the initial issue #98724 is fixed with my current PR implementation. So it's easy to revert if something goes wrong.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @vstinner for commit 86d8878 🤖

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

[vstinner]

buildbot/ARM64 Windows PR

test_ast.test_recursion_direct() crash with a stack overflow. It sounds unrelated to this change. The ARM64 Windows 3.x buildbot is sick for 2 days: it fails with "exception" (don't build+test Python).

test_parse_in_error (test.test_ast.ASTHelpers_Test.test_parse_in_error) ... ok
Windows fatal exception: stack overflow
Current thread 0x0000142c (most recent call first):
  File "C:\Workspace\buildarea\pull_request.linaro-win-arm64\build\Lib\test\test_ast.py", line 1252 in test_recursion_direct

[vstinner]

Apart of the sick "ARM64 Windows 3.x" buildbot, all buildbots were happy with this change.

I fixed a fix typos. I amended the commit message.

[vstinner]

buildbot/ARM64 Windows PR: test_ast.test_recursion_direct() crash with a stack overflow

FYI I proposed #100104 to fix this bug (unrelated to this change).