gh-118331: Fix a couple of issues when list allocation fails #130811
+26
−4
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes a couple of bugs that are triggered when list allocation fails. Specifically, when the list object is allocated
successfully, but allocation of the items array fails.
Use-after-free on the items array
We didn't set the items array pointer in the list object to
NULL
after freeing the items array. As a result, we could end up with a list object added to the free list that contained a pointer to an already-freed items array. A subsequent list allocation that successfully retrieved a list object from the free list but failed to allocate a new items would deallocate thelist object:
cpython/Objects/listobject.c
Lines 251 to 255 in bbf1979
list_dealloc
would then try to use the previously freed items array:cpython/Objects/listobject.c
Lines 526 to 536 in bbf1979
Incorrect stackpointer assertion
We check that either there is no Python code executing (frame is
NULL
) or the stack pointer for the current frame is set when executing_Py_Dealloc
:cpython/Objects/object.c
Lines 2987 to 2991 in bbf1979
I think the intent here is to catch places in the interpreter loop that escape due to decrefs where we aren't setting / clearing the stack pointer correctly (e.g. due to shortcomings in our analysis or escaping calls that are incorrectly marked as non-escaping). The assertion is overly conservative. It will catch all potentially escaping decrefs, but it will also catch decrefs that can never escape. In this case,
_PyList_FromStackRefStealOnSuccess
is, correctly, I think, marked as non-escaping. The only decref it can perform is on an exact list (not a subtype). However, it triggers this assertion.There are a few options I can see for fixing this:
_PyList_FromStackRefStealOnSuccess
as escaping.I went with (3) because it's correct, if pessimistic, and I don't think it'll impact performance too much. I think the assertion is worth keeping around and I'm not sure of a good way to do (2) generically. Happy to do something else if reviewers feel strongly otherwise.