Re: [PATCH v2] Fix hardware watchpoints in replay mode

[Hannes Domani <ssbssa@yahoo.de>]

 Am Dienstag, 21. November 2023 um 13:52:42 MEZ hat Guinevere Larsen <blarsen@redhat.com> Folgendes geschrieben:

> On 20/11/2023 16:59, Hannes Domani wrote:
> >  Am Montag, 20. November 2023, 14:09:34 MEZ hat Guinevere Larsen <blarsen@redhat.com> Folgendes geschrieben:
> >
> >> On 14/09/2023 18:27, Hannes Domani via Gdb-patches wrote:
> >>> Changes introduced by commit 9e8915c6cee5c37637521b424d723e990e06d597
> >>> caused a regression that meant hardware watchpoint stops would not
> >>> trigger in reverse execution or replay mode.  This was documented in
> >>> PR breakpoints/21969.
> >>> The problem is that record_check_stopped_by_breakpoint always overwrites
> >>> record_full_stop_reason, thus loosing the TARGET_STOPPED_BY_WATCHPOINT
> >>> value which would be checked afterwards.
> >>>
> >>> This commit fixes that by checking if record_full_stop_reason is
> >>> TARGET_STOPPED_BY_BREAKPOINT and, if so, not overriding it.
> >>>
> >>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=21969
> >>> ---
> >> I have sent a review to this patch. Your spam filter must have catched
> >> it or something similar. If you can't find it, here's the link for the
> >> public inbox:
> >> https://inbox.sourceware.org/gdb-patches/2eb65a2d-aa80-5930-3033-bfd82baa2ed9@redhat.com/
> > I saw that, but it doesn't really make clear what I should do now.
> > And from what I can remember of our discussion on IRC, you wanted to wait if Pedro had anything to add.
> >
> Ah right, sorry, this last month has been pretty hectic, our
> conversation completely slipped my mind.

All right, I'll do that.


> I took a second, long look at record_check_stopped_by_breakpoint and
> convinced myself, I don't see why it should overwrite the reason if gdb
> is not stopped at a watchpoint. I would say send in a v3 with that
> solution instead, CC Pedro, and I'll give some time for him to react
> before approving the changes.
>
> Also, the changes you made to gdb.reverse/watch-reverse.exp should also
> be made on gdb.reverse/watch-precsave.exp, since it has the same problem
> that you identified with watch-reverse.

We also discussed this on IRC, it doesn't work for watch-precsave.exp because
restored recordings are handled like corefile targets, and there hardware watchpoints
are always disabled.
But I can mention this in the commit message as well.


Regards
Hannes