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: Ulrich Weigand <uweigand at de dot ibm dot com>
- Cc: Joel Brobecker <brobecker at adacore dot com>, 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: Wed, 23 Jun 2010 14:20:46 -0300
- Subject: Re: [PATCH 2/4] Hardware accelerated watchpoint conditions
- References: <201006091327.o59DRmsb032026@d12av02.megacenter.de.ibm.com>
On Wed, 2010-06-09 at 15:27 +0200, Ulrich Weigand wrote:
> Thiago Jung Bauermann wrote:
> Unfortunately, I think this doesn't quite work: the problem is not that
> accesses have been evaluated (that should be fine as you said), but rather
> the problem is that you do not guarantee that the comparison value is
> *constant*. Now you do have the check against not_lval, which will
> indeed exclude simple non-constant values like direct register or
> memory references. However, if the right side is a *complex* but
> non-constant expression, you'll also see non_lval and accept it.
>
> For example, I think your code would currently (erroneously) claim to
> be able to use hardware assists to watch something like:
>
> watch x if x == y + z
>
> with global variables x, y, z.
>
> The prior attempt got this correct (but sub-optimal) by disallowing
> any complex expression. What we really want is to allow any *constant*
> expresssion, i.e. any expression whose value does not depend on the
> contents of registers or memory.
Indeed. I didn't realised that. I changed the patch to use the approach
found in can_use_hardware_watchpoint: evaluate the expression and then
go through the value chain generated by the evaluation to check whether
there's any memory or register access. What do you think?
If this doesn't work then I'll go back to the simple approach of the
first patch. :-)
> > One important thing about this patch is that I had to add a new target
> > method: target_can_accel_watchpoint_condition. It is called in
> > watchpoint_locations_match to avoid making GDB insert only one of two
> > bp_locations which are at the same address but with different
> > conditions. When the target supports evaluating the watchpoint
> > expression in hardware, GDB needs to insert both hardware watchpoints
> > even if they are at the same address, so that their different conditions
> > have a chance to trigger.
>
> Hmm, I'm wondering -- it seems a bit odd that there is no feedback
> from target_insert_watchpoint itself whether the condition could be
> honored or not -- it's just silently ignored, and you have to consider
> this extra target hook. Maybe a more straightforward interface would
> be to have target_insert_watchpoint fail if called with a condition
> it cannot implement -- common code could then retry with a NULL
> condition, and reset the breakpoint type to remember that for this
> watchpoint, software condition checks need to be applied ...
We need to determine whether the condition can be implemented in
hardware before target_insert_watchpoint is called because
update_global_location_list will flag duplicate watchpoint locations and
those won't be inserted at all...
> (But this is really a separate issue from the main purpose of this
> patch; please do not consider this as a matter that would block
> acceptance of the patch as is, as far as I'm concerned.)
Thanks! I'm in favor of avoiding adding unnecessary flags and hooks, but
in this case I think it's necessary.
> > +/* Calculate the enable bits and the contents of the Data Value Compare
> > + debug register present in BookE processors.
> > +
> > + ADDR is the address to be watched, LEN is the length of watched data
> > + and DATA_VALUE is the value which will trigger the watchpoint. On exit,
> > + CONDITION_MODE will hold the enable bits for the DVC, and CONDITION_VALUE
> > + will hold the value which should be put in the DVC register. */
>
> DATA_LEN is not described. I'm wondering why two lengths are needed here?
I described it now. We need two lengths because the watched value and
the constant in the condition may have different types. For instance, if
you have a "short int foo" and your condition is "foo == 2", then the
watched variable is of type short int and the condition constant is an
int.
> > + *data_value = 0;
> > + *data_value_len = TYPE_LENGTH (value_type (val));
> > + copy_len = *data_value_len > sizeof (CORE_ADDR)?
> > + sizeof (CORE_ADDR) : *data_value_len;
> > + memcpy (data_value, value_contents (val), copy_len);
>
> There seem to be byte order assumptions here. In theory, in a multi-arch
> setup, the byte order of VAL might differ from the native byte order of
> a CORE_ADDR ... Can this not be done via value_as_long, possibly
> followed by shifts?
I didn't realise that. This version follows your suggestion.
Thanks and regards,
Thiago Jung Bauermann
IBM Linux Technology Center
2010-06-23 Sergio Durigan Junior <sergiodj@linux.vnet.ibm.com>
Thiago Jung Bauermann <bauerman@br.ibm.com>
* breakpoint.c (insert_bp_location): Pass watchpoint condition in
target_insert_watchpoint.
(remove_breakpoint_1) Pass watchpoint condition in
target_remove_watchpoint.
(watchpoint_locations_match): Call
target_can_accel_watchpoint_condition.
* ppc-linux-nat.c: Include exceptions.h and wrapper.h
(ppc_linux_region_ok_for_hw_watchpoint): Formatting fix.
(can_use_watchpoint_cond_accel): New function.
(calculate_dvc): Likewise.
(num_memory_accesses): Likewise.
(fetch_subexp_value): Likewise.
(free_value_chain): Likewise.
(check_condition): Likewise.
(ppc_linux_can_accel_watchpoint_condition): Likewise
(ppc_linux_insert_watchpoint): Call can_use_watchpoint_cond_accel,
check_condition and calculate_dvc.
(ppc_linux_remove_watchpoint): Likewise.
(_initialize_ppc_linux_nat): Set to_can_accel_watchpoint_condition to
ppc_linux_can_accel_watchpoint_condition
* target.c (debug_to_insert_watchpoint): Add argument for watchpoint
condition.
(debug_to_remove_watchpoint): Likewise.
(debug_to_can_accel_watchpoint_condition): New function.
(update_current_target): Set to_can_accel_watchpoint_condition.
(setup_target_debug): Set to_can_accel_watchpoint_condition.
* target.h: Add opaque declarations for struct expression.
(struct target_ops) <to_insert_watchpoint>,
<to_remove_watchpoint>: Add new arguments to pass the watchpoint
<to_can_accel_watchpoint_condition>: New member.
condition. Update all callers and implementations.
(target_can_accel_watchpoint_condition): New macro.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index f2b973a..9ee7ad9 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1796,7 +1796,8 @@ Note: automatically using hardware breakpoints for read-only addresses.\n"));
{
val = target_insert_watchpoint (bpt->address,
bpt->length,
- bpt->watchpoint_type);
+ bpt->watchpoint_type,
+ bpt->owner->cond_exp);
/* If trying to set a read-watchpoint, and it turns out it's not
supported, try emulating one with an access watchpoint. */
@@ -1824,7 +1825,8 @@ Note: automatically using hardware breakpoints for read-only addresses.\n"));
{
val = target_insert_watchpoint (bpt->address,
bpt->length,
- hw_access);
+ hw_access,
+ bpt->owner->cond_exp);
if (val == 0)
bpt->watchpoint_type = hw_access;
}
@@ -2493,8 +2495,8 @@ remove_breakpoint_1 (struct bp_location *b, insertion_state_t is)
else if (b->loc_type == bp_loc_hardware_watchpoint)
{
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->cond_exp);
/* Failure to remove any of the hardware watchpoints comes here. */
if ((is == mark_uninserted) && (b->inserted))
@@ -5262,6 +5264,21 @@ watchpoint_locations_match (struct bp_location *loc1, struct bp_location *loc2)
gdb_assert (loc1->owner != NULL);
gdb_assert (loc2->owner != NULL);
+ /* If the target can evaluate the condition expression in hardware, then we
+ we need to insert both watchpoints even if they are at the same place.
+ Otherwise the watchpoint will only trigger when the condition of whichever
+ watchpoint was inserted evaluates to true, not giving a chance for GDB to
+ check the condition of the other watchpoint. */
+ if ((loc1->owner->cond_exp
+ && target_can_accel_watchpoint_condition (loc1->address, loc1->length,
+ loc1->watchpoint_type,
+ loc1->owner->cond_exp))
+ || (loc2->owner->cond_exp
+ && target_can_accel_watchpoint_condition (loc2->address, loc2->length,
+ loc2->watchpoint_type,
+ loc2->owner->cond_exp)))
+ return 0;
+
/* Note that this checks the owner's type, not the location's. In
case the target does not support read watchpoints, but does
support access watchpoints, we'll have bp_read_watchpoint
diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index 82c51d7..eaa3644 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -484,7 +484,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 *cond)
{
int retval;
@@ -511,7 +512,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 *cond)
{
int retval;
diff --git a/gdb/ia64-linux-nat.c b/gdb/ia64-linux-nat.c
index e6a7077..d33e88e 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 *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 *cond)
{
int idx;
long dbr_addr, dbr_mask;
diff --git a/gdb/inf-ttrace.c b/gdb/inf-ttrace.c
index 199ceb9..aa0462f 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 *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 *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..e9e7c02 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 *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 *cond)
{
int retval;
int deleted_one;
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 2741ecf..a8cd1cd 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -1506,13 +1506,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 *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 *cond)
{
return procfs_hw_watchpoint (addr, len, type);
}
diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index f61ac5d..052ccc2 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -29,6 +29,8 @@
#include "gdb_assert.h"
#include "target.h"
#include "linux-nat.h"
+#include "exceptions.h"
+#include "wrapper.h"
#include <stdint.h>
#include <sys/types.h>
@@ -1492,7 +1494,7 @@ ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
if (booke_debug_info.data_bp_alignment
&& (addr + len > (addr & ~(booke_debug_info.data_bp_alignment - 1))
+ booke_debug_info.data_bp_alignment))
- return 0;
+ return 0;
}
/* addr+len must fall in the 8 byte watchable region for DABR-based
processors (i.e., server processors). Without the new BookE ptrace
@@ -1685,8 +1687,309 @@ get_trigger_type (int rw)
return t;
}
+/* Check whether we have at least one free DVC register. */
+static int
+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_booke_interface () || cnt == 0)
+ 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;
+}
+
+/* Calculate the enable bits and the contents of the Data Value Compare
+ debug register present in BookE processors.
+
+ ADDR is the address to be watched, LEN is the length of watched data
+ and DATA_VALUE is the value which will trigger the watchpoint.
+ DATA_VALUE_LEN is the length in bytes of DATA_VALUE, that is, the length
+ of the type of the value given by the user in the condition expression,
+ which may be different from the type of the watched value (e.g., watching
+ a short integer but with the condition given as an integer).
+ On exit, CONDITION_MODE will hold the enable bits for the DVC, and
+ CONDITION_VALUE will hold the value which should be put in the
+ DVC register. */
+static void
+calculate_dvc (CORE_ADDR addr, int len, CORE_ADDR data_value,
+ int data_value_len, uint32_t *condition_mode,
+ uint64_t *condition_value)
+{
+ int i, num_byte_enable, align_offset, num_bytes_off_dvc,
+ rightmost_enabled_byte;
+ CORE_ADDR addr_end_data, addr_end_dvc;
+
+ /* The DVC register compares bytes within fixed-length windows which
+ are word-aligned, with length equal to that of the DVC register.
+ We need to calculate where our watch region is relative to that
+ window and enable comparison of the bytes which fall within it. */
+
+ align_offset = addr % booke_debug_info.sizeof_condition;
+ addr_end_data = addr + len;
+ addr_end_dvc = (addr - align_offset
+ + booke_debug_info.sizeof_condition);
+ num_bytes_off_dvc = (addr_end_data > addr_end_dvc)?
+ addr_end_data - addr_end_dvc : 0;
+ num_byte_enable = len - num_bytes_off_dvc;
+ /* Here, bytes are numbered from right to left. */
+ rightmost_enabled_byte = (addr_end_data < addr_end_dvc)?
+ addr_end_dvc - addr_end_data : 0;
+
+ *condition_mode = PPC_BREAKPOINT_CONDITION_AND;
+ for (i = 0; i < num_byte_enable; i++)
+ *condition_mode |= PPC_BREAKPOINT_CONDITION_BE (i + rightmost_enabled_byte);
+
+ /* Now we need to match the position within the DVC of the comparison
+ value with where the watch region is relative to the window
+ (i.e., the ALIGN_OFFSET). */
+
+ /* The size of the user-provided data value matters because the value is
+ "left-aligned" within DATA_VALUE, e.g: a 1-byte data type will occupy the
+ first byte of DATA_VALUE, a two-byte data type the first two, and so on.
+ This means that we need to adjust the user-provided value within
+ DATA_VALUE when copying it to CONDITION_VALUE. */
+
+ if (data_value_len - len > align_offset)
+ /* The user-provided value type is larger than the watched value type,
+ and it is also to the right of the offset within the DVC register
+ where it should be. */
+ *condition_value = (uint64_t) data_value << (data_value_len - len
+ - align_offset) * 8;
+ else
+ /* The user-provided value type is either smaller than the watched
+ value type, or else it is equal or larger than the watched value
+ type but to the left of the offset within the DVC register where
+ it should be. */
+ *condition_value = (uint64_t) data_value >> (align_offset
+ - (data_value_len - len)) * 8;
+}
+
+/* Return the number of memory locations that need to be accessed to
+ evaluate the expression which generated the given value chain.
+ Returns -1 if there's any register access involved. */
+static int
+num_memory_accesses (struct value *v)
+{
+ int found_memory_cnt = 0;
+ struct value *head = v;
+
+ /* The idea here is that evaluating an expression generates a series
+ of values, one holding the value of every subexpression. (The
+ expression a*b+c has five subexpressions: a, b, a*b, c, and
+ a*b+c.) GDB's values hold almost enough information to establish
+ the criteria given above --- they identify memory lvalues,
+ register lvalues, computed values, etcetera. So we can evaluate
+ the expression, and then scan the chain of values that leaves
+ behind to determine the memory locations involved in the evaluation
+ of an expression.
+
+ However, I don't think that the values returned by inferior
+ function calls are special in any way. So this function may not
+ notice that an expression contains an inferior function call.
+ FIXME. */
+
+ for (; v; v = value_next (v))
+ {
+ if (VALUE_LVAL (v) == lval_memory)
+ {
+ /* A lazy memory lvalue is one that GDB never needed to fetch;
+ we either just used its address (e.g., `a' in `a.b') or
+ we never needed it at all (e.g., `a' in `a,b'). */
+ if (!value_lazy (v))
+ found_memory_cnt++;
+ }
+ else if (VALUE_LVAL (v) != not_lval
+ && deprecated_value_modifiable (v) == 0)
+ return -1; /* ??? What does this represent? */
+ else if (VALUE_LVAL (v) == lval_register)
+ return -1; /* cannot watch a register with a HW watchpoint */
+ }
+
+ return found_memory_cnt;
+}
+
+/* Find the current value of EXP. Return the value in *VALP and *RESULTP
+ and the chain of intermediate and final values in *VAL_CHAIN. RESULTP
+ and VAL_CHAIN may be NULL if the caller does not need them.
+
+ If a memory error occurs while evaluating the expression, *RESULTP will
+ be set to NULL. *RESULTP may be a lazy value, if the result could
+ not be read from memory. It is used to determine whether a value
+ is user-specified (we should watch the whole value) or intermediate
+ (we should watch only the bit used to locate the final value).
+
+ If the final value, or any intermediate value, could not be read
+ from memory, *VALP will be set to NULL. *VAL_CHAIN will still be
+ set to any referenced values. *VALP will never be a lazy value.
+
+ If VAL_CHAIN is non-NULL, *VAL_CHAIN will be released from the
+ value chain. The caller must free the values individually. If
+ VAL_CHAIN is NULL, all generated values will be left on the value
+ chain. */
+static void
+fetch_subexp_value (struct expression *exp, int *pc, struct value **valp,
+ struct value **resultp, struct value **val_chain)
+{
+ struct value *mark, *new_mark, *result;
+ volatile struct gdb_exception ex;
+
+ *valp = NULL;
+ if (resultp)
+ *resultp = NULL;
+ if (val_chain)
+ *val_chain = NULL;
+
+ /* Evaluate the expression. */
+ mark = value_mark ();
+ result = NULL;
+
+ TRY_CATCH (ex, RETURN_MASK_ALL)
+ {
+ result = evaluate_subexp (NULL_TYPE, exp, pc, EVAL_NORMAL);
+ }
+ if (ex.reason < 0)
+ {
+ /* Ignore memory errors, we want watchpoints pointing at
+ inaccessible memory to still be created; otherwise, throw the
+ error to some higher catcher. */
+ switch (ex.error)
+ {
+ case MEMORY_ERROR:
+ break;
+ default:
+ throw_exception (ex);
+ break;
+ }
+ }
+
+ new_mark = value_mark ();
+ if (mark == new_mark)
+ return;
+ if (resultp)
+ *resultp = result;
+
+ if (result != NULL
+ && (!value_lazy (result) || gdb_value_fetch_lazy (result)))
+ *valp = result;
+
+ if (val_chain)
+ {
+ /* Return the chain of intermediate values. */
+ *val_chain = new_mark;
+ value_release_to_mark (mark);
+ }
+}
+
+/* Frees all the elements in a chain of values. */
+static void
+free_value_chain (struct value *v)
+{
+ struct value *next;
+
+ for (; v; v = next)
+ {
+ next = value_next (v);
+ value_free (v);
+ }
+}
+
+/* Verifies whether the expression COND can be implemented using the
+ DVC (Data Value Compare) register in BookE processors. The expression
+ must test the watch value for equality with a constant expression. */
static int
-ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+check_condition (CORE_ADDR watch_addr, struct expression *cond,
+ CORE_ADDR *data_value, int *data_value_len)
+{
+ int pc = 1, num_accesses_left, num_accesses_right;
+ struct value *val, *left_val, *right_val, *left_chain, *right_chain;
+
+ if (cond->elts[0].opcode != BINOP_EQUAL)
+ return 0;
+
+ fetch_subexp_value (cond, &pc, &left_val, NULL, &left_chain);
+ num_accesses_left = num_memory_accesses (left_chain);
+
+ if (left_val == NULL || num_accesses_left < 0)
+ {
+ free_value_chain (left_chain);
+
+ return 0;
+ }
+
+ fetch_subexp_value (cond, &pc, &right_val, NULL, &right_chain);
+ num_accesses_right = num_memory_accesses (right_chain);
+
+ if (right_val == NULL || num_accesses_right < 0)
+ {
+ free_value_chain (left_chain);
+ free_value_chain (right_chain);
+
+ return 0;
+ }
+
+ if (num_accesses_left == 1 && num_accesses_right == 0
+ && VALUE_LVAL (left_val) == lval_memory
+ && value_address (left_val) == watch_addr)
+ val = right_val;
+ else if (num_accesses_left == 0 && num_accesses_right == 1
+ && VALUE_LVAL (right_val) == lval_memory
+ && value_address (right_val) == watch_addr)
+ val = left_val;
+ else
+ {
+ free_value_chain (left_chain);
+ free_value_chain (right_chain);
+
+ return 0;
+ }
+
+ *data_value = value_as_long (val);
+ *data_value_len = TYPE_LENGTH (value_type (val));
+ *data_value = *data_value << (sizeof (CORE_ADDR) - *data_value_len) * 8;
+
+ free_value_chain (left_chain);
+ free_value_chain (right_chain);
+
+ return 1;
+}
+
+/* Return non-zero if the target is capable of using hardware to evaluate
+ the condition expression, thus only triggering the watchpoint when it is
+ true. */
+static int
+ppc_linux_can_accel_watchpoint_condition (CORE_ADDR addr, int len, int rw,
+ struct expression *cond)
+{
+ int data_value_len;
+ CORE_ADDR data_value;
+
+ return (have_ptrace_booke_interface ()
+ && booke_debug_info.num_condition_regs > 0
+ && check_condition (addr, cond, &data_value, &data_value_len));
+}
+
+static int
+ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw,
+ struct expression *cond)
{
struct lwp_info *lp;
ptid_t ptid;
@@ -1694,15 +1997,25 @@ ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
if (have_ptrace_booke_interface ())
{
+ int data_value_len;
struct ppc_hw_breakpoint p;
+ CORE_ADDR data_value;
+
+ if (cond && can_use_watchpoint_cond_accel ()
+ && check_condition (addr, cond, &data_value, &data_value_len))
+ calculate_dvc (addr, len, data_value, data_value_len,
+ &p.condition_mode, &p.condition_value);
+ 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));
@@ -1749,7 +2062,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;
@@ -1759,7 +2073,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 *cond)
{
struct lwp_info *lp;
ptid_t ptid;
@@ -1767,15 +2082,25 @@ ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw)
if (have_ptrace_booke_interface ())
{
+ int data_value_len;
struct ppc_hw_breakpoint p;
+ CORE_ADDR data_value;
+
+ if (cond && booke_debug_info.num_condition_regs > 0
+ && check_condition (addr, cond, &data_value, &data_value_len))
+ calculate_dvc (addr, len, data_value, data_value_len,
+ &p.condition_mode, &p.condition_value);
+ 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));
@@ -1786,7 +2111,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;
@@ -2137,6 +2463,7 @@ _initialize_ppc_linux_nat (void)
t->to_stopped_by_watchpoint = ppc_linux_stopped_by_watchpoint;
t->to_stopped_data_address = ppc_linux_stopped_data_address;
t->to_watchpoint_addr_within_range = ppc_linux_watchpoint_addr_within_range;
+ t->to_can_accel_watchpoint_condition = ppc_linux_can_accel_watchpoint_condition;
t->to_read_description = ppc_linux_read_description;
t->to_auxv_parse = ppc_linux_auxv_parse;
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 10aaa48..3bef9ab 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -5161,7 +5161,8 @@ procfs_stopped_data_address (struct target_ops *targ, CORE_ADDR *addr)
}
static int
-procfs_insert_watchpoint (CORE_ADDR addr, int len, int type)
+procfs_insert_watchpoint (CORE_ADDR addr, int len, int type,
+ struct expression *cond)
{
if (!target_have_steppable_watchpoint
&& !gdbarch_have_nonsteppable_watchpoint (target_gdbarch))
@@ -5182,7 +5183,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 *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 ad21774..ca450d5 100644
--- a/gdb/remote-m32r-sdi.c
+++ b/gdb/remote-m32r-sdi.c
@@ -1421,7 +1421,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 *cond)
{
int i;
@@ -1445,7 +1446,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 *cond)
{
int i;
diff --git a/gdb/remote-mips.c b/gdb/remote-mips.c
index 28d2ecb..ec2e5eb 100644
--- a/gdb/remote-mips.c
+++ b/gdb/remote-mips.c
@@ -2401,7 +2401,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 *cond)
{
if (mips_set_breakpoint (addr, len, type))
return -1;
@@ -2412,7 +2413,8 @@ mips_insert_watchpoint (CORE_ADDR addr, int len, int type)
/* Remove a watchpoint. */
int
-mips_remove_watchpoint (CORE_ADDR addr, int len, int type)
+mips_remove_watchpoint (CORE_ADDR addr, int len, int type,
+ struct expression *cond)
{
if (mips_clear_breakpoint (addr, len, type))
return -1;
diff --git a/gdb/remote.c b/gdb/remote.c
index 35d517d..c5c8d3e 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -7537,7 +7537,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 *cond)
{
struct remote_state *rs = get_remote_state ();
char *p;
@@ -7570,7 +7571,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 *cond)
{
struct remote_state *rs = get_remote_state ();
char *p;
diff --git a/gdb/s390-nat.c b/gdb/s390-nat.c
index 3af42ff..b412fda 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 *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 *cond)
{
struct lwp_info *lp;
ptid_t ptid;
diff --git a/gdb/target.c b/gdb/target.c
index 7aa77e6..c3fb29b 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -117,9 +117,11 @@ 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 *);
-static int debug_to_remove_watchpoint (CORE_ADDR, int, int);
+static int debug_to_remove_watchpoint (CORE_ADDR, int, int,
+ struct expression *);
static int debug_to_stopped_by_watchpoint (void);
@@ -130,6 +132,9 @@ static int debug_to_watchpoint_addr_within_range (struct target_ops *,
static int debug_to_region_ok_for_hw_watchpoint (CORE_ADDR, int);
+static int debug_to_can_accel_watchpoint_condition (CORE_ADDR, int, int,
+ struct expression *);
+
static void debug_to_terminal_init (void);
static void debug_to_terminal_inferior (void);
@@ -607,6 +612,7 @@ update_current_target (void)
INHERIT (to_stopped_by_watchpoint, t);
INHERIT (to_watchpoint_addr_within_range, t);
INHERIT (to_region_ok_for_hw_watchpoint, t);
+ INHERIT (to_can_accel_watchpoint_condition, t);
INHERIT (to_terminal_init, t);
INHERIT (to_terminal_inferior, t);
INHERIT (to_terminal_ours_for_output, t);
@@ -726,10 +732,10 @@ 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 *))
return_minus_one);
de_fault (to_remove_watchpoint,
- (int (*) (CORE_ADDR, int, int))
+ (int (*) (CORE_ADDR, int, int, struct expression *))
return_minus_one);
de_fault (to_stopped_by_watchpoint,
(int (*) (void))
@@ -741,6 +747,9 @@ update_current_target (void)
default_watchpoint_addr_within_range);
de_fault (to_region_ok_for_hw_watchpoint,
default_region_ok_for_hw_watchpoint);
+ de_fault (to_can_accel_watchpoint_condition,
+ (int (*) (CORE_ADDR, int, int, struct expression *))
+ return_zero);
de_fault (to_terminal_init,
(void (*) (void))
target_ignore);
@@ -3306,6 +3315,21 @@ debug_to_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
}
static int
+debug_to_can_accel_watchpoint_condition (CORE_ADDR addr, int len, int rw,
+ struct expression *cond)
+{
+ int retval;
+
+ retval = debug_target.to_can_accel_watchpoint_condition (addr, len, rw, cond);
+
+ fprintf_unfiltered (gdb_stdlog,
+ "target_can_accel_watchpoint_condition (0x%lx, %d, %d, 0x%lx) = %ld\n",
+ (unsigned long) addr, len, rw, (unsigned long) cond,
+ (unsigned long) retval);
+ return retval;
+}
+
+static int
debug_to_stopped_by_watchpoint (void)
{
int retval;
@@ -3380,28 +3404,32 @@ 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 *cond)
{
int retval;
- retval = debug_target.to_insert_watchpoint (addr, len, type);
+ retval = debug_target.to_insert_watchpoint (addr, len, type, cond);
fprintf_unfiltered (gdb_stdlog,
- "target_insert_watchpoint (0x%lx, %d, %d) = %ld\n",
- (unsigned long) addr, len, type, (unsigned long) retval);
+ "target_insert_watchpoint (0x%lx, %d, %d, 0x%ld) = %ld\n",
+ (unsigned long) addr, len, type, (unsigned long) cond,
+ (unsigned long) retval);
return retval;
}
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 *cond)
{
int retval;
- retval = debug_target.to_remove_watchpoint (addr, len, type);
+ retval = debug_target.to_remove_watchpoint (addr, len, type, cond);
fprintf_unfiltered (gdb_stdlog,
- "target_remove_watchpoint (0x%lx, %d, %d) = %ld\n",
- (unsigned long) addr, len, type, (unsigned long) retval);
+ "target_remove_watchpoint (0x%lx, %d, %d, 0x%ld) = %ld\n",
+ (unsigned long) addr, len, type, (unsigned long) cond,
+ (unsigned long) retval);
return retval;
}
@@ -3656,6 +3684,7 @@ setup_target_debug (void)
current_target.to_stopped_data_address = debug_to_stopped_data_address;
current_target.to_watchpoint_addr_within_range = debug_to_watchpoint_addr_within_range;
current_target.to_region_ok_for_hw_watchpoint = debug_to_region_ok_for_hw_watchpoint;
+ current_target.to_can_accel_watchpoint_condition = debug_to_can_accel_watchpoint_condition;
current_target.to_terminal_init = debug_to_terminal_init;
current_target.to_terminal_inferior = debug_to_terminal_inferior;
current_target.to_terminal_ours_for_output = debug_to_terminal_ours_for_output;
diff --git a/gdb/target.h b/gdb/target.h
index 870a1eb..9ef2947 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -36,6 +36,8 @@ struct trace_status;
struct uploaded_tsv;
struct uploaded_tp;
+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 +422,12 @@ 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 belwo are expected to do is
+ provided with the corresponding target_* macros. */
+ int (*to_remove_watchpoint) (CORE_ADDR, int, int, struct expression *);
+ int (*to_insert_watchpoint) (CORE_ADDR, int, int, struct expression *);
+
int (*to_stopped_by_watchpoint) (void);
int to_have_steppable_watchpoint;
int to_have_continuable_watchpoint;
@@ -429,6 +435,8 @@ struct target_ops
int (*to_watchpoint_addr_within_range) (struct target_ops *,
CORE_ADDR, CORE_ADDR, int);
int (*to_region_ok_for_hw_watchpoint) (CORE_ADDR, int);
+ int (*to_can_accel_watchpoint_condition) (CORE_ADDR, int, int,
+ struct expression *);
void (*to_terminal_init) (void);
void (*to_terminal_inferior) (void);
void (*to_terminal_ours_for_output) (void);
@@ -1282,14 +1290,15 @@ extern char *normal_pid_to_str (ptid_t ptid);
/* 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.
+ COND is the expression for its condition, or NULL if there's none.
Returns 0 for success, 1 if the watchpoint type is not supported,
-1 for failure. */
-#define target_insert_watchpoint(addr, len, type) \
- (*current_target.to_insert_watchpoint) (addr, len, type)
+#define target_insert_watchpoint(addr, len, type, cond) \
+ (*current_target.to_insert_watchpoint) (addr, len, type, cond)
-#define target_remove_watchpoint(addr, len, type) \
- (*current_target.to_remove_watchpoint) (addr, len, type)
+#define target_remove_watchpoint(addr, len, type, cond) \
+ (*current_target.to_remove_watchpoint) (addr, len, type, cond)
#define target_insert_hw_breakpoint(gdbarch, bp_tgt) \
(*current_target.to_insert_hw_breakpoint) (gdbarch, bp_tgt)
@@ -1306,6 +1315,12 @@ extern char *normal_pid_to_str (ptid_t ptid);
#define target_watchpoint_addr_within_range(target, addr, start, length) \
(*target.to_watchpoint_addr_within_range) (target, addr, start, length)
+/* Return non-zero if the target is capable of using hardware to evaluate
+ the condition expression, thus only triggering the watchpoint when it is
+ true. */
+#define target_can_accel_watchpoint_condition(addr, len, type, cond) \
+ (*current_target.to_can_accel_watchpoint_condition) (addr, len, type, cond)
+
/* Target can execute in reverse? */
#define target_can_execute_reverse \
(current_target.to_can_execute_reverse ? \