[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)

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 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 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 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 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 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 4/4] testsuite: Add option to capture gdbserver debug

[Alan Hayward]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2478 bytes --]

(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.\x16º&Öéj×!zÊÞ¶êçמw÷Ib²Ö«r\x18\x1dn–­r\x17¬