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

[Pedro Alves <pedro at codesourcery dot com>]

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