Refactor stepping code, fix stepping over a breakpoint

src/ARMeilleure/State/ExecutionContext.cs

@@ -98,18 +98,19 @@ namespace ARMeilleure.State
 
         private readonly ExceptionCallbackNoArgs _interruptCallback;
         private readonly ExceptionCallback _breakCallback;
+        private readonly ExceptionCallbackNoArgs _stepCallback;
         private readonly ExceptionCallback _supervisorCallback;
         private readonly ExceptionCallback _undefinedCallback;
 
         internal int ShouldStep;
         public ulong DebugPc { get; set; }
-        public Barrier StepBarrier { get; }
 
         public ExecutionContext(
             IJitMemoryAllocator allocator,
             ICounter counter,
             ExceptionCallbackNoArgs interruptCallback = null,
             ExceptionCallback breakCallback = null,
+            ExceptionCallbackNoArgs stepCallback = null,
             ExceptionCallback supervisorCallback = null,
             ExceptionCallback undefinedCallback = null)
         {
@@ -117,11 +118,11 @@ namespace ARMeilleure.State
             _counter = counter;
             _interruptCallback = interruptCallback;
             _breakCallback = breakCallback;
+            _stepCallback = stepCallback;
             _supervisorCallback = supervisorCallback;
             _undefinedCallback = undefinedCallback;
 
             Running = true;
-            StepBarrier = new Barrier(2);
 
             _nativeContext.SetCounter(MinCountForCheck);
         }
@@ -155,6 +156,11 @@ namespace ARMeilleure.State
             Interrupted = true;
         }
 
+        public void StepHandler()
+        {
+            _stepCallback.Invoke(this);
+        }
+        
         public void RequestDebugStep()
         {
             Interlocked.Exchange(ref ShouldStep, 1);

src/ARMeilleure/Translation/Translator.cs

@@ -148,8 +148,7 @@ namespace ARMeilleure.Translation
                     if (Interlocked.CompareExchange(ref context.ShouldStep, 0, 1) == 1)
                     {
                         context.DebugPc = Step(context, context.DebugPc);
-                        context.StepBarrier.SignalAndWait();
+                        context.StepHandler();
-                        context.StepBarrier.SignalAndWait();
                     }
                     else
                     {

src/Ryujinx.Cpu/AppleHv/HvExecutionContext.cs

@@ -93,7 +93,6 @@ namespace Ryujinx.Cpu.AppleHv
             _shadowContext = new HvExecutionContextShadow();
             _impl = _shadowContext;
             _exceptionCallbacks = exceptionCallbacks;
-            StepBarrier = new(2);
             Running = true;
         }
 
@@ -119,6 +118,11 @@ namespace Ryujinx.Cpu.AppleHv
             _exceptionCallbacks.BreakCallback?.Invoke(this, address, imm);
         }
 
+        private void StepHandler()
+        {
+            _exceptionCallbacks.StepCallback?.Invoke(this);
+        }
+
         private void SupervisorCallHandler(ulong address, int imm)
         {
             _exceptionCallbacks.SupervisorCallback?.Invoke(this, address, imm);
@@ -167,8 +171,6 @@ namespace Ryujinx.Cpu.AppleHv
             }
         }
 
-        public Barrier StepBarrier { get; }
-
         /// <inheritdoc/>
         public void StopRunning()
         {
@@ -272,9 +274,7 @@ namespace Ryujinx.Cpu.AppleHv
                     HvApi.hv_vcpu_set_sys_reg(vcpuHandle, HvSysReg.SPSR_EL1, spsr).ThrowOnError();
                     HvApi.hv_vcpu_set_sys_reg(vcpuHandle, HvSysReg.MDSCR_EL1, 0);
                     ReturnToPool(vcpu);
-                    StepBarrier.SignalAndWait();
+                    StepHandler();
-                    StepBarrier.SignalAndWait();
-                    InterruptHandler();
                     vcpu = RentFromPool(memoryManager.AddressSpace, vcpu);
                     break;
                 case ExceptionClass.BrkAarch64:

src/Ryujinx.Cpu/ExceptionCallbacks.cs

@@ -29,6 +29,11 @@ namespace Ryujinx.Cpu
         /// </summary>
         public readonly ExceptionCallback BreakCallback;
 
+        /// <summary>
+        /// Handler for CPU software interrupts caused by single-stepping.
+        /// </summary>
+        public readonly ExceptionCallbackNoArgs StepCallback;
+
         /// <summary>
         /// Handler for CPU software interrupts caused by the Arm SVC instruction.
         /// </summary>
@@ -47,16 +52,19 @@ namespace Ryujinx.Cpu
         /// </remarks>
         /// <param name="interruptCallback">Handler for CPU interrupts triggered using <see cref="IExecutionContext.RequestInterrupt"/></param>
         /// <param name="breakCallback">Handler for CPU software interrupts caused by the Arm BRK instruction</param>
+        /// <param name="stepCallback">Handler for CPU software interrupts caused by single-stepping</param>
         /// <param name="supervisorCallback">Handler for CPU software interrupts caused by the Arm SVC instruction</param>
         /// <param name="undefinedCallback">Handler for CPU software interrupts caused by any undefined Arm instruction</param>
         public ExceptionCallbacks(
             ExceptionCallbackNoArgs interruptCallback = null,
             ExceptionCallback breakCallback = null,
+            ExceptionCallbackNoArgs stepCallback = null,
             ExceptionCallback supervisorCallback = null,
             ExceptionCallback undefinedCallback = null)
         {
             InterruptCallback = interruptCallback;
             BreakCallback = breakCallback;
+            StepCallback = stepCallback;
             SupervisorCallback = supervisorCallback;
             UndefinedCallback = undefinedCallback;
         }

src/Ryujinx.Cpu/IExecutionContext.cs

@@ -121,20 +121,10 @@ namespace Ryujinx.Cpu
         /// <remarks>
         /// The thread might not pause immediately.
         /// One must not assume that guest code is no longer being executed by the thread after calling this function.
-        /// After single stepping, the thread should signal and wait on <see cref="StepBarrier"/> twice to allow
+        /// After single stepping, the thread should call call <see cref="ExceptionCallbacks.StepCallback"/>.
-        /// changing the thread state after stepping.
         /// </remarks>
         void RequestDebugStep();
 
-        /// <summary>
-        /// Step barrier
-        /// </summary>
-        /// <remarks>
-        /// The participant count should be 2.
-        /// Should be signaled and waited on twice after single-stepping.
-        /// </remarks>
-        Barrier StepBarrier { get; }
-
         /// <summary>
         /// Current Program Counter (for debugging).
         /// </summary>

src/Ryujinx.Cpu/Jit/JitExecutionContext.cs

@@ -74,6 +74,7 @@ namespace Ryujinx.Cpu.Jit
                 counter,
                 InterruptHandler,
                 BreakHandler,
+                StepHandler,
                 SupervisorCallHandler,
                 UndefinedHandler);
 
@@ -102,6 +103,11 @@ namespace Ryujinx.Cpu.Jit
             _exceptionCallbacks.BreakCallback?.Invoke(this, address, imm);
         }
 
+        private void StepHandler(ExecutionContext context)
+        {
+            _exceptionCallbacks.StepCallback?.Invoke(this);
+        }
+        
         private void SupervisorCallHandler(ExecutionContext context, ulong address, int imm)
         {
             _exceptionCallbacks.SupervisorCallback?.Invoke(this, address, imm);
@@ -128,9 +134,6 @@ namespace Ryujinx.Cpu.Jit
             set => _impl.DebugPc = value;
         }
 
-        /// <inheritdoc/>
-        public Barrier StepBarrier => _impl.StepBarrier;
-
         /// <inheritdoc/>
         public void StopRunning()
         {

src/Ryujinx.HLE/Debugger/Debugger.cs

@@ -737,34 +737,17 @@ namespace Ryujinx.HLE.Debugger
             }
         }
 
-        public void ThreadBreak(IExecutionContext ctx, ulong address, int imm)
+        public void BreakHandler(IExecutionContext ctx, ulong address, int imm)
         {
             Logger.Notice.Print(LogClass.GdbStub, $"Break hit on thread {ctx.ThreadUid} at pc {address:x016}");
 
             Messages.Add(new ThreadBreakMessage(ctx, address, imm));
+            DebugProcess.DebugInterruptHandler(ctx);
+        }
 
-            KThread currentThread = DebugProcess.GetThread(ctx.ThreadUid);
+        public void StepHandler(IExecutionContext ctx)
-
+        {
-            if (currentThread.Context.Running &&
+            DebugProcess.DebugInterruptHandler(ctx);
-                currentThread.Owner != null &&
-                currentThread.GetUserDisableCount() != 0 &&
-                currentThread.Owner.PinnedThreads[currentThread.CurrentCore] == null)
-            {
-                KernelContext.CriticalSection.Enter();
-
-                currentThread.Owner.PinThread(currentThread);
-
-                currentThread.SetUserInterruptFlag();
-
-                KernelContext.CriticalSection.Leave();
-            }
-
-            if (currentThread.IsSchedulable)
-            {
-                KernelContext.Schedulers[currentThread.CurrentCore].Schedule();
-            }
-
-            currentThread.HandlePostSyscall();
         }
     }
 }

src/Ryujinx.HLE/Debugger/IDebuggableProcess.cs

@@ -1,4 +1,5 @@
-using Ryujinx.HLE.HOS.Kernel.Threading;
+using Ryujinx.Cpu;
+using Ryujinx.HLE.HOS.Kernel.Threading;
 using Ryujinx.Memory;
 
 namespace Ryujinx.HLE.Debugger
@@ -11,6 +12,7 @@ namespace Ryujinx.HLE.Debugger
         KThread GetThread(ulong threadUid);
         DebugState GetDebugState();
         ulong[] GetThreadUids();
+        public void DebugInterruptHandler(IExecutionContext ctx);
         IVirtualMemoryManager CpuMemory { get; }
         void InvalidateCacheRegion(ulong address, ulong size);
     }

src/Ryujinx.HLE/HOS/Kernel/Process/KProcess.cs

@@ -13,6 +13,7 @@ using System.Collections.Generic;
 using System.Linq;
 using System.Threading;
 using ExceptionCallback = Ryujinx.Cpu.ExceptionCallback;
+using ExceptionCallbackNoArgs = Ryujinx.Cpu.ExceptionCallbackNoArgs;
 
 namespace Ryujinx.HLE.HOS.Kernel.Process
 {
@@ -733,15 +734,18 @@ namespace Ryujinx.HLE.HOS.Kernel.Process
         public IExecutionContext CreateExecutionContext()
         {
             ExceptionCallback breakCallback = null;
+            ExceptionCallbackNoArgs stepCallback = null;
 
             if (KernelContext.Device.Configuration.EnableGdbStub)
             {
-                breakCallback = KernelContext.Device.Debugger.ThreadBreak;
+                breakCallback = KernelContext.Device.Debugger.BreakHandler;
+                stepCallback = KernelContext.Device.Debugger.StepHandler;
             }
 
             return Context?.CreateExecutionContext(new ExceptionCallbacks(
                 InterruptHandler,
                 breakCallback,
+                stepCallback,
                 KernelContext.SyscallHandler.SvcCall,
                 UndefinedInstructionHandler));
         }
@@ -1189,14 +1193,17 @@ namespace Ryujinx.HLE.HOS.Kernel.Process
 
         private class DebuggerInterface : IDebuggableProcess
         {
+            private Barrier StepBarrier;
             private readonly KProcess _parent;
-            private readonly KernelContext KernelContext;
+            private readonly KernelContext _kernelContext;
+            private KThread steppingThread;
             private int _debugState = (int)DebugState.Running;
 
             public DebuggerInterface(KProcess p)
             {
                 _parent = p;
-                KernelContext = p.KernelContext;
+                _kernelContext = p.KernelContext;
+                StepBarrier = new(2);
             }
 
             public void DebugStop()
@@ -1207,7 +1214,7 @@ namespace Ryujinx.HLE.HOS.Kernel.Process
                     return;
                 }
 
-                KernelContext.CriticalSection.Enter();
+                _kernelContext.CriticalSection.Enter();
                 lock (_parent._threadingLock)
                 {
                     foreach (KThread thread in _parent._threads)
@@ -1219,7 +1226,7 @@ namespace Ryujinx.HLE.HOS.Kernel.Process
                 }
 
                 _debugState = (int)DebugState.Stopped; 
-                KernelContext.CriticalSection.Leave();
+                _kernelContext.CriticalSection.Leave();
             }
 
             public void DebugContinue()
@@ -1230,7 +1237,7 @@ namespace Ryujinx.HLE.HOS.Kernel.Process
                     return;
                 }
 
-                KernelContext.CriticalSection.Enter();
+                _kernelContext.CriticalSection.Enter();
                 lock (_parent._threadingLock)
                 {
                     foreach (KThread thread in _parent._threads)
@@ -1238,7 +1245,7 @@ namespace Ryujinx.HLE.HOS.Kernel.Process
                         thread.Resume(ThreadSchedState.ThreadPauseFlag);
                     }
                 }
-                KernelContext.CriticalSection.Leave();
+                _kernelContext.CriticalSection.Leave();
             }
 
             public bool DebugStep(KThread target)
@@ -1247,7 +1254,8 @@ namespace Ryujinx.HLE.HOS.Kernel.Process
                 {
                     return false;
                 }
-                KernelContext.CriticalSection.Enter();
+                _kernelContext.CriticalSection.Enter();
+                steppingThread = target;
                 bool waiting = target.MutexOwner != null || target.WaitingSync || target.WaitingInArbitration;
                 target.Context.RequestDebugStep();
                 if (waiting)
@@ -1264,12 +1272,12 @@ namespace Ryujinx.HLE.HOS.Kernel.Process
                 {
                     target.Resume(ThreadSchedState.ThreadPauseFlag);
                 }
-                KernelContext.CriticalSection.Leave();
+                _kernelContext.CriticalSection.Leave();
 
-                target.Context.StepBarrier.SignalAndWait();
+                StepBarrier.SignalAndWait();
-                target.Context.StepBarrier.SignalAndWait();
 
-                KernelContext.CriticalSection.Enter();
+                _kernelContext.CriticalSection.Enter();
+                steppingThread = null;
                 if (waiting)
                 {
                     lock (_parent._threadingLock)
@@ -1284,7 +1292,8 @@ namespace Ryujinx.HLE.HOS.Kernel.Process
                 {
                     target.Suspend(ThreadSchedState.ThreadPauseFlag);
                 }
-                KernelContext.CriticalSection.Leave();
+                _kernelContext.CriticalSection.Leave();
+                StepBarrier.SignalAndWait();
                 return true;
             }
 
@@ -1309,6 +1318,19 @@ namespace Ryujinx.HLE.HOS.Kernel.Process
                 }
             }
 
+            public void DebugInterruptHandler(IExecutionContext ctx)
+            {
+                _kernelContext.CriticalSection.Enter();
+                bool stepping = steppingThread != null;
+                _kernelContext.CriticalSection.Leave();
+                if (stepping)
+                {
+                    StepBarrier.SignalAndWait();
+                    StepBarrier.SignalAndWait();
+                }
+                _parent.InterruptHandler(ctx);
+            }
+
             public IVirtualMemoryManager CpuMemory { get { return _parent.CpuMemory; } }
 
             public void InvalidateCacheRegion(ulong address, ulong size)

src/Ryujinx.HLE/HOS/Kernel/Process/ProcessExecutionContext.cs

@@ -25,7 +25,6 @@ namespace Ryujinx.HLE.HOS.Kernel.Process
         private readonly ulong[] _x = new ulong[32];
 
         public ulong DebugPc { get; set; }
-        public Barrier StepBarrier { get; }
 
         public ulong GetX(int index) => _x[index];
         public void SetX(int index, ulong value) => _x[index] = value;