Re: [RFA 2] Debug register support in win32-nat.c (need opinions)

[Christopher Faylor <cgf@redhat.com>]

On Mon, Jan 14, 2002 at 12:33:26AM +0100, muller@cerbere.u-strasbg.fr wrote:
>At 13:39 13/01/02 -0500, Christopher Faylor wrote:
>>I applied this patch but it doesn't seem to build.  I get a:
>>libgdb.a(win32-nat.o): In function `child_mourn_inferior':
>>/cygnus/src/uberbaum/gdb/win32-nat.c:1398: undefined reference to `_i386_cleanup_dregs'
>
>Did I forget to incorporate the change in config/i386/cygwin.mh ?
>No, I rechecked the reference patch and it is included.
>
>Did you rerun configure in gdb build directory?

I did reconfigure, yes.  However, I just wiped out my build directory
and reconfigured.  It built now.  Sorry for the false alarm.

>>- In do_initial_child_stuff, I'd prefer that you either use sizeof to
>>  derive the size of the dr array for zeroing or use a defined constant,
>>  rather than just a raw "7".
>
>OK, so I should rewrite it to 
>+  for (i = 0; i < sizeof (dr) / sizeof (dr[0]); i++)
>+    dr[i] = 0;
>   
>Is that correct?  (Please excuse me to still ask so basic C questions,
>but I never used C outside GDB code itself...)

That's ok, yes.  You could write a simple test case to convince yourself
of this, if you wanted.

>>- I'm wondering if your implementation is thread safe?  You're storing
>>  debug registers in a global array and copying them into a structure
>>  as needed.  Couldn't they just be stored in the per-thread structure?
>>  You could add a debug_registers_used value to the structure, if necessary.
>
>The basic idea of my implementation is that 
>watchpoints should be triggered by an expression that is modifed by
>any thread, and thus the same value should be written to 
>all threads. 
>This means that:
> - we only need one copy of the debug registers.
> - we need to write their value to all threads
>(including newly created ones, which is the major difference between
>this patch respective to the first patch).  - this of course assumes
>that the debuggee does not itself change its debug registers, in that
>case the patch will not work correctly, but I think that this is a
>reasonable limitation.
>
>I agree that the linux implementation does not set the debug registers
>for all threads but this means that if a watched expression is modified
>by another thread than the current thread at the time of setting the
>watchpoint will not be caught and that is much worse...

You described this in your original email.  I should have responded to
it.

I don't think it makes sense to make gratuitous changes to the way gdb
works.  If you're implementing an improvement for gdb for Windows then
I think it should probably work the same way for Windows as it does
for linux.

I guess I need a ruling from more experienced maintainers about this.

How should gdb behave in this scenario?

cgf