From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20033 invoked by alias); 15 Mar 2013 19:54:17 -0000 Received: (qmail 20024 invoked by uid 22791); 15 Mar 2013 19:54:15 -0000 X-SWARE-Spam-Status: No, hits=-7.4 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_SPAMHAUS_DROP,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_CP X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 15 Mar 2013 19:54:07 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r2FJs5EH032399 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 15 Mar 2013 15:54:05 -0400 Received: from host2.jankratochvil.net (ovpn-116-42.ams2.redhat.com [10.36.116.42]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r2FJrx1v006758 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Fri, 15 Mar 2013 15:54:02 -0400 Date: Fri, 15 Mar 2013 19:55:00 -0000 From: Jan Kratochvil To: gdb-patches@sourceware.org Cc: Hui Zhu Subject: [patch+7.6] Fix 7.5 regression crashing GDB if gdbserver dies Message-ID: <20130315195359.GA19841@host2.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) 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: 2013-03/txt/msg00692.txt.bz2 Hi, there was a patch: [RFA/tracepoint] Make GDB can work with some old GDB server http://sourceware.org/ml/gdb-patches/2011-06/msg00368.html http://sourceware.org/ml/gdb-patches/2011-07/msg00231.html Message-ID: 872b7668686ffae68f1f3397de9d68512679e8e7 I did not try to find that kgdb but I made a reproducer with: #--- a/gdb/gdbserver/tracepoint.c #+++ b/gdb/gdbserver/tracepoint.c #@@ -4203,7 +4203,8 @@ handle_tracepoint_query (char *packet) # { # if (strcmp ("qTStatus", packet) == 0) # { #- cmd_qtstatus (packet); #+ strcpy (packet, "E22"); #+ if (0) cmd_qtstatus (packet); # return 1; # } # else if (strncmp ("qTP:", packet, strlen ("qTP:")) == 0) which does not work with Hui's original patch so probably kgdb behaves better than this reproducer. This I believe if GDB survives this reproducer such GDB should survive even kgdb. By my change to the Hui's patch I caused a regression if gdbserver dies. In such case GDB crashes as the "Remote connection closed" error gets filtered out but the callers no longer have valid inferior and at least the proceed function then crashes on stale local variable pointer. The original goal of the "Remote connection closed" error was to completely abort the current command. #0 error (string=0xf5d5e0 "Remote connection closed") at utils.c:716 #1 in readchar (timeout=2) at remote.c:7051 #2 in getpkt_or_notif_sane_1 (buf=0x1eb8c60 , sizeof_buf=0x1eb8c68 , forever=0, expecting_notif=0, is_notif=0x0) at remote.c:7566 #3 in getpkt_sane (buf=0x1eb8c60 , sizeof_buf=0x1eb8c68 , forever=0) at remote.c:7664 #4 in getpkt (buf=0x1eb8c60 , sizeof_buf=0x1eb8c68 , forever=0) at remote.c:7506 #5 in remote_get_noisy_reply (buf_p=0x1eb8c60 , sizeof_buf=0x1eb8c68 ) at remote.c:450 #6 in remote_get_trace_status (ts=0x1eba8a0 ) at remote.c:10698 10696 TRY_CATCH (ex, RETURN_MASK_ERROR) 10698 p = remote_get_noisy_reply (&target_buf, &target_buf_size); #7 in remote_can_download_tracepoint () at remote.c:10553 #8 in download_tracepoint_locations () at breakpoint.c:12174 #9 in update_global_location_list (should_insert=1) at breakpoint.c:12661 #10 in insert_breakpoints () at breakpoint.c:2732 #11 in proceed (addr=18446744073709551615, siggnal=GDB_SIGNAL_DEFAULT, step=1) at infrun.c:2243 #12 in step_once (skip_subroutines=0, single_inst=1, count=1, thread=-1) at infcmd.c:1092 #13 in step_1 (skip_subroutines=0, single_inst=1, count_string=0x0) at infcmd.c:927 #14 in stepi_command (count_string=0x0, from_tty=1) at infcmd.c:863 Therefore I limited the TRY_CATCH to 'E2...' errors which are reported as "trace API error 0x%s." so it should hopefully fix the kgdb compatibility problem without causing anything else. No regressions on {x86_64,x86_64-m32,i686}-fedora19pre-linux-gnu and with gdbserver. Thanks, Jan gdb/ 2013-03-15 Jan Kratochvil * exceptions.h (enum errors): New entry TRACE_ERROR. * remote.c (trace_error) <2>: Use throw_error for TRACE_ERROR. (remote_get_trace_status): Call throw_exception if EX is not TRACE_ERROR. gdb/testsuite/ 2013-03-15 Jan Kratochvil * gdb.server/server-kill.c: New file. * gdb.server/server-kill.exp: New file. diff --git a/gdb/exceptions.h b/gdb/exceptions.h index 0d67719..b11af87 100644 --- a/gdb/exceptions.h +++ b/gdb/exceptions.h @@ -86,6 +86,9 @@ enum errors { /* DW_OP_GNU_entry_value resolving failed. */ NO_ENTRY_VALUE_ERROR, + /* remote.c got error from remote end on trace packet. */ + TRACE_ERROR, + /* Add more errors here. */ NR_ERRORS }; diff --git a/gdb/remote.c b/gdb/remote.c index 21d86f7..207180d 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -431,7 +431,7 @@ trace_error (char *buf) error (_("remote.c: error in outgoing packet at field #%ld."), strtol (buf, NULL, 16)); case '2': - error (_("trace API error 0x%s."), ++buf); + throw_error (TRACE_ERROR, _("trace API error 0x%s."), ++buf); default: error (_("Target returns error code '%s'."), buf); } @@ -10703,8 +10699,12 @@ remote_get_trace_status (struct trace_status *ts) } if (ex.reason < 0) { - exception_fprintf (gdb_stderr, ex, "qTStatus: "); - return -1; + if (ex.error == TRACE_ERROR) + { + exception_fprintf (gdb_stderr, ex, "qTStatus: "); + return -1; + } + throw_exception (ex); } /* If the remote target doesn't do tracing, flag it. */ diff --git a/gdb/testsuite/gdb.server/server-kill.c b/gdb/testsuite/gdb.server/server-kill.c new file mode 100644 index 0000000..1949d64 --- /dev/null +++ b/gdb/testsuite/gdb.server/server-kill.c @@ -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; +} diff --git a/gdb/testsuite/gdb.server/server-kill.exp b/gdb/testsuite/gdb.server/server-kill.exp new file mode 100644 index 0000000..3bf5207 --- /dev/null +++ b/gdb/testsuite/gdb.server/server-kill.exp @@ -0,0 +1,55 @@ +# 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 ![is_remote target] { + return 0 +} + +if { [target_info exists sockethost] + && [target_info sockethost] != "localhost:" + && [target_info sockethost] != "stdio" } { + # "remote_exec target" unsuccessfully tries "rsh" even with localhost. + # Therefore use "remote_exec host" instead limiting this testcase for + # testing on localhost only. + return 0 +} + +if { [prepare_for_testing ${testfile}.exp ${testfile}] } { + return -1 +} + +if ![runto_main] { + return -1 +} + +# Otherwise the breakpoint at 'main' would not cause insert breakpoints during +# first step. +delete_breakpoints + +# It should be "remote_exec target" but it does not work, see above. +set server_pid [exp_pid -i [board_info target fileid]] +remote_exec host "kill -9 $server_pid" + +gdb_test "step" "Remote connection closed"