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

Pedro Alves
2012-01-02T19:14:47Z

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