Re: Watchpoint resource accounting broken (Re: [5/6] breakpoints_ops for all kinds of breakpoints: new watchpoints instance type)

[Edjunior Barbosa Machado <emachado@linux.vnet.ibm.com>]

On 09/13/2011 09:55 AM, Pedro Alves wrote:
> On Monday 12 September 2011 16:16:53, Edjunior Barbosa Machado wrote:
>> On 09/12/2011 12:03 PM, Ulrich Weigand wrote:
>>> Note how just "i", the result of hw_watchpoint_used_count, is passed to
>>> target_can_use_hardware_watchpoint -- this works only if the current
>>> watchpoint is on the list that hw_watchpoint_used_count iterates over.
>>
>> I noticed this problem too and was considering use "i + reg_cnt" instead
>> of only "i" when calling hw_watchpoint_used_count() (actually, I saw gdb
>> used to use this previously).
>> However, with this change, due to the same problem with the watchpoint
>> added to breakpoint list that Ulrich mentioned, watchpoints added before
>> run the inferior will not work.
> 
> Can you expand on this?  I don't think I'm understanding that problem.
> 

The problem of setting watches before run the inferior occurs because
update_watchpoint() expect that the watchpoint being parsed shouldn't be
in the breakpoint_chain.

When you adds the watchpoint it passes through update_watchpoint() the
first time without reparsing it, but it adds the watch to the
breakpoint_chain right after (in install_breakpoint()). Then, after
start the inferior, when running update_watchpoint() again,
hw_watchpoint_used_count() will count this watch previously added and
reparse will not evaluate the available resources properly.
Moreover, I've noticed it doesn't occur on x86 since
target_can_use_hardware_watchpoint() (i386_can_use_hw_breakpoint())
always returns 1, but affects archs such as ppc64. This problem in
specific can be seen on gdb.base/watchpoint-hw.exp testcase.

By the way, had the chance to retest your new patch on ppc64 and it
seems to fix several issues related to watchpoints, including the
failure pointed by gdb.base/watchpoint-hw.exp.

Thanks,
-- 
Edjunior