[RFA] Keep breakpoints always inserted.

[Daniel Jacobowitz]

On Thu, Feb 28, 2008 at 05:17:13PM +0300, Vladimir Prus wrote:
> 
> This is hopefully final revision of my patch to keep
> breakpoints always inserted, which is needed to support
> non-stop mode. A new variable 'breakpoint always-inserted'
> is introduced that enables this mode.  When enabled,
> breakpoints are inserted into target immediately when created,
> and they are not removed when we stop.
> 
> Compared to the previous version of this patch:
> - The always-inserted mode is actually configurable
> - In always-inserted mode, breakpoints are removed
> from target on detach/disconnect.
> 
> OK?

This patch is OK, if you will add the missing pieces: a NEWS entry and
documentation for the new command, and a testcase that enables
always-inserted and verifies that it isn't a brick.

> +static void
> +insert_breakpoint_locations (void);

No newline before function name in prototypes.

> +/* If 1, gdb will keep breakpoints inserted even as 
> +   inferior is stopped, and immediately insert any new
> +   breakoints.
> +   If 0, gdb will insert breakpoints into inferior only
> +   when rusuming it, and will remove breakpoints
> +   upon stop.  */

Typos: breakoints, rusuming.  I've noticed your comments always
wrap early, around column 60 instead of 72 or 79; is there any
particular reason for that?  It looks awkward when other comments in
the same file wrap further over.

> +  /* Identify bp_location instances that are not
> +     longer present in the new list, and therefore should

no longer present

> +      /* If this location is not longer present, and

ditto

> +	 inserted, look if there's maybe new location

a new location

> +	 at the same address.  If so, mark that one
> +	 inserted, and don't remove this one.  
> +
> +	 This is needed so that we don't have a window
> +	 where a breakpoint at certian location is not

certain

> +		/* For the sake of should_insert_location.  The
> +		   call the check_duplicates will fix up this later.  */

the call to

> +      if (loc->inserted && !found_object && !found_address)
>  	{
> -	  tmp = &((*tmp)->global_next);
> +	  /* FIXME? Handle error? */
> +	  remove_breakpoint (loc, mark_uninserted);
>  	}

Can you take care of this FIXME?

> +int breakpoints_always_inserted_mode ()
> +{
> +  return always_inserted_mode;
> +}

(void)


> +
>  
>  /* This help string is used for the break, hbreak, tbreak and thbreak commands.
>     It is defined as a macro to prevent duplication.
> @@ -8341,6 +8393,19 @@ a warning will be emitted for such breakpoints."),
>  			   show_automatic_hardware_breakpoints,
>  			   &breakpoint_set_cmdlist,
>  			   &breakpoint_show_cmdlist);
> +
> +  add_setshow_boolean_cmd ("always-inserted", class_support,
> +			   &always_inserted_mode, _("\
> +Set mode for inserting breakpoints."), _("\
> +Show mode for inserting breakpoints."), _("\
> +When this mode is off (which is default), breakpoints are inserted in\n\

which is the default

> +inferior when it is resumed, and removed when execution stops.  When this\n\
> +mode is on, breakpoints are inserted immediately and removed only when\n\
> +the user deletes the breakpoint."),
> +extern int breakpoints_always_inserted_mode ();

(void)

> @@ -1350,10 +1358,6 @@ handle_inferior_event (struct execution_control_state *ecs)
>           established.  */
>        if (stop_soon == NO_STOP_QUIETLY)
>  	{
> -	  /* Remove breakpoints, SOLIB_ADD might adjust
> -	     breakpoint addresses via breakpoint_re_set.  */
> -	  remove_breakpoints ();
> -
>  	  /* Check for any newly added shared libraries if we're
>  	     supposed to be adding them automatically.  Switch
>  	     terminal for any messages produced by
> @@ -1393,9 +1397,6 @@ handle_inferior_event (struct execution_control_state *ecs)
>  
>  	  /* NOTE drow/2007-05-11: This might be a good place to check
>  	     for "catch load".  */
> -
> -	  /* Reinsert breakpoints and continue.  */
> -	  insert_breakpoints ();
>  	}
>  
>        /* If we are skipping through a shell, or through shared library

This does the right thing even with breakpoints not always inserted,
right?

-- 
Daniel Jacobowitz
CodeSourcery