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: Tue, 06 Jul 2010 19:22:27 -0300
- Subject: Re: [PATCH 2/4] Hardware accelerated watchpoint conditions
- References: <201007021319.o62DJvv4018879@d12av02.megacenter.de.ibm.com>
On Fri, 2010-07-02 at 15:19 +0200, Ulrich Weigand wrote:
> Thiago Jung Bauermann wrote:
> > > > + 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? */
> > >
> > > These are values from the history (e.g. $1). In this context, they
> > > should be treated as non-lvalues, so they should be fine ...
> >
> > I removed that else if then. I'll submit a separate patch to replace
> > that comment in can_use_hardware_watchpoint.
>
> Actually, thinking about this a bit more, the logic still does not
> seem quite correct. What we really want is:
>
> - not_lval values: these are fine
> - deprecated_value_modifiable (v) == 0 (history) values: treat just
> like not_lval, no matter what their VALUE_LVAL says
> - lval_memory values: check whether they are lazy or not and count them
> - all other values (lval_computed, lval_internalvar ...): *not* OK
>
> In particular, the missing check on lval_computed may become more important
> now that DWARF multi-piece values are always represented as lval_computed ...
> (This may also affect watchpoint code in breakpoint.c, but I guess that's
> a different issue.)
Ok, makes sense. That loop now reads:
for (; v; v = value_next (v))
{
/* Constants and values from the history are fine. */
if (VALUE_LVAL (v) == not_lval || deprecated_value_modifiable (v) == 0)
continue;
else 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++;
}
/* Other kinds of values are not fine. */
else
return -1;
}
> Except for the num_memory_accesses logic discussed above, the patch
> is otherwise OK now.
Great! Except for the change in the loop above, the rest of the patch is
the same.
Thanks for your help with this work.
2010-07-06 Sergio Durigan Junior <sergiodj@linux.vnet.ibm.com>
Thiago Jung Bauermann <bauerman@br.ibm.com>
* breakpoint.c (fetch_watchpoint_value): Rename to fetch_subexp_value
and move to eval.c. Change callers.
(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.
* eval.c: Include wrapper.h.
(fetch_subexp_value): Moved from breakpoint.c.
* ppc-linux-nat.c (ppc_linux_region_ok_for_hw_watchpoint):
Formatting fix.
(can_use_watchpoint_cond_accel): New function.
(calculate_dvc): Likewise.
(num_memory_accesses): 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 declaration 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.
* value.c (free_value_chain): New function.
* value.h (fetch_subexp_value): New prototype.
(free_value_chain): Likewise.
Index: gdb/breakpoint.c
===================================================================
--- gdb.orig/breakpoint.c 2010-07-06 19:05:12.000000000 -0300
+++ gdb/breakpoint.c 2010-07-06 19:07:53.000000000 -0300
@@ -1234,84 +1234,6 @@ is_watchpoint (const struct breakpoint *
|| bpt->type == bp_watchpoint);
}
-/* Find the current value of a watchpoint on 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.
- This is the value which we store in struct breakpoint.
-
- 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_watchpoint_value (struct expression *exp, 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_expression (exp);
- }
- 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;
-
- /* Make sure it's not lazy, so that after the target stops again we
- have a non-lazy previous value to compare with. */
- if (result != NULL
- && (!value_lazy (result) || gdb_value_fetch_lazy (result)))
- *valp = result;
-
- if (val_chain)
- {
- /* Return the chain of intermediate values. We use this to
- decide which addresses to watch. */
- *val_chain = new_mark;
- value_release_to_mark (mark);
- }
-}
-
/* Assuming that B is a watchpoint: returns true if the current thread
and its running state are safe to evaluate or update watchpoint B.
Watchpoints on local expressions need to be evaluated in the
@@ -1469,10 +1391,11 @@ update_watchpoint (struct breakpoint *b,
}
else if (within_current_scope && b->exp)
{
+ int pc = 0;
struct value *val_chain, *v, *result, *next;
struct program_space *frame_pspace;
- fetch_watchpoint_value (b->exp, &v, &result, &val_chain);
+ fetch_subexp_value (b->exp, &pc, &v, &result, &val_chain);
/* Avoid setting b->val if it's already set. The meaning of
b->val is 'the last value' user saw, and we should update
@@ -1828,7 +1751,8 @@ Note: automatically using hardware break
{
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. */
@@ -1856,7 +1780,8 @@ Note: automatically using hardware break
{
val = target_insert_watchpoint (bpt->address,
bpt->length,
- hw_access);
+ hw_access,
+ bpt->owner->cond_exp);
if (val == 0)
bpt->watchpoint_type = hw_access;
}
@@ -2525,8 +2450,8 @@ remove_breakpoint_1 (struct bp_location
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))
@@ -3679,10 +3604,11 @@ watchpoint_check (void *p)
call free_all_values. We can't call free_all_values because
we might be in the middle of evaluating a function call. */
+ int pc = 0;
struct value *mark = value_mark ();
struct value *new_val;
- fetch_watchpoint_value (b->exp, &new_val, NULL, NULL);
+ fetch_subexp_value (b->exp, &pc, &new_val, NULL, NULL);
/* We use value_equal_contents instead of value_equal because the latter
coerces an array to a pointer, thus comparing just the address of the
@@ -5262,6 +5188,21 @@ watchpoint_locations_match (struct bp_lo
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
@@ -8057,6 +7998,7 @@ watch_command_1 (char *arg, int accessfl
enum bptype bp_type;
int mem_cnt = 0;
int thread = -1;
+ int pc = 0;
/* Make sure that we actually have parameters to parse. */
if (arg != NULL && arg[0] != '\0')
@@ -8143,7 +8085,7 @@ watch_command_1 (char *arg, int accessfl
exp_valid_block = innermost_block;
mark = value_mark ();
- fetch_watchpoint_value (exp, &val, NULL, NULL);
+ fetch_subexp_value (exp, &pc, &val, NULL, NULL);
if (val != NULL)
release_value (val);
Index: gdb/eval.c
===================================================================
--- gdb.orig/eval.c 2010-07-02 17:03:03.000000000 -0300
+++ gdb/eval.c 2010-07-06 19:07:53.000000000 -0300
@@ -43,6 +43,7 @@
#include "gdb_obstack.h"
#include "objfiles.h"
#include "python/python.h"
+#include "wrapper.h"
#include "gdb_assert.h"
@@ -186,6 +187,84 @@ evaluate_subexpression_type (struct expr
return evaluate_subexp (NULL_TYPE, exp, &subexp, EVAL_AVOID_SIDE_EFFECTS);
}
+/* Find the current value of a watchpoint on 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.
+ This is the value which we store in struct breakpoint.
+
+ 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. */
+
+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;
+
+ /* Make sure it's not lazy, so that after the target stops again we
+ have a non-lazy previous value to compare with. */
+ if (result != NULL
+ && (!value_lazy (result) || gdb_value_fetch_lazy (result)))
+ *valp = result;
+
+ if (val_chain)
+ {
+ /* Return the chain of intermediate values. We use this to
+ decide which addresses to watch. */
+ *val_chain = new_mark;
+ value_release_to_mark (mark);
+ }
+}
+
/* Extract a field operation from an expression. If the subexpression
of EXP starting at *SUBEXP is not a structure dereference
operation, return NULL. Otherwise, return the name of the
Index: gdb/i386-nat.c
===================================================================
--- gdb.orig/i386-nat.c 2010-07-02 17:03:03.000000000 -0300
+++ gdb/i386-nat.c 2010-07-06 19:07:53.000000000 -0300
@@ -484,7 +484,8 @@ Invalid value %d of operation in i386_ha
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,
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;
Index: gdb/ia64-linux-nat.c
===================================================================
--- gdb.orig/ia64-linux-nat.c 2010-07-02 17:03:03.000000000 -0300
+++ gdb/ia64-linux-nat.c 2010-07-06 19:07:53.000000000 -0300
@@ -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
}
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;
Index: gdb/inf-ttrace.c
===================================================================
--- gdb.orig/inf-ttrace.c 2010-07-02 17:03:03.000000000 -0300
+++ gdb/inf-ttrace.c 2010-07-06 19:07:53.000000000 -0300
@@ -314,7 +314,8 @@ inf_ttrace_disable_page_protections (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
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);
Index: gdb/mips-linux-nat.c
===================================================================
--- gdb.orig/mips-linux-nat.c 2010-07-02 17:03:03.000000000 -0300
+++ gdb/mips-linux-nat.c 2010-07-06 19:07:53.000000000 -0300
@@ -926,7 +926,8 @@ populate_regs_from_watches (struct pt_wa
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
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;
Index: gdb/nto-procfs.c
===================================================================
--- gdb.orig/nto-procfs.c 2010-07-02 17:03:03.000000000 -0300
+++ gdb/nto-procfs.c 2010-07-06 19:07:53.000000000 -0300
@@ -1506,13 +1506,15 @@ procfs_can_use_hw_breakpoint (int type,
}
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);
}
Index: gdb/ppc-linux-nat.c
===================================================================
--- gdb.orig/ppc-linux-nat.c 2010-07-02 17:03:03.000000000 -0300
+++ gdb/ppc-linux-nat.c 2010-07-06 19:07:53.000000000 -0300
@@ -1492,7 +1492,7 @@ ppc_linux_region_ok_for_hw_watchpoint (C
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 +1685,202 @@ get_trigger_type (int rw)
return t;
}
+/* Check whether we have at least one free DVC register. */
static int
-ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw)
+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.
+ 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,
+ 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). */
+
+ *condition_value = ((uint64_t) data_value >> num_bytes_off_dvc * 8
+ << rightmost_enabled_byte * 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, or if there are
+ other kinds of values which are not acceptable in a condition
+ expression (e.g., lval_computed or lval_internalvar). */
+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))
+ {
+ /* Constants and values from the history are fine. */
+ if (VALUE_LVAL (v) == not_lval || deprecated_value_modifiable (v) == 0)
+ continue;
+ else 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++;
+ }
+ /* Other kinds of values are not fine. */
+ else
+ return -1;
+ }
+
+ return found_memory_cnt;
+}
+
+/* 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.
+ If the function returns 1, DATA_VALUE will contain the constant against
+ which the watch value should be compared. */
+static int
+check_condition (CORE_ADDR watch_addr, struct expression *cond,
+ CORE_ADDR *data_value)
+{
+ int pc = 1, num_accesses_left, num_accesses_right;
+ struct value *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)
+ *data_value = value_as_long (right_val);
+ else if (num_accesses_left == 0 && num_accesses_right == 1
+ && VALUE_LVAL (right_val) == lval_memory
+ && value_address (right_val) == watch_addr)
+ *data_value = value_as_long (left_val);
+ else
+ {
+ free_value_chain (left_chain);
+ free_value_chain (right_chain);
+
+ return 0;
+ }
+
+ 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)
+{
+ CORE_ADDR data_value;
+
+ return (have_ptrace_booke_interface ()
+ && booke_debug_info.num_condition_regs > 0
+ && check_condition (addr, cond, &data_value));
+}
+
+static int
+ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw,
+ struct expression *cond)
{
struct lwp_info *lp;
ptid_t ptid;
@@ -1695,14 +1889,23 @@ ppc_linux_insert_watchpoint (CORE_ADDR a
if (have_ptrace_booke_interface ())
{
struct ppc_hw_breakpoint p;
+ CORE_ADDR data_value;
+
+ if (cond && can_use_watchpoint_cond_accel ()
+ && check_condition (addr, cond, &data_value))
+ calculate_dvc (addr, len, data_value, &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 +1952,8 @@ ppc_linux_insert_watchpoint (CORE_ADDR a
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 +1963,8 @@ ppc_linux_insert_watchpoint (CORE_ADDR a
}
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;
@@ -1768,14 +1973,23 @@ ppc_linux_remove_watchpoint (CORE_ADDR a
if (have_ptrace_booke_interface ())
{
struct ppc_hw_breakpoint p;
+ CORE_ADDR data_value;
+
+ if (cond && booke_debug_info.num_condition_regs > 0
+ && check_condition (addr, cond, &data_value))
+ calculate_dvc (addr, len, data_value, &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 +2000,8 @@ ppc_linux_remove_watchpoint (CORE_ADDR a
{
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 +2352,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;
Index: gdb/procfs.c
===================================================================
--- gdb.orig/procfs.c 2010-07-06 19:05:12.000000000 -0300
+++ gdb/procfs.c 2010-07-06 19:07:53.000000000 -0300
@@ -5161,7 +5161,8 @@ procfs_stopped_data_address (struct targ
}
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
}
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);
}
Index: gdb/remote-m32r-sdi.c
===================================================================
--- gdb.orig/remote-m32r-sdi.c 2010-07-02 17:03:03.000000000 -0300
+++ gdb/remote-m32r-sdi.c 2010-07-06 19:07:53.000000000 -0300
@@ -1421,7 +1421,8 @@ m32r_can_use_hw_watchpoint (int type, in
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,
}
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;
Index: gdb/remote-mips.c
===================================================================
--- gdb.orig/remote-mips.c 2010-07-02 17:03:04.000000000 -0300
+++ gdb/remote-mips.c 2010-07-06 19:07:53.000000000 -0300
@@ -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,
/* 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;
Index: gdb/remote.c
===================================================================
--- gdb.orig/remote.c 2010-07-06 19:05:12.000000000 -0300
+++ gdb/remote.c 2010-07-06 19:07:53.000000000 -0300
@@ -7646,7 +7646,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;
@@ -7679,7 +7680,8 @@ remote_insert_watchpoint (CORE_ADDR addr
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;
Index: gdb/s390-nat.c
===================================================================
--- gdb.orig/s390-nat.c 2010-07-02 17:03:04.000000000 -0300
+++ gdb/s390-nat.c 2010-07-06 19:07:53.000000000 -0300
@@ -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,
}
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;
Index: gdb/target.c
===================================================================
--- gdb.orig/target.c 2010-07-06 19:05:12.000000000 -0300
+++ gdb/target.c 2010-07-06 19:07:53.000000000 -0300
@@ -117,9 +117,11 @@ static int debug_to_insert_hw_breakpoint
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_with
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);
@@ -728,10 +734,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))
@@ -743,6 +749,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);
@@ -3314,6 +3323,21 @@ debug_to_region_ok_for_hw_watchpoint (CO
}
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;
@@ -3388,28 +3412,32 @@ debug_to_remove_hw_breakpoint (struct gd
}
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;
}
@@ -3664,6 +3692,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;
Index: gdb/target.h
===================================================================
--- gdb.orig/target.h 2010-07-06 19:05:12.000000000 -0300
+++ gdb/target.h 2010-07-06 19:07:53.000000000 -0300
@@ -37,6 +37,8 @@ struct uploaded_tsv;
struct uploaded_tp;
struct static_tracepoint_marker;
+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
@@ -426,8 +428,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 below 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;
@@ -435,6 +441,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);
@@ -1298,14 +1306,15 @@ extern char *normal_pid_to_str (ptid_t p
/* 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)
@@ -1322,6 +1331,19 @@ extern char *normal_pid_to_str (ptid_t p
#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. In this case, if the condition is false when
+ the watched memory location changes, execution may continue without the
+ debugger being notified.
+
+ Due to limitations in the hardware implementation, it may be capable of
+ avoiding triggering the watchpoint in some cases where the condition
+ expression is false, but may report some false positives as well.
+ For this reason, GDB will still evaluate the condition expression when
+ the watchpoint triggers. */
+#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 ? \
Index: gdb/value.c
===================================================================
--- gdb.orig/value.c 2010-07-06 19:05:12.000000000 -0300
+++ gdb/value.c 2010-07-06 19:07:53.000000000 -0300
@@ -722,6 +722,20 @@ free_all_values (void)
all_values = 0;
}
+/* Frees all the elements in a chain of values. */
+
+void
+free_value_chain (struct value *v)
+{
+ struct value *next;
+
+ for (; v; v = next)
+ {
+ next = value_next (v);
+ value_free (v);
+ }
+}
+
/* Remove VAL from the chain all_values
so it will not be freed automatically. */
Index: gdb/value.h
===================================================================
--- gdb.orig/value.h 2010-07-06 19:05:12.000000000 -0300
+++ gdb/value.h 2010-07-06 19:07:53.000000000 -0300
@@ -538,6 +538,10 @@ extern struct value *evaluate_subexp (st
extern struct value *evaluate_subexpression_type (struct expression *exp,
int subexp);
+extern void fetch_subexp_value (struct expression *exp, int *pc,
+ struct value **valp, struct value **resultp,
+ struct value **val_chain);
+
extern char *extract_field_op (struct expression *exp, int *subexp);
extern struct value *evaluate_subexp_with_coercion (struct expression *,
@@ -635,6 +639,8 @@ extern void value_free (struct value *va
extern void free_all_values (void);
+extern void free_value_chain (struct value *v);
+
extern void release_value (struct value *val);
extern int record_latest_value (struct value *val);