Repository navigation
test_capi.test_basic_loop(): _PyInstruction_GetLength() assertion error on s390x Fedora Clang 3.x buildbot #107082
Description
Activity
@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.)
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 :-(
My theory about how the debug fragment
or 1would cause the C assertion failure is flawed -- the code inPyUnstable_GetExecutorthat calls_PyInstruction_GetLengthalways starts at the first instruction and only ever calls that function with a valid starting point, soopcodeshould 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
structis supposed to always placecodein the first byte andargin the second byte. But maybe Clang on s390x somehow has its own ideas? If somehow the bytes were swapped (andargcame beforecode, or some weird padding occurred) I could easily see how we'd end up withopcode == 0, failing the assert.CC @markshannon
Is the issue fixed or not? This issue was closed.
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).
The buildbot still fails, so I reopen the issue: https://buildbot.python.org/all/#/builders/3/builds/4314
Reacted by MauriceThanks to the GCC Farm, I got access to a PPC64 big endian machine. I hacked PyCodeObject to dump the optimized bytecode:
test_pop_jump_if_not_none (test.test_capi.test_misc.TestUops.test_pop_jump_if_not_none) ... co_code: 0 RESUME 0 2 LOAD_FAST 0 4 GET_ITER >> 6 FOR_ITER 9 (to 28) 10 STORE_FAST 1 12 LOAD_FAST 1 14 POP_JUMP_IF_NOT_NONE 2 (to 20) 16 JUMP_BACKWARD 7 (to 6) >> 20 LOAD_CONST 1 22 STORE_FAST 1 24 JUMP_BACKWARD 11 (to 6) >> 28 END_FOR 30 RETURN_CONST 0 co_code_opt: 0 RESUME 0 2 LOAD_FAST 0 4 GET_ITER >> 6 FOR_ITER_LIST 9 (to 28) 10 STORE_FAST 1 12 LOAD_FAST 1 14 POP_JUMP_IF_NOT_NONE 2 (to 20) 16 JUMP_BACKWARD 7 (to 6) >> 20 LOAD_CONST 1 22 STORE_FAST 1 24 ENTER_EXECUTOR 0 (to 26) >> 26 CACHE >> 28 END_FOR 30 RETURN_CONST 0 GetExecutor: 0x7fff8bd17e40 - query offset=16 GetExecutor: i=0, i*2=0, code=151, arg=0 GetExecutor: i=1, i*2=2, code=124, arg=0 GetExecutor: i=2, i*2=4, code=68, arg=0 GetExecutor: i=3, i*2=6, code=111, arg=9 GetExecutor: i=5, i*2=10, code=125, arg=1 GetExecutor: i=6, i*2=12, code=124, arg=1 GetExecutor: i=7, i*2=14, code=128, arg=2 GetExecutor: i=8, i*2=16, code=140, arg=7 GetExecutor: i=10, i*2=20, code=100, arg=1 GetExecutor: i=11, i*2=22, code=125, arg=1 GetExecutor: i=12, i*2=24, code=230, arg=0 GetExecutor: i=13, i*2=26, code=0, arg=1 python: Python/instrumentation.c:262: _PyInstruction_GetLength: Assertion `opcode != 0' failed.The assertion occurs in PyUnstable_GetExecutor() at the
CACHEinstruction (i=13, i*2=26, code=0, arg=1).On little endian x86-64, I get:
test_pop_jump_if_not_none (test.test_capi.test_misc.TestUops.test_pop_jump_if_not_none) ... co_code: 0 RESUME 0 2 LOAD_FAST 0 4 GET_ITER >> 6 FOR_ITER 9 (to 28) 10 STORE_FAST 1 12 LOAD_FAST 1 14 POP_JUMP_IF_NOT_NONE 2 (to 20) 16 JUMP_BACKWARD 7 (to 6) >> 20 LOAD_CONST 1 22 STORE_FAST 1 24 JUMP_BACKWARD 11 (to 6) >> 28 END_FOR 30 RETURN_CONST 0 co_code_opt: 0 RESUME 0 2 LOAD_FAST 0 4 GET_ITER >> 6 FOR_ITER_LIST 9 (to 28) 10 STORE_FAST 1 12 LOAD_FAST 1 14 POP_JUMP_IF_NOT_NONE 2 (to 20) 16 JUMP_BACKWARD 7 (to 6) >> 20 LOAD_CONST 1 22 STORE_FAST 1 24 ENTER_EXECUTOR 0 (to 26) >> 26 POP_TOP >> 28 END_FOR 30 RETURN_CONST 0 GetExecutor: 0x7fea3b387e40 - query offset=16 GetExecutor: i=0, i*2=0, code=151, arg=0 GetExecutor: i=1, i*2=2, code=124, arg=0 GetExecutor: i=2, i*2=4, code=68, arg=0 GetExecutor: i=3, i*2=6, code=111, arg=9 GetExecutor: i=5, i*2=10, code=125, arg=1 GetExecutor: i=6, i*2=12, code=124, arg=1 GetExecutor: i=7, i*2=14, code=128, arg=2 GetExecutor: i=8, i*2=16, code=140, arg=7 GetExecutor: i=10, i*2=20, code=100, arg=1 GetExecutor: i=11, i*2=22, code=125, arg=1 GetExecutor: i=12, i*2=24, code=230, arg=0 GetExecutor: i=13, i*2=26, code=1, arg=0 GetExecutor: i=14, i*2=28, code=4, arg=0 GetExecutor: i=15, i*2=30, code=121, arg=0Here in the optimized code, the
ENTER_EXECUTORinstruction is followed by:POP_TOP,END_FORandRETURN_CONST. There is noCACHEinstruction.Thanks, that's super helpful! It seems that in
_PyInstruction_GetLength()the instruction length for the instruction containingENTER_EXECUTORshould dig the underlyingJUMP_BACKWARDinstruction and base the instruction width on that. This is a bit unfortunate but we have to do it.On ppc64, when _PyOptimizer_BackEdge() is called,
code->co_code_adaptive[24]is 140 (JUMP_BACKWARD) as expected, butcode->co_code_adaptive[26]is 0 andcode->co_code_adaptive[27]is 1 (can it be thePOP_TOPinstruction?).It seems like the code was modified already and when POP_TOP was written, it was written in the wrong endian: {0, POP_TOP} instead of {POP_TOP, 0}.
Which code is responsible to write POP_TOP?
There is not really a
POP_TOPinstruction. There is a cache entry immediately after theJUMP_BACKWARDinstruction containing a 16-bit counter that has the value 1. This cache entry is for counting how many times the jump has been executed, to decide whether to replace theJUMP_BACKWARDinstruction with anENTER_EXECUTORinstruction. On little-endian machines, the low byte of the counter is seen as the opcode, which is typically a valid instruction (but nonsensical in context). 1 is indeedPOP_TOP(see opcode.py).On a big-endian machine, the high byte of the counter is seen as the opcode, and as it is zero it triggers the assert.
The length calculation doesn't understand (yet) that it should special-case
ENTER_EXECUTOR, so the length of theENTER_EXECUTORis seen as 1 unit rather than 2 (the length calculated must include the cache size), and on the next call we are trying to decode the cache entry as if it was an instruction. See my fix in gh-107256 for details.There is not really a POP_TOP instruction. There is a cache entry immediately after the JUMP_BACKWARD instruction containing a 16-bit counter that has the value 1.
Ah right. Using hardware watchpoint in gdb I found
here[1].cache += (1 << OPTIMIZER_BITS_IN_COUNTER);write inTARGET(JUMP_BACKWARD). I understood that it's not really an instruction and I noticed that dis.dis() gently hides this cache :-)I found other code that could potentially run into the same problem. There's a lot of code that walks over code, instruction by instruction, using
opcode = _PyOpcode_Deopt[opcode]and thenindex += _PyOpcode_Caches[opcode]. I opened gh-107265 for this.
s390x Fedora Clang 3.x: https://buildbot.python.org/all/#/builders/3/builds/4312
Differences between the two builds:
Error:
cc @gvanrossum
Linked PRs