GH-128682: Stronger checking of PyStackRef_CLOSE and DEAD.#128683
GH-128682: Stronger checking of PyStackRef_CLOSE and DEAD.#128683markshannon merged 5 commits intopython:mainfrom
PyStackRef_CLOSE and DEAD.#128683Conversation
DEAD and DECREF_INPUTSPyStackRef_CLOSE and DEAD.
brandtbucher
left a comment
There was a problem hiding this comment.
My impression was that, rather than enforcing a closing order, we would instead have the code generator insert NULL assignments to the stack where necessary. I vaguely remember situations where that would be really useful, such as when pushing or popping multiple stack items that are created or freed in the "wrong order", or where overlapping lifetimes would force us to convert things to heap references (and back again) to move things around on the stack.
Are we abandoning that idea, or is this a step towards that?
(Either way, the code in this PR seems correct.)
| DEOPT_IF(!PyType_Check(callable_o)); | ||
| PyTypeObject *tp = (PyTypeObject *)callable_o; | ||
| int total_args = oparg; | ||
| _PyStackRef *arguments = args; |
There was a problem hiding this comment.
What's the reasoning behind this alias? I see we do this in a couple of places now...
If the reasoning is that the analysis chokes on array accesses, maybe we should teach it to ignore them?
There was a problem hiding this comment.
We modify arguments, but the code generator assumes that args is a fixed array on the stack.
| "RELOAD_STACK": self.reload_stack, | ||
| "PyStackRef_CLOSE": self.stackref_close, | ||
| "PyStackRef_CLOSE_SPECIALIZED": self.stackref_close, | ||
| "PyStackRef_CLOSE_SPECIALIZED": self.stackref_close_no_escape, |
There was a problem hiding this comment.
Would it be useful to have this one "inherit" it's escaping-ness from its second arg? I could see us adding something for tuples or whatever and breaking this by accident.
That only applies when several references are removed from the stack at once, by If references are popped individually, they must be popped in the right order. |
Stronger checking of the stack state for
PyStackRef_CLOSE. Only allow references to be closed from top to bottom, to ensure stack is valid when closing a reference.Fix up bytecodes.c accordingly.