core/debugger: Improved stepping mechanism and misc fixes

6 files changed, 87 insertions, 80 deletions

diff --git a/src/core/arm/arm_interface.cpp b/src/core/arm/arm_interface.cpp
index 1310f72bf..9b5a5ca57 100644
--- a/src/core/arm/arm_interface.cpp
+++ b/src/core/arm/arm_interface.cpp
@@ -11,6 +11,7 @@
 #include "core/core.h"
 #include "core/debugger/debugger.h"
 #include "core/hle/kernel/k_process.h"
+#include "core/hle/kernel/svc.h"
 #include "core/loader/loader.h"
 #include "core/memory.h"
 
@@ -89,8 +90,48 @@ void ARM_Interface::LogBacktrace() const {
     }
 }
 
-bool ARM_Interface::ShouldStep() const {
-    return system.DebuggerEnabled() && system.GetDebugger().IsStepping();
+void ARM_Interface::Run() {
+    using Kernel::StepState;
+    using Kernel::SuspendType;
+
+    while (true) {
+        Kernel::KThread* current_thread{system.Kernel().CurrentScheduler()->GetCurrentThread()};
+        Dynarmic::HaltReason hr{};
+
+        // Notify the debugger and go to sleep if a step was performed
+        // and this thread has been scheduled again.
+        if (current_thread->GetStepState() == StepState::StepPerformed) {
+            system.GetDebugger().NotifyThreadStopped(current_thread);
+            current_thread->RequestSuspend(SuspendType::Debug);
+            break;
+        }
+
+        // Otherwise, run the thread.
+        if (current_thread->GetStepState() == StepState::StepPending) {
+            hr = StepJit();
+
+            if (Has(hr, step_thread)) {
+                current_thread->SetStepState(StepState::StepPerformed);
+            }
+        } else {
+            hr = RunJit();
+        }
+
+        // Notify the debugger and go to sleep if a breakpoint was hit.
+        if (Has(hr, breakpoint)) {
+            system.GetDebugger().NotifyThreadStopped(current_thread);
+            current_thread->RequestSuspend(Kernel::SuspendType::Debug);
+            break;
+        }
+
+        // Handle syscalls and scheduling (this may change the current thread)
+        if (Has(hr, svc_call)) {
+            Kernel::Svc::Call(system, GetSvcNumber());
+        }
+        if (Has(hr, break_loop) || !uses_wall_clock) {
+            break;
+        }
+    }
 }
 
 } // namespace Core
diff --git a/src/core/arm/arm_interface.h b/src/core/arm/arm_interface.h
index 7842c626b..66f6107e9 100644
--- a/src/core/arm/arm_interface.h
+++ b/src/core/arm/arm_interface.h
@@ -6,6 +6,9 @@
 
 #include <array>
 #include <vector>
+
+#include <dynarmic/interface/halt_reason.h>
+
 #include "common/common_funcs.h"
 #include "common/common_types.h"
 #include "core/hardware_properties.h"
@@ -64,7 +67,7 @@ public:
     static_assert(sizeof(ThreadContext64) == 0x320);
 
     /// Runs the CPU until an event happens
-    virtual void Run() = 0;
+    void Run();
 
     /// Clear all instruction cache
     virtual void ClearInstructionCache() = 0;
@@ -191,7 +194,10 @@ public:
 
     void LogBacktrace() const;
 
-    bool ShouldStep() const;
+    static constexpr Dynarmic::HaltReason step_thread = Dynarmic::HaltReason::Step;
+    static constexpr Dynarmic::HaltReason break_loop = Dynarmic::HaltReason::UserDefined2;
+    static constexpr Dynarmic::HaltReason svc_call = Dynarmic::HaltReason::UserDefined3;
+    static constexpr Dynarmic::HaltReason breakpoint = Dynarmic::HaltReason::UserDefined4;
 
 protected:
     /// System context that this ARM interface is running under.
@@ -200,6 +206,10 @@ protected:
     bool uses_wall_clock;
 
     static void SymbolicateBacktrace(Core::System& system, std::vector<BacktraceEntry>& out);
+
+    virtual Dynarmic::HaltReason RunJit() = 0;
+    virtual Dynarmic::HaltReason StepJit() = 0;
+    virtual u32 GetSvcNumber() const = 0;
 };
 
 } // namespace Core
diff --git a/src/core/arm/dynarmic/arm_dynarmic_32.cpp b/src/core/arm/dynarmic/arm_dynarmic_32.cpp
index 894c1c527..7c82d0b96 100644
--- a/src/core/arm/dynarmic/arm_dynarmic_32.cpp
+++ b/src/core/arm/dynarmic/arm_dynarmic_32.cpp
@@ -26,10 +26,6 @@ namespace Core {
 
 using namespace Common::Literals;
 
-constexpr Dynarmic::HaltReason break_loop = Dynarmic::HaltReason::UserDefined2;
-constexpr Dynarmic::HaltReason svc_call = Dynarmic::HaltReason::UserDefined3;
-constexpr Dynarmic::HaltReason breakpoint = Dynarmic::HaltReason::UserDefined4;
-
 class DynarmicCallbacks32 : public Dynarmic::A32::UserCallbacks {
 public:
     explicit DynarmicCallbacks32(ARM_Dynarmic_32& parent_)
@@ -82,8 +78,8 @@ public:
 
     void ExceptionRaised(u32 pc, Dynarmic::A32::Exception exception) override {
         if (parent.system.DebuggerEnabled()) {
-            parent.breakpoint_pc = pc;
-            parent.jit.load()->HaltExecution(breakpoint);
+            parent.jit.load()->Regs()[15] = pc;
+            parent.jit.load()->HaltExecution(ARM_Interface::breakpoint);
             return;
         }
 
@@ -95,7 +91,7 @@ public:
 
     void CallSVC(u32 swi) override {
         parent.svc_swi = swi;
-        parent.jit.load()->HaltExecution(svc_call);
+        parent.jit.load()->HaltExecution(ARM_Interface::svc_call);
     }
 
     void AddTicks(u64 ticks) override {
@@ -240,35 +236,16 @@ std::shared_ptr<Dynarmic::A32::Jit> ARM_Dynarmic_32::MakeJit(Common::PageTable*
     return std::make_unique<Dynarmic::A32::Jit>(config);
 }
 
-void ARM_Dynarmic_32::Run() {
-    while (true) {
-        const auto hr = ShouldStep() ? jit.load()->Step() : jit.load()->Run();
-        if (Has(hr, svc_call)) {
-            Kernel::Svc::Call(system, svc_swi);
-        }
-
-        // Check to see if breakpoint is triggered.
-        // Recheck step condition in case stop is no longer desired.
-        Kernel::KThread* current_thread = system.Kernel().GetCurrentEmuThread();
-        if (Has(hr, breakpoint)) {
-            jit.load()->Regs()[15] = breakpoint_pc;
+Dynarmic::HaltReason ARM_Dynarmic_32::RunJit() {
+    return jit.load()->Run();
+}
 
-            if (system.GetDebugger().NotifyThreadStopped(current_thread)) {
-                current_thread->RequestSuspend(Kernel::SuspendType::Debug);
-            }
-            break;
-        }
-        if (ShouldStep()) {
-            // When stepping, this should be the only thread running.
-            ASSERT(system.GetDebugger().NotifyThreadStopped(current_thread));
-            current_thread->RequestSuspend(Kernel::SuspendType::Debug);
-            break;
-        }
+Dynarmic::HaltReason ARM_Dynarmic_32::StepJit() {
+    return jit.load()->Step();
+}
 
-        if (Has(hr, break_loop) || !uses_wall_clock) {
-            break;
-        }
-    }
+u32 ARM_Dynarmic_32::GetSvcNumber() const {
+    return svc_swi;
 }
 
 ARM_Dynarmic_32::ARM_Dynarmic_32(System& system_, CPUInterrupts& interrupt_handlers_,
diff --git a/src/core/arm/dynarmic/arm_dynarmic_32.h b/src/core/arm/dynarmic/arm_dynarmic_32.h
index 0557d5940..5b1d60005 100644
--- a/src/core/arm/dynarmic/arm_dynarmic_32.h
+++ b/src/core/arm/dynarmic/arm_dynarmic_32.h
@@ -41,7 +41,6 @@ public:
     void SetVectorReg(int index, u128 value) override;
     u32 GetPSTATE() const override;
     void SetPSTATE(u32 pstate) override;
-    void Run() override;
     VAddr GetTlsAddress() const override;
     void SetTlsAddress(VAddr address) override;
     void SetTPIDR_EL0(u64 value) override;
@@ -69,6 +68,11 @@ public:
 
     std::vector<BacktraceEntry> GetBacktrace() const override;
 
+protected:
+    Dynarmic::HaltReason RunJit() override;
+    Dynarmic::HaltReason StepJit() override;
+    u32 GetSvcNumber() const override;
+
 private:
     std::shared_ptr<Dynarmic::A32::Jit> MakeJit(Common::PageTable* page_table) const;
 
@@ -94,9 +98,6 @@ private:
 
     // SVC callback
     u32 svc_swi{};
-
-    // Debug restart address
-    u32 breakpoint_pc{};
 };
 
 } // namespace Core
diff --git a/src/core/arm/dynarmic/arm_dynarmic_64.cpp b/src/core/arm/dynarmic/arm_dynarmic_64.cpp
index 1f596cfef..d4c67eafd 100644
--- a/src/core/arm/dynarmic/arm_dynarmic_64.cpp
+++ b/src/core/arm/dynarmic/arm_dynarmic_64.cpp
@@ -26,10 +26,6 @@ namespace Core {
 using Vector = Dynarmic::A64::Vector;
 using namespace Common::Literals;
 
-constexpr Dynarmic::HaltReason break_loop = Dynarmic::HaltReason::UserDefined2;
-constexpr Dynarmic::HaltReason svc_call = Dynarmic::HaltReason::UserDefined3;
-constexpr Dynarmic::HaltReason breakpoint = Dynarmic::HaltReason::UserDefined4;
-
 class DynarmicCallbacks64 : public Dynarmic::A64::UserCallbacks {
 public:
     explicit DynarmicCallbacks64(ARM_Dynarmic_64& parent_)
@@ -123,8 +119,8 @@ public:
             return;
         default:
             if (parent.system.DebuggerEnabled()) {
-                parent.breakpoint_pc = pc;
-                parent.jit.load()->HaltExecution(breakpoint);
+                parent.jit.load()->SetPC(pc);
+                parent.jit.load()->HaltExecution(ARM_Interface::breakpoint);
                 return;
             }
 
@@ -136,7 +132,7 @@ public:
 
     void CallSVC(u32 swi) override {
         parent.svc_swi = swi;
-        parent.jit.load()->HaltExecution(svc_call);
+        parent.jit.load()->HaltExecution(ARM_Interface::svc_call);
     }
 
     void AddTicks(u64 ticks) override {
@@ -300,35 +296,16 @@ std::shared_ptr<Dynarmic::A64::Jit> ARM_Dynarmic_64::MakeJit(Common::PageTable*
     return std::make_shared<Dynarmic::A64::Jit>(config);
 }
 
-void ARM_Dynarmic_64::Run() {
-    while (true) {
-        const auto hr = jit.load()->Run();
-        if (Has(hr, svc_call)) {
-            Kernel::Svc::Call(system, svc_swi);
-        }
-
-        // Check to see if breakpoint is triggered.
-        // Recheck step condition in case stop is no longer desired.
-        Kernel::KThread* current_thread = system.Kernel().GetCurrentEmuThread();
-        if (Has(hr, breakpoint)) {
-            jit.load()->SetPC(breakpoint_pc);
+Dynarmic::HaltReason ARM_Dynarmic_64::RunJit() {
+    return jit.load()->Run();
+}
 
-            if (system.GetDebugger().NotifyThreadStopped(current_thread)) {
-                current_thread->RequestSuspend(Kernel::SuspendType::Debug);
-            }
-            break;
-        }
-        if (ShouldStep()) {
-            // When stepping, this should be the only thread running.
-            ASSERT(system.GetDebugger().NotifyThreadStopped(current_thread));
-            current_thread->RequestSuspend(Kernel::SuspendType::Debug);
-            break;
-        }
+Dynarmic::HaltReason ARM_Dynarmic_64::StepJit() {
+    return jit.load()->Step();
+}
 
-        if (Has(hr, break_loop) || !uses_wall_clock) {
-            break;
-        }
-    }
+u32 ARM_Dynarmic_64::GetSvcNumber() const {
+    return svc_swi;
 }
 
 ARM_Dynarmic_64::ARM_Dynarmic_64(System& system_, CPUInterrupts& interrupt_handlers_,
diff --git a/src/core/arm/dynarmic/arm_dynarmic_64.h b/src/core/arm/dynarmic/arm_dynarmic_64.h
index aa7054e0c..abfbc3c3f 100644
--- a/src/core/arm/dynarmic/arm_dynarmic_64.h
+++ b/src/core/arm/dynarmic/arm_dynarmic_64.h
@@ -39,7 +39,6 @@ public:
     void SetVectorReg(int index, u128 value) override;
     u32 GetPSTATE() const override;
     void SetPSTATE(u32 pstate) override;
-    void Run() override;
     VAddr GetTlsAddress() const override;
     void SetTlsAddress(VAddr address) override;
     void SetTPIDR_EL0(u64 value) override;
@@ -63,6 +62,11 @@ public:
 
     std::vector<BacktraceEntry> GetBacktrace() const override;
 
+protected:
+    Dynarmic::HaltReason RunJit() override;
+    Dynarmic::HaltReason StepJit() override;
+    u32 GetSvcNumber() const override;
+
 private:
     std::shared_ptr<Dynarmic::A64::Jit> MakeJit(Common::PageTable* page_table,
                                                 std::size_t address_space_bits) const;
@@ -87,9 +91,6 @@ private:
 
     // SVC callback
     u32 svc_swi{};
-
-    // Debug restart address
-    u64 breakpoint_pc{};
 };
 
 } // namespace Core