From 5287bfb4090d895df0c756cda60d4297640bdf8a Mon Sep 17 00:00:00 2001 From: Andrea Date: Mon, 20 Mar 2023 13:25:25 +0000 Subject: [PATCH] Breakpoints: some new features (#1191) * Debugger: add new flags to breakpoints. Stop / no stop. Hit count Keep temp breakpoint alive so they can be inspected. Signed-off-by: Andrea Odetti * Debugger: ensure temporary breakpoints are removed when the execution restarts. This code: _BWZ_Clear(pBP, iBreakpoint); was actually a bug since the function needs the root points of all breakpoints, not to a particular one. * Breakpoints: some extra tweaks. Signed-off-by: Andrea Odetti * Remove reundant code and comment. Signed-off-by: Andrea Odetti * Breakpoints: coding standards. Signed-off-by: Andrea Odetti --------- Signed-off-by: Andrea Odetti --- source/Debugger/Debug.cpp | 162 ++++++++++++++++++-------- source/Debugger/Debug.h | 4 + source/Debugger/Debugger_Commands.cpp | 1 + source/Debugger/Debugger_Help.cpp | 2 +- source/Debugger/Debugger_Types.h | 13 ++- 5 files changed, 128 insertions(+), 54 deletions(-) diff --git a/source/Debugger/Debug.cpp b/source/Debugger/Debug.cpp index 3bfceea3..dd0946e2 100644 --- a/source/Debugger/Debug.cpp +++ b/source/Debugger/Debug.cpp @@ -85,7 +85,7 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA static DebugBreakOnDMA g_DebugBreakOnDMA[NUM_BREAK_ON_DMA]; static DebugBreakOnDMA g_DebugBreakOnDMAIO; - static int g_bDebugBreakpointHit = 0; // See: BreakpointHit_t + int g_bDebugBreakpointHit = 0; // See: BreakpointHit_t int g_nBreakpoints = 0; Breakpoint_t g_aBreakpoints[ MAX_BREAKPOINTS ]; @@ -376,7 +376,7 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA // bool CheckBreakpoint (WORD address, BOOL memory); bool _CmdBreakpointAddReg ( Breakpoint_t *pBP, BreakpointSource_t iSrc, BreakpointOperator_t iCmp, WORD nAddress, int nLen, bool bIsTempBreakpoint ); int _CmdBreakpointAddCommonArg ( int iArg, int nArg, BreakpointSource_t iSrc, BreakpointOperator_t iCmp, bool bIsTempBreakpoint=false ); - void _BWZ_Clear( Breakpoint_t * aBreakWatchZero, int iSlot ); + void _BWZ_RemoveOne( Breakpoint_t *aBreakWatchZero, const int iSlot, int & nTotal ); // Config - Save bool ConfigSave_BufferToDisk ( const char *pFileName, ConfigSave_t eConfigSave ); @@ -1096,6 +1096,14 @@ bool GetBreakpointInfo ( WORD nOffset, bool & bBreakpointActive_, bool & bBreakp return false; } +// returns the hit type if the breakpoint stops +static BreakpointHit_t hitBreakpoint(Breakpoint_t * pBP, BreakpointHit_t eHitType) +{ + pBP->bHit = true; + ++pBP->nHitCount; + return pBP->bStop ? eHitType : BP_HIT_NONE; +} + // Returns true if we should continue checking breakpoint details, else false //=========================================================================== @@ -1115,6 +1123,29 @@ bool _BreakpointValid( Breakpoint_t *pBP ) //, BreakpointSource_t iSrc ) return true; } +// Stepping +void ClearTempBreakpoints() +{ + for (int iBreakpoint = 0; iBreakpoint < MAX_BREAKPOINTS; iBreakpoint++) + { + Breakpoint_t *pBP = &g_aBreakpoints[iBreakpoint]; + + if (! _BreakpointValid( pBP )) + continue; + + if (pBP->bHit && pBP->bTemp) + _BWZ_RemoveOne(g_aBreakpoints, iBreakpoint, g_nBreakpoints); + + pBP->bHit = false; + } +} + +static void DebugEnterStepping() +{ + ClearTempBreakpoints(); + g_nAppMode = MODE_STEPPING; + GetFrame().FrameRefreshStatus(DRAW_TITLE | DRAW_DISK_STATUS); +} //=========================================================================== bool _CheckBreakpointValue( Breakpoint_t *pBP, int nVal ) @@ -1215,7 +1246,7 @@ int CheckBreakpointsIO () NO_6502_TARGET }; int nBytes; - bool bBreakpointHit = 0; + int bBreakpointHit = 0; int iTarget; int nAddress; @@ -1246,17 +1277,21 @@ int CheckBreakpointsIO () if (pBP->eSource == BP_SRC_MEM_RW) { - return BP_HIT_MEM; + bBreakpointHit |= hitBreakpoint(pBP, BP_HIT_MEM); } else if (pBP->eSource == BP_SRC_MEM_READ_ONLY) { if (g_aOpcodes[opcode].nMemoryAccess & (MEM_RI|MEM_R)) - return BP_HIT_MEMR; + { + bBreakpointHit |= hitBreakpoint(pBP, BP_HIT_MEMR); + } } else if (pBP->eSource == BP_SRC_MEM_WRITE_ONLY) { if (g_aOpcodes[opcode].nMemoryAccess & (MEM_WI|MEM_W)) - return BP_HIT_MEMW; + { + bBreakpointHit |= hitBreakpoint(pBP, BP_HIT_MEMW); + } } else { @@ -1276,7 +1311,7 @@ int CheckBreakpointsIO () //=========================================================================== int CheckBreakpointsReg () { - int bBreakpointHit = 0; + int iAnyBreakpointHit = 0; for (int iBreakpoint = 0; iBreakpoint < MAX_BREAKPOINTS; iBreakpoint++) { @@ -1285,6 +1320,8 @@ int CheckBreakpointsReg () if (! _BreakpointValid( pBP )) continue; + bool bBreakpointHit = 0; + switch (pBP->eSource) { case BP_SRC_REG_PC: @@ -1311,36 +1348,18 @@ int CheckBreakpointsReg () if (bBreakpointHit) { - bBreakpointHit = BP_HIT_REG; - if (pBP->bTemp) - _BWZ_Clear(pBP, iBreakpoint); - - break; + iAnyBreakpointHit = hitBreakpoint(pBP, BP_HIT_REG); } } - return bBreakpointHit; -} - -void ClearTempBreakpoints () -{ - for (int iBreakpoint = 0; iBreakpoint < MAX_BREAKPOINTS; iBreakpoint++) - { - Breakpoint_t *pBP = &g_aBreakpoints[iBreakpoint]; - - if (! _BreakpointValid( pBP )) - continue; - - if (pBP->bTemp) - _BWZ_Clear(pBP, iBreakpoint); - } + return iAnyBreakpointHit; } // Returns true if a video breakpoint is triggered //=========================================================================== int CheckBreakpointsVideo() { - int bBreakpointHit = 0; + int iBreakpointHit = 0; for (int iBreakpoint = 0; iBreakpoint < MAX_BREAKPOINTS; iBreakpoint++) { @@ -1355,13 +1374,12 @@ int CheckBreakpointsVideo() uint16_t vert = NTSC_GetVideoVertForDebugger(); // update video scanner's vert/horz position - needed for when in fullspeed (GH#1164) if (_CheckBreakpointValue(pBP, vert)) { - bBreakpointHit = BP_HIT_VIDEO_POS; + iBreakpointHit = hitBreakpoint(pBP, BP_HIT_VIDEO_POS); pBP->bEnabled = false; // Disable, otherwise it'll trigger many times on this scan-line - break; } } - return bBreakpointHit; + return iBreakpointHit; } //=========================================================================== @@ -1547,6 +1565,9 @@ bool _CmdBreakpointAddReg( Breakpoint_t *pBP, BreakpointSource_t iSrc, Breakpoin pBP->bSet = true; pBP->bEnabled = true; pBP->bTemp = bIsTempBreakpoint; + pBP->bStop = true; + pBP->bHit = false; + pBP->nHitCount = 0; bStatus = true; } @@ -1745,18 +1766,13 @@ Update_t CmdBreakpointAddVideo(int nArgs) } //=========================================================================== -void _BWZ_Clear( Breakpoint_t * aBreakWatchZero, int iSlot ) -{ - aBreakWatchZero[ iSlot ].bSet = false; - aBreakWatchZero[ iSlot ].bEnabled = false; - aBreakWatchZero[ iSlot ].nLength = 0; -} - void _BWZ_RemoveOne( Breakpoint_t *aBreakWatchZero, const int iSlot, int & nTotal ) { if (aBreakWatchZero[iSlot].bSet) { - _BWZ_Clear( aBreakWatchZero, iSlot ); + aBreakWatchZero[ iSlot ].bSet = false; + aBreakWatchZero[ iSlot ].bEnabled = false; + aBreakWatchZero[ iSlot ].nLength = 0; nTotal--; } } @@ -1877,9 +1893,55 @@ Update_t CmdBreakpointEnable (int nArgs) { } +Update_t CmdBreakpointChange (int nArgs) { + + if (! g_nBreakpoints) + return ConsoleDisplayError("There are no (PC) Breakpoints defined."); + + if (nArgs != 2) + return Help_Arg_1( CMD_BREAKPOINT_CHANGE ); + + const int iSlot = g_aArgs[1].nValue; + if (iSlot >= 0 && iSlot < MAX_BREAKPOINTS && g_aBreakpoints[iSlot].bSet) + { + Breakpoint_t & bp = g_aBreakpoints[iSlot]; + const char * sArg = g_aArgs[2].sArg; + const int nArgLen = g_aArgs[2].nArgLen; + for (int i = 0; i < nArgLen; ++i) + { + switch (sArg[i]) + { + case 'E': + bp.bEnabled = true; + break; + case 'e': + bp.bEnabled = false; + break; + case 'T': + bp.bTemp = true; + break; + case 't': + bp.bTemp = false; + break; + case 'S': + bp.bStop = true; + break; + case 's': + bp.bStop = false; + break; + } + } + } + + return UPDATE_BREAKPOINTS; +} + void _BWZ_List( const Breakpoint_t * aBreakWatchZero, const int iBWZ ) //, bool bZeroBased ) { - static const char sFlags[] = "-*"; + static const char sEnabledFlags[] = "-E"; + static const char sStopFlags[] = "-S"; + static const char sTempFlags[] = "-T"; + static const char sHitFlags[] = " *"; std::string sAddressBuf; std::string const& sSymbol = GetSymbol(aBreakWatchZero[iBWZ].nAddress, 2, sAddressBuf); @@ -1888,10 +1950,14 @@ void _BWZ_List( const Breakpoint_t * aBreakWatchZero, const int iBWZ ) //, bool : aBreakWatchZero[iBWZ].eSource == BP_SRC_MEM_WRITE_ONLY ? 'W' : ' '; - ConsoleBufferPushFormat( " #%d %c %04X %c %s", + ConsoleBufferPushFormat( " #%d %c %c %c %c %08X %04X %c %s", // (bZeroBased ? iBWZ + 1 : iBWZ), iBWZ, - sFlags[ aBreakWatchZero[ iBWZ ].bEnabled ? 1 : 0 ], + sEnabledFlags[ aBreakWatchZero[ iBWZ ].bEnabled ? 1 : 0 ], + sStopFlags[ aBreakWatchZero[ iBWZ ].bStop ? 1 : 0 ], + sTempFlags[ aBreakWatchZero[ iBWZ ].bTemp ? 1 : 0 ], + sHitFlags[ aBreakWatchZero[ iBWZ ].bHit ? 1 : 0 ], + aBreakWatchZero[ iBWZ ].nHitCount, aBreakWatchZero[ iBWZ ].nAddress, cBPM, sSymbol.c_str() @@ -2175,8 +2241,7 @@ static Update_t CmdGo (int nArgs, const bool bFullSpeed) g_bLastGoCmdWasFullSpeed = bFullSpeed; g_bGoCmd_ReinitFlag = true; - g_nAppMode = MODE_STEPPING; - GetFrame().FrameRefreshStatus(DRAW_TITLE | DRAW_DISK_STATUS); + DebugEnterStepping(); SoundCore_SetFade(FADE_IN); @@ -2244,8 +2309,8 @@ Update_t CmdTrace (int nArgs) g_nDebugStepCycles = 0; g_nDebugStepStart = regs.pc; g_nDebugStepUntil = -1; - g_nAppMode = MODE_STEPPING; - GetFrame().FrameRefreshStatus(DRAW_TITLE | DRAW_DISK_STATUS); + + DebugEnterStepping(); DebugContinueStepping(true); return UPDATE_ALL; // TODO: Verify // 0 @@ -2302,8 +2367,7 @@ Update_t CmdTraceLine (int nArgs) g_nDebugStepStart = regs.pc; g_nDebugStepUntil = -1; - g_nAppMode = MODE_STEPPING; - GetFrame().FrameRefreshStatus(DRAW_TITLE | DRAW_DISK_STATUS); + DebugEnterStepping(); DebugContinueStepping(true); return UPDATE_ALL; // TODO: Verify // 0 @@ -8343,6 +8407,7 @@ void DebugBegin () //=========================================================================== void DebugExitDebugger () { + ClearTempBreakpoints(); // make sure we remove temp breakpoints before checking if (g_nBreakpoints == 0 && g_hTraceFile == NULL) { DebugEnd(); @@ -8598,7 +8663,6 @@ void DebugStopStepping(void) return; g_nDebugSteps = 0; // On next DebugContinueStepping(), stop single-stepping and transition to MODE_DEBUG - ClearTempBreakpoints(); } //=========================================================================== diff --git a/source/Debugger/Debug.h b/source/Debugger/Debug.h index f7c37fb5..bf7f3f21 100644 --- a/source/Debugger/Debug.h +++ b/source/Debugger/Debug.h @@ -45,6 +45,8 @@ , BP_HIT_VIDEO_POS = (1 << 12) }; + extern int g_bDebugBreakpointHit; + extern int g_nBreakpoints; extern Breakpoint_t g_aBreakpoints[ MAX_BREAKPOINTS ]; @@ -187,3 +189,5 @@ bool IsDebugSteppingAtFullSpeed(void); void DebuggerBreakOnDmaToOrFromIoMemory(WORD nAddress, bool isDmaToMemory); bool DebuggerCheckMemBreakpoints(WORD nAddress, WORD nSize, bool isDmaToMemory); + + void ClearTempBreakpoints(); diff --git a/source/Debugger/Debugger_Commands.cpp b/source/Debugger/Debugger_Commands.cpp index 13049a38..b96a5838 100644 --- a/source/Debugger/Debugger_Commands.cpp +++ b/source/Debugger/Debugger_Commands.cpp @@ -99,6 +99,7 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA {TEXT("BPL") , CmdBreakpointList , CMD_BREAKPOINT_LIST , "List all breakpoints" }, // SoftICE // {TEXT("BPLOAD") , CmdBreakpointLoad , CMD_BREAKPOINT_LOAD , "Loads breakpoints" }, {TEXT("BPSAVE") , CmdBreakpointSave , CMD_BREAKPOINT_SAVE , "Saves breakpoints" }, + {TEXT("BPCHANGE") , CmdBreakpointChange , CMD_BREAKPOINT_CHANGE , "Change breakpoint" }, // Config {TEXT("BENCHMARK") , CmdBenchmark , CMD_BENCHMARK , "Benchmark the emulator" }, {TEXT("BW") , CmdConfigColorMono , CMD_CONFIG_BW , "Sets/Shows RGB for Black & White scheme" }, diff --git a/source/Debugger/Debugger_Help.cpp b/source/Debugger/Debugger_Help.cpp index 32bc9291..573caeda 100644 --- a/source/Debugger/Debugger_Help.cpp +++ b/source/Debugger/Debugger_Help.cpp @@ -499,7 +499,7 @@ Update_t CmdHelpSpecific (int nArgs) switch ( iParam ) { case PARAM_CAT_BOOKMARKS : iCmdBegin = CMD_BOOKMARK ; iCmdEnd = CMD_BOOKMARK_SAVE ; break; - case PARAM_CAT_BREAKPOINTS: iCmdBegin = CMD_BREAK_INVALID ; iCmdEnd = CMD_BREAKPOINT_SAVE ; break; + case PARAM_CAT_BREAKPOINTS: iCmdBegin = CMD_BREAK_INVALID ; iCmdEnd = CMD_BREAKPOINT_CHANGE ; break; case PARAM_CAT_CONFIG : iCmdBegin = CMD_BENCHMARK ; iCmdEnd = CMD_CONFIG_SET_DEBUG_DIR; break; case PARAM_CAT_CPU : iCmdBegin = CMD_ASSEMBLE ; iCmdEnd = CMD_UNASSEMBLE ; break; case PARAM_CAT_FLAGS : diff --git a/source/Debugger/Debugger_Types.h b/source/Debugger/Debugger_Types.h index f2caeaad..9c5860ac 100644 --- a/source/Debugger/Debugger_Types.h +++ b/source/Debugger/Debugger_Types.h @@ -208,13 +208,16 @@ struct Breakpoint_t { - WORD nAddress; // for registers, functions as nValue - UINT nLength ; + WORD nAddress ; // for registers, functions as nValue + UINT nLength ; BreakpointSource_t eSource; BreakpointOperator_t eOperator; - bool bSet ; // used to be called enabled pre 2.0 + bool bSet ; // used to be called enabled pre 2.0 bool bEnabled; - bool bTemp; // If true then remove BP when hit or stepping cancelled (eg. G xxxx) + bool bTemp ; // If true then remove BP when hit or stepping cancelled (eg. G xxxx) + bool bHit ; // true when the breakpoint has just been hit + bool bStop ; // true if the debugger stops when it is hit + DWORD nHitCount; // number of times the breakpoint was hit }; typedef Breakpoint_t Bookmark_t; @@ -345,6 +348,7 @@ , CMD_BREAKPOINT_LIST // , CMD_BREAKPOINT_LOAD , CMD_BREAKPOINT_SAVE + , CMD_BREAKPOINT_CHANGE // Benchmark / Timing // , CMD_BENCHMARK_START // , CMD_BENCHMARK_STOP @@ -648,6 +652,7 @@ Update_t CmdBreakpointDisable (int nArgs); Update_t CmdBreakpointEdit (int nArgs); Update_t CmdBreakpointEnable (int nArgs); + Update_t CmdBreakpointChange (int nArgs); Update_t CmdBreakpointList (int nArgs); // Update_t CmdBreakpointLoad (int nArgs); Update_t CmdBreakpointSave (int nArgs);