From a8ebca4fc60ecea550c879ecfd6135f066645174 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 31 Aug 2021 11:30:55 -0400 Subject: [PATCH 1/2] MMU: Invert conditionals in Memcheck() Lets us unindent code a little bit. --- Source/Core/Core/PowerPC/MMU.cpp | 51 +++++++++++++++++--------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/Source/Core/Core/PowerPC/MMU.cpp b/Source/Core/Core/PowerPC/MMU.cpp index 87cd6102d7..28915b660c 100644 --- a/Source/Core/Core/PowerPC/MMU.cpp +++ b/Source/Core/Core/PowerPC/MMU.cpp @@ -501,32 +501,35 @@ TryReadResult HostTryReadInstruction(const u32 address, RequestedAddressSpa static void Memcheck(u32 address, u32 var, bool write, size_t size) { - if (PowerPC::memchecks.HasAny()) + if (!memchecks.HasAny()) + return; + + TMemCheck* mc = memchecks.GetMemCheck(address, size); + if (mc == nullptr) + return; + + if (CPU::IsStepping()) { - TMemCheck* mc = PowerPC::memchecks.GetMemCheck(address, size); - if (mc) - { - if (CPU::IsStepping()) - { - // Disable when stepping so that resume works. - return; - } - mc->num_hits++; - bool pause = mc->Action(&PowerPC::debug_interface, var, address, write, size, PC); - if (pause) - { - CPU::Break(); - // Fake a DSI so that all the code that tests for it in order to skip - // the rest of the instruction will apply. (This means that - // watchpoints will stop the emulator before the offending load/store, - // not after like GDB does, but that's better anyway. Just need to - // make sure resuming after that works.) - // It doesn't matter if ReadFromHardware triggers its own DSI because - // we'll take it after resuming. - PowerPC::ppcState.Exceptions |= EXCEPTION_DSI | EXCEPTION_FAKE_MEMCHECK_HIT; - } - } + // Disable when stepping so that resume works. + return; } + + mc->num_hits++; + + const bool pause = mc->Action(&debug_interface, var, address, write, size, PC); + if (!pause) + return; + + CPU::Break(); + + // Fake a DSI so that all the code that tests for it in order to skip + // the rest of the instruction will apply. (This means that + // watchpoints will stop the emulator before the offending load/store, + // not after like GDB does, but that's better anyway. Just need to + // make sure resuming after that works.) + // It doesn't matter if ReadFromHardware triggers its own DSI because + // we'll take it after resuming. + ppcState.Exceptions |= EXCEPTION_DSI | EXCEPTION_FAKE_MEMCHECK_HIT; } u8 Read_U8(const u32 address) From 7a6b63b309cd1dac4f402641b9d80c646442638e Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 31 Aug 2021 11:35:10 -0400 Subject: [PATCH 2/2] MMU: Don't truncate 64-bit values when calling Memcheck() Previously in Read_U64 and Write_U64 the value that was read or written would be truncated to a 32-bit value before being passed off to the memcheck handler, which can result in incorrect values being logged out. --- Source/Core/Core/PowerPC/BreakPoints.cpp | 2 +- Source/Core/Core/PowerPC/BreakPoints.h | 2 +- Source/Core/Core/PowerPC/MMU.cpp | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Source/Core/Core/PowerPC/BreakPoints.cpp b/Source/Core/Core/PowerPC/BreakPoints.cpp index f47c90d55f..9690d11c89 100644 --- a/Source/Core/Core/PowerPC/BreakPoints.cpp +++ b/Source/Core/Core/PowerPC/BreakPoints.cpp @@ -293,7 +293,7 @@ bool MemChecks::OverlapsMemcheck(u32 address, u32 length) const }); } -bool TMemCheck::Action(Common::DebugInterface* debug_interface, u32 value, u32 addr, bool write, +bool TMemCheck::Action(Common::DebugInterface* debug_interface, u64 value, u32 addr, bool write, size_t size, u32 pc) { if (!is_enabled) diff --git a/Source/Core/Core/PowerPC/BreakPoints.h b/Source/Core/Core/PowerPC/BreakPoints.h index a1c6f9bb61..5fedd8f807 100644 --- a/Source/Core/Core/PowerPC/BreakPoints.h +++ b/Source/Core/Core/PowerPC/BreakPoints.h @@ -40,7 +40,7 @@ struct TMemCheck u32 num_hits = 0; // returns whether to break - bool Action(Common::DebugInterface* dbg_interface, u32 value, u32 addr, bool write, size_t size, + bool Action(Common::DebugInterface* debug_interface, u64 value, u32 addr, bool write, size_t size, u32 pc); }; diff --git a/Source/Core/Core/PowerPC/MMU.cpp b/Source/Core/Core/PowerPC/MMU.cpp index 28915b660c..9d28dbcc21 100644 --- a/Source/Core/Core/PowerPC/MMU.cpp +++ b/Source/Core/Core/PowerPC/MMU.cpp @@ -499,7 +499,7 @@ TryReadResult HostTryReadInstruction(const u32 address, RequestedAddressSpa return TryReadResult(); } -static void Memcheck(u32 address, u32 var, bool write, size_t size) +static void Memcheck(u32 address, u64 var, bool write, size_t size) { if (!memchecks.HasAny()) return; @@ -556,7 +556,7 @@ u32 Read_U32(const u32 address) u64 Read_U64(const u32 address) { u64 var = ReadFromHardware(address); - Memcheck(address, (u32)var, false, 8); + Memcheck(address, var, false, 8); return var; } @@ -679,7 +679,7 @@ void Write_U32_Swap(const u32 var, const u32 address) void Write_U64(const u64 var, const u32 address) { - Memcheck(address, (u32)var, true, 8); + Memcheck(address, var, true, 8); WriteToHardware(address, static_cast(var >> 32), 4); WriteToHardware(address + sizeof(u32), static_cast(var), 4); }