[PATCH 2/4] gdbserver: Add debug-file option

[PATCH 2/4] gdbserver: Add debug-file option

[Alan Hayward]

Add command line option to send all debug output to a given file.
Always default back to stderr.

Add matching monitor command. Add documentation.

gdb/doc/ChangeLog:

2019-04-16  Alan Hayward  <alan.hayward@arm.com>

	* gdb.texinfo
	(Other Command-Line Arguments for gdbserver): Add debug-file
	option.
	(Monitor Commands for gdbserver): Likewise.
	(gdbserver man): Likewise.

gdb/gdbserver/ChangeLog:

2019-04-16  Alan Hayward  <alan.hayward@arm.com>

	* debug.c (debug_set_output): New function.
	(debug_vprintf): Send output to debug_file.
	(debug_flush): Likewise.
	* debug.h (debug_set_output): New declaration.
	* server.c (handle_monitor_command): Add debug-file option.
	(captured_main): Likewise.
---
 gdb/doc/gdb.texinfo    | 16 ++++++++++++++--
 gdb/gdbserver/debug.c  | 42 +++++++++++++++++++++++++++++++++++++++---
 gdb/gdbserver/debug.h  |  5 +++++
 gdb/gdbserver/server.c |  6 ++++++
 4 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f410d026b8..1716466609 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21332,8 +21332,11 @@ The @option{--debug} option tells @code{gdbserver} to display extra
 status information about the debugging process.
 @cindex @option{--remote-debug}, @code{gdbserver} option
 The @option{--remote-debug} option tells @code{gdbserver} to display
-remote protocol debug output.  These options are intended for
-@code{gdbserver} development and for bug reports to the developers.
+remote protocol debug output.
+@cindex @option{--debug-file}, @code{gdbserver} option
+The @option{--debug-file=filename} option tells @code{gdbserver} to
+write any debug output to the given file.  These options are intended
+for @code{gdbserver} development and for bug reports to the developers.
 
 @cindex @option{--debug-format}, @code{gdbserver} option
 The @option{--debug-format=option1[,option2,...]} option tells
@@ -21433,6 +21436,10 @@ Disable or enable general debugging messages.
 Disable or enable specific debugging messages associated with the remote
 protocol (@pxref{Remote Protocol}).
 
+@item monitor set debug-file filename
+@itemx monitor set debug-file
+Send any debug output to the given file, or to stderr.
+
 @item monitor set debug-format option1@r{[},option2,...@r{]}
 Specify additional text to add to debugging messages.
 Possible options are:
@@ -44563,6 +44570,11 @@ Instruct @code{gdbserver} to display remote protocol debug output.
 This option is intended for @code{gdbserver} development and for bug reports to
 the developers.
 
+@item --debug-file=filename
+Instruct @code{gdbserver} to send any debug output to the given file.
+This option is intended for @code{gdbserver} development and for bug reports to
+the developers.
+
 @item --debug-format=option1@r{[},option2,...@r{]}
 Instruct @code{gdbserver} to include extra information in each line
 of debugging output.
diff --git a/gdb/gdbserver/debug.c b/gdb/gdbserver/debug.c
index 7c4c77afe2..53647c1ea6 100644
--- a/gdb/gdbserver/debug.c
+++ b/gdb/gdbserver/debug.c
@@ -23,6 +23,9 @@
 int remote_debug = 0;
 #endif
 
+/* Output file for debugging.  Default to standard error.  */
+FILE *debug_file = stderr;
+
 /* Enable miscellaneous debugging output.  The name is historical - it
    was originally used to debug LinuxThreads support.  */
 int debug_threads;
@@ -30,6 +33,39 @@ int debug_threads;
 /* Include timestamps in debugging output.  */
 int debug_timestamp;
 
+#if !defined (IN_PROCESS_AGENT)
+
+/* See debug.h.  */
+
+void
+debug_set_output (const char *new_debug_file)
+{
+  /* Close any existing file and reset to standard error.  */
+  if (debug_file != stderr)
+    {
+      fclose (debug_file);
+    }
+  debug_file = stderr;
+
+  /* Catch empty filenames.  */
+  if (new_debug_file == nullptr || strlen (new_debug_file) == 0)
+    return;
+
+  FILE *fptr = fopen (new_debug_file, "w");
+
+  if (fptr == nullptr)
+    {
+      debug_printf ("Cannot open %s for writing. %s. Switching to stderr.\n",
+		    new_debug_file, strerror (errno));
+      return;
+    }
+
+  debug_file = fptr;
+  return;
+}
+
+#endif
+
 /* Print a debugging message.
    If the text begins a new line it is preceded by a timestamp.
    We don't get fancy with newline checking, we just check whether the
@@ -50,11 +86,11 @@ debug_vprintf (const char *format, va_list ap)
       seconds s = duration_cast<seconds> (now.time_since_epoch ());
       microseconds us = duration_cast<microseconds> (now.time_since_epoch ()) - s;
 
-      fprintf (stderr, "%ld.%06ld ", (long) s.count (), (long) us.count ());
+      fprintf (debug_file, "%ld.%06ld ", (long) s.count (), (long) us.count ());
     }
 #endif
 
-  vfprintf (stderr, format, ap);
+  vfprintf (debug_file, format, ap);
 
 #if !defined (IN_PROCESS_AGENT)
   if (*format)
@@ -69,7 +105,7 @@ debug_vprintf (const char *format, va_list ap)
 void
 debug_flush (void)
 {
-  fflush (stderr);
+  fflush (debug_file);
 }
 
 /* Notify the user that the code is entering FUNCTION_NAME.
diff --git a/gdb/gdbserver/debug.h b/gdb/gdbserver/debug.h
index c8d5e3365e..f65c91c9eb 100644
--- a/gdb/gdbserver/debug.h
+++ b/gdb/gdbserver/debug.h
@@ -21,6 +21,11 @@
 
 #if !defined (IN_PROCESS_AGENT)
 extern int remote_debug;
+
+/* Switch all debug output to DEBUG_FILE.  If DEBUG_FILE is nullptr or an
+   empty string, or if the file cannot be opened, then debug output is sent to
+   stderr.  */
+void debug_set_output (const char *debug_file);
 #endif
 
 extern int debug_threads;
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 3f6c849dbc..36510ad1b2 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1403,6 +1403,10 @@ handle_monitor_command (char *mon, char *own_buf)
 	  write_enn (own_buf);
 	}
     }
+  else if (strcmp (mon, "set debug-file") == 0)
+    debug_set_output (nullptr);
+  else if (startswith (mon, "set debug-file "))
+    debug_set_output (mon + sizeof ("set debug-file ") - 1);
   else if (strcmp (mon, "help") == 0)
     monitor_show_help ();
   else if (strcmp (mon, "exit") == 0)
@@ -3649,6 +3653,8 @@ captured_main (int argc, char *argv[])
 	}
       else if (strcmp (*next_arg, "--remote-debug") == 0)
 	remote_debug = 1;
+      else if (startswith (*next_arg, "--debug-file="))
+	debug_set_output ((*next_arg) + sizeof ("--debug-file=") -1);
       else if (strcmp (*next_arg, "--disable-packet") == 0)
 	{
 	  gdbserver_show_disableable (stdout);
-- 
2.20.1 (Apple Git-117)

[PATCH 4/4] testsuite: Add option to capture gdbserver debug

[Alan Hayward]

Add board option which enables gdbserver debug and sends it to the
file gdbserver.log, located in the output directory for the current
test.  Document this.

Add debug versions of the native gdbserver board files.

Disable tspeed.exp when debugging to prevent the log file filling
many gigabytes then timing out.

gdb/testsuite/ChangeLog:

2019-04-16  Alan Hayward  <alan.hayward@arm.com>

	* README (gdbserver,debug): Add board setting.
	* boards/native-extended-gdbserver-debug.exp: New file.
	* boards/native-gdbserver-debug.exp: New file.
	* gdb.trace/tspeed.exp: Skip when debugging.
	* lib/gdbserver-support.exp: Check for gdbserver,debug.
---
 gdb/testsuite/README                          |  8 ++++++
 .../native-extended-gdbserver-debug.exp       | 26 +++++++++++++++++++
 .../boards/native-gdbserver-debug.exp         | 25 ++++++++++++++++++
 gdb/testsuite/gdb.trace/tspeed.exp            |  5 ++++
 gdb/testsuite/lib/gdbserver-support.exp       | 16 +++++++++++-
 5 files changed, 79 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/boards/native-extended-gdbserver-debug.exp
 create mode 100644 gdb/testsuite/boards/native-gdbserver-debug.exp

diff --git a/gdb/testsuite/README b/gdb/testsuite/README
index db90ea4698..01595d35aa 100644
--- a/gdb/testsuite/README
+++ b/gdb/testsuite/README
@@ -497,6 +497,14 @@ gdb,nopie_flag
   The flag required to force the compiler to produce non-position-independent
   executables.
 
+gdbserver,debug
+
+  When set gdbserver debug is outputed to the file gdbserver.log in the test
+  output directory.  Valid values are:
+  "debug"  - turn on gdbserver debug.
+  "remote" - turn on gdbserver remote debug.
+  "all" - turn on all the above debug options.
+
 Testsuite Organization
 **********************
 
diff --git a/gdb/testsuite/boards/native-extended-gdbserver-debug.exp b/gdb/testsuite/boards/native-extended-gdbserver-debug.exp
new file mode 100644
index 0000000000..aa79416081
--- /dev/null
+++ b/gdb/testsuite/boards/native-extended-gdbserver-debug.exp
@@ -0,0 +1,26 @@
+# Copyright 2019 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/>.
+
+# This file is a dejagnu "board file" and is used to run the testsuite
+# natively with gdbserver, in extended-remote mode, with gdbserver debug
+# turned on.
+#
+# To use this file:
+# bash$ cd ${build_dir}/gdb
+# bash$ make check RUNTESTFLAGS="--target_board=native-extended-gdbserver-debug"
+
+load_board_description "native-extended-gdbserver"
+
+set_board_info gdbserver,debug "all"
diff --git a/gdb/testsuite/boards/native-gdbserver-debug.exp b/gdb/testsuite/boards/native-gdbserver-debug.exp
new file mode 100644
index 0000000000..ffca2cb8d5
--- /dev/null
+++ b/gdb/testsuite/boards/native-gdbserver-debug.exp
@@ -0,0 +1,25 @@
+# Copyright 2019 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/>.
+
+# This file is a dejagnu "board file" and is used to run the testsuite
+# natively with gdbserver, with gdbserver debug turned on.
+#
+# To use this file:
+# bash$ cd ${build_dir}/gdb
+# bash$ make check RUNTESTFLAGS="--target_board=native-gdbserver-debug"
+
+load_board_description "native-gdbserver"
+
+set_board_info gdbserver,debug "all"
diff --git a/gdb/testsuite/gdb.trace/tspeed.exp b/gdb/testsuite/gdb.trace/tspeed.exp
index 6fd812e4f6..70a8e1f5b4 100644
--- a/gdb/testsuite/gdb.trace/tspeed.exp
+++ b/gdb/testsuite/gdb.trace/tspeed.exp
@@ -19,6 +19,11 @@ if {[skip_shlib_tests]} {
     return 0
 }
 
+# Do not run if gdbsever debug is enabled - the output file is many Gb.
+if [target_info exists gdbserver,debug] {
+    return 0
+}
+
 standard_testfile
 set executable $testfile
 
diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index 2cb64f7d2f..745cf670b1 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -283,12 +283,26 @@ proc gdbserver_start { options arguments } {
 	# If gdbserver_reconnect will be called $gdbserver_reconnect_p must be
 	# set to true already during gdbserver_start.
 	global gdbserver_reconnect_p
+	global srcdir
+	global subdir
 	if {![info exists gdbserver_reconnect_p] || !$gdbserver_reconnect_p} {
 	    # GDB client could accidentally connect to a stale server.
-	    # append gdbserver_command " --debug --once"
 	    append gdbserver_command " --once"
 	}
 
+	# Set debug according to the board setting.
+	if [target_info exists gdbserver,debug] {
+	  set gdbserverdebug [target_info gdbserver,debug]
+	  set debugfile [standard_output_file gdbserver.log]
+	  if { $gdbserverdebug == "debug" } {
+	    append gdbserver_command " --debug --debug-file=$debugfile"
+	  } elseif { $gdbserverdebug == "remote" } {
+	    append gdbserver_command " --remote-debug --debug-file=$debugfile"
+	  } elseif { $gdbserverdebug == "all" } {
+	    append gdbserver_command " --debug --remote-debug --debug-file=$debugfile"
+	  }
+	}
+
 	if { $options != "" } {
 	    append gdbserver_command " $options"
 	}
-- 
2.20.1 (Apple Git-117)

[PATCH 0/4] gdbserver/testsuite : Capture gdbserver debug output during testing

[Alan Hayward]

Trying to debug some of the racy failures when running native-gdbserver, I
found it would be very useful to enable the debug output from gdbserver.
However, doing this results in the output getting mixed with the standard
test output causing the testsuite to fall over horribly, with most tests
timing out.

This set of patches adds a flag to gdbserver to send all debug output to a
file, makes sure none of gdbserver writes to stderr directly, and adds a
board option to use the flags.  Using this board option, debug is safely 
sent to gdbserver.log in the directory for each test, without effecting the
test itself.

I've added two board files native-gdbserver-debug and
native-remote-gdbserver-debug that auto enable all the debug. When testing I
found no regressions when switching from the original non-debug versions.

Example results of a test after a testsuite run:
$ ls -lh outputs/gdb.base/arrayidx/
total 64K
-rwxrwxr-x 1 alahay01 alahay01  11K Apr 16 10:57 arrayidx
-rw-rw-r-- 1 alahay01 alahay01 6.4K Apr 16 10:57 gdb.log
-rw-rw-r-- 1 alahay01 alahay01  37K Apr 16 10:57 gdbserver.log
-rw-rw-r-- 1 alahay01 alahay01  901 Apr 16 10:57 gdb.sum

I don't think these options should be on in any of the exisiting boards as
they could effect the timings of the test.

The first patch is a cleanup which I'm happy to delete if people disagree.

Not sure if I should add a statement to the NEWS file (can add in a follow
on patch).


Alan Hayward (4):
  gdbserver: Move remote_debug to a single place
  gdbserver: Add debug-file option
  gdbserver: Ensure all debug output uses debug functions
  testsuite: Add option to capture gdbserver debug

 gdb/doc/gdb.texinfo                           | 16 +++++-
 gdb/gdbserver/ax.c                            |  4 ++
 gdb/gdbserver/debug.c                         | 55 ++++++++++++++++++-
 gdb/gdbserver/debug.h                         | 19 ++++---
 gdb/gdbserver/hostio.c                        |  2 -
 gdb/gdbserver/linux-low.c                     |  7 +--
 gdb/gdbserver/remote-utils.c                  |  3 -
 gdb/gdbserver/remote-utils.h                  |  2 -
 gdb/gdbserver/server.c                        |  8 ++-
 gdb/nat/linux-waitpid.c                       |  2 +-
 gdb/testsuite/README                          |  8 +++
 .../native-extended-gdbserver-debug.exp       | 26 +++++++++
 .../boards/native-gdbserver-debug.exp         | 25 +++++++++
 gdb/testsuite/gdb.trace/tspeed.exp            |  5 ++
 gdb/testsuite/lib/gdbserver-support.exp       | 16 +++++-
 15 files changed, 171 insertions(+), 27 deletions(-)
 create mode 100644 gdb/testsuite/boards/native-extended-gdbserver-debug.exp
 create mode 100644 gdb/testsuite/boards/native-gdbserver-debug.exp

-- 
2.20.1 (Apple Git-117)

[PATCH 3/4] gdbserver: Ensure all debug output uses debug functions

[Alan Hayward]

All debug output needs to go via debug functions to ensure it writes to the
correct output stream.

gdb/ChangeLog:

2019-04-16  Alan Hayward  <alan.hayward@arm.com>

	* nat/linux-waitpid.c (linux_debug): Call debug_vprintf.

gdb/gdbserver/ChangeLog:

2019-04-16  Alan Hayward  <alan.hayward@arm.com>

	* ax.c (ax_vdebug): Call debug_printf.
	* debug.c (debug_write): New function.
	* debug.h (debug_write): New declaration.
	* linux-low.c (sigchld_handler): Call debug_write.
---
 gdb/gdbserver/ax.c        | 4 ++++
 gdb/gdbserver/debug.c     | 9 +++++++++
 gdb/gdbserver/debug.h     | 3 +++
 gdb/gdbserver/linux-low.c | 7 +++----
 gdb/nat/linux-waitpid.c   | 2 +-
 5 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/gdb/gdbserver/ax.c b/gdb/gdbserver/ax.c
index a16fba1ccd..7b8df91749 100644
--- a/gdb/gdbserver/ax.c
+++ b/gdb/gdbserver/ax.c
@@ -36,7 +36,11 @@ ax_vdebug (const char *fmt, ...)
 
   va_start (ap, fmt);
   vsprintf (buf, fmt, ap);
+#ifdef IN_PROCESS_AGENT
   fprintf (stderr, PROG "/ax: %s\n", buf);
+#else
+  debug_printf (PROG "/ax: %s\n", buf);
+#endif
   va_end (ap);
 }
 
diff --git a/gdb/gdbserver/debug.c b/gdb/gdbserver/debug.c
index 53647c1ea6..adb35292eb 100644
--- a/gdb/gdbserver/debug.c
+++ b/gdb/gdbserver/debug.c
@@ -131,3 +131,12 @@ do_debug_exit (const char *function_name)
   if (function_name != NULL)
     debug_printf ("<<<< exiting %s\n", function_name);
 }
+
+/* See debug.h.  */
+
+size_t
+debug_write (const void *buf, size_t nbyte)
+{
+  int fd = fileno (debug_file);
+  return write (fd, buf, nbyte);
+}
diff --git a/gdb/gdbserver/debug.h b/gdb/gdbserver/debug.h
index f65c91c9eb..29e58ad8a4 100644
--- a/gdb/gdbserver/debug.h
+++ b/gdb/gdbserver/debug.h
@@ -35,6 +35,9 @@ void debug_flush (void);
 void do_debug_enter (const char *function_name);
 void do_debug_exit (const char *function_name);
 
+/* Async signal safe debug output function that calls write directly.  */
+size_t debug_write (const void *buf, size_t nbyte);
+
 /* These macros are for use in major functions that produce a lot of
    debugging output.  They help identify in the mass of debugging output
    when these functions enter and exit.  debug_enter is intended to be
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 168f4b2abc..917b1c290b 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -6185,10 +6185,9 @@ sigchld_handler (int signo)
     {
       do
 	{
-	  /* fprintf is not async-signal-safe, so call write
-	     directly.  */
-	  if (write (2, "sigchld_handler\n",
-		     sizeof ("sigchld_handler\n") - 1) < 0)
+	  /* Use the async signal safe debug function.  */
+	  if (debug_write ("sigchld_handler\n",
+			   sizeof ("sigchld_handler\n") - 1) < 0)
 	    break; /* just ignore */
 	} while (0);
     }
diff --git a/gdb/nat/linux-waitpid.c b/gdb/nat/linux-waitpid.c
index e31c088f66..a7d11ab8d3 100644
--- a/gdb/nat/linux-waitpid.c
+++ b/gdb/nat/linux-waitpid.c
@@ -42,7 +42,7 @@ linux_debug (const char *format, ...)
     {
       va_list args;
       va_start (args, format);
-      vfprintf (stderr, format, args);
+      debug_vprintf (format, args);
       va_end (args);
     }
 #endif
-- 
2.20.1 (Apple Git-117)

[PATCH 1/4] gdbserver: Move remote_debug to a single place

[Alan Hayward]

A comment in debug.h (written in 2014) states: "We declare debug format
variables here, and debug_threads but no other debug content variables
(e.g., not remote_debug) because while this file is not currently used by
IPA it may be some day, and IPA may have its own set of debug content
variables".

This has resulted in remote_debug being declared in many .c/.h files
throughout gdbserver.

It would be much simplier to define it one place.  The most logical place to
define it is in debug.h, surrounded by #define guards.  If IPA is changed,
then at that point the variable can be moved elsewhere.

Also removes the unused gdb_stdlog variable.

gdb/gdbserver/ChangeLog:

2019-04-16  Alan Hayward  <alan.hayward@arm.com>

	* debug.c (remote_debug): Add definition.
	* debug.h (remote_debug): Add declaration.
	* hostio.c (remote_debug): Remove declaration.
	* remote-utils.c (struct ui_file): Likewise.
	(remote_debug): Likewise.
	* remote-utils.h (remote_debug): Likewise,
	* server.c (remote_debug): Remove definition.
---
 gdb/gdbserver/debug.c        |  4 ++++
 gdb/gdbserver/debug.h        | 11 ++++-------
 gdb/gdbserver/hostio.c       |  2 --
 gdb/gdbserver/remote-utils.c |  3 ---
 gdb/gdbserver/remote-utils.h |  2 --
 gdb/gdbserver/server.c       |  2 --
 6 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/gdb/gdbserver/debug.c b/gdb/gdbserver/debug.c
index a5b791b6df..7c4c77afe2 100644
--- a/gdb/gdbserver/debug.c
+++ b/gdb/gdbserver/debug.c
@@ -19,6 +19,10 @@
 #include "server.h"
 #include <chrono>
 
+#if !defined (IN_PROCESS_AGENT)
+int remote_debug = 0;
+#endif
+
 /* Enable miscellaneous debugging output.  The name is historical - it
    was originally used to debug LinuxThreads support.  */
 int debug_threads;
diff --git a/gdb/gdbserver/debug.h b/gdb/gdbserver/debug.h
index e40f28c771..c8d5e3365e 100644
--- a/gdb/gdbserver/debug.h
+++ b/gdb/gdbserver/debug.h
@@ -19,13 +19,10 @@
 #ifndef GDBSERVER_DEBUG_H
 #define GDBSERVER_DEBUG_H
 
-/* We declare debug format variables here, and debug_threads but no other
-   debug content variables (e.g., not remote_debug) because while this file
-   is not currently used by IPA it may be some day, and IPA may have its own
-   set of debug content variables.  It's ok to declare debug_threads here
-   because it is misnamed - a better name is debug_basic or some such,
-   which can work for any program, gdbserver or IPA.  If/when this file is
-   used with IPA it is recommended to fix debug_thread's name.  */
+#if !defined (IN_PROCESS_AGENT)
+extern int remote_debug;
+#endif
+
 extern int debug_threads;
 extern int debug_timestamp;
 
diff --git a/gdb/gdbserver/hostio.c b/gdb/gdbserver/hostio.c
index cf75de0c00..eedf6d9fe7 100644
--- a/gdb/gdbserver/hostio.c
+++ b/gdb/gdbserver/hostio.c
@@ -29,8 +29,6 @@
 #include <sys/stat.h>
 #include "common/fileio.h"
 
-extern int remote_debug;
-
 struct fd_list
 {
   int fd;
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 4e6f9c62de..903d77349d 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -108,9 +108,6 @@ struct sym_cache
   struct sym_cache *next;
 };
 
-int remote_debug = 0;
-struct ui_file *gdb_stdlog;
-
 static int remote_is_stdio = 0;
 
 static gdb_fildes_t remote_desc = INVALID_DESCRIPTOR;
diff --git a/gdb/gdbserver/remote-utils.h b/gdb/gdbserver/remote-utils.h
index 587afdb028..4ca5d9435e 100644
--- a/gdb/gdbserver/remote-utils.h
+++ b/gdb/gdbserver/remote-utils.h
@@ -19,8 +19,6 @@
 #ifndef GDBSERVER_REMOTE_UTILS_H
 #define GDBSERVER_REMOTE_UTILS_H
 
-extern int remote_debug;
-
 int gdb_connected (void);
 
 #define STDIO_CONNECTION_NAME "stdio"
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index f2c8f15169..3f6c849dbc 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -328,8 +328,6 @@ attach_inferior (int pid)
   return 0;
 }
 
-extern int remote_debug;
-
 /* Decode a qXfer read request.  Return 0 if everything looks OK,
    or -1 otherwise.  */
 
-- 
2.20.1 (Apple Git-117)

Re: [PATCH 2/4] gdbserver: Add debug-file option

[Eli Zaretskii]

> From: Alan Hayward <Alan.Hayward@arm.com>
> CC: nd <nd@arm.com>, Alan Hayward <Alan.Hayward@arm.com>
> Date: Tue, 16 Apr 2019 10:17:41 +0000
> 
> gdb/doc/ChangeLog:
> 
> 2019-04-16  Alan Hayward  <alan.hayward@arm.com>
> 
> 	* gdb.texinfo
> 	(Other Command-Line Arguments for gdbserver): Add debug-file

The node name in parentheses should be on the same line as
gdb.texinfo.

> 	(Monitor Commands for gdbserver): Likewise.
> 	(gdbserver man): Likewise.

It is better to have a list of node names in parentheses, with only
one description, than having separate entries that say "Likewise".

> +@cindex @option{--debug-file}, @code{gdbserver} option

I think it would be good to have here an additional index entry:

  @cindex @code{gdbserver}, send all debug output to a single file

> +The @option{--debug-file=filename} option tells @code{gdbserver} to
                            ^^^^^^^^
"filename" should be in @var here, as it is a parameter.

> +write any debug output to the given file.  These options are intended
                          ^^^^^^^^^^^^^^^^^
"to the given @var{filename}", so as to reference the parameter.

> +@item --debug-file=filename
> +Instruct @code{gdbserver} to send any debug output to the given file.

Same here.

The documentation changes are okay with these nits fixed.

Thanks.

Re: [PATCH 0/4] gdbserver/testsuite : Capture gdbserver debug output during testing

[Eli Zaretskii]

> From: Alan Hayward <Alan.Hayward@arm.com>
> CC: nd <nd@arm.com>, Alan Hayward <Alan.Hayward@arm.com>
> Date: Tue, 16 Apr 2019 10:17:35 +0000
> 
> Not sure if I should add a statement to the NEWS file (can add in a follow
> on patch).

I think you should.

Re: [PATCH 1/4] gdbserver: Move remote_debug to a single place

[Tom Tromey]

>>>>> "Alan" == Alan Hayward <Alan.Hayward@arm.com> writes:

Alan> Also removes the unused gdb_stdlog variable.

I looked and this is referenced from common/observable.h; but I suppose
that could be converted to use debug_vprintf or similar if it's ever
used in gdbserver.

Alan> 2019-04-16  Alan Hayward  <alan.hayward@arm.com>
Alan> 	* debug.c (remote_debug): Add definition.
Alan> 	* debug.h (remote_debug): Add declaration.
Alan> 	* hostio.c (remote_debug): Remove declaration.
Alan> 	* remote-utils.c (struct ui_file): Likewise.
Alan> 	(remote_debug): Likewise.
Alan> 	* remote-utils.h (remote_debug): Likewise,
Alan> 	* server.c (remote_debug): Remove definition.

Thanks, this is ok.

Tom

Re: [PATCH 2/4] gdbserver: Add debug-file option

[Tom Tromey]

>>>>> "Alan" == Alan Hayward <Alan.Hayward@arm.com> writes:

Alan> Add command line option to send all debug output to a given file.
Alan> Always default back to stderr.

Alan> Add matching monitor command. Add documentation.

Thanks.  The code parts of this are ok.

One little nit:

Alan> +  debug_file = fptr;
Alan> +  return;
Alan> +}

...this "return;" can be removed.

Tom

Re: [PATCH 3/4] gdbserver: Ensure all debug output uses debug functions

[Tom Tromey]

>>>>> "Alan" == Alan Hayward <Alan.Hayward@arm.com> writes:

Alan> All debug output needs to go via debug functions to ensure it writes to the
Alan> correct output stream.

Alan> gdb/ChangeLog:

Alan> 2019-04-16  Alan Hayward  <alan.hayward@arm.com>

Alan> 	* nat/linux-waitpid.c (linux_debug): Call debug_vprintf.

Alan> gdb/gdbserver/ChangeLog:

Alan> 2019-04-16  Alan Hayward  <alan.hayward@arm.com>

Alan> 	* ax.c (ax_vdebug): Call debug_printf.
Alan> 	* debug.c (debug_write): New function.
Alan> 	* debug.h (debug_write): New declaration.
Alan> 	* linux-low.c (sigchld_handler): Call debug_write.

Thanks, this is ok.

Tom

Re: [PATCH 4/4] testsuite: Add option to capture gdbserver debug

[Tom Tromey]

>>>>> "Alan" == Alan Hayward <Alan.Hayward@arm.com> writes:

Alan> Add board option which enables gdbserver debug and sends it to the
Alan> file gdbserver.log, located in the output directory for the current
Alan> test.  Document this.

Alan> Add debug versions of the native gdbserver board files.

Alan> Disable tspeed.exp when debugging to prevent the log file filling
Alan> many gigabytes then timing out.

Thanks.

Alan> +gdbserver,debug
Alan> +
Alan> +  When set gdbserver debug is outputed to the file gdbserver.log in the test

I think it should say "is sent" rather than "is outputed".

Alan> diff --git a/gdb/testsuite/boards/native-extended-gdbserver-debug.exp b/gdb/testsuite/boards/native-extended-gdbserver-debug.exp

I wonder if a new board is needed for this?
Could it be done some other way, like a command-line setting?

TBH I'm not sure what the typical approach is for something like this.

Alan>  	if {![info exists gdbserver_reconnect_p] || !$gdbserver_reconnect_p} {
Alan>  	    # GDB client could accidentally connect to a stale server.
Alan> -	    # append gdbserver_command " --debug --once"
Alan>  	    append gdbserver_command " --once"

Was this intentional?


One random thought I had about this series is that it would be nice to
have a way to get "set remotelogfile" output in the test directory.
That way a failing test could be re-run with gdbreplay without much
trouble.

Tom

Re: [PATCH 2/4] gdbserver: Add debug-file option

[Alan Hayward]

> 
> On 16 Apr 2019, at 15:43, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Alan Hayward <Alan.Hayward@arm.com>
>> CC: nd <nd@arm.com>, Alan Hayward <Alan.Hayward@arm.com>
>> Date: Tue, 16 Apr 2019 10:17:41 +0000
>> 
>> gdb/doc/ChangeLog:
>> 
>> 2019-04-16  Alan Hayward  <alan.hayward@arm.com>
>> 
>> 	* gdb.texinfo
>> 	(Other Command-Line Arguments for gdbserver): Add debug-file
> 
> The node name in parentheses should be on the same line as
> gdb.texinfo.

Fixed.

> 
>> 	(Monitor Commands for gdbserver): Likewise.
>> 	(gdbserver man): Likewise.
> 
> It is better to have a list of node names in parentheses, with only
> one description, than having separate entries that say "Likewise”.

Wasn’t aware it could be done that way. Fixed.

> 
>> +@cindex @option{--debug-file}, @code{gdbserver} option
> 
> I think it would be good to have here an additional index entry:
> 
> @cindex @code{gdbserver}, send all debug output to a single file

Makes sense. Added.

> 
>> +The @option{--debug-file=filename} option tells @code{gdbserver} to
>                           ^^^^^^^^
> "filename" should be in @var here, as it is a parameter.

Done.

> 
>> +write any debug output to the given file.  These options are intended
>                         ^^^^^^^^^^^^^^^^^
> "to the given @var{filename}", so as to reference the parameter.

Done.

> 
>> +@item --debug-file=filename
>> +Instruct @code{gdbserver} to send any debug output to the given file.
> 
> Same here.

Done.

> 
> The documentation changes are okay with these nits fixed.
> 
> Thanks.

> 
> On 16 Apr 2019, at 20:39, Tom Tromey <tom@tromey.com> wrote:
> 
>>>>>> "Alan" == Alan Hayward <Alan.Hayward@arm.com> writes:
> 
> Alan> Add command line option to send all debug output to a given file.
> Alan> Always default back to stderr.
> 
> Alan> Add matching monitor command. Add documentation.
> 
> Thanks.  The code parts of this are ok.
> 
> One little nit:
> 
> Alan> +  debug_file = fptr;
> Alan> +  return;
> Alan> +}
> 
> ...this "return;" can be removed.
> 

Done.



Thanks!

Pushed the following:




diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 27e65ea56c..64073278fa 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,9 @@
+2019-04-17  Alan Hayward  <alan.hayward@arm.com>
+
+       * gdb.texinfo (Other Command-Line Arguments for gdbserver)
+       (Monitor Commands for gdbserver)
+       (gdbserver man): Add debug-file option.
+
2019-04-08  Kevin Buettner  <kevinb@redhat.com>

       * python.texi (Inferiors In Python): Rename
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f410d026b8..a3a5f3e28c 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21332,8 +21332,12 @@ The @option{--debug} option tells @code{gdbserver} to display extra
status information about the debugging process.
@cindex @option{--remote-debug}, @code{gdbserver} option
The @option{--remote-debug} option tells @code{gdbserver} to display
-remote protocol debug output.  These options are intended for
-@code{gdbserver} development and for bug reports to the developers.
+remote protocol debug output.
+@cindex @option{--debug-file}, @code{gdbserver} option
+@cindex @code{gdbserver}, send all debug output to a single file
+The @option{--debug-file=@var{filename}} option tells @code{gdbserver} to
+write any debug output to the given @var{filename}.  These options are intended
+for @code{gdbserver} development and for bug reports to the developers.

@cindex @option{--debug-format}, @code{gdbserver} option
The @option{--debug-format=option1[,option2,...]} option tells
@@ -21433,6 +21437,10 @@ Disable or enable general debugging messages.
Disable or enable specific debugging messages associated with the remote
protocol (@pxref{Remote Protocol}).

+@item monitor set debug-file filename
+@itemx monitor set debug-file
+Send any debug output to the given file, or to stderr.
+
@item monitor set debug-format option1@r{[},option2,...@r{]}
Specify additional text to add to debugging messages.
Possible options are:
@@ -44563,6 +44571,11 @@ Instruct @code{gdbserver} to display remote protocol debug output.
This option is intended for @code{gdbserver} development and for bug reports to
the developers.

+@item --debug-file=@var{filename}
+Instruct @code{gdbserver} to send any debug output to the given @var{filename}.
+This option is intended for @code{gdbserver} development and for bug reports to
+the developers.
+
@item --debug-format=option1@r{[},option2,...@r{]}
Instruct @code{gdbserver} to include extra information in each line
of debugging output.
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 0581f59b5d..d3380d6145 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,12 @@
+2019-04-17  Alan Hayward  <alan.hayward@arm.com>
+
+       * debug.c (debug_set_output): New function.
+       (debug_vprintf): Send output to debug_file.
+       (debug_flush): Likewise.
+       * debug.h (debug_set_output): New declaration.
+       * server.c (handle_monitor_command): Add debug-file option.
+       (captured_main): Likewise.
+
2019-04-17  Alan Hayward  <alan.hayward@arm.com>

       * debug.c (remote_debug): Add definition.
diff --git a/gdb/gdbserver/debug.c b/gdb/gdbserver/debug.c
index 7c4c77afe2..d80cd52540 100644
--- a/gdb/gdbserver/debug.c
+++ b/gdb/gdbserver/debug.c
@@ -23,6 +23,9 @@
int remote_debug = 0;
#endif

+/* Output file for debugging.  Default to standard error.  */
+FILE *debug_file = stderr;
+
/* Enable miscellaneous debugging output.  The name is historical - it
   was originally used to debug LinuxThreads support.  */
int debug_threads;
@@ -30,6 +33,38 @@ int debug_threads;
/* Include timestamps in debugging output.  */
int debug_timestamp;

+#if !defined (IN_PROCESS_AGENT)
+
+/* See debug.h.  */
+
+void
+debug_set_output (const char *new_debug_file)
+{
+  /* Close any existing file and reset to standard error.  */
+  if (debug_file != stderr)
+    {
+      fclose (debug_file);
+    }
+  debug_file = stderr;
+
+  /* Catch empty filenames.  */
+  if (new_debug_file == nullptr || strlen (new_debug_file) == 0)
+    return;
+
+  FILE *fptr = fopen (new_debug_file, "w");
+
+  if (fptr == nullptr)
+    {
+      debug_printf ("Cannot open %s for writing. %s. Switching to stderr.\n",
+                   new_debug_file, strerror (errno));
+      return;
+    }
+
+  debug_file = fptr;
+}
+
+#endif
+
/* Print a debugging message.
   If the text begins a new line it is preceded by a timestamp.
   We don't get fancy with newline checking, we just check whether the
@@ -50,11 +85,11 @@ debug_vprintf (const char *format, va_list ap)
      seconds s = duration_cast<seconds> (now.time_since_epoch ());
      microseconds us = duration_cast<microseconds> (now.time_since_epoch ()) - s;

-      fprintf (stderr, "%ld.%06ld ", (long) s.count (), (long) us.count ());
+      fprintf (debug_file, "%ld.%06ld ", (long) s.count (), (long) us.count ());
    }
#endif

-  vfprintf (stderr, format, ap);
+  vfprintf (debug_file, format, ap);

#if !defined (IN_PROCESS_AGENT)
  if (*format)
@@ -69,7 +104,7 @@ debug_vprintf (const char *format, va_list ap)
void
debug_flush (void)
{
-  fflush (stderr);
+  fflush (debug_file);
}

/* Notify the user that the code is entering FUNCTION_NAME.
diff --git a/gdb/gdbserver/debug.h b/gdb/gdbserver/debug.h
index c8d5e3365e..f65c91c9eb 100644
--- a/gdb/gdbserver/debug.h
+++ b/gdb/gdbserver/debug.h
@@ -21,6 +21,11 @@

#if !defined (IN_PROCESS_AGENT)
extern int remote_debug;
+
+/* Switch all debug output to DEBUG_FILE.  If DEBUG_FILE is nullptr or an
+   empty string, or if the file cannot be opened, then debug output is sent to
+   stderr.  */
+void debug_set_output (const char *debug_file);
#endif

extern int debug_threads;
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 3f6c849dbc..36510ad1b2 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1403,6 +1403,10 @@ handle_monitor_command (char *mon, char *own_buf)
         write_enn (own_buf);
       }
    }
+  else if (strcmp (mon, "set debug-file") == 0)
+    debug_set_output (nullptr);
+  else if (startswith (mon, "set debug-file "))
+    debug_set_output (mon + sizeof ("set debug-file ") - 1);
  else if (strcmp (mon, "help") == 0)
    monitor_show_help ();
  else if (strcmp (mon, "exit") == 0)
@@ -3649,6 +3653,8 @@ captured_main (int argc, char *argv[])
       }
      else if (strcmp (*next_arg, "--remote-debug") == 0)
       remote_debug = 1;
+      else if (startswith (*next_arg, "--debug-file="))
+       debug_set_output ((*next_arg) + sizeof ("--debug-file=") -1);
      else if (strcmp (*next_arg, "--disable-packet") == 0)
       {
         gdbserver_show_disableable (stdout);

Re: [PATCH 4/4] testsuite: Add option to capture gdbserver debug

[Alan Hayward]

(Patches 1 to 3 pushed).

> On 16 Apr 2019, at 20:49, Tom Tromey <tom@tromey.com> wrote:
> 
>>>>>> "Alan" == Alan Hayward <Alan.Hayward@arm.com> writes:
> 
> Alan> Add board option which enables gdbserver debug and sends it to the
> Alan> file gdbserver.log, located in the output directory for the current
> Alan> test.  Document this.
> 
> Alan> Add debug versions of the native gdbserver board files.
> 
> Alan> Disable tspeed.exp when debugging to prevent the log file filling
> Alan> many gigabytes then timing out.
> 
> Thanks.
> 
> Alan> +gdbserver,debug
> Alan> +
> Alan> +  When set gdbserver debug is outputed to the file gdbserver.log in the test
> 
> I think it should say "is sent" rather than "is outputed”.

Done.

> Alan> diff --git a/gdb/testsuite/boards/native-extended-gdbserver-debug.exp b/gdb/testsuite/boards/native-extended-gdbserver-debug.exp
> 
> I wonder if a new board is needed for this?
> Could it be done some other way, like a command-line setting?

Agreed, it’s a little awkward. I’ve been playing round with an alternative
implementation and have added it an environment variable (in addition to the
board setting).

So, now you can do:
	make check GDBSERVER_DEBUG=all

Which is much nicer. However, see final comment.

> 
> TBH I'm not sure what the typical approach is for something like this.
> 
> Alan>  	if {![info exists gdbserver_reconnect_p] || !$gdbserver_reconnect_p} {
> Alan>  	    # GDB client could accidentally connect to a stale server.
> Alan> -	    # append gdbserver_command " --debug --once"
> Alan>  	    append gdbserver_command " --once"
> 
> Was this intentional?

Yes, I removed this commented out line. I suspect it was there so that you could
quickly switch over to using "—debug". With my changes you shouldn’t need it.

> 
> 
> One random thought I had about this series is that it would be nice to
> have a way to get "set remotelogfile" output in the test directory.
> That way a failing test could be re-run with gdbreplay without much
> trouble.

Yes, I think it’d be possible using a similar approach.  You’d end up with another
environment variable, REPLAY_LOG=1 or something.  My only concern is that it’s
starting down the road of adding more environment vars to the make line - which
people might not want?


I’ll post the updated version as a V2 so that I can use git send-email.


Alan.

Re: [PATCH 3/4] gdbserver: Ensure all debug output uses debug functions

[Tom de Vries]

On 16-04-19 21:43, Tom Tromey wrote:
>>>>>> "Alan" == Alan Hayward <Alan.Hayward@arm.com> writes:
> 
> Alan> All debug output needs to go via debug functions to ensure it writes to the
> Alan> correct output stream.
> 
> Alan> gdb/ChangeLog:
> 
> Alan> 2019-04-16  Alan Hayward  <alan.hayward@arm.com>
> 
> Alan> 	* nat/linux-waitpid.c (linux_debug): Call debug_vprintf.
> 
> Alan> gdb/gdbserver/ChangeLog:
> 
> Alan> 2019-04-16  Alan Hayward  <alan.hayward@arm.com>
> 
> Alan> 	* ax.c (ax_vdebug): Call debug_printf.
> Alan> 	* debug.c (debug_write): New function.
> Alan> 	* debug.h (debug_write): New declaration.
> Alan> 	* linux-low.c (sigchld_handler): Call debug_write.
> 

Building gdb with clang, I run into:
...
src/gdb/gdbserver/linux-low.c:6190:41: error: comparison of unsigned
expression < 0 is always false [-Werror,-Wtautological-compare]
                           sizeof ("sigchld_handler\n") - 1) < 0)
                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
...

This seems to fix it.

Thanks,
- Tom

diff --git a/gdb/gdbserver/debug.c b/gdb/gdbserver/debug.c
index a1cf5dbf7a..19f11fc17c 100644
--- a/gdb/gdbserver/debug.c
+++ b/gdb/gdbserver/debug.c
@@ -133,7 +133,7 @@ do_debug_exit (const char *function_name)

 /* See debug.h.  */

-size_t
+ssize_t
 debug_write (const void *buf, size_t nbyte)
 {
   int fd = fileno (debug_file);
diff --git a/gdb/gdbserver/debug.h b/gdb/gdbserver/debug.h
index 29e58ad8a4..07e94eac6e 100644
--- a/gdb/gdbserver/debug.h
+++ b/gdb/gdbserver/debug.h
@@ -36,7 +36,7 @@ void do_debug_enter (const char *function_name);
 void do_debug_exit (const char *function_name);

 /* Async signal safe debug output function that calls write directly.  */
-size_t debug_write (const void *buf, size_t nbyte);
+ssize_t debug_write (const void *buf, size_t nbyte);

 /* These macros are for use in major functions that produce a lot of
    debugging output.  They help identify in the mass of debugging output

Re: [PATCH 3/4] gdbserver: Ensure all debug output uses debug functions

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> This seems to fix it.

This needs a ChangeLog entry but is otherwise ok.  Thanks.

Tom

Re: [PATCH 3/4] gdbserver: Ensure all debug output uses debug functions

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 231 bytes --]

On 19-06-19 15:17, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> This seems to fix it.
> 
> This needs a ChangeLog entry but is otherwise ok.  Thanks.
> 

Committed as attached.

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-Fix-clang-buildbreaker.patch --]
[-- Type: text/x-patch, Size: 1906 bytes --]

[gdb] Fix clang buildbreaker

Building gdb with clang, I run into:
...
src/gdb/gdbserver/linux-low.c:6190:41: error: comparison of unsigned \
  expression < 0 is always false [-Werror,-Wtautological-compare]
          if (debug_write ("sigchld_handler\n",
                           sizeof ("sigchld_handler\n") - 1) < 0)
                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
...

This regression is introduced by commit a7e559cc08 "gdbserver: Ensure all
debug output uses debug functions", which replaces calls to write with result
type ssize_t with calls to debug_write with result type size_t.

Fix this by making debug_write return ssize_t.

Build and reg-tested on x86_64-linux.

gdb/gdbserver/ChangeLog:

2019-06-19  Tom de Vries  <tdevries@suse.de>

	* debug.h (debug_write): Change return type to ssize_t.
	* debug.c (debug_write): Same.

---
 gdb/gdbserver/debug.c | 2 +-
 gdb/gdbserver/debug.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/gdbserver/debug.c b/gdb/gdbserver/debug.c
index a1cf5dbf7a..19f11fc17c 100644
--- a/gdb/gdbserver/debug.c
+++ b/gdb/gdbserver/debug.c
@@ -133,7 +133,7 @@ do_debug_exit (const char *function_name)
 
 /* See debug.h.  */
 
-size_t
+ssize_t
 debug_write (const void *buf, size_t nbyte)
 {
   int fd = fileno (debug_file);
diff --git a/gdb/gdbserver/debug.h b/gdb/gdbserver/debug.h
index 29e58ad8a4..07e94eac6e 100644
--- a/gdb/gdbserver/debug.h
+++ b/gdb/gdbserver/debug.h
@@ -36,7 +36,7 @@ void do_debug_enter (const char *function_name);
 void do_debug_exit (const char *function_name);
 
 /* Async signal safe debug output function that calls write directly.  */
-size_t debug_write (const void *buf, size_t nbyte);
+ssize_t debug_write (const void *buf, size_t nbyte);
 
 /* These macros are for use in major functions that produce a lot of
    debugging output.  They help identify in the mass of debugging output

Re: [PATCH 4/4] testsuite: Add option to capture gdbserver debug

[Tom Tromey]

>>>>> "Alan" == Alan Hayward <Alan.Hayward@arm.com> writes:

Alan> 2019-04-23  Alan Hayward  <alan.hayward@arm.com>

Alan> 	* Makefile.in: Pass through GDBSERVER_DEBUG.
Alan>         * README (Testsuite Parameters): Add GDBSERVER_DEBUG.
Alan>         (gdbserver,debug): Add board setting.
Alan>         * gdb.trace/tspeed.exp: Skip when debugging.
Alan> 	* lib/gdb.exp (gdbserver_debug_enabled): New procedure.
Alan> 	* lib/gdbserver-support.exp: Likewise

Thanks, this is ok.

Tom

[PATCH 4/4] testsuite: Add option to capture gdbserver debug

[Alan Hayward]

Add both board option and environment variable which enables gdbserver
debug and sends it to the file gdbserver.debug, located in the output
directory for the current test.  Document this.

Add support for the environment variable in the Makefile.

The testsuite can be run with gdbserver debug enabled in the following way:

	make check GDBSERVER_DEBUG=all

Disable tspeed.exp when debugging to prevent the log file filling
many gigabytes then timing out.

gdb/testsuite/ChangeLog:

2019-04-23  Alan Hayward  <alan.hayward@arm.com>

	* Makefile.in: Pass through GDBSERVER_DEBUG.
        * README (Testsuite Parameters): Add GDBSERVER_DEBUG.
        (gdbserver,debug): Add board setting.
        * gdb.trace/tspeed.exp: Skip when debugging.
	* lib/gdb.exp (gdbserver_debug_enabled): New procedure.
	* lib/gdbserver-support.exp: Likewise
---
 gdb/testsuite/Makefile.in               |  4 ++-
 gdb/testsuite/README                    | 19 +++++++++++++
 gdb/testsuite/gdb.trace/tspeed.exp      |  5 ++++
 gdb/testsuite/lib/gdb.exp               |  7 +++++
 gdb/testsuite/lib/gdbserver-support.exp | 38 ++++++++++++++++++++++++-
 5 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/Makefile.in b/gdb/testsuite/Makefile.in
index 6625512d50..40225e3b91 100644
--- a/gdb/testsuite/Makefile.in
+++ b/gdb/testsuite/Makefile.in
@@ -53,6 +53,7 @@ RUNTESTFLAGS =
 FORCE_PARALLEL =
 
 GDB_DEBUG =
+GDBSERVER_DEBUG =
 
 # Default number of iterations that we will use to run the testsuite
 # if the user does not specify the RACY_ITER environment variable
@@ -165,6 +166,7 @@ check-read1:
 TIMESTAMP = $(if $(TS),| $(srcdir)/print-ts.py $(if $(TS_FORMAT),$(TS_FORMAT),),)
 
 gdb_debug = $(if $(GDB_DEBUG),GDB_DEBUG=$(GDB_DEBUG) ; export GDB_DEBUG ;,)
+gdbserver_debug = $(if $(GDBSERVER_DEBUG),GDBSERVER_DEBUG=$(GDBSERVER_DEBUG) ; export GDBSERVER_DEBUG ;,)
 
 # All the hair to invoke dejagnu.  A given invocation can just append
 # $(RUNTESTFLAGS)
@@ -172,7 +174,7 @@ DO_RUNTEST = \
 	rootme=`pwd`; export rootme; \
 	srcdir=${srcdir} ; export srcdir ; \
 	EXPECT=${EXPECT} ; export EXPECT ; \
-	EXEEXT=${EXEEXT} ; export EXEEXT ; $(gdb_debug) \
+	EXEEXT=${EXEEXT} ; export EXEEXT ; $(gdb_debug) $(gdbserver_debug) \
         $(RPATH_ENVVAR)=$$rootme/../../expect:$$rootme/../../libstdc++:$$rootme/../../tk/unix:$$rootme/../../tcl/unix:$$rootme/../../bfd:$$rootme/../../opcodes:$$$(RPATH_ENVVAR); \
 	export $(RPATH_ENVVAR); \
 	if [ -f $${rootme}/../../expect/expect ] ; then  \
diff --git a/gdb/testsuite/README b/gdb/testsuite/README
index e17f218da8..43f35a9d4b 100644
--- a/gdb/testsuite/README
+++ b/gdb/testsuite/README
@@ -302,6 +302,17 @@ For example, to turn on debugging for infrun and target, you can do:
 
 	make check GDB_DEBUG="infrun,target"
 
+GDBSERVER_DEBUG
+
+When set gdbserver debug is sent to the file gdbserver.debug in the test
+output directory.  Valid values are:
+	debug  - turn on gdbserver debug.
+	remote - turn on gdbserver remote debug.
+	all - turn on all the above debug options.
+For example, to turn on all gdbserver debugging, you can do:
+
+	make check GDBSERVER_DEBUG=all
+
 Race detection
 **************
 
@@ -513,6 +524,14 @@ gdb,debug
   components. For example, to turn on debugging for infrun and target, set to
   "infrun,target".
 
+gdbserver,debug
+
+  When set gdbserver debug is sent to the file gdbserver.debug in the test
+  output directory.  Valid values are:
+  "debug"  - turn on gdbserver debug.
+  "remote" - turn on gdbserver remote debug.
+  "all" - turn on all the above debug options.
+
 Testsuite Organization
 **********************
 
diff --git a/gdb/testsuite/gdb.trace/tspeed.exp b/gdb/testsuite/gdb.trace/tspeed.exp
index 6fd812e4f6..9afec64acf 100644
--- a/gdb/testsuite/gdb.trace/tspeed.exp
+++ b/gdb/testsuite/gdb.trace/tspeed.exp
@@ -19,6 +19,11 @@ if {[skip_shlib_tests]} {
     return 0
 }
 
+# Do not run if gdbsever debug is enabled - the output file is many Gb.
+if [gdbserver_debug_enabled] {
+    return 0
+}
+
 standard_testfile
 set executable $testfile
 
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 896cd80741..35d65d0e43 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -6410,5 +6410,12 @@ proc gdb_debug_init { } {
   gdb_test "set logging on" ".*" "Enable logging"
 }
 
+# Check if debugging is enabled for gdbserver.
+
+proc gdbserver_debug_enabled { } {
+    # Always disabled for GDB only setups.
+    return 0
+}
+
 # Always load compatibility stuff.
 load_lib future.exp
diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index 2cb64f7d2f..164a1d1d3c 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -283,12 +283,26 @@ proc gdbserver_start { options arguments } {
 	# If gdbserver_reconnect will be called $gdbserver_reconnect_p must be
 	# set to true already during gdbserver_start.
 	global gdbserver_reconnect_p
+	global srcdir
+	global subdir
 	if {![info exists gdbserver_reconnect_p] || !$gdbserver_reconnect_p} {
 	    # GDB client could accidentally connect to a stale server.
-	    # append gdbserver_command " --debug --once"
 	    append gdbserver_command " --once"
 	}
 
+	# Enable debug if set.
+	if [gdbserver_debug_enabled] {
+	    global gdbserverdebug
+	    set debugfile [standard_output_file gdbserver.debug]
+	    if { $gdbserverdebug == "debug" } {
+		append gdbserver_command " --debug --debug-file=$debugfile"
+	    } elseif { $gdbserverdebug == "remote" } {
+		append gdbserver_command " --remote-debug --debug-file=$debugfile"
+	    } elseif { $gdbserverdebug == "all" } {
+		append gdbserver_command " --debug --remote-debug --debug-file=$debugfile"
+	    }
+	}
+
 	if { $options != "" } {
 	    append gdbserver_command " $options"
 	}
@@ -561,3 +575,25 @@ proc mi_gdbserver_start_multi { } {
 
     return [mi_gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport]
 }
+
+# Check if debugging is enabled for gdbserver.
+
+proc gdbserver_debug_enabled { } {
+    global gdbserverdebug
+
+    # If not already read, get the debug setting from environment or board setting.
+    if ![info exists gdbserverdebug] {
+	global env
+	if [info exists env(GDBSERVER_DEBUG)] {
+	    set gdbserverdebug $env(GDBSERVER_DEBUG)
+	} elseif [target_info exists gdbserver,debug] {
+	    set gdbserverdebug [target_info gdbserver,debug]
+	} else {
+	    return 0
+	}
+    }
+
+    # Only return success on valid values.
+    return [expr { $gdbserverdebug == "debug" || $gdbserverdebug == "remote"
+		   || $gdbserverdebug == "all" }]
+}
-- 
2.20.1 (Apple Git-117)