This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA]: Fix for watchpoint regressions


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.







Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]