Re: [RFC] Rework debug register accounting for hardware watchpoints

[Pedro Alves <pedro at codesourcery dot com>]

On Sunday 31 July 2011 21:43:54, Thiago Jung Bauermann wrote:
> Hi,
> 
> This patch implements Pedro's suggestion on how GDB should check
> available hardware resources [1].
> 
> It's work in progress because I'm still working on the hardware
> breakpoints side of things so at this moment I think GDB will get
> confused if the user adds both hardware breakpoints and watchpoints.
> Still, I wanted to post it to see if you think this is going in the
> right direction.

I think it is.  Thanks for working on this.

> There are no regressions on i386-linux.
> 
> [1] - http://sourceware.org/ml/gdb-patches/2011-05/msg00136.html


> @@ -1962,7 +2001,15 @@ insert_breakpoint_locations (void)
>  
>        if (bpt->disposition == disp_del_at_next_stop)
>         continue;
> -      
> +
> +      /* watch_command_1 creates a watchpoint but only sets its number if
> +         update_watchpoint succeeds in creating its bp_locations.  */
> +      if (bpt->number == 0)
> +       continue;

Momentary breakpoints have bpt->number == 0 (step-resume breakpoints, etc.).
Isn't this breaking those?

Hmm, I see the similar hack on delete_breakpoint:

  /* watch_command_1 creates a watchpoint but only sets its number if
     update_watchpoint succeeds in creating its bp_locations.  If there's
     a problem in that process, we'll be asked to delete the half-created
     watchpoint.  In that case, don't announce the deletion.  */
  if (bpt->number)
    observer_notify_breakpoint_deleted (bpt);

Is that still necessary?  watch_command_1 now creates and initializes the
watchpoint without adding it to the breakpoint list, and only on success
does it call install_breakpoint.  It would likely be cleaner if we
made delete_breakpoint check if the breakpoint is in the breakpoint
list (meaning the observers have been notified of its existance) instead of
checking its number.

-- 
Pedro Alves