Skip to content

Commit 12e5b3b

Browse files
BrzVladCopilot
andauthored
[clr-interp] Fix handling of continuation return when it is not used by the method (#128403)
Consider the scenario where we are emitting an async method call where the return value is not used at all by the method and, by the time we are actually doing the suspend operation, a new live variable is created. This means that the new variable could share the same offset as the return offset. Example interp IR: ``` IR_0074: call [64 <- 96], .Program:ThrowsAsync[System.__Canon](System.Func`1[System.Threading.Tasks.Task]) // var at offset 56 was present on the interp execution stack and it gets moved to a temporary that shares the same offset with the dead return // this means that, when we restore the context and write the continuation return to the interp stack, we overwritte this IR_0078: mov.8 [64 <- 56], IR_007b: handle.continuation [72 <- nil], AsyncSuspendData[continuationTypeHnd=0x7fff7980c310, flags=3144, liveLocalsIntervals=[{64, 71},], zeroedLocalsIntervals=[], keepAliveOffset=32], 0x7fff797479c0 (CORINFO_HELP_ALLOC_CONTINUATION)(indirect) IR_007f: capture.context.on.suspend [80 <- 72], AsyncSuspendData[continuationTypeHnd=0x7fff7980c310, flags=3144, liveLocalsIntervals=[{64, 71},], zeroedLocalsIntervals=[], keepAliveOffset=32] IR_0083: restore.contexts.on.suspend [72 <- 72 0 16], AsyncSuspendData[continuationTypeHnd=0x7fff7980c310, flags=3144, liveLocalsIntervals=[{64, 71},], zeroedLocalsIntervals=[], keepAliveOffset=32] IR_0089: handle.continuation.suspend [nil <- 72], AsyncSuspendData[continuationTypeHnd=0x7fff7980c310, flags=3144, liveLocalsIntervals=[{64, 71},], zeroedLocalsIntervals=[], keepAliveOffset=32] IR_008c: handle.continuation.resume [nil <- nil], AsyncSuspendData[continuationTypeHnd=0x7fff7980c310, flags=3144, liveLocalsIntervals=[{64, 71},], zeroedLocalsIntervals=[], keepAliveOffset=32] IR_008e: ldind.i8 [96 <- 64], 8 ``` This bug was exposed by #127931, which changed the order of the write to the result var, during resume. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent af9da6d commit 12e5b3b

2 files changed

Lines changed: 14 additions & 2 deletions

File tree

src/coreclr/interpreter/compiler.cpp

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6451,7 +6451,19 @@ void InterpCompiler::UpdateLocalIntervalMaps()
64516451
int32_t varIndex = suspendData->returnValueVarStackOffset;
64526452
if (varIndex != -1)
64536453
{
6454-
suspendData->returnValueVarStackOffset = m_pVars[varIndex].offset;
6454+
assert(!m_pVars[varIndex].global);
6455+
if (m_pVars[varIndex].liveStart == m_pVars[varIndex].liveEnd)
6456+
{
6457+
// The return result of the async call is not used by the method, so the allocated
6458+
// stack offset will be immediately reused by any vars that become live. Keep the
6459+
// continuation layout unchanged, but mark the return value stack offset as invalid
6460+
// so resume skips copying the stored result back to the interpreter stack.
6461+
suspendData->returnValueVarStackOffset = -1;
6462+
}
6463+
else
6464+
{
6465+
suspendData->returnValueVarStackOffset = m_pVars[varIndex].offset;
6466+
}
64556467
}
64566468
}
64576469
}

src/coreclr/vm/interpexec.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4581,7 +4581,7 @@ do \
45814581

45824582
// Explicitly copy the return value from the continuation's result storage
45834583
// to the interpreter stack.
4584-
if (pAsyncSuspendData->returnValueContinuationDataSize > 0)
4584+
if (pAsyncSuspendData->returnValueVarStackOffset != -1)
45854585
{
45864586
memcpy(LOCAL_VAR_ADDR(pAsyncSuspendData->returnValueVarStackOffset, uint8_t),
45874587
continuation->GetResultStorage(),

0 commit comments

Comments
 (0)