Re: [PATCH 14/15] Create nat/i386-dregs.c

Pedro Alves
2014-06-17T16:56:01Z

On 06/17/2014 03:12 PM, Gary Benson wrote:
> @@ -596,34 +172,8 @@ i386_remove_watchpoint (struct target_ops *self,
> {
> struct i386_debug_reg_state *state
> = i386_debug_reg_state (ptid_get_pid (inferior_ptid));
> - int retval;
> - /* Work on a local copy of the debug registers, and on success,
> - commit the change back to the inferior. */
> - struct i386_debug_reg_state local_state = *state;
> -
> - if (((len != 1 && len != 2 && len != 4)
> - && !(TARGET_HAS_DR_LEN_8 && len == 8))
> - || addr % len != 0)
> - {
> - retval = i386_handle_nonaligned_watchpoint (&local_state,
> - WP_REMOVE,
> - addr, len, type);
> - }
> - else
> - {
> - unsigned len_rw = i386_dr_length_and_rw_bits (len, type);
> -
> - retval = i386_dr_remove_aligned_watchpoint (&local_state,
> - addr, len_rw);
> - }
> -
> - if (retval == 0)
> - i386_dr_update_inferior_debug_regs (state, &local_state);
> -
> - if (debug_hw_points)
> - i386_dr_show (state, "remove_watchpoint", addr, len, type);
>
> - return retval;
> + return i386_dr_remove_watchpoint (state, type, addr, len);

Hmm, there's still one remaining difference in gdb vs gdbserver,
after this, in hw breakpoints.

i386_insert_hw_breakpoint and i386_remove_hw_breakpoint on
the GDB side still have the "work on a local copy"
bits, and the i386_dr_update_inferior_debug_regs and
i386_dr_show calls:

/* Insert a hardware-assisted breakpoint at BP_TGT->placed_address.
Return 0 on success, EBUSY on failure. */
static int
i386_insert_hw_breakpoint (struct target_ops *self, struct gdbarch *gdbarch,
struct bp_target_info *bp_tgt)
{
struct i386_debug_reg_state *state
= i386_debug_reg_state (ptid_get_pid (inferior_ptid));
unsigned len_rw = i386_dr_length_and_rw_bits (1, hw_execute);
CORE_ADDR addr = bp_tgt->placed_address;
/* Work on a local copy of the debug registers, and on success,
commit the change back to the inferior. */
struct i386_debug_reg_state local_state = *state;
int retval = i386_dr_insert_aligned_watchpoint (&local_state,
addr,
len_rw) ? EBUSY : 0;

if (retval == 0)
i386_dr_update_inferior_debug_regs (state, &local_state);

if (debug_hw_points)
i386_dr_show (state, "insert_hwbp", addr, 1, hw_execute);

return retval;
}

While on the gdbserver side, we just call i386_dr_insert_watchpoint:

static int
x86_insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
int size, struct raw_breakpoint *bp)
{
switch (type)
{
case raw_bkpt_type_hw:
...
{
...
return i386_dr_insert_watchpoint (state, hw_type, addr, size);

Any reason GDB can't do the same?

--
Pedro Alves