Daniel Jacobowitz wrote:
> On Thu, Dec 09, 2004 at 06:36:13PM -0500, Jeff Johnston wrote:
> 
>>The following is a modified version of my thread watchpoint patch from 
>>October/November.  It removes the code I had used to switch between lwp 
>>ptids and thread ptids now that Daniel's lwp patch is in place.  It uses 
>>the former version of my observer that is linux-specific and is activated 
>>in attach_thread in linux-thread-db.c.  Eli, I renamed the observer as 
>>asked to indicate this.
>>
>>I also addressed Ulrich's comments regarding simplifying the S390 code and 
>>using the s390_fix_watch_points call to actually put the watchpoints on the 
>>new thread.
>>
>>Ulrich/Daniel can you take a look to verify everything is in place.  
>>Daniel, I realize that this touches files that are currently in patch state 
>>for you.  I have no problem waiting for your latest patch to apply and 
>>retrofitting my changes at check-in if necessary.
>>
>>As I mentioned before, more is required to get ia64 threaded watchpoints to 
>>work.  For S390, this change allows it to set and recognize threaded 
>>watchpoints.
> 
> 
> Two formatting comments: please replace "linux" in comments with
> "GNU/Linux", and please check copyright years on the modified files.
> 

Done.  I have reattached the code patch with these fixes.

> On the technical side, two questions:
> 
> 1) I can see that it will be a bit of work to rearrange i386-linux to
> use this, but it should be doable.  Do you know offhand of any
> i386-specific problems other than inserting watchpoints for all
> threads?
>

Actually, with i386/x86-64 I discovered that the debug registers are global in 
scope for the setting of watchpoints (i.e. I didn't have to use the observer). 
The status register, however, is thread-specific for reporting them.  I have 
gotten the watchthreads.exp testcase working for both platforms.  Your lwp fix 
helps a lot with this.  We call TIDGET()/PIDGET() in the low-level code which 
used to get called in the wrong ptid mode so we kept checking the main-thread 
for the watchpoint.

> 2) What should to_stopped_by_watchpoint do in the presence of multiple
> threads?  It looks like it relies on inferior_ptid being the thread
> which stopped at a watchpoint; I'm worried that that may not be
> consistently true in a heavily threaded application.  Maybe it should
> iterate over all threads.
>

It works fine for the watchthreads.exp test once all the mechanisms are in place 
(I have a few more patches to go).  We don't want to iterate over all threads 
unless we know the platform has a problem.  Otherwise, we won't be able to pin 
down a specific watchpoint triggered with the thread/source line that triggered 
it.  Is there a valid scenario where inferior_ptid should not be the thread for 
the signal chosen by the low-level linux-nat code?  If not, I would prefer to 
treat that as a bug that requires pinning down.

> The to_stopped_data_address has its own problems with threads; but the
> case of handling hitting two watchpoints at once, I think, we can leave
> for another day.
> 

The multiple watchpoint scenario occurs in watchthreads.exp.  For ia64, it is a 
real pain which I will describe in my next patch.

> This is looking very good so far!
> 

Thanks.

-- Jeff J.