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

[Pedro Alves <palves@redhat.com>]

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