From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8904 invoked by alias); 23 Feb 2012 17:25:20 -0000 Received: (qmail 8755 invoked by uid 22791); 23 Feb 2012 17:25:05 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 23 Feb 2012 17:24:43 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q1NHOdkr028366 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 23 Feb 2012 12:24:39 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q1NHObcO007887; Thu, 23 Feb 2012 12:24:37 -0500 Message-ID: <4F467655.6040603@redhat.com> Date: Thu, 23 Feb 2012 17:38:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20120131 Thunderbird/10.0 MIME-Version: 1.0 To: "Gustavo, Luis" CC: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [rfc target-side break conditions 3/5 v2] GDB-side changes References: <4F230A13.9060400@mentor.com> <4F3107C2.2060400@mentor.com> <4F32CDF2.8090906@redhat.com> <4F32E167.6050600@mentor.com> <4F33C27E.2090304@redhat.com> <4F4508BA.8060901@mentor.com> In-Reply-To: <4F4508BA.8060901@mentor.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-02/txt/msg00508.txt.bz2 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 > > * 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) > : 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) : New field. > (bp_location) : 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