GDB assumes that watchpoint set via the gdbstub remote protocol will behave in the same way as hardware watchpoints for the target. In particular, whether the CPU stops with the PC before or after the insn which triggers the watchpoint is target dependent. Allow guest CPU code to specify which behaviour to use. This fixes a bug where with guest CPUs which stop before the accessing insn GDB would manually step forward over what it thought was the insn and end up one insn further forward than it should be.
We set this flag for the CPU architectures which set gdbarch_have_nonsteppable_watchpoint in gdb 7.7: ARM, CRIS, LM32, MIPS and Xtensa. Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> --- I have tested ARM and confirmed that this patch is needed to give correct watchpoint behaviour with a gdb attached to our debug stub. I haven't tested the other targets beyond compile testing, but I have checked the gdb sources to get the list of which targets needed changes. gdbstub.c | 32 +++++++++++++++++++++++--------- include/qom/cpu.h | 3 +++ target-arm/cpu.c | 1 + target-cris/cpu.c | 1 + target-lm32/cpu.c | 1 + target-mips/cpu.c | 1 + target-xtensa/cpu.c | 1 + 7 files changed, 31 insertions(+), 9 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 8afe0b7..d500cc5 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -625,11 +625,23 @@ void gdb_register_coprocessor(CPUState *cpu, } #ifndef CONFIG_USER_ONLY -static const int xlat_gdb_type[] = { - [GDB_WATCHPOINT_WRITE] = BP_GDB | BP_MEM_WRITE, - [GDB_WATCHPOINT_READ] = BP_GDB | BP_MEM_READ, - [GDB_WATCHPOINT_ACCESS] = BP_GDB | BP_MEM_ACCESS, -}; +/* Translate GDB watchpoint type to a flags value for cpu_watchpoint_* */ +static inline int xlat_gdb_type(CPUState *cpu, int gdbtype) +{ + static const int xlat[] = { + [GDB_WATCHPOINT_WRITE] = BP_GDB | BP_MEM_WRITE, + [GDB_WATCHPOINT_READ] = BP_GDB | BP_MEM_READ, + [GDB_WATCHPOINT_ACCESS] = BP_GDB | BP_MEM_ACCESS, + }; + + CPUClass *cc = CPU_GET_CLASS(cpu); + int cputype = xlat[gdbtype]; + + if (cc->gdb_stop_before_watchpoint) { + cputype |= BP_STOP_BEFORE_ACCESS; + } + return cputype; +} #endif static int gdb_breakpoint_insert(target_ulong addr, target_ulong len, int type) @@ -656,10 +668,11 @@ static int gdb_breakpoint_insert(target_ulong addr, target_ulong len, int type) case GDB_WATCHPOINT_READ: case GDB_WATCHPOINT_ACCESS: CPU_FOREACH(cpu) { - err = cpu_watchpoint_insert(cpu, addr, len, xlat_gdb_type[type], - NULL); - if (err) + err = cpu_watchpoint_insert(cpu, addr, len, + xlat_gdb_type(cpu, type), NULL); + if (err) { break; + } } return err; #endif @@ -692,7 +705,8 @@ static int gdb_breakpoint_remove(target_ulong addr, target_ulong len, int type) case GDB_WATCHPOINT_READ: case GDB_WATCHPOINT_ACCESS: CPU_FOREACH(cpu) { - err = cpu_watchpoint_remove(cpu, addr, len, xlat_gdb_type[type]); + err = cpu_watchpoint_remove(cpu, addr, len, + xlat_gdb_type(cpu, type)); if (err) break; } diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 370b3eb..24aa0aa 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -99,6 +99,8 @@ struct TranslationBlock; * @vmsd: State description for migration. * @gdb_num_core_regs: Number of core registers accessible to GDB. * @gdb_core_xml_file: File name for core registers GDB XML description. + * @gdb_stop_before_watchpoint: Indicates whether GDB expects the CPU to stop + * before the insn which triggers a watchpoint rather than after it. * * Represents a CPU family or model. */ @@ -149,6 +151,7 @@ typedef struct CPUClass { const struct VMStateDescription *vmsd; int gdb_num_core_regs; const char *gdb_core_xml_file; + bool gdb_stop_before_watchpoint; } CPUClass; #ifdef HOST_WORDS_BIGENDIAN diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 5b5531a..47ebf77 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -1066,6 +1066,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data) #endif cc->gdb_num_core_regs = 26; cc->gdb_core_xml_file = "arm-core.xml"; + cc->gdb_stop_before_watchpoint = true; cc->debug_excp_handler = arm_debug_excp_handler; } diff --git a/target-cris/cpu.c b/target-cris/cpu.c index 20d8809..355bba0 100644 --- a/target-cris/cpu.c +++ b/target-cris/cpu.c @@ -290,6 +290,7 @@ static void cris_cpu_class_init(ObjectClass *oc, void *data) #endif cc->gdb_num_core_regs = 49; + cc->gdb_stop_before_watchpoint = true; } static const TypeInfo cris_cpu_type_info = { diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c index 419d664..a51490f 100644 --- a/target-lm32/cpu.c +++ b/target-lm32/cpu.c @@ -272,6 +272,7 @@ static void lm32_cpu_class_init(ObjectClass *oc, void *data) cc->vmsd = &vmstate_lm32_cpu; #endif cc->gdb_num_core_regs = 32 + 7; + cc->gdb_stop_before_watchpoint = true; cc->debug_excp_handler = lm32_debug_excp_handler; } diff --git a/target-mips/cpu.c b/target-mips/cpu.c index b3e0e6c..a23e0b7 100644 --- a/target-mips/cpu.c +++ b/target-mips/cpu.c @@ -150,6 +150,7 @@ static void mips_cpu_class_init(ObjectClass *c, void *data) #endif cc->gdb_num_core_regs = 73; + cc->gdb_stop_before_watchpoint = true; } static const TypeInfo mips_cpu_type_info = { diff --git a/target-xtensa/cpu.c b/target-xtensa/cpu.c index 936d526..ca95878 100644 --- a/target-xtensa/cpu.c +++ b/target-xtensa/cpu.c @@ -146,6 +146,7 @@ static void xtensa_cpu_class_init(ObjectClass *oc, void *data) cc->set_pc = xtensa_cpu_set_pc; cc->gdb_read_register = xtensa_cpu_gdb_read_register; cc->gdb_write_register = xtensa_cpu_gdb_write_register; + cc->gdb_stop_before_watchpoint = true; #ifndef CONFIG_USER_ONLY cc->do_unaligned_access = xtensa_cpu_do_unaligned_access; cc->get_phys_page_debug = xtensa_cpu_get_phys_page_debug; -- 1.9.1