diff --git a/CHANGES b/CHANGES index a60d4cb5b..1162b0a57 100644 --- a/CHANGES +++ b/CHANGES @@ -41,6 +41,7 @@ Misc: - Debugger: Simplify debugger state machine to play nicer with the GBA thread loop - Debugger: Merge Thumb BL instructions when disassembling - Debugger: Clean up debugger interface, removing obsolete state (fixes #67) + - Debugger: Watchpoints now report address watched (fixes #68) 0.1.1: (2015-01-24) Bugfixes: diff --git a/src/debugger/cli-debugger.c b/src/debugger/cli-debugger.c index 450e8e60f..52045d23d 100644 --- a/src/debugger/cli-debugger.c +++ b/src/debugger/cli-debugger.c @@ -407,7 +407,7 @@ static void _setWatchpoint(struct CLIDebugger* debugger, struct CLIDebugVector* static void _breakIntoDefault(int signal) { UNUSED(signal); - ARMDebuggerEnter(&_activeDebugger->d, DEBUGGER_ENTER_MANUAL); + ARMDebuggerEnter(&_activeDebugger->d, DEBUGGER_ENTER_MANUAL, 0); } static uint32_t _performOperation(enum Operation operation, uint32_t current, uint32_t next, struct CLIDebugVector* dv) { @@ -653,20 +653,32 @@ static void _commandLine(struct ARMDebugger* debugger) { } } -static void _reportEntry(struct ARMDebugger* debugger, enum DebuggerEntryReason reason) { +static void _reportEntry(struct ARMDebugger* debugger, enum DebuggerEntryReason reason, struct DebuggerEntryInfo* info) { UNUSED(debugger); switch (reason) { case DEBUGGER_ENTER_MANUAL: case DEBUGGER_ENTER_ATTACHED: break; case DEBUGGER_ENTER_BREAKPOINT: - printf("Hit breakpoint\n"); + if (info) { + printf("Hit breakpoint at 0x%08X\n", info->address); + } else { + printf("Hit breakpoint\n"); + } break; case DEBUGGER_ENTER_WATCHPOINT: - printf("Hit watchpoint\n"); + if (info) { + printf("Hit watchpoint at 0x%08X: (old value = 0x%08X)\n", info->address, info->oldValue); + } else { + printf("Hit watchpoint\n"); + } break; case DEBUGGER_ENTER_ILLEGAL_OP: - printf("Hit illegal opcode\n"); + if (info) { + printf("Hit illegal opcode at 0x%08X: 0x%08X\n", info->address, info->opcode); + } else { + printf("Hit illegal opcode\n"); + } break; } } diff --git a/src/debugger/debugger.c b/src/debugger/debugger.c index e7d7fc2f1..5dca4870e 100644 --- a/src/debugger/debugger.c +++ b/src/debugger/debugger.c @@ -22,7 +22,10 @@ static void _checkBreakpoints(struct ARMDebugger* debugger) { } for (breakpoint = debugger->breakpoints; breakpoint; breakpoint = breakpoint->next) { if (breakpoint->address + instructionLength == (uint32_t) debugger->cpu->gprs[ARM_PC]) { - ARMDebuggerEnter(debugger, DEBUGGER_ENTER_BREAKPOINT); + struct DebuggerEntryInfo info = { + .address = breakpoint->address + }; + ARMDebuggerEnter(debugger, DEBUGGER_ENTER_BREAKPOINT, &info); break; } } @@ -81,10 +84,10 @@ void ARMDebuggerRun(struct ARMDebugger* debugger) { } } -void ARMDebuggerEnter(struct ARMDebugger* debugger, enum DebuggerEntryReason reason) { +void ARMDebuggerEnter(struct ARMDebugger* debugger, enum DebuggerEntryReason reason, struct DebuggerEntryInfo* info) { debugger->state = DEBUGGER_PAUSED; if (debugger->entered) { - debugger->entered(debugger, reason); + debugger->entered(debugger, reason, info); } } @@ -110,15 +113,15 @@ void ARMDebuggerSetWatchpoint(struct ARMDebugger* debugger, uint32_t address) { if (!debugger->watchpoints) { ARMDebuggerInstallMemoryShim(debugger); } - struct DebugBreakpoint* watchpoint = malloc(sizeof(struct DebugBreakpoint)); + struct DebugWatchpoint* watchpoint = malloc(sizeof(struct DebugWatchpoint)); watchpoint->address = address; watchpoint->next = debugger->watchpoints; debugger->watchpoints = watchpoint; } void ARMDebuggerClearWatchpoint(struct ARMDebugger* debugger, uint32_t address) { - struct DebugBreakpoint** previous = &debugger->watchpoints; - struct DebugBreakpoint* breakpoint; + struct DebugWatchpoint** previous = &debugger->watchpoints; + struct DebugWatchpoint* breakpoint; for (; (breakpoint = *previous); previous = &breakpoint->next) { if (breakpoint->address == address) { *previous = breakpoint->next; diff --git a/src/debugger/debugger.h b/src/debugger/debugger.h index 040dd17d0..f205a51b5 100644 --- a/src/debugger/debugger.h +++ b/src/debugger/debugger.h @@ -24,6 +24,18 @@ struct DebugBreakpoint { uint32_t address; }; +enum WatchpointType { + WATCHPOINT_WRITE = 1, + WATCHPOINT_READ = 2, + WATCHPOINT_RW = 3 +}; + +struct DebugWatchpoint { + struct DebugWatchpoint* next; + uint32_t address; + enum WatchpointType type; +}; + enum DebuggerEntryReason { DEBUGGER_ENTER_MANUAL, DEBUGGER_ENTER_ATTACHED, @@ -32,6 +44,20 @@ enum DebuggerEntryReason { DEBUGGER_ENTER_ILLEGAL_OP }; +struct DebuggerEntryInfo { + uint32_t address; + union { + struct { + uint32_t oldValue; + enum WatchpointType watchType; + }; + + struct { + uint32_t opcode; + }; + }; +}; + enum DebuggerLogLevel { DEBUGGER_LOG_DEBUG = 0x01, DEBUGGER_LOG_INFO = 0x02, @@ -45,13 +71,13 @@ struct ARMDebugger { struct ARMCore* cpu; struct DebugBreakpoint* breakpoints; - struct DebugBreakpoint* watchpoints; + struct DebugWatchpoint* watchpoints; struct ARMMemory originalMemory; void (*init)(struct ARMDebugger*); void (*deinit)(struct ARMDebugger*); void (*paused)(struct ARMDebugger*); - void (*entered)(struct ARMDebugger*, enum DebuggerEntryReason); + void (*entered)(struct ARMDebugger*, enum DebuggerEntryReason, struct DebuggerEntryInfo*); void (*custom)(struct ARMDebugger*); __attribute__((format (printf, 3, 4))) @@ -60,7 +86,7 @@ struct ARMDebugger { void ARMDebuggerCreate(struct ARMDebugger*); void ARMDebuggerRun(struct ARMDebugger*); -void ARMDebuggerEnter(struct ARMDebugger*, enum DebuggerEntryReason); +void ARMDebuggerEnter(struct ARMDebugger*, enum DebuggerEntryReason, struct DebuggerEntryInfo*); void ARMDebuggerSetBreakpoint(struct ARMDebugger* debugger, uint32_t address); void ARMDebuggerClearBreakpoint(struct ARMDebugger* debugger, uint32_t address); void ARMDebuggerSetWatchpoint(struct ARMDebugger* debugger, uint32_t address); diff --git a/src/debugger/gdb-stub.c b/src/debugger/gdb-stub.c index e6623fca1..57be118e2 100644 --- a/src/debugger/gdb-stub.c +++ b/src/debugger/gdb-stub.c @@ -34,16 +34,34 @@ static void _gdbStubDeinit(struct ARMDebugger* debugger) { } } -static void _gdbStubEntered(struct ARMDebugger* debugger, enum DebuggerEntryReason reason) { +static void _gdbStubEntered(struct ARMDebugger* debugger, enum DebuggerEntryReason reason, struct DebuggerEntryInfo* info) { struct GDBStub* stub = (struct GDBStub*) debugger; switch (reason) { case DEBUGGER_ENTER_MANUAL: snprintf(stub->outgoing, GDB_STUB_MAX_LINE - 4, "S%02x", SIGINT); break; case DEBUGGER_ENTER_BREAKPOINT: - case DEBUGGER_ENTER_WATCHPOINT: // TODO: Make watchpoints raise with address snprintf(stub->outgoing, GDB_STUB_MAX_LINE - 4, "S%02x", SIGTRAP); break; + case DEBUGGER_ENTER_WATCHPOINT: // TODO: Make watchpoints raise with address + if (info) { + const char* type = 0; + switch (info->watchType) { + case WATCHPOINT_WRITE: + type = "watch"; + break; + case WATCHPOINT_READ: + type = "rwatch"; + break; + case WATCHPOINT_RW: + type = "awatch"; + break; + } + snprintf(stub->outgoing, GDB_STUB_MAX_LINE - 4, "T%02x%s:%08X", SIGTRAP, type, info->address); + } else { + snprintf(stub->outgoing, GDB_STUB_MAX_LINE - 4, "S%02x", SIGTRAP); + } + break; case DEBUGGER_ENTER_ILLEGAL_OP: snprintf(stub->outgoing, GDB_STUB_MAX_LINE - 4, "S%02x", SIGILL); break; @@ -279,7 +297,7 @@ static void _processVReadCommand(struct GDBStub* stub, const char* message) { stub->outgoing[0] = '\0'; if (!strncmp("Attach", message, 6)) { strncpy(stub->outgoing, "1", GDB_STUB_MAX_LINE - 4); - ARMDebuggerEnter(&stub->d, DEBUGGER_ENTER_MANUAL); + ARMDebuggerEnter(&stub->d, DEBUGGER_ENTER_MANUAL, 0); } _sendMessage(stub); } @@ -348,7 +366,7 @@ size_t _parseGDBMessage(struct GDBStub* stub, const char* message) { ++message; break; case '\x03': - ARMDebuggerEnter(&stub->d, DEBUGGER_ENTER_MANUAL); + ARMDebuggerEnter(&stub->d, DEBUGGER_ENTER_MANUAL, 0); return parsed; default: _nak(stub); @@ -514,7 +532,7 @@ void GDBStubUpdate(struct GDBStub* stub) { if (!SocketSetBlocking(stub->connection, false)) { goto connectionLost; } - ARMDebuggerEnter(&stub->d, DEBUGGER_ENTER_ATTACHED); + ARMDebuggerEnter(&stub->d, DEBUGGER_ENTER_ATTACHED, 0); } else if (errno == EWOULDBLOCK || errno == EAGAIN) { return; } else { diff --git a/src/debugger/memory-debugger.c b/src/debugger/memory-debugger.c index 1bad59879..88a759d5c 100644 --- a/src/debugger/memory-debugger.c +++ b/src/debugger/memory-debugger.c @@ -9,7 +9,8 @@ #include -static bool _checkWatchpoints(struct DebugBreakpoint* watchpoints, uint32_t address, int width); +static bool _checkWatchpoints(struct ARMDebugger* debugger, uint32_t address, struct DebuggerEntryInfo* info, int width); + static uint32_t _popcount32(unsigned bits) { bits = bits - ((bits >> 1) & 0x55555555); bits = (bits & 0x33333333) + ((bits >> 2) & 0x33333333); @@ -39,8 +40,9 @@ static uint32_t _popcount32(unsigned bits) { static RETURN ARMDebuggerShim_ ## NAME TYPES { \ struct ARMDebugger* debugger; \ FIND_DEBUGGER(debugger, cpu); \ - if (_checkWatchpoints(debugger->watchpoints, address, WIDTH)) { \ - ARMDebuggerEnter(debugger, DEBUGGER_ENTER_WATCHPOINT); \ + struct DebuggerEntryInfo info = { }; \ + if (_checkWatchpoints(debugger, address, &info, WIDTH)) { \ + ARMDebuggerEnter(debugger, DEBUGGER_ENTER_WATCHPOINT, &info); \ } \ return debugger->originalMemory.NAME(cpu, ARGS); \ } @@ -61,8 +63,9 @@ static uint32_t _popcount32(unsigned bits) { } \ unsigned i; \ for (i = 0; i < popcount; ++i) { \ - if (_checkWatchpoints(debugger->watchpoints, base + 4 * i, 4)) { \ - ARMDebuggerEnter(debugger, DEBUGGER_ENTER_WATCHPOINT); \ + struct DebuggerEntryInfo info = { }; \ + if (_checkWatchpoints(debugger, base + 4 * i, &info, 4)) { \ + ARMDebuggerEnter(debugger, DEBUGGER_ENTER_WATCHPOINT, &info); \ } \ } \ return debugger->originalMemory.NAME(cpu, address, mask, direction, cycleCounter); \ @@ -78,10 +81,24 @@ CREATE_MULTIPLE_WATCHPOINT_SHIM(loadMultiple) CREATE_MULTIPLE_WATCHPOINT_SHIM(storeMultiple) CREATE_SHIM(setActiveRegion, void, (struct ARMCore* cpu, uint32_t address), address) -static bool _checkWatchpoints(struct DebugBreakpoint* watchpoints, uint32_t address, int width) { - width -= 1; - for (; watchpoints; watchpoints = watchpoints->next) { +static bool _checkWatchpoints(struct ARMDebugger* debugger, uint32_t address, struct DebuggerEntryInfo* info, int width) { + --width; + struct DebugWatchpoint* watchpoints; + for (watchpoints = debugger->watchpoints; watchpoints; watchpoints = watchpoints->next) { if (!((watchpoints->address ^ address) & ~width)) { + switch (width + 1) { + case 1: + info->oldValue = debugger->originalMemory.load8(debugger->cpu, address, 0); + break; + case 2: + info->oldValue = debugger->originalMemory.load16(debugger->cpu, address, 0); + break; + case 4: + info->oldValue = debugger->originalMemory.load32(debugger->cpu, address, 0); + break; + } + info->address = address; + info->watchType = watchpoints->type; return true; } } diff --git a/src/gba/gba-cli.c b/src/gba/gba-cli.c index c0d21e72b..55dea6340 100644 --- a/src/gba/gba-cli.c +++ b/src/gba/gba-cli.c @@ -67,7 +67,7 @@ static bool _GBACLIDebuggerCustom(struct CLIDebuggerSystem* debugger) { if (gbaDebugger->frameAdvance) { if (!gbaDebugger->inVblank && GBARegisterDISPSTATIsInVblank(gbaDebugger->context->gba->memory.io[REG_DISPSTAT >> 1])) { - ARMDebuggerEnter(&gbaDebugger->d.p->d, DEBUGGER_ENTER_BREAKPOINT); + ARMDebuggerEnter(&gbaDebugger->d.p->d, DEBUGGER_ENTER_MANUAL, 0); gbaDebugger->frameAdvance = false; return false; } diff --git a/src/gba/gba-thread.c b/src/gba/gba-thread.c index 7a46cbbdd..a308f139b 100644 --- a/src/gba/gba-thread.c +++ b/src/gba/gba-thread.c @@ -166,7 +166,7 @@ static THREAD_ENTRY _GBAThreadRun(void* context) { if (threadContext->debugger) { threadContext->debugger->log = GBADebuggerLogShim; GBAAttachDebugger(&gba, threadContext->debugger); - ARMDebuggerEnter(threadContext->debugger, DEBUGGER_ENTER_ATTACHED); + ARMDebuggerEnter(threadContext->debugger, DEBUGGER_ENTER_ATTACHED, 0); } GBASIOSetDriverSet(&gba.sio, &threadContext->sioDrivers); diff --git a/src/gba/gba.c b/src/gba/gba.c index 8c0c063df..ddcacf8c4 100644 --- a/src/gba/gba.c +++ b/src/gba/gba.c @@ -623,7 +623,11 @@ void GBAHitStub(struct ARMCore* cpu, uint32_t opcode) { enum GBALogLevel level = GBA_LOG_FATAL; if (gba->debugger) { level = GBA_LOG_STUB; - ARMDebuggerEnter(gba->debugger, DEBUGGER_ENTER_ILLEGAL_OP); + struct DebuggerEntryInfo info = { + .address = cpu->gprs[ARM_PC], + .opcode = opcode + }; + ARMDebuggerEnter(gba->debugger, DEBUGGER_ENTER_ILLEGAL_OP, &info); } GBALog(gba, level, "Stub opcode: %08x", opcode); } @@ -632,7 +636,11 @@ void GBAIllegal(struct ARMCore* cpu, uint32_t opcode) { struct GBA* gba = (struct GBA*) cpu->master; GBALog(gba, GBA_LOG_WARN, "Illegal opcode: %08x", opcode); if (gba->debugger) { - ARMDebuggerEnter(gba->debugger, DEBUGGER_ENTER_ILLEGAL_OP); + struct DebuggerEntryInfo info = { + .address = cpu->gprs[ARM_PC], + .opcode = opcode + }; + ARMDebuggerEnter(gba->debugger, DEBUGGER_ENTER_ILLEGAL_OP, &info); } } diff --git a/src/platform/qt/GDBController.cpp b/src/platform/qt/GDBController.cpp index 68511a092..0738714d8 100644 --- a/src/platform/qt/GDBController.cpp +++ b/src/platform/qt/GDBController.cpp @@ -40,7 +40,7 @@ void GDBController::attach() { return; } m_gameController->setDebugger(&m_gdbStub.d); - ARMDebuggerEnter(&m_gdbStub.d, DEBUGGER_ENTER_ATTACHED); + ARMDebuggerEnter(&m_gdbStub.d, DEBUGGER_ENTER_ATTACHED, 0); } void GDBController::detach() { diff --git a/src/platform/sdl/sdl-events.c b/src/platform/sdl/sdl-events.c index 12bb76697..ec116030b 100644 --- a/src/platform/sdl/sdl-events.c +++ b/src/platform/sdl/sdl-events.c @@ -120,7 +120,7 @@ static void _GBASDLHandleKeypress(struct GBAThread* context, struct GBASDLEvents switch (event->keysym.sym) { case SDLK_F11: if (context->debugger) { - ARMDebuggerEnter(context->debugger, DEBUGGER_ENTER_MANUAL); + ARMDebuggerEnter(context->debugger, DEBUGGER_ENTER_MANUAL, 0); } return; #ifdef USE_PNG