From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17413 invoked by alias); 2 May 2008 11:34:36 -0000 Received: (qmail 17400 invoked by uid 22791); 2 May 2008 11:34:35 -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, 02 May 2008 11:34:17 +0000 Received: (qmail 21083 invoked from network); 2 May 2008 11:34:15 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 2 May 2008 11:34:15 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [RFA] Make continuations per-thread. Date: Fri, 02 May 2008 11:34:00 -0000 User-Agent: KMail/1.9.9 Cc: Daniel Jacobowitz , Vladimir Prus References: <200804242045.39246.vladimir@codesourcery.com> <20080502030012.GB28580@caradoc.them.org> In-Reply-To: <20080502030012.GB28580@caradoc.them.org> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_0wvGIlDD8NuKWTH" Message-Id: <200805021234.12472.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-05/txt/msg00066.txt.bz2 --Boundary-00=_0wvGIlDD8NuKWTH Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 1378 A Friday 02 May 2008 04:00:12, Daniel Jacobowitz wrote: > On Thu, Apr 24, 2008 at 07:45:38PM +0300, Vladimir Prus wrote: > > Right now, continuations are global. This means that if we're doing > > 'finish' in one thread, then we cannot do 'finish' or anything that > > also uses continuations, in any other thread. This seems unnecessary > > limitation, and this patch makes continuations per-thread. > > > > Further into non-stop series, it really allows me to do 'finish' or > > 'step' in several threads independently. > > > > OK? > > Could you explain why this is safe? We will do continuations for the > thread which reports an event only. So it seems like continuations > for other threads will linger in their struct thread. > > For example, if we finish from one thread and hit a breakpoint in > another thread before the finish returns. That's true. Attached is what we have next on the non-stop series to fix that. I'm not thrilled by it, but there are intermediate context switches in handle_inferior_event that make it much uglier to try to not make it centralized. This is one of the things that gets much better looking when we switch completelly to always-a-thread, and get rid of context-switching. I'm introducing another variable, instead of using previous_inferior_ptid, which would be a good candidate, but I have other plans for it. -- Pedro Alves --Boundary-00=_0wvGIlDD8NuKWTH Content-Type: text/x-diff; charset="iso-8859-1"; name="switch_context_for_intermediate_continuations.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="switch_context_for_intermediate_continuations.patch" Content-length: 3689 2008-05-02 Pedro Alves * inferior.h (step_ptid): Declare variable. * infcmd.c (step_1): Set step_ptid. * infrun.c (step_ptid): New variable. (fetch_inferior_event): Run the intermediate continuations in the right context, and clear step_ptid. --- gdb/infcmd.c | 1 + gdb/inferior.h | 4 ++++ gdb/infrun.c | 36 +++++++++++++++++++++++++++++++++++- 3 files changed, 40 insertions(+), 1 deletion(-) Index: src/gdb/inferior.h =================================================================== --- src.orig/gdb/inferior.h 2008-05-02 12:32:10.000000000 +0100 +++ src/gdb/inferior.h 2008-05-02 12:32:11.000000000 +0100 @@ -371,6 +371,10 @@ enum stop_kind extern enum stop_kind stop_soon; +/* The thread we are executing a `step n'-like operation on. Used in + all-stop mode to run the intermediate-continuations. */ +extern ptid_t step_ptid; + /* Nonzero if proceed is being used for a "finish" command or a similar situation when stop_registers should be saved. */ Index: src/gdb/infcmd.c =================================================================== --- src.orig/gdb/infcmd.c 2008-05-02 12:32:10.000000000 +0100 +++ src/gdb/infcmd.c 2008-05-02 12:32:11.000000000 +0100 @@ -789,6 +789,7 @@ which has no line number information.\n" and handle them one at the time, through step_once(). */ else { + step_ptid = inferior_ptid; step_once (skip_subroutines, single_inst, count); /* We are running, and the contination is installed. It will disable the longjmp breakpoint as appropriate. */ Index: src/gdb/infrun.c =================================================================== --- src.orig/gdb/infrun.c 2008-05-02 12:32:10.000000000 +0100 +++ src/gdb/infrun.c 2008-05-02 12:32:11.000000000 +0100 @@ -74,6 +74,8 @@ static void set_schedlock_func (char *ar struct execution_control_state; +static void context_switch (struct execution_control_state *ecs); + static int currently_stepping (struct execution_control_state *ecs); static void xdb_handle_command (char *args, int from_tty); @@ -447,6 +449,10 @@ follow_exec (int pid, char *execd_pathna until after the `wait' in `wait_for_inferior'. */ static int singlestep_breakpoints_inserted_p = 0; +/* The thread we are executing a `step n'-like operation on. Used in + all-stop mode to run the intermediate-continuations. */ +ptid_t step_ptid; + /* The thread we inserted single-step breakpoints for. */ static ptid_t singlestep_ptid; @@ -1521,11 +1527,39 @@ fetch_inferior_event (void *client_data) { delete_step_resume_breakpoint (&step_resume_breakpoint); + /* In all-stop mode, if an event in another thread stops a + requested `step n'-like operation, we must still run the + intermediate continuations. Since continuations are per + thread, we must context switch back to the stepping thread to + run them. */ + if (!ptid_equal (step_ptid, null_ptid) + && !ptid_equal (step_ptid, inferior_ptid) + && in_thread_list (step_ptid) + && in_thread_list (inferior_ptid)) + { + volatile struct gdb_exception e; + ptid_t current_ptid = inferior_ptid; + + async_ecs->ptid = step_ptid; + context_switch (async_ecs); + + TRY_CATCH (e, RETURN_MASK_ERROR) + { + do_all_intermediate_continuations (0); + } + + async_ecs->ptid = current_ptid; + context_switch (async_ecs); + } + normal_stop (); if (step_multi && stop_step) inferior_event_handler (INF_EXEC_CONTINUE, NULL); else - inferior_event_handler (INF_EXEC_COMPLETE, NULL); + { + inferior_event_handler (INF_EXEC_COMPLETE, NULL); + step_ptid = null_ptid; + } } } --Boundary-00=_0wvGIlDD8NuKWTH--