From fe5c7d7a3525897b045b9accc39c200f4f051e9e Mon Sep 17 00:00:00 2001 From: Anatoly Parshintsev <114445139+aap-sc@users.noreply.github.com> Date: Fri, 14 Oct 2022 19:40:16 +0300 Subject: [PATCH] [riscv] step operation handler should respect handle_breakpoints parameter (#741) * [riscv] step operation handler should respect handle_breakpoints parameter When step operation is requested the OpenOCD frontend (like gdb server or TCL server) has an option to control how existing breakpoints are handled upon step. Some OpenOCD frontends (like gdbserver) may choose to disable special handling of existing breakpoints - thus handle_breakpoints is set to 0, while others (like TCL server) expect target handler to temporary disable the matching breakpoint to allow the step operation to complete successfully. In the current implementation handle_breakpoints parameter was ignored by target-specific handler. Thus, the following sequence of commands: ``` halt bp 4 step ``` Resulted in *step* operation to not change PC because of bp match. This commit addresses this issue. * Adjusted calls to logging facilities (addressed review comments) Co-authored-by: Tim Newsome Signed-off-by: Anatoly Parshintsev <114445139+aap-sc@users.noreply.github.com> Signed-off-by: Anatoly Parshintsev <114445139+aap-sc@users.noreply.github.com> Co-authored-by: Tim Newsome --- src/target/riscv/riscv.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 4a1145bb6..9a1e55ccb 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -2289,10 +2289,24 @@ int riscv_openocd_poll(struct target *target) int riscv_openocd_step(struct target *target, int current, target_addr_t address, int handle_breakpoints) { - LOG_DEBUG("stepping rtos hart"); + LOG_TARGET_DEBUG(target, "stepping hart"); - if (!current) - riscv_set_register(target, GDB_REGNO_PC, address); + if (!current) { + if (riscv_set_register(target, GDB_REGNO_PC, address) != ERROR_OK) + return ERROR_FAIL; + } + + struct breakpoint *breakpoint = NULL; + /* the front-end may request us not to handle breakpoints */ + if (handle_breakpoints) { + if (current) { + if (riscv_get_register(target, &address, GDB_REGNO_PC) != ERROR_OK) + return ERROR_FAIL; + } + breakpoint = breakpoint_find(target, address); + if (breakpoint && (riscv_remove_breakpoint(target, breakpoint) != ERROR_OK)) + return ERROR_FAIL; + } riscv_reg_t trigger_state[RISCV_MAX_HWBPS] = {0}; if (disable_triggers(target, trigger_state) != ERROR_OK) @@ -2332,6 +2346,11 @@ _exit: LOG_ERROR("unable to enable triggers"); } + if (breakpoint && (riscv_add_breakpoint(target, breakpoint) != ERROR_OK)) { + success = false; + LOG_TARGET_ERROR(target, "unable to restore the disabled breakpoint"); + } + if (success) { target->state = TARGET_RUNNING; target_call_event_callbacks(target, TARGET_EVENT_RESUMED);