[debugger] Fix crash during Async Break when APC and CET are enabled

[thaystg]

When we are running an APC Callback we are not allowed to SetIP because when APC Callback is resuming it will check if the IP is not changed if CET is enabled.
To avoid this problem we use the APC to suspend the thread, but then we enable the single step and continue the thread execution, this will exit the apc callback and pause in the single step, so we are allowed to SetIP to FuncEvalHijack to run FuncEvals.

Fixes #110552

[janvorli]

A nit - can you please keep this function pointer unchanged and add the following function instead?

void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext)
{
    HandleSuspensionForInterruptedThread(interruptedContext, false);
}

[janvorli]

Looking at this and the call site, it might be nicer to pass in the ActivationReason instead of bool.

[noahfalk]

If I understand correctly, the pause will occur after executing forward one instruction in the jitted managed code? For example if the CONTEXT where the debugger initially suspended had IP = 0x7ffe1000, and code:
0x7ffe1000: mov ecx, 5
0x7ffe1005: mov rax, 0x27c3ad47000
0x7ffe100f: jmp qword ptr [rax]

Once we set the single step bit, resume, and get notified that the single step has completed will the IP now be 0x7ffe1005 and we've executed that first mov instruction? If so that could be problematic because executing that instruction may have changed what line of source we are on, changed the value of variables, or shifted the thread from a GC-safe-point to a non-GC-safe-point.

If that is the case, a couple potential alternatives:

1. Perhaps we don't need to hijack at all and we could run the func-eval from inside the APC? The purpose of the hijack is to get control of the thread to run what code we wanted to run and the APC has already done that for us. To make this work we'd have to be sure that stack working works correctly across the APC frame and that it is legal to recursively nest APCs on the stack.
2. Instead of a single-step maybe we could use a breakpoint to stop execution instead?
3. In the worst case, we might need to disallow func-eval on threads that were suspended by APC but I'm hoping that wouldn't impact most func-evals that debuggers do. Debuggers commonly pick the thread which sent the debug event to run the func-eval and that thread shouldn't have needed the APC to transition because it was already getting sent into some exception handler or runtime helper that triggered the debug event.

[thaystg]

What is the problem of changing the line of source we are on, or change the value of the variables? When we ask for an APC we don't know exactly when it will be executed right? What is the problem in pausing not exactly where it was paused by the APC call, but on the next instruction?

[janvorli]

The APC serves us for suspending the thread. It is delivered to it at "random" place. However, with the single step issue that Noah has raised, there can be a problem. When we get the APC callback (the ApcActivationCallback is called), we first check if the context instruction pointer is at activation safe point and we don't call the HandleSuspensionForInterruptedThread if it was not. The check for the activation safe point is here:

runtime/src/coreclr/vm/threadsuspend.cpp

Lines 5728 to 5745 in dda0d4e

	BOOL CheckActivationSafePoint(SIZE_T ip)
	{
	Thread *pThread = GetThreadNULLOk();
	// The criteria for safe activation is to be running managed code.
	// Also we are not interested in handling interruption if we are already in preemptive mode nor if we are single stepping
	BOOL isActivationSafePoint = pThread != NULL &&
	(pThread->m_StateNC & Thread::TSNC_DebuggerIsStepping) == 0 &&
	pThread->PreemptiveGCDisabled() &&
	ExecutionManager::IsManagedCode(ip);
	if (!isActivationSafePoint)
	{
	pThread->m_hasPendingActivation = false;
	}
	return isActivationSafePoint;
	}

Now with this single step mechanism, we could end up moving the instruction pointer to a location that's not activation safe in case the instruction was a call or jump to native code.