[commit+7.6] [patch+7.6] Fix 7.5 regression crashing GDB if gdbserver dies

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

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  <jan.kratochvil@redhat.com>
+
+	* exceptions.h (enum errors): New entry TARGET_CLOSE_ERROR.
+	* remote.c (trace_error): Remove the special handling of '2'.
+	(readchar) <SERIAL_EOF>
+	(readchar) <SERIAL_ERROR>
+	(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  <palves@redhat.com>
 	    Yao Qi  <yao@codesourcery.com>
 	    Mark Kettenis  <kettenis@gnu.org>
--- 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  <jan.kratochvil@redhat.com>
+	    Pedro Alves  <palves@redhat.com>
+
+	* gdb.server/server-kill.c: New file.
+	* gdb.server/server-kill.exp: New file.
+
 2013-03-21  Pedro Alves  <palves@redhat.com>
 
 	* 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 <http://www.gnu.org/licenses/>.  */
+
+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 <http://www.gnu.org/licenses/>.
+
+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"