This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: x86 watchpoints bug (Re: ping: Re: PATCH : allow to set length of hw watchpoints (e.g. for Valgrind gdbserver))


Here it is. New test, fixes for both gdbserver and gdb.
....
Any comments on this?

--
Pedro Alves

2011-06-01 Pedro Alves <pedro@codesourcery.com>


I think there is still something wrong with the DR registers, at least
with gdbserver.

First it looks like addr 0 is special, and keeps the register
busy. See below the trace of first the gdb session,
followed by the gdbserver session.

Reading symbols from /home/philippe/set-length-limit/s...done.
(gdb) tar rem :1234
Remote debugging using :1234
Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
Loaded symbols for /lib64/ld-linux-x86-64.so.2
0x00007ffff7de2a60 in ?? () from /lib64/ld-linux-x86-64.so.2
(gdb) set breakpoint always-inserted on
(gdb)  mo set debug-hw-points 1
H/W point debugging output enabled.
(gdb) watch * (int *) 0
Hardware watchpoint 1: * (int *) 0
(gdb) watch * (long *) 0
Hardware watchpoint 2: * (long *) 0
(gdb) watch * (int *) 8
Hardware watchpoint 3: * (int *) 8
(gdb) watch * (long *) 8
Hardware watchpoint 4: * (long *) 8
(gdb) del
Delete all breakpoints? (y or n) y
(gdb)


philippe@gcc10:~/set-length-limit/cvs/dr_busy_fix$ ./install/bin/gdbserver :1234 /home/philippe/set-length-limit/s Process /home/philippe/set-length-limit/s created; pid = 20075 Listening on port 1234 Remote debugging from host 127.0.0.1 insert_watchpoint (addr=0, len=4, type=data-write): CONTROL (DR7): 000d0101 STATUS (DR6): 00000000 DR0: addr=0x0, ref.count=1 DR1: addr=0x0, ref.count=0 DR2: addr=0x0, ref.count=0 DR3: addr=0x0, ref.count=0 insert_watchpoint (addr=0, len=8, type=data-write): CONTROL (DR7): 009d0105 STATUS (DR6): 00000000 DR0: addr=0x0, ref.count=1 DR1: addr=0x0, ref.count=1 DR2: addr=0x0, ref.count=0 DR3: addr=0x0, ref.count=0 insert_watchpoint (addr=8, len=4, type=data-write): CONTROL (DR7): 0d9d0115 STATUS (DR6): 00000000 DR0: addr=0x0, ref.count=1 DR1: addr=0x0, ref.count=1 DR2: addr=0x8, ref.count=1 DR3: addr=0x0, ref.count=0 insert_watchpoint (addr=8, len=8, type=data-write): CONTROL (DR7): 9d9d0155 STATUS (DR6): 00000000 DR0: addr=0x0, ref.count=1 DR1: addr=0x0, ref.count=1 DR2: addr=0x8, ref.count=1 DR3: addr=0x8, ref.count=1 remove_watchpoint (addr=0, len=4, type=data-write): CONTROL (DR7): 9d9d0154 STATUS (DR6): 00000000 DR0: addr=0x0, ref.count=1 DR1: addr=0x0, ref.count=1 DR2: addr=0x8, ref.count=1 DR3: addr=0x8, ref.count=1 remove_watchpoint (addr=0, len=8, type=data-write): CONTROL (DR7): 9d9d0150 STATUS (DR6): 00000000 DR0: addr=0x0, ref.count=1 DR1: addr=0x0, ref.count=1 DR2: addr=0x8, ref.count=1 DR3: addr=0x8, ref.count=1 remove_watchpoint (addr=8, len=4, type=data-write): CONTROL (DR7): 9d9d0140 STATUS (DR6): 00000000 DR0: addr=0x0, ref.count=1 DR1: addr=0x0, ref.count=1 DR2: addr=0x0, ref.count=0 DR3: addr=0x8, ref.count=1 remove_watchpoint (addr=8, len=8, type=data-write): CONTROL (DR7): 9d9d0100 STATUS (DR6): 00000000 DR0: addr=0x0, ref.count=1 DR1: addr=0x0, ref.count=1 DR2: addr=0x0, ref.count=0 DR3: addr=0x0, ref.count=0

I suspect the problem might be in the following piece of code:
static void
update_inferior (struct i386_debug_reg_state *inf_state,
  struct i386_debug_reg_state *new_state)
{
 int i;

 ALL_DEBUG_REGISTERS (i)
   {
     if (new_state->dr_mirror[i] != inf_state->dr_mirror[i]
  || (new_state->dr_ref_count[i] != 0
      && inf_state->dr_ref_count[i] == 0))
{

The dr_mirror is the address being watched.
But if address being watched is 0x0, then a 'busy' register
watching 0x0 and a non-busy register will have equal dr_mirror.
Then the || condition is bizarre as the ref.count will be updated
only if the current inf_state ref.count is 0.
It looks to me it would be clearer (and correct?) to always also
enter in the block updating really the inferior
if the dr_ref_counts are != (similarly to the dr_mirror).


After that, I have done some tests combining the patch fixing the DR busy and the patch that allows to set the length of remote hw watchpoints. This allows to test gdbserver when it has to share low level watchpoints. It looks like in this case, gdbserver does not properly maintain the ref.count: (!!!! so the below is with a gdb which includes the two patches, so that a watchpoint of 16 bytes is accepted by gdb and transmitted to gdbserver).


philippe@gcc10:~/set-length-limit/cvs/dr_busy_fix_and_set_length_from_src$ ./install/bin/gdb ~/set-length-limit/s GNU gdb (GDB) 7.3.50.20110608-cvs Copyright (C) 2011 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-unknown-linux-gnu". For bug reporting instructions, please see: <http://www.gnu.org/software/gdb/bugs/>... Reading symbols from /home/philippe/set-length-limit/s...done. (gdb) tar rem :1234 Remote debugging using :1234 Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done. Loaded symbols for /lib64/ld-linux-x86-64.so.2 0x00007ffff7de2a60 in ?? () from /lib64/ld-linux-x86-64.so.2 (gdb) set breakpoint always-inserted on (gdb) mo set debug-hw-points 1 H/W point debugging output enabled. (gdb) watch * (long *) 0x0 Hardware watchpoint 1: * (long *) 0x0 (gdb) watch * (char (*)[16]) 0x0 Hardware watchpoint 2: * (char (*)[16]) 0x0 (gdb) del Delete all breakpoints? (y or n) y (gdb)

philippe@gcc10:~/set-length-limit/cvs/dr_busy_fix_and_set_length_from_src$ ./install/bin/gdbserver :1234 /home/philippe/set-length-limit/s
Process /home/philippe/set-length-limit/s created; pid = 7504
Listening on port 1234
Remote debugging from host 127.0.0.1
insert_watchpoint (addr=0, len=8, type=data-write):
CONTROL (DR7): 00090101 STATUS (DR6): 00000000
DR0: addr=0x0, ref.count=1 DR1: addr=0x0, ref.count=0
DR2: addr=0x0, ref.count=0 DR3: addr=0x0, ref.count=0
insert_watchpoint (addr=0, len=16, type=data-write):
CONTROL (DR7): 00990105 STATUS (DR6): 00000000
DR0: addr=0x0, ref.count=1 DR1: addr=0x8, ref.count=1
DR2: addr=0x0, ref.count=0 DR3: addr=0x0, ref.count=0
remove_watchpoint (addr=0, len=8, type=data-write):
CONTROL (DR7): 00990104 STATUS (DR6): 00000000
DR0: addr=0x0, ref.count=1 DR1: addr=0x8, ref.count=1
DR2: addr=0x0, ref.count=0 DR3: addr=0x0, ref.count=0
remove_watchpoint (addr=0, len=16, type=data-write):
CONTROL (DR7): 00990104 STATUS (DR6): 00000000
DR0: addr=0x0, ref.count=1 DR1: addr=0x8, ref.count=1
DR2: addr=0x0, ref.count=0 DR3: addr=0x0, ref.count=0



I double checked that I was testing with the gdbserver containing the fix by attaching to gdbserver with another gdb, and putting a break in the function update_inferior: (gdb) attach 7499 Attaching to process 7499 Reading symbols from /home/philippe/set-length-limit/cvs/dr_busy_fix_and_set_length_from_src/install/bin/gdbserver...done. Reading symbols from /lib/libdl.so.2...(no debugging symbols found)...done. Loaded symbols for /lib/libdl.so.2 Reading symbols from /lib/libc.so.6...(no debugging symbols found)...done. Loaded symbols for /lib/libc.so.6 Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done. Loaded symbols for /lib64/ld-linux-x86-64.so.2 0x00007ffff7953d03 in select () from /lib/libc.so.6 (gdb) break update_inferior Breakpoint 1 at 0x422010: file i386-low.c, line 441. (gdb)

We see that the gdbserver is properly re-using a low level watchpoint
for a 2nd high level watchpoint, but the ref.count is kept to 1,
while it should go to 2.

We have similar ref.count problem with an address != 0:

philippe@gcc10:~/set-length-limit/cvs/dr_busy_fix_and_set_length_from_src$ ./install/bin/gdb ~/set-length-limit/s
GNU gdb (GDB) 7.3.50.20110608-cvs
Copyright (C) 2011 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-unknown-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /home/philippe/set-length-limit/s...done.
(gdb) tar rem :1234
Remote debugging using :1234
Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
Loaded symbols for /lib64/ld-linux-x86-64.so.2
0x00007ffff7de2a60 in ?? () from /lib64/ld-linux-x86-64.so.2
(gdb) set breakpoint always-inserted on
(gdb) mo set debug-hw-points 1
H/W point debugging output enabled.
(gdb) watch * (long *) 0x8
Hardware watchpoint 1: * (long *) 0x8
(gdb) watch * (char (*)[16]) 0x8
Hardware watchpoint 2: * (char (*)[16]) 0x8
(gdb) del
Delete all breakpoints? (y or n) y
(gdb)


Process /home/philippe/set-length-limit/s created; pid = 9959 Listening on port 1234 Remote debugging from host 127.0.0.1 insert_watchpoint (addr=8, len=8, type=data-write): CONTROL (DR7): 00090101 STATUS (DR6): 00000000 DR0: addr=0x8, ref.count=1 DR1: addr=0x0, ref.count=0 DR2: addr=0x0, ref.count=0 DR3: addr=0x0, ref.count=0 insert_watchpoint (addr=8, len=16, type=data-write): CONTROL (DR7): 00990105 STATUS (DR6): 00000000 DR0: addr=0x8, ref.count=1 DR1: addr=0x10, ref.count=1 DR2: addr=0x0, ref.count=0 DR3: addr=0x0, ref.count=0 remove_watchpoint (addr=8, len=8, type=data-write): CONTROL (DR7): 00990104 STATUS (DR6): 00000000 DR0: addr=0x0, ref.count=0 DR1: addr=0x10, ref.count=1 DR2: addr=0x0, ref.count=0 DR3: addr=0x0, ref.count=0 remove_watchpoint (addr=8, len=16, type=data-write): CONTROL (DR7): 00990104 STATUS (DR6): 00000000 DR0: addr=0x0, ref.count=0 DR1: addr=0x10, ref.count=1 DR2: addr=0x0, ref.count=0 DR3: addr=0x0, ref.count=0

Maybe it would be possible to add some asserts in the handling
of the DR ? E.g. if the ref.count > 0, then the register must be 'active'
and if ref.count == 0, then register must be 'inactive' ?

Finally, a question about the below comment from a new test.
If I understood correctly, the 'single (low level)' should rather be 'single (high level)' ?

+# Regression test for a bug where the i386 watchpoints support backend
+# would leave some debug registers occupied even if not enough debug
+# registers were available to cover a single (low level) watchpoint.


Philippe



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]