From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5332 invoked by alias); 25 Jun 2008 23:08:40 -0000 Received: (qmail 5318 invoked by uid 22791); 25 Jun 2008 23:08:38 -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; Wed, 25 Jun 2008 23:08:12 +0000 Received: (qmail 8336 invoked from network); 25 Jun 2008 23:08:09 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 25 Jun 2008 23:08:09 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Make the remote target always register a thread Date: Thu, 26 Jun 2008 02:22:00 -0000 User-Agent: KMail/1.9.9 MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_Y/sYIYI5hBaQMX9" Message-Id: <200806260008.08596.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/msg00461.txt.bz2 --Boundary-00=_Y/sYIYI5hBaQMX9 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 1699 Hi, This patch makes the remote target always put a thread in the thread list. This is needed so we can get rid of context switching on the core side. This is also the fix I suggested for the crash reported in this thread: "How to catch GDB crash" http://sourceware.org/ml/gdb/2008-06/msg00242.html (The crash will still be there for other targets that don't always register the main thread) The remote target is special in its handling of threads, as depending on the optional thread related packets supported, inferior_ptid can "upgrade" from magic_null_ptid to a real ptid, hence care must be taken to not leave an old thread in the thread list with magic_null_ptid ptid, if threads are reported. I've tested those special cases manually by hacking GDB, so I could test combinations like, target remote, no qC support, but qfThreadInfo supported, target remote, no qC support, but T AA thread:TID is reported, target extended-remote, attaching, with no threads support (no qC, or qfThreadInfo or T AA thread:TID, or the old thread listing protocol) target extended-remote, running, again with no threads support. All the above, but after connecting, enabling T AA thread:TID All the above, but after connecting, enabling qC GDB behaved correctly, in those cases, that is, the
thread is upgraded to a real ptid correctly. The start_remote change is needed, because we're adding the thread to the list before calling it, so we can't call init_thread_list there. Instead the call is moved to the start_remote called. There's another caller in monitor.c, so that also gets the obvious change. Regtested with a native gdbserver on x86_64-unknown-linux-gnu. OK? -- Pedro Alves --Boundary-00=_Y/sYIYI5hBaQMX9 Content-Type: text/x-diff; charset="utf-8"; name="002-remote_always_a_thread.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="002-remote_always_a_thread.diff" Content-length: 6028 2008-06-25 Pedro Alves * infrun.c (start_remote): Don't clear thread list here. * monitor.c (monitor_open): 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. --- gdb/infrun.c | 1 - gdb/monitor.c | 3 +++ gdb/remote.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 52 insertions(+), 6 deletions(-) Index: src/gdb/infrun.c =================================================================== --- src.orig/gdb/infrun.c 2008-06-25 20:09:25.000000000 +0100 +++ src/gdb/infrun.c 2008-06-25 21:13:35.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-25 20:09:25.000000000 +0100 +++ src/gdb/monitor.c 2008-06-25 21:13:35.000000000 +0100 @@ -804,6 +804,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-25 21:13:34.000000000 +0100 +++ src/gdb/remote.c 2008-06-25 22:34:48.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. */ --Boundary-00=_Y/sYIYI5hBaQMX9--