This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/4] Hardware accelerated watchpoint conditions
- From: Thiago Jung Bauermann <bauerman at br dot ibm dot com>
- To: Joel Brobecker <brobecker at adacore dot com>
- Cc: gdb-patches at sourceware dot org, Luis Machado <luisgpm at linux dot vnet dot ibm dot com>, Matt Tyrlik <tyrlik at us dot ibm dot com>
- Date: Thu, 4 Feb 2010 19:48:33 -0200
- Subject: Re: [PATCH 2/4] Hardware accelerated watchpoint conditions
- References: <200912232231.01798.bauerman@br.ibm.com> <200912311519.12125.bauerman@br.ibm.com> <20100112105045.GK2007@adacore.com>
On Tue 12 Jan 2010 08:50:45 Joel Brobecker wrote:
> Sorry for the delay looking into this; I needed a little bit of time
> to let this brew a little...
On my part, sorry for the delay in taking this long to rework the patch
and send it back. I was on vacation...
Thanks for the in-depth review!
> 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.
> Conceptually, I think that the target now needs to be able to look
> at the watchpoint expression, and the watchpoint condition, in order
> to determine whether the condition evaluation can be evaluated
> by hardware. I propose that we expand the insert_watchpoint routine
> to pass this data, as 2 extra parameters; and let these routines
> decide whether to use hardware acceleration or not for the condition.
I liked the idea. This patches implements that approach.
> One of the downsides of this approach is that GDB will no longer
> be able to print that the condition will be hardware-accelerated
> when the user creates the watchpoint. I think that's OK for now.
Yeah, I would like to have that message printed though. Wil have
to think about it.
> I am also a bit suspicious of adding the following bits to the bp_location
>
> structure directly:
> > + /* Flag to indicate if the condition is going to be accelerated
> > + by hardware. If its value is non-zero, then GDB checks the
> > + condition using hardware acceleration; otherwise it uses the
> > + regular software-based checking. */
> > + int cond_hw_accel : 1;
> > +
> > + /* If the condition can be hardware-accelerated, then we must
> > + get the condition's variable address so that GDB can
> > + properly set the evaluation via hardware. */
> > + CORE_ADDR cond_hw_addr;
>
> I think that they are too target specific. Initially, we can do without,
> with our not-so-efficient approach, by recomputing these address every
> time we insert the watchpoint. I think that'd still be an interesting
> first step. Then we can look at storing that info as a private data
> structure, allocated by the target. I think that this would be doable
> without too many changes to the current framework - we just need to
> be a little careful as watchpoint expressions and conditions get
> re-evaluated sometimes.
Those are gone. I don't think recomputing the address everytime
GDB stops will get in our way at this point. Parsing the condition
expression should be a quick enough operation I think.
> > +static int
> > +exp_is_address_p (struct breakpoint *b)
>
> If you want, you can drop the _p prefix in this type of situation.
> I think the use of the verb _is_ in the names makes it obvious that
> this function is a predicate.
I didn't like those _p suffixes much either. They're all gone.
> > +int
> > +default_watch_address_if_var_equal_literal_p (struct bp_location *b,
> > + CORE_ADDR *data_value)
>
> This is nitpicking on function names, but I don't understand the
> "default" here. Would there be some non-default implementations
> of the same routine? Perhaps: watch_address_if_var_equal_literal_p
> is just going to be sufficient?
Not sure either. I removed the default_ prefix from those function names.
> > + /* Make sure that the two addresses are the same. */
> > + if (exp_address != cond_address)
> > + {
> > + printf_filtered (_("\
> > +Memory location for the watchpoint expression and its condition need\n\
> > +to be the same.\n"));
> > + return 0;
> > + }
>
> Not sure whether you really meant to keep this message. It sounds like
> an error, when in fact this could occur in a perfectly legitimate
> situation, no? For instance, if the user entered:
>
> (gdb) watch A if B == 3
>
> The message could be kept as a debug trace, but it should be more
> neutral, IMO, something like:
>
> address of variable in condition does not match expression being
> watched.
>
> would provide all the information without suggesting that something
> might have gone wrong.
I agree. In this version GDB silently uses software condition checking.
But there should be a way to let the user know that the watchpoint
he created is having hardware-accelerated condition checking or not
(hence my comment up there that I'd like some message to be shown
when the watchpoint is created).
> Let me know what you think. I realize that I'm significantly reducing
> the scope of the first attempt at getting h/w-accelerated watchpoint
> conditions, but I'm hoping it'll help focusing on each issue separately.
> I'm perfectly happy to make this feature another patch series on its own,
> in parallel to the other hardware-breakpoint patches. We can also take it
> one patch at a time. Whatever works best for you.
Mmm.... perhaps I'm too thick today, but I don't see how you are
significantly reducing the scope here, except for the hardware-accelerated
message not being shown at watchpoint creation time.
> BTW:
> > @@ -186,6 +186,11 @@ struct bp_target_info
> > is used to determine the type of breakpoint to insert. */
> > CORE_ADDR placed_address;
> >
> > + /* If this is a ranged hardware breakpoint, then we can use this
> > + field in order to store the length of the range that will be
> > + watched for execution. */
> > + ULONGEST length;
> > +
>
> Oops! This should be part of another patch ;-).
Ooops. Gone.
--
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center
2010-02-04 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 cfa17ed01a406be5668f81c0374f34256eb61ec2
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..13013ca 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,218 @@ 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>
+
+ If it is, 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;
+
+ /* At this point, all verifications were positive, so we can use
+ hardware-assisted data-matching. Set the data value, and return
+ non-zero. */
+ 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>
+
+ If it is, 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;
+
+ /* At this point, all verifications were positive, so we can use
+ hardware-assisted data-matching. Set the data value, and return
+ non-zero. */
+ 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 <VAR> if <VAR> == <LITERAL>
+
+ If it is, 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;
+
+ /* At this point, all verifications were positive, so we can use
+ hardware-assisted data-matching. Set the data value, and return
+ non-zero. */
+ 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 *<ADDRESS> if *<ADDRESS> == <LITERAL>
+
+ If it is, 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;
+
+ /* At this point, all verifications were positive, so we can use
+ hardware-assisted data-matching. Set the data value, and return
+ non-zero. */
+ 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..23d49c4 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -998,4 +998,24 @@ 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);
+/* 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'. */
+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..83f0c3f 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -1517,6 +1517,35 @@ booke_find_thread_points_by_tid (int tid, int alloc_new)
return t;
}
+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 +1687,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 +1696,30 @@ 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 +1766,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 +1777,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 +1786,30 @@ 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 +1820,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..7adc96e 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -44,6 +44,8 @@
#include "exec.h"
#include "inline-frame.h"
+struct expression;
+
static void target_info (char *, int);
static void default_terminal_info (char *, int);
@@ -116,9 +118,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 +711,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 +3260,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 +3275,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..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;
+
/* 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 +424,10 @@ 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);
+ 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;
@@ -1260,19 +1266,20 @@ extern char *normal_pid_to_str (ptid_t ptid);
#define target_can_use_hardware_watchpoint(TYPE,CNT,OTHERTYPE) \
(*current_target.to_can_use_hw_breakpoint) (TYPE, CNT, OTHERTYPE);
+#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
/* 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)
-#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)