From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8155 invoked by alias); 12 Mar 2009 02:45:30 -0000 Received: (qmail 8138 invoked by uid 22791); 12 Mar 2009 02:45:27 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 12 Mar 2009 02:45:21 +0000 Received: (qmail 16129 invoked from network); 12 Mar 2009 02:45:18 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 12 Mar 2009 02:45:18 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: [gdbserver] fix spurious SIGTRAPs Date: Thu, 12 Mar 2009 10:29:00 -0000 User-Agent: KMail/1.9.10 MIME-Version: 1.0 Content-Disposition: inline Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <200903120245.21837.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: 2009-03/txt/msg00165.txt.bz2 While working on multi-process/non-stop gdbserver, using my favorite multithreaded test, I noticed that gdbserver sometimes would report spurious SIGTRAPs. Here's what would happen: The test consists of several threads all running the same loop. There is a breakpoint set in the loop, hence all threads may hit it. The test then issues several "next" commands in a loop. Here's what would happen in gdbserver: 1) We issue a "continue", and wait until a thread hits the breakpoint. Could be any thread, but assume thread 1 hits it. 2) We issue a "next" --- this single-steps thread 1, and resumes all other threads. 3) thread 2, due to scheduler-locking off, hits the breakpoint. gdbserver stops all other threads by sending them SIGSTOPs. 4) While being stopped in step 3, thread 1 reports a SIGTRAP, that corresponds to the finished single-step of step 2. gdbserver leaves the SIGTRAP pending to report later. 5) We issue another "next" --- this requests thread 2 to single-step, and all other threads to continue, including thread 1. Before resuming any thread, gdbserver notices that it remembers from step 4 a pending SIGTRAP to report for thread 1, so reports it now. 6) From GDB's perpective, this SIGTRAP can't represent a finished single-step, since thread 1 was not single-stepping (it was continued in step 5). Neither does this SIGTRAP correspond to a breakpoint hit. GDB reports to the user a spurious SIGTRAP. Older GDBs happened to paper over the problem, as clear_proceed_status used to only clear the status of one thread. As a side effect, GDB resumed the threads that hit such spurious SIGTRAPs. To fix this, when stopping all threads to report a stop to GDB, if we find a thread stops with a SIGTRAP due to a finished single-step, do not leave the SIGTRAP pending. Tested against x86_64-linux gdbserver, new test included. OK to apply? -- Pedro Alves gdb/gdbserver/ 2009-03-12 Pedro Alves * linux-low.c (linux_wait_for_event): Don't clear the `stepping' flag. (wait_for_sigstop): Don't leave a finished single-step SIGTRAP pending. (linux_continue_one_thread): Only preserve the stepping flag if there's a pending breakpoint. gdb/testsuite/ 2009-03-12 Pedro Alves * gdb.threads/pending-step.c, gdb.threads/pending-step.exp: New. --- gdb/gdbserver/linux-low.c | 43 ++++++++----- gdb/testsuite/gdb.threads/pending-step.c | 62 +++++++++++++++++++ gdb/testsuite/gdb.threads/pending-step.exp | 93 +++++++++++++++++++++++++++++ 3 files changed, 182 insertions(+), 16 deletions(-) Index: src/gdb/gdbserver/linux-low.c =================================================================== --- src.orig/gdb/gdbserver/linux-low.c 2009-03-12 02:17:22.000000000 +0000 +++ src/gdb/gdbserver/linux-low.c 2009-03-12 02:17:36.000000000 +0000 @@ -122,6 +122,7 @@ static int linux_wait_for_event (struct static int check_removed_breakpoint (struct process_info *event_child); static void *add_process (unsigned long pid); static int my_waitpid (int pid, int *status, int flags); +static int linux_stopped_by_watchpoint (void); struct pending_signals { @@ -899,19 +900,11 @@ linux_wait_for_event (struct thread_info fprintf (stderr, "Hit a non-gdbserver breakpoint.\n"); /* If we were single-stepping, we definitely want to report the - SIGTRAP. The single-step operation has completed, so also - clear the stepping flag; in general this does not matter, - because the SIGTRAP will be reported to the client, which - will give us a new action for this thread, but clear it for - consistency anyway. It's safe to clear the stepping flag - because the only consumer of get_stop_pc () after this point - is check_removed_breakpoint, and pending_is_breakpoint is not - set. It might be wiser to use a step_completed flag instead. */ + SIGTRAP. Although the single-step operation has completed, + do not clear clear the stepping flag yet; we need to check it + in wait_for_sigstop. */ if (event_child->stepping) - { - event_child->stepping = 0; - return wstat; - } + return wstat; /* A SIGTRAP that we can't explain. It may have been a breakpoint. Check if it is a breakpoint, and if so mark the process information @@ -1095,8 +1088,24 @@ wait_for_sigstop (struct inferior_list_e if (debug_threads) fprintf (stderr, "LWP %ld stopped with non-sigstop status %06x\n", process->lwpid, wstat); - process->status_pending_p = 1; - process->status_pending = wstat; + + /* Do not leave a pending single-step finish to be reported to + the client. The client will give us a new action for this + thread, possibly a continue request --- otherwise, the client + would consider this pending SIGTRAP reported later a spurious + signal. */ + if (WSTOPSIG (wstat) == SIGTRAP + && process->stepping + && !linux_stopped_by_watchpoint ()) + { + if (debug_threads) + fprintf (stderr, " single-step SIGTRAP ignored\n"); + } + else + { + process->status_pending_p = 1; + process->status_pending = wstat; + } process->stop_expected = 1; } @@ -1281,8 +1290,10 @@ linux_continue_one_thread (struct inferi if (process->resume->leave_stopped) return; - if (process->resume->thread == -1) - step = process->stepping || process->resume->step; + if (process->resume->thread == -1 + && process->stepping + && process->pending_is_breakpoint) + step = 1; else step = process->resume->step; Index: src/gdb/testsuite/gdb.threads/pending-step.c =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ src/gdb/testsuite/gdb.threads/pending-step.c 2009-03-12 00:39:29.000000000 +0000 @@ -0,0 +1,62 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2002, 2003, 2004, 2007, 2008, 2009 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include +#include +#include +#include + +void *thread_function(void *arg); /* Pointer to function executed by each thread */ + +#define NUM 1 + +unsigned int args[NUM+1]; + +int main() { + int res; + pthread_t threads[NUM]; + void *thread_result; + long i; + + for (i = 1; i <= NUM; i++) + { + args[i] = 1; + res = pthread_create(&threads[i], + NULL, + thread_function, + (void *) i); + } + + /* schedlock.exp: last thread start. */ + args[0] = 1; + thread_function ((void *) 0); + + exit(EXIT_SUCCESS); +} + +void *thread_function(void *arg) { + int my_number = (long) arg; + int *myp = (int *) &args[my_number]; + + /* Don't run forever. Run just short of it :) */ + while (*myp > 0) + { + (*myp) ++; /* insert breakpoint here */ + } + + pthread_exit(NULL); +} Index: src/gdb/testsuite/gdb.threads/pending-step.exp =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ src/gdb/testsuite/gdb.threads/pending-step.exp 2009-03-12 01:40:49.000000000 +0000 @@ -0,0 +1,93 @@ +# Copyright (C) 1996, 1997, 2002, 2003, 2007, 2008, 2009 +# Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Test that a resume cancels a previously unfinished or unreported +# single-step correctly. +# +# The test consists of several threads all running the same loop. +# There is a breakpoint set in the loop, hence all threads may hit it. +# The test then issues several "next" commands in a loop. +# +# scheduler-locking must be set to the default of "off". +# +# Here's what would happen in gdbserver: +# +# 1) We issue a "continue", and wait until a thread hits the +# breakpoint. Could be any thread, but assume thread 1 hits it. +# +# 2) We issue a "next" --- this single-steps thread 1, and resumes all +# other threads. +# +# 3) thread 2, due to scheduler-locking off, hits the breakpoint. +# gdbserver stops all other threads by sending them SIGSTOPs. +# +# 4) While being stopped in step 3, thread 1 reports a SIGTRAP, that +# corresponds to the finished single-step of step 2. gdbserver +# leaves the SIGTRAP pending to report later. +# +# 5) We issue another "next" --- this requests thread 2 to +# single-step, and all other threads to continue, including thread +# 1. Before resuming any thread, gdbserver notices that it +# remembers from step 4 a pending SIGTRAP to report for thread 1, +# so reports it now. +# +# 6) From GDB's perpective, this SIGTRAP can't represent a finished +# single-step, since thread 1 was not single-stepping (it was +# continued in step 5). Neither does this SIGTRAP correspond to a +# breakpoint hit. The SIGTRAP is then reported to the user as a +# spurious stop. + +set testfile "pending-step" +set srcfile ${testfile}.c +set binfile ${objdir}/${subdir}/${testfile} + +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "incdir=${objdir}"]] != "" } { + return -1 +} + +# Start with a fresh gdb. + +gdb_exit +gdb_start +gdb_reinitialize_dir $srcdir/$subdir +gdb_load ${binfile} + +if ![runto_main] then { + fail "Can't run to main" + return 0 +} + +gdb_breakpoint [gdb_get_line_number "insert breakpoint here"] +gdb_continue_to_breakpoint "continue to first breakpoint hit" + +set test "next with breakpoints in multiple threads" +set iterations 20 +for {set i 0} {$i < $iterations} {incr i} { + + set ok 0 + gdb_test_multiple "next" "$test" { + -re "SIGTRAP.*$gdb_prompt $" { + fail "$test (spurious SIGTRAP)" + } + -re "$gdb_prompt $" { + set ok 1 + } + } + + if { $ok == 0 } { + break + } +}