From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10770 invoked by alias); 3 Mar 2010 23:44:43 -0000 Received: (qmail 10759 invoked by uid 22791); 3 Mar 2010 23:44:39 -0000 X-SWARE-Spam-Status: No, hits=-7.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 03 Mar 2010 23:44:33 +0000 Received: from int-mx05.intmail.prod.int.phx2.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.18]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o23NiNQS001551 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 3 Mar 2010 18:44:23 -0500 Received: from localhost.localdomain (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx05.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o23NiMpe027789 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Wed, 3 Mar 2010 18:44:23 -0500 Date: Wed, 03 Mar 2010 23:44:00 -0000 From: Kevin Buettner To: gdb-patches@sourceware.org Cc: Pedro Alves Subject: Re: [RFC] remote-mips.c: Fix bit rot associated with the inferior's state Message-ID: <20100303164422.076323dd@redhat.com> In-Reply-To: <201003022337.46465.pedro@codesourcery.com> References: <20100301172548.1672f69e@redhat.com> <20100302160920.76f1c85d@redhat.com> <201003022337.46465.pedro@codesourcery.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-03/txt/msg00139.txt.bz2 On Tue, 2 Mar 2010 23:37:46 +0000 Pedro Alves wrote: > Looks mostly okay, but are you missing a target_thread_alive > implementation? Yes, I was. Thanks for the review. Thanks, too, for the pointers to your work on this file. > See these: >=20 > http://sourceware.org/ml/gdb-patches/2008-08/msg00475.html This looks like an earlier version of msg00531, below. (Missing from it are the thread_alive and pid_to_str implementations.) > http://sourceware.org/ml/gdb-patches/2008-08/msg00479.html > http://sourceware.org/ml/gdb-patches/2008-08/msg00531.html >=20 > Could you take a look and see if the other changes make > sense to you too? I've incorporated much of msg0051, "[02/02] remote-mips.c, always a thread (take 2)", into my current patch. (So, yes, the other changes in your patch did make sense to me.) I'll include your name in the ChangeLog entry when it eventually gets committed. I'd appreciate it if you could look over the changes to mips_close(), below, since my current version differs from what you proposed in your patch. (I cribbed off of remote-sim.c for those changes.) Also new from my last patch, in addition to your changes, is the fact that mips_mourn_inferior() no longer calls generic_mourn_inferior(). That's now done in mips_close() which is called when the target is popped. > You may also be interested in this: >=20 > http://sourceware.org/ml/gdb-patches/2008-08/msg00530.html This looks interesting, but I haven't tried it yet. I'd like to get the current patch and my other pending changes in first. Below is my current patch, incorporating your work, plus a few additional changes of my own. Thanks again for looking this over. Kevin * remote-mips.c (gdbthread.h): Include. (remote_mips_ptid): Declare. (mips_error): Only mourn the inferior when inferior_ptid is non-null. (common_open): Set inferior_ptid, add it as an inferior, and as a thread too. Set up the inferior's address spaces. (mips_close): Invoke generic_mourn_inferior(). Delete the thread and the inferior. Delete FIXME comment regarding common_open(). (mips_kill): Make sure that target_mourn_inferior is invoked. (mips_mourn_inferior): Don't invoke generic_mourn_inferior, as it's now invoked from mips_close(). (mips_load): Don't null out inferior_ptid. Don't call clear_symtab_users(). (mips_thread_alive, mips_pid_to_str): New functions. (_initialize_remote_mips): Initialize remote_mips_ptid. Initialize to_thread_alive and to_pid_to_str operations. Index: remote-mips.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D 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 3 Mar 2010 22:57:34 -0000 @@ -35,6 +35,7 @@ #include "regcache.h" #include #include "mips-tdep.h" +#include "gdbthread.h" =0C =20 /* Breakpoint types. Values 0, 1, and 2 must agree with the watch @@ -440,6 +441,11 @@ struct lsi_error lsi_error_table[] =3D of warnings returned by PMON when hardware breakpoints are used. */ static int monitor_warnings; =20 +/* This is the ptid we use while we're connected to the remote. Its + value is arbitrary, as the remote-mips target doesn't have a notion of + processes or threads, but we need something non-null to place in + inferior_ptid. */ +static ptid_t remote_mips_ptid; =20 static void close_ports (void) @@ -483,7 +489,8 @@ mips_error (char *string,...) close_ports (); =20 printf_unfiltered ("Ending remote MIPS debugging.\n"); - target_mourn_inferior (); + if (!ptid_equal (inferior_ptid, null_ptid)) + target_mourn_inferior (); =20 deprecated_throw_reason (RETURN_ERROR); } @@ -1468,6 +1475,7 @@ common_open (struct target_ops *ops, cha char *remote_name =3D 0; char *local_name =3D 0; char **argv; + struct inferior *inf; =20 if (name =3D=3D 0) error ( @@ -1563,7 +1571,11 @@ device is attached to the target board ( /* Switch to using remote target now. */ push_target (ops); =20 - /* FIXME: Should we call start_remote here? */ + inferior_ptid =3D remote_mips_ptid; + inf =3D add_inferior_silent (ptid_get_pid (inferior_ptid)); + inf->aspace =3D current_program_space->aspace; + inf->pspace =3D current_program_space; + add_thread_silent (inferior_ptid); =20 /* Try to figure out the processor model if possible. */ deprecated_mips_set_processor_regs_hack (); @@ -1639,6 +1651,10 @@ mips_close (int quitting) =20 close_ports (); } + + generic_mourn_inferior (); + delete_thread_silent (remote_mips_ptid); + delete_inferior_silent (ptid_get_pid (remote_mips_ptid)); } =20 /* Detach from the remote board. */ @@ -2140,7 +2156,10 @@ static void mips_kill (struct target_ops *ops) { if (!mips_wait_flag) - return; + { + target_mourn_inferior (); + return; + } =20 interrupt_count++; =20 @@ -2173,6 +2192,8 @@ Give up (and stop debugging it)? "))) =20 serial_send_break (mips_desc); =20 + target_mourn_inferior (); + #if 0 if (mips_is_open) { @@ -2210,19 +2231,17 @@ Can't pass arguments to remote MIPS boar =20 init_wait_for_inferior (); =20 - /* FIXME: Should we set inferior_ptid here? */ - regcache_write_pc (get_current_regcache (), entry_pt); } =20 -/* Clean up after a process. Actually nothing to do. */ +/* Clean up after a process. The bulk of the work is done in mips_close(), + which is called when unpushing the target. */ =20 static void mips_mourn_inferior (struct target_ops *ops) { if (current_ops !=3D NULL) unpush_target (current_ops); - generic_mourn_inferior (); } =0C /* We can write a breakpoint and read the shadow contents in one @@ -3296,18 +3315,36 @@ mips_load (char *file, int from_tty) } if (exec_bfd) regcache_write_pc (regcache, bfd_get_start_address (exec_bfd)); +} =20 - inferior_ptid =3D 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 l= oaded - new code (and just changed the PC). Another way to do this might be to= call - normal_stop, except that the stack may not be valid, and things would g= et - horribly confused... */ +/* Check to see if a thread is still alive. */ +=20 +static int +mips_thread_alive (struct target_ops *ops, ptid_t ptid) +{ + if (ptid_equal (ptid, remote_mips_ptid)) + /* The monitor's task is always alive. */ + return 1; =20 - clear_symtab_users (); + return 0; } =20 +/* Convert a thread ID to a string. Returns the string in a static + buffer. */ + +static char * +mips_pid_to_str (struct target_ops *ops, ptid_t ptid) +{ + static char buf[64]; + + if (ptid_equal (ptid, remote_mips_ptid)) + { + xsnprintf (buf, sizeof buf, "Thread
"); + return buf; + } + + return normal_pid_to_str (ptid); +} =20 /* Pass the command argument as a packet to PMON verbatim. */ =20 @@ -3351,6 +3388,8 @@ _initialize_remote_mips (void) mips_ops.to_load =3D mips_load; mips_ops.to_create_inferior =3D mips_create_inferior; mips_ops.to_mourn_inferior =3D mips_mourn_inferior; + mips_ops.to_thread_alive =3D mips_thread_alive; + mips_ops.to_pid_to_str =3D mips_pid_to_str; mips_ops.to_log_command =3D serial_log_command; mips_ops.to_stratum =3D process_stratum; mips_ops.to_has_all_memory =3D default_child_has_all_memory; @@ -3458,4 +3497,5 @@ Use \"on\" to enable the masking and \"o NULL, NULL, /* FIXME: i18n: */ &setlist, &showlist); + remote_mips_ptid =3D ptid_build (42000, 0, 42000); }