[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 <current_pc> 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 <tim@sifive.com>
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 <tim@sifive.com>

1 files changed, 22 insertions, 3 deletions

diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c
index 4a1145b..9a1e55c 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);