From 2c8c2cb6b1426afc73519a7445a71a0aed36cf0f Mon Sep 17 00:00:00 2001 From: Marek Vrbka Date: Mon, 18 Sep 2023 14:32:44 +0200 Subject: [PATCH 1/2] command: Prepend logs during command capture Previously, if you ran a tcl command in capture like so: "capture { reg 0x1000 hw }" Such command did overwrite the tcl result if LOG_LVL_INFO or lower was logged during it. This patch changes it by prepending the log to the tcl result instead. As the tcl results should not be lost during capture. Change-Id: I37381b45e15c931ba2844d65c9d38f6ed2f6e4fd Signed-off-by: Marek Vrbka Reviewed-on: https://review.openocd.org/c/openocd/+/7902 Reviewed-by: Antonio Borneo Tested-by: jenkins Reviewed-by: Jan Matyas --- doc/openocd.texi | 2 +- src/helper/command.c | 26 ++++++++++++++------------ 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/doc/openocd.texi b/doc/openocd.texi index 2d59238b8..6ec280ad2 100644 --- a/doc/openocd.texi +++ b/doc/openocd.texi @@ -12475,7 +12475,7 @@ Return information about the flash banks @item @b{capture} <@var{command}> Run <@var{command}> and return full log output that was produced during -its execution. Example: +its execution together with the command output. Example: @example > capture "reset init" diff --git a/src/helper/command.c b/src/helper/command.c index 945b890b3..ef50ab5bd 100644 --- a/src/helper/command.c +++ b/src/helper/command.c @@ -99,8 +99,7 @@ static struct log_capture_state *command_log_capture_start(Jim_Interp *interp) * The tcl return value is empty for openocd commands that provide * progress output. * - * Therefore we set the tcl return value only if we actually - * captured output. + * For other commands, we prepend the logs to the tcl return value. */ static void command_log_capture_finish(struct log_capture_state *state) { @@ -109,15 +108,18 @@ static void command_log_capture_finish(struct log_capture_state *state) log_remove_callback(tcl_output, state); - int length; - Jim_GetString(state->output, &length); + int loglen; + const char *log_result = Jim_GetString(state->output, &loglen); + int reslen; + const char *cmd_result = Jim_GetString(Jim_GetResult(state->interp), &reslen); - if (length > 0) - Jim_SetResult(state->interp, state->output); - else { - /* No output captured, use tcl return value (which could - * be empty too). */ - } + // Just in case the log doesn't end with a newline, we add it + if (loglen != 0 && reslen != 0 && log_result[loglen - 1] != '\n') + Jim_AppendString(state->interp, state->output, "\n", 1); + + Jim_AppendString(state->interp, state->output, cmd_result, reslen); + + Jim_SetResult(state->interp, state->output); Jim_DecrRefCount(state->interp, state->output); free(state); @@ -691,8 +693,8 @@ COMMAND_HANDLER(handle_echo) return ERROR_OK; } -/* Capture progress output and return as tcl return value. If the - * progress output was empty, return tcl return value. +/* Return both the progress output (LOG_INFO and higher) + * and the tcl return value of a command. */ static int jim_capture(Jim_Interp *interp, int argc, Jim_Obj *const *argv) { From eba5d211937d1ebcb3669810ff63ad1083600b67 Mon Sep 17 00:00:00 2001 From: Marek Vrbka Date: Fri, 22 Sep 2023 13:57:04 +0200 Subject: [PATCH 2/2] breakpoints: add rwp all command This patch adds the "all" option to the rwp command. It removes all watchpoints, much like rbp all removes all breakpoints. Change-Id: Id58dd103085e558f17afa4a287888cf085566ca9 Signed-off-by: Marek Vrbka Reviewed-on: https://review.openocd.org/c/openocd/+/7907 Tested-by: jenkins Reviewed-by: Antonio Borneo --- doc/openocd.texi | 4 +- src/target/breakpoints.c | 104 +++++++++++++++++++++++++++------------ src/target/breakpoints.h | 1 + src/target/target.c | 25 +++++++--- 4 files changed, 93 insertions(+), 41 deletions(-) diff --git a/doc/openocd.texi b/doc/openocd.texi index 6ec280ad2..a2965189f 100644 --- a/doc/openocd.texi +++ b/doc/openocd.texi @@ -9358,8 +9358,8 @@ for similar mechanisms that do not consume hardware breakpoints.) Remove the breakpoint at @var{address} or all breakpoints. @end deffn -@deffn {Command} {rwp} address -Remove data watchpoint on @var{address} +@deffn {Command} {rwp} @option{all} | address +Remove data watchpoint on @var{address} or all watchpoints. @end deffn @deffn {Command} {wp} [address len [(@option{r}|@option{w}|@option{a}) [value [mask]]]] diff --git a/src/target/breakpoints.c b/src/target/breakpoints.c index 4a613cc28..07d0a7371 100644 --- a/src/target/breakpoints.c +++ b/src/target/breakpoints.c @@ -17,6 +17,11 @@ #include "breakpoints.h" #include "smp.h" +enum breakpoint_watchpoint { + BREAKPOINT, + WATCHPOINT, +}; + static const char * const breakpoint_type_strings[] = { "hardware", "software" @@ -375,26 +380,90 @@ int breakpoint_remove(struct target *target, target_addr_t address) return retval; } -int breakpoint_remove_all(struct target *target) +static int watchpoint_free(struct target *target, struct watchpoint *watchpoint_to_remove) { + struct watchpoint *watchpoint = target->watchpoints; + struct watchpoint **watchpoint_p = &target->watchpoints; + int retval; + + while (watchpoint) { + if (watchpoint == watchpoint_to_remove) + break; + watchpoint_p = &watchpoint->next; + watchpoint = watchpoint->next; + } + + if (!watchpoint) + return ERROR_OK; + retval = target_remove_watchpoint(target, watchpoint); + if (retval != ERROR_OK) { + LOG_TARGET_ERROR(target, "could not remove watchpoint #%d on this target", + watchpoint->number); + return retval; + } + + LOG_DEBUG("free WPID: %d --> %d", watchpoint->unique_id, retval); + (*watchpoint_p) = watchpoint->next; + free(watchpoint); + + return ERROR_OK; +} + +static int watchpoint_remove_all_internal(struct target *target) +{ + struct watchpoint *watchpoint = target->watchpoints; + int retval = ERROR_OK; + + while (watchpoint) { + struct watchpoint *tmp = watchpoint; + watchpoint = watchpoint->next; + int status = watchpoint_free(target, tmp); + if (status != ERROR_OK) + retval = status; + } + + return retval; +} + +int breakpoint_watchpoint_remove_all(struct target *target, enum breakpoint_watchpoint bp_wp) +{ + assert(bp_wp == BREAKPOINT || bp_wp == WATCHPOINT); int retval = ERROR_OK; if (target->smp) { struct target_list *head; foreach_smp_target(head, target->smp_targets) { struct target *curr = head->target; - int status = breakpoint_remove_all_internal(curr); + + int status = ERROR_OK; + if (bp_wp == BREAKPOINT) + status = breakpoint_remove_all_internal(curr); + else + status = watchpoint_remove_all_internal(curr); if (status != ERROR_OK) retval = status; } } else { - retval = breakpoint_remove_all_internal(target); + if (bp_wp == BREAKPOINT) + retval = breakpoint_remove_all_internal(target); + else + retval = watchpoint_remove_all_internal(target); } return retval; } +int breakpoint_remove_all(struct target *target) +{ + return breakpoint_watchpoint_remove_all(target, BREAKPOINT); +} + +int watchpoint_remove_all(struct target *target) +{ + return breakpoint_watchpoint_remove_all(target, WATCHPOINT); +} + static int breakpoint_clear_target_internal(struct target *target) { LOG_DEBUG("Delete all breakpoints for target: %s", @@ -532,35 +601,6 @@ int watchpoint_add(struct target *target, target_addr_t address, } } -static int watchpoint_free(struct target *target, struct watchpoint *watchpoint_to_remove) -{ - struct watchpoint *watchpoint = target->watchpoints; - struct watchpoint **watchpoint_p = &target->watchpoints; - int retval; - - while (watchpoint) { - if (watchpoint == watchpoint_to_remove) - break; - watchpoint_p = &watchpoint->next; - watchpoint = watchpoint->next; - } - - if (!watchpoint) - return ERROR_OK; - retval = target_remove_watchpoint(target, watchpoint); - if (retval != ERROR_OK) { - LOG_TARGET_ERROR(target, "could not remove watchpoint #%d on this target", - watchpoint->number); - return retval; - } - - LOG_DEBUG("free WPID: %d --> %d", watchpoint->unique_id, retval); - (*watchpoint_p) = watchpoint->next; - free(watchpoint); - - return ERROR_OK; -} - static int watchpoint_remove_internal(struct target *target, target_addr_t address) { struct watchpoint *watchpoint = target->watchpoints; diff --git a/src/target/breakpoints.h b/src/target/breakpoints.h index daa31f711..64c0ce2a5 100644 --- a/src/target/breakpoints.h +++ b/src/target/breakpoints.h @@ -73,6 +73,7 @@ int watchpoint_add(struct target *target, target_addr_t address, uint32_t length, enum watchpoint_rw rw, uint64_t value, uint64_t mask); int watchpoint_remove(struct target *target, target_addr_t address); +int watchpoint_remove_all(struct target *target); /* report type and address of just hit watchpoint */ int watchpoint_hit(struct target *target, enum watchpoint_rw *rw, diff --git a/src/target/target.c b/src/target/target.c index acd351a66..bed8a2cbc 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -4136,17 +4136,28 @@ COMMAND_HANDLER(handle_wp_command) COMMAND_HANDLER(handle_rwp_command) { + int retval; + if (CMD_ARGC != 1) return ERROR_COMMAND_SYNTAX_ERROR; - target_addr_t addr; - COMMAND_PARSE_ADDRESS(CMD_ARGV[0], addr); - struct target *target = get_current_target(CMD_CTX); - int retval = watchpoint_remove(target, addr); + if (!strcmp(CMD_ARGV[0], "all")) { + retval = watchpoint_remove_all(target); - if (retval != ERROR_OK) - command_print(CMD, "Error during removal of watchpoint at address " TARGET_ADDR_FMT, addr); + if (retval != ERROR_OK) { + command_print(CMD, "Error encountered during removal of all watchpoints."); + command_print(CMD, "Some watchpoints may have remained set."); + } + } else { + target_addr_t addr; + COMMAND_PARSE_ADDRESS(CMD_ARGV[0], addr); + + retval = watchpoint_remove(target, addr); + + if (retval != ERROR_OK) + command_print(CMD, "Error during removal of watchpoint at address " TARGET_ADDR_FMT, addr); + } return retval; } @@ -7065,7 +7076,7 @@ static const struct command_registration target_exec_command_handlers[] = { .handler = handle_rwp_command, .mode = COMMAND_EXEC, .help = "remove watchpoint", - .usage = "address", + .usage = "'all' | address", }, { .name = "load_image",