RE: Patch for gdb5.0; enable hardware watchpoints on UnixWare

[Eli Zaretskii <eliz@delorie.com>]

>   From: "John Hughes" <john@Calva.COM>
>   Date: Tue, 31 Oct 2000 12:20:13 +0100
> 
>   could you steal the svr42mp specific
>   bits of it for 5.1?  (debug registers held in pr_family element of
>   proc status, PCSDEBUG operation to write debug registers).

I will try, but I cannot promise this.  What I'm trying to do is to
write generic code for managing the x86 debug registers, and provide
hooks for target-specific code to use that.  People who maintain
specific targets will then have to revisit their current
implementations and rework them so that they use the code I write.

I _will_ try to make the generic code general enough to be easily
usable by all targets which already have watchpoint support.  In
particular, I will take a look at your code (which I already stashed
away for my reference), to make sure svr42mp will fit into that
framework.

>   1. Why not zap the waddr arg to go32_..._watchpoint?  It's not used.

It might be there because GDB needs that argument for some other
(non-x86) target.  I will investigate this when I come to finalizing
the API.

>   2. In go32_insert_aligned_watchpoint (and ...remove_aligned...) you
>      have a "switch" to work out the len_bits for the watchpoint, then a
>      loop to find an existing watchpoint that might match, then an
>      "if" to check for non-aligned watchpoints.  (Which could not have
>      matched in the loop).

I think you are looking at an old version of go32-nat.c.  The
functions in version one I have have a different structure, so most of
your comments do not apply.  Thanks anyway, I will make a good use of
those which do apply ;-)

>   3. You're not using the DR_FIRSADDR/DR_LASTADDR defines.

Sorry, I'm not sure what defines are you talking about, and what are
DR_FIRSTADDR and DR_LASTADDR in the first place.

In general, the underlying system calls used by DJGPP ignore some of
the bits of the debug registers, so the code might not use them
correctly, or not at all.  While I generalize the code, I will
bring it up to date with the letter of Intel's manuals.

>   4. You have a define DR_RW_READWRITE, other people (SVR3/SVR4/SVR5) seem
>      to have DR_RW_READ with the same value (0x3).

READWRITE is correct, I've just checked this with the Intel manuals.
READ is 0x2.  It is possible that the systems which define READ to be
0x3 have stale code, dating back to 386/486 CPUs, where 0x2 is
undefined.

>   5. Not everybody has DR_CONTROL_MASK.
>
>	   #define DR_CONTROL_MASK ((1<<DR_CONTROL_SIZE) - 1)

That's just a #define, isn't it?  I can always define it, since the
underlying functionality (the bits extracted by this mask) are
supported, right?  Or do I miss something?

Thanks again for your comments.
From msalter@redhat.com Tue Oct 31 05:15:00 2000
From: Mark Salter <msalter@redhat.com>
To: eliz@is.elta.co.il
Cc: jtc@redback.com, gdb-patches@sources.redhat.com
Subject: Re: Remote watchpoint support.
Date: Tue, 31 Oct 2000 05:15:00 -0000
Message-id: <200010311317.e9VDHpw08879@deneb.localdomain>
References: <200010301416.e9UEG6m18981@deneb.localdomain> <5mg0leoyg6.fsf@jtc.redback.com> <200010310906.EAA21820@indy.delorie.com>
X-SW-Source: 2000-10/msg00298.html
Content-length: 714

>>>>> Eli Zaretskii writes:

>> From: jtc@redback.com (J.T. Conklin)
>> Date: 30 Oct 2000 12:39:53 -0800
>> 
>> One thing I dislike about it is that the debug agent never explicitly
>> tells GDB that a watchpoint has been hit, but rather it is implicitly
>> coded in the return value of the query.

> I agree that this is not a good design.

Ok. It seems pretty clear that there is consensus on getting the watchpoint
data address (and possibly other info) when the target stops and not through
a separate query packet. I will try to work up a patch today that does this.
I will probably limit it to the 'T' packet for now. The 'T' packet is already
documented as being extendible for this sort of thing.

--Mark