From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26187 invoked by alias); 14 Nov 2008 21:05:14 -0000 Received: (qmail 26026 invoked by uid 22791); 14 Nov 2008 21:05:13 -0000 X-Spam-Check-By: sourceware.org Received: from mtagate3.de.ibm.com (HELO mtagate3.de.ibm.com) (195.212.29.152) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 14 Nov 2008 21:04:22 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate3.de.ibm.com (8.13.8/8.13.8) with ESMTP id mAEL4Jhk156404 for ; Fri, 14 Nov 2008 21:04:19 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 v9.1) with ESMTP id mAEL4Jx53408094 for ; Fri, 14 Nov 2008 22:04:19 +0100 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 mAEL4JBA024223 for ; Fri, 14 Nov 2008 22:04:19 +0100 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 mAEL4JBX024219; Fri, 14 Nov 2008 22:04:19 +0100 Message-Id: <200811142104.mAEL4JBX024219@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Fri, 14 Nov 2008 22:04:18 +0100 Subject: [rfc] Fix PR 2250 - multithreaded single-step problems in all-stop mode To: gdb-patches@sourceware.org Date: Sat, 15 Nov 2008 16:42:00 -0000 From: "Ulrich Weigand" Cc: drow@false.org, pedro@codesourcery.com 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: 2008-11/txt/msg00374.txt.bz2 Hello, a while ago I posted a patch to fix problem PR 2250: http://sourceware.org/ml/gdb-patches/2008-05/msg00089.html This problem is about the behaviour of GDB when single-stepping a thread of a multi-threaded application. If during a single-step operation, some other event happens in another thread, GDB may get into an inconsistent state, where some internal data structures still think a single-step operation is going on, while this is in fact no longer true. The patch I originally proposed, however, conflicted with the new non-stop mode (where single-step operations can be outstanding in multiple threads at the same time), so it was never applied. Now that all the non-stop code is in mainline, the original bug is still present in the all-stop mode. I've reworked the patch to fix the problem in all-stop mode, while keeping the new behaviour in non-stop mode unchanges. As discussed in the message refered to above, I'm implementing the the following two changes: - If the step operation is interrupted by an *internal* breakpoint that is handled transparently, the operation continues in a transparent and consistent manner after the breakpoint was handled. (This is the handle_inferior_event change in the patch below.) - If the step operation is interrupted by an *explicit* breakpoint that breaks to a user prompt, it is completely cancelled. It is then up to the user how to continue from the prompt. (This is the clear_proceed_status change in the patch below.) Regression tested on powerpc64-linux. This patch fixes the problem originally reported as PR 2250 (thanks to Emi Suzuki for verifying!). I didn't test anything with all-stop mode, but by construction the patch should not modify the behaviour at all in this case. Does this look right? If there are no objections, I'm planning on committing this in a couple of days. Bye, Ulrich ChangeLog: PR gdb/2250 * infrun.c (clear_proceed_status_thread): New function. (clear_proceed_status_callback): New function. (clear_proceed_status): In all-stop mode, clear per-thread proceed status of *all* threads, not only the current. (handle_inferior_event): In all-stop mode, if we're stepping one thread, but got some inferior event in another thread that does not cause GDB to break to the user interface, ensure the interrupted stepping operation continues in the original thread. (currently_stepping): Move thread-related tests to ... (currently_stepping_thread): ... this new function. (currently_stepping_callback): New function. Index: gdb/infrun.c =================================================================== RCS file: /cvs/src/src/gdb/infrun.c,v retrieving revision 1.336 diff -u -p -r1.336 infrun.c --- gdb/infrun.c 5 Nov 2008 20:23:07 -0000 1.336 +++ gdb/infrun.c 13 Nov 2008 19:54:06 -0000 @@ -75,6 +75,8 @@ static void set_schedlock_func (char *ar static int currently_stepping (struct thread_info *tp); +static int currently_stepping_callback (struct thread_info *tp, void *data); + static void xdb_handle_command (char *args, int from_tty); static int prepare_to_proceed (int); @@ -1161,31 +1163,59 @@ a command like `return' or `jump' to con /* Clear out all variables saying what to do when inferior is continued. First do this, then set the ones you want, then call `proceed'. */ +static void +clear_proceed_status_thread (struct thread_info *tp) +{ + if (debug_infrun) + fprintf_unfiltered (gdb_stdlog, + "infrun: clear_proceed_status_thread (%s)\n", + target_pid_to_str (tp->ptid)); + + tp->trap_expected = 0; + tp->step_range_start = 0; + tp->step_range_end = 0; + tp->step_frame_id = null_frame_id; + tp->step_over_calls = STEP_OVER_UNDEBUGGABLE; + tp->stop_requested = 0; + + tp->stop_step = 0; + + tp->proceed_to_finish = 0; + + /* Discard any remaining commands or status from previous stop. */ + bpstat_clear (&tp->stop_bpstat); +} + +static int +clear_proceed_status_callback (struct thread_info *tp, void *data) +{ + if (is_exited (tp->ptid)) + return 0; + + clear_proceed_status_thread (tp); + return 0; +} + void clear_proceed_status (void) { if (!ptid_equal (inferior_ptid, null_ptid)) { - struct thread_info *tp; struct inferior *inferior; - tp = inferior_thread (); - - tp->trap_expected = 0; - tp->step_range_start = 0; - tp->step_range_end = 0; - tp->step_frame_id = null_frame_id; - tp->step_over_calls = STEP_OVER_UNDEBUGGABLE; - tp->stop_requested = 0; - - tp->stop_step = 0; - - tp->proceed_to_finish = 0; - - /* Discard any remaining commands or status from previous - stop. */ - bpstat_clear (&tp->stop_bpstat); - + if (non_stop) + { + /* If in non-stop mode, only delete the per-thread status + of the current thread. */ + clear_proceed_status_thread (inferior_thread ()); + } + else + { + /* In all-stop mode, delete the per-thread status of + *all* threads. */ + iterate_over_threads (clear_proceed_status_callback, NULL); + } + inferior = current_inferior (); inferior->stop_soon = NO_STOP_QUIETLY; } @@ -3222,6 +3252,43 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME ( test for stepping. But, if not stepping, do not stop. */ + /* In all-stop mode, if we're currently stepping but have stopped in + some other thread, we need to switch back to the stepped thread. */ + if (!non_stop) + { + struct thread_info *tp; + tp = iterate_over_threads (currently_stepping_callback, + ecs->event_thread); + if (tp) + { + /* However, if the current thread is blocked on some internal + breakpoint, and we simply need to step over that breakpoint + to get it going again, do that first. */ + if ((ecs->event_thread->trap_expected + && ecs->event_thread->stop_signal != TARGET_SIGNAL_TRAP) + || ecs->event_thread->stepping_over_breakpoint) + { + keep_going (ecs); + return; + } + + /* Otherwise, we no longer expect a trap in the current thread. + Clear the trap_expected flag before switching back -- this is + what keep_going would do as well, if we called it. */ + ecs->event_thread->trap_expected = 0; + + if (debug_infrun) + fprintf_unfiltered (gdb_stdlog, + "infrun: switching back to stepped thread\n"); + + ecs->event_thread = tp; + ecs->ptid = tp->ptid; + context_switch (ecs->ptid); + keep_going (ecs); + return; + } + } + /* Are we stepping to get the inferior out of the dynamic linker's hook (and possibly the dld itself) after catching a shlib event? */ @@ -3641,12 +3708,25 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME ( /* Are we in the middle of stepping? */ static int +currently_stepping_thread (struct thread_info *tp) +{ + return (tp->step_range_end && tp->step_resume_breakpoint == NULL) + || tp->trap_expected + || tp->stepping_through_solib_after_catch; +} + +static int +currently_stepping_callback (struct thread_info *tp, void *data) +{ + /* Return true if any thread *but* the one passed in "data" is + in the middle of stepping. */ + return tp != data && currently_stepping_thread (tp); +} + +static int currently_stepping (struct thread_info *tp) { - return (((tp->step_range_end && tp->step_resume_breakpoint == NULL) - || tp->trap_expected) - || tp->stepping_through_solib_after_catch - || bpstat_should_step ()); + return currently_stepping_thread (tp) || bpstat_should_step (); } /* Inferior has stepped into a subroutine call with source code that -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com