GH-118074: Executors in the COLD_EXITS array are not GC'able #118117
Conversation
Implement a `tp_is_gc` slot that tests for this.
|
@adoxalim Can you verify that this fix works for you? (Thanks again for the bug report.) |
Python/optimizer.c
Outdated
| static int | ||
| executor_is_gc(PyObject *o) | ||
| { | ||
| if ((PyObject *)&COLD_EXITS[0] <= o && o < (PyObject *)&COLD_EXITS[UOP_MAX_TRACE_LENGTH]) { |
There was a problem hiding this comment.
The address sanitizer warning here seems plausibly correct:
array subscript 800 is above array bounds of ‘_PyExecutorObject[200]’ [-Warray-bounds]
COLD_EXITS has a length of UOP_MAX_TRACE_LENGTH/4. If there's some other reason this is correct, maybe add a comment / a way to suppress the warning?
There was a problem hiding this comment.
Oh, you're right. That line was written by Copilot and it looked so plausible but was so wrong. :-(
|
The emulated Linux (ARM64) test failures are known, caused by the the QEMU emulation. I'm merging now. |
| static int | ||
| executor_is_gc(PyObject *o) | ||
| { | ||
| if ((PyObject *)&COLD_EXITS[0] <= o && o < (PyObject *)&COLD_EXITS[COLD_EXIT_COUNT]) { |
There was a problem hiding this comment.
This is undefined behavior in C. Although it will mostly work in practice.
There was a problem hiding this comment.
So I take it this should work?
| if ((PyObject *)&COLD_EXITS[0] <= o && o < (PyObject *)&COLD_EXITS[COLD_EXIT_COUNT]) { | |
| if (_Py_IsImmortal(o)) { |
There was a problem hiding this comment.
Maybe add a similar thing to the top of executor_traverse()? (Although, really, cold exits don't have exits, so their exit_count should always be zero, right? So why bother.
Anyway, see gh-118182 for a fix.
|
Sorry for not reviewing this earlier. I'd rather not use As a short term fix, we could change |
Implement a
tp_is_gcslot that tests for this.