This is the mail archive of the
mailing list for the GDB project.
Re: [RFA 2] Debug register support in win32-nat.c
- From: muller at cerbere dot u-strasbg dot fr
- To: gdb-patches at sources dot redhat dot com
- Date: Mon, 14 Jan 2002 00:33:26 +0100
- Subject: Re: [RFA 2] Debug register support in win32-nat.c
- References: <firstname.lastname@example.org>
At 13:39 13/01/02 -0500, Christopher Faylor wrote:
>On Tue, Jan 08, 2002 at 10:26:12AM +0100, Pierre Muller wrote:
>>At 10:20 08/01/2002 , vous avez ?crit:
>>> This is a follow up of my first
>>>proposal for win32 debug register support
>>>(which enables hardware watchpoints,
>>>I never tested the hardware breakpoints, as they don't have
>>>much advantages over normal breakpoints on i386 processors).
>>Sorry, I forgot to add the link
>>and all the follow-ups.
>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
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?
This is necessary as the nm.h link
is changed by my patch to cygwin.mh,
but might be missed by the Makefile itself.
Run ../../src/gdb/configure in build/gdb
if src and build dirs are at same dir level should suppress
>I assume that this is related to your other patch.
>In the meantime, I noticed a couple of things:
>- ChangeLog needs to be wrapped to 80 columns.
>- ChangeLog wording needs more verbs and more description. For instance:
> (debug_registers_changed): Non zero whenever the debug registers
where changed and
> need to be written to inferior.
> You need to mention that this is a new variable:
> (debug_registers_changed): New variable. Reflects when debug
registers are changed and
> need to be written to inferior.
>- 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); 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...)
>- 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
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
If Christopher is able to build and test my patch
after a rerun of the configure scriptand if
he agrees with my remarks above, I will be happy
to resubmit a third patch taking his remarks into account.
Thanks for reviewing my patch, Christopher.