Re: [RFC] remote-mips.c: Fix bit rot associated with the inferior's state

Pedro Alves
2010-03-02T23:37:57Z

On Tuesday 02 March 2010 23:09:20, Kevin Buettner wrote:
> Below is a slight revision over yesterday's patch. The only difference
> between this patch and the previous one is the addition of the change to
> mips_error(). That change avoids an internal error when inferior_ptid
> is the same as null_ptid.
>
> I'll wait a few more days for comments before checking this in.
>
> Kevin
>
> * remote-mips.c (gdbthread.h): Include.
> (mips_error): Only mourn the inferior when inferior_ptid is non-null.
> (magic_null_ptid): Declare.
> (common_open): Set inferior_ptid, add it as an inferior, and
> as a thread too. Set up the inferior's address spaces.
> (mips_kill): Make sure that target_mourn_inferior is invoked.
> (mips_load): Don't null out inferior_ptid.
> (_initialize_remote_mips): Initialize magic_null_ptid.

Looks mostly okay, but are you missing a target_thread_alive
implementation? See these:

http://sourceware.org/ml/gdb-patches/2008-08/msg00475.html
http://sourceware.org/ml/gdb-patches/2008-08/msg00479.html
http://sourceware.org/ml/gdb-patches/2008-08/msg00531.html

Could you take a look and see if the other changes make
sense to you too?

You may also be interested in this:

http://sourceware.org/ml/gdb-patches/2008-08/msg00530.html

>
> Index: remote-mips.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/remote-mips.c,v
> retrieving revision 1.108
> diff -u -p -r1.108 remote-mips.c
> --- remote-mips.c 26 Feb 2010 23:11:24 -0000 1.108
> +++ remote-mips.c 2 Mar 2010 23:02:32 -0000
> @@ -35,6 +35,7 @@
> #include "regcache.h"
> #include <ctype.h>
> #include "mips-tdep.h"
> +#include "gdbthread.h"
> \f
>
> /* Breakpoint types. Values 0, 1, and 2 must agree with the watch
> @@ -483,7 +484,8 @@ mips_error (char *string,...)
> close_ports ();
>
> printf_unfiltered ("Ending remote MIPS debugging.\n");
> - target_mourn_inferior ();
> + if (!ptid_equal (inferior_ptid, null_ptid))
> + target_mourn_inferior ();
>
> deprecated_throw_reason (RETURN_ERROR);
> }
> @@ -1457,6 +1459,8 @@ mips_initialize (void)
> mips_request ('r', 0, 0, &err, mips_receive_wait, NULL);
> }
>
> +static ptid_t magic_null_ptid;
> +
> /* Open a connection to the remote board. */
> static void
> common_open (struct target_ops *ops, char *name, int from_tty,
> @@ -1468,6 +1472,7 @@ common_open (struct target_ops *ops, cha
> char *remote_name = 0;
> char *local_name = 0;
> char **argv;
> + struct inferior *inf;
>
> if (name == 0)
> error (
> @@ -1563,6 +1568,12 @@ device is attached to the target board (
> /* Switch to using remote target now. */
> push_target (ops);
>
> + inferior_ptid = magic_null_ptid;
> + inf = add_inferior_silent (ptid_get_pid (inferior_ptid));
> + inf->aspace = current_program_space->aspace;
> + inf->pspace = current_program_space;
> + add_thread_silent (inferior_ptid);
> +
> /* FIXME: Should we call start_remote here? */
>
> /* Try to figure out the processor model if possible. */
> @@ -2140,7 +2151,10 @@ static void
> mips_kill (struct target_ops *ops)
> {
> if (!mips_wait_flag)
> - return;
> + {
> + target_mourn_inferior ();
> + return;
> + }
>
> interrupt_count++;
>
> @@ -2173,6 +2187,8 @@ Give up (and stop debugging it)? ")))
>
> serial_send_break (mips_desc);
>
> + target_mourn_inferior ();
> +
> #if 0
> if (mips_is_open)
> {
> @@ -3297,8 +3313,6 @@ mips_load (char *file, int from_tty)
> if (exec_bfd)
> regcache_write_pc (regcache, bfd_get_start_address (exec_bfd));
>
> - inferior_ptid = null_ptid; /* No process now */
> -
> /* This is necessary because many things were based on the PC at the time that
> we attached to the monitor, which is no longer valid now that we have loaded
> new code (and just changed the PC). Another way to do this might be to call
> @@ -3458,4 +3472,5 @@ Use \"on\" to enable the masking and \"o
> NULL,
> NULL, /* FIXME: i18n: */
> &setlist, &showlist);
> + magic_null_ptid = ptid_build (42000, 1, -1);
> }
>
>

--
Pedro Alves