From fd8a3c9516d6580ab749e4127f6dab0077a8b078 Mon Sep 17 00:00:00 2001 From: Tomas Vanek Date: Thu, 24 Jan 2019 14:33:16 +0100 Subject: [PATCH] target, breakpoints: improve error handling handle_bp_command_set() showed the error message "Failure setting breakpoint, the same address(IVA) is already used" on any error returned from (xxx_)breakpoint_add(). Paradoxically breakpoint_add() returned ERROR_OK if it detected duplicated bp address. context_breakpoint_add() and hybrid_breakpoint_add() returned -1 instead of OpenOCD compatible error if they detected duplicity. Introduce ERROR_TARGET_DUPLICATE_BREAKPOINT Unify error handling to LOG_ERROR() any error in (xxx_)breakpoint_add() Remove misleading error messages from handle_bp_command_set() handle_bp_command_set() returns error if the target does not implement add_context_breakpoint or add_hybrid_breakpoint. Change-Id: If17dfad1756d82a77028ebdc4b305f9c8e1365ba Signed-off-by: Tomas Vanek Reviewed-on: http://openocd.zylin.com/4871 Tested-by: jenkins Reviewed-by: Matthias Welwarsky --- src/target/breakpoints.c | 16 ++++++++-------- src/target/target.c | 27 ++++++++++----------------- src/target/target.h | 1 + 3 files changed, 19 insertions(+), 25 deletions(-) diff --git a/src/target/breakpoints.c b/src/target/breakpoints.c index 58bcc86159..94e8a823d0 100644 --- a/src/target/breakpoints.c +++ b/src/target/breakpoints.c @@ -60,9 +60,9 @@ int breakpoint_add_internal(struct target *target, * breakpoint" ... check all the parameters before * succeeding. */ - LOG_DEBUG("Duplicate Breakpoint address: " TARGET_ADDR_FMT " (BP %" PRIu32 ")", + LOG_ERROR("Duplicate Breakpoint address: " TARGET_ADDR_FMT " (BP %" PRIu32 ")", address, breakpoint->unique_id); - return ERROR_OK; + return ERROR_TARGET_DUPLICATE_BREAKPOINT; } breakpoint_p = &breakpoint->next; breakpoint = breakpoint->next; @@ -124,9 +124,9 @@ int context_breakpoint_add_internal(struct target *target, * breakpoint" ... check all the parameters before * succeeding. */ - LOG_DEBUG("Duplicate Breakpoint asid: 0x%08" PRIx32 " (BP %" PRIu32 ")", + LOG_ERROR("Duplicate Breakpoint asid: 0x%08" PRIx32 " (BP %" PRIu32 ")", asid, breakpoint->unique_id); - return -1; + return ERROR_TARGET_DUPLICATE_BREAKPOINT; } breakpoint_p = &breakpoint->next; breakpoint = breakpoint->next; @@ -176,13 +176,13 @@ int hybrid_breakpoint_add_internal(struct target *target, * breakpoint" ... check all the parameters before * succeeding. */ - LOG_DEBUG("Duplicate Hybrid Breakpoint asid: 0x%08" PRIx32 " (BP %" PRIu32 ")", + LOG_ERROR("Duplicate Hybrid Breakpoint asid: 0x%08" PRIx32 " (BP %" PRIu32 ")", asid, breakpoint->unique_id); - return -1; + return ERROR_TARGET_DUPLICATE_BREAKPOINT; } else if ((breakpoint->address == address) && (breakpoint->asid == 0)) { - LOG_DEBUG("Duplicate Breakpoint IVA: " TARGET_ADDR_FMT " (BP %" PRIu32 ")", + LOG_ERROR("Duplicate Breakpoint IVA: " TARGET_ADDR_FMT " (BP %" PRIu32 ")", address, breakpoint->unique_id); - return -1; + return ERROR_TARGET_DUPLICATE_BREAKPOINT; } breakpoint_p = &breakpoint->next; diff --git a/src/target/target.c b/src/target/target.c index 7e30d789b6..693eb519eb 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -3719,38 +3719,31 @@ static int handle_bp_command_set(struct command_context *cmd_ctx, if (asid == 0) { retval = breakpoint_add(target, addr, length, hw); + /* error is always logged in breakpoint_add(), do not print it again */ if (ERROR_OK == retval) command_print(cmd_ctx, "breakpoint set at " TARGET_ADDR_FMT "", addr); - else { - LOG_ERROR("Failure setting breakpoint, the same address(IVA) is already used"); - return retval; - } + } else if (addr == 0) { if (target->type->add_context_breakpoint == NULL) { - LOG_WARNING("Context breakpoint not available"); - return ERROR_OK; + LOG_ERROR("Context breakpoint not available"); + return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; } retval = context_breakpoint_add(target, asid, length, hw); + /* error is always logged in context_breakpoint_add(), do not print it again */ if (ERROR_OK == retval) command_print(cmd_ctx, "Context breakpoint set at 0x%8.8" PRIx32 "", asid); - else { - LOG_ERROR("Failure setting breakpoint, the same address(CONTEXTID) is already used"); - return retval; - } + } else { if (target->type->add_hybrid_breakpoint == NULL) { - LOG_WARNING("Hybrid breakpoint not available"); - return ERROR_OK; + LOG_ERROR("Hybrid breakpoint not available"); + return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; } retval = hybrid_breakpoint_add(target, addr, asid, length, hw); + /* error is always logged in hybrid_breakpoint_add(), do not print it again */ if (ERROR_OK == retval) command_print(cmd_ctx, "Hybrid breakpoint set at 0x%8.8" PRIx32 "", asid); - else { - LOG_ERROR("Failure setting breakpoint, the same address is already used"); - return retval; - } } - return ERROR_OK; + return retval; } COMMAND_HANDLER(handle_bp_command) diff --git a/src/target/target.h b/src/target/target.h index fb9d714658..983f450ac0 100644 --- a/src/target/target.h +++ b/src/target/target.h @@ -725,6 +725,7 @@ void target_handle_event(struct target *t, enum target_event e); #define ERROR_TARGET_TRANSLATION_FAULT (-309) #define ERROR_TARGET_NOT_RUNNING (-310) #define ERROR_TARGET_NOT_EXAMINED (-311) +#define ERROR_TARGET_DUPLICATE_BREAKPOINT (-312) extern bool get_target_reset_nag(void); -- 2.30.2