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 commented on 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 commented on Nov 21, 2024
Here's a simpler (if slightly cursed) reproducer:
brandtbucher commented on Nov 21, 2024
Thanks @JeffersGlass!
I talked with @markshannon, and we think it's probably worth reworking translate_bytecode_to_trace to 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 commented on 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.py has 590 ops in its buffer before preparation, but all the possible DEOPTs mean it has 839 ops prepare_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 commented on 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 commented on 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 commented on Jan 4, 2025
Thought about this some more, and I don't think translate_bytecode_to_trace needs 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 commented on Jan 4, 2026
This should be done thanks to @corona10