From patchwork Fri Mar 14 18:45:35 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kratochvil X-Patchwork-Id: 93 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx20.g.dreamhost.com (caibbdcaaahc.dreamhost.com [208.113.200.72]) by wilcox.dreamhost.com (Postfix) with ESMTP id 065C336007D for ; Fri, 14 Mar 2014 11:46:14 -0700 (PDT) Received: by homiemail-mx20.g.dreamhost.com (Postfix, from userid 14314964) id AE360418572BB; Fri, 14 Mar 2014 11:46:14 -0700 (PDT) X-Original-To: gdb@patchwork.siddhesh.in Delivered-To: x14314964@homiemail-mx20.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx20.g.dreamhost.com (Postfix) with ESMTPS id 6EE57418572AC for ; Fri, 14 Mar 2014 11:46:14 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:subject:message-id:references :mime-version:content-type:in-reply-to; q=dns; s=default; b=N5nT F7lSB1ie6iY5xUUFOvsEgoilR4+A6sP7qZMyTojAPKGMFc5Z7aJimPrVXiKW6/19 l4GZ7SxfPvYFJSOktQzZnACuvlh4N62ORS+UVEIE7ttXyoEPr7P+q1sYItLKcAam VD26hTVL3K2pML06MakauHQ4/viwDbDUDSpT2W8= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:subject:message-id:references :mime-version:content-type:in-reply-to; s=default; bh=JqGrlQcXT+ 3bROlIMLku0mL+X/k=; b=QYJNU0ktmGLOjTnwU6oOwJ7BuT5WRUqip7Z4R7ih7J FPG+Tw7LuSNQFZXEWg35U85i7IV6R7BK3qyOdLEM0o6FIAJtizX0KjTDuDV2wcS+ au7Njq7XxQwIYoNhl5bcRT0ASr89nokcjnqfr+SH9U0aK1BIjeMwzypOQrj589zz s= Received: (qmail 7002 invoked by alias); 14 Mar 2014 18:45:44 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 6934 invoked by uid 89); 14 Mar 2014 18:45:43 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.1 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 14 Mar 2014 18:45:41 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s2EIjd1E016098 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 14 Mar 2014 14:45:39 -0400 Received: from host2.jankratochvil.net (ovpn-116-86.ams2.redhat.com [10.36.116.86]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s2EIjZfM027151 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO) for ; Fri, 14 Mar 2014 14:45:38 -0400 Date: Fri, 14 Mar 2014 19:45:35 +0100 From: Jan Kratochvil To: gdb-patches@sourceware.org Subject: [patchv2] Fix SIGTERM signal safety (PR gdb/15358) [refresh] Message-ID: <20140314184535.GA30853@host2.jankratochvil.net> References: <20140314183709.GA28050@host2.jankratochvil.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140314183709.GA28050@host2.jankratochvil.net> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-DH-Original-To: gdb@patchwork.siddhesh.in Hi, the post several minutes ago (not the year old one) had accidentally corrupted testcase. ------------------------------------------------------------------------------ reposting with no changes, the last post is a year old one, it had been already approved by Tom that time so I will check it in in some time. [patch] Fix SIGTERM signal safety (PR gdb/15358) https://sourceware.org/ml/gdb-patches/2013-07/msg00094.html Message-ID: <20130702200010.GA23478@host2.jankratochvil.net> The statement about PR remote/15297 dependency is IMO false, this patch works on its own. The PR remote/15297 patch remains unfinished/pending, but that is off-topic here. ------------------------------------------------------------------------------ gdb deadlock due to gdb calling calloc() in signal handler http://sourceware.org/bugzilla/show_bug.cgi?id=15358 Patch depends in its functionality on: [patchv2 2/2] Fix CTRL-C for remote.c (PR remote/15297) http://sourceware.org/ml/gdb-patches/2013-06/msg00943.html Message-ID: <20130630181110.GB29548@host2.jankratochvil.net> async mode seems easy. The sync mode is a bit difficult - assuming it is safe to call quit_force from any place of QUIT;. OTOH the patch above assumes it can do: if (check_quit_flag ()) send_interrupt_sequence (); which clears quit_flag but we used set_quit_flag () to call quit_force, not just to throw the quit exception. This is why QUIT now checks also for SYNC_QUIT_FORCE_RUN. The change in linux-nat.c comes from testing i386-linux-nat.c (therefore 32-bit host GDB). i386_linux_resume there calls QUIT; via target_read (). This is a bug on its own, filed as: http://sourceware.org/bugzilla/show_bug.cgi?id=15713 But I have seen another bug in linux-nat.c, it was depending on PTRACE_KILL but at least Linux kernel ptrace expert Oleg Nesterov considers PTRACE_KILL superseded by kill(SIGKILL). Therefore I used there (also) more safe SIGKILL so that the possibly inconsistent state of inferior from i386_linux_resume does not matter. No regressions on {x86_64,x86_64-m32,i686}-fedora19pre-linux-gnu and in gdbserver mode. Thanks, Jan gdb/ 2013-07-02 Jan Kratochvil PR gdb/15358 * defs.h (sync_quit_force_run): New declaration. (QUIT): Check also SYNC_QUIT_FORCE_RUN. * event-top.c (async_sigterm_handler): New declaration. (async_sigterm_token): New variable. (async_init_signals): Create also async_sigterm_token. (async_sigterm_handler): New function. (sync_quit_force_run): New variable. (handle_sigterm): Replace quit_force call by other calls. * linux-nat.c (linux_nat_kill): Use kill_callback first. Extend the comment for stop_callback. * utils.c (quit): Call quit_force if SYNC_QUIT_FORCE_RUN. gdb/testsuite/ 2013-07-02 Jan Kratochvil PR gdb/15358 * gdb.base/gdb-sigterm.c: New file. * gdb.base/gdb-sigterm.exp: New file. diff --git a/gdb/defs.h b/gdb/defs.h index 480133d..47da43a 100644 --- a/gdb/defs.h +++ b/gdb/defs.h @@ -171,6 +171,9 @@ extern int check_quit_flag (void); /* * Set the quit flag. */ extern void set_quit_flag (void); +/* Flag that function quit should call quit_force. */ +extern volatile int sync_quit_force_run; + extern int immediate_quit; extern void quit (void); @@ -183,7 +186,7 @@ extern void quit (void); needed. */ #define QUIT { \ - if (check_quit_flag ()) quit (); \ + if (check_quit_flag () || sync_quit_force_run) quit (); \ if (deprecated_interactive_hook) deprecated_interactive_hook (); \ } diff --git a/gdb/event-top.c b/gdb/event-top.c index fbe89bd..af1562c 100644 --- a/gdb/event-top.c +++ b/gdb/event-top.c @@ -72,6 +72,7 @@ static void async_float_handler (gdb_client_data); #ifdef STOP_SIGNAL static void async_stop_sig (gdb_client_data); #endif +static void async_sigterm_handler (gdb_client_data arg); /* Readline offers an alternate interface, via callback functions. These are all included in the file callback.c in the @@ -135,6 +136,7 @@ static struct async_signal_handler *sigfpe_token; #ifdef STOP_SIGNAL static struct async_signal_handler *sigtstp_token; #endif +static struct async_signal_handler *async_sigterm_token; /* Structure to save a partially entered command. This is used when the user types '\' at the end of a command line. This is necessary @@ -770,6 +772,8 @@ async_init_signals (void) create_async_signal_handler (async_stop_sig, NULL); #endif + async_sigterm_token = + create_async_signal_handler (async_sigterm_handler, NULL); } /* Tell the event loop what to do if SIGINT is received. @@ -797,13 +801,31 @@ handle_sigint (int sig) gdb_call_async_signal_handler (sigint_token, immediate_quit); } +/* Handle GDB exit upon receiving SIGTERM if target_can_async_p (). */ + +static void +async_sigterm_handler (gdb_client_data arg) +{ + quit_force (NULL, stdin == instream); +} + +/* See defs.h. */ +volatile int sync_quit_force_run; + /* Quit GDB if SIGTERM is received. GDB would quit anyway, but this way it will clean up properly. */ void handle_sigterm (int sig) { signal (sig, handle_sigterm); - quit_force ((char *) 0, stdin == instream); + + if (target_can_async_p ()) + mark_async_signal_handler (async_sigterm_token); + else + { + sync_quit_force_run = 1; + set_quit_flag (); + } } /* Do the quit. All the checks have been done by the caller. */ diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index b615423..ec84188 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -3777,8 +3777,15 @@ linux_nat_kill (struct target_ops *ops) { ptid_t ptid = pid_to_ptid (ptid_get_pid (inferior_ptid)); + /* Kill all LWP's before trying to stop them. In rare cases the + lwp_info state may not match the inferior and + stop_wait_callback could lock up. */ + iterate_over_lwps (ptid, kill_callback, NULL); + /* Stop all threads before killing them, since ptrace requires - that the thread is stopped to sucessfully PTRACE_KILL. */ + that the thread is stopped to sucessfully PTRACE_KILL. + kill_callback normally already turned the inferior into a zombie + except for old Linux kernels 2.4.x. */ iterate_over_lwps (ptid, stop_callback, NULL); /* ... and wait until all of them have reported back that they're no longer running. */ diff --git a/gdb/utils.c b/gdb/utils.c index 9d068c5..364470c 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -999,6 +999,12 @@ print_sys_errmsg (const char *string, int errcode) void quit (void) { + if (sync_quit_force_run) + { + sync_quit_force_run = 0; + quit_force (NULL, stdin == instream); + } + #ifdef __MSDOS__ /* No steenking SIGINT will ever be coming our way when the program is resumed. Don't lie. */ diff --git a/gdb/testsuite/gdb.base/gdb-sigterm.c b/gdb/testsuite/gdb.base/gdb-sigterm.c new file mode 100644 index 0000000..ffa09e4 --- /dev/null +++ b/gdb/testsuite/gdb.base/gdb-sigterm.c @@ -0,0 +1,26 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2013 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 + +int +main (void) +{ + alarm (60); + + for (;;); /* loop-line */ +} diff --git a/gdb/testsuite/gdb.base/gdb-sigterm.exp b/gdb/testsuite/gdb.base/gdb-sigterm.exp new file mode 100644 index 0000000..8baeb96 --- /dev/null +++ b/gdb/testsuite/gdb.base/gdb-sigterm.exp @@ -0,0 +1,94 @@ +# This testcase is part of GDB, the GNU debugger. +# +# Copyright 2013 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 . + +standard_testfile + +if { [build_executable ${testfile}.exp ${testfile}] == -1 } { + return -1 +} + +proc do_test { pass } { + global testfile gdb_prompt binfile pf_prefix + + if ![runto_main] { + return -1 + } + + gdb_breakpoint "${testfile}.c:[gdb_get_line_number "loop-line" ${testfile}.c]" \ + temporary + + # gdb_continue_to_breakpoint would print a pass message. + gdb_test "continue" "Temporary breakpoint .* loop-line .*" "" + + gdb_test_no_output "set range-stepping off" "" + gdb_test_no_output "set debug infrun 1" "" + gdb_test_no_output "set debug lin-lwp 1" "" + + set test "run a bit #$pass" + set abort 1 + gdb_test_multiple "step" $test { + -re "infrun: stepping inside range" { + # Suppress pass $test + verbose -log "$pf_prefix $test" + set abort 0 + } + } + if $abort { + return + } + + set gdb_pid [exp_pid -i [board_info host fileid]] + remote_exec host "kill -TERM ${gdb_pid}" + + set test "expect eof #$pass" + set abort 1 + set stepping 0 + gdb_test_multiple "" $test { + eof { + verbose -log "$pf_prefix $test" + set abort 0 + } + -re "infrun: stepping inside range" { + incr stepping + if { $stepping > 200 } { + fail "$test (stepping inside range $stepping times)" + } else { + exp_continue + } + } + } + if $abort { + return + } +} + + +for {set pass 0} {$pass < 50} {incr pass} { + + clean_restart ${testfile} + gdb_test_no_output "set target-async off" "" + with_test_prefix "sync" { + do_test $pass + } + + clean_restart ${testfile} + gdb_test_no_output "set target-async on" "" + with_test_prefix "async" { + do_test $pass + } +} +pass "$pass SIGTERM passes"