Re: read watchpoints ignored?

[Daniel Jacobowitz <drow at false dot org>]

On Mon, Nov 14, 2005 at 12:38:46AM +0200, Eli Zaretskii wrote:
> > I can reproduce this on Debian/unstable.
> 
> So can I, as I wrote in my reply.

Eli, why do you sound so offended?  The first part of your message was
a question about why GDB behaved a particular way for you, completely
independent of the original problem.  I tried my best to explain.  My
questions were based on the knowledge that this depends heavily on:
  - The version of GDB
  - The code generated for the function
  - The debug info generated by your compiler.

You reproduced it on a Debian system at least a year out of date.  I
have learned painfully over time that trying to generalize from the
behavior of one toolchain to the behavior of another is not time well
spent.  So I asked for some more information.  Between then, Debian
switched from GCC3 to GCC4.

Anyway, it doesn't matter.  Moving on.

> > > So I'm guessing that the problem happens because GDB misses the data
> > > write into a in line 7, and thus doesn't take notice that a was
> > > assigned the value 10.  That's why it gets confused when "b = a;"
> > > reads from a.
> > 
> > It misses it, but shouldn't.  When GDB single steps over the
> > temporarily removed breakpoint at "main",
>                       ^^^^^^^^^^
> Did you mean ``watchpoint''?  If not, I'm afraid I don't follow.

No, I mean breakpoint.  This is why I wanted to see the disassembly of
the function for you.

I'm talking about the breakpoint you set at main.  It's only while
single-stepping over that breakpoint, in order to "continue", that this
problem will manifest.  The breakpoint was set on the same instruction
which stored to "a", i.e. would have triggered the access watchpoint.

> >                                           it is not updating the values
> > of watchpoints before resuming the instruction.  That instruction
> > happened to modify the value behind a read watchpoint.
> 
> Then that's the bug, I'd say.  Do you agree that this is what we
> should fix to solve this bug?  Because the following discussion of
> problems with read watchpoints is only remotely related to the bug
> reported by Vladimir.

Yes, certainly this is the bug, I just don't know which way to fix it.

> > Presumably because trap_expected is set, infrun never calls
> > bpstat_stop_status, so the watchpoint is never checked.  Maybe we
> > should separate that code out into a separate function, or maybe we
> > should call it more rigorously.
> 
> Sorry, I don't see how the suggestions in your second sentence would
> solve the problem identified in the first.  Could you please explain?

I'm talking about watchpoint_check.  If that had been called, we would
have updated the value stored in the watchpoint for "a", and at the
next read watchpoint, it would not appear to have changed.

> > It looks like we _always_ ignore changed values for
> > bp_read_watchpoint, which means that the core of GDB does not work
> > on targets which support true read watchpoints.  Which IIRC includes
> > S/390, which disabled them for this reason.
> 
> We've been through this: it's inaccurate to say that ``GDB does not
> work on targets which support true read watchpoints.''  A more
> accurate statement would be ``on targets which support true read
> watchpoints, GDB does not announce read watchpoints when the watched
> value changes,'' which is quite a different story, and a fairly rare
> case.
> 
> If someone finds a way to announce a read watchpoint when the value
> changed on a target that supports that, without losing support for
> read watchpoints on x86 (which is by far the most popular platform
> these days), I'll gladly agree to such a change.  IIRC, until now no
> one did find how to do that.  I object to fixing read watchpoints in
> such rare situations (i.e., when the data is only read, not written,
> but the value still changes as a side effect) at a price of losing
> read watchpoints on x86.  The solution proposed by Vladimir would do
> precisely that, so I don't think we should accept it.

I didn't suggest that we should.

I also happen to disagree with you about the severity of this problem.
Fortunately any platform with read watchpoints probably also has access
watchpoints, so we can get by with using those instead.  But the first
read after the value changes has a high chance of being the important
one while debugging.

> > This should be controlled by an architecture method indicating that
> > there are only access watchpoints.
> 
> And then what? disable `rwatch'?  I object to such ``solution'', since
> the emulated read watchpoints proved to be most useful to me since the
> code we discuss was introduced.

I thought this was fairly straightforward, so I didn't go into detail.
Certainly there's no reason to disable rwatch.  Conditionalize "skip
this watchpoint if the value has changed" on "this is really an access
watchpoint because the target does not support read watchpoints".

It should be trivial to fix if you had a platform with read watchpoints
handy.  Which I don't easily.

-- 
Daniel Jacobowitz
CodeSourcery, LLC