Re: [RFA]: Watchpoints per thread patch

Jeff Johnston
2004-11-09T20:48:44Z

Daniel Jacobowitz wrote:
> On Tue, Nov 09, 2004 at 02:06:07PM -0500, Jeff Johnston wrote:
>
>>Time out here for a second. I have been modifying this patch according to
>>"your" comments. I have had a design that had no observers and one that
>>kept the observation isolated to the linux code.
>
>
> The design without observers had plenty of other problems, e.g. it also
> broke remote debugging.
>
> My suggestion about putting the observer in add_thread was a bad one.
> I've never claimed to be an infallible lord of development. A
> new_thread observer does indeed belong in add_thread, but is not
> suitable for your use; and I didn't understand why until later.
>

Fair enough, but it was certainly uncalled for to categorize my patch(s) as
"crap" as I was attempting to implement your suggested changes.

> We could use a Linux native specific observer, or handle this through
> the target stack. I think handling it through the target stack makes
> more sense, but I haven't sketched out what the target method would
> look like. If other GDB developers think that the precedent of a
> native-code-only observer isn't a bad one, then maybe we should go back
> to your previous placement of the observer and give it a Linux specific
> name. This will be aided by renaming thread-db to be clearly Linux
> native code.
>

Ok. I'll wait and see if anybody else has objections. Eli, I understand that I
would have to rename the observer and change its definition appropriately.

>
>>One key issue of my latest patch you seem to object to is the fact that I
>>now have to massage the ptid. This was not necessary in the previous
>>design where I was observing within the linux code where the lwp was
>>readily available. We both know the low-level code is fundamentally wrong
>>in its assumption regarding the ptids. They cannot be assumed to be in
>>PID, LWP, 0 format. We get lucky with register accesses only because the
>>thread-db code is flushing registers in the lwp format. It is not
>>documented and when low-level code accesses ptids outside of thread-db, it
>>is wrong. Watchpoints are in the this boat because they are accessed by
>>breakpoint.c and infrun.c where the ptid is in the wrong format (PID, 0,
>>TID).
>>
>>I feel your objection to temporarily massaging these ptids as thread-db.c
>>does is unreasonable. We need to think of the end-user. The amount of
>>code added is small and it is trivial to remove this code once the
>>preferred solution is in place. There is currently no work-around to
>>solving thread bugs involving memory corruption.
>>
>>If you have a fix planned soon regarding the ptid format, I have absolutely
>>no objection to waiting for it. However, if you can't get around to this
>>for a while due to other commitments or it is going to take some hashing
>>out on the list, let's stop punishing the end-user and do something helpful
>>while we work things out proper.
>
>
> Jeff, I've already posted the thread-db change to make this
> unnecessary. I was asked to rename the file first, and I've posted
> that too.
>

Ok, my misunderstanding. Like I said, if something is currently in the works, I
have no problem in waiting.

-- Jeff J.