From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5166 invoked by alias); 1 Aug 2007 19:52:42 -0000 Received: (qmail 5155 invoked by uid 22791); 1 Aug 2007 19:52:40 -0000 X-Spam-Check-By: sourceware.org Received: from mtagate6.de.ibm.com (HELO mtagate6.de.ibm.com) (195.212.29.155) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 01 Aug 2007 19:52:34 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate6.de.ibm.com (8.13.8/8.13.8) with ESMTP id l71JqV6W192908 for ; Wed, 1 Aug 2007 19:52:31 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v8.4) with ESMTP id l71JqUun1597598 for ; Wed, 1 Aug 2007 21:52:31 +0200 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id l71JqOmI015395 for ; Wed, 1 Aug 2007 21:52:24 +0200 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with SMTP id l71JqO6e015264 for ; Wed, 1 Aug 2007 21:52:24 +0200 Message-Id: <200708011952.l71JqO6e015264@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Wed, 1 Aug 2007 21:52:21 +0200 Subject: [patch, rfc, rft] Multi-threaded single-step vs. breakpoint problems (prepare_to_proceed) To: gdb-patches@sourceware.org Date: Wed, 01 Aug 2007 19:52:00 -0000 From: "Ulrich Weigand" X-Mailer: ELM [version 2.5 PL2] MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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: 2007-08/txt/msg00011.txt.bz2 Hello, here's a patch to fix the problem reported in http://sourceware.org/ml/gdb-patches/2007-07/msg00278.html The main changes to prepare_to_proceed are do only ever switch threads if we stopped due to hitting a breakpoint which is still set, and the user switched threads in the meantime. If we're also about to single- step, we remember which thread we were supposed to step, and switch back to it in handle_inferior_event after we're stepped past that breakpoint. As cleanup, the patch follows a suggestion in a comment in prepare_to_proceed and makes switch_to_thread in thread.c global, and calls it instead of duplicating its contents. That routine is then also called by the new code in handle_inferior_event to switch back; it also appeared natural to use the routine in context_switch. This patch does not attempt to unify handling of this condition with the stepping_past_singlestep_breakpoint mechanism. The problem is that this condition must be detected already in proceed, before the main "struct execution_control_state" is even set up. Thus, we cannot do a proper context_switch back and forth. Maybe there is a better way to handle this ... Tested with no new regressions on i368-linux and powerpc64-linux. I'd appreciate comments on this approach! Bye, Ulrich ChangeLog: * infrun.c (stepping_past_breakpoint): New global variable. (stepping_past_breakpoint_ptid): Likewise. (prepare_to_proceed): Add STEP parameter. Do not check for Ctrl-C. Only switch threads if we need to single-step over a breakpoint hit in the previously selected thread. If stepping, remember previous thread to switch back to in STEPPING_PAST_BREAKPOINT[_PTID]. Call switch_to_thread instead of copying its contents. (proceed): Pass STEP to prepare_to_proceed. Always set ONEPROC if prepare_to_proceed returns true. (init_wait_for_inferior): Reset STEPPING_PAST_BREAKPOINT. (context_switch): Call switch_to_thread. (handle_inferior_event): Switch back to previous thread if requested in STEPPING_PAST_BREAKPOINT[_PTID] by prepare_to_proceed. * gdbthread.h (switch_to_thread): Add prototype. * thread.c (switch_to_thread): Make global. Index: gdb/gdbthread.h =================================================================== RCS file: /cvs/src/src/gdb/gdbthread.h,v retrieving revision 1.14 diff -c -p -r1.14 gdbthread.h *** gdb/gdbthread.h 9 Jan 2007 17:58:51 -0000 1.14 --- gdb/gdbthread.h 1 Aug 2007 18:30:09 -0000 *************** extern void load_infrun_state (ptid_t pt *** 137,142 **** --- 137,145 ---- int *current_line, struct symtab **current_symtab); + /* Switch from one thread to another. */ + extern void switch_to_thread (ptid_t ptid); + /* Commands with a prefix of `thread'. */ extern struct cmd_list_element *thread_cmd_list; Index: gdb/infrun.c =================================================================== RCS file: /cvs/src/src/gdb/infrun.c,v retrieving revision 1.244 diff -c -p -r1.244 infrun.c *** gdb/infrun.c 2 Jul 2007 21:29:27 -0000 1.244 --- gdb/infrun.c 1 Aug 2007 18:30:10 -0000 *************** static int currently_stepping (struct ex *** 80,86 **** static void xdb_handle_command (char *args, int from_tty); ! static int prepare_to_proceed (void); void _initialize_infrun (void); --- 80,86 ---- static void xdb_handle_command (char *args, int from_tty); ! static int prepare_to_proceed (int); void _initialize_infrun (void); *************** static CORE_ADDR singlestep_pc; *** 447,452 **** --- 447,457 ---- thread here so that we can resume single-stepping it later. */ static ptid_t saved_singlestep_ptid; static int stepping_past_singlestep_breakpoint; + + /* Similarly, if we are stepping another thread past a breakpoint, + save the original thread here so that we can resume stepping it later. */ + static ptid_t stepping_past_breakpoint_ptid; + static int stepping_past_breakpoint; /* Things to clean up if we QUIT out of resume (). */ *************** clear_proceed_status (void) *** 644,650 **** /* This should be suitable for any targets that support threads. */ static int ! prepare_to_proceed (void) { ptid_t wait_ptid; struct target_waitstatus wait_status; --- 649,655 ---- /* This should be suitable for any targets that support threads. */ static int ! prepare_to_proceed (int step) { ptid_t wait_ptid; struct target_waitstatus wait_status; *************** prepare_to_proceed (void) *** 652,693 **** /* Get the last target status returned by target_wait(). */ get_last_target_status (&wait_ptid, &wait_status); ! /* Make sure we were stopped either at a breakpoint, or because ! of a Ctrl-C. */ if (wait_status.kind != TARGET_WAITKIND_STOPPED ! || (wait_status.value.sig != TARGET_SIGNAL_TRAP ! && wait_status.value.sig != TARGET_SIGNAL_INT)) { return 0; } if (!ptid_equal (wait_ptid, minus_one_ptid) ! && !ptid_equal (inferior_ptid, wait_ptid)) { ! /* Switched over from WAIT_PID. */ ! CORE_ADDR wait_pc = read_pc_pid (wait_ptid); ! ! if (wait_pc != read_pc ()) { ! /* Switch back to WAIT_PID thread. */ ! inferior_ptid = wait_ptid; ! ! /* FIXME: This stuff came from switch_to_thread() in ! thread.c (which should probably be a public function). */ ! reinit_frame_cache (); ! registers_changed (); ! stop_pc = wait_pc; } /* We return 1 to indicate that there is a breakpoint here, ! so we need to step over it before continuing to avoid ! hitting it straight away. */ ! if (breakpoint_here_p (wait_pc)) ! return 1; } return 0; - } /* Record the pc of the program the last time it stopped. This is --- 657,691 ---- /* Get the last target status returned by target_wait(). */ get_last_target_status (&wait_ptid, &wait_status); ! /* Make sure we were stopped at a breakpoint. */ if (wait_status.kind != TARGET_WAITKIND_STOPPED ! || wait_status.value.sig != TARGET_SIGNAL_TRAP) { return 0; } + /* Switched over from WAIT_PID. */ if (!ptid_equal (wait_ptid, minus_one_ptid) ! && !ptid_equal (inferior_ptid, wait_ptid) ! && breakpoint_here_p (read_pc_pid (wait_ptid))) { ! /* If stepping, remember current thread to switch back to. */ ! if (step) { ! stepping_past_breakpoint = 1; ! stepping_past_breakpoint_ptid = inferior_ptid; } + /* Switch back to WAIT_PID thread. */ + switch_to_thread (wait_ptid); + /* We return 1 to indicate that there is a breakpoint here, ! so we need to step over it before continuing to avoid ! hitting it straight away. */ ! return 1; } return 0; } /* Record the pc of the program the last time it stopped. This is *************** proceed (CORE_ADDR addr, enum target_sig *** 753,759 **** prepare_to_proceed checks the current thread against the thread that reported the most recent event. If a step-over is required it returns TRUE and sets the current thread to the old thread. */ ! if (prepare_to_proceed () && breakpoint_here_p (read_pc ())) oneproc = 1; if (oneproc) --- 751,757 ---- prepare_to_proceed checks the current thread against the thread that reported the most recent event. If a step-over is required it returns TRUE and sets the current thread to the old thread. */ ! if (prepare_to_proceed (step)) oneproc = 1; if (oneproc) *************** init_wait_for_inferior (void) *** 874,879 **** --- 872,878 ---- clear_proceed_status (); stepping_past_singlestep_breakpoint = 0; + stepping_past_breakpoint = 0; } /* This enum encodes possible reasons for doing a target_wait, so that *************** context_switch (struct execution_control *** 1143,1150 **** &ecs->stepping_through_solib_catchpoints, &ecs->current_line, &ecs->current_symtab); } ! inferior_ptid = ecs->ptid; ! reinit_frame_cache (); } static void --- 1142,1149 ---- &ecs->stepping_through_solib_catchpoints, &ecs->current_line, &ecs->current_symtab); } ! ! switch_to_thread (ecs->ptid); } static void *************** handle_inferior_event (struct execution_ *** 1606,1611 **** --- 1605,1634 ---- stepping_past_singlestep_breakpoint = 0; + if (stepping_past_breakpoint) + { + stepping_past_breakpoint = 0; + + /* If we stopped for some other reason than single-stepping, ignore + the fact that we were supposed to switch back. */ + if (stop_signal == TARGET_SIGNAL_TRAP) + { + if (debug_infrun) + fprintf_unfiltered (gdb_stdlog, + "infrun: stepping_past_breakpoint\n"); + + /* Note: We do not call context_switch at this point, as the + context is already set up for stepping the original thread. */ + switch_to_thread (stepping_past_breakpoint_ptid); + /* Suppress spurious "Switching to ..." message. */ + previous_inferior_ptid = inferior_ptid; + + resume (1, TARGET_SIGNAL_0); + prepare_to_wait (ecs); + return; + } + } + /* See if a thread hit a thread-specific breakpoint that was meant for another thread. If so, then step that thread past the breakpoint, and continue it. */ Index: gdb/thread.c =================================================================== RCS file: /cvs/src/src/gdb/thread.c,v retrieving revision 1.53 diff -c -p -r1.53 thread.c *** gdb/thread.c 10 Apr 2007 14:53:46 -0000 1.53 --- gdb/thread.c 1 Aug 2007 18:30:10 -0000 *************** static int thread_alive (struct thread_i *** 61,67 **** static void info_threads_command (char *, int); static void thread_apply_command (char *, int); static void restore_current_thread (ptid_t); - static void switch_to_thread (ptid_t ptid); static void prune_threads (void); static struct cleanup *make_cleanup_restore_current_thread (ptid_t, struct frame_id); --- 61,66 ---- *************** info_threads_command (char *arg, int fro *** 454,460 **** /* Switch from one thread to another. */ ! static void switch_to_thread (ptid_t ptid) { if (ptid_equal (ptid, inferior_ptid)) --- 453,459 ---- /* Switch from one thread to another. */ ! void switch_to_thread (ptid_t ptid) { if (ptid_equal (ptid, inferior_ptid)) -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com