[patch 0/4] hw watchpoints across fork() + multi-inf

[patch 0/4] hw watchpoints across fork() + multi-inf

[Jan Kratochvil]

Hi,

this is an Archer branch
	archer-jankratochvil-watchpoint3
being posted here since 2007 and (different impl.) present in Fedora:
	[patch] Fix disarmed hw watchpoints after inferior's fork()
	http://sourceware.org/ml/gdb-patches/2007-10/msg00367.html
	http://sourceware.org/ml/gdb-patches/2007-11/msg00454.html
	http://sourceware.org/ml/gdb-patches/2008-01/msg00042.html
	[patch 4/4] Fix hw watchpoints: across fork()
	http://sourceware.org/ml/gdb-patches/2009-08/msg00258.html
	http://sourceware.org/ml/gdb-patches/2009-11/msg00353.html

problem #1: Disarmed watchpoints - watchpoints are not caught after fork().

problem #2: Leftover watchpoints - there will be a core file from the child.
  (on very recent Linux kernels it no longer crashes - see [patch 3/4])

echo 'int v;main(){fork();v++;}'|gcc -g -x c -;(ulimit -c unlimited;./gdb -nx ./a.out -ex start -ex 'watch v' -ex c -ex q)

Only (part of) the main testcase remained; the code did not expect
multi-inferior and for multi-inferior it would have to be reimplemented later
anyway.  Therefore at least this patch also fixes hw watchpoints to be
multi-inferior capable/compatible.

Currently the hw watchpoints are made specific to one inferior.
There is no way to create a single watchpoint across-all-inferiors.
Currently they were set globally but they SIGTRAP various way in
multi-inferior mode.  More of this per-inferior differentiation is probably
in the scope of Tom Tromey's multi-inferior breakpoints work.

The hw watchpoints are now tracked per-PID.  While one can create
breakpoints/watchpoints also per-TID (by the "thread" keyword) this is
currently (inefficiently) implemented only by GDB evaluation while the
breakpoints/watchpoints are still global per-PID, for all TIDs.
I do not try to target this performance optimization in this patch.

This is only linux-nat fix; gdbserver FAILs on these testcases now.

Still remains a problem of hw watchpoints being silently ignored in some
virtual machines, at least:
 * qemu-system-x86_64 0.9.1-6.fc9.x86_64
 * qemu-kvm kvm-65-7.fc9.x86_64 + kernel-2.6.25.9-76.fc9.x86_64.
There should be a runtime test to verify the watchpoints work, like linux-nat.c
has already some other runtime tests.  But that would be an unrelated patch.

The ppc part is only tested on non-BookE (=DABR) machine (RHEL-6).  It fixes
there some new testcases but still many `set follow-fork-mode child' FAIL but I
find it as an unrelated problem:
	Error in re-setting breakpoint 1: reading register r31 (#31): No such process.^M

No regressions on {x86_64,x86_64-m32,i686}-fedora14-linux-gnu.


Thanks,
Jan

Re: [patch 0/4] hw watchpoints across fork() + multi-inf

[Pedro Alves]

On Monday 06 December 2010 11:11:35, Jan Kratochvil wrote:
> Hi,
> 
> this is an Archer branch
> 	archer-jankratochvil-watchpoint3
> being posted here since 2007 and (different impl.) present in Fedora:
> 	[patch] Fix disarmed hw watchpoints after inferior's fork()
> 	http://sourceware.org/ml/gdb-patches/2007-10/msg00367.html
> 	http://sourceware.org/ml/gdb-patches/2007-11/msg00454.html
> 	http://sourceware.org/ml/gdb-patches/2008-01/msg00042.html
> 	[patch 4/4] Fix hw watchpoints: across fork()
> 	http://sourceware.org/ml/gdb-patches/2009-08/msg00258.html
> 	http://sourceware.org/ml/gdb-patches/2009-11/msg00353.html
> 
> problem #1: Disarmed watchpoints - watchpoints are not caught after fork().
> 
> problem #2: Leftover watchpoints - there will be a core file from the child.
>   (on very recent Linux kernels it no longer crashes - see [patch 3/4])
> 
> echo 'int v;main(){fork();v++;}'|gcc -g -x c -;(ulimit -c unlimited;./gdb -nx ./a.out -ex start -ex 'watch v' -ex c -ex q)
> 
> Only (part of) the main testcase remained; the code did not expect
> multi-inferior and for multi-inferior it would have to be reimplemented later
> anyway.  Therefore at least this patch also fixes hw watchpoints to be
> multi-inferior capable/compatible.
> 
> Currently the hw watchpoints are made specific to one inferior.
> There is no way to create a single watchpoint across-all-inferiors.
> Currently they were set globally but they SIGTRAP various way in
> multi-inferior mode.  More of this per-inferior differentiation is probably
> in the scope of Tom Tromey's multi-inferior breakpoints work.
> 
> The hw watchpoints are now tracked per-PID.  While one can create
> breakpoints/watchpoints also per-TID (by the "thread" keyword) this is
> currently (inefficiently) implemented only by GDB evaluation while the
> breakpoints/watchpoints are still global per-PID, for all TIDs.
> I do not try to target this performance optimization in this patch.
> 
> This is only linux-nat fix; gdbserver FAILs on these testcases now.
> 
> Still remains a problem of hw watchpoints being silently ignored in some
> virtual machines, at least:
>  * qemu-system-x86_64 0.9.1-6.fc9.x86_64
>  * qemu-kvm kvm-65-7.fc9.x86_64 + kernel-2.6.25.9-76.fc9.x86_64.
> There should be a runtime test to verify the watchpoints work, like linux-nat.c
> has already some other runtime tests.  But that would be an unrelated patch.
> 
> The ppc part is only tested on non-BookE (=DABR) machine (RHEL-6).  It fixes
> there some new testcases but still many `set follow-fork-mode child' FAIL but I
> find it as an unrelated problem:
> 	Error in re-setting breakpoint 1: reading register r31 (#31): No such process.^M
> 
> No regressions on {x86_64,x86_64-m32,i686}-fedora14-linux-gnu.

I'm trying to look at this series, and it looks like how gdbserver
has been made to support watchpoints was overlooked, and so we end up
with divergence on how things are implemented.  :-(

 - gdbserver already has a per-process structure for the debug registers,
   yet, your implementation is different, which makes it gratuitously harder
   to share and move code between the gdb and gdbserver implementations.

   (FYI, we've written patches that move shareable bits of
    gdb and gdbserver into common/, seeding the way to such duplication
    removal.  we should be posting them in the foreseeable future)

 - gdbserver gets away without the iteration over all threads setting the
   debug registers synchronously, which merged to native gdb, I think
   could get rid of some of the churn in your patches, I think, in addition
   to fixing watchpoints in non-stop mode (PR10729).

 - gdbserver already only installs watchpoints to in threads of a given
   process only.

What gdbserver doesn't do is follow forks, because there's no remote
protocol support defined for that yet.

-- 
Pedro Alves

Re: [patch 0/4] hw watchpoints across fork() + multi-inf

[Jan Kratochvil]

On Fri, 17 Dec 2010 22:24:41 +0100, Pedro Alves wrote:
> On Monday 06 December 2010 11:11:35, Jan Kratochvil wrote:
> > this is an Archer branch
> > 	archer-jankratochvil-watchpoint3
> > being posted here since 2007 and (different impl.) present in Fedora:
> > 	[patch] Fix disarmed hw watchpoints after inferior's fork()
> > 	http://sourceware.org/ml/gdb-patches/2007-10/msg00367.html
[...]
> I'm trying to look at this series, and it looks like how gdbserver
> has been made to support watchpoints was overlooked,

My per-TID implementation was posted in 2007.  I have even updated it
according to the review but it got unreplied (I did not ping it, though).

Doug Evan's multi-inferior implementation was posted first in 2009:
	http://sourceware.org/ml/gdb-patches/2009-04/msg00804.html

If we want to start talking about "overlooking" it happened the opposite way.
I do not mind wrt the patch, but I do not agree with your wording.


>  - gdbserver already has a per-process structure for the debug registers,
>    yet, your implementation is different, which makes it gratuitously harder
>    to share and move code between the gdb and gdbserver implementations.
> 
>    (FYI, we've written patches that move shareable bits of
>     gdb and gdbserver into common/, seeding the way to such duplication
>     removal.  we should be posting them in the foreseeable future)

What is the status of these existing common/ unifications?

Considering syncing back linux-nat to gdbserver (as Doug did not post it as
a two pieces series - (1) sync gdbserver to linux-nat (2) extend it in
gdbserver) first as a base for this patchset.  But I may create by such
linux-nat sync something already obsoleted by your existing non-public
patches.


Thanks,
Jan