From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26414 invoked by alias); 8 Feb 2012 23:14:38 -0000 Received: (qmail 26403 invoked by uid 22791); 8 Feb 2012 23:14:35 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,TW_EG X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 08 Feb 2012 23:14:14 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1RvGiL-0003PI-Hr from Luis_Gustavo@mentor.com ; Wed, 08 Feb 2012 15:14:13 -0800 Received: from NA1-MAIL.mgc.mentorg.com ([147.34.98.181]) by svr-orw-fem-01.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Wed, 8 Feb 2012 15:14:13 -0800 Received: from [0.0.0.0] ([172.16.63.104]) by NA1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Wed, 8 Feb 2012 15:14:12 -0800 Message-ID: <4F3301C1.9070400@mentor.com> Date: Wed, 08 Feb 2012 23:14:00 -0000 From: Luis Gustavo Reply-To: "Gustavo, Luis" User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.23) Gecko/20110922 Lightning/1.0b2 Thunderbird/3.1.15 MIME-Version: 1.0 To: Stan Shebs CC: gdb-patches@sourceware.org Subject: Re: [rfc target-side break conditions 5/5 v2] GDBServer-side changes References: <4F230A29.3060404@mentor.com> <4F31A249.9000800@earthlink.net> In-Reply-To: <4F31A249.9000800@earthlink.net> Content-Type: multipart/mixed; boundary="------------010204050702050408050802" X-IsSubscribed: yes 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/msg00121.txt.bz2 This is a multi-part message in MIME format. --------------010204050702050408050802 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 643 On 02/07/2012 08:14 PM, Stan Shebs wrote: > On 1/27/12 12:33 PM, Luis Gustavo wrote: >> This updated patch addresses all the comments made before and also >> handles the updated Z packet format with the "conditions" marker. >> >> I've added a function that parses (a generic set of) options passed >> through the Z packets. > > This all looks ready to go in, modulo my syntax characters comment from > earlier. > > Incidentally, don't we officially say "GDBserver" rather than "GDBServer"? The code that handles parsing of conditions in the packet has been updated. I fixed the NEWS and other places to use GDBserver instead. Thanks. Luis --------------010204050702050408050802 Content-Type: text/x-patch; name="0004-break_condition_gdbserver.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0004-break_condition_gdbserver.diff" Content-length: 10833 2012-02-08 Luis Machado gdbserver/ * server.c (handle_query): Advertise support for target-side breakpoint condition evaluation. (process_point_options): New function. (process_serial_event): When inserting a breakpoint, check for a target-side condition that should be evaluated. * mem-break.c: Include regcache.h and ax.h. (point_cond_list_t): New data structure. (breakpoint) : New field. (find_gdb_breakpoint_at): Make non-static. (delete_gdb_breakpoint_at): Clear any target-side conditions. (clear_gdb_breakpoint_conditions): New function. (add_condition_to_breakpoint): Likewise. (add_breakpoint_condition): Likewise. (gdb_condition_true_at_breakpoint): Likewise. (gdb_breakpoint_here): Return result directly instead of going through a local variable. * mem-break.h (find_gdb_breakpoint_at): New prototype. (clear_gdb_breakpoint_conditions): Likewise. (add_breakpoint_condition): Likewise. (gdb_condition_true_at_breakpoint): Likewise. * linux-low.c (linux_wait_1): Evaluate target-side breakpoint condition. (need_step_over_p): Take target-side breakpoint condition into consideration. Index: gdb/gdb/gdbserver/server.c =================================================================== --- gdb.orig/gdb/gdbserver/server.c 2012-02-08 15:57:07.399075002 -0200 +++ gdb/gdb/gdbserver/server.c 2012-02-08 15:57:33.139074999 -0200 @@ -1621,6 +1621,9 @@ handle_query (char *own_buf, int packet_ strcat (own_buf, ";tracenz+"); } + /* Support target-side breakpoint conditions. */ + strcat (own_buf, ";ConditionalBreakpoints+"); + return; } @@ -2825,6 +2828,35 @@ main (int argc, char *argv[]) } } +/* Process options coming from Z packets for *point at address + POINT_ADDR. PACKET is the packet buffer. *PACKET is updated + to point to the first char after the last processed option. */ + +static void +process_point_options (CORE_ADDR point_addr, char **packet) +{ + char *dataptr = *packet; + + while (dataptr[0] == ';') + { + dataptr++; + + if (!strncmp (dataptr, "conditions=", strlen ("conditions="))) + { + /* We have conditions to parse. */ + dataptr += strlen ("conditions="); + + /* Insert conditions. */ + do + { + add_breakpoint_condition (point_addr, &dataptr); + } while (*dataptr == 'X'); + } + } + + *packet = dataptr; +} + /* Event loop callback that handles a serial event. The first byte in the serial buffer gets us here. We expect characters to arrive at a brisk pace, so we read the rest of the packet with a blocking @@ -3147,7 +3179,22 @@ process_serial_event (void) case '4': /* access watchpoint */ require_running (own_buf); if (insert && the_target->insert_point != NULL) - res = (*the_target->insert_point) (type, addr, len); + { + /* Insert the breakpoint. If it is already inserted, nothing + will take place. */ + res = (*the_target->insert_point) (type, addr, len); + + /* GDB may have sent us a list of *point parameters to be + evaluated on the target's side. Read such list here. If we + already have a list of parameters, GDB is telling us to drop + that list and use this one instead. */ + if (!res && (type == '0' || type == '1')) + { + /* Remove previous conditions. */ + clear_gdb_breakpoint_conditions (addr); + process_point_options (addr, &dataptr); + } + } else if (!insert && the_target->remove_point != NULL) res = (*the_target->remove_point) (type, addr, len); break; Index: gdb/gdb/gdbserver/mem-break.c =================================================================== --- gdb.orig/gdb/gdbserver/mem-break.c 2012-02-08 15:57:07.387074997 -0200 +++ gdb/gdb/gdbserver/mem-break.c 2012-02-08 15:57:33.143074994 -0200 @@ -20,6 +20,8 @@ along with this program. If not, see . */ #include "server.h" +#include "regcache.h" +#include "ax.h" const unsigned char *breakpoint_data; int breakpoint_len; @@ -85,6 +87,16 @@ enum bkpt_type other_breakpoint, }; +typedef struct point_cond_list_t +{ + /* Pointer to the agent expression that is the breakpoint's + conditional. */ + struct agent_expr *cond; + + /* Pointer to the next condition. */ + struct point_cond_list_t *next; +} point_cond_list_t; + /* A high level (in gdbserver's perspective) breakpoint. */ struct breakpoint { @@ -93,6 +105,12 @@ struct breakpoint /* The breakpoint's type. */ enum bkpt_type type; + /* Pointer to the condition list that should be evaluated on + the target or NULL if the breakpoint is unconditional or + if GDB doesn't want us to evaluate the conditionals on the + target's side. */ + struct point_cond_list_t *cond_list; + /* Link to this breakpoint's raw breakpoint. This is always non-NULL. */ struct raw_breakpoint *raw; @@ -632,7 +650,7 @@ delete_breakpoint (struct breakpoint *to return delete_breakpoint_1 (proc, todel); } -static struct breakpoint * +struct breakpoint * find_gdb_breakpoint_at (CORE_ADDR where) { struct process_info *proc = current_process (); @@ -692,6 +710,9 @@ delete_gdb_breakpoint_at (CORE_ADDR addr if (bp == NULL) return -1; + /* Before deleting the breakpoint, make sure to free + its condition list. */ + clear_gdb_breakpoint_conditions (addr); err = delete_breakpoint (bp); if (err) return -1; @@ -699,12 +720,118 @@ delete_gdb_breakpoint_at (CORE_ADDR addr return 0; } +/* Clear all conditions associated with this breakpoint address. */ + +void +clear_gdb_breakpoint_conditions (CORE_ADDR addr) +{ + struct breakpoint *bp = find_gdb_breakpoint_at (addr); + struct point_cond_list_t *cond, **cond_p; + + if (!bp || (bp && !bp->cond_list)) + return; + + cond = bp->cond_list; + cond_p = &bp->cond_list->next; + + while (cond != NULL) + { + free (cond->cond); + free (cond); + cond = *cond_p; + cond_p = &cond->next; + } + + bp->cond_list = NULL; +} + +/* Add condition CONDITION to GDBServer's breakpoint BP. */ + +void +add_condition_to_breakpoint (struct breakpoint *bp, + struct agent_expr *condition) +{ + point_cond_list_t *new_cond; + + /* Create new condition. */ + new_cond = xcalloc (1, sizeof (*new_cond)); + new_cond->cond = condition; + + /* Add condition to the list. */ + new_cond->next = bp->cond_list; + bp->cond_list = new_cond; +} + +/* Add a target-side condition CONDITION to the breakpoint at ADDR. */ + int -gdb_breakpoint_here (CORE_ADDR where) +add_breakpoint_condition (CORE_ADDR addr, char **condition) +{ + struct breakpoint *bp = find_gdb_breakpoint_at (addr); + char *actparm = (char *) *condition; + struct agent_expr *cond; + + if (!bp) + return 1; + + if (condition == NULL) + return 1; + + cond = gdb_parse_agent_expr (&actparm); + + if (!cond) + { + fprintf (stderr, "Condition evaluation failed. " + "Assuming unconditional.\n"); + return 0; + } + + add_condition_to_breakpoint (bp, cond); + + *condition = actparm; + + return 0; +} + +/* Evaluate condition (if any) at breakpoint BP. Return 1 if + true and 0 otherwise. */ + +int +gdb_condition_true_at_breakpoint (CORE_ADDR where) { + /* Fetch registers for the current inferior. */ struct breakpoint *bp = find_gdb_breakpoint_at (where); + ULONGEST value = 0; + struct point_cond_list_t *cl; - return (bp != NULL); + struct regcache *regcache = get_thread_regcache (current_inferior, 1); + + if (!bp) + return 0; + + /* Check if the breakpoint is unconditional. If it is, + the condition always evaluates to TRUE. */ + if (!bp->cond_list) + return 1; + + /* Evaluate each condition in the breakpoint's list of conditions. + Return true if any of the conditions evaluate to TRUE. */ + for (cl = bp->cond_list; cl && !value; cl = cl->next) + { + /* Evaluate the condition. */ + gdb_eval_agent_expr (regcache, NULL, cl->cond, &value); + } + + return (value != 0); +} + +/* Return 1 if there is a breakpoint inserted in address WHERE + and if its condition, if it exists, is true. */ + +int +gdb_breakpoint_here (CORE_ADDR where) +{ + return (find_gdb_breakpoint_at (where) != NULL); } void Index: gdb/gdb/gdbserver/mem-break.h =================================================================== --- gdb.orig/gdb/gdbserver/mem-break.h 2012-02-08 15:57:07.411075000 -0200 +++ gdb/gdb/gdbserver/mem-break.h 2012-02-08 15:57:33.143074994 -0200 @@ -25,6 +25,11 @@ struct breakpoint; struct fast_tracepoint_jump; +/* Locate a breakpoint placed at address WHERE and return a pointer + to its structure. */ + +struct breakpoint * find_gdb_breakpoint_at (CORE_ADDR where); + /* Create a new GDB breakpoint at WHERE. Returns -1 if breakpoints are not supported on this target, 0 otherwise. */ @@ -39,6 +44,19 @@ int breakpoint_here (CORE_ADDR addr); int breakpoint_inserted_here (CORE_ADDR addr); +/* Clear all breakpoint conditions associated with this address. */ + +void clear_gdb_breakpoint_conditions (CORE_ADDR addr); + +/* Set target-side condition CONDITION to the breakpoint at ADDR. */ + +int add_breakpoint_condition (CORE_ADDR addr, char **condition); + +/* Evaluation condition (if any) at breakpoint BP. Return 1 if + true and 0 otherwise. */ + +int gdb_condition_true_at_breakpoint (CORE_ADDR where); + /* Returns TRUE if there's a GDB breakpoint set at ADDR. */ int gdb_breakpoint_here (CORE_ADDR where); Index: gdb/gdb/gdbserver/linux-low.c =================================================================== --- gdb.orig/gdb/gdbserver/linux-low.c 2012-02-08 15:57:07.427075000 -0200 +++ gdb/gdb/gdbserver/linux-low.c 2012-02-08 15:57:33.147074994 -0200 @@ -2398,7 +2398,8 @@ Check if we're already there.\n", || event_child->stopped_by_watchpoint || (!step_over_finished && !bp_explains_trap && !trace_event) - || gdb_breakpoint_here (event_child->stop_pc)); + || (gdb_breakpoint_here (event_child->stop_pc) + && gdb_condition_true_at_breakpoint (event_child->stop_pc))); /* We found no reason GDB would want us to stop. We either hit one of our own breakpoints, or finished an internal step GDB @@ -3261,8 +3262,10 @@ need_step_over_p (struct inferior_list_e if (breakpoint_here (pc) || fast_tracepoint_jump_here (pc)) { /* Don't step over a breakpoint that GDB expects to hit - though. */ - if (gdb_breakpoint_here (pc)) + though. If the condition is being evaluated on the target's side + and it evaluate to false, step over this breakpoint as well. */ + if (gdb_breakpoint_here (pc) + && gdb_condition_true_at_breakpoint (pc)) { if (debug_threads) fprintf (stderr, --------------010204050702050408050802--