From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1563 invoked by alias); 22 Mar 2013 20:43:05 -0000 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 Received: (qmail 1103 invoked by uid 89); 22 Mar 2013 20:42:57 -0000 X-Spam-SWARE-Status: No, score=-8.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 22 Mar 2013 20:42:53 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r2MKgp7q029417 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 22 Mar 2013 16:42:51 -0400 Received: from host2.jankratochvil.net (ovpn-116-100.ams2.redhat.com [10.36.116.100]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r2MKgiMm029750 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Fri, 22 Mar 2013 16:42:47 -0400 Date: Sat, 23 Mar 2013 18:31:00 -0000 From: Jan Kratochvil To: Pedro Alves Cc: gdb-patches@sourceware.org, Hui Zhu Subject: [commit+7.6] [patch+7.6] Fix 7.5 regression crashing GDB if gdbserver dies Message-ID: <20130322204243.GA31871@host2.jankratochvil.net> References: <20130315195359.GA19841@host2.jankratochvil.net> <514C50EF.6030202@redhat.com> <20130322191841.GA29259@host2.jankratochvil.net> <514CB6D4.9070909@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <514CB6D4.9070909@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-SW-Source: 2013-03/txt/msg00870.txt.bz2 On Fri, 22 Mar 2013 20:53:56 +0100, Pedro Alves wrote: > Debugging a live production system with kgdb? I don't buy > that, really. If one cares to debug the kernel with kgdb, then > one can just as easily patch the kernel, just like one patches the > kernel for security bugs and the like. This is becoming offtopic but the production systems I face are isolated from Internet and no security patches get ever applied. One of the many reasons is to keep all the systems the same, with interchangeable hard drives. > > People still face for example the bug > > new gdbserver is incompat. with old gdb - ;core: suffix > > http://sourceware.org/bugzilla/show_bug.cgi?id=15230 > > which is "fixed" in GDB (also) since 2010: > > commit 3d972c80ab566c07f5e55d356821fb883c9ade88 > > Date: Tue Jan 12 21:40:23 2010 +0000 > > > > I've said before that I regret not having noticed that problem > at patch review time or before the release. I still do. I don't > see the parallel here though. We can't go back in time and fix older > GDBs to cope with ;core... Users can upgrade their GDBs though. In practice in such cases people rather stay with older application, for whatever reason. > Because it's a workaround that does not need to exist. It adds > maintenance burden in the wrong place. The time we've wasted > collectively iterating on this shows it. Otherwise people would need to carry the workarounds downstream. So far I think upstream GDB is supporting workarounds of broken system components. > I like this version much better than the original. If the exception > swallowing ever turns out problematic again, I'll propose reverting > the original patch again. So in interest of moving forward, this one's > fine with me. Checked in: http://sourceware.org/ml/gdb-cvs/2013-03/msg00199.html and for 7.6: http://sourceware.org/ml/gdb-cvs/2013-03/msg00200.html Thanks, jan http://sourceware.org/ml/gdb-cvs/2013-03/msg00199.html --- src/gdb/ChangeLog 2013/03/22 20:25:40 1.15305 +++ src/gdb/ChangeLog 2013/03/22 20:39:28 1.15306 @@ -1,3 +1,19 @@ +2013-03-22 Jan Kratochvil + + * exceptions.h (enum errors): New entry TARGET_CLOSE_ERROR. + * remote.c (trace_error): Remove the special handling of '2'. + (readchar) + (readchar) + (getpkt_or_notif_sane_1): Use TARGET_CLOSE_ERROR for them. + (remote_get_trace_status): Call throw_exception if EX is + TARGET_CLOSE_ERROR. + * utils.c (perror_with_name): Rename to ... + (throw_perror_with_name): ... here. New parameter errcode, describe it + in the function comment. + (perror_with_name): New function wrapper. + * utils.h (enum errors): New stub declaration. + (throw_perror_with_name): New declaration. + 2013-03-22 Pedro Alves Yao Qi Mark Kettenis --- src/gdb/testsuite/ChangeLog 2013/03/21 19:18:25 1.3594 +++ src/gdb/testsuite/ChangeLog 2013/03/22 20:39:28 1.3595 @@ -1,3 +1,9 @@ +2013-03-22 Jan Kratochvil + Pedro Alves + + * gdb.server/server-kill.c: New file. + * gdb.server/server-kill.exp: New file. + 2013-03-21 Pedro Alves * gdb.trace/trace-buffer-size.exp (get default buffer size): --- src/gdb/exceptions.h 2013/01/01 06:32:42 1.38 +++ src/gdb/exceptions.h 2013/03/22 20:39:28 1.39 @@ -86,6 +86,10 @@ /* DW_OP_GNU_entry_value resolving failed. */ NO_ENTRY_VALUE_ERROR, + /* Target throwing an error has been closed. Current command should be + aborted as the inferior state is no longer valid. */ + TARGET_CLOSE_ERROR, + /* Add more errors here. */ NR_ERRORS }; --- src/gdb/remote.c 2013/03/22 19:07:03 1.531 +++ src/gdb/remote.c 2013/03/22 20:39:28 1.532 @@ -430,8 +430,6 @@ else error (_("remote.c: error in outgoing packet at field #%ld."), strtol (buf, NULL, 16)); - case '2': - error (_("trace API error 0x%s."), ++buf); default: error (_("Target returns error code '%s'."), buf); } @@ -7048,12 +7046,13 @@ { case SERIAL_EOF: remote_unpush_target (); - error (_("Remote connection closed")); + throw_error (TARGET_CLOSE_ERROR, _("Remote connection closed")); /* no return */ case SERIAL_ERROR: remote_unpush_target (); - perror_with_name (_("Remote communication error. " - "Target disconnected.")); + throw_perror_with_name (TARGET_CLOSE_ERROR, + _("Remote communication error. " + "Target disconnected.")); /* no return */ case SERIAL_TIMEOUT: break; @@ -7576,7 +7575,9 @@ { QUIT; remote_unpush_target (); - error (_("Watchdog timeout has expired. Target detached.")); + throw_error (TARGET_CLOSE_ERROR, + _("Watchdog timeout has expired. " + "Target detached.")); } if (remote_debug) fputs_filtered ("Timed out.\n", gdb_stdlog); @@ -10699,8 +10700,12 @@ } if (ex.reason < 0) { - exception_fprintf (gdb_stderr, ex, "qTStatus: "); - return -1; + if (ex.error != TARGET_CLOSE_ERROR) + { + exception_fprintf (gdb_stderr, ex, "qTStatus: "); + return -1; + } + throw_exception (ex); } /* If the remote target doesn't do tracing, flag it. */ --- src/gdb/utils.c 2013/03/21 17:37:29 1.295 +++ src/gdb/utils.c 2013/03/22 20:39:28 1.296 @@ -966,11 +966,11 @@ } /* Print the system error message for errno, and also mention STRING - as the file name for which the error was encountered. - Then return to command level. */ + as the file name for which the error was encountered. Use ERRCODE + for the thrown exception. Then return to command level. */ void -perror_with_name (const char *string) +throw_perror_with_name (enum errors errcode, const char *string) { char *err; char *combined; @@ -987,7 +987,15 @@ bfd_set_error (bfd_error_no_error); errno = 0; - error (_("%s."), combined); + throw_error (errcode, _("%s."), combined); +} + +/* See throw_perror_with_name, ERRCODE defaults here to GENERIC_ERROR. */ + +void +perror_with_name (const char *string) +{ + throw_perror_with_name (GENERIC_ERROR, string); } /* Print the system error message for ERRCODE, and also mention STRING --- src/gdb/utils.h 2013/03/21 17:37:29 1.6 +++ src/gdb/utils.h 2013/03/22 20:39:28 1.7 @@ -278,6 +278,9 @@ extern void fprintf_symbol_filtered (struct ui_file *, const char *, enum language, int); +enum errors; +extern void throw_perror_with_name (enum errors errcode, const char *string) + ATTRIBUTE_NORETURN; extern void perror_with_name (const char *) ATTRIBUTE_NORETURN; extern void print_sys_errmsg (const char *, int); --- src/gdb/testsuite/gdb.server/server-kill.c +++ src/gdb/testsuite/gdb.server/server-kill.c 2013-03-22 20:42:07.933259467 +0000 @@ -0,0 +1,24 @@ +/* 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 . */ + +int +main (void) +{ + int i = 0; + + return i; +} --- src/gdb/testsuite/gdb.server/server-kill.exp +++ src/gdb/testsuite/gdb.server/server-kill.exp 2013-03-22 20:42:08.423295331 +0000 @@ -0,0 +1,43 @@ +# 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 . + +load_lib gdbserver-support.exp + +standard_testfile + +if {[skip_gdbserver_tests]} { + return 0 +} + +if { [prepare_for_testing ${testfile}.exp ${testfile}] } { + return -1 +} + +# Make sure we're disconnected, in case we're testing with an +# extended-remote board, therefore already connected. +gdb_test "disconnect" ".*" + +gdbserver_run "" + +# Otherwise the breakpoint at 'main' would not cause insert +# breakpoints during first step. +delete_breakpoints + +set server_pid [exp_pid -i [board_info target fileid]] +remote_exec target "kill -9 $server_pid" + +gdb_test "step" "Remote connection closed"