Re: [patch 2/2] Fix watchpoints for multi-inferior

[Pedro Alves <alves.ped@gmail.com>]

On 01/02/2012 04:46 PM, Jan Kratochvil wrote:
> Hi,
>
> this is a new post of:
> 	[patch 4/4] hw watchpoints made multi-inferior
> 	http://sourceware.org/ml/gdb-patches/2010-12/msg00045.html
>
> Currently watchpoints somehow apply to all the inferiors but they do not work
> that way anyway.  I believe watchpoints should be specific for each inferior.

Yeah.

>   /* Get data specific for INFERIOR_PTID LWP.  Return special data area
>      for processes being detached.  */
>
>   static struct i386_inferior_data *
>   i386_inferior_data_get (void)
>   {
> -  /* Intermediate patch stub.  */
> -  static struct i386_inferior_data inf_data_local;
>     struct inferior *inf = current_inferior ();
> -  struct i386_inferior_data *inf_data =&inf_data_local;
> +  struct i386_inferior_data *inf_data;
> +
> +  inf_data = inferior_data (inf, i386_inferior_data);
> +  if (inf_data == NULL)
> +    {
> +      inf_data = xzalloc (sizeof (*inf_data));
> +      set_inferior_data (current_inferior (), i386_inferior_data, inf_data);
> +    }

Do we clear inf_data or inf_data's contents anywhere on inferior
exit or startup, so to not leave debug registers stale across runs?
(The cleanup only runs when the inferior is deleted.)


> --- /dev/null
> +++ b/gdb/testsuite/gdb.multi/watchpoint-multi.c
> @@ -0,0 +1,51 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2012 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see<http://www.gnu.org/licenses/>.  */
> +
> +#include<pthread.h>
> +#include<assert.h>
> +
> +static volatile int a, b, c;
> +
> +static void
> +marker_exit (void)
> +{
> +  a = 1;
> +}
> +
> +static void *
> +start (void *arg)
> +{
> +  b = 2;
> +  c = 3;
> +
> +  return NULL;
> +}
> +
> +int
> +main (void)
> +{
> +  pthread_t thread;
> +  int i;
> +
> +  i = pthread_create (&thread, NULL, start, NULL);
> +  assert (i == 0);
> +  i = pthread_join (thread, NULL);
> +  assert (i == 0);
> +
> +  marker_exit ();
> +  return 0;
> +}
> --- /dev/null
> +++ b/gdb/testsuite/gdb.multi/watchpoint-multi.exp
> @@ -0,0 +1,92 @@
> +# Copyright 2012 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see<http://www.gnu.org/licenses/>.
> +
> +if [is_remote target] {
> +    # It is KFAIL.
> +    continue

Did you mean to turn this into a real kfail?  What are the
gdbserver problems, btw?

> +
> +# Simulate non-stop+target-async which also uses breakpoint always-inserted.
> +gdb_test_no_output "set breakpoint always-inserted on"
> +# displaced-stepping is also needed as other GDB sometimes still removes the
> +# breakpoints, even with always-inserted on.
> +gdb_test_no_output "set displaced-stepping on"

'if ![support_displaced_stepping] bail' missing, as well as the usual
hw watchpoints support checks.

Otherwise looks okay.

-- 
Pedro Alves