From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4269 invoked by alias); 19 Aug 2008 19:38:06 -0000 Received: (qmail 4256 invoked by uid 22791); 19 Aug 2008 19:38:05 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 19 Aug 2008 19:37:04 +0000 Received: (qmail 25840 invoked from network); 19 Aug 2008 19:37:02 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 19 Aug 2008 19:37:02 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: [02/02] remote-mips.c, always a thread (take 2) Date: Tue, 19 Aug 2008 19:38:00 -0000 User-Agent: KMail/1.9.9 References: <200808181329.28252.pedro@codesourcery.com> <200808181407.35078.pedro@codesourcery.com> In-Reply-To: <200808181407.35078.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_IEyqIYSm6OUHClq" Message-Id: <200808192037.44136.pedro@codesourcery.com> 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: 2008-08/txt/msg00531.txt.bz2 --Boundary-00=_IEyqIYSm6OUHClq Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 1961 On Monday 18 August 2008 14:07:34, Pedro Alves wrote: > I forgot to add a new target_thread_alive function, otherwise, > info threads gets rid of the thread I just added. And here's a new patch that fixes that. Here are the same notes I posted yesterday: - nowhere in the file was inferior_ptid set, but there were places where it was being set to null_ptid (you can see one such place in the patch). - mips_load was clearing inferior_ptid, and calling clear_symtab_users. I notice that inferior_ptid is being cleared after a load command, but that is wrong, I believe. See this change of Jim's to monitor.c, that removed this clearing from the monitor target: http://sourceware.org/ml/gdb/2001-09/msg00125.html I'm applying the exact same reasoning and change here. If this patch goes in without the previous patch, issuing "run" and the user answering yes to "The program being debugged has been started already. Start it from the beginning?" ends up calling target_kill which sends the remote a break. With the previous patch installed, The user can use ctrl-c at the terminal to interrupt the remote, and target_kill will do nothing, just like monitor.c does (which seems to share some history with remote-mips.c). - There's a FIXME in mips_create_inferior, wondering if it should set inferior_ptid there. I believe the answer is negative. When you get to mips_create_inferior, the target is already with execution -- you have it since target_open. The "run" command against this target merelly sets the PC to the entry points, and proceeds. The user could have also just "target mips ..." followed by "continue". See Jim's email above, his very nice explanation makes it clearer. I've checked that this at least compiles. Keep in mind that there is no current configuration using this file (which I had noticed before). I would definitelly not opposse GCing this file instead. -- Pedro Alves --Boundary-00=_IEyqIYSm6OUHClq Content-Type: text/x-diff; charset="utf-8"; name="002-remote_mips_always_a_thread.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="002-remote_mips_always_a_thread.diff" Content-length: 4154 2008-08-19 Pedro Alves * remote-mips.c: Include "gdbthread.h". (remote_mips_ptid): New. (common_open): Set inferior_ptid to remote_mips_ptid and add it as a thread to GDB's thread list. (mips_close): Delete remote_mips_ptid from GDB's thread list. (mips_create_inferior): Remove FIXME note. (mips_load): Don't delete symtab users, or reset inferior_ptid. (_initialize_remote_mips): Initialize remote_mips_ptid. --- gdb/remote-mips.c | 52 +++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 9 deletions(-) Index: src/gdb/remote-mips.c =================================================================== --- src.orig/gdb/remote-mips.c 2008-08-19 20:04:11.000000000 +0100 +++ src/gdb/remote-mips.c 2008-08-19 20:04:40.000000000 +0100 @@ -36,6 +36,7 @@ #include #include "mips-tdep.h" #include +#include "gdbthread.h" /* Breakpoint types. Values 0, 1, and 2 must agree with the watch @@ -457,6 +458,11 @@ struct lsi_error lsi_error_table[] = of warnings returned by PMON when hardware breakpoints are used. */ static int monitor_warnings; +/* This is the ptid we use while we're connected to the remote. Its + value is arbitrary, as the remote-sim target don't have a notion or + processes or threads, but we need something non-null to place in + inferior_ptid. */ +static ptid_t remote_mips_ptid; static void close_ports (void) @@ -1584,6 +1590,9 @@ device is attached to the target board ( /* Try to figure out the processor model if possible. */ deprecated_mips_set_processor_regs_hack (); + inferior_ptid = remote_mips_ptid; + add_thread_silent (remote_mips_ptid); + /* This is really the job of start_remote however, that makes an assumption that the target is about to print out a status message of some sort. That doesn't happen here (in fact, it may not be @@ -1655,6 +1664,8 @@ mips_close (int quitting) close_ports (); } + + delete_thread_silent (remote_mips_ptid); } /* Detach from the remote board. */ @@ -2264,8 +2275,6 @@ Can't pass arguments to remote MIPS boar init_wait_for_inferior (); - /* FIXME: Should we set inferior_ptid here? */ - write_pc (entry_pt); } @@ -3343,18 +3352,37 @@ mips_load (char *file, int from_tty) } if (exec_bfd) write_pc (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 - normal_stop, except that the stack may not be valid, and things would get - horribly confused... */ +/* Check to see if a thread is still alive. */ - clear_symtab_users (); +static int +mips_thread_alive (ptid_t ptid) +{ + if (ptid_equal (ptid, remote_mips_ptid)) + /* The monitor's task is always alive. */ + return 1; + + return 0; } +/* Convert a thread ID to a string. Returns the string in a static + buffer. */ + +static char * +mips_pid_to_str (ptid_t ptid) +{ + static char buf[64]; + + if (ptid_equal (remote_mips_ptid, ptid)) + { + xsnprintf (buf, sizeof buf, "Thread
"); + return buf; + } + + return normal_pid_to_str (ptid); +} /* Pass the command argument as a packet to PMON verbatim. */ @@ -3399,6 +3427,8 @@ _initialize_remote_mips (void) mips_ops.to_load = mips_load; mips_ops.to_create_inferior = mips_create_inferior; mips_ops.to_mourn_inferior = mips_mourn_inferior; + mips_ops.to_thread_alive = mips_thread_alive; + mips_ops.to_pid_to_str = mips_pid_to_str; mips_ops.to_log_command = serial_log_command; mips_ops.to_stratum = process_stratum; mips_ops.to_has_all_memory = 1; @@ -3506,4 +3536,8 @@ Use \"on\" to enable the masking and \"o NULL, NULL, /* FIXME: i18n: */ &setlist, &showlist); + + /* Yes, 42000 is arbitrary. The only sense out of it, is that it + isn't 0. */ + remote_mips_ptid = ptid_build (42000, 0, 42000); } --Boundary-00=_IEyqIYSm6OUHClq--