From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31604 invoked by alias); 27 Jun 2008 11:58:34 -0000 Received: (qmail 31594 invoked by uid 22791); 27 Jun 2008 11:58:31 -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; Fri, 27 Jun 2008 11:58:07 +0000 Received: (qmail 29451 invoked from network); 27 Jun 2008 11:58:04 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 27 Jun 2008 11:58:04 -0000 From: Pedro Alves To: Daniel Jacobowitz Subject: Re: Make the remote target always register a thread Date: Fri, 27 Jun 2008 12:30:00 -0000 User-Agent: KMail/1.9.9 Cc: gdb-patches@sourceware.org References: <200806260008.08596.pedro@codesourcery.com> <20080626024814.GB20206@caradoc.them.org> In-Reply-To: <20080626024814.GB20206@caradoc.them.org> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_QXNZIonvDqx+IgG" Message-Id: <200806271258.08547.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-06/txt/msg00493.txt.bz2 --Boundary-00=_QXNZIonvDqx+IgG Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 1109 A Thursday 26 June 2008 03:48:14, Daniel Jacobowitz wrote: > On Thu, Jun 26, 2008 at 12:08:08AM +0100, Pedro Alves wrote: > > (The crash will still be there for other targets that > > don't always register the main thread) > > Be nice to get rid of the crash, one way or another. It will take a while to make all targets consistent, so although I'd prefer to fix them all, practically speaking, the assertion should be removed for now, and perhaps leave a comment there so when we do have all targets consistent, the assert will be catching a real bug. The fix seems to me to be a one or two liner, but, as I've said at gdb@, I don't know if we should output thread-id=0 or not output thread-id at all, or maybe even output thread-id=all, as if there are no threads in the list, this is the special case of there being only one "task" really, but the target is not registering it. (OTOH, having an assert at least makes it easy to see which target needs to be changed.) > OK. Thanks, I've checked it in, with an obvious change as attached. I needed to include gdbthread.h in monitor.c. -- Pedro Alves --Boundary-00=_QXNZIonvDqx+IgG Content-Type: text/x-diff; charset="iso-8859-1"; name="002-remote_always_a_thread.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="002-remote_always_a_thread.diff" Content-length: 7137 2008-06-27 Pedro Alves * infrun.c (start_remote): Don't clear thread list here. * monitor.c (monitor_open): Include "gdbthread.h". Clear thread list here. * remote.c (record_currthread): Upgrade the main thread and its entry in the thread list if this is the first time we hear about threads. (remote_thread_alive): Consider magic_null_ptid or a ptid without a tid member always alive. (remote_find_new_threads): Don't update the main thread here. (remote_start_remote): Clear thread list here. Always add the main thread. (extended_remote_attach_1): Add the main thread here. (extended_remote_mourn_1): Re-add the main thread here. (extended_remote_create_inferior_1): Add a main thread. * Makefile.in (monitor.o): Depend on $(gdbthread_h). --- gdb/Makefile.in | 2 +- gdb/infrun.c | 1 - gdb/monitor.c | 4 ++++ gdb/remote.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++----- 4 files changed, 54 insertions(+), 7 deletions(-) Index: src/gdb/infrun.c =================================================================== --- src.orig/gdb/infrun.c 2008-06-27 12:30:13.000000000 +0100 +++ src/gdb/infrun.c 2008-06-27 12:33:21.000000000 +0100 @@ -1281,7 +1281,6 @@ proceed (CORE_ADDR addr, enum target_sig void start_remote (int from_tty) { - init_thread_list (); init_wait_for_inferior (); stop_soon = STOP_QUIETLY_REMOTE; stepping_over_breakpoint = 0; Index: src/gdb/monitor.c =================================================================== --- src.orig/gdb/monitor.c 2008-06-27 12:30:13.000000000 +0100 +++ src/gdb/monitor.c 2008-06-27 12:37:31.000000000 +0100 @@ -54,6 +54,7 @@ #include "gdb_regex.h" #include "srec.h" #include "regcache.h" +#include "gdbthread.h" static char *dev_name; static struct target_ops *targ_ops; @@ -804,6 +805,9 @@ monitor_open (char *args, struct monitor push_target (targ_ops); + /* Start afresh. */ + init_thread_list (); + inferior_ptid = pid_to_ptid (42000); /* Make run command think we are busy... */ /* Give monitor_wait something to read */ Index: src/gdb/remote.c =================================================================== --- src.orig/gdb/remote.c 2008-06-27 12:33:04.000000000 +0100 +++ src/gdb/remote.c 2008-06-27 12:33:21.000000000 +0100 @@ -1089,7 +1089,31 @@ record_currthread (ptid_t currthread) /* If this is a new thread, add it to GDB's thread list. If we leave it up to WFI to do this, bad things will happen. */ if (!in_thread_list (currthread)) - add_thread (currthread); + { + if (ptid_equal (pid_to_ptid (ptid_get_pid (currthread)), inferior_ptid)) + { + /* inferior_ptid has no thread member yet. This can happen + with the vAttach -> remote_wait,"TAAthread:" path if the + stub doesn't support qC. This is the first stop reported + after an attach, so this is the main thread. Update the + ptid in the thread list. */ + struct thread_info *th = find_thread_pid (inferior_ptid); + inferior_ptid = th->ptid = currthread; + } + else if (ptid_equal (magic_null_ptid, inferior_ptid)) + { + /* inferior_ptid is not set yet. This can happen with the + vRun -> remote_wait,"TAAthread:" path if the stub + doesn't support qC. This is the first stop reported + after an attach, so this is the main thread. Update the + ptid in the thread list. */ + struct thread_info *th = find_thread_pid (inferior_ptid); + inferior_ptid = th->ptid = currthread; + } + else + /* This is really a new thread. Add it. */ + add_thread (currthread); + } } static char *last_pass_packet; @@ -1212,6 +1236,16 @@ remote_thread_alive (ptid_t ptid) struct remote_state *rs = get_remote_state (); int tid = ptid_get_tid (ptid); + if (ptid_equal (ptid, magic_null_ptid)) + /* The main thread is always alive. */ + return 1; + + if (ptid_get_pid (ptid) != 0 && ptid_get_tid (ptid) == 0) + /* The main thread is always alive. This can happen after a + vAttach, if the remote side doesn't support + multi-threading. */ + return 1; + if (tid < 0) xsnprintf (rs->buf, get_remote_packet_size (), "T-%08x", -tid); else @@ -1925,9 +1959,6 @@ remote_find_new_threads (void) { remote_threadlist_iterator (remote_newthread_step, 0, CRAZY_MAX_THREADS); - if (ptid_equal (inferior_ptid, magic_null_ptid)) - /* We don't know the current thread yet. Query it. */ - inferior_ptid = remote_current_thread (inferior_ptid); } /* @@ -2289,6 +2320,9 @@ remote_start_remote (struct ui_out *uiou strcpy (wait_status, rs->buf); } + /* Start afresh. */ + init_thread_list (); + /* Let the stub know that we want it to return the thread. */ set_continue_thread (minus_one_ptid); @@ -2304,6 +2338,9 @@ remote_start_remote (struct ui_out *uiou /* Now, if we have thread information, update inferior_ptid. */ inferior_ptid = remote_current_thread (inferior_ptid); + /* Always add the main thread. */ + add_thread_silent (inferior_ptid); + get_offsets (); /* Get text, data & bss offsets. */ /* Use the previously fetched status. */ @@ -2934,6 +2971,9 @@ extended_remote_attach_1 (struct target_ /* Now, if we have thread information, update inferior_ptid. */ inferior_ptid = remote_current_thread (inferior_ptid); + /* Now, add the main thread to the thread list. */ + add_thread_silent (inferior_ptid); + attach_flag = 1; /* Next, if the target can specify a description, read it. We do @@ -5152,7 +5192,8 @@ extended_remote_mourn_1 (struct target_o /* Assume that the target has been restarted. Set inferior_ptid so that bits of core GDB realizes there's something here, e.g., so that the user can say "kill" again. */ - inferior_ptid = magic_null_ptid; + inferior_ptid = remote_current_thread (magic_null_ptid); + add_thread_silent (inferior_ptid); } else { @@ -5267,6 +5308,9 @@ extended_remote_create_inferior_1 (char /* Now mark the inferior as running before we do anything else. */ attach_flag = 0; inferior_ptid = magic_null_ptid; + + add_thread_silent (inferior_ptid); + target_mark_running (&extended_remote_ops); /* Get updated offsets, if the stub uses qOffsets. */ Index: src/gdb/Makefile.in =================================================================== --- src.orig/gdb/Makefile.in 2008-06-27 12:37:12.000000000 +0100 +++ src/gdb/Makefile.in 2008-06-27 12:38:20.000000000 +0100 @@ -2540,7 +2540,7 @@ mn10300-tdep.o: mn10300-tdep.c $(defs_h) $(symtab_h) $(dwarf2_frame_h) $(osabi_h) $(target_h) $(mn10300_tdep_h) monitor.o: monitor.c $(defs_h) $(gdbcore_h) $(target_h) $(exceptions_h) \ $(gdb_string_h) $(command_h) $(serial_h) $(monitor_h) $(gdbcmd_h) \ - $(inferior_h) $(gdb_regex_h) $(srec_h) $(regcache_h) + $(inferior_h) $(gdb_regex_h) $(srec_h) $(regcache_h) $(gdbthread_h) mt-tdep.o: mt-tdep.c $(defs_h) $(frame_h) $(frame_unwind_h) $(frame_base_h) \ $(symtab_h) $(dis_asm_h) $(arch_utils_h) $(gdbtypes_h) \ $(gdb_string_h) $(regcache_h) $(reggroups_h) $(gdbcore_h) \ --Boundary-00=_QXNZIonvDqx+IgG--