Do not reset size of TailCallArgBuffer#118365
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in the tail call buffer implementation where the State field was being incorrectly set using a native integer operation instead of a 32-bit integer operation, causing unintended resets of the Size field and unnecessary buffer re-allocations.
Key changes:
- Changes
EmitSTIND_I()toEmitSTIND_I4()when setting the TailCallArgBuffer state to ensure only the 32-bit State field is modified
|
Tagging subscribers to this area: @mangod9 |
|
This should fix the stress problem hit by the CI. I have noticed that the thread in the bad state is always in the CFG probe called from There is still a corner case stress bug hidden somewhere. It is likely specific to combination of WinServer 2016 w/ CFG (and potentially Debug CRT). I suspect that it may be in special casing of CFG inside SetThreadContext in the OS. It should be rare enough that we can live with it. |
TailCallArgBuffer:State is 32-bit int. The code incorrectly used native int operation to set. It lead to TailCallArgBuffer::Size to be reset as well and needlessly re-allocating the tail call buffer. Fixes dotnet#118166
|
I have realized that we can call malloc/free in pre-emptive mode to make this even more robust, going to push an update in a bit. |
…ocation Move the fast path of the TailCallArgBuffer allocation helper to C# to avoid perf overload of switching to preemptive mode. Also, aggressively inline it to allow zero initialization to be unrolled by the JIT.
|
This is ready for review again |
TailCallArgBuffer:State is 32-bit int. The code incorrectly used native int operation to set. It lead to TailCallArgBuffer::Size to be reset as well and needlessly re-allocating the tail call buffer.
Fixes #118166