From 19aea4096be158ca2e557ed39fa5abd66cb75d75 Mon Sep 17 00:00:00 2001 From: Lior Halphon Date: Fri, 1 Jul 2016 18:24:21 +0300 Subject: [PATCH] Added condition breakpoint. Fixed a possible crash when deleting a breakpoint. --- Core/debugger.c | 84 ++++++++++++++++++++++++++++++++++++++++++------- Core/gb.h | 4 ++- 2 files changed, 75 insertions(+), 13 deletions(-) diff --git a/Core/debugger.c b/Core/debugger.c index 547d94c8..45c05a61 100644 --- a/Core/debugger.c +++ b/Core/debugger.c @@ -20,6 +20,11 @@ typedef struct { }; } lvalue_t; +struct GB_breakpoint_s { + uint16_t addr; + char *condition; +}; + static uint16_t read_lvalue(GB_gameboy_t *gb, lvalue_t lvalue) { /* Not used until we add support for operators like += */ @@ -438,8 +443,8 @@ static uint16_t find_breakpoint(GB_gameboy_t *gb, uint16_t addr) int max = gb->n_breakpoints; while (min < max) { uint16_t pivot = (min + max) / 2; - if (gb->breakpoints[pivot] == addr) return pivot; - if (gb->breakpoints[pivot] > addr) { + if (gb->breakpoints[pivot].addr == addr) return pivot; + if (gb->breakpoints[pivot].addr > addr) { max = pivot - 1; } else { @@ -452,24 +457,55 @@ static uint16_t find_breakpoint(GB_gameboy_t *gb, uint16_t addr) static bool breakpoint(GB_gameboy_t *gb, char *arguments) { if (strlen(lstrip(arguments)) == 0) { - GB_log(gb, "Usage: breakpoint \n"); + GB_log(gb, "Usage: breakpoint [ if ]\n"); return true; } + char *condition = NULL; + if ((condition = strstr(arguments, " if "))) { + *condition = 0; + condition += strlen(" if "); + /* Verify condition is sane (Todo: This might have side effects!) */ + bool error; + debugger_evaluate(gb, arguments, (unsigned int)strlen(arguments), &error); + if (error) return true; + + } + bool error; uint16_t result = debugger_evaluate(gb, arguments, (unsigned int)strlen(arguments), &error); if (error) return true; uint16_t index = find_breakpoint(gb, result); - if (index < gb->n_breakpoints && gb->breakpoints[index] == result) { + if (index < gb->n_breakpoints && gb->breakpoints[index].addr == result) { GB_log(gb, "Breakpoint already set at %04x\n", result); + if (!gb->breakpoints[index].condition && condition) { + GB_log(gb, "Added condition to breakpoint\n"); + gb->breakpoints[index].condition = strdup(condition); + } + else if (gb->breakpoints[index].condition && condition) { + GB_log(gb, "Replaced breakpoint condition\n"); + free(gb->breakpoints[index].condition); + gb->breakpoints[index].condition = strdup(condition); + } + else if (gb->breakpoints[index].condition && !condition) { + GB_log(gb, "Removed breakpoint condition\n"); + free(gb->breakpoints[index].condition); + gb->breakpoints[index].condition = NULL; + } return true; } gb->breakpoints = realloc(gb->breakpoints, (gb->n_breakpoints + 1) * sizeof(gb->breakpoints[0])); memmove(&gb->breakpoints[index + 1], &gb->breakpoints[index], (gb->n_breakpoints - index) * sizeof(gb->breakpoints[0])); - gb->breakpoints[index] = result; + gb->breakpoints[index].addr = result; + if (condition) { + gb->breakpoints[index].condition = strdup(condition); + } + else { + gb->breakpoints[index].condition = NULL; + } gb->n_breakpoints++; GB_log(gb, "Breakpoint set at %04x\n", result); @@ -482,6 +518,11 @@ static bool delete(GB_gameboy_t *gb, char *arguments) GB_log(gb, "Delete all breakpoints? "); char *answer = gb->input_callback(gb); if (answer[0] == 'Y' || answer[0] == 'y') { + for (unsigned i = gb->n_breakpoints; i--;) { + if (gb->breakpoints[i].condition) { + free(gb->breakpoints[i].condition); + } + } free(gb->breakpoints); gb->breakpoints = NULL; gb->n_breakpoints = 0; @@ -495,12 +536,16 @@ static bool delete(GB_gameboy_t *gb, char *arguments) if (error) return true; uint16_t index = find_breakpoint(gb, result); - if (index >= gb->n_breakpoints || gb->breakpoints[index] != result) { + if (index >= gb->n_breakpoints || gb->breakpoints[index].addr != result) { GB_log(gb, "No breakpoint set at %04x\n", result); return true; } - memmove(&gb->breakpoints[index], &gb->breakpoints[index + 1], (gb->n_breakpoints - index) * sizeof(gb->breakpoints[0])); + if (gb->breakpoints[index].condition) { + free(gb->breakpoints[index].condition); + } + + memmove(&gb->breakpoints[index], &gb->breakpoints[index + 1], (gb->n_breakpoints - index - 1) * sizeof(gb->breakpoints[0])); gb->n_breakpoints--; gb->breakpoints = realloc(gb->breakpoints, gb->n_breakpoints * sizeof(gb->breakpoints[0])); @@ -522,7 +567,12 @@ static bool list(GB_gameboy_t *gb, char *arguments) GB_log(gb, "%d breakpoint(s) set:\n", gb->n_breakpoints); for (uint16_t i = 0; i < gb->n_breakpoints; i++) { - GB_log(gb, " %d. %04x\n", i + 1, gb->breakpoints[i]); + if (gb->breakpoints[i].condition) { + GB_log(gb, " %d. %04x (Condition: %s)\n", i + 1, gb->breakpoints[i].addr, gb->breakpoints[i].condition); + } + else { + GB_log(gb, " %d. %04x\n", i + 1, gb->breakpoints[i].addr); + } } return true; @@ -531,8 +581,19 @@ static bool list(GB_gameboy_t *gb, char *arguments) static bool should_break(GB_gameboy_t *gb, uint16_t addr) { uint16_t index = find_breakpoint(gb, addr); - if (index < gb->n_breakpoints && gb->breakpoints[index] == addr) { - return true; + if (index < gb->n_breakpoints && gb->breakpoints[index].addr == addr) { + if (!gb->breakpoints[index].condition) { + return true; + } + bool error; + bool condition = debugger_evaluate(gb, gb->breakpoints[index].condition, + (unsigned int)strlen(gb->breakpoints[index].condition), &error); + if (error) { + /* Should never happen */ + GB_log(gb, "An internal error has occured\n"); + return true; + } + return condition; } return false; } @@ -611,7 +672,6 @@ static bool mbc(GB_gameboy_t *gb, char *arguments) GB_log(gb, "Cart contains a real time clock\n"); } - return true; } @@ -625,7 +685,7 @@ static const debugger_command_t commands[] = { {"registers", 1, registers, "Print values of processor registers and other important registers"}, {"cartridge", 2, mbc, "Displays information about the MBC and cartridge"}, {"mbc", 3, mbc, NULL}, - {"breakpoint", 1, breakpoint, "Add a new breakpoint at the specified address/expression"}, + {"breakpoint", 1, breakpoint, "Add a new breakpoint at the specified address/expression. Can also modify the condition of existing breakpoints."}, {"list", 1, list, "List all set breakpoints"}, {"delete", 2, delete, "Delete a breakpoint by its address, or all breakpoints"}, {"print", 1, print, "Evaluate and print an expression"}, diff --git a/Core/gb.h b/Core/gb.h index e7bbed2e..68df73ca 100644 --- a/Core/gb.h +++ b/Core/gb.h @@ -180,6 +180,8 @@ typedef struct { /* Todo: We might want to typedef our own bool if this prevents SameBoy from working on specific platforms. */ _Static_assert(sizeof(bool) == 1, "sizeof(bool) != 1"); +struct GB_breakpoint_s; + typedef struct GB_gameboy_s { GB_SECTION(header, /* The magic makes sure a state file is: @@ -315,7 +317,7 @@ typedef struct GB_gameboy_s { int debug_call_depth; bool debug_fin_command, debug_next_command; uint16_t n_breakpoints; - uint16_t *breakpoints; + struct GB_breakpoint_s *breakpoints; bool stack_leak_detection; uint16_t sp_for_call_depth[0x200]; /* Should be much more than enough */ uint16_t addr_for_call_depth[0x200];