From 9972e5b10ad2d41d68b89ed37cfc3329866096c8 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Thu, 4 Aug 2016 12:39:29 -0700 Subject: [PATCH] Debugger: Allow logging on CPU breakpoints. --- Core/Debugger/Breakpoints.cpp | 52 +++++++++++++++++++++++---- Core/Debugger/Breakpoints.h | 25 +++++++++++-- Core/MIPS/IR/IRInterpreter.cpp | 10 ++---- Core/MIPS/x86/Jit.cpp | 7 ++-- Windows/Debugger/BreakpointWindow.cpp | 28 ++++++--------- Windows/Debugger/CtrlDisAsmView.cpp | 20 +++++------ Windows/Debugger/Debugger_Lists.cpp | 12 +++---- 7 files changed, 96 insertions(+), 58 deletions(-) diff --git a/Core/Debugger/Breakpoints.cpp b/Core/Debugger/Breakpoints.cpp index 1e727ffa12..41503fc1c5 100644 --- a/Core/Debugger/Breakpoints.cpp +++ b/Core/Debugger/Breakpoints.cpp @@ -114,7 +114,7 @@ size_t CBreakPoints::FindBreakpoint(u32 addr, bool matchTemp, bool temp) const auto &bp = breakPoints_[i]; if (bp.addr == addr && (!matchTemp || bp.temporary == temp)) { - if (bp.enabled) + if (bp.IsEnabled()) return i; // Hold out until the first enabled one. if (found == INVALID_BREAKPOINT) @@ -139,14 +139,15 @@ size_t CBreakPoints::FindMemCheck(u32 start, u32 end) bool CBreakPoints::IsAddressBreakPoint(u32 addr) { size_t bp = FindBreakpoint(addr); - return bp != INVALID_BREAKPOINT && breakPoints_[bp].enabled; + return bp != INVALID_BREAKPOINT && breakPoints_[bp].result != BREAK_ACTION_IGNORE; } bool CBreakPoints::IsAddressBreakPoint(u32 addr, bool* enabled) { size_t bp = FindBreakpoint(addr); if (bp == INVALID_BREAKPOINT) return false; - if (enabled != NULL) *enabled = breakPoints_[bp].enabled; + if (enabled != nullptr) + *enabled = breakPoints_[bp].IsEnabled(); return true; } @@ -174,16 +175,16 @@ void CBreakPoints::AddBreakPoint(u32 addr, bool temp) if (bp == INVALID_BREAKPOINT) { BreakPoint pt; - pt.enabled = true; + pt.result |= BREAK_ACTION_PAUSE; pt.temporary = temp; pt.addr = addr; breakPoints_.push_back(pt); Update(addr); } - else if (!breakPoints_[bp].enabled) + else if (!breakPoints_[bp].IsEnabled()) { - breakPoints_[bp].enabled = true; + breakPoints_[bp].result |= BREAK_ACTION_PAUSE; breakPoints_[bp].hasCond = false; Update(addr); } @@ -210,7 +211,20 @@ void CBreakPoints::ChangeBreakPoint(u32 addr, bool status) size_t bp = FindBreakpoint(addr); if (bp != INVALID_BREAKPOINT) { - breakPoints_[bp].enabled = status; + if (status) + breakPoints_[bp].result |= BREAK_ACTION_PAUSE; + else + breakPoints_[bp].result = BreakAction(breakPoints_[bp].result & ~BREAK_ACTION_PAUSE); + Update(addr); + } +} + +void CBreakPoints::ChangeBreakPoint(u32 addr, BreakAction result) +{ + size_t bp = FindBreakpoint(addr); + if (bp != INVALID_BREAKPOINT) + { + breakPoints_[bp].result = result; Update(addr); } } @@ -272,6 +286,30 @@ BreakPointCond *CBreakPoints::GetBreakPointCondition(u32 addr) return NULL; } +BreakAction CBreakPoints::ExecBreakPoint(u32 addr) { + size_t bp = FindBreakpoint(addr, false); + if (bp != INVALID_BREAKPOINT) { + if (breakPoints_[bp].hasCond) { + // Evaluate the breakpoint and abort if necessary. + auto cond = CBreakPoints::GetBreakPointCondition(currentMIPS->pc); + if (cond && !cond->Evaluate()) + return BREAK_ACTION_IGNORE; + } + + if (breakPoints_[bp].result & BREAK_ACTION_LOG) { + NOTICE_LOG(JIT, "BKP PC=%08x (%s)", addr, g_symbolMap->GetDescription(addr).c_str()); + } + if (breakPoints_[bp].result & BREAK_ACTION_PAUSE) { + Core_EnableStepping(true); + host->SetDebugMode(true); + } + + return breakPoints_[bp].result; + } + + return BREAK_ACTION_IGNORE; +} + void CBreakPoints::AddMemCheck(u32 start, u32 end, MemCheckCondition cond, BreakAction result) { // This will ruin any pending memchecks. diff --git a/Core/Debugger/Breakpoints.h b/Core/Debugger/Breakpoints.h index b783a10453..c2135e4e38 100644 --- a/Core/Debugger/Breakpoints.h +++ b/Core/Debugger/Breakpoints.h @@ -26,10 +26,17 @@ enum BreakAction BREAK_ACTION_IGNORE = 0x00, BREAK_ACTION_LOG = 0x01, BREAK_ACTION_PAUSE = 0x02, - - BREAK_ACTION_BOTH = 0x03, }; +static inline BreakAction &operator |= (BreakAction &lhs, const BreakAction &rhs) { + lhs = BreakAction(lhs | rhs); + return lhs; +} + +static inline BreakAction operator | (const BreakAction &lhs, const BreakAction &rhs) { + return BreakAction((u32)lhs | (u32)rhs); +} + struct BreakPointCond { DebugInterface *debug; @@ -54,12 +61,17 @@ struct BreakPoint BreakPoint() : hasCond(false) {} u32 addr; - bool enabled; bool temporary; + BreakAction result; + bool hasCond; BreakPointCond cond; + bool IsEnabled() const { + return (result & BREAK_ACTION_PAUSE) != 0; + } + bool operator == (const BreakPoint &other) const { return addr == other.addr; } @@ -100,6 +112,10 @@ struct MemCheck void Log(u32 addr, bool write, int size, u32 pc); + bool IsEnabled() const { + return (result & BREAK_ACTION_PAUSE) != 0; + } + bool operator == (const MemCheck &other) const { return start == other.start && end == other.end; } @@ -121,6 +137,7 @@ public: static void AddBreakPoint(u32 addr, bool temp = false); static void RemoveBreakPoint(u32 addr); static void ChangeBreakPoint(u32 addr, bool enable); + static void ChangeBreakPoint(u32 addr, BreakAction result); static void ClearAllBreakPoints(); static void ClearTemporaryBreakPoints(); @@ -129,6 +146,8 @@ public: static void ChangeBreakPointRemoveCond(u32 addr); static BreakPointCond *GetBreakPointCondition(u32 addr); + static BreakAction ExecBreakPoint(u32 addr); + static void AddMemCheck(u32 start, u32 end, MemCheckCondition cond, BreakAction result); static void RemoveMemCheck(u32 start, u32 end); static void ChangeMemCheck(u32 start, u32 end, MemCheckCondition cond, BreakAction result); diff --git a/Core/MIPS/IR/IRInterpreter.cpp b/Core/MIPS/IR/IRInterpreter.cpp index aca01aaee8..74e8c98925 100644 --- a/Core/MIPS/IR/IRInterpreter.cpp +++ b/Core/MIPS/IR/IRInterpreter.cpp @@ -37,14 +37,8 @@ u32 RunBreakpoint(u32 pc) { if (CBreakPoints::CheckSkipFirst() == pc) return 0; - auto cond = CBreakPoints::GetBreakPointCondition(pc); - if (cond && !cond->Evaluate()) - return 0; - - Core_EnableStepping(true); - host->SetDebugMode(true); - - return 1; + CBreakPoints::ExecBreakPoint(currentMIPS->pc); + return coreState != CORE_RUNNING ? 1 : 0; } u32 RunMemCheck(u32 pc, u32 addr) { diff --git a/Core/MIPS/x86/Jit.cpp b/Core/MIPS/x86/Jit.cpp index 4bfce6814e..952da9627a 100644 --- a/Core/MIPS/x86/Jit.cpp +++ b/Core/MIPS/x86/Jit.cpp @@ -73,13 +73,10 @@ u32 JitBreakpoint() if (CBreakPoints::CheckSkipFirst() == currentMIPS->pc) return 0; - auto cond = CBreakPoints::GetBreakPointCondition(currentMIPS->pc); - if (cond && !cond->Evaluate()) + BreakAction result = CBreakPoints::ExecBreakPoint(currentMIPS->pc); + if ((result & BREAK_ACTION_PAUSE) == 0) return 0; - Core_EnableStepping(true); - host->SetDebugMode(true); - // There's probably a better place for this. if (USE_JIT_MISSMAP) { std::map notJitSorted; diff --git a/Windows/Debugger/BreakpointWindow.cpp b/Windows/Debugger/BreakpointWindow.cpp index da1899452d..945b17a6e6 100644 --- a/Windows/Debugger/BreakpointWindow.cpp +++ b/Windows/Debugger/BreakpointWindow.cpp @@ -28,7 +28,6 @@ INT_PTR CALLBACK BreakpointWindow::dlgFunc(HWND hwnd, UINT iMsg, WPARAM wParam, EnableWindow(GetDlgItem(hwnd,IDC_BREAKPOINT_ONCHANGE),bp->memory); EnableWindow(GetDlgItem(hwnd,IDC_BREAKPOINT_SIZE),bp->memory); EnableWindow(GetDlgItem(hwnd,IDC_BREAKPOINT_CONDITION),!bp->memory); - EnableWindow(GetDlgItem(hwnd,IDC_BREAKPOINT_LOG),bp->memory); if (bp->address != -1) @@ -55,7 +54,6 @@ INT_PTR CALLBACK BreakpointWindow::dlgFunc(HWND hwnd, UINT iMsg, WPARAM wParam, EnableWindow(GetDlgItem(hwnd,IDC_BREAKPOINT_ONCHANGE),bp->memory); EnableWindow(GetDlgItem(hwnd,IDC_BREAKPOINT_SIZE),bp->memory); EnableWindow(GetDlgItem(hwnd,IDC_BREAKPOINT_CONDITION),!bp->memory); - EnableWindow(GetDlgItem(hwnd,IDC_BREAKPOINT_LOG),bp->memory); break; } break; @@ -69,7 +67,6 @@ INT_PTR CALLBACK BreakpointWindow::dlgFunc(HWND hwnd, UINT iMsg, WPARAM wParam, EnableWindow(GetDlgItem(hwnd,IDC_BREAKPOINT_ONCHANGE),bp->memory); EnableWindow(GetDlgItem(hwnd,IDC_BREAKPOINT_SIZE),bp->memory); EnableWindow(GetDlgItem(hwnd,IDC_BREAKPOINT_CONDITION),!bp->memory); - EnableWindow(GetDlgItem(hwnd,IDC_BREAKPOINT_LOG),bp->memory); break; } break; @@ -183,6 +180,12 @@ bool BreakpointWindow::exec() void BreakpointWindow::addBreakpoint() { + BreakAction result = BREAK_ACTION_IGNORE; + if (log) + result |= BREAK_ACTION_LOG; + if (enabled) + result |= BREAK_ACTION_PAUSE; + if (memory) { // add memcheck @@ -194,13 +197,7 @@ void BreakpointWindow::addBreakpoint() if (onChange) cond |= MEMCHECK_WRITE_ONCHANGE; - int result = BREAK_ACTION_IGNORE; - if (log) - result |= BREAK_ACTION_LOG; - if (enabled) - result |= BREAK_ACTION_PAUSE; - - CBreakPoints::AddMemCheck(address, address + size, (MemCheckCondition)cond, (BreakAction)result); + CBreakPoints::AddMemCheck(address, address + size, (MemCheckCondition)cond, result); } else { // add breakpoint CBreakPoints::AddBreakPoint(address,false); @@ -214,10 +211,7 @@ void BreakpointWindow::addBreakpoint() CBreakPoints::ChangeBreakPointAddCond(address,cond); } - if (enabled == false) - { - CBreakPoints::ChangeBreakPoint(address,false); - } + CBreakPoints::ChangeBreakPoint(address, result); } } @@ -240,12 +234,12 @@ void BreakpointWindow::loadFromBreakpoint(BreakPoint& breakpoint) { memory = false; - enabled = breakpoint.enabled; + log = (breakpoint.result & BREAK_ACTION_LOG) != 0; + enabled = (breakpoint.result & BREAK_ACTION_PAUSE) != 0; address = breakpoint.addr; size = 1; - if (breakpoint.hasCond) - { + if (breakpoint.hasCond) { strcpy(condition,breakpoint.cond.expressionString); } else { condition[0] = 0; diff --git a/Windows/Debugger/CtrlDisAsmView.cpp b/Windows/Debugger/CtrlDisAsmView.cpp index 7da836ab7e..b0afc5f9a5 100644 --- a/Windows/Debugger/CtrlDisAsmView.cpp +++ b/Windows/Debugger/CtrlDisAsmView.cpp @@ -1,4 +1,4 @@ -// NOTE: Apologies for the quality of this code, this is really from pre-opensource Dolphin - that is, 2003. +// NOTE: Apologies for the quality of this code, this is really from pre-opensource Dolphin - that is, 2003. #include "Windows/resource.h" #include "Core/MemMap.h" @@ -550,7 +550,7 @@ void CtrlDisAsmView::onPaint(WPARAM wParam, LPARAM lParam) if (isInInterval(address,line.totalSize,debugger->getPC())) { - TextOut(hdc,pixelPositions.opcodeStart-8,rowY1,L"■",1); + TextOut(hdc,pixelPositions.opcodeStart-8,rowY1,L"\x25A0",1); } // display whether the condition of a branch is met @@ -834,22 +834,18 @@ void CtrlDisAsmView::redraw() void CtrlDisAsmView::toggleBreakpoint(bool toggleEnabled) { bool enabled; - if (CBreakPoints::IsAddressBreakPoint(curAddress,&enabled)) - { - if (!enabled) - { + if (CBreakPoints::IsAddressBreakPoint(curAddress, &enabled)) { + if (!enabled) { // enable disabled breakpoints - CBreakPoints::ChangeBreakPoint(curAddress,true); - } else if (!toggleEnabled && CBreakPoints::GetBreakPointCondition(curAddress) != NULL) - { + CBreakPoints::ChangeBreakPoint(curAddress, true); + } else if (!toggleEnabled && CBreakPoints::GetBreakPointCondition(curAddress) != nullptr) { // don't just delete a breakpoint with a custom condition int ret = MessageBox(wnd,L"This breakpoint has a custom condition.\nDo you want to remove it?",L"Confirmation",MB_YESNO); if (ret == IDYES) CBreakPoints::RemoveBreakPoint(curAddress); - } else if (toggleEnabled) - { + } else if (toggleEnabled) { // disable breakpoint - CBreakPoints::ChangeBreakPoint(curAddress,false); + CBreakPoints::ChangeBreakPoint(curAddress, false); } else { // otherwise just remove breakpoint CBreakPoints::RemoveBreakPoint(curAddress); diff --git a/Windows/Debugger/Debugger_Lists.cpp b/Windows/Debugger/Debugger_Lists.cpp index a8efbe9d51..7d08b09ee6 100644 --- a/Windows/Debugger/Debugger_Lists.cpp +++ b/Windows/Debugger/Debugger_Lists.cpp @@ -324,9 +324,9 @@ void CtrlBreakpointList::reloadBreakpoints() continue; if (isMemory) - SetCheckState(i, (displayedMemChecks_[index].result & BREAK_ACTION_PAUSE) != 0); + SetCheckState(i, displayedMemChecks_[index].IsEnabled()); else - SetCheckState(i, displayedBreakPoints_[index].enabled); + SetCheckState(i, displayedBreakPoints_[index].IsEnabled()); } } @@ -368,7 +368,7 @@ void CtrlBreakpointList::toggleEnabled(int itemIndex) CBreakPoints::ChangeMemCheck(mcPrev.start, mcPrev.end, mcPrev.cond, BreakAction(mcPrev.result ^ BREAK_ACTION_PAUSE)); } else { BreakPoint bpPrev = displayedBreakPoints_[index]; - CBreakPoints::ChangeBreakPoint(bpPrev.addr, !bpPrev.enabled); + CBreakPoints::ChangeBreakPoint(bpPrev.addr, BreakAction(bpPrev.result ^ BREAK_ACTION_PAUSE)); } } @@ -605,9 +605,9 @@ void CtrlBreakpointList::showBreakpointMenu(int itemIndex, const POINT &pt) HMENU subMenu = GetSubMenu(g_hPopupMenus, POPUP_SUBMENU_ID_BREAKPOINTLIST); if (isMemory) { - CheckMenuItem(subMenu, ID_DISASM_DISABLEBREAKPOINT, MF_BYCOMMAND | (mcPrev.result & BREAK_ACTION_PAUSE ? MF_CHECKED : MF_UNCHECKED)); + CheckMenuItem(subMenu, ID_DISASM_DISABLEBREAKPOINT, MF_BYCOMMAND | (mcPrev.IsEnabled() ? MF_CHECKED : MF_UNCHECKED)); } else { - CheckMenuItem(subMenu, ID_DISASM_DISABLEBREAKPOINT, MF_BYCOMMAND | (bpPrev.enabled ? MF_CHECKED : MF_UNCHECKED)); + CheckMenuItem(subMenu, ID_DISASM_DISABLEBREAKPOINT, MF_BYCOMMAND | (bpPrev.IsEnabled() ? MF_CHECKED : MF_UNCHECKED)); } switch (TrackPopupMenuEx(subMenu, TPM_RIGHTBUTTON | TPM_RETURNCMD, screenPt.x, screenPt.y, GetHandle(), 0)) @@ -616,7 +616,7 @@ void CtrlBreakpointList::showBreakpointMenu(int itemIndex, const POINT &pt) if (isMemory) { CBreakPoints::ChangeMemCheck(mcPrev.start, mcPrev.end, mcPrev.cond, BreakAction(mcPrev.result ^ BREAK_ACTION_PAUSE)); } else { - CBreakPoints::ChangeBreakPoint(bpPrev.addr, !bpPrev.enabled); + CBreakPoints::ChangeBreakPoint(bpPrev.addr, BreakAction(bpPrev.result ^ BREAK_ACTION_PAUSE)); } break; case ID_DISASM_EDITBREAKPOINT: