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: [PATCH 2/4] Hardware accelerated watchpoint conditions


On Fri 05 Feb 2010 03:15:55 Joel Brobecker wrote:
> Good news (see below).

Great. :-)
 
> > > I think, from your patch, that GDB will still evaluate the condition
> > > once after the watchpoint and its condition trigger. I think that
> > > we might want to fix that eventually, but I am actually more than
> > > happy to ignore this minor issue for now.  I like baby steps :).
> >
> > If I'm following your thought here, I added that on purpose. Before
> > letting GDB evaluate the condition one more time, GDB wouldn't know
> > which watchpoint triggered if there were two at the same location,
> > with different conditions.
> 
> I knew that! (if the name DiNozzo comes to mind, then I don't
> know what you're talking about ;-) )

Oh, sorry then!
 
> Just a few minor nits - pre-approved after the comments are addressed.

I addressed your comments. I won't commit the patch yet because the kernel patch enabling this code to 
work hasn't been committed upstream yet. I expect it will be accepted soon, so I hope to get this in in 
time for 7.1.

The same applies to the first patch in this series (adapting ppc-linux-nat.c to the new ptrace interface), 
which you approved a while ago...
 
> > +/* This function is used to determine whether the condition associated
> > +   with bp_location B is of the form:
> > +
> > +   watch *<ADDRESS> if <VAR> == <LITERAL>
> > +
> > +   If it is, then it sets DATA_VALUE to LITERAL and returns 1.
> > +   Otherwise, it returns 0.  */
> 
> Can you add a note that VAR must be stored at ADDRESS too?  Same for
> similar functions...

Done.
 
> > +  /* At this point, all verifications were positive, so we can use
> > +     hardware-assisted data-matching.  Set the data value, and return
> > +     non-zero.  */
> 
> The comment here is a bit too purpose-specific. It betrays its origin :),
> but I think we should either drop it (I think that the function description
> is sufficently clear), or change it to something more neutral.  Again,
> same for all other functions that used the same kind of comment.

Dropped.

> > +/* Return greater than zero if the condition associated with
> > +   the watchpoint `b' can be treated by the hardware; zero otherwise.
> > +
> > +   Also stores the data value in `data_value'.  */
> 
> I'd drop this comment in breakpoint.h. It duplicates the descriptions
> in breakpoint.c...

Dropped.
 
> > +static int
> > +ppc_linux_can_use_watchpoint_cond_accel (void)
> 
> This function now needs a short description. It's kind of obvious,
> what this function does, I agree, but we're trying to be consistent
> in our style, and provide always function descriptions unless the
> function implements a routine (function pointer) that's already
> documented (eg a gdbarch method, or a target_ops routine).

I added this concise description:

/* Check whether we have at least one free DVC register.  */
 
> > +	if (p->hw_breaks[i].hw_break != NULL
> > +	    && p->hw_breaks[i].hw_break->condition_mode
> > +		    != PPC_BREAKPOINT_CONDITION_NONE)
> 
> I'm not quite sure, here, but I think that the GNU Coding Style asks
> us to parenthesize your not-equal condition, purely for the benefit of
> GNU indent:
> 
>      if (p->hw_breaks[i].hw_break != NULL
>          && (p->hw_breaks[i].hw_break->condition_mode
>              != PPC_BREAKPOINT_CONDITION_NONE))
> 
> Can you double-check for me?

It does mention that. Changed.
 
> > +      if (cond && ppc_linux_can_use_watchpoint_cond_accel ()
> > +	  && (watch_address_if_address_equal_literal (exp, cond, &cond_addr)
> > +	      || watch_var_if_var_equal_literal (exp, cond, &cond_addr)
> > +	      || watch_var_if_address_equal_literal (exp, cond, &cond_addr)
> > +	      || watch_address_if_var_equal_literal (exp, cond, &cond_addr))) {
> 
> Formatting nit: The curly brace should be on the next line.  There are
> a few instances where this needs to be fixed.

Fixed this and the other instances.
 
> > +	p.condition_mode  = PPC_BREAKPOINT_CONDITION_AND
> > +			    | PPC_BREAKPOINT_CONDITION_BE (byte_to_enable);
> 
> Similar to above, I think you'll need to parenthesize the rhs.
> 

Fixed this and the other instance.

> > diff --git a/gdb/target.c b/gdb/target.c
> > index e6659c9..7adc96e 100644
> > --- a/gdb/target.c
> > +++ b/gdb/target.c
> > @@ -44,6 +44,8 @@
> >  #include "exec.h"
> >  #include "inline-frame.h"
> >
> > +struct expression;
> > +
> 
> This change is unnecessary (already declared in target.h)...

Dropped.
 
> > diff --git a/gdb/target.h b/gdb/target.h
> > index 7103ab2..a65e900 100644
> > --- a/gdb/target.h
> > +++ b/gdb/target.h
> > @@ -36,6 +36,10 @@ struct trace_status;
> >  struct uploaded_tsv;
> >  struct uploaded_tp;
> >
> > +struct bp_location;
> > +struct breakpoint;
> > +struct expression;
> 
> I don't think that declaring struct bp_location is necessary.
> I just see that struct breakpoint should probably have been declared
> before your patch - so OK.

Dropped struct bp_location, kept the other two.
 
> > +    int (*to_remove_watchpoint) (CORE_ADDR, int, int, struct expression
> > *, +						      struct expression *);
> > +    int (*to_insert_watchpoint) (CORE_ADDR, int, int, struct expression
> > *, +						      struct expression *);
> 
> Would you mind adding a comment explaining that documentation of what
> these routines are expected to do is provided with the corresponding
> target_* macros? I personally think that the complete description
> of what these routines are supposed to do should be right there rather
> than with the target_ macros, but that's only a very minor matter and
> nothing to do with your patch.

Added:

   /* Documentation of what the two routines below are expected to do is
       provided with the corresponding target_* macros.  */

> > +#ifndef target_region_ok_for_hw_watchpoint
> >  #define target_region_ok_for_hw_watchpoint(addr, len) \
> >      (*current_target.to_region_ok_for_hw_watchpoint) (addr, len)
> > -
> > +#endif
> 
> This is not right - we no longer allow a macro to be overridden.

Ok, dropped.
 
> >  /* Set/clear a hardware watchpoint starting at ADDR, for LEN bytes. 
> > TYPE is 0 for write, 1 for read, and 2 for read/write accesses.  Returns
> > 0 for success, non-zero for failure.  */
> >
> > -#define	target_insert_watchpoint(addr, len, type)	\
> > -     (*current_target.to_insert_watchpoint) (addr, len, type)
> > +#define	target_insert_watchpoint(addr, len, type, watch_exp, cond) \
> > +     (*current_target.to_insert_watchpoint) (addr, len, type, watch_exp,
> > cond)
> 
> The macro description needs to be updated (to mention the two new
> parameters).

Updated.
 
> > -#define	target_remove_watchpoint(addr, len, type)	\
> > -     (*current_target.to_remove_watchpoint) (addr, len, type)
> > +#define	target_remove_watchpoint(addr, len, type, watch_exp, cond) \
> > +     (*current_target.to_remove_watchpoint) (addr, len, type, watch_exp,
> > cond)
> 
> Can you add a short documentation for this macro?
 
It is already documented, in a comment above target_insert_watchpoint (which
is defined right before target_remove_watchpoint):

/* Set/clear a hardware watchpoint starting at ADDR, for LEN bytes.  TYPE is 0
   for write, 1 for read, and 2 for read/write accesses.  WATCH_EXP is the
   watchpoint's expression and COND is the expression for its condition,
   or NULL if there's none.  Returns 0 for success, non-zero for failure.  */

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


2010-02-08  Sergio Durigan Junior  <sergiodj@linux.vnet.ibm.com>
	    Thiago Jung Bauermann  <bauerman@br.ibm.com>

	* target.h: Add opaque declarations for bp_location, breakpoint and
	expression.
	(struct target_ops) <to_insert_watchpoint>,
	<to_remove_watchpoint>: Add new arguments to pass the watchpoint
	expression and its condition.  Update all callers and implementations.
	(target_region_ok_for_hw_watchpoint): Surround with ifndef.
	* breakpoint.c (exp_is_address): New function.
	(exp_is_var): Ditto.
	(exp_is_address_equal_literal): Ditto.
	(exp_is_var_equal_literal): Ditto.
	(watch_address_if_var_equal_literal): Ditto.
	(watch_var_if_address_equal_literal): Ditto.
	(watch_var_if_var_equal_literal): Ditto.
	(watch_address_if_address_equal_literal): Ditto.
	* breakpoint.h (watch_address_if_address_equal_literal): Declare.
	(watch_var_if_var_equal_literal): Ditto.
	(watch_address_if_var_equal_literal): Ditto.
	(watch_var_if_address_equal_literal): Ditto.
	* ppc-linux-nat.c (ppc_linux_can_use_watchpoint_cond_accel): New
	function.
	(ppc_linux_insert_watchpoint): Check if it is possible to use
	hardware-accelerated condition checking.
	(ppc_linux_remove_watchpoint): Check if the watchpoint to delete
	has hardware-accelerated condition checking.
commit f29957ec421daceca6e557d16549e0efc0358ff8
Author: Thiago Jung Bauermann <bauermann@hactar.cps.virtua.com.br>
Date:   Mon Dec 28 14:25:01 2009 -0200

    Hardware accelerated watchpoint conditions.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 0d55862..98f1cd1 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1485,9 +1485,10 @@ Note: automatically using hardware breakpoints for read-only addresses.\n"));
 	      watchpoints.  It's not clear that it's necessary... */
 	   && bpt->owner->disposition != disp_del_at_next_stop)
     {
-      val = target_insert_watchpoint (bpt->address, 
+      val = target_insert_watchpoint (bpt->address,
 				      bpt->length,
-				      bpt->watchpoint_type);
+				      bpt->watchpoint_type,
+				      bpt->owner->exp, bpt->cond);
       bpt->inserted = (val != -1);
     }
 
@@ -2109,12 +2110,10 @@ remove_breakpoint_1 (struct bp_location *b, insertion_state_t is)
     }
   else if (b->loc_type == bp_loc_hardware_watchpoint)
     {
-      struct value *v;
-      struct value *n;
-
       b->inserted = (is == mark_inserted);
-      val = target_remove_watchpoint (b->address, b->length, 
-				      b->watchpoint_type);
+      val = target_remove_watchpoint (b->address, b->length,
+				      b->watchpoint_type,
+				      b->owner->exp, b->cond);
 
       /* Failure to remove any of the hardware watchpoints comes here.  */
       if ((is == mark_uninserted) && (b->inserted))
@@ -10156,6 +10155,210 @@ tracepoint_save_command (char *args, int from_tty)
   return;
 }
 
+/* This function checks if the expression is of the form "*<address>".
+   It returns 1 if it is, 0 otherwise.  */
+static int
+exp_is_address (struct expression *exp)
+{
+  if (exp->nelts != 5
+      || exp->elts[0].opcode != UNOP_IND
+      || exp->elts[1].opcode != OP_LONG)
+      return 0;
+  return 1;
+}
+
+/* This function checks if the expression is of the form "<var>".
+   It returns 1 if it is, 0 otherwise.  */
+static int
+exp_is_var (struct expression *exp)
+{
+  if (exp->nelts != 4
+      || exp->elts[0].opcode != OP_VAR_VALUE)
+      return 0;
+  return 1;
+}
+
+/* This function checks if the expression is of the form
+   "*<address> == LITERAL".  It returns 1 if it is, 0 otherwise.  */
+static int
+exp_is_address_equal_literal (struct expression *exp)
+{
+  if (exp->nelts == 10
+      && exp->elts[0].opcode == BINOP_EQUAL
+      && exp->elts[1].opcode == UNOP_IND
+      && exp->elts[2].opcode == OP_LONG
+      && exp->elts[6].opcode == OP_LONG)
+      return 1;
+  else if (exp->nelts == 11
+      && exp->elts[0].opcode == BINOP_EQUAL
+      && exp->elts[1].opcode == UNOP_IND
+      && exp->elts[2].opcode == OP_LONG
+      && exp->elts[6].opcode == UNOP_NEG
+      && exp->elts[7].opcode == OP_LONG)
+      return 1;
+
+  return 0;
+}
+
+/* This function checks if the expression is of the form "<var> == LITERAL".
+   It returns 1 if it is, 0 otherwise.  */
+static int
+exp_is_var_equal_literal (struct expression *exp)
+{
+  if (exp->nelts == 9
+      && exp->elts[0].opcode == BINOP_EQUAL
+      && exp->elts[1].opcode == OP_VAR_VALUE
+      && exp->elts[5].opcode == OP_LONG)
+      return 1;
+  else if (exp->nelts == 10
+      && exp->elts[0].opcode == BINOP_EQUAL
+      && exp->elts[1].opcode == OP_VAR_VALUE
+      && exp->elts[5].opcode == UNOP_NEG
+      && exp->elts[6].opcode == OP_LONG)
+      return 1;
+
+  return 0;
+}
+
+/* This function is used to determine whether the condition associated
+   with bp_location B is of the form:
+
+   watch *<ADDRESS> if <VAR> == <LITERAL>
+
+   It also checks whether <VAR> is stored at <ADDRESS>.  If these
+   conditions are met, then it sets DATA_VALUE to LITERAL and returns 1.
+   Otherwise, it returns 0.  */
+int
+watch_address_if_var_equal_literal (struct expression *exp,
+				    struct expression *cond,
+				    CORE_ADDR *data_value)
+{
+  int pc;
+  CORE_ADDR exp_address, cond_address;
+
+  if (!exp_is_address (exp)
+      || !exp_is_var_equal_literal (cond))
+    return 0;
+
+  exp_address = exp->elts[3].longconst;
+  cond_address = value_as_address (address_of_variable (cond->elts[3].symbol,
+							cond->elts[2].block));
+
+  /* Make sure that the two addresses are the same.  */
+  if (exp_address != cond_address)
+    return 0;
+
+  pc = 5;
+  *data_value = value_as_long (evaluate_subexp (NULL_TYPE, cond, &pc,
+						EVAL_NORMAL));
+
+  return 1;
+}
+
+/* This function is used to determine whether the condition associated
+   with bp_location B is of the form:
+
+   watch <VAR> if *<ADDRESS> == <LITERAL>
+
+   It also checks whether <VAR> is stored at <ADDRESS>.  If these
+   conditions are met, then it sets DATA_VALUE to LITERAL and returns 1.
+   Otherwise, it returns 0.  */
+int
+watch_var_if_address_equal_literal (struct expression *exp,
+				    struct expression *cond,
+				    CORE_ADDR *data_value)
+{
+  int pc;
+  CORE_ADDR exp_address, cond_address;
+
+  if (!exp_is_var (exp)
+      || !exp_is_address_equal_literal (cond))
+    return 0;
+
+  exp_address = value_as_address (address_of_variable (exp->elts[2].symbol,
+						       exp->elts[1].block));
+  cond_address = cond->elts[4].longconst;
+
+  /* Make sure that the two addresses are the same.  */
+  if (exp_address != cond_address)
+    return 0;
+
+  pc = 6;
+  *data_value = value_as_long (evaluate_subexp (NULL_TYPE, cond, &pc,
+						EVAL_NORMAL));
+
+  return 1;
+}
+
+/* This function is used to determine whether the condition associated
+   with bp_location B is of the form:
+
+   watch <VAR1> if <VAR2> == <LITERAL>
+
+   It also checks whether <VAR1> and <VAR2> are stored at the same address.
+   If these conditions are met, then it sets DATA_VALUE to LITERAL and
+   returns 1.  Otherwise, it returns 0.  */
+int
+watch_var_if_var_equal_literal (struct expression *exp,
+				struct expression *cond,
+				CORE_ADDR *data_value)
+{
+  int pc;
+  CORE_ADDR exp_address, cond_address;
+
+  if (!exp_is_var (exp)
+      || !exp_is_var_equal_literal (cond))
+    return 0;
+
+  exp_address = value_as_address (address_of_variable (exp->elts[2].symbol,
+						       exp->elts[1].block));
+  cond_address = value_as_address (address_of_variable (cond->elts[3].symbol,
+							cond->elts[2].block));
+
+  if (exp_address != cond_address)
+    return 0;
+
+  pc = 5;
+  *data_value = value_as_long (evaluate_subexp (NULL_TYPE, cond, &pc,
+						EVAL_NORMAL));
+
+  return 1;
+}
+
+/* This function is used to determine whether the condition associated
+   with bp_location B is of the form:
+
+   watch *<ADDRESS1> if *<ADDRESS2> == <LITERAL>
+
+   It also checks whether <ADDRESS1> AND <ADDRESS2> are the same.  If these
+   conditions are met, then it sets DATA_VALUE to LITERAL and returns 1.
+   Otherwise, it returns 0.  */
+int
+watch_address_if_address_equal_literal (struct expression *exp,
+					struct expression *cond,
+					CORE_ADDR *data_value)
+{
+  int pc;
+  CORE_ADDR exp_address, cond_address;
+
+  if (!exp_is_address (exp)
+      || !exp_is_address_equal_literal (cond))
+    return 0;
+
+  exp_address = exp->elts[3].longconst;
+  cond_address = cond->elts[4].longconst;
+
+  /* Make sure that the two addresses are the same.  */
+  if (exp_address != cond_address)
+    return 0;
+
+  pc = 6;
+  *data_value = value_as_long (evaluate_subexp (NULL_TYPE, cond, &pc,
+						EVAL_NORMAL));
+
+  return 1;
+}
+
 /* Create a vector of all tracepoints.  */
 
 VEC(breakpoint_p) *
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 6b373a3..1864a84 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -998,4 +998,20 @@ extern struct breakpoint *get_tracepoint_by_number (char **arg, int multi_p,
    is newly allocated; the caller should free when done with it.  */
 extern VEC(breakpoint_p) *all_tracepoints (void);
 
+extern int watch_address_if_address_equal_literal (struct expression *exp,
+						     struct expression *cond,
+						     CORE_ADDR *data_value);
+
+extern int watch_var_if_var_equal_literal (struct expression *exp,
+					     struct expression *cond,
+					     CORE_ADDR *data_value);
+
+extern int watch_address_if_var_equal_literal (struct expression *exp,
+						 struct expression *cond,
+						 CORE_ADDR *data_value);
+
+extern int watch_var_if_address_equal_literal (struct expression *exp,
+					     struct expression *cond,
+					     CORE_ADDR *data_value);
+
 #endif /* !defined (BREAKPOINT_H) */
diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index fa0cce6..1062b74 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -483,7 +483,8 @@ Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n"),
    of the type TYPE.  Return 0 on success, -1 on failure.  */
 
 static int
-i386_insert_watchpoint (CORE_ADDR addr, int len, int type)
+i386_insert_watchpoint (CORE_ADDR addr, int len, int type,
+			struct expression *exp, struct expression *cond)
 {
   int retval;
 
@@ -507,7 +508,8 @@ i386_insert_watchpoint (CORE_ADDR addr, int len, int type)
    address ADDR, whose length is LEN bytes, and for accesses of the
    type TYPE.  Return 0 on success, -1 on failure.  */
 static int
-i386_remove_watchpoint (CORE_ADDR addr, int len, int type)
+i386_remove_watchpoint (CORE_ADDR addr, int len, int type,
+			struct expression *exp, struct expression *cond)
 {
   int retval;
 
diff --git a/gdb/ia64-linux-nat.c b/gdb/ia64-linux-nat.c
index e6a7077..f68f77a 100644
--- a/gdb/ia64-linux-nat.c
+++ b/gdb/ia64-linux-nat.c
@@ -530,7 +530,8 @@ is_power_of_2 (int val)
 }
 
 static int
-ia64_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+ia64_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw,
+			      struct expression *exp, struct expression *cond)
 {
   struct lwp_info *lp;
   ptid_t ptid;
@@ -584,7 +585,8 @@ ia64_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
 }
 
 static int
-ia64_linux_remove_watchpoint (CORE_ADDR addr, int len, int type)
+ia64_linux_remove_watchpoint (CORE_ADDR addr, int len, int type,
+			      struct expression *exp, struct expression *cond)
 {
   int idx;
   long dbr_addr, dbr_mask;
diff --git a/gdb/inf-ttrace.c b/gdb/inf-ttrace.c
index c9ab548..dab9065 100644
--- a/gdb/inf-ttrace.c
+++ b/gdb/inf-ttrace.c
@@ -314,7 +314,8 @@ inf_ttrace_disable_page_protections (pid_t pid)
    type TYPE.  */
 
 static int
-inf_ttrace_insert_watchpoint (CORE_ADDR addr, int len, int type)
+inf_ttrace_insert_watchpoint (CORE_ADDR addr, int len, int type,
+			      struct expression *exp, struct expression *cond)
 {
   const int pagesize = inf_ttrace_page_dict.pagesize;
   pid_t pid = ptid_get_pid (inferior_ptid);
@@ -337,7 +338,8 @@ inf_ttrace_insert_watchpoint (CORE_ADDR addr, int len, int type)
    type TYPE.  */
 
 static int
-inf_ttrace_remove_watchpoint (CORE_ADDR addr, int len, int type)
+inf_ttrace_remove_watchpoint (CORE_ADDR addr, int len, int type,
+			      struct expression *exp, struct expression *cond)
 {
   const int pagesize = inf_ttrace_page_dict.pagesize;
   pid_t pid = ptid_get_pid (inferior_ptid);
diff --git a/gdb/mips-linux-nat.c b/gdb/mips-linux-nat.c
index fe05192..9d36a39 100644
--- a/gdb/mips-linux-nat.c
+++ b/gdb/mips-linux-nat.c
@@ -926,7 +926,8 @@ populate_regs_from_watches (struct pt_watch_regs *regs)
    watch.  Return zero on success.  */
 
 static int
-mips_linux_insert_watchpoint (CORE_ADDR addr, int len, int type)
+mips_linux_insert_watchpoint (CORE_ADDR addr, int len, int type,
+			      struct expression *exp, struct expression *cond)
 {
   struct pt_watch_regs regs;
   struct mips_watchpoint *new_watch;
@@ -975,7 +976,8 @@ mips_linux_insert_watchpoint (CORE_ADDR addr, int len, int type)
    Return zero on success.  */
 
 static int
-mips_linux_remove_watchpoint (CORE_ADDR addr, int len, int type)
+mips_linux_remove_watchpoint (CORE_ADDR addr, int len, int type,
+			      struct expression *exp, struct expression *cond)
 {
   int retval;
   int deleted_one;
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index d8f3c91..d37f259 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -1509,13 +1509,15 @@ procfs_can_use_hw_breakpoint (int type, int cnt, int othertype)
 }
 
 static int
-procfs_remove_hw_watchpoint (CORE_ADDR addr, int len, int type)
+procfs_remove_hw_watchpoint (CORE_ADDR addr, int len, int type,
+			     struct expression *exp, struct expression *cond)
 {
   return procfs_hw_watchpoint (addr, -1, type);
 }
 
 static int
-procfs_insert_hw_watchpoint (CORE_ADDR addr, int len, int type)
+procfs_insert_hw_watchpoint (CORE_ADDR addr, int len, int type,
+			     struct expression *exp, struct expression *cond)
 {
   return procfs_hw_watchpoint (addr, len, type);
 }
diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 9c76387..56c5aa7 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -1517,6 +1517,36 @@ booke_find_thread_points_by_tid (int tid, int alloc_new)
   return t;
 }
 
+/* Check whether we have at least one free DVC register.  */
+static int
+ppc_linux_can_use_watchpoint_cond_accel (void)
+{
+  struct thread_points *p;
+  int tid = TIDGET (inferior_ptid);
+  int cnt = booke_debug_info.num_condition_regs, i;
+  CORE_ADDR tmp_value;
+
+  if (!have_ptrace_new_debug_booke)
+    return 0;
+
+  p = booke_find_thread_points_by_tid (tid, 0);
+
+  if (p)
+    {
+      for (i = 0; i < max_slots_number; i++)
+	if (p->hw_breaks[i].hw_break != NULL
+	    && (p->hw_breaks[i].hw_break->condition_mode
+		!= PPC_BREAKPOINT_CONDITION_NONE))
+	  cnt--;
+
+      /* There are no available slots now.  */
+      if (cnt <= 0)
+	return 0;
+    }
+
+  return 1;
+}
+
 /* This function is a generic wrapper that is responsible for inserting a
    *point (i.e., calling `ptrace' in order to issue the request to the
    kernel) and registering it internally in GDB.  */
@@ -1658,7 +1688,8 @@ get_trigger_type (int rw)
 }
 
 static int
-ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw,
+			     struct expression *exp, struct expression *cond)
 {
   struct lwp_info *lp;
   ptid_t ptid;
@@ -1666,15 +1697,33 @@ ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
 
   if (have_ptrace_new_debug_booke)
     {
+      int byte_to_enable;
       struct ppc_hw_breakpoint p;
+      CORE_ADDR cond_addr;
+
+      if (cond && ppc_linux_can_use_watchpoint_cond_accel ()
+	  && (watch_address_if_address_equal_literal (exp, cond, &cond_addr)
+	      || watch_var_if_var_equal_literal (exp, cond, &cond_addr)
+	      || watch_var_if_address_equal_literal (exp, cond, &cond_addr)
+	      || watch_address_if_var_equal_literal (exp, cond, &cond_addr)))
+	{
+	  byte_to_enable = addr % 4;
+	  cond_addr >>= (byte_to_enable * 8);
+	  p.condition_mode  = (PPC_BREAKPOINT_CONDITION_AND
+			       | PPC_BREAKPOINT_CONDITION_BE (byte_to_enable));
+	  p.condition_value = (uint64_t) cond_addr;
+	}
+      else
+	{
+	  p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+	  p.condition_value = 0;
+	}
 
       p.version         = PPC_DEBUG_CURRENT_VERSION;
       p.trigger_type    = get_trigger_type (rw);
       p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
-      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
       p.addr            = (uint64_t) addr;
       p.addr2           = 0;
-      p.condition_value = 0;
 
       ALL_LWPS (lp, ptid)
 	booke_insert_point (&p, TIDGET (ptid));
@@ -1721,7 +1770,8 @@ ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
       saved_dabr_value = dabr_value;
 
       ALL_LWPS (lp, ptid)
-	if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
+	if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0,
+		    saved_dabr_value) < 0)
 	  return -1;
 
       ret = 0;
@@ -1731,7 +1781,8 @@ ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
 }
 
 static int
-ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw)
+ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw,
+			     struct expression *exp, struct expression *cond)
 {
   struct lwp_info *lp;
   ptid_t ptid;
@@ -1739,15 +1790,33 @@ ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw)
 
   if (have_ptrace_new_debug_booke)
     {
+      int byte_to_enable;
       struct ppc_hw_breakpoint p;
+      CORE_ADDR cond_addr;
+
+      if (cond && ppc_linux_can_use_watchpoint_cond_accel ()
+	  && (watch_address_if_address_equal_literal (exp, cond, &cond_addr)
+	      || watch_var_if_var_equal_literal (exp, cond, &cond_addr)
+	      || watch_var_if_address_equal_literal (exp, cond, &cond_addr)
+	      || watch_address_if_var_equal_literal (exp, cond, &cond_addr)))
+	{
+	  byte_to_enable = addr % 4;
+	  cond_addr >>= (byte_to_enable * 8);
+	  p.condition_mode  = (PPC_BREAKPOINT_CONDITION_AND
+			       | PPC_BREAKPOINT_CONDITION_BE (byte_to_enable));
+	  p.condition_value = (uint64_t) cond_addr;
+	}
+      else
+	{
+	  p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+	  p.condition_value = 0;
+	}
 
       p.version         = PPC_DEBUG_CURRENT_VERSION;
       p.trigger_type    = get_trigger_type (rw);
       p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
-      p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
       p.addr            = (uint64_t) addr;
       p.addr2           = 0;
-      p.condition_value = 0;
 
       ALL_LWPS (lp, ptid)
 	booke_remove_point (&p, TIDGET (ptid));
@@ -1758,7 +1827,8 @@ ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw)
     {
       saved_dabr_value = 0;
       ALL_LWPS (lp, ptid)
-	if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0)
+	if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0,
+		    saved_dabr_value) < 0)
 	  return -1;
 
       ret = 0;
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 1dd39b2..f373511 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -5502,7 +5502,8 @@ procfs_stopped_by_watchpoint (void)
 }
 
 static int
-procfs_insert_watchpoint (CORE_ADDR addr, int len, int type)
+procfs_insert_watchpoint (CORE_ADDR addr, int len, int type,
+			  struct expression *exp, struct expression *cond)
 {
   if (!target_have_steppable_watchpoint
       && !gdbarch_have_nonsteppable_watchpoint (target_gdbarch))
@@ -5523,7 +5524,8 @@ procfs_insert_watchpoint (CORE_ADDR addr, int len, int type)
 }
 
 static int
-procfs_remove_watchpoint (CORE_ADDR addr, int len, int type)
+procfs_remove_watchpoint (CORE_ADDR addr, int len, int type,
+			  struct expression *exp, struct expression *cond)
 {
   return procfs_set_watchpoint (inferior_ptid, addr, 0, 0, 0);
 }
diff --git a/gdb/remote-m32r-sdi.c b/gdb/remote-m32r-sdi.c
index be6a564..675bf15 100644
--- a/gdb/remote-m32r-sdi.c
+++ b/gdb/remote-m32r-sdi.c
@@ -1415,7 +1415,8 @@ m32r_can_use_hw_watchpoint (int type, int cnt, int othertype)
    watchpoint. */
 
 static int
-m32r_insert_watchpoint (CORE_ADDR addr, int len, int type)
+m32r_insert_watchpoint (CORE_ADDR addr, int len, int type,
+			struct expression *exp, struct expression *cond)
 {
   int i;
 
@@ -1439,7 +1440,8 @@ m32r_insert_watchpoint (CORE_ADDR addr, int len, int type)
 }
 
 static int
-m32r_remove_watchpoint (CORE_ADDR addr, int len, int type)
+m32r_remove_watchpoint (CORE_ADDR addr, int len, int type,
+			struct expression *exp, struct expression *cond)
 {
   int i;
 
diff --git a/gdb/remote-mips.c b/gdb/remote-mips.c
index f2fb8f3..cc4d9f8 100644
--- a/gdb/remote-mips.c
+++ b/gdb/remote-mips.c
@@ -2296,7 +2296,8 @@ calculate_mask (CORE_ADDR addr, int len)
    watchpoint. */
 
 int
-mips_insert_watchpoint (CORE_ADDR addr, int len, int type)
+mips_insert_watchpoint (CORE_ADDR addr, int len, int type,
+			struct expression *exp, struct expression *cond)
 {
   if (mips_set_breakpoint (addr, len, type))
     return -1;
@@ -2305,7 +2306,8 @@ mips_insert_watchpoint (CORE_ADDR addr, int len, int type)
 }
 
 int
-mips_remove_watchpoint (CORE_ADDR addr, int len, int type)
+mips_remove_watchpoint (CORE_ADDR addr, int len, int type,
+			struct expression *exp, struct expression *cond)
 {
   if (mips_clear_breakpoint (addr, len, type))
     return -1;
diff --git a/gdb/remote.c b/gdb/remote.c
index 709e424..c8a8906 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -7339,7 +7339,8 @@ watchpoint_to_Z_packet (int type)
 }
 
 static int
-remote_insert_watchpoint (CORE_ADDR addr, int len, int type)
+remote_insert_watchpoint (CORE_ADDR addr, int len, int type,
+			  struct expression *watch_exp, struct expression *cond)
 {
   struct remote_state *rs = get_remote_state ();
   char *p;
@@ -7371,7 +7372,8 @@ remote_insert_watchpoint (CORE_ADDR addr, int len, int type)
 
 
 static int
-remote_remove_watchpoint (CORE_ADDR addr, int len, int type)
+remote_remove_watchpoint (CORE_ADDR addr, int len, int type,
+			  struct expression *watch_exp, struct expression *cond)
 {
   struct remote_state *rs = get_remote_state ();
   char *p;
diff --git a/gdb/s390-nat.c b/gdb/s390-nat.c
index 3af42ff..41ebf9b 100644
--- a/gdb/s390-nat.c
+++ b/gdb/s390-nat.c
@@ -335,7 +335,8 @@ s390_fix_watch_points (ptid_t ptid)
 }
 
 static int
-s390_insert_watchpoint (CORE_ADDR addr, int len, int type)
+s390_insert_watchpoint (CORE_ADDR addr, int len, int type,
+			struct expression *exp, struct expression *cond)
 {
   struct lwp_info *lp;
   ptid_t ptid;
@@ -356,7 +357,8 @@ s390_insert_watchpoint (CORE_ADDR addr, int len, int type)
 }
 
 static int
-s390_remove_watchpoint (CORE_ADDR addr, int len, int type)
+s390_remove_watchpoint (CORE_ADDR addr, int len, int type,
+			struct expression *exp, struct expression *cond)
 {
   struct lwp_info *lp;
   ptid_t ptid;
diff --git a/gdb/target.c b/gdb/target.c
index e6659c9..95ab2fc 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -116,9 +116,13 @@ static int debug_to_insert_hw_breakpoint (struct gdbarch *,
 static int debug_to_remove_hw_breakpoint (struct gdbarch *,
 					  struct bp_target_info *);
 
-static int debug_to_insert_watchpoint (CORE_ADDR, int, int);
+static int debug_to_insert_watchpoint (CORE_ADDR, int, int,
+				       struct expression *,
+				       struct expression *);
 
-static int debug_to_remove_watchpoint (CORE_ADDR, int, int);
+static int debug_to_remove_watchpoint (CORE_ADDR, int, int,
+				       struct expression *,
+				       struct expression *);
 
 static int debug_to_stopped_by_watchpoint (void);
 
@@ -705,10 +709,12 @@ update_current_target (void)
 	    (int (*) (struct gdbarch *, struct bp_target_info *))
 	    return_minus_one);
   de_fault (to_insert_watchpoint,
-	    (int (*) (CORE_ADDR, int, int))
+	    (int (*) (CORE_ADDR, int, int, struct expression *,
+		      struct expression *))
 	    return_minus_one);
   de_fault (to_remove_watchpoint,
-	    (int (*) (CORE_ADDR, int, int))
+	    (int (*) (CORE_ADDR, int, int, struct expression *,
+		      struct expression *))
 	    return_minus_one);
   de_fault (to_stopped_by_watchpoint,
 	    (int (*) (void))
@@ -3252,11 +3258,13 @@ debug_to_remove_hw_breakpoint (struct gdbarch *gdbarch,
 }
 
 static int
-debug_to_insert_watchpoint (CORE_ADDR addr, int len, int type)
+debug_to_insert_watchpoint (CORE_ADDR addr, int len, int type,
+			    struct expression *watch_exp,
+			    struct expression *cond)
 {
   int retval;
 
-  retval = debug_target.to_insert_watchpoint (addr, len, type);
+  retval = debug_target.to_insert_watchpoint (addr, len, type, watch_exp, cond);
 
   fprintf_unfiltered (gdb_stdlog,
 		      "target_insert_watchpoint (0x%lx, %d, %d) = %ld\n",
@@ -3265,11 +3273,13 @@ debug_to_insert_watchpoint (CORE_ADDR addr, int len, int type)
 }
 
 static int
-debug_to_remove_watchpoint (CORE_ADDR addr, int len, int type)
+debug_to_remove_watchpoint (CORE_ADDR addr, int len, int type,
+			    struct expression *watch_exp,
+			    struct expression *cond)
 {
   int retval;
 
-  retval = debug_target.to_remove_watchpoint (addr, len, type);
+  retval = debug_target.to_remove_watchpoint (addr, len, type, watch_exp, cond);
 
   fprintf_unfiltered (gdb_stdlog,
 		      "target_remove_watchpoint (0x%lx, %d, %d) = %ld\n",
diff --git a/gdb/target.h b/gdb/target.h
index 7103ab2..4f747c5 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -36,6 +36,9 @@ struct trace_status;
 struct uploaded_tsv;
 struct uploaded_tp;
 
+struct breakpoint;
+struct expression;
+
 /* This include file defines the interface between the main part
    of the debugger, and the part which is target-specific, or
    specific to the communications interface between us and the
@@ -420,8 +423,14 @@ struct target_ops
     int (*to_can_use_hw_breakpoint) (int, int, int);
     int (*to_insert_hw_breakpoint) (struct gdbarch *, struct bp_target_info *);
     int (*to_remove_hw_breakpoint) (struct gdbarch *, struct bp_target_info *);
-    int (*to_remove_watchpoint) (CORE_ADDR, int, int);
-    int (*to_insert_watchpoint) (CORE_ADDR, int, int);
+
+    /* Documentation of what the two routines below are expected to do is
+       provided with the corresponding target_* macros.  */
+    int (*to_remove_watchpoint) (CORE_ADDR, int, int, struct expression *,
+						      struct expression *);
+    int (*to_insert_watchpoint) (CORE_ADDR, int, int, struct expression *,
+						      struct expression *);
+
     int (*to_stopped_by_watchpoint) (void);
     int to_have_steppable_watchpoint;
     int to_have_continuable_watchpoint;
@@ -1263,16 +1272,16 @@ extern char *normal_pid_to_str (ptid_t ptid);
 #define target_region_ok_for_hw_watchpoint(addr, len) \
     (*current_target.to_region_ok_for_hw_watchpoint) (addr, len)
 
-
 /* Set/clear a hardware watchpoint starting at ADDR, for LEN bytes.  TYPE is 0
-   for write, 1 for read, and 2 for read/write accesses.  Returns 0 for
-   success, non-zero for failure.  */
+   for write, 1 for read, and 2 for read/write accesses.  WATCH_EXP is the
+   watchpoint's expression and COND is the expression for its condition,
+   or NULL if there's none.  Returns 0 for success, non-zero for failure.  */
 
-#define	target_insert_watchpoint(addr, len, type)	\
-     (*current_target.to_insert_watchpoint) (addr, len, type)
+#define	target_insert_watchpoint(addr, len, type, watch_exp, cond) \
+     (*current_target.to_insert_watchpoint) (addr, len, type, watch_exp, cond)
 
-#define	target_remove_watchpoint(addr, len, type)	\
-     (*current_target.to_remove_watchpoint) (addr, len, type)
+#define	target_remove_watchpoint(addr, len, type, watch_exp, cond) \
+     (*current_target.to_remove_watchpoint) (addr, len, type, watch_exp, cond)
 
 #define target_insert_hw_breakpoint(gdbarch, bp_tgt) \
      (*current_target.to_insert_hw_breakpoint) (gdbarch, bp_tgt)

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