Merge pull request #11554 from JosJuice/host-lock-cpu

DolphinQt: Properly lock CPU before accessing emulated memory

Source/Core/Common/Debug/CodeTrace.cpp

@@ -122,14 +122,14 @@ InstructionAttributes CodeTrace::GetInstructionAttributes(const TraceOutput& ins
   return tmp_attributes;
 }
 
-TraceOutput CodeTrace::SaveCurrentInstruction() const
+TraceOutput CodeTrace::SaveCurrentInstruction(const Core::CPUThreadGuard* guard) const
 {
   auto& system = Core::System::GetInstance();
   auto& ppc_state = system.GetPPCState();
 
   // Quickly save instruction and memory target for fast logging.
   TraceOutput output;
-  const std::string instr = PowerPC::debug_interface.Disassemble(ppc_state.pc);
+  const std::string instr = PowerPC::debug_interface.Disassemble(guard, ppc_state.pc);
   output.instruction = instr;
   output.address = ppc_state.pc;
 
@@ -139,14 +139,15 @@ TraceOutput CodeTrace::SaveCurrentInstruction() const
   return output;
 }
 
-AutoStepResults CodeTrace::AutoStepping(bool continue_previous, AutoStop stop_on)
+AutoStepResults CodeTrace::AutoStepping(const Core::CPUThreadGuard& guard, bool continue_previous,
+                                        AutoStop stop_on)
 {
   AutoStepResults results;
 
-  if (!CPU::IsStepping() || m_recording)
+  if (m_recording)
     return results;
 
-  TraceOutput pc_instr = SaveCurrentInstruction();
+  TraceOutput pc_instr = SaveCurrentInstruction(&guard);
   const InstructionAttributes instr = GetInstructionAttributes(pc_instr);
 
   // Not an instruction we should start autostepping from (ie branches).
@@ -187,7 +188,6 @@ AutoStepResults CodeTrace::AutoStepping(bool continue_previous, AutoStop stop_on
   else if (stop_on == AutoStop::Changed)
     stop_condition = HitType::ACTIVE;
 
-  CPU::PauseAndLock(true, false);
   PowerPC::breakpoints.ClearAllTemporary();
   using clock = std::chrono::steady_clock;
   clock::time_point timeout = clock::now() + std::chrono::seconds(4);
@@ -199,7 +199,7 @@ AutoStepResults CodeTrace::AutoStepping(bool continue_previous, AutoStop stop_on
   {
     PowerPC::SingleStep();
 
-    pc_instr = SaveCurrentInstruction();
+    pc_instr = SaveCurrentInstruction(&guard);
     hit = TraceLogic(pc_instr);
     results.count += 1;
   } while (clock::now() < timeout && hit < stop_condition &&
@@ -210,7 +210,6 @@ AutoStepResults CodeTrace::AutoStepping(bool continue_previous, AutoStop stop_on
     results.timed_out = true;
 
   PowerPC::SetMode(old_mode);
-  CPU::PauseAndLock(false, false);
   m_recording = false;
 
   results.reg_tracked = m_reg_autotrack;

Source/Core/Common/Debug/CodeTrace.h

@@ -10,6 +10,11 @@
 
 #include "Common/CommonTypes.h"
 
+namespace Core
+{
+class CPUThreadGuard;
+}
+
 struct InstructionAttributes
 {
   u32 address = 0;
@@ -63,11 +68,12 @@ class CodeTrace
   };
 
   void SetRegTracked(const std::string& reg);
-  AutoStepResults AutoStepping(bool continue_previous = false, AutoStop stop_on = AutoStop::Always);
+  AutoStepResults AutoStepping(const Core::CPUThreadGuard& guard, bool continue_previous = false,
+                               AutoStop stop_on = AutoStop::Always);
 
 private:
   InstructionAttributes GetInstructionAttributes(const TraceOutput& line) const;
-  TraceOutput SaveCurrentInstruction() const;
+  TraceOutput SaveCurrentInstruction(const Core::CPUThreadGuard* guard) const;
   HitType TraceLogic(const TraceOutput& current_instr, bool first_hit = false);
 
   bool m_recording = false;

Source/Core/Common/Debug/MemoryPatches.cpp

@@ -23,44 +23,45 @@ MemoryPatch::MemoryPatch(u32 address_, u32 value_)
 MemoryPatches::MemoryPatches() = default;
 MemoryPatches::~MemoryPatches() = default;
 
-void MemoryPatches::SetPatch(u32 address, u32 value)
+void MemoryPatches::SetPatch(const Core::CPUThreadGuard& guard, u32 address, u32 value)
 {
   const std::size_t index = m_patches.size();
   m_patches.emplace_back(address, value);
-  Patch(index);
+  Patch(guard, index);
 }
 
-void MemoryPatches::SetPatch(u32 address, std::vector<u8> value)
+void MemoryPatches::SetPatch(const Core::CPUThreadGuard& guard, u32 address, std::vector<u8> value)
 {
-  UnsetPatch(address);
+  UnsetPatch(guard, address);
   const std::size_t index = m_patches.size();
   m_patches.emplace_back(address, std::move(value));
-  Patch(index);
+  Patch(guard, index);
 }
 
-void MemoryPatches::SetFramePatch(u32 address, u32 value)
+void MemoryPatches::SetFramePatch(const Core::CPUThreadGuard& guard, u32 address, u32 value)
 {
   const std::size_t index = m_patches.size();
   m_patches.emplace_back(address, value);
   m_patches.back().type = MemoryPatch::ApplyType::EachFrame;
-  Patch(index);
+  Patch(guard, index);
 }
 
-void MemoryPatches::SetFramePatch(u32 address, std::vector<u8> value)
+void MemoryPatches::SetFramePatch(const Core::CPUThreadGuard& guard, u32 address,
+                                  std::vector<u8> value)
 {
-  UnsetPatch(address);
+  UnsetPatch(guard, address);
   const std::size_t index = m_patches.size();
   m_patches.emplace_back(address, std::move(value));
   m_patches.back().type = MemoryPatch::ApplyType::EachFrame;
-  Patch(index);
+  Patch(guard, index);
 }
 
 const std::vector<MemoryPatch>& MemoryPatches::GetPatches() const
 {
   return m_patches;
 }
 
-void MemoryPatches::UnsetPatch(u32 address)
+void MemoryPatches::UnsetPatch(const Core::CPUThreadGuard& guard, u32 address)
 {
   const auto it = std::find_if(m_patches.begin(), m_patches.end(),
                                [address](const auto& patch) { return patch.address == address; });
@@ -69,23 +70,23 @@ void MemoryPatches::UnsetPatch(u32 address)
     return;
 
   const std::size_t index = std::distance(m_patches.begin(), it);
-  RemovePatch(index);
+  RemovePatch(guard, index);
 }
 
-void MemoryPatches::EnablePatch(std::size_t index)
+void MemoryPatches::EnablePatch(const Core::CPUThreadGuard& guard, std::size_t index)
 {
   if (m_patches[index].is_enabled == MemoryPatch::State::Enabled)
     return;
   m_patches[index].is_enabled = MemoryPatch::State::Enabled;
-  Patch(index);
+  Patch(guard, index);
 }
 
-void MemoryPatches::DisablePatch(std::size_t index)
+void MemoryPatches::DisablePatch(const Core::CPUThreadGuard& guard, std::size_t index)
 {
   if (m_patches[index].is_enabled == MemoryPatch::State::Disabled)
     return;
   m_patches[index].is_enabled = MemoryPatch::State::Disabled;
-  Patch(index);
+  Patch(guard, index);
 }
 
 bool MemoryPatches::HasEnabledPatch(u32 address) const
@@ -95,19 +96,19 @@ bool MemoryPatches::HasEnabledPatch(u32 address) const
   });
 }
 
-void MemoryPatches::RemovePatch(std::size_t index)
+void MemoryPatches::RemovePatch(const Core::CPUThreadGuard& guard, std::size_t index)
 {
-  DisablePatch(index);
+  DisablePatch(guard, index);
   UnPatch(index);
   m_patches.erase(m_patches.begin() + index);
 }
 
-void MemoryPatches::ClearPatches()
+void MemoryPatches::ClearPatches(const Core::CPUThreadGuard& guard)
 {
   const std::size_t size = m_patches.size();
   for (std::size_t index = 0; index < size; ++index)
   {
-    DisablePatch(index);
+    DisablePatch(guard, index);
     UnPatch(index);
   }
   m_patches.clear();

Source/Core/Common/Debug/MemoryPatches.h

@@ -9,6 +9,11 @@
 
 #include "Common/CommonTypes.h"
 
+namespace Core
+{
+class CPUThreadGuard;
+}
+
 namespace Common::Debug
 {
 struct MemoryPatch
@@ -40,21 +45,21 @@ class MemoryPatches
   MemoryPatches();
   virtual ~MemoryPatches();
 
-  void SetPatch(u32 address, u32 value);
-  void SetPatch(u32 address, std::vector<u8> value);
-  void SetFramePatch(u32 address, u32 value);
-  void SetFramePatch(u32 address, std::vector<u8> value);
+  void SetPatch(const Core::CPUThreadGuard& guard, u32 address, u32 value);
+  void SetPatch(const Core::CPUThreadGuard& guard, u32 address, std::vector<u8> value);
+  void SetFramePatch(const Core::CPUThreadGuard& guard, u32 address, u32 value);
+  void SetFramePatch(const Core::CPUThreadGuard& guard, u32 address, std::vector<u8> value);
   const std::vector<MemoryPatch>& GetPatches() const;
-  void UnsetPatch(u32 address);
-  void EnablePatch(std::size_t index);
-  void DisablePatch(std::size_t index);
+  void UnsetPatch(const Core::CPUThreadGuard& guard, u32 address);
+  void EnablePatch(const Core::CPUThreadGuard& guard, std::size_t index);
+  void DisablePatch(const Core::CPUThreadGuard& guard, std::size_t index);
   bool HasEnabledPatch(u32 address) const;
-  void RemovePatch(std::size_t index);
-  void ClearPatches();
-  virtual void ApplyExistingPatch(std::size_t index) = 0;
+  void RemovePatch(const Core::CPUThreadGuard& guard, std::size_t index);
+  void ClearPatches(const Core::CPUThreadGuard& guard);
+  virtual void ApplyExistingPatch(const Core::CPUThreadGuard& guard, std::size_t index) = 0;
 
 protected:
-  virtual void Patch(std::size_t index) = 0;
+  virtual void Patch(const Core::CPUThreadGuard& guard, std::size_t index) = 0;
   virtual void UnPatch(std::size_t index) = 0;
 
   std::vector<MemoryPatch> m_patches;

Source/Core/Common/Debug/Threads.h

@@ -11,6 +11,11 @@
 
 #include "Common/CommonTypes.h"
 
+namespace Core
+{
+class CPUThreadGuard;
+};
+
 namespace Common::Debug
 {
 struct PartialContext
@@ -41,7 +46,7 @@ class ThreadView
     LWPThread,  // devkitPro libogc thread
   };
 
-  virtual PartialContext GetContext() const = 0;
+  virtual PartialContext GetContext(const Core::CPUThreadGuard& guard) const = 0;
   virtual u32 GetAddress() const = 0;
   virtual u16 GetState() const = 0;
   virtual bool IsSuspended() const = 0;
@@ -53,8 +58,8 @@ class ThreadView
   virtual std::size_t GetStackSize() const = 0;
   virtual s32 GetErrno() const = 0;
   // Implementation specific, used to store arbitrary data
-  virtual std::string GetSpecific() const = 0;
-  virtual bool IsValid() const = 0;
+  virtual std::string GetSpecific(const Core::CPUThreadGuard& guard) const = 0;
+  virtual bool IsValid(const Core::CPUThreadGuard& guard) const = 0;
 };
 
 using Threads = std::vector<std::unique_ptr<ThreadView>>;

Source/Core/Common/DebugInterface.h

@@ -16,6 +16,11 @@ struct MemoryPatch;
 struct Watch;
 }  // namespace Common::Debug
 
+namespace Core
+{
+class CPUThreadGuard;
+}  // namespace Core
+
 namespace Common
 {
 class DebugInterface
@@ -42,24 +47,29 @@ class DebugInterface
   virtual void ClearWatches() = 0;
 
   // Memory Patches
-  virtual void SetPatch(u32 address, u32 value) = 0;
-  virtual void SetPatch(u32 address, std::vector<u8> value) = 0;
-  virtual void SetFramePatch(u32 address, u32 value) = 0;
-  virtual void SetFramePatch(u32 address, std::vector<u8> value) = 0;
+  virtual void SetPatch(const Core::CPUThreadGuard& guard, u32 address, u32 value) = 0;
+  virtual void SetPatch(const Core::CPUThreadGuard& guard, u32 address, std::vector<u8> value) = 0;
+  virtual void SetFramePatch(const Core::CPUThreadGuard& guard, u32 address, u32 value) = 0;
+  virtual void SetFramePatch(const Core::CPUThreadGuard& guard, u32 address,
+                             std::vector<u8> value) = 0;
   virtual const std::vector<Debug::MemoryPatch>& GetPatches() const = 0;
-  virtual void UnsetPatch(u32 address) = 0;
-  virtual void EnablePatch(std::size_t index) = 0;
-  virtual void DisablePatch(std::size_t index) = 0;
+  virtual void UnsetPatch(const Core::CPUThreadGuard& guard, u32 address) = 0;
+  virtual void EnablePatch(const Core::CPUThreadGuard& guard, std::size_t index) = 0;
+  virtual void DisablePatch(const Core::CPUThreadGuard& guard, std::size_t index) = 0;
   virtual bool HasEnabledPatch(u32 address) const = 0;
-  virtual void RemovePatch(std::size_t index) = 0;
-  virtual void ClearPatches() = 0;
-  virtual void ApplyExistingPatch(std::size_t index) = 0;
+  virtual void RemovePatch(const Core::CPUThreadGuard& guard, std::size_t index) = 0;
+  virtual void ClearPatches(const Core::CPUThreadGuard& guard) = 0;
+  virtual void ApplyExistingPatch(const Core::CPUThreadGuard& guard, std::size_t index) = 0;
 
   // Threads
-  virtual Debug::Threads GetThreads() const = 0;
+  virtual Debug::Threads GetThreads(const Core::CPUThreadGuard& guard) const = 0;
 
-  virtual std::string Disassemble(u32 /*address*/) const { return "NODEBUGGER"; }
-  virtual std::string GetRawMemoryString(int /*memory*/, u32 /*address*/) const
+  virtual std::string Disassemble(const Core::CPUThreadGuard* /*guard*/, u32 /*address*/) const
+  {
+    return "NODEBUGGER";
+  }
+  virtual std::string GetRawMemoryString(const Core::CPUThreadGuard& /*guard*/, int /*memory*/,
+                                         u32 /*address*/) const
   {
     return "NODEBUGGER";
   }
@@ -72,10 +82,20 @@ class DebugInterface
   virtual void ClearAllMemChecks() {}
   virtual bool IsMemCheck(u32 /*address*/, size_t /*size*/) const { return false; }
   virtual void ToggleMemCheck(u32 /*address*/, bool /*read*/, bool /*write*/, bool /*log*/) {}
-  virtual u32 ReadMemory(u32 /*address*/) const { return 0; }
-  virtual void WriteExtraMemory(int /*memory*/, u32 /*value*/, u32 /*address*/) {}
-  virtual u32 ReadExtraMemory(int /*memory*/, u32 /*address*/) const { return 0; }
-  virtual u32 ReadInstruction(u32 /*address*/) const { return 0; }
+  virtual u32 ReadMemory(const Core::CPUThreadGuard& /*guard*/, u32 /*address*/) const { return 0; }
+  virtual void WriteExtraMemory(const Core::CPUThreadGuard& /*guard*/, int /*memory*/,
+                                u32 /*value*/, u32 /*address*/)
+  {
+  }
+  virtual u32 ReadExtraMemory(const Core::CPUThreadGuard& /*guard*/, int /*memory*/,
+                              u32 /*address*/) const
+  {
+    return 0;
+  }
+  virtual u32 ReadInstruction(const Core::CPUThreadGuard& /*guard*/, u32 /*address*/) const
+  {
+    return 0;
+  }
   virtual std::optional<u32>
   GetMemoryAddressFromInstruction(const std::string& /*instruction*/) const
   {
@@ -85,8 +105,11 @@ class DebugInterface
   virtual void SetPC(u32 /*address*/) {}
   virtual void Step() {}
   virtual void RunToBreakpoint() {}
-  virtual u32 GetColor(u32 /*address*/) const { return 0xFFFFFFFF; }
+  virtual u32 GetColor(const Core::CPUThreadGuard* /*guard*/, u32 /*address*/) const
+  {
+    return 0xFFFFFFFF;
+  }
   virtual std::string GetDescription(u32 /*address*/) const = 0;
-  virtual void Clear() = 0;
+  virtual void Clear(const Core::CPUThreadGuard& guard) = 0;
 };
 }  // namespace Common

Source/Core/Common/SymbolDB.h

@@ -15,6 +15,11 @@
 
 #include "Common/CommonTypes.h"
 
+namespace Core
+{
+class CPUThreadGuard;
+}
+
 namespace Common
 {
 struct SCall
@@ -68,7 +73,7 @@ class SymbolDB
   virtual ~SymbolDB();
 
   virtual Symbol* GetSymbolFromAddr(u32 addr) { return nullptr; }
-  virtual Symbol* AddFunction(u32 start_addr) { return nullptr; }
+  virtual Symbol* AddFunction(const Core::CPUThreadGuard& guard, u32 start_addr) { return nullptr; }
   void AddCompleteSymbol(const Symbol& symbol);
 
   Symbol* GetSymbolFromName(std::string_view name);