[PATCH 2/4] Hardware accelerated watchpoint conditions
Thiago Jung Bauermann
bauerman@br.ibm.com
Tue Jul 6 22:22:00 GMT 2010
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: condition.diff
Type: text/x-patch
Size: 39207 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20100706/2dd9b3e8/attachment.bin>
More information about the Gdb-patches
mailing list