This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [rfc target-side break conditions 3/5 v2] GDB-side changes


On 02/22/2012 03:24 PM, Luis Gustavo wrote:
> On 02/09/2012 10:56 AM, Pedro Alves wrote:
>> On 02/08/2012 08:56 PM, Luis Gustavo wrote:
>>
>>>> I'm confused.  How does this (and all similar places) address the issued I
>>>> pointed out before?  If you're passing one to update_global_location_list
>>>> when deleting a breakpoint, you're pretty much defeating the whole
>>>> purpose of the update_global_location_list's argument in the first place.
>>>>
>>>
>>> Updates will only take place when the user explicitly removes/disables a breakpoint. Functions that are deleting breakpoints (like remove_thread_event_breakpoints) won't cause insertion of any breakpoints since they go through the "delete_breakpoint" path, not the delete_breakpoint_with_update one.
>>
>> Ah, got it now.  Thanks.  But please:
>>
>>> +   structures.  If UPDATE is true, proceed to update the list of
>>> +   locations, otherwise don't update it.  */
>>>
>>> -void
>>> -delete_breakpoint (struct breakpoint *bpt)
>>> +static void
>>> +delete_breakpoint_1 (struct breakpoint *bpt, int update)
>>
>> "update" here is quite confusing, because that's not what is happening.  Or at
>> least, update is so overloaded that I'm not understanding it the way you are.
>> The list of locations is still updated/regenerated, and we do delete locations
>> from the target, but, we don't allow new insertions.  So, could we
>> s/update/somethingelse/ please?  Even pointing at update_global_location_list's
>> description of its parameter would be good.
>>
>> Or maybe, I just had an idea --- I wonder if we made update_global_location_list
>> still re-insert _only_ the _already inserted_ locations when the
>> condition changes, then we'd be good.  That is, something like:
>>
>> update_global_location_list:
>> ...
>> -  if (breakpoints_always_inserted_mode ()&&  should_insert
>> -&&  (have_live_inferiors ()
>> -      || (gdbarch_has_global_breakpoints (target_gdbarch))))
>> -    insert_breakpoint_locations ();
>> +  if (breakpoints_always_inserted_mode ()
>> +&&  (have_live_inferiors ()
>> +      || (gdbarch_has_global_breakpoints (target_gdbarch))))
>> +      {
>> +     if (should_insert)
>> +          insert_breakpoint_locations ();
>> +        else
>> +          update_inserted_breakpoint_locations ();
>> +      }
>>
>> The problem is that a delete_breakpoint would trigger insertions of all
>> _other_ breakpoints.  But if we're allow "reinserting" breakpoints that
>> are _already_ inserted, I think we're fine.
> 
> I implemented this change. update_inserted_breakpoint_locations is a simpler version of insert_breakpoint_locations. Its purpose is to synch conditions of already-inserted breakpoint locations with the target.
> 
>>
>>> Is disabling breakpoints also something we would like to do without triggering insertions? If so, i'm inclined to go with the same solution as for deleting user breakpoints.
>>
>> Maybe, not sure.  Better be safe, I think.
>>
>> It'd be nice to come up with a better way to solve the initial problem that
>> led to update_global_location_list having an argument in the first place.  :-/
> 
> I've confirmed this works with both delete and disable. Looks like a good solution for now.
> 

Yes, looks much better.  Thanks.  The patch looks mostly good to me now.  Only a
few comments below.

> The breakpoint infrastructure could use a little cleaning and refactoring i suppose. :-(

What doesn't? :-)

> 
> Also, i changed remote.c to not pass the "conditions" marker anymore.
> 
> Luis
> 
> 0002-break_condition_bytecode.diff
> 
> 
> 2012-02-22  Luis Machado  <lgustavo@codesourcery.com>
> 
> 	* remote.c (remote_supports_cond_breakpoints): New forward
> 	declaration.
> 	(remote_add_target_side_condition): New function.
> 	(remote_insert_breakpoint): Add target-side breakpoint
> 	conditional if supported.
> 	(remote_insert_hw_breakpoint): Likewise.
> 	(init_remote_ops): Set to_supports_evaluation_of_breakpoint_conditions
> 	hook.
> 
> 	* target.c (update_current_target): Inherit
> 	to_supports_evaluation_of_breakpoint_conditions.
> 	Default to_supports_evaluation_of_breakpoint_conditions to return_zero.
> 
> 	* target.h (struct target_ops)
> 	<to_supports_evaluation_of_breakpoint_conditions>: New field.
> 	(target_supports_evaluation_of_breakpoint_conditions): New #define.
> 
> 	* breakpoint.c (get_first_locp_gte_addr): New forward declaration.
> 	(condition_evaluation_both, condition_evaluation_auto,
> 	condition_evaluation_host, condition_evaluation_target,
> 	condition_evaluation_enums, condition_evaluation_mode_1,
> 	condition_evaluation_mode): New	static globals.
> 	(translate_condition_evaluation_mode): New function.
> 	(breakpoint_condition_evaluation_mode): New function.
> 	(gdb_evaluates_breakpoint_condition_p): New function.
> 	(ALL_BP_LOCATIONS_AT_ADDR): New helper macro.
> 	(mark_breakpoint_modified): New function.
> 	(mark_breakpoint_location_modified): New function.
> 	(set_condition_evaluation_mode): New function.
> 	(show_condition_evaluation_mode): New function.
> 	(get_first_location_gte_addr): New helper function.
> 	(set_breakpoint_condition): Free condition bytecode if locations
> 	has become unconditional.  Call mark_breakpoint_modified (...).
> 	(condition_command): Call update_global_location_list (1) for
> 	breakpoints.
> 	(breakpoint_xfer_memory): Use is_breakpoint (...).
> 	(is_breakpoint): New function.
> 	(parse_cond_to_aexpr): New function.
> 	(build_target_condition_list): New function.
> 	(insert_bp_location): Handle target-side conditional
> 	breakpoints and call build_target_condition_list (...).
> 	(update_inserted_breakpoint_locations): New function.
> 	(insert_breakpoint_locations): Handle target-side conditional
> 	breakpoints.
> 	(bpstat_check_breakpoint_conditions): Add comment.
> 	(bp_condition_evaluator): New function.
> 	(bp_location_condition_evaluator): New function.
> 	(print_breakpoint_location): Print information on where the condition
> 	will be evaluated.
> 	(print_one_breakpoint_location): Likewise.
> 	(init_bp_location): Call mark_breakpoint_location_modified (...) for
> 	breakpoint location.
> 	(force_breakpoint_reinsertion): New functions.
> 	(update_global_location_list): Handle target-side breakpoint
> 	conditions.
> 	Reinsert locations that are already inserted if conditions have
> 	changed.
> 	(bp_location_dtor): Free agent expression bytecode.
> 	(disable_breakpoint): Call mark_breakpoint_modified (...).
> 	Call update_global_location_list (...) with parameter 1 for breakpoints.
> 	(disable_command): Call mark_breakpoint_location_modified (...).
> 	Call update_global_location_list (...) with parameter 1 for breakpoints.
> 	(enable_breakpoint_disp): Call mark_breakpoint_modified (...).
> 	(enable_command): mark_breakpoint_location_modified (...).
> 	(_initialize_breakpoint): Update documentation and add
> 	condition-evaluation breakpoint subcommand.
> 
> 	* breakpoint.h: Include ax.h.
> 	(condition_list): New data structure.
> 	(condition_status): New enum.
> 	(bp_target_info) <cond_list>: New field.
> 	(bp_location) <condition_changed, cond_bytecode>: New fields.
> 	(is_breakpoint): New prototype.
> 
> Index: gdb/gdb/remote.c
> ===================================================================
> --- gdb.orig/gdb/remote.c	2012-02-22 13:22:16.930553985 -0200
> +++ gdb/gdb/remote.c	2012-02-22 13:22:49.174553987 -0200
> @@ -242,6 +242,8 @@ static int remote_read_description_p (st
>  
>  static void remote_console_output (char *msg);
>  
> +static int remote_supports_cond_breakpoints (void);
> +
>  /* The non-stop remote protocol provisions for one pending stop reply.
>     This is where we keep it until it is acknowledged.  */
>  
> @@ -7729,6 +7731,43 @@ extended_remote_create_inferior (struct
>  }
>  
>  
> +/* Given a location's target info BP_TGT and the packet buffer BUF,  output
> +   the list of conditions (in agent expression bytecode format), if any, the
> +   target needs to evaluate.  The output is placed into the packet buffer
> +   BUF.  */
> +
> +static int
> +remote_add_target_side_condition (struct gdbarch *gdbarch,
> +				  struct bp_target_info *bp_tgt, char *buf)
> +{
> +  struct agent_expr *aexpr = NULL;
> +  int i, ix;
> +  char *pkt;
> +  char *buf_start = buf;
> +
> +  if (VEC_length (agent_expr_p, bp_tgt->conditions))

if (!VEC_empty...)

if better:


  if (VEC_empty...)
    return 0;

  sprintf (buf + strlen (buf), "%s", ";");
  ...

> +    {
> +      sprintf (buf + strlen (buf), "%s", ";");
> +    }
> +  else
> +    return 0;
> +

  and all these repeated strlen calls are unnecessary:

  buf += strlen (buf);
  sprintf (buf, "%s", ";");


> +  /* Send conditions to the target and free the vector.  */
> +  for (ix = 0;
> +       VEC_iterate (agent_expr_p, bp_tgt->conditions, ix, aexpr);
> +       ix++)
> +    {
> +      sprintf (buf + strlen (buf), "X%x,", aexpr->len);
> +      pkt = buf + strlen (buf);
> +      for (i = 0; i < aexpr->len; ++i)
> +	pkt = pack_hex_byte (pkt, aexpr->buf[i]);
> +      *pkt = '\0';

      sprintf (buf, "X%x,", aexpr->len);
      buf += strlen (buf);
      for (i = 0; i < aexpr->len; ++i)
	buf = pack_hex_byte (buf, aexpr->buf[i]);
      *buf = '\0';


> +    }
> +
> +  VEC_free (agent_expr_p, bp_tgt->conditions);
> +  return 0;
> +}
> +
>  /* Insert a breakpoint.  On targets that have software breakpoint
>     support, we ask the remote target to do the work; on targets
>     which don't, we insert a traditional memory breakpoint.  */
> @@ -7748,6 +7787,7 @@ remote_insert_breakpoint (struct gdbarch
>        struct remote_state *rs;
>        char *p;
>        int bpsize;
> +      struct condition_list *cond = NULL;
>  
>        gdbarch_remote_breakpoint_from_pc (gdbarch, &addr, &bpsize);
>  
> @@ -7761,6 +7801,9 @@ remote_insert_breakpoint (struct gdbarch
>        p += hexnumstr (p, addr);
>        sprintf (p, ",%d", bpsize);
>  
> +      if (remote_supports_cond_breakpoints ())
> +	remote_add_target_side_condition (gdbarch, bp_tgt, p);
> +
>        putpkt (rs->buf);
>        getpkt (&rs->buf, &rs->buf_size, 0);
>  
> @@ -7986,6 +8029,9 @@ remote_insert_hw_breakpoint (struct gdba
>    p += hexnumstr (p, (ULONGEST) addr);
>    sprintf (p, ",%x", bp_tgt->placed_size);
>  
> +  if (remote_supports_cond_breakpoints ())
> +    remote_add_target_side_condition (gdbarch, bp_tgt, p);
> +
>    putpkt (rs->buf);
>    getpkt (&rs->buf, &rs->buf_size, 0);
>  
> @@ -10781,6 +10827,7 @@ Specify the serial device it is connecte
>    remote_ops.to_fileio_readlink = remote_hostio_readlink;
>    remote_ops.to_supports_enable_disable_tracepoint = remote_supports_enable_disable_tracepoint;
>    remote_ops.to_supports_string_tracing = remote_supports_string_tracing;
> +  remote_ops.to_supports_evaluation_of_breakpoint_conditions = remote_supports_cond_breakpoints;
>    remote_ops.to_trace_init = remote_trace_init;
>    remote_ops.to_download_tracepoint = remote_download_tracepoint;
>    remote_ops.to_can_download_tracepoint = remote_can_download_tracepoint;
> Index: gdb/gdb/target.c
> ===================================================================
> --- gdb.orig/gdb/target.c	2012-02-22 13:22:16.946553985 -0200
> +++ gdb/gdb/target.c	2012-02-22 13:22:49.174553987 -0200
> @@ -699,6 +699,7 @@ update_current_target (void)
>        INHERIT (to_static_tracepoint_markers_by_strid, t);
>        INHERIT (to_traceframe_info, t);
>        INHERIT (to_magic, t);
> +      INHERIT (to_supports_evaluation_of_breakpoint_conditions, t);
>        /* Do not inherit to_memory_map.  */
>        /* Do not inherit to_flash_erase.  */
>        /* Do not inherit to_flash_done.  */
> @@ -925,6 +926,9 @@ update_current_target (void)
>    de_fault (to_traceframe_info,
>  	    (struct traceframe_info * (*) (void))
>  	    tcomplain);
> +  de_fault (to_supports_evaluation_of_breakpoint_conditions,
> +	    (int (*) (void))
> +	    return_zero);
>    de_fault (to_execution_direction, default_execution_direction);
>  
>  #undef de_fault
> Index: gdb/gdb/target.h
> ===================================================================
> --- gdb.orig/gdb/target.h	2012-02-22 13:22:16.894553984 -0200
> +++ gdb/gdb/target.h	2012-02-22 13:22:49.178553986 -0200
> @@ -662,6 +662,9 @@ struct target_ops
>      /* Does this target support the tracenz bytecode for string collection?  */
>      int (*to_supports_string_tracing) (void);
>  
> +    /* Does this target support evaluation breakpoint conditions on its end?  */

either "evaluation of breakpoint conditions", or "evaluating breakpoint conditions"?


> +    int (*to_supports_evaluation_of_breakpoint_conditions) (void);
> +
>      /* Determine current architecture of thread PTID.
>  
>         The target is supposed to determine the architecture of the code where
> @@ -968,6 +971,12 @@ int target_supports_disable_randomizatio
>  #define target_supports_string_tracing() \
>    (*current_target.to_supports_string_tracing) ()
>  
> +/* Returns true if this target can handle breakpoint conditions
> +   on its end.  */
> +
> +#define target_supports_evaluation_of_breakpoint_conditions() \
> +  (*current_target.to_supports_evaluation_of_breakpoint_conditions) ()
> +
>  /* Invalidate all target dcaches.  */
>  extern void target_dcache_invalidate (void);
>  
> Index: gdb/gdb/breakpoint.c
> ===================================================================
> --- gdb.orig/gdb/breakpoint.c	2012-02-22 13:22:16.882553985 -0200
> +++ gdb/gdb/breakpoint.c	2012-02-22 13:22:58.038553985 -0200
> @@ -66,6 +66,7 @@
>  #include "skip.h"
>  #include "record.h"
>  #include "gdb_regex.h"
> +#include "ax-gdb.h"
>  
>  /* readline include files */
>  #include "readline/readline.h"
> @@ -258,6 +259,8 @@ static void trace_pass_command (char *,
>  
>  static int is_masked_watchpoint (const struct breakpoint *b);
>  
> +static struct bp_location **get_first_locp_gte_addr (CORE_ADDR address);
> +
>  /* Return 1 if B refers to a static tracepoint set by marker ("-m"), zero
>     otherwise.  */
>  
> @@ -406,6 +409,64 @@ breakpoints_always_inserted_mode (void)
>  	  && !RECORD_IS_USED);
>  }
>  
> +static const char condition_evaluation_both[] = "host or target";
> +
> +/* Modes for breakpoint condition evaluation.  */
> +static const char condition_evaluation_auto[] = "auto";
> +static const char condition_evaluation_host[] = "host";
> +static const char condition_evaluation_target[] = "target";
> +static const char *const condition_evaluation_enums[] = {
> +  condition_evaluation_auto,
> +  condition_evaluation_host,
> +  condition_evaluation_target,
> +  NULL
> +};
> +
> +/* Global that holds the current mode for breakpoint condition evaluation.  */
> +static const char *condition_evaluation_mode_1 = condition_evaluation_auto;
> +
> +/* Global that we use to display information to the user (gets its value from
> +   condition_evaluation_mode_1.  */
> +static const char *condition_evaluation_mode = condition_evaluation_auto;
> +
> +/* Translate a condition evaluation mode MODE into either "gdb"

It's "host" now, no longer "gdb".

> +   or "target".  This is used mostly to translate from "auto" to the
> +   real setting that is being used.  It returns the translated
> +   evaluation mode.  */
> +
> +static const char *
> +translate_condition_evaluation_mode (const char *mode)
> +{
> +  if (mode == condition_evaluation_auto)
> +    {
> +      if (target_supports_evaluation_of_breakpoint_conditions ())
> +	return condition_evaluation_target;
> +      else
> +	return condition_evaluation_host;
> +    }
> +  else
> +    return mode;
> +}
> +
> +/* Discovers what condition_evaluation_auto translates to.  */
> +
> +static const char *
> +breakpoint_condition_evaluation_mode (void)
> +{
> +  return translate_condition_evaluation_mode (condition_evaluation_mode);
> +}
> +
> +/* Return true if GDB should evaluate breakpoint conditions or false
> +   otherwise.  */
> +
> +static int
> +gdb_evaluates_breakpoint_condition_p (void)
> +{
> +  const char *mode = breakpoint_condition_evaluation_mode ();
> +
> +  return (mode == condition_evaluation_host);
> +}
> +
>  void _initialize_breakpoint (void);
>  
>  /* Are we executing breakpoint commands?  */
> @@ -437,6 +498,19 @@ int target_exact_watchpoints = 0;
>  	     BP_TMP < bp_location + bp_location_count && (B = *BP_TMP);	\
>  	     BP_TMP++)
>  
> +/* Iterates through locations with address ADDRESS for the currently selected
> +   program space.  BP_LOCP_TMP points to each object.  BP_LOCP_START points
> +   to where the loop should start from.
> +   If BP_LOCP_START is a NULL pointer, the macro automatically seeks the
> +   appropriate location to start with.  */
> +
> +#define ALL_BP_LOCATIONS_AT_ADDR(BP_LOCP_TMP, BP_LOCP_START, ADDRESS)	\
> +	for (BP_LOCP_START = BP_LOCP_START == NULL ? get_first_locp_gte_addr (ADDRESS) : BP_LOCP_START, \
> +	     BP_LOCP_TMP = BP_LOCP_START;				\
> +	     (BP_LOCP_TMP < bp_location + bp_location_count		\
> +	     && (*BP_LOCP_TMP)->address == ADDRESS);			\
> +	     BP_LOCP_TMP++)
> +
>  /* Iterator for tracepoints only.  */
>  
>  #define ALL_TRACEPOINTS(B)  \
> @@ -620,6 +694,144 @@ get_breakpoint (int num)
>  
>  
>  
> +/* Mark locations as "conditions have changed" in case the target supports
> +   evaluating conditions on its side.  */
> +
> +static void
> +mark_breakpoint_modified (struct breakpoint *b)
> +{
> +  struct bp_location *loc;
> +
> +  /* This is only meaningful if the target is
> +     evaluating conditions and if the user has
> +     opted for condition evaluation on the target's
> +     side.  */
> +  if (gdb_evaluates_breakpoint_condition_p ()
> +      || !target_supports_evaluation_of_breakpoint_conditions ())
> +    return;
> +
> +  if (!is_breakpoint (b))
> +    return;
> +
> +  for (loc = b->loc; loc; loc = loc->next)
> +    loc->condition_changed = condition_modified;
> +}
> +
> +/* Mark location as "conditions have changed" in case the target supports
> +   evaluating conditions on its side.  */
> +
> +static void
> +mark_breakpoint_location_modified (struct bp_location *loc)
> +{
> +  /* This is only meaningful if the target is
> +     evaluating conditions and if the user has
> +     opted for condition evaluation on the target's
> +     side.  */
> +  if (gdb_evaluates_breakpoint_condition_p ()
> +      || !target_supports_evaluation_of_breakpoint_conditions ())
> +
> +    return;
> +
> +  if (!is_breakpoint (loc->owner))
> +    return;
> +
> +  loc->condition_changed = condition_modified;
> +}
> +
> +/* Sets the condition-evaluation mode using the static global
> +   condition_evaluation_mode.  */
> +
> +static void
> +set_condition_evaluation_mode (char *args, int from_tty,
> +			       struct cmd_list_element *c)
> +{
> +  struct breakpoint *b;
> +  const char *old_mode, *new_mode;
> +
> +  if ((condition_evaluation_mode_1 == condition_evaluation_target)
> +      && !target_supports_evaluation_of_breakpoint_conditions ())
> +    {
> +      condition_evaluation_mode_1 = condition_evaluation_mode;
> +      warning (_("Target does not support breakpoint condition evaluation.\n"
> +		 "Using GDB evaluation mode instead."));

s/GDB/host/ ?

> +      return;
> +    }
> +
> +  new_mode = translate_condition_evaluation_mode (condition_evaluation_mode_1);
> +  old_mode = translate_condition_evaluation_mode (condition_evaluation_mode);
> +
> +  /* Only update the mode if the user picked a different one.  */
> +  if (new_mode != old_mode)
> +    {
> +      struct bp_location *loc, **loc_tmp;
> +      /* If the user switched to a different evaluation mode, we
> +	 need to synch the changes with the target as follows:
> +
> +	 "gdb" -> "target": Send all (valid) conditions to the target.
> +	 "target" -> "gdb": Remove all the conditions from the target.

s/gdb/host/ ?

> +      */
> +
> +      /* Flip the switch.  */
> +      condition_evaluation_mode = condition_evaluation_mode_1;
> +
> +      if (new_mode == condition_evaluation_target)
> +	{
> +	  /* Mark everything modified and to synch conditions with the
> +	     target.  */
> +	  ALL_BP_LOCATIONS (loc, loc_tmp)
> +	    mark_breakpoint_location_modified (loc);
> +  	}
> +      else
> +	{
> +	  /* Manually mark non-duplicate locations to synch conditions
> +	     with the target.  We do this to remove all the conditions the
> +	     target knows about.  */
> +	  ALL_BP_LOCATIONS (loc, loc_tmp)
> +	    if (is_breakpoint (loc->owner) && loc->inserted)
> +	      loc->needs_update = 1;
> +	}
> +
> +      /* Do the update.  */
> +      update_global_location_list (1);
> +    }
> +
> +  return;
> +}
> +
> +/* Shows the current mode of breakpoint condition evaluation.  Explicitly shows
> +   what "auto" is translating to.  */
> +
> +static void
> +show_condition_evaluation_mode (struct ui_file *file, int from_tty,
> +				struct cmd_list_element *c, const char *value)
> +{
> +  if (condition_evaluation_mode == condition_evaluation_auto)
> +    fprintf_filtered (file,
> +		      _("Breakpoint condition evaluation "
> +			"mode is %s (currently %s).\n"),
> +		      value,
> +		      breakpoint_condition_evaluation_mode ());
> +  else
> +    fprintf_filtered (file, _("Breakpoint condition evaluation mode is %s.\n"),
> +		      value);
> +}
> +
> +/* Helper function to skip all bp_locations with addresses
> +   less than ADDRESS.  It returns the first bp_location that
> +   is greater than or equal to ADDRESS.  */
> +
> +static struct bp_location **
> +get_first_locp_gte_addr (CORE_ADDR address)
> +{
> +  struct bp_location **locp = bp_location;
> +
> +  while (locp < bp_location + bp_location_count
> +	 && (*locp)->address < address)
> +    locp++;
> +
> +  return locp;
> +}
> +
>  void
>  set_breakpoint_condition (struct breakpoint *b, char *exp,
>  			  int from_tty)
> @@ -642,6 +854,10 @@ set_breakpoint_condition (struct breakpo
>  	{
>  	  xfree (loc->cond);
>  	  loc->cond = NULL;
> +
> +	  /* No need to free the condition agent expression
> +	     bytecode (if we have one).  We will handle this
> +	     when we go through update_global_location_list.  */
>  	}
>      }
>  
> @@ -684,6 +900,8 @@ set_breakpoint_condition (struct breakpo
>  	    }
>  	}
>      }
> +  mark_breakpoint_modified (b);
> +
>    breakpoints_changed ();
>    observer_notify_breakpoint_modified (b);
>  }
> @@ -717,6 +935,10 @@ condition_command (char *arg, int from_t
>  	  error (_("Cannot set a condition where a Python 'stop' "
>  		   "method has been defined in the breakpoint."));
>  	set_breakpoint_condition (b, p, from_tty);
> +
> +	if (is_breakpoint (b))
> +	  update_global_location_list (1);
> +
>  	return;
>        }
>  
> @@ -1216,6 +1438,15 @@ breakpoint_xfer_memory (gdb_byte *readbu
>  }
>  
>  
> +/* Return true if BPT is either a software breakpoint or a hardware
> +   breakpoint.  */
> +int

Space between comment and function, please.

> +is_breakpoint (const struct breakpoint *bpt)
> +{
> +  return (bpt->type == bp_breakpoint
> +	  || bpt->type == bp_hardware_breakpoint);
> +}
> +
>  /* Return true if BPT is of any hardware watchpoint kind.  */
>  
>  static int
> @@ -1658,6 +1889,143 @@ unduplicated_should_be_inserted (struct
>    return result;
>  }
>  
> +/* Parses a conditional described by an expression COND into an
> +   agent expression bytecode suitable for evaluation
> +   by the bytecode interpreter.  Return NULL if there was
> +   any error during parsing.  */
> +
> +static struct agent_expr *
> +parse_cond_to_aexpr (CORE_ADDR scope, struct expression *cond)
> +{
> +  struct agent_expr *aexpr = NULL;
> +  struct cleanup *old_chain = NULL;
> +  volatile struct gdb_exception ex;
> +
> +  if (!cond)
> +    return NULL;
> +
> +  /* We don't want to stop processing, so catch any errors
> +     that may show up.  */
> +  TRY_CATCH (ex, RETURN_MASK_ERROR)
> +    {
> +      aexpr = gen_eval_for_expr (scope, cond);
> +    }
> +
> +  if (ex.reason < 0)
> +    {
> +      /* If we got here, it means the condition could not be parsed to a valid
> +	 bytecode expression and thus can't be evaluated on the target's side.
> +	 It's no use iterating through the conditions.  */
> +      return NULL;
> +    }
> +
> +  /* We have a valid agent expression.  */
> +  return aexpr;
> +}
> +
> +/* Based on location BL, create a list of breakpoint conditions to be
> +   passed on to the target.  If we have duplicated locations with different
> +   conditions, we will add such conditions to the list.  The idea is that the
> +   target will evaluate the list of conditions and will only notify GDB when
> +   one of them is true.  */
> +
> +static void
> +build_target_condition_list (struct bp_location *bl)
> +{
> +  struct bp_location **locp = NULL, **loc2p;
> +  int null_condition_or_parse_error = 0;
> +  int modified = bl->needs_update;
> +  struct bp_location *loc;
> +
> +  /* This is only meaningful if the target is
> +     evaluating conditions and if the user has
> +     opted for condition evaluation on the target's
> +     side.  */
> +  if (gdb_evaluates_breakpoint_condition_p ()
> +      || !target_supports_evaluation_of_breakpoint_conditions ())
> +    return;
> +
> +  /* Do a first pass to check for locations with no assigned
> +     conditions or conditions that fail to parse to a valid agent expression
> +     bytecode.  If any of these happen, then it's no use to send conditions
> +     to the target since this location will always trigger and generate a
> +     response back to GDB.  */
> +  ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, bl->address)
> +    {
> +      loc = (*loc2p);
> +      if (is_breakpoint (loc->owner) && loc->pspace == bl->pspace)
> +	{
> +	  if (modified)
> +	    {
> +	      struct agent_expr *aexpr;
> +
> +	      /* Re-parse the conditions since something changed.  In that
> +		 case we already freed the condition bytecodes (see
> +		 force_breakpoint_reinsertion).  We just
> +		 need to parse the condition to bytecodes again.  */
> +	      aexpr = parse_cond_to_aexpr (bl->address, loc->cond);
> +	      loc->cond_bytecode = aexpr;
> +
> +	      /* Check if we managed to parse the conditional expression
> +		 correctly.  If not, we will not send this condition
> +		 to the target.  */
> +	      if (aexpr)
> +		continue;
> +	    }
> +
> +	  /* If we have a NULL bytecode expression, it means something
> +	     went wrong or we have a null condition expression.  */
> +	  if (!loc->cond_bytecode)
> +	    {
> +	      null_condition_or_parse_error = 1;
> +	      break;
> +	    }
> +	}
> +    }
> +
> +  /* If any of these happened, it means we will have to evaluate the conditions
> +     for the location's address on gdb's side.  It is no use keeping bytecodes
> +     for all the other duplicate locations, thus we free all of them here.
> +
> +     This is so we have a finer control over which locations' conditions are
> +     being evaluated by GDB or the remote stub.  */
> +  if (null_condition_or_parse_error)
> +    {
> +      ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, bl->address)
> +	{
> +	  loc = (*loc2p);
> +	  if (is_breakpoint (loc->owner) && loc->pspace == bl->pspace)
> +	    {
> +	      /* Only go as far as the first NULL bytecode is
> +		 located.  */
> +	      if (!loc->cond_bytecode)
> +		return;
> +
> +	      free_agent_expr (loc->cond_bytecode);
> +	      loc->cond_bytecode = NULL;
> +	    }
> +	}
> +    }
> +
> +  /* No NULL conditions or failed bytecode generation.  Build a condition list
> +     for this location's address.  */
> +  ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, bl->address)
> +    {
> +      loc = (*loc2p);
> +      if (loc->cond
> +	  && is_breakpoint (loc->owner)
> +	  && loc->pspace == bl->pspace
> +	  && loc->owner->enable_state == bp_enabled
> +	  && loc->enabled)
> +	/* Add the condition to the vector.  This will be used later to send the
> +	   conditions to the target.  */
> +	VEC_safe_push (agent_expr_p, bl->target_info.conditions,
> +		       loc->cond_bytecode);
> +    }
> +
> +  return;
> +}
> +
>  /* Insert a low-level "breakpoint" of some type.  BL is the breakpoint
>     location.  Any error messages are printed to TMP_ERROR_STREAM; and
>     DISABLED_BREAKS, and HW_BREAKPOINT_ERROR are used to report problems.
> @@ -1674,7 +2042,7 @@ insert_bp_location (struct bp_location *
>  {
>    int val = 0;
>  
> -  if (!should_be_inserted (bl) || bl->inserted)
> +  if (!should_be_inserted (bl) || (bl->inserted && !bl->needs_update))
>      return 0;
>  
>    /* Initialize the target-specific information.  */
> @@ -1683,6 +2051,18 @@ insert_bp_location (struct bp_location *
>    bl->target_info.placed_address_space = bl->pspace->aspace;
>    bl->target_info.length = bl->length;
>  
> +  /* When working with target-side conditions, we must pass all the conditions
> +     for the same breakpoint address down to the target since GDB will not
> +     insert those locations.  With a list of breakpoint conditions, the target
> +     can decide when to stop and notify GDB.  */
> +
> +  if (is_breakpoint (bl->owner))
> +    {
> +      build_target_condition_list (bl);
> +      /* Reset the condition modification marker.  */
> +      bl->needs_update = 0;
> +    }
> +
>    if (bl->loc_type == bp_loc_software_breakpoint
>        || bl->loc_type == bp_loc_hardware_breakpoint)
>      {
> @@ -1991,6 +2371,66 @@ insert_breakpoints (void)
>      insert_breakpoint_locations ();
>  }
>  
> +/* This is used when we need to synch breakpoint conditions between GDB and the
> +   target.  It is the case with deleting and disabling of breakpoints when using
> +   always-inserted mode.  */
> +
> +static void
> +update_inserted_breakpoint_locations (void)
> +{
> +  struct bp_location *bl, **blp_tmp;
> +  int error_flag = 0;
> +  int val = 0;
> +  int disabled_breaks = 0;
> +  int hw_breakpoint_error = 0;
> +
> +  struct ui_file *tmp_error_stream = mem_fileopen ();
> +  struct cleanup *cleanups = make_cleanup_ui_file_delete (tmp_error_stream);
> +
> +  /* Explicitly mark the warning -- this will only be printed if
> +     there was an error.  */
> +  fprintf_unfiltered (tmp_error_stream, "Warning:\n");
> +
> +  save_current_space_and_thread ();
> +
> +  ALL_BP_LOCATIONS (bl, blp_tmp)
> +    {
> +      /* We only want to update software breakpoints and hardware
> +	 breakpoints.  */
> +      if (!is_breakpoint (bl->owner))
> +	continue;
> +
> +      /* We only want to update locations that are already inserted
> +	 and need updating.  This is to avoid unwanted insertion during
> +	 deletion of breakpoints.  */
> +      if (!bl->inserted || (bl->inserted && !bl->needs_update))
> +	continue;
> +
> +      switch_to_program_space_and_thread (bl->pspace);
> +
> +      /* For targets that support global breakpoints, there's no need
> +	 to select an inferior to insert breakpoint to.  In fact, even
> +	 if we aren't attached to any process yet, we should still
> +	 insert breakpoints.  */
> +      if (!gdbarch_has_global_breakpoints (target_gdbarch)
> +	  && ptid_equal (inferior_ptid, null_ptid))
> +	continue;
> +
> +      val = insert_bp_location (bl, tmp_error_stream, &disabled_breaks,
> +				    &hw_breakpoint_error);
> +      if (val)
> +	error_flag = val;
> +    }
> +
> +  if (error_flag)
> +    {
> +      target_terminal_ours_for_output ();
> +      error_stream (tmp_error_stream);
> +    }
> +
> +  do_cleanups (cleanups);
> +}
> +
>  /* Used when starting or continuing the program.  */
>  
>  static void
> @@ -2014,7 +2454,7 @@ insert_breakpoint_locations (void)
>  
>    ALL_BP_LOCATIONS (bl, blp_tmp)
>      {
> -      if (!should_be_inserted (bl) || bl->inserted)
> +      if (!should_be_inserted (bl) || (bl->inserted && !bl->needs_update))
>  	continue;
>  
>        /* There is no point inserting thread-specific breakpoints if
> @@ -4092,6 +4532,10 @@ bpstat_check_breakpoint_conditions (bpst
>    b = bs->breakpoint_at;
>    gdb_assert (b != NULL);
>  
> +  /* Even if the target evaluated the condition on its end and notified GDB, we
> +     need to do so again since GDB does not know if we stopped due to a
> +     breakpoint or a single step breakpoint.  */
> +
>    if (frame_id_p (b->frame_id)
>        && !frame_id_eq (b->frame_id, get_stack_frame_id (get_current_frame ())))
>      bs->stop = 0;
> @@ -4669,6 +5113,66 @@ wrap_indent_at_field (struct ui_out *uio
>    return NULL;
>  }
>  
> +/* Determine if the locations of this breakpoint will have their conditions
> +   evaluated by the target, host or a mix of both.  Returns the following:
> +
> +    "host": Host evals condition.
> +    "host or target": Host or Target evals condition.
> +    "target": Target evals condition.
> +*/
> +
> +static const char *
> +bp_condition_evaluator (struct breakpoint *b)
> +{
> +  struct bp_location *bl;
> +  char host_evals = 0;
> +  char target_evals = 0;
> +
> +  if (!b)
> +    return NULL;
> +
> +  if (!is_breakpoint (b))
> +    return NULL;
> +
> +  if (gdb_evaluates_breakpoint_condition_p ()
> +      || !target_supports_evaluation_of_breakpoint_conditions ())
> +    return condition_evaluation_host;
> +
> +  for (bl = b->loc; bl; bl = bl->next)
> +    {
> +      if (bl->cond_bytecode)
> +	target_evals++;
> +      else
> +	host_evals++;
> +    }
> +
> +  if (host_evals && target_evals)
> +    return condition_evaluation_both;
> +  else if (target_evals)
> +    return condition_evaluation_target;
> +  else
> +    return condition_evaluation_host;
> +}
> +
> +/* Determine the breakpoint location's condition evaluator.  This is
> +   similar to bp_condition_evaluator, but for locations.  */
> +
> +static const char *
> +bp_location_condition_evaluator (struct bp_location *bl)
> +{
> +  if (bl && !is_breakpoint (bl->owner))
> +    return NULL;
> +
> +  if (gdb_evaluates_breakpoint_condition_p ()
> +      || !target_supports_evaluation_of_breakpoint_conditions ())
> +    return condition_evaluation_host;
> +
> +  if (bl && bl->cond_bytecode)
> +    return condition_evaluation_target;
> +  else
> +    return condition_evaluation_host;
> +}
> +
>  /* Print the LOC location out of the list of B->LOC locations.  */
>  
>  static void
> @@ -4727,6 +5231,16 @@ print_breakpoint_location (struct breakp
>    else
>      ui_out_field_string (uiout, "pending", b->addr_string);
>  
> +  if (loc && is_breakpoint (b)
> +      && breakpoint_condition_evaluation_mode () == condition_evaluation_target
> +      && bp_condition_evaluator (b) == condition_evaluation_both)
> +    {
> +      ui_out_text (uiout, " (");
> +      ui_out_field_string (uiout, "evaluated-by",
> +			   bp_location_condition_evaluator (loc));
> +      ui_out_text (uiout, ")");
> +    }
> +
>    do_cleanups (old_chain);
>  }
>  
> @@ -5002,6 +5516,18 @@ print_one_breakpoint_location (struct br
>        else
>  	ui_out_text (uiout, "\tstop only if ");
>        ui_out_field_string (uiout, "cond", b->cond_string);
> +
> +      /* Print whether the target is doing the breakpoint's condition
> +	 evaluation.  If GDB is doing the evaluation, don't print anything.  */
> +      if (is_breakpoint (b)
> +	  && breakpoint_condition_evaluation_mode ()
> +	  == condition_evaluation_target)
> +	{
> +	  ui_out_text (uiout, " (");
> +	  ui_out_field_string (uiout, "evaluated-by",
> +			       bp_condition_evaluator (b));
> +	  ui_out_text (uiout, " evals)");
> +	}
>        ui_out_text (uiout, "\n");
>      }
>  
> @@ -5731,6 +6257,7 @@ init_bp_location (struct bp_location *lo
>    loc->ops = ops;
>    loc->owner = owner;
>    loc->cond = NULL;
> +  loc->cond_bytecode = NULL;
>    loc->shlib_disabled = 0;
>    loc->enabled = 1;
>  
> @@ -5758,9 +6285,11 @@ init_bp_location (struct bp_location *lo
>      case bp_gnu_ifunc_resolver:
>      case bp_gnu_ifunc_resolver_return:
>        loc->loc_type = bp_loc_software_breakpoint;
> +      mark_breakpoint_location_modified (loc);
>        break;
>      case bp_hardware_breakpoint:
>        loc->loc_type = bp_loc_hardware_breakpoint;
> +      mark_breakpoint_location_modified (loc);
>        break;
>      case bp_hardware_watchpoint:
>      case bp_read_watchpoint:
> @@ -10717,6 +11246,7 @@ swap_insertion (struct bp_location *left
>  {
>    const int left_inserted = left->inserted;
>    const int left_duplicate = left->duplicate;
> +  const int left_needs_update = left->needs_update;
>    const struct bp_target_info left_target_info = left->target_info;
>  
>    /* Locations of tracepoints can never be duplicated.  */
> @@ -10727,12 +11257,67 @@ swap_insertion (struct bp_location *left
>  
>    left->inserted = right->inserted;
>    left->duplicate = right->duplicate;
> +  left->needs_update = right->needs_update;
>    left->target_info = right->target_info;
>    right->inserted = left_inserted;
>    right->duplicate = left_duplicate;
> +  right->needs_update = left_needs_update;
>    right->target_info = left_target_info;
>  }
>  
> +/* Force the re-insertion of the locations at ADDRESS.  This is called
> +   once a new/deleted/modified duplicate location is found and we are evaluating
> +   conditions on the target's side.  Such conditions need to be updated on
> +   the target.  */
> +
> +static void
> +force_breakpoint_reinsertion (struct bp_location *bl)
> +{
> +  struct bp_location **locp = NULL, **loc2p;
> +  struct bp_location *loc;
> +  CORE_ADDR address = 0;
> +  struct program_space *pspace = NULL;
> +
> +  address = bl->address;
> +  pspace = bl->pspace;
> +
> +  /* This is only meaningful if the target is
> +     evaluating conditions and if the user has
> +     opted for condition evaluation on the target's
> +     side.  */
> +  if (gdb_evaluates_breakpoint_condition_p ()
> +      || !target_supports_evaluation_of_breakpoint_conditions ())
> +    return;
> +
> +  /* Flag all breakpoint locations with this address and
> +     the same program space as the location
> +     as "its condition has changed".  We need to
> +     update the conditions on the target's side.  */
> +  ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, address)

It's unfortunate that we're introducing these extra iterations over
all locations.  I think we can easily get rid of them by using
bsearch'ing for the initial location at address (and then rewinding
a bit for the first location that matches).  I'm not making it
a requirement for acceptance, but I think it may be a good idea.

> +    {
> +      loc = *loc2p;
> +
> +      if (!is_breakpoint (loc->owner)
> +	  || pspace != loc->pspace)
> +	continue;
> +
> +      /* Flag the location appropriately.  We use a different number to

number?  I think this was referring to before you had enums, right?
s/number/state/ would work, I think.

> +	 let everyone know that we already updated the set of locations
> +	 with addr bl->address and program space bl->pspace.  This is so
> +	 we don't have to keep calling this functions just to mark locations
> +	 that have already been marked.  */
> +      loc->condition_changed = condition_updated;
> +
> +      /* Free the agent expression bytecode as well.  We will compute
> +	 it later on.  */
> +      if (loc->cond_bytecode)
> +	{
> +	  free_agent_expr (loc->cond_bytecode);
> +	  loc->cond_bytecode = NULL;
> +	}
> +    }
> +}
> +
>  /* If SHOULD_INSERT is false, do not insert any breakpoint locations
>     into the inferior, only remove already-inserted locations that no
>     longer should be inserted.  Functions that delete a breakpoint or
> @@ -10754,6 +11339,10 @@ update_global_location_list (int should_
>    struct breakpoint *b;
>    struct bp_location **locp, *loc;
>    struct cleanup *cleanups;
> +  /* Last breakpoint location address that was marked for update.  */
> +  CORE_ADDR last_addr = 0;
> +  /* Last breakpoint location program space that was marked for updated.  */

"for update".

> +  struct program_space *last_pspace = NULL;
>  
>    /* Used in the duplicates detection below.  When iterating over all
>       bp_locations, points to the first bp_location of a given address.
> @@ -10826,13 +11415,27 @@ update_global_location_list (int should_
>  	    && (*loc2p)->address == old_loc->address);
>  	   loc2p++)
>  	{
> +	  /* Check if this is a new/duplicated location or a duplicated
> +	     location that had its condition modified.  If so, we want to send
> +	     its condition to the target if evaluation of conditions is taking
> +	     place there.  */
> +
> +	  if ((*loc2p)->condition_changed == condition_modified
> +	      && last_addr != old_loc->address)
> +	    force_breakpoint_reinsertion ((*loc2p));

Unnecessary double parens '(('.

I think we need to check pspaces as well, in case the addresses are
the same, but the pspaces aren't.

> +
>  	  if (*loc2p == old_loc)
> -	    {
> -	      found_object = 1;
> -	      break;
> -	    }
> +	    found_object = 1;
>  	}
>  
> +      /* We have already handled this address, update it so that we don't
> +	 have to go through updates again.  */
> +      last_addr = old_loc->address;
> +
> +      /* Target-side condition evaluation: Handle deleted locations.  */
> +      if (!found_object)
> +	force_breakpoint_reinsertion (old_loc);
> +
>        /* If this location is no longer present, and inserted, look if
>  	 there's maybe a new location at the same address.  If so,
>  	 mark that one inserted, and don't remove this one.  This is
> @@ -10852,6 +11455,10 @@ update_global_location_list (int should_
>  	    }
>  	  else
>  	    {
> +	      /* This location still exists, but it won't be kept in the
> +		 target since it may have been disabled.  We proceed to
> +		 remove its target-side condition.  */
> +
>  	      /* The location is either no longer present, or got
>  		 disabled.  See if there's another location at the
>  		 same address, in which case we don't need to remove
> @@ -11004,7 +11611,11 @@ update_global_location_list (int should_
>  	   never duplicated.  See the comments in field `duplicate' of
>  	   `struct bp_location'.  */
>  	  || is_tracepoint (b))
> -	continue;
> +	{
> +	  /* Clear the condition modification flag.  */
> +	  loc->condition_changed = condition_unchanged;
> +	  continue;
> +	}
>  
>        /* Permanent breakpoint should always be inserted.  */
>        if (b->enable_state == bp_permanent && ! loc->inserted)
> @@ -11027,6 +11638,13 @@ update_global_location_list (int should_
>  	{
>  	  *loc_first_p = loc;
>  	  loc->duplicate = 0;
> +
> +	  if (is_breakpoint (loc->owner) && loc->condition_changed)
> +	    {
> +	      loc->needs_update = 1;
> +	      /* Clear the condition modification flag.  */
> +	      loc->condition_changed = condition_unchanged;
> +	    }
>  	  continue;
>  	}
>  
> @@ -11038,6 +11656,9 @@ update_global_location_list (int should_
>  	swap_insertion (loc, *loc_first_p);
>        loc->duplicate = 1;
>  
> +      /* Clear the condition modification flag.  */
> +      loc->condition_changed = condition_unchanged;
> +
>        if ((*loc_first_p)->owner->enable_state == bp_permanent && loc->inserted
>  	  && b->enable_state != bp_permanent)
>  	internal_error (__FILE__, __LINE__,
> @@ -11045,10 +11666,21 @@ update_global_location_list (int should_
>  			"a permanent breakpoint"));
>      }
>  
> -  if (breakpoints_always_inserted_mode () && should_insert
> +  if (breakpoints_always_inserted_mode ()
>        && (have_live_inferiors ()
>  	  || (gdbarch_has_global_breakpoints (target_gdbarch))))
> -    insert_breakpoint_locations ();
> +    {
> +      if (should_insert)
> +	insert_breakpoint_locations ();
> +      else
> +	{
> +	  /* Though should_insert is false, we may need to update conditions
> +	     on the target's side if it is evaluating such conditions.  We
> +	     only update conditions for locations that are marked
> +	     "needs_update".  */
> +	  update_inserted_breakpoint_locations ();
> +	}
> +    }
>  
>    if (should_insert)
>      download_tracepoint_locations ();
> @@ -11162,6 +11794,8 @@ static void
>  bp_location_dtor (struct bp_location *self)
>  {
>    xfree (self->cond);
> +  if (self->cond_bytecode)
> +    free_agent_expr (self->cond_bytecode);
>    xfree (self->function_name);
>    xfree (self->source_file);
>  }
> @@ -12855,6 +13489,9 @@ disable_breakpoint (struct breakpoint *b
>  
>    bpt->enable_state = bp_disabled;
>  
> +  /* Mark breakpoint locations modified.  */
> +  mark_breakpoint_modified (bpt);
> +
>    if (target_supports_enable_disable_tracepoint ()
>        && current_trace_status ()->running && is_tracepoint (bpt))
>      {
> @@ -12902,7 +13539,11 @@ disable_command (char *args, int from_tt
>        struct bp_location *loc = find_location_by_number (args);
>        if (loc)
>  	{
> -	  loc->enabled = 0;
> +	  if (loc->enabled)
> +	    {
> +	      loc->enabled = 0;
> +	      mark_breakpoint_location_modified (loc);
> +	    }
>  	  if (target_supports_enable_disable_tracepoint ()
>  	      && current_trace_status ()->running && loc->owner
>  	      && is_tracepoint (loc->owner))
> @@ -12959,6 +13600,11 @@ enable_breakpoint_disp (struct breakpoin
>    if (bpt->enable_state != bp_permanent)
>      bpt->enable_state = bp_enabled;
>  
> +  bpt->enable_state = bp_enabled;
> +
> +  /* Mark breakpoint locations modified.  */
> +  mark_breakpoint_modified (bpt);
> +
>    if (target_supports_enable_disable_tracepoint ()
>        && current_trace_status ()->running && is_tracepoint (bpt))
>      {
> @@ -13018,7 +13664,11 @@ enable_command (char *args, int from_tty
>        struct bp_location *loc = find_location_by_number (args);
>        if (loc)
>  	{
> -	  loc->enabled = 1;
> +	  if (!loc->enabled)
> +	    {
> +	      loc->enabled = 1;
> +	      mark_breakpoint_location_modified (loc);
> +	    }
>  	  if (target_supports_enable_disable_tracepoint ()
>  	      && current_trace_status ()->running && loc->owner
>  	      && is_tracepoint (loc->owner))
> @@ -14459,8 +15109,9 @@ The \"Type\" column indicates one of:\n\
>  \twatchpoint     - watchpoint\n\
>  The \"Disp\" column contains one of \"keep\", \"del\", or \"dis\" to indicate\n\
>  the disposition of the breakpoint after it gets hit.  \"dis\" means that the\n\
> -breakpoint will be disabled.  The \"Address\" and \"What\" columns indicate the\n\
> -address and file/line number respectively.\n\
> +breakpoint will be disabled.  The \"Address\" and \"What\" columns indicate\n\
> +the address and file/line number respectively.  The \"Cond Eval\" column\n\
> +indicates where the breakpoint condition will be evaluated.\n\
>  \n\
>  Convenience variable \"$_\" and default examine address for \"x\"\n\
>  are set to the address of the last breakpoint listed unless the command\n\
> @@ -14476,8 +15127,9 @@ The \"Type\" column indicates one of:\n\
>  \twatchpoint     - watchpoint\n\
>  The \"Disp\" column contains one of \"keep\", \"del\", or \"dis\" to indicate\n\
>  the disposition of the breakpoint after it gets hit.  \"dis\" means that the\n\
> -breakpoint will be disabled.  The \"Address\" and \"What\" columns indicate the\n\
> -address and file/line number respectively.\n\
> +breakpoint will be disabled.  The \"Address\" and \"What\" columns indicate\n\
> +the address and file/line number respectively.  The \"Cond Eval\" column\n\
> +indicates where the breakpoint condition will be evaluated.\n\
>  \n\
>  Convenience variable \"$_\" and default examine address for \"x\"\n\
>  are set to the address of the last breakpoint listed unless the command\n\
> @@ -14495,8 +15147,9 @@ The \"Type\" column indicates one of:\n\
>  \twatchpoint     - watchpoint\n\
>  The \"Disp\" column contains one of \"keep\", \"del\", or \"dis\" to indicate\n\
>  the disposition of the breakpoint after it gets hit.  \"dis\" means that the\n\
> -breakpoint will be disabled.  The \"Address\" and \"What\" columns indicate the\n\
> -address and file/line number respectively.\n\
> +breakpoint will be disabled.  The \"Address\" and \"What\" columns indicate\n\
> +the address and file/line number respectively.  The \"Cond Eval\" column\n\
> +indicates where the breakpoint condition will be evaluated.\n\
>  \n\
>  Convenience variable \"$_\" and default examine address for \"x\"\n\
>  are set to the address of the last breakpoint listed unless the command\n\
> @@ -14515,8 +15168,9 @@ The \"Type\" column indicates one of:\n\
>  \tfinish         - internal breakpoint used by the \"finish\" command\n\
>  The \"Disp\" column contains one of \"keep\", \"del\", or \"dis\" to indicate\n\
>  the disposition of the breakpoint after it gets hit.  \"dis\" means that the\n\
> -breakpoint will be disabled.  The \"Address\" and \"What\" columns indicate the\n\
> -address and file/line number respectively.\n\
> +breakpoint will be disabled.  The \"Address\" and \"What\" columns indicate\n\
> +the address and file/line number respectively.  The \"Cond Eval\" column\n\
> +indicates where the breakpoint condition will be evaluated.\n\

It didn't look like it is really true that we have a new "Cond Eval" column?
Can you show an example of what the new output looks like?


>  \n\
>  Convenience variable \"$_\" and default examine address for \"x\"\n\
>  are set to the address of the last breakpoint listed unless the command\n\
> @@ -14793,6 +15447,23 @@ inferior in all-stop mode, gdb behaves a
>  			   &breakpoint_set_cmdlist,
>  			   &breakpoint_show_cmdlist);
>  
> +  add_setshow_enum_cmd ("condition-evaluation", class_breakpoint,
> +			condition_evaluation_enums,
> +			&condition_evaluation_mode_1, _("\
> +Set mode of breakpoint condition evaluation."), _("\
> +Show mode of breakpoint condition evaluation."), _("\
> +When this is set to \"gdb\", breakpoint conditions will be\n\
> +evaluated on the host's side by GDB.  When it is set to \"target\",\n\
> +breakpoint conditions will be downloaded to the target (if the target\n\
> +supports such feature) and conditions will be evaluated on the target's side.\n\
> +If this is set to \"auto\" (default), this will be automatically set to\n\
> +\"target\" if it supports condition evaluation, otherwise it will\n\
> +be set to \"gdb\""),
> +			   &set_condition_evaluation_mode,
> +			   &show_condition_evaluation_mode,
> +			   &breakpoint_set_cmdlist,
> +			   &breakpoint_show_cmdlist);
> +
>    add_com ("break-range", class_breakpoint, break_range_command, _("\
>  Set a breakpoint for an address range.\n\
>  break-range START-LOCATION, END-LOCATION\n\
> Index: gdb/gdb/breakpoint.h
> ===================================================================
> --- gdb.orig/gdb/breakpoint.h	2012-02-22 13:22:16.906553986 -0200
> +++ gdb/gdb/breakpoint.h	2012-02-22 13:22:49.190553987 -0200

> +/* Returns true if BPT is a breakpoint of any kind.  */
> +
> +extern int is_breakpoint (const struct breakpoint *bpt);

I see you've updated the comment on the .c side.  Please update it here as
well (to be more specific).

-- 
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]