Skip to content

Commit

Permalink
[release/9.0-staging] [debugger] Fix a step that becomes a go (#110533)
Browse files Browse the repository at this point in the history
* Fixing step becomes a go

* Trying to avoid step that becomes a go

* Adding comments and more protections.

* fixing comment

* Checking if removing this comments, CI failures are gone.

* Adding part of the changes to understand the failures on CI

* Update src/coreclr/debug/ee/controller.cpp

Co-authored-by: mikelle-rogers <[email protected]>

* Fixing wrong fix.

---------

Co-authored-by: Thays Grazia <[email protected]>
Co-authored-by: Thays Grazia <[email protected]>
Co-authored-by: mikelle-rogers <[email protected]>
  • Loading branch information
4 people authored Dec 11, 2024
1 parent 83db46b commit 23d6092
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 4 deletions.
10 changes: 9 additions & 1 deletion src/coreclr/debug/ee/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7410,7 +7410,15 @@ bool DebuggerStepper::TriggerSingleStep(Thread *thread, const BYTE *ip)
if (!g_pEEInterface->IsManagedNativeCode(ip))
{
LOG((LF_CORDB,LL_INFO10000, "DS::TSS: not in managed code, Returning false (case 0)!\n"));
DisableSingleStep();
// Sometimes we can get here with a callstack that is coming from an APC
// this will disable the single stepping and incorrectly resume an app that the user
// is stepping through.
#ifdef FEATURE_THREAD_ACTIVATION
if ((thread->m_State & Thread::TS_DebugWillSync) == 0)
#endif
{
DisableSingleStep();
}
return false;
}

Expand Down
12 changes: 9 additions & 3 deletions src/coreclr/vm/threadsuspend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5746,8 +5746,9 @@ 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.
BOOL isActivationSafePoint = pThread != NULL &&
// 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);

Expand Down Expand Up @@ -5932,7 +5933,12 @@ bool Thread::InjectActivation(ActivationReason reason)
{
return true;
}

// Avoid APC calls when the thread is in single step state to avoid any
// wrong resume because it's running a native code.
if ((m_StateNC & Thread::TSNC_DebuggerIsStepping) != 0)
{
return false;
}
#ifdef FEATURE_SPECIAL_USER_MODE_APC
_ASSERTE(UseSpecialUserModeApc());

Expand Down

0 comments on commit 23d6092

Please sign in to comment.