test_capi.test_basic_loop(): _PyInstruction_GetLength() assertion error on s390x Fedora Clang 3.x buildbot

[vstinner]

s390x Fedora Clang 3.x: https://buildbot.python.org/all/#/builders/3/builds/4312

• Last successful build, build 4216 (July 7): https://buildbot.python.org/all/#/builders/3/builds/4216 -- commit 67a7988
• First failed build, build 4217 (July 7): https://buildbot.python.org/all/#/builders/3/builds/4217 -- commit e1d45b8

Differences between the two builds:

commit e1d45b8ed43e1590862319fec33539f8adbc0849
Author: Guido van Rossum <guido@python.org>
Date: Thu Jul 6 16:46:06 2023 -0700

 gh-104584: Handle EXTENDED_ARG in superblock creation (#106489)
 
 With test.

commit c60df361ce2d734148d503f4a711e67c110fe223
Author: Gregory P. Smith <greg@krypto.org>
Date: Thu Jul 6 15:46:50 2023 -0700

 gh-90876: Restore the ability to import multiprocessing when `sys.executable` is `None` (#106464)
 
 Prevent `multiprocessing.spawn` from failing to *import* in environments
 where `sys.executable` is `None`. This regressed in 3.11 with the addition
 of support for path-like objects in multiprocessing.
 
 Adds a test decorator to have tests only run when part of test_multiprocessing_spawn to `_test_multiprocessing.py` so we can start to avoid re-running the same not-global-state specific test in all 3 modes when there is no need.

commit 76fac7bce55302a8e9a524d72f5384fd89e6dfde
Author: Guido van Rossum <guido@python.org>
Date: Thu Jul 6 15:45:56 2023 -0700

 gh-104584: Clean up and fix uops tests and fix crash (#106492)
 
 The uops test wasn't testing anything by default,
 and was failing when run with -Xuops.
 
 Made the two executor-related context managers global,
 so TestUops can use them (notably `with temporary_optimizer(opt)`).
 
 Made clear_executor() a little more thorough.
 
 Fixed a crash upon finalizing a uop optimizer,
 by adding a `tp_dealloc` handler.

Error:

0:05:26 load avg: 7.08 [435/447/1] test_capi crashed (Exit code -6) -- running: (...)
python: Python/instrumentation.c:262: int _PyInstruction_GetLength(PyCodeObject *, int): Assertion `opcode != 0' failed.
Fatal Python error: Aborted

Current thread 0x000003ff990770a0 (most recent call first):
 File "/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.clang/build/Lib/test/test_capi/test_misc.py", line 2441 in get_first_executor
 File "/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.clang/build/Lib/test/test_capi/test_misc.py", line 2459 in test_basic_loop
(...)

0:07:22 load avg: 3.60 Re-running test_capi in verbose mode
(...)
test_gilstate_ensure_no_deadlock (test.test_capi.test_misc.TestThreadState.test_gilstate_ensure_no_deadlock) ... ok
test_gilstate_matches_current (test.test_capi.test_misc.TestThreadState.test_gilstate_matches_current) ... ok
test_thread_state (test.test_capi.test_misc.TestThreadState.test_thread_state) ... ok
python: Python/instrumentation.c:262: int _PyInstruction_GetLength(PyCodeObject *, int): Assertion `opcode != 0' failed.
Fatal Python error: Aborted

Current thread 0x000003ff854770a0 (most recent call first):
 File "/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.clang/build/Lib/test/test_capi/test_misc.py", line 2441 in get_first_executor
 File "/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.clang/build/Lib/test/test_capi/test_misc.py", line 2459 in test_basic_loop
 File "/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.clang/build/Lib/unittest/case.py", line 589 in _callTestMethod
 File "/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.clang/build/Lib/unittest/case.py", line 634 in run
 File "/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.clang/build/Lib/unittest/case.py", line 690 in __call__
 File "/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.clang/build/Lib/unittest/suite.py", line 122 in run
(...)

cc @gvanrossum

Linked PRs

• gh-107082: Remove debug hack from get_first_executor in test_capi.test_misc #107085
• gh-107082: Fix _Py_CODEUNIT struct for big endian #107216
• gh-107082: Fix instruction size computation for ENTER_EXECUTOR #107256

[vstinner]

This issue looks like issue #107083.

[gvanrossum]

@vstinner Thanks, I think I have a fix. Should I trigger the buildbots once CI passes, to verify? (It might waste fewer resources if you trigger that particular buildbot manually? I don't recall how.)

[vstinner]

Should I trigger the buildbots once CI passes, to verify?

Since there are many broken buildbots, I failed to identify on which buildbots this error occurs. I don't recall if "s390x Fedora Clang 3.x" is connected to the GitHub "buildbot" checks. Well, you can try to add the buildbot label, and so if it's run or not :-)

I don'thave access to this machine to validate a fix manually :-(

[gvanrossum]

My theory about how the debug fragment or 1 would cause the C assertion failure is flawed -- the code in PyUnstable_GetExecutor that calls _PyInstruction_GetLength always starts at the first instruction and only ever calls that function with a valid starting point, so opcode should never be zero.

However, maybe the culprit is byte order. The machine where this fails is big-endian. IIRC all machines in regular CI are little-endian (Intel, ARM).

I think this structure defined in Include/cpython/code.h is at fault:

typedef union{
 uint16_t cache;
 struct{
 uint8_t code;
 uint8_t arg;
 } op;
} _Py_CODEUNIT;

In particular, the struct is supposed to always place code in the first byte and arg in the second byte. But maybe Clang on s390x somehow has its own ideas? If somehow the bytes were swapped (and arg came before code, or some weird padding occurred) I could easily see how we'd end up with opcode == 0, failing the assert.

CC @markshannon

[vstinner]

Is the issue fixed or not? This issue was closed.

[vstinner]

I think this structure defined in Include/cpython/code.h is at fault:

Can you just exchange code and arg depending on the endianness? You can use PY_BIG_ENDIAN and PY_LITTLE_ENDIAN macros (they are equal to 0 or 1).

[vstinner]

The buildbot still fails, so I reopen the issue: https://buildbot.python.org/all/#/builders/3/builds/4314