Re: [RFA]: Fix for watchpoint regressions

[Jeff Johnston <jjohnstn@redhat.com>]

Eli Zaretskii wrote:
>>Date: Mon, 21 Jun 2004 11:40:31 -0400
>>From: Jeff Johnston <jjohnstn@redhat.com>
>>
>>2004-06-18  Jeff Johnston  <jjohnstn@redhat.com>
>>
>>         * infrun.c (handle_inferior_event): Initialize stopped_by_watchpoint
>>         to -1.
>>         * breakpoint.c (bpstat_stop_status): Move check for ignoring
>>         untriggered watchpoints to a separate if clause.  Update function
>>         comment regarding STOPPED_BY_WATCHPOINT argument.
> 
> 
> Won't this break the cases that caused the addition of testing that
> stopped_by_watchpoint is non-zero in bpstat_stop_status?
>

I took care to try to ensure that the problem the original patch was fixing was 
still being addressed.

The stopped_by_watchpoint flag was only being set by handle_inferior_event if 
HAVE_CONTINUABLE_WATCHPOINT is true.  In the cases where the flag should be 
tested, it will be set correctly to false (0) or true (1).  The problem is that 
there is third state (don't know) which is due to the fact that the flag is not 
set in certain cases and you don't really know if you are stopped by a 
watchpoint or not.  This occurs, for example, when HAVE_NONSTEPPABLE_WATCHPOINT 
or HAVE_STEPPABLE_WATCHPOINT is true as in the case of ia64.  The flags indicate 
that the watchpoint has triggered prior to the actual change or value.  The 
handle_inferior_event function steps through the watchpoint change.  At that 
point, the STOPPED_BY_WATCHPOINT test doesn't return true for ia64, were it to 
be tested.  My original attempt at this was to try and set the flag regardless 
of the HAVE_CONTINUABLE_WATCHPOINT.  It did not work because 
STOPPED_BY_WATCHPOINT is not true after the step.

Things worked in the past for ia64 because the code near the bottom of the loop 
in bpstat_stop_status would check whether a hardware watchpoint value had 
changed.  With Ulrich's original patch in place, this never happens because we 
never get to that piece of code in the loop.  With the revised patch, for the 
original scenario that Ulrich was fixing, we will not get to the watchpoint 
checking code, but for platforms that do not have HAVE_CONTINUABLE_WATCHPOINT, 
we will.  We won't yet be able to handle the breakpoint insn in code scenario 
for ia64 that Ulrich was fixing, but that is a separate fix.

I am looking at making another patch which would mark a watchpoint for the 
stepping cases so we can tell if a particular watchpoint triggered or not prior 
to the step and thus allow gdb to avoid having to randomly test all value 
changes.  This would help to eliminate the cases where we might report n 
different watchpoint changes for a line of code where the code in question may 
only be responsible for one of the value changes.  This would also allow the 
breakpoint insn in code scenario to work on ia64.

-- Jeff J.