[RFC stub-side break conditions 5/5] Group agent expression-specific code in its own file

[Luis Machado <luis_gustavo at mentor dot com>]

Hi,

This patch makes GDBServer advertise support for evaluating breakpoint 
conditions and understand and interpret agent expressions sent from GDB 
via the z0/z1 packets.

GDBServer's behavior upon receiving a z0/z1 packet is still idempotent 
as it will drop any previous conditions it knows about and will consider 
only the ones contained within this received packet. If the packet does 
not contain conditions, it is considered an unconditional breakpoint.

I ran the testsuite with GDBServer and only one regression showed up in 
gdb.cp/mb-templates.exp. It may be a hidden bug that was exposed by 
these changes since it gives me an internal error when trying to parse 
the conditional expression.

Luis
lgustavo@codesourcery.com

2012-01-05  Luis Machado  <lgustavo@codesourcery>

	gdbserver/
	* server.c (handle_query): Advertise support for target-side
	breakpoint condition evaluation.
	(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) <cond_list>: 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-01-04 14:02:33.686431998 -0200
+++ gdb/gdb/gdbserver/server.c	2012-01-04 14:03:08.014431998 -0200
@@ -1617,6 +1617,9 @@ handle_query (char *own_buf, int packet_
 	  strcat (own_buf, ";tracenz+");
 	}
 
+      /* Support target-side breakpoint conditions.  */
+      strcat (own_buf, ";ConditionalBreakpoints+");
+
       return;
     }
 
@@ -3124,7 +3127,29 @@ 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 conditions to be evaluated
+		   on the target's side.  Read such list here.  If we already
+		   have a list of conditions, 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);
+
+		    /* Insert conditions.  */
+		    while (dataptr[0] == ':')
+		      {
+			dataptr++;
+
+			add_breakpoint_condition (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-01-04 14:02:33.682431998 -0200
+++ gdb/gdb/gdbserver/mem-break.c	2012-01-04 14:03:08.014431998 -0200
@@ -20,6 +20,8 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #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,119 @@ 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 != NULL && !value; cl = cl->next)
+    {
+      /* Evaluate the condition.  */
+      gdb_eval_agent_expr (regcache, NULL, cl->cond, &value);
+      value = value? 1 : 0;
+    }
+
+  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-01-04 14:02:33.682431998 -0200
+++ gdb/gdb/gdbserver/mem-break.h	2012-01-04 14:03:08.014431998 -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-01-04 14:02:33.682431998 -0200
+++ gdb/gdb/gdbserver/linux-low.c	2012-01-04 14:03:08.026431998 -0200
@@ -2391,7 +2391,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
@@ -3254,8 +3255,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,