auracaster/sw_openocd
3
0
Fork 0
forked from auracaster/openocd
Code Pull Requests Activity

target: Rework 'set' variable of break-/watchpoints

Browse Source 
The 'set' variable name suggests a boolean data type which determines
whether a breakpoint (or watchpoint) is active. However, it is also
used to store the number of the breakpoint.

This encoding leads to inconsistent value assignments: boolean and
integer values are mixed. Also, associated hardware comparator
numbers, which are usually numbered from 0, cannot be used directly.
An additional offset is required to store the comparator numbers.

In order to make the code more readable and the value assignment more
consistent, change the variable name to 'is_set', its data type to 'bool'
and introduce a dedicated variable for the break-/watchpoint
number.

In order to make the review easier, the data types of various related
variables (e.g. number of breakpoints) are not changed.

While at it, fix a few coding style issues.

Change-Id: I2193f5639247cce6b80580d4c1c6afee916aeb82
Signed-off-by: Marc Schink <dev@zapb.de>
Reviewed-on: https://review.openocd.org/c/openocd/+/6319
Tested-by: jenkins
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
This commit is contained in:
Marc Schink
2021-06-13 11:21:18 +02:00
committed by Antonio Borneo
parent ab43721be6
commit fb43f1ff4e
21 changed files with 258 additions and 242 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
src/target/x86_32_common.c
View File

@@ -862,7 +862,7 @@ int x86_32_common_remove_watchpoint(struct target *t, struct watchpoint *wp)
{
if (check_not_halted(t))
return ERROR_TARGET_NOT_HALTED;
if (wp->set)
if (wp->is_set)
unset_watchpoint(t, wp);
return ERROR_OK;
}
@@ -883,7 +883,7 @@ int x86_32_common_remove_breakpoint(struct target *t, struct breakpoint *bp)
LOG_DEBUG("type=%d, addr=" TARGET_ADDR_FMT, bp->type, bp->address);
if (check_not_halted(t))
return ERROR_TARGET_NOT_HALTED;
if (bp->set)
if (bp->is_set)
unset_breakpoint(t, bp);
return ERROR_OK;
@@ -995,7 +995,7 @@ static int set_hwbp(struct target *t, struct breakpoint *bp)
}
if (set_debug_regs(t, bp->address, hwbp_num, DR7_BP_EXECUTE, 1) != ERROR_OK)
return ERROR_FAIL;
bp->set = hwbp_num + 1;
breakpoint_hw_set(bp, hwbp_num);
debug_reg_list[hwbp_num].used = 1;
debug_reg_list[hwbp_num].bp_value = bp->address;
LOG_USER("%s hardware breakpoint %" PRIu32 " set at 0x%08" PRIx32 " (hwreg=%" PRIu8 ")", __func__,
@@ -1007,9 +1007,9 @@ static int unset_hwbp(struct target *t, struct breakpoint *bp)
{
struct x86_32_common *x86_32 = target_to_x86_32(t);
struct x86_32_dbg_reg *debug_reg_list = x86_32->hw_break_list;
int hwbp_num = bp->set - 1;
int hwbp_num = bp->number;
if ((hwbp_num < 0) || (hwbp_num >= x86_32->num_hw_bpoints)) {
if (hwbp_num >= x86_32->num_hw_bpoints) {
LOG_ERROR("%s invalid breakpoint number=%d, bpid=%" PRIu32,
__func__, hwbp_num, bp->unique_id);
return ERROR_OK;
@@ -1055,7 +1055,7 @@ static int set_swbp(struct target *t, struct breakpoint *bp)
__func__, readback, *bp->orig_instr);
return ERROR_FAIL;
}
bp->set = SW_BP_OPCODE; /* just non 0 */
bp->is_set = true;
/* add the memory patch */
struct swbp_mem_patch *new_patch = malloc(sizeof(struct swbp_mem_patch));
@@ -1134,7 +1134,7 @@ static int set_breakpoint(struct target *t, struct breakpoint *bp)
int error = ERROR_OK;
struct x86_32_common *x86_32 = target_to_x86_32(t);
LOG_DEBUG("type=%d, addr=" TARGET_ADDR_FMT, bp->type, bp->address);
if (bp->set) {
if (bp->is_set) {
LOG_ERROR("breakpoint already set");
return error;
}
@@ -1164,7 +1164,7 @@ static int set_breakpoint(struct target *t, struct breakpoint *bp)
static int unset_breakpoint(struct target *t, struct breakpoint *bp)
{
LOG_DEBUG("type=%d, addr=" TARGET_ADDR_FMT, bp->type, bp->address);
if (!bp->set) {
if (!bp->is_set) {
LOG_WARNING("breakpoint not set");
return ERROR_OK;
}
@@ -1182,7 +1182,7 @@ static int unset_breakpoint(struct target *t, struct breakpoint *bp)
return ERROR_FAIL;
}
}
bp->set = 0;
bp->is_set = false;
return ERROR_OK;
}
@@ -1193,7 +1193,7 @@ static int set_watchpoint(struct target *t, struct watchpoint *wp)
int wp_num = 0;
LOG_DEBUG("type=%d, addr=" TARGET_ADDR_FMT, wp->rw, wp->address);
if (wp->set) {
if (wp->is_set) {
LOG_ERROR("%s watchpoint already set", __func__);
return ERROR_OK;
}
@@ -1233,7 +1233,7 @@ static int set_watchpoint(struct target *t, struct watchpoint *wp)
LOG_ERROR("%s only 'access' or 'write' watchpoints are supported", __func__);
break;
}
wp->set = wp_num + 1;
watchpoint_set(wp, wp_num);
debug_reg_list[wp_num].used = 1;
debug_reg_list[wp_num].bp_value = wp->address;
LOG_USER("'%s' watchpoint %d set at " TARGET_ADDR_FMT " with length %" PRIu32 " (hwreg=%d)",
@@ -1248,13 +1248,13 @@ static int unset_watchpoint(struct target *t, struct watchpoint *wp)
struct x86_32_common *x86_32 = target_to_x86_32(t);
struct x86_32_dbg_reg *debug_reg_list = x86_32->hw_break_list;
LOG_DEBUG("type=%d, addr=" TARGET_ADDR_FMT, wp->rw, wp->address);
if (!wp->set) {
if (!wp->is_set) {
LOG_WARNING("watchpoint not set");
return ERROR_OK;
}
int wp_num = wp->set - 1;
if ((wp_num < 0) || (wp_num >= x86_32->num_hw_bpoints)) {
int wp_num = wp->number;
if (wp_num >= x86_32->num_hw_bpoints) {
LOG_DEBUG("Invalid FP Comparator number in watchpoint");
return ERROR_OK;
}
@@ -1263,7 +1263,7 @@ static int unset_watchpoint(struct target *t, struct watchpoint *wp)
debug_reg_list[wp_num].used = 0;
debug_reg_list[wp_num].bp_value = 0;
wp->set = 0;
wp->is_set = false;
LOG_USER("'%s' watchpoint %d removed from " TARGET_ADDR_FMT " with length %" PRIu32 " (hwreg=%d)",
wp->rw == WPT_READ ? "read" : wp->rw == WPT_WRITE ?