Repository navigation
Disabling the uop optimizer leads to crashes #127083
Description
Crash report
When running:
PYTHON_UOPS_OPTIMIZE=0 PYTHON_LLTRACE=1 ./python -m test test_multiprocessing_fork.test_processes -m test_free_from_gc
On a debug build, we get an assertion failure:
Using random seed: 2033438694
0:00:00 load avg: 1.57 Run 1 test sequentially in a single process
0:00:00 load avg: 1.57 [1/1] test_multiprocessing_fork.test_processes
Created a proto-trace for _path_join (<frozen importlib._bootstrap_external>:131) at byte offset 110 -- length 29
Created a proto-trace for _path_join (<frozen importlib._bootstrap_external>:131) at byte offset 116 -- length 13
Created a proto-trace for Heap._free_pending_blocks (/home/brandtbucher/cpython/Lib/multiprocessing/heap.py:258) at byte offset 126 -- length 482
Created a proto-trace for _TestHeap.test_free_from_gc (/home/brandtbucher/cpython/Lib/test/_test_multiprocessing.py:4114) at byte offset 486 -- length 27
Created a proto-trace for BufferWrapper.__init__ (/home/brandtbucher/cpython/Lib/multiprocessing/heap.py:326) at byte offset 0 -- length 516
python: Python/optimizer.c:1277: uop_optimize: Assertion `length <= UOP_MAX_TRACE_LENGTH' failed.
Fatal Python error: Aborted
Current thread 0x00007f6a0dc56740 (most recent call first):
File "/home/brandtbucher/cpython/Lib/multiprocessing/heap.py", line 326 in __init__
File "__init__", line ??? in __init__
File "/home/brandtbucher/cpython/Lib/test/_test_multiprocessing.py", line 4130 in test_free_from_gc
File "/home/brandtbucher/cpython/Lib/unittest/case.py", line 606 in _callTestMethod
File "/home/brandtbucher/cpython/Lib/unittest/case.py", line 660 in run
Fatal Python error: Segmentation fault
Segmentation fault (core dumped)
On a non-debug build, we get a buffer overflow:
Using random seed: 1659198442
0:00:00 load avg: 13.83 Run 1 test sequentially in a single process
0:00:00 load avg: 13.83 [1/1] test_multiprocessing_fork.test_processes
*** stack smashing detected ***: terminated
Fatal Python error: Aborted
Current thread 0x00007f11f9a27740 (most recent call first):
File "/home/brandtbucher/cpython/Lib/multiprocessing/heap.py", line 326 in __init__
File "__init__", line ??? in __init__
File "/home/brandtbucher/cpython/Lib/test/_test_multiprocessing.py", line 4130 in test_free_from_gc
File "/home/brandtbucher/cpython/Lib/unittest/case.py", line 606 in _callTestMethod
File "/home/brandtbucher/cpython/Lib/unittest/case.py", line 660 in run
File Fatal Python error: Segmentation fault
Segmentation fault (core dumped)
It looks like the optimizer just happens to do enough useful work that adding error and exit stubs doesn't put us over UOP_MAX_TRACE_LENGTH anymore. My hunch is that the real bug is in translate_bytecode_to_trace, where we're not reserving enough space for some instruction(s).
CC @Fidget-Spinner and @markshannon.
Activity
JeffersGlass commentedon Nov 21, 2024
For what it's worth, there seem to be a number of tests which cause this failure. Below is a list of tests, and the final UOp that fails to be added, with the message
No room for (UOP) (need X, got Y).Note that a few of the tests don't seem to show this kind of error, but instead underflow the trace stack.
JeffersGlass commentedon Nov 21, 2024
Here's a simpler (if slightly cursed) reproducer:
brandtbucher commentedon Nov 21, 2024
Thanks @JeffersGlass!
I talked with @markshannon, and we think it's probably worth reworking
translate_bytecode_to_traceto be less error-prone. The problem is that we need to reserve enough space for the next bytecode's uop expansion before translating it, which is difficult to get right and difficult to check. Rather, we should be able to gracefully handle the case where we run out of space in the middle of translating an instruction, by reverting to a known safepoint (the last complete expansion) and ending the trace there.Here's some pseudocode to capture the general idea:
JeffersGlass commentedon Nov 22, 2024
This makes sense to me!
I think the current crash is related to
prepare_for_execution, where 'spare' positions at the end of the trace are used to make implicit deopts into explicit ones, without verifying that there's actually enough spare space at the end of the buffer. For instance,foo.pyhas 590 ops in its buffer before preparation, but all the possible DEOPTs mean it has 839 opsprepare_for_execution, and the assertion fails/buffer overflows.So probably some bounds checking is necessary inside the loop... what would the desired behavior be if we run out of room at the end of the buffer during this step?
brandtbucher commentedon Nov 22, 2024
I think it makes sense to check during trace projection, rather than after. During trace projection, we know whether or not a given instruction is only partly-translated when we run out of space (which we need to avoid). If we run out of space, too, we can still successfully create a trace.
By the time we’re inserting exit/error stubs, in my opinion it’s too late. There’s a calculation that’s off during projection, and the fact that it doesn’t manifest until we insert stubs is a symptom, not a cause.
Fidget-Spinner commentedon Jan 4, 2025
I think we need to check at both spots. Exit/error stubs can become quite large with the partial evaluator. So we need to check while doing the initial trace, and once while inserting stubs too.
Fidget-Spinner commentedon Jan 4, 2025
Thought about this some more, and I don't think
translate_bytecode_to_traceneeds reworking. Jeff is probably right that it's where we insert exit stubs that's the bug. I will fix this along with when we get a partial evaluator (as that will now be responsible for inserting side exits).Fidget-Spinner commentedon Jan 4, 2026
This should be done thanks to @corona10