Re: [PATCH]: Make Linux use the new unified x86 watchpoint support

[Michael Snyder <msnyder@redhat.com>]

Mark Kettenis wrote:
> 
>    Date: Tue, 27 Mar 2001 08:55:00 +0200 (IST)
>    From: Eli Zaretskii <eliz@is.elta.co.il>
> 
>    On Mon, 26 Mar 2001, Michael Snyder wrote:
> 
>    > Guys, this implementation has problems.  You have it hard-coded
>    > so that on a linux host, it unconditionally calls native linux
>    > methods involving ptrace to get the debug registers.  This breaks
>    > very badly if you're using a native linux host to debug a remote
>    > i386 target.
> 
> Just like the old implementation.  I suspect the old implementation
> just happened to work because it didn't do any strict error checking.

No, you're on the wrong track.  I haven't looked at how the old
implementation worked, but the problem with the new implementation
is _not_ the perror call.  It's the ptrace call that preceeds it.
You can't call ptrace if the process you're debugging is remote.

 
> The fundamental problem is that the watpoint-stuff isn't part of the
> target vector.

Yes, you're absolutely right about this.  This is why I don't
like ptrace calls being made from outside of the target vector.

 
>    Sorry, I'm not following.  The watchpoint-related macros are defined
>    on files which should be used only with native debugging (i386-nat.c
>    and nm-i386.h).  On top of that, the macros only get exposed if the
>    port defines I386_USE_GENERIC_WATCHPOINTS.  A port which doesn't want
>    that should get rid of the watchpoints for free, by simply not
>    defining I386_USE_GENERIC_WATCHPOINTS.
> 
>    Which part of the above misfires, and why?
> 
> It used to be possible to debug a remote i386 target with a native
> Linux/x86 debugger.
> 
>    > Seems to me, what you need to do is add these debug registers to the
>    > reg cache, and treat them like ordinary registers.
> 
>    This possibility has been discussed back in November, but the
>    conclusion was that it's not a good idea.  I don't remember the
>    details, but the reasons had something to do with threads and how
>    the register cache is used in conjunction with threads.  (I can dig
>    out the URLs of the relevant messages, if you want to read them.)
> 
> I suggested doing this, but several people objected to exposing the
> debug registers in this way.  Threads have nothing to do with it
> (actually doing the correct thing for threads would make it easier if
> the debug registers would be part of the register cache).

Yes.  But it's OK, 'cause breakpoints don't really work on a
per-thread basis either.  The process will stop no matter which
thread hits the breakpoint, and then GDB will decide whether 
the breakpoint was hit by the thread-of-interest.  Can do the
same for watchpoints.

>    > Then you can just use the ordinary read_register interface to get
>    > them, and remote.c will do the right thing for you (assuming the
>    > target knows about these extra registers).
> 
>    Mark explicitly didn't want the watchpoint code to be in the
>    target-depenent files, so watchpoints cannot currently work for remote
>    targets.
> 
> Actually, our problems would only be bigger if the watchpoint code was
> part of the target-dependent files.  But if we add the debug registers
> to the register cache and add the watchpoint stuff to the target
> vector, only a few changes are necessary to make the watchpoint code
> ready for a move to i386-tdep.c.
> 
> Mark