This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch 2/3] Implement support for PowerPC BookE ranged watchpoints
- From: Pedro Alves <pedro at codesourcery dot com>
- To: Thiago Jung Bauermann <bauerman at br dot ibm dot com>
- Cc: gdb-patches at sourceware dot org, Jan Kratochvil <jan dot kratochvil at redhat dot com>, Joel Brobecker <brobecker at adacore dot com>, Eli Zaretskii <eliz at gnu dot org>
- Date: Tue, 28 Dec 2010 15:29:04 +0000
- Subject: Re: [patch 2/3] Implement support for PowerPC BookE ranged watchpoints
- References: <1290549100.3164.47.camel@hactar> <201012232017.11120.pedro@codesourcery.com> <1293484743.1544.78.camel@hactar>
On Monday 27 December 2010 21:19:03, Thiago Jung Bauermann wrote:
> @@ -1394,25 +1397,33 @@ update_watchpoint (struct breakpoint *b, int reparse)
> if ((b->type == bp_watchpoint || b->type == bp_hardware_watchpoint)
> && reparse)
> {
> - int i, mem_cnt, other_type_used;
> -
> - /* We need to determine how many resources are already used
> - for all other hardware watchpoints to see if we still have
> - enough resources to also fit this watchpoint in as well.
> - To avoid the hw_watchpoint_used_count call below from counting
> - this watchpoint, make sure that it is marked as a software
> - watchpoint. */
> - b->type = bp_watchpoint;
> - i = hw_watchpoint_used_count (bp_hardware_watchpoint,
> - &other_type_used);
> - mem_cnt = can_use_hardware_watchpoint (val_chain);
> -
> - if (!mem_cnt)
> + int reg_cnt;
> +
> + reg_cnt = can_use_hardware_watchpoint (val_chain, b->exact);
> +
> + if (!reg_cnt)
> b->type = bp_watchpoint;
> else
> {
> - int target_resources_ok = target_can_use_hardware_watchpoint
> - (bp_hardware_watchpoint, i + mem_cnt, other_type_used);
> + int i, other_type_used, target_resources_ok;
> + enum bptype orig_type;
> +
> + if (b->ops && b->ops->extra_resources_needed)
> + reg_cnt += b->ops->extra_resources_needed (b, NULL);
I'm taking a look at the resource accounting changes, which I
mostly ignored before, and I'm confused. can_use_hardware_watchpoint
(above) calls target_region_ok_for_hw_watchpoint, which has been changed
to return the number of resources normally necessary. Yet,
extra_resources_needed_watchpoint calls target_region_ok_for_hw_watchpoint
as well (discounting 1, it seems). Aren't we ending up with gdb
thinking it needs more resources than are really necessary?
> +
> + /* We need to determine how many resources are already used
> + for all other hardware watchpoints to see if we still have
> + enough resources to also fit this watchpoint in as well.
> + To avoid the hw_watchpoint_used_count call below from
> + counting this watchpoint, make sure that it is marked as a
> + software watchpoint. */
> + orig_type = b->type;
> + b->type = bp_watchpoint;
Something I notice: It sounds like if you remove this hack above,
> + i = hw_watchpoint_used_count (bp_hardware_watchpoint,
> + &other_type_used);
then this above accounts for the new watchpoint too, so...
> +
> + target_resources_ok = target_can_use_hardware_watchpoint
> + (bp_hardware_watchpoint, i + reg_cnt, other_type_used)
... this would just be ", i,", instead of ", i + reg_cnt,".
> if (target_resources_ok <= 0)
> b->type = bp_watchpoint;
> else
> > What does the "TYPE_NFIELDS (t) == 1" do ?
>
> TYPE_NFIELDS (t) == 1 && TYPE_CODE (TYPE_INDEX_TYPE (t)) == TYPE_CODE_RANGE
> is a way to determine that the given array type has known bounds.
Didn't know about that. I thought TYPE_NFIELDS only made sense
for structs/classes/unions. My grep foo isn't finding other similar uses.
I tried:
compunit1.c:
extern int array[];
void foo ()
{
...
}
compunit2.c:
int array[];
and in foo, ptype array gave me array[0], TYPE_NFIELDS==1.
> > Should be "Use of exact watchpoints is ...", I think? Thus getting rid
> > of the small lie that not all watchpoints (in the user perspective) will
> > use just one debug register, leaving the just one debug register
> > issue explanation to the help string (as you already have).
>
> Good point. Changed to:
(...)
Thanks.
--
Pedro Alves