public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 2/4] GDBServer: give more complete usage information
  2015-03-21  2:35 [PATCH 0/4] GDBServer: introduce a dedicated stderr stream Cleber Rosa
  2015-03-21  2:35 ` [PATCH 4/4] GDBServer: add 'monitor set server-stderr' command Cleber Rosa
  2015-03-21  2:35 ` [PATCH 1/4] GDBServer: introduce a stderr stream dedicated to the server Cleber Rosa
@ 2015-03-21  2:35 ` Cleber Rosa
  2015-03-21 17:05   ` Pedro Alves
  2015-03-21  2:35 ` [PATCH 3/4] GDBServer: introduce --server-stderr command line option Cleber Rosa
  2015-03-21 15:05 ` [PATCH 0/4] GDBServer: introduce a dedicated stderr stream Pedro Alves
  4 siblings, 1 reply; 19+ messages in thread
From: Cleber Rosa @ 2015-03-21  2:35 UTC (permalink / raw)
  To: gdb-patches; +Cc: crosa, areis

--attach/--multi are currently only mentioned on the usage info first lines,
the meaning of PROG is completely absent and COMM/PID text is unstructured.

The words and structure chosen may not be perfect but IMHO this is an improvement.

gdb/gdbserver/ChangeLog:
2015-03-20  Cleber Rosa  <crosa@redhat.com>

	* server.c (gdbserver_usage): Reorganized the usage message.
---
 gdb/gdbserver/server.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 618e457..9dec972 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2997,10 +2997,18 @@ gdbserver_usage (FILE *stream)
 	   "\tgdbserver [OPTIONS] --attach COMM PID\n"
 	   "\tgdbserver [OPTIONS] --multi COMM\n"
 	   "\n"
-	   "COMM may either be a tty device (for serial debugging), or \n"
-	   "HOST:PORT to listen for a TCP connection.\n"
+	   "Start mode options:\n"
+	   "  --attach              Attach to a given process\n"
+	   "  --multi               Start server without a specific program, and\n"
+	   "                        only quit when explicitly commanded\n"
 	   "\n"
-	   "Options:\n"
+	   "    COMM                tty device (for serial debugging) or HOST:PORT\n"
+	   "                        to listen for a TCP connection\n"
+	   "    PROG                Path to program executable\n"
+	   "    PID                 Process ID to attach to\n"
+	   "    ARGS                Arguments to be passed to debugged program\n"
+	   "\n"
+	   "Other options:\n"
 	   "  --debug               Enable general debugging output.\n"
 	   "  --debug-format=opt1[,opt2,...]\n"
 	   "                        Specify extra content in debugging output.\n"
-- 
1.9.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 0/4] GDBServer: introduce a dedicated stderr stream
@ 2015-03-21  2:35 Cleber Rosa
  2015-03-21  2:35 ` [PATCH 4/4] GDBServer: add 'monitor set server-stderr' command Cleber Rosa
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Cleber Rosa @ 2015-03-21  2:35 UTC (permalink / raw)
  To: gdb-patches; +Cc: crosa, areis

This  patch series add command line options and monitor commands that
will redirect all of the gdbserver's own output (always sent to stderr)
to a separate file. This feature makes it possible to distinguish between
the inferior process stderr and gdbserver's own stderr.

This happens to be needed for the Avocado[1] project GDB support[2][3].
One of Avocado's GDB features is that it lets users "transparently"
run binaries instrumented by GDB. A test that executes binaries
can give the user the opportunity to interact with the process running
within gdb(server). This uses a combination of gdb (with MI) and gdbserver.
So far so good.

But, since Avocado is a test framework, it lets tests PASS or FAIL
depending on executed commands' STDERR and STDOUT matching what the
test writer expects. Matching against STDOUT is already doable as
gdbserver doesn't write to stdout, but it does write to stderr, the
same stderr that the inferior process writes to.

I appreciate any comments/questions/reviews.

Thanks,
Cleber Rosa.

[1] - http://github.com/avocado-framework
[2] - http://avocado-framework.readthedocs.org/en/latest/DebuggingWithGDB.html
[3] - https://github.com/avocado-framework/avocado/blob/master/avocado/gdb.py

---
 gdb/doc/gdb.texinfo                     |  13 +++++
 gdb/gdbserver/ax.c                      |   3 +-
 gdb/gdbserver/debug.c                   |   6 +--
 gdb/gdbserver/event-loop.c              |   2 +-
 gdb/gdbserver/linux-aarch64-low.c       |  28 +++++------
 gdb/gdbserver/linux-low.c               |  10 ++--
 gdb/gdbserver/lynx-low.c                |   8 +--
 gdb/gdbserver/mem-break.c               |   4 +-
 gdb/gdbserver/notif.c                   |   4 +-
 gdb/gdbserver/remote-utils.c            |  54 ++++++++++-----------
 gdb/gdbserver/server.c                  | 145 +++++++++++++++++++++++++++++++++++++------------------
 gdb/gdbserver/server.h                  |   4 ++
 gdb/gdbserver/spu-low.c                 |  14 +++---
 gdb/gdbserver/target.c                  |   4 +-
 gdb/gdbserver/thread-db.c               |   4 +-
 gdb/gdbserver/utils.c                   |  25 +++++-----
 gdb/testsuite/gdb.server/server-mon.exp |   8 +++

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 4/4] GDBServer: add 'monitor set server-stderr' command
  2015-03-21  2:35 [PATCH 0/4] GDBServer: introduce a dedicated stderr stream Cleber Rosa
@ 2015-03-21  2:35 ` Cleber Rosa
  2015-03-21  8:29   ` Eli Zaretskii
  2015-03-21  2:35 ` [PATCH 1/4] GDBServer: introduce a stderr stream dedicated to the server Cleber Rosa
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Cleber Rosa @ 2015-03-21  2:35 UTC (permalink / raw)
  To: gdb-patches; +Cc: crosa, areis

This monitor command, just like the command line option --server-stderr,
will redirect all of the gdbserver's own output (always sent to stderr)
to a separate file.

gdb/doc/Changelog:
2015-03-20  Cleber Rosa  <crosa@redhat.com>

	* gdb.texinfo (info): Add documentation about the 'monitor set
	server-stderr' command

gdb/gdbserver/Changelog:
2015-03-20  Cleber Rosa  <crosa@redhat.com>

	* server.c (monitor_show_help): Add help message about the
	'monitor set server-stderr' command
	(handle_monitor_command): Respond to 'set server-stderr' command

gdb/testsuite/ChangeLog:
2015-03-20  Cleber Rosa  <crosa@redhat.com>

	* gdb.server/server-mon.exp: Add tests with the 'monitor set
	server-stderr' command succeeding and failing
---
 gdb/doc/gdb.texinfo                     |  3 +++
 gdb/gdbserver/server.c                  | 22 +++++++++++++++++++---
 gdb/testsuite/gdb.server/server-mon.exp |  8 ++++++++
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 2834794..3a6bbde 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -19314,6 +19314,9 @@ Include a timestamp in each line of debugging output.
 Options are processed in order.  Thus, for example, if @option{none}
 appears last then no additional information is added to debugging output.
 
+@item monitor set server-stderr [PATH]
+Redirect the server stderr to a file given by @var{path}.
+
 @item monitor set libthread-db-search-path [PATH]
 @cindex gdbserver, search path for @code{libthread_db}
 When this command is issued, @var{path} is a colon-separated list of
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index db26f24..afa7aed 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -754,9 +754,9 @@ monitor_show_help (void)
   monitor_output ("    Enable remote protocol debugging messages\n");
   monitor_output ("  set debug-format option1[,option2,...]\n");
   monitor_output ("    Add additional information to debugging messages\n");
-  monitor_output ("    Options: all, none");
-  monitor_output (", timestamp");
-  monitor_output ("\n");
+  monitor_output ("    Options: all, none, timestamp\n");
+  monitor_output ("  set server-stderr <path>\n");
+  monitor_output ("    Redirects the server stderr content to another file\n");
   monitor_output ("  exit\n");
   monitor_output ("    Quit GDBserver\n");
 }
@@ -1105,6 +1105,22 @@ handle_monitor_command (char *mon, char *own_buf)
 	  xfree (error_msg);
 	}
     }
+  else if (strncmp (mon, "set server-stderr ",
+		    sizeof ("set server-stderr ") -1) == 0)
+    {
+      char *path = mon + (sizeof ("set server-stderr ") - 1);
+      if (set_server_stderr (path) == 0) {
+	char *monitor_msg = xstrprintf ("Redirected server stderr to '%s'\n", path);
+	monitor_output (monitor_msg);
+	xfree (monitor_msg);
+      }
+      else
+	{
+	  char *monitor_msg = xstrprintf ("Failed to set server stderr to '%s'\n", path);
+	  monitor_output (monitor_msg);
+	  xfree (monitor_msg);
+	}
+    }
   else if (strcmp (mon, "help") == 0)
     monitor_show_help ();
   else if (strcmp (mon, "exit") == 0)
diff --git a/gdb/testsuite/gdb.server/server-mon.exp b/gdb/testsuite/gdb.server/server-mon.exp
index a4c03ee..34cd1ce 100644
--- a/gdb/testsuite/gdb.server/server-mon.exp
+++ b/gdb/testsuite/gdb.server/server-mon.exp
@@ -54,3 +54,11 @@ gdb_test "monitor set debug-format all" \
     "All extra debug format options enabled\\."
 gdb_test "monitor set debug-format none" \
     "All extra debug format options disabled\\."
+
+# Tests the monitor command that sends stderr output to another file
+gdb_test "monitor set server-stderr /dev/null" \
+    "Redirected server stderr to '/dev/null'"
+
+# Tests the monitor command that sends stderr output to another file
+gdb_test "monitor set server-stderr /im/pro/ba/ble/pa/th" \
+    "Failed to set server stderr to '/im/pro/ba/ble/pa/th'"
-- 
1.9.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/4] GDBServer: introduce a stderr stream dedicated to the server
  2015-03-21  2:35 [PATCH 0/4] GDBServer: introduce a dedicated stderr stream Cleber Rosa
  2015-03-21  2:35 ` [PATCH 4/4] GDBServer: add 'monitor set server-stderr' command Cleber Rosa
@ 2015-03-21  2:35 ` Cleber Rosa
  2015-03-21  2:35 ` [PATCH 2/4] GDBServer: give more complete usage information Cleber Rosa
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Cleber Rosa @ 2015-03-21  2:35 UTC (permalink / raw)
  To: gdb-patches; +Cc: crosa, areis

This dedicated stderr, named server_stderr, is intended to be the
only stream written by the server itself to announce errors. At this
point it is just a pointer to the regular stderr.

gdb/gdbserver/Changelog:
2015-03-20  Cleber Rosa  <crosa@redhat.com>

	* ax.c (ax_vdebug): Conditional write to stderr if compiling
	IPA, or to server's dedicated stderr if compiling gdbserver.
	* debug.c (debug_vprintf): Replace stderr with gdbserver's own stderr.
	(debug_flush): Likewise.
	* event-loop.c (handle_file_event): Likewise.
	* linux-aarch64-low.c (aarch64_show_debug_reg_state): Likewise.
	(debug_reg_change_callback): Likewise.
	(aarch64_handle_unaligned_watchpoint): Likewise.
	(aarch64_insert_point): Likewise.
	(aarch64_remove_point): Likewise.
	(aarch64_linux_prepare_to_resume): Likewise.
	* linux-low.c (linux_create_inferior): Likewise.
	(linux_attach): Likewise.
	(check_zombie_leaders): Likewise.
	(linux_resume_one_lwp): Likewise.
	* lynx-low.c (lynx_ptrace): Likewise.
	(lynx_create_inferior): Likewise.
	* mem-break.c (add_breakpoint_condition): Likewise.
	(add_breakpoint_commands): Likewise.
	* notif.c (handle_notif_ack): Likewise.
	(notif_event_enque): Likewise.
	* remote-utils.c (handle_accept_event): Likewise.
	(remote_open): Likewise.
	(putpkt_binary_1): Likewise.
	(input_interrupt): Likewise.
	(readchar): Likewise.
	(getpkt): Likewise.
	* server.c: New server_stderr stream.
	(start_inferior): Replace stderr with gdbserver's own stderr.
	(attach_inferior): Likewise.
	(handle_general_set): Likewise.
	(handle_v_requests): Likewise.
	(print_started_pid): Likewise.
	(print_attached_pid): Likewise.
	(detach_or_kill_for_exit): Likewise.
	(detach_or_kill_for_exit_cleanup): Likewise.
	(captured_main): Point server_stderr to standard stderr.
	(captured_main): Replace stderr with gdbserver's own stderr.
	(main): Likewise.
	(process_point_options): Likewise.
	(process_serial_event): Likewise.
	* server.h: New server_stderr stream definition.
	* spu-low.c (spu_create_inferior): Replace stderr with gdbserver's own
	stderr.
	(spu_attach): Likewise.
	(spu_resume): Likewise.
	(spu_wait): Likewise.
	* target.c (mywait): Likewise.
	* thread-db.c (thread_db_create_event): Likewise.
	(try_thread_db_load): Likewise.
	* utils.c: Define server_stderr as an alias of stderr when compiling
	the IPA.
	(malloc_failure): Replace stderr with gdbserver's own stderr.
	(verror): Likewise.
	(vwarning): Likewise.
	(internal_verror): Likewise.
	(internal_vwarning): Likewise.
---
 gdb/gdbserver/ax.c                |  3 +-
 gdb/gdbserver/debug.c             |  6 +--
 gdb/gdbserver/event-loop.c        |  2 +-
 gdb/gdbserver/linux-aarch64-low.c | 28 +++++++-------
 gdb/gdbserver/linux-low.c         | 10 ++---
 gdb/gdbserver/lynx-low.c          |  8 ++--
 gdb/gdbserver/mem-break.c         |  4 +-
 gdb/gdbserver/notif.c             |  4 +-
 gdb/gdbserver/remote-utils.c      | 54 +++++++++++++-------------
 gdb/gdbserver/server.c            | 81 ++++++++++++++++++++-------------------
 gdb/gdbserver/server.h            |  4 ++
 gdb/gdbserver/spu-low.c           | 14 +++----
 gdb/gdbserver/target.c            |  4 +-
 gdb/gdbserver/thread-db.c         |  4 +-
 gdb/gdbserver/utils.c             | 25 ++++++------
 15 files changed, 130 insertions(+), 121 deletions(-)

diff --git a/gdb/gdbserver/ax.c b/gdb/gdbserver/ax.c
index c5b65fa..c446a50 100644
--- a/gdb/gdbserver/ax.c
+++ b/gdb/gdbserver/ax.c
@@ -26,6 +26,7 @@ static void ax_vdebug (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
 
 #ifdef IN_PROCESS_AGENT
 int debug_agent = 0;
+#define server_stderr stderr
 #endif
 
 static void
@@ -36,7 +37,7 @@ ax_vdebug (const char *fmt, ...)
 
   va_start (ap, fmt);
   vsprintf (buf, fmt, ap);
-  fprintf (stderr, PROG "/ax: %s\n", buf);
+  fprintf (server_stderr, PROG "/ax: %s\n", buf);
   va_end (ap);
 }
 
diff --git a/gdb/gdbserver/debug.c b/gdb/gdbserver/debug.c
index 1a1e333..b4a4905 100644
--- a/gdb/gdbserver/debug.c
+++ b/gdb/gdbserver/debug.c
@@ -48,11 +48,11 @@ debug_vprintf (const char *format, va_list ap)
       /* If gettimeofday doesn't exist, and as a portability solution it has
 	 been replaced with, e.g., time, then it doesn't make sense to print
 	 the microseconds field.  Is there a way to check for that?  */
-      fprintf (stderr, "%ld:%06ld ", (long) tm.tv_sec, (long) tm.tv_usec);
+      fprintf (server_stderr, "%ld:%06ld ", (long) tm.tv_sec, (long) tm.tv_usec);
     }
 #endif
 
-  vfprintf (stderr, format, ap);
+  vfprintf (server_stderr, format, ap);
 
 #if !defined (IN_PROCESS_AGENT)
   if (*format)
@@ -67,7 +67,7 @@ debug_vprintf (const char *format, va_list ap)
 void
 debug_flush (void)
 {
-  fflush (stderr);
+  fflush (server_stderr);
 }
 
 /* Notify the user that the code is entering FUNCTION_NAME.
diff --git a/gdb/gdbserver/event-loop.c b/gdb/gdbserver/event-loop.c
index 08a503f..69160a9 100644
--- a/gdb/gdbserver/event-loop.c
+++ b/gdb/gdbserver/event-loop.c
@@ -412,7 +412,7 @@ handle_file_event (gdb_fildes_t event_file_desc)
 
 	  if (file_ptr->ready_mask & GDB_EXCEPTION)
 	    {
-	      fprintf (stderr, "Exception condition detected on fd %s\n",
+	      fprintf (server_stderr, "Exception condition detected on fd %s\n",
 		       pfildes (file_ptr->fd));
 	      file_ptr->error = 1;
 	    }
diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index 7934e78..871d40e 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -323,26 +323,26 @@ aarch64_show_debug_reg_state (struct aarch64_debug_reg_state *state,
 {
   int i;
 
-  fprintf (stderr, "%s", func);
+  fprintf (server_stderr, "%s", func);
   if (addr || len)
-    fprintf (stderr, " (addr=0x%08lx, len=%d, type=%s)",
+    fprintf (server_stderr, " (addr=0x%08lx, len=%d, type=%s)",
 	     (unsigned long) addr, len,
 	     type == hw_write ? "hw-write-watchpoint"
 	     : (type == hw_read ? "hw-read-watchpoint"
 		: (type == hw_access ? "hw-access-watchpoint"
 		   : (type == hw_execute ? "hw-breakpoint"
 		      : "??unknown??"))));
-  fprintf (stderr, ":\n");
+  fprintf (server_stderr, ":\n");
 
-  fprintf (stderr, "\tBREAKPOINTs:\n");
+  fprintf (server_stderr, "\tBREAKPOINTs:\n");
   for (i = 0; i < aarch64_num_bp_regs; i++)
-    fprintf (stderr, "\tBP%d: addr=0x%s, ctrl=0x%08x, ref.count=%d\n",
+    fprintf (server_stderr, "\tBP%d: addr=0x%s, ctrl=0x%08x, ref.count=%d\n",
 	     i, paddress (state->dr_addr_bp[i]),
 	     state->dr_ctrl_bp[i], state->dr_ref_count_bp[i]);
 
-  fprintf (stderr, "\tWATCHPOINTs:\n");
+  fprintf (server_stderr, "\tWATCHPOINTs:\n");
   for (i = 0; i < aarch64_num_wp_regs; i++)
-    fprintf (stderr, "\tWP%d: addr=0x%s, ctrl=0x%08x, ref.count=%d\n",
+    fprintf (server_stderr, "\tWP%d: addr=0x%s, ctrl=0x%08x, ref.count=%d\n",
 	     i, paddress (state->dr_addr_wp[i]),
 	     state->dr_ctrl_wp[i], state->dr_ref_count_wp[i]);
 }
@@ -631,8 +631,8 @@ debug_reg_change_callback (struct inferior_list_entry *entry, void *ptr)
 
   if (show_debug_regs)
     {
-      fprintf (stderr, "debug_reg_change_callback: \n\tOn entry:\n");
-      fprintf (stderr, "\tpid%d, tid: %ld, dr_changed_bp=0x%llx, "
+      fprintf (server_stderr, "debug_reg_change_callback: \n\tOn entry:\n");
+      fprintf (server_stderr, "\tpid%d, tid: %ld, dr_changed_bp=0x%llx, "
 	       "dr_changed_wp=0x%llx\n",
 	       pid, lwpid_of (thread), info->dr_changed_bp,
 	       info->dr_changed_wp);
@@ -682,7 +682,7 @@ debug_reg_change_callback (struct inferior_list_entry *entry, void *ptr)
 
   if (show_debug_regs)
     {
-      fprintf (stderr, "\tOn exit:\n\tpid%d, tid: %ld, dr_changed_bp=0x%llx, "
+      fprintf (server_stderr, "\tOn exit:\n\tpid%d, tid: %ld, dr_changed_bp=0x%llx, "
 	       "dr_changed_wp=0x%llx\n",
 	       pid, lwpid_of (thread), info->dr_changed_bp,
 	       info->dr_changed_wp);
@@ -921,7 +921,7 @@ aarch64_handle_unaligned_watchpoint (enum target_hw_bp_type type,
 						 aligned_len);
 
       if (show_debug_regs)
-	fprintf (stderr,
+	fprintf (server_stderr,
  "handle_unaligned_watchpoint: is_insert: %d\n"
  "                             aligned_addr: 0x%s, aligned_len: %d\n"
  "                                next_addr: 0x%s,    next_len: %d\n",
@@ -977,7 +977,7 @@ aarch64_insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
   enum target_hw_bp_type targ_type;
 
   if (show_debug_regs)
-    fprintf (stderr, "insert_point on entry (addr=0x%08lx, len=%d)\n",
+    fprintf (server_stderr, "insert_point on entry (addr=0x%08lx, len=%d)\n",
 	     (unsigned long) addr, len);
 
   /* Determine the type from the raw breakpoint type.  */
@@ -1013,7 +1013,7 @@ aarch64_remove_point (enum raw_bkpt_type type, CORE_ADDR addr,
   enum target_hw_bp_type targ_type;
 
   if (show_debug_regs)
-    fprintf (stderr, "remove_point on entry (addr=0x%08lx, len=%d)\n",
+    fprintf (server_stderr, "remove_point on entry (addr=0x%08lx, len=%d)\n",
 	     (unsigned long) addr, len);
 
   /* Determine the type from the raw breakpoint type.  */
@@ -1154,7 +1154,7 @@ aarch64_linux_prepare_to_resume (struct lwp_info *lwp)
 	= &proc->priv->arch_private->debug_reg_state;
 
       if (show_debug_regs)
-	fprintf (stderr, "prepare_to_resume thread %ld\n", lwpid_of (thread));
+	fprintf (server_stderr, "prepare_to_resume thread %ld\n", lwpid_of (thread));
 
       /* Watchpoints.  */
       if (DR_HAS_CHANGED (info->dr_changed_wp))
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 0c54115..f79967e 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -671,9 +671,9 @@ linux_create_inferior (char *program, char **allargs)
       if (errno == ENOENT)
 	execvp (program, allargs);
 
-      fprintf (stderr, "Cannot exec %s: %s.\n", program,
+      fprintf (server_stderr, "Cannot exec %s: %s.\n", program,
 	       strerror (errno));
-      fflush (stderr);
+      fflush (server_stderr);
       _exit (0177);
     }
 
@@ -1453,7 +1453,7 @@ check_zombie_leaders (void)
 	     thread execs).  */
 
 	  if (debug_threads)
-	    fprintf (stderr,
+	    fprintf (server_stderr,
 		     "CZL: Thread group leader %d zombie "
 		     "(it exited, or another thread execd).\n",
 		     leader_pid);
@@ -3465,9 +3465,9 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp,
 	  if (fast_tp_collecting == 0)
 	    {
 	      if (step == 0)
-		fprintf (stderr, "BAD - reinserting but not stepping.\n");
+		fprintf (server_stderr, "BAD - reinserting but not stepping.\n");
 	      if (lwp->suspended)
-		fprintf (stderr, "BAD - reinserting and suspended(%d).\n",
+		fprintf (server_stderr, "BAD - reinserting and suspended(%d).\n",
 			 lwp->suspended);
 	    }
 
diff --git a/gdb/gdbserver/lynx-low.c b/gdb/gdbserver/lynx-low.c
index 2f85829..295178b 100644
--- a/gdb/gdbserver/lynx-low.c
+++ b/gdb/gdbserver/lynx-low.c
@@ -195,14 +195,14 @@ lynx_ptrace (int request, ptid_t ptid, int addr, int data, int addr2)
   int saved_errno;
 
   if (debug_threads)
-    fprintf (stderr, "PTRACE (%s, pid=%d(pid=%d, tid=%d), addr=0x%x, "
+    fprintf (server_stderr, "PTRACE (%s, pid=%d(pid=%d, tid=%d), addr=0x%x, "
              "data=0x%x, addr2=0x%x)",
              ptrace_request_to_str (request), pid, PIDGET (pid), TIDGET (pid),
              addr, data, addr2);
   result = ptrace (request, pid, addr, data, addr2);
   saved_errno = errno;
   if (debug_threads)
-    fprintf (stderr, " -> %d (=0x%x)\n", result, result);
+    fprintf (server_stderr, " -> %d (=0x%x)\n", result, result);
 
   errno = saved_errno;
   return result;
@@ -250,8 +250,8 @@ lynx_create_inferior (char *program, char **allargs)
       ioctl (0, TIOCSPGRP, &pgrp);
       lynx_ptrace (PTRACE_TRACEME, null_ptid, 0, 0, 0);
       execv (program, allargs);
-      fprintf (stderr, "Cannot exec %s: %s.\n", program, strerror (errno));
-      fflush (stderr);
+      fprintf (server_stderr, "Cannot exec %s: %s.\n", program, strerror (errno));
+      fflush (server_stderr);
       _exit (0177);
     }
 
diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index 70fab2e..856ba91 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -1179,7 +1179,7 @@ add_breakpoint_condition (struct breakpoint *bp, char **condition)
 
   if (cond == NULL)
     {
-      fprintf (stderr, "Condition evaluation failed. "
+      fprintf (server_stderr, "Condition evaluation failed. "
 	       "Assuming unconditional.\n");
       return 0;
     }
@@ -1279,7 +1279,7 @@ add_breakpoint_commands (struct breakpoint *bp, char **command,
 
   if (cmd == NULL)
     {
-      fprintf (stderr, "Command evaluation failed. "
+      fprintf (server_stderr, "Command evaluation failed. "
 	       "Disabling.\n");
       return 0;
     }
diff --git a/gdb/gdbserver/notif.c b/gdb/gdbserver/notif.c
index 436c1b8..f3ba70a 100644
--- a/gdb/gdbserver/notif.c
+++ b/gdb/gdbserver/notif.c
@@ -104,7 +104,7 @@ handle_notif_ack (char *own_buf, int packet_len)
 	= QUEUE_deque (notif_event_p, np->queue);
 
       if (remote_debug)
-	fprintf (stderr, "%s: acking %d\n", np->ack_name,
+	fprintf (server_stderr, "%s: acking %d\n", np->ack_name,
 		 QUEUE_length (notif_event_p, np->queue));
 
       xfree (head);
@@ -124,7 +124,7 @@ notif_event_enque (struct notif_server *notif,
   QUEUE_enque (notif_event_p, notif->queue, event);
 
   if (remote_debug)
-    fprintf (stderr, "pending events: %s %d\n", notif->notif_name,
+    fprintf (server_stderr, "pending events: %s %d\n", notif->notif_name,
 	     QUEUE_length (notif_event_p, notif->queue));
 
 }
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 1de86be..520fde1 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -194,7 +194,7 @@ handle_accept_event (int err, gdb_client_data client_data)
   delete_file_handler (listen_desc);
 
   /* Convert IP address to string.  */
-  fprintf (stderr, "Remote debugging from host %s\n",
+  fprintf (server_stderr, "Remote debugging from host %s\n",
 	   inet_ntoa (sockaddr.sin_addr));
 
   enable_async_notification (remote_desc);
@@ -296,7 +296,7 @@ remote_open (char *name)
 
   if (strcmp (name, STDIO_CONNECTION_NAME) == 0)
     {
-      fprintf (stderr, "Remote debugging using stdio\n");
+      fprintf (server_stderr, "Remote debugging using stdio\n");
 
       /* Use stdin as the handle of the connection.
 	 We only select on reads, for example.  */
@@ -368,7 +368,7 @@ remote_open (char *name)
       }
 #endif
 
-      fprintf (stderr, "Remote debugging using %s\n", name);
+      fprintf (server_stderr, "Remote debugging using %s\n", name);
 
       enable_async_notification (remote_desc);
 
@@ -389,8 +389,8 @@ remote_open (char *name)
 	perror_with_name ("Can't determine port");
       port = ntohs (sockaddr.sin_port);
 
-      fprintf (stderr, "Listening on port %d\n", port);
-      fflush (stderr);
+      fprintf (server_stderr, "Listening on port %d\n", port);
+      fflush (server_stderr);
 
       /* Register the event loop handler.  */
       add_file_handler (listen_desc, handle_accept_event, NULL);
@@ -658,18 +658,18 @@ putpkt_binary_1 (char *buf, int cnt, int is_notif)
 	  if (remote_debug)
 	    {
 	      if (is_notif)
-		fprintf (stderr, "putpkt (\"%s\"); [notif]\n", buf2);
+		fprintf (server_stderr, "putpkt (\"%s\"); [notif]\n", buf2);
 	      else
-		fprintf (stderr, "putpkt (\"%s\"); [noack mode]\n", buf2);
-	      fflush (stderr);
+		fprintf (server_stderr, "putpkt (\"%s\"); [noack mode]\n", buf2);
+	      fflush (server_stderr);
 	    }
 	  break;
 	}
 
       if (remote_debug)
 	{
-	  fprintf (stderr, "putpkt (\"%s\"); [looking for ack]\n", buf2);
-	  fflush (stderr);
+	  fprintf (server_stderr, "putpkt (\"%s\"); [looking for ack]\n", buf2);
+	  fflush (server_stderr);
 	}
 
       cc = readchar ();
@@ -682,8 +682,8 @@ putpkt_binary_1 (char *buf, int cnt, int is_notif)
 
       if (remote_debug)
 	{
-	  fprintf (stderr, "[received '%c' (0x%x)]\n", cc, cc);
-	  fflush (stderr);
+	  fprintf (server_stderr, "[received '%c' (0x%x)]\n", cc, cc);
+	  fflush (server_stderr);
 	}
 
       /* Check for an input interrupt while we're here.  */
@@ -744,16 +744,16 @@ input_interrupt (int unused)
 
       if (cc == 0)
 	{
-	  fprintf (stderr, "client connection closed\n");
+	  fprintf (server_stderr, "client connection closed\n");
 	  return;
 	}
       else if (cc != 1 || c != '\003' || current_thread == NULL)
 	{
-	  fprintf (stderr, "input_interrupt, count = %d c = %d ", cc, c);
+	  fprintf (server_stderr, "input_interrupt, count = %d c = %d ", cc, c);
 	  if (isprint (c))
-	    fprintf (stderr, "('%c')\n", c);
+	    fprintf (server_stderr, "('%c')\n", c);
 	  else
-	    fprintf (stderr, "('\\x%02x')\n", c & 0xff);
+	    fprintf (server_stderr, "('\\x%02x')\n", c & 0xff);
 	  return;
 	}
 
@@ -881,7 +881,7 @@ readchar (void)
       if (readchar_bufcnt <= 0)
 	{
 	  if (readchar_bufcnt == 0)
-	    fprintf (stderr, "readchar: Got EOF\n");
+	    fprintf (server_stderr, "readchar: Got EOF\n");
 	  else
 	    perror ("readchar");
 
@@ -959,8 +959,8 @@ getpkt (char *buf)
 	    break;
 	  if (remote_debug)
 	    {
-	      fprintf (stderr, "[getpkt: discarding char '%c']\n", c);
-	      fflush (stderr);
+	      fprintf (server_stderr, "[getpkt: discarding char '%c']\n", c);
+	      fflush (server_stderr);
 	    }
 
 	  if (c < 0)
@@ -988,7 +988,7 @@ getpkt (char *buf)
 
       if (noack_mode)
 	{
-	  fprintf (stderr,
+	  fprintf (server_stderr,
 		   "Bad checksum, sentsum=0x%x, csum=0x%x, "
 		   "buf=%s [no-ack-mode, Bad medium?]\n",
 		   (c1 << 4) + c2, csum, buf);
@@ -996,7 +996,7 @@ getpkt (char *buf)
 	  break;
 	}
 
-      fprintf (stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n",
+      fprintf (server_stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n",
 	       (c1 << 4) + c2, csum, buf);
       if (write_prim ("-", 1) != 1)
 	return -1;
@@ -1006,8 +1006,8 @@ getpkt (char *buf)
     {
       if (remote_debug)
 	{
-	  fprintf (stderr, "getpkt (\"%s\");  [sending ack] \n", buf);
-	  fflush (stderr);
+	  fprintf (server_stderr, "getpkt (\"%s\");  [sending ack] \n", buf);
+	  fflush (server_stderr);
 	}
 
       if (write_prim ("+", 1) != 1)
@@ -1015,16 +1015,16 @@ getpkt (char *buf)
 
       if (remote_debug)
 	{
-	  fprintf (stderr, "[sent ack]\n");
-	  fflush (stderr);
+	  fprintf (server_stderr, "[sent ack]\n");
+	  fflush (server_stderr);
 	}
     }
   else
     {
       if (remote_debug)
 	{
-	  fprintf (stderr, "getpkt (\"%s\");  [no ack sent] \n", buf);
-	  fflush (stderr);
+	  fprintf (server_stderr, "getpkt (\"%s\");  [no ack sent] \n", buf);
+	  fflush (server_stderr);
 	}
     }
 
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 96b31b8..618e457 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -60,6 +60,7 @@ int multi_process;
 int non_stop;
 int swbreak_feature;
 int hwbreak_feature;
+FILE *server_stderr;
 
 /* Whether we should attempt to disable the operating system's address
    space randomization feature before starting an inferior.  */
@@ -231,9 +232,9 @@ start_inferior (char **argv)
 
   /* FIXME: we don't actually know at this point that the create
      actually succeeded.  We won't know that until we wait.  */
-  fprintf (stderr, "Process %s created; pid = %ld\n", argv[0],
+  fprintf (server_stderr, "Process %s created; pid = %ld\n", argv[0],
 	   signal_pid);
-  fflush (stderr);
+  fflush (server_stderr);
 
 #ifdef SIGTTOU
   signal (SIGTTOU, SIG_IGN);
@@ -297,8 +298,8 @@ attach_inferior (int pid)
   if (myattach (pid) != 0)
     return -1;
 
-  fprintf (stderr, "Attached; pid = %d\n", pid);
-  fflush (stderr);
+  fprintf (server_stderr, "Attached; pid = %d\n", pid);
+  fflush (server_stderr);
 
   /* FIXME - It may be that we should get the SIGNAL_PID from the
      attach function, so that it can be the main thread instead of
@@ -578,8 +579,8 @@ handle_general_set (char *own_buf)
     {
       if (remote_debug)
 	{
-	  fprintf (stderr, "[noack mode enabled]\n");
-	  fflush (stderr);
+	  fprintf (server_stderr, "[noack mode enabled]\n");
+	  fflush (server_stderr);
 	}
 
       noack_mode = 1;
@@ -601,7 +602,7 @@ handle_general_set (char *own_buf)
 	{
 	  /* We don't know what this mode is, so complain to
 	     GDB.  */
-	  fprintf (stderr, "Unknown non-stop mode requested: %s\n",
+	  fprintf (server_stderr, "Unknown non-stop mode requested: %s\n",
 		   own_buf);
 	  write_enn (own_buf);
 	  return;
@@ -610,7 +611,7 @@ handle_general_set (char *own_buf)
       req_str = req ? "non-stop" : "all-stop";
       if (start_non_stop (req) != 0)
 	{
-	  fprintf (stderr, "Setting %s mode failed\n", req_str);
+	  fprintf (server_stderr, "Setting %s mode failed\n", req_str);
 	  write_enn (own_buf);
 	  return;
 	}
@@ -618,7 +619,7 @@ handle_general_set (char *own_buf)
       non_stop = req;
 
       if (remote_debug)
-	fprintf (stderr, "[%s mode enabled]\n", req_str);
+	fprintf (server_stderr, "[%s mode enabled]\n", req_str);
 
       write_ok (own_buf);
       return;
@@ -635,9 +636,9 @@ handle_general_set (char *own_buf)
       if (remote_debug)
 	{
 	  if (disable_randomization)
-	    fprintf (stderr, "[address space randomization disabled]\n");
+	    fprintf (server_stderr, "[address space randomization disabled]\n");
 	  else
-	    fprintf (stderr, "[address space randomization enabled]\n");
+	    fprintf (server_stderr, "[address space randomization enabled]\n");
 	}
 
       write_ok (own_buf);
@@ -667,7 +668,7 @@ handle_general_set (char *own_buf)
       /* Update the flag.  */
       use_agent = req;
       if (remote_debug)
-	fprintf (stderr, "[%s agent]\n", req ? "Enable" : "Disable");
+	fprintf (server_stderr, "[%s agent]\n", req ? "Enable" : "Disable");
       write_ok (own_buf);
       return;
     }
@@ -2696,7 +2697,7 @@ handle_v_requests (char *own_buf, int packet_len, int *new_packet_len)
     {
       if ((!extended_protocol || !multi_process) && target_running ())
 	{
-	  fprintf (stderr, "Already debugging a process\n");
+	  fprintf (server_stderr, "Already debugging a process\n");
 	  write_enn (own_buf);
 	  return;
 	}
@@ -2708,7 +2709,7 @@ handle_v_requests (char *own_buf, int packet_len, int *new_packet_len)
     {
       if ((!extended_protocol || !multi_process) && target_running ())
 	{
-	  fprintf (stderr, "Already debugging a process\n");
+	  fprintf (server_stderr, "Already debugging a process\n");
 	  write_enn (own_buf);
 	  return;
 	}
@@ -2720,7 +2721,7 @@ handle_v_requests (char *own_buf, int packet_len, int *new_packet_len)
     {
       if (!target_running ())
 	{
-	  fprintf (stderr, "No process to kill\n");
+	  fprintf (server_stderr, "No process to kill\n");
 	  write_enn (own_buf);
 	  return;
 	}
@@ -3088,7 +3089,7 @@ print_started_pid (struct inferior_list_entry *entry)
   if (! process->attached)
     {
       int pid = ptid_get_pid (process->entry.id);
-      fprintf (stderr, " %d", pid);
+      fprintf (server_stderr, " %d", pid);
     }
 }
 
@@ -3103,7 +3104,7 @@ print_attached_pid (struct inferior_list_entry *entry)
   if (process->attached)
     {
       int pid = ptid_get_pid (process->entry.id);
-      fprintf (stderr, " %d", pid);
+      fprintf (server_stderr, " %d", pid);
     }
 }
 
@@ -3120,15 +3121,15 @@ detach_or_kill_for_exit (void)
 
   if (have_started_inferiors_p ())
     {
-      fprintf (stderr, "Killing process(es):");
+      fprintf (server_stderr, "Killing process(es):");
       for_each_inferior (&all_processes, print_started_pid);
-      fprintf (stderr, "\n");
+      fprintf (server_stderr, "\n");
     }
   if (have_attached_inferiors_p ())
     {
-      fprintf (stderr, "Detaching process(es):");
+      fprintf (server_stderr, "Detaching process(es):");
       for_each_inferior (&all_processes, print_attached_pid);
-      fprintf (stderr, "\n");
+      fprintf (server_stderr, "\n");
     }
 
   /* Now we can kill or detach the inferiors.  */
@@ -3153,7 +3154,7 @@ detach_or_kill_for_exit_cleanup (void *ignore)
   CATCH (exception, RETURN_MASK_ALL)
     {
       fflush (stdout);
-      fprintf (stderr, "Detach or kill failed: %s\n", exception.message);
+      fprintf (server_stderr, "Detach or kill failed: %s\n", exception.message);
       exit_code = 1;
     }
   END_CATCH
@@ -3173,6 +3174,8 @@ captured_main (int argc, char *argv[])
   volatile int attach = 0;
   int was_running;
 
+  server_stderr = stderr;
+
   while (*next_arg != NULL && **next_arg == '-')
     {
       if (strcmp (*next_arg, "--version") == 0)
@@ -3199,7 +3202,7 @@ captured_main (int argc, char *argv[])
 
 	  if (next_arg == wrapper_argv || *next_arg == NULL)
 	    {
-	      gdbserver_usage (stderr);
+	      gdbserver_usage (server_stderr);
 	      exit (1);
 	    }
 
@@ -3216,7 +3219,7 @@ captured_main (int argc, char *argv[])
 
 	  if (error_msg != NULL)
 	    {
-	      fprintf (stderr, "%s", error_msg);
+	      fprintf (server_stderr, "%s", error_msg);
 	      exit (1);
 	    }
 	}
@@ -3253,9 +3256,9 @@ captured_main (int argc, char *argv[])
 		}
 	      else
 		{
-		  fprintf (stderr, "Don't know how to disable \"%s\".\n\n",
+		  fprintf (server_stderr, "Don't know how to disable \"%s\".\n\n",
 			   tok);
-		  gdbserver_show_disableable (stderr);
+		  gdbserver_show_disableable (server_stderr);
 		  exit (1);
 		}
 	    }
@@ -3275,7 +3278,7 @@ captured_main (int argc, char *argv[])
 	run_once = 1;
       else
 	{
-	  fprintf (stderr, "Unknown argument: %s\n", *next_arg);
+	  fprintf (server_stderr, "Unknown argument: %s\n", *next_arg);
 	  exit (1);
 	}
 
@@ -3322,7 +3325,7 @@ captured_main (int argc, char *argv[])
 
   if (bad_attach)
     {
-      gdbserver_usage (stderr);
+      gdbserver_usage (server_stderr);
       exit (1);
     }
 
@@ -3410,7 +3413,7 @@ captured_main (int argc, char *argv[])
 	  if (exit_requested || run_once)
 	    throw_quit ("Quit");
 
-	  fprintf (stderr,
+	  fprintf (server_stderr,
 		   "Remote side has terminated connection.  "
 		   "GDBserver will reopen the connection.\n");
 
@@ -3442,7 +3445,7 @@ captured_main (int argc, char *argv[])
 		}
 	      else
 		{
-		  fprintf (stderr,
+		  fprintf (server_stderr,
 			   "Disconnected tracing disabled; "
 			   "stopping trace run.\n");
 		  stop_tracing ();
@@ -3476,8 +3479,8 @@ main (int argc, char *argv[])
       if (exception.reason == RETURN_ERROR)
 	{
 	  fflush (stdout);
-	  fprintf (stderr, "%s\n", exception.message);
-	  fprintf (stderr, "Exiting\n");
+	  fprintf (server_stderr, "%s\n", exception.message);
+	  fprintf (server_stderr, "Exiting\n");
 	  exit_code = 1;
 	}
 
@@ -3538,7 +3541,7 @@ process_point_options (struct breakpoint *bp, char **packet)
 	}
       else
 	{
-	  fprintf (stderr, "Unknown token %c, ignoring.\n",
+	  fprintf (server_stderr, "Unknown token %c, ignoring.\n",
 		   *dataptr);
 	  /* Skip tokens until we find one that we recognize.  */
 	  skip_to_semicolon (&dataptr);
@@ -3618,12 +3621,12 @@ process_serial_event (void)
 	    }
 
 	  if (tracing && disconnected_tracing)
-	    fprintf (stderr,
+	    fprintf (server_stderr,
 		     "Disconnected tracing in effect, "
 		     "leaving gdbserver attached to the process\n");
 
 	  if (any_persistent_commands ())
-	    fprintf (stderr,
+	    fprintf (server_stderr,
 		     "Persistent commands are present, "
 		     "leaving gdbserver attached to the process\n");
 
@@ -3653,7 +3656,7 @@ process_serial_event (void)
 	  break; /* from switch/case */
 	}
 
-      fprintf (stderr, "Detaching from process %d\n", pid);
+      fprintf (server_stderr, "Detaching from process %d\n", pid);
       stop_tracing ();
       if (detach_inferior (pid) != 0)
 	write_enn (own_buf);
@@ -3907,7 +3910,7 @@ process_serial_event (void)
 	   reply to it, either.  */
 	return 0;
 
-      fprintf (stderr, "Killing all inferiors\n");
+      fprintf (server_stderr, "Killing all inferiors\n");
       for_each_inferior (&all_processes, kill_inferior_callback);
 
       /* When using the extended protocol, we wait with no program
@@ -3951,7 +3954,7 @@ process_serial_event (void)
 	  if (target_running ())
 	    for_each_inferior (&all_processes,
 			       kill_inferior_callback);
-	  fprintf (stderr, "GDBserver restarting\n");
+	  fprintf (server_stderr, "GDBserver restarting\n");
 
 	  /* Wait till we are at 1st instruction in prog.  */
 	  if (program_argv != NULL)
@@ -4000,7 +4003,7 @@ process_serial_event (void)
 	  /* Be transparent when GDB is connected through stdio -- no
 	     need to spam GDB's console.  */
 	  if (!remote_connection_is_stdio ())
-	    fprintf (stderr, "GDBserver exiting\n");
+	    fprintf (server_stderr, "GDBserver exiting\n");
 	  remote_close ();
 	  exit (0);
 	}
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index 91d4080..4d4de02 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -86,6 +86,10 @@ extern int run_once;
 extern int multi_process;
 extern int non_stop;
 
+/* dedicated stream for the server error messages so that the inferior
+   stderr is not mixed with server's own.  */
+extern FILE *server_stderr;
+
 /* True if the "swbreak+" feature is active.  In that case, GDB wants
    us to report whether a trap is explained by a software breakpoint
    and for the server to handle PC adjustment if necessary on this
diff --git a/gdb/gdbserver/spu-low.c b/gdb/gdbserver/spu-low.c
index 73f1786..2f9678b 100644
--- a/gdb/gdbserver/spu-low.c
+++ b/gdb/gdbserver/spu-low.c
@@ -281,9 +281,9 @@ spu_create_inferior (char *program, char **allargs)
       if (errno == ENOENT)
 	execvp (program, allargs);
 
-      fprintf (stderr, "Cannot exec %s: %s.\n", program,
+      fprintf (server_stderr, "Cannot exec %s: %s.\n", program,
 	       strerror (errno));
-      fflush (stderr);
+      fflush (server_stderr);
       _exit (0177);
     }
 
@@ -304,9 +304,9 @@ spu_attach (unsigned long  pid)
 
   if (ptrace (PTRACE_ATTACH, pid, 0, 0) != 0)
     {
-      fprintf (stderr, "Cannot attach to process %ld: %s (%d)\n", pid,
+      fprintf (server_stderr, "Cannot attach to process %ld: %s (%d)\n", pid,
 	       strerror (errno), errno);
-      fflush (stderr);
+      fflush (server_stderr);
       _exit (0177);
     }
 
@@ -396,7 +396,7 @@ spu_resume (struct thread_resume *resume_info, size_t n)
   /* We don't support hardware single-stepping right now, assume
      GDB knows to use software single-stepping.  */
   if (resume_info[i].kind == resume_step)
-    fprintf (stderr, "Hardware single-step not supported.\n");
+    fprintf (server_stderr, "Hardware single-step not supported.\n");
 
   regcache_invalidate ();
 
@@ -445,7 +445,7 @@ spu_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
 
   if (WIFEXITED (w))
     {
-      fprintf (stderr, "\nChild exited with retcode = %x \n", WEXITSTATUS (w));
+      fprintf (server_stderr, "\nChild exited with retcode = %x \n", WEXITSTATUS (w));
       ourstatus->kind =  TARGET_WAITKIND_EXITED;
       ourstatus->value.integer = WEXITSTATUS (w);
       clear_inferiors ();
@@ -453,7 +453,7 @@ spu_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
     }
   else if (!WIFSTOPPED (w))
     {
-      fprintf (stderr, "\nChild terminated with signal = %x \n", WTERMSIG (w));
+      fprintf (server_stderr, "\nChild terminated with signal = %x \n", WTERMSIG (w));
       ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
       ourstatus->value.sig = gdb_signal_from_host (WTERMSIG (w));
       clear_inferiors ();
diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index 14999e6..0244cdc 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -120,10 +120,10 @@ mywait (ptid_t ptid, struct target_waitstatus *ourstatus, int options,
   if (!remote_connection_is_stdio ())
     {
       if (ourstatus->kind == TARGET_WAITKIND_EXITED)
-	fprintf (stderr,
+	fprintf (server_stderr,
 		 "\nChild exited with status %d\n", ourstatus->value.integer);
       else if (ourstatus->kind == TARGET_WAITKIND_SIGNALLED)
-	fprintf (stderr, "\nChild terminated with signal = 0x%x (%s)\n",
+	fprintf (server_stderr, "\nChild terminated with signal = 0x%x (%s)\n",
 		 gdb_signal_to_host (ourstatus->value.sig),
 		 gdb_signal_to_name (ourstatus->value.sig));
     }
diff --git a/gdb/gdbserver/thread-db.c b/gdb/gdbserver/thread-db.c
index 5bf808e..3559290 100644
--- a/gdb/gdbserver/thread-db.c
+++ b/gdb/gdbserver/thread-db.c
@@ -205,7 +205,7 @@ thread_db_create_event (CORE_ADDR where)
      (except for its own creation, of course).  */
   err = thread_db->td_ta_event_getmsg_p (thread_db->thread_agent, &msg);
   if (err != TD_OK)
-    fprintf (stderr, "thread getmsg err: %s\n",
+    fprintf (server_stderr, "thread getmsg err: %s\n",
 	     thread_db_err_str (err));
 
   /* If we do not know about the main thread yet, this would be a good time to
@@ -726,7 +726,7 @@ try_thread_db_load (const char *library)
 	  const char *const libpath = dladdr_to_soname (td_init);
 
 	  if (libpath != NULL)
-	    fprintf (stderr, "Host %s resolved to: %s.\n",
+	    fprintf (server_stderr, "Host %s resolved to: %s.\n",
 		     library, libpath);
 	}
     }
diff --git a/gdb/gdbserver/utils.c b/gdb/gdbserver/utils.c
index e89a862..46b9690 100644
--- a/gdb/gdbserver/utils.c
+++ b/gdb/gdbserver/utils.c
@@ -21,6 +21,7 @@
 #ifdef IN_PROCESS_AGENT
 #  define PREFIX "ipa: "
 #  define TOOLNAME "GDBserver in-process agent"
+#  define server_stderr stderr
 #else
 #  define PREFIX "gdbserver: "
 #  define TOOLNAME "GDBserver"
@@ -31,7 +32,7 @@
 void
 malloc_failure (long size)
 {
-  fprintf (stderr,
+  fprintf (server_stderr,
 	   PREFIX "ran out of memory while trying to allocate %lu bytes\n",
 	   (unsigned long) size);
   exit (1);
@@ -78,8 +79,8 @@ verror (const char *string, va_list args)
 {
 #ifdef IN_PROCESS_AGENT
   fflush (stdout);
-  vfprintf (stderr, string, args);
-  fprintf (stderr, "\n");
+  vfprintf (server_stderr, string, args);
+  fprintf (server_stderr, "\n");
   exit (1);
 #else
   throw_verror (GENERIC_ERROR, string, args);
@@ -89,9 +90,9 @@ verror (const char *string, va_list args)
 void
 vwarning (const char *string, va_list args)
 {
-  fprintf (stderr, PREFIX);
-  vfprintf (stderr, string, args);
-  fprintf (stderr, "\n");
+  fprintf (server_stderr, PREFIX);
+  vfprintf (server_stderr, string, args);
+  fprintf (server_stderr, "\n");
 }
 
 /* Report a problem internal to GDBserver, and exit.  */
@@ -99,10 +100,10 @@ vwarning (const char *string, va_list args)
 void
 internal_verror (const char *file, int line, const char *fmt, va_list args)
 {
-  fprintf (stderr,  "\
+  fprintf (server_stderr,  "\
 %s:%d: A problem internal to " TOOLNAME " has been detected.\n", file, line);
-  vfprintf (stderr, fmt, args);
-  fprintf (stderr, "\n");
+  vfprintf (server_stderr, fmt, args);
+  fprintf (server_stderr, "\n");
   exit (1);
 }
 
@@ -111,10 +112,10 @@ internal_verror (const char *file, int line, const char *fmt, va_list args)
 void
 internal_vwarning (const char *file, int line, const char *fmt, va_list args)
 {
-  fprintf (stderr,  "\
+  fprintf (server_stderr,  "\
 %s:%d: A problem internal to " TOOLNAME " has been detected.\n", file, line);
-  vfprintf (stderr, fmt, args);
-  fprintf (stderr, "\n");
+  vfprintf (server_stderr, fmt, args);
+  fprintf (server_stderr, "\n");
 }
 
 /* Convert a CORE_ADDR into a HEX string, like %lx.
-- 
1.9.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 3/4] GDBServer: introduce --server-stderr command line option
  2015-03-21  2:35 [PATCH 0/4] GDBServer: introduce a dedicated stderr stream Cleber Rosa
                   ` (2 preceding siblings ...)
  2015-03-21  2:35 ` [PATCH 2/4] GDBServer: give more complete usage information Cleber Rosa
@ 2015-03-21  2:35 ` Cleber Rosa
  2015-03-21  8:26   ` Eli Zaretskii
  2015-03-21 15:05 ` [PATCH 0/4] GDBServer: introduce a dedicated stderr stream Pedro Alves
  4 siblings, 1 reply; 19+ messages in thread
From: Cleber Rosa @ 2015-03-21  2:35 UTC (permalink / raw)
  To: gdb-patches; +Cc: crosa, areis

This command line option will redirect all of the gdbserver's own
output (always sent to stderr) to a separate file. This feature
makes it possible to distinguish between the inferior process stderr
and gdbserver's own stderr.

gdb/doc/ChangeLog:
2015-03-20  Cleber Rosa  <crosa@redhat.com>

	* gdb.texinfo (info): Added section on command line and monitor
	commands.
	(gdbserver man): Added section on new command line option.

gdb/gdbserver/ChangeLog:
2015-03-20  Cleber Rosa  <crosa@redhat.com>

	* server.c (set_server_stderr): New utility function.
	(gdbserver_usage): Add help on '--server-stderr' option.
	(captured_main): Add command line option parsing for
	'--server-stderr'. Replace stderr with gdbserver's own
	stderr.
---
 gdb/doc/gdb.texinfo    | 10 ++++++++++
 gdb/gdbserver/server.c | 28 +++++++++++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 552da31..2834794 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -19205,6 +19205,12 @@ 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.
 
+@cindex @option{--server-stderr}, @code{gdbserver} option
+The @option{--server-stderr} option tells @code{gdbserver} that any content
+that it would normally generate itself to its own @code{stderr} should be
+redirected to another file. This is useful if you want to keep the
+inferior @code{stderr} separate from the one generated by @code{gdbserver}.
+
 @cindex @option{--debug-format}, @code{gdbserver} option
 The @option{--debug-format=option1[,option2,...]} option tells
 @code{gdbserver} to include additional information in each output.
@@ -40886,6 +40892,10 @@ Instruct @code{gdbserver} to include extra information in each line
 of debugging output.
 @xref{Other Command-Line Arguments for gdbserver}.
 
+@item --server-stderr
+Instruct @code{gdbserver} to redirect its own @code{stderr} to another
+file.
+
 @item --wrapper
 Specify a wrapper to launch programs
 for debugging.  The option should be followed by the name of the
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 9dec972..db26f24 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -723,6 +723,25 @@ get_features_xml (const char *annex)
   return NULL;
 }
 
+/* Opens a file and redirects the server own stderr to it */
+
+static int
+set_server_stderr (char *path)
+{
+  FILE *tmp;
+
+  tmp = fopen (path, "w");
+  if (tmp == NULL)
+    {
+      fprintf (stderr,
+	       "Could not open server stderr file '%s': %s\n",
+	       path, strerror(errno));
+      return -1;
+    }
+  server_stderr = tmp;
+  return 0;
+}
+
 void
 monitor_show_help (void)
 {
@@ -3017,6 +3036,7 @@ gdbserver_usage (FILE *stream)
 	   "                            none\n"
 	   "                            timestamp\n"
 	   "  --remote-debug        Enable remote protocol debugging output.\n"
+	   "  --server-stderr=PATH  Redirect server's STDERR to file at PATH.\n"
 	   "  --version             Display version information and exit.\n"
 	   "  --wrapper WRAPPER --  Run WRAPPER to start new programs.\n"
 	   "  --once                Exit after the first connection has "
@@ -3186,7 +3206,13 @@ captured_main (int argc, char *argv[])
 
   while (*next_arg != NULL && **next_arg == '-')
     {
-      if (strcmp (*next_arg, "--version") == 0)
+      if (strncmp (*next_arg, "--server-stderr=",
+		   sizeof ("--server-stderr=") - 1) == 0)
+	{
+	  char *path = *next_arg + (sizeof ("--server-stderr=") - 1);
+	  set_server_stderr (path);
+	}
+      else if (strcmp (*next_arg, "--version") == 0)
 	{
 	  gdbserver_version ();
 	  exit (0);
-- 
1.9.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/4] GDBServer: introduce --server-stderr command line option
  2015-03-21  2:35 ` [PATCH 3/4] GDBServer: introduce --server-stderr command line option Cleber Rosa
@ 2015-03-21  8:26   ` Eli Zaretskii
  2015-03-23 18:51     ` Cleber Rosa
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2015-03-21  8:26 UTC (permalink / raw)
  To: Cleber Rosa; +Cc: gdb-patches, crosa, areis

> From: Cleber Rosa <crosa@redhat.com>
> Cc: crosa@redhat.com, areis@redhat.com
> Date: Fri, 20 Mar 2015 23:34:24 -0300
> 
> This command line option will redirect all of the gdbserver's own
> output (always sent to stderr) to a separate file. This feature
> makes it possible to distinguish between the inferior process stderr
> and gdbserver's own stderr.
> [...]
> +@cindex @option{--server-stderr}, @code{gdbserver} option
> +The @option{--server-stderr} option tells @code{gdbserver} that any content
> +that it would normally generate itself to its own @code{stderr} should be
> +redirected to another file. This is useful if you want to keep the
> +inferior @code{stderr} separate from the one generated by @code{gdbserver}.

Note how the text description of the rationale for the change you
posted to explain it to us is much more useful than the technical
description in the manual.  Why not say in the manual what you told
us?

From the user POV, the fact that gdbserver uses stderr for its own
output is an implementation detail that is almost unimportant.  (It
could be important for redirection purposes, but the command-line
option you introduce eliminates the need to redirect in most cases,
right?)  What _is_ important is that gdbserver's own output will be
redirected to that file, and that important information gets lost in
the confusing "it would normally generate itself to its own 'stderr'",
which leaves unanswered the question what part of gdbserver's output
is "normally" excluded from that.

For this reason, I suggest to name the option "--server-output".

And I think we want a NEWS entry for this change.

> +@item --server-stderr
> +Instruct @code{gdbserver} to redirect its own @code{stderr} to another
> +file.

The option requires an argument, so the argument should be mentioned
with the option and referenced in the text that describes it.

> +static int
> +set_server_stderr (char *path)
> +{
> +  FILE *tmp;
> +
> +  tmp = fopen (path, "w");
> +  if (tmp == NULL)
> +    {
> +      fprintf (stderr,
> +	       "Could not open server stderr file '%s': %s\n",
> +	       path, strerror(errno));
> +      return -1;
> +    }
> +  server_stderr = tmp;
> +  return 0;
> +}
> +
>  void
>  monitor_show_help (void)
>  {
> @@ -3017,6 +3036,7 @@ gdbserver_usage (FILE *stream)
>  	   "                            none\n"
>  	   "                            timestamp\n"
>  	   "  --remote-debug        Enable remote protocol debugging output.\n"
> +	   "  --server-stderr=PATH  Redirect server's STDERR to file at PATH.\n"
>  	   "  --version             Display version information and exit.\n"
>  	   "  --wrapper WRAPPER --  Run WRAPPER to start new programs.\n"
>  	   "  --once                Exit after the first connection has "
> @@ -3186,7 +3206,13 @@ captured_main (int argc, char *argv[])
>  
>    while (*next_arg != NULL && **next_arg == '-')
>      {
> -      if (strcmp (*next_arg, "--version") == 0)
> +      if (strncmp (*next_arg, "--server-stderr=",
> +		   sizeof ("--server-stderr=") - 1) == 0)
> +	{
> +	  char *path = *next_arg + (sizeof ("--server-stderr=") - 1);
> +	  set_server_stderr (path);
> +	}
> +      else if (strcmp (*next_arg, "--version") == 0)

AFAIK, GNU Coding Standards frown on using "path" for anything that is
not PATH-style list of directories.  So please use "file" or "file
name" here.

Thanks.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/4] GDBServer: add 'monitor set server-stderr' command
  2015-03-21  2:35 ` [PATCH 4/4] GDBServer: add 'monitor set server-stderr' command Cleber Rosa
@ 2015-03-21  8:29   ` Eli Zaretskii
  2015-03-23 20:09     ` Cleber Rosa
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2015-03-21  8:29 UTC (permalink / raw)
  To: Cleber Rosa; +Cc: gdb-patches, areis

> From: Cleber Rosa <crosa@redhat.com>
> Cc: crosa@redhat.com, areis@redhat.com
> Date: Fri, 20 Mar 2015 23:34:25 -0300
> 
> gdb/doc/Changelog:
> 2015-03-20  Cleber Rosa  <crosa@redhat.com>
> 
> 	* gdb.texinfo (info): Add documentation about the 'monitor set
> 	server-stderr' command

Don't forget the period at the end of the sentence.

> +@item monitor set server-stderr [PATH]

You want "[@var{path}]" instead.  And please don't call this "path",
for the reasons I explained in my other message.

> +Redirect the server stderr to a file given by @var{path}.

Same comments as in my other message: instead of referencing "stderr",
reference the server's output.

Thanks.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] GDBServer: introduce a dedicated stderr stream
  2015-03-21  2:35 [PATCH 0/4] GDBServer: introduce a dedicated stderr stream Cleber Rosa
                   ` (3 preceding siblings ...)
  2015-03-21  2:35 ` [PATCH 3/4] GDBServer: introduce --server-stderr command line option Cleber Rosa
@ 2015-03-21 15:05 ` Pedro Alves
  2015-03-24 17:07   ` Cleber Rosa
  4 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2015-03-21 15:05 UTC (permalink / raw)
  To: Cleber Rosa, gdb-patches; +Cc: areis, Sergio Durigan Junior

On 03/21/2015 02:34 AM, Cleber Rosa wrote:
> This  patch series add command line options and monitor commands that
> will redirect all of the gdbserver's own output (always sent to stderr)
> to a separate file. This feature makes it possible to distinguish between
> the inferior process stderr and gdbserver's own stderr.

A specific FILE* is a fragile approach; libraries that gdbserver loads
may well print to stdout/stderr or write to file descriptors 1 or
2 directly, for example.  If we're doing this, redirection is best done
at the lower OS file descriptor layer, not at C-runtime stdio (stdout/stderr)
layer, with e.g., dup/dup2.

And, gdbserver itself may print to stdout/stderr _before_ the redirection
command-line option is processed.  Thus it's safer/better to just start gdbserver
with its input/output redirected already.  Of course, then because new
inferiors inherit the input/output from gdbserver, we'd need a way to
start the inferior with input/output redirected somewhere instead.

When native debugging, we can already do exactly that: we can
tell gdb to starts inferior with input/output redirected, using the
"set inferior-tty" command.  I'd be very desirable to be able to do that
with gdbserver as well, in the context of local/remote parity too.  That makes
it possible to have one single gdbserver start multiple programs on separate
ttys, for example.

And I think that would cover your use case too.
You'd start gdbserver with input/output redirected to a pipe, like you
seem to already do (for example), and pass it --inferior-tty=`tty` so
that new inferiors start with input/output connected to that tty.

What do you think?

The code to do this in gdb is in fork-child.c and inflow.c.  Ideally
we'd share it with gdbserver...  Sergio has been on and off working
on exactly sharing that code, for startup-with-shell.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/4] GDBServer: give more complete usage information
  2015-03-21  2:35 ` [PATCH 2/4] GDBServer: give more complete usage information Cleber Rosa
@ 2015-03-21 17:05   ` Pedro Alves
  2015-03-24 14:15     ` Cleber Rosa
  0 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2015-03-21 17:05 UTC (permalink / raw)
  To: Cleber Rosa, gdb-patches; +Cc: areis

On 03/21/2015 02:34 AM, Cleber Rosa wrote:
> --attach/--multi are currently only mentioned on the usage info first lines,
> the meaning of PROG is completely absent and COMM/PID text is unstructured.
> 
> The words and structure chosen may not be perfect but IMHO this is an improvement.

Thanks, indeed this is in clear need of TLC.

I don't think we should put the FOO BAR options under structured
output though.  I don't think it's usual.  Looking at a few core
utils's --help, I saw none that did that.  I think its better to just
extend the intro part to explain PROG, etc.

I went ahead and did that, along with a few more changes.  Let me
know what you think.

Thanks!

------------
From 2e305602202b753b29d4088ff722150dd7b7fccd Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Sat, 21 Mar 2015 17:00:23 +0000
Subject: [PATCH] GDBServer: give more complete usage information

--attach/--multi are currently only mentioned on the usage info first
lines, the meaning of PROG is completely absent and the COMM text does
not mention 'stdio'.

A few options are missing:

 . '-/stdio' is not mentioned as COMM variant.

 . --disable-randomization / --no-disable-randomization is not mentioned.

Although the manual has a comment saying these are superceded by
QDisableRandomization, that only makes sense for "run" in
extended-remote mode.  When we start gdbserver passing it a PROG,
--disable-randomization / --no-disable-randomization do take effect.
So I think we should document these.

 . We show --debug / --remote-debug, so might as well show --disable-packet too.

GDB's --help has this "For more information, consult the GDB manual"
blurb that is missing in GDBserver's --help.

Then shuffle things around a bit into "Operating modes", "Other
options" and "Debug options" sections, similarly to GDB's --help
structure.

Before:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
$ ./gdbserver/gdbserver --help
Usage:  gdbserver [OPTIONS] COMM PROG [ARGS ...]
        gdbserver [OPTIONS] --attach COMM PID
        gdbserver [OPTIONS] --multi COMM

COMM may either be a tty device (for serial debugging), or
HOST:PORT to listen for a TCP connection.

Options:
  --debug               Enable general debugging output.
  --debug-format=opt1[,opt2,...]
                        Specify extra content in debugging output.
                          Options:
                            all
                            none
                            timestamp
  --remote-debug        Enable remote protocol debugging output.
  --version             Display version information and exit.
  --wrapper WRAPPER --  Run WRAPPER to start new programs.
  --once                Exit after the first connection has closed.
Report bugs to "<http://www.gnu.org/software/gdb/bugs/>".
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

After:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
$ ./gdbserver/gdbserver --help
Usage:  gdbserver [OPTIONS] COMM PROG [ARGS ...]
        gdbserver [OPTIONS] --attach COMM PID
        gdbserver [OPTIONS] --multi COMM

COMM may either be a tty device (for serial debugging),
HOST:PORT to listen for a TCP connection, or '-' or 'stdio' to use
stdin/stdout of gdbserver.
PROG is the executable program.  ARGS are arguments passed to inferior.
PID is the process ID to attach to, when --attach is specified.

Operating modes:

  --attach              Attach to running process PID.
  --multi               Start server without a specific program, and
                        only quit when explicitly commanded.
  --once                Exit after the first connection has closed.
  --help                Print this message and then exit.
  --version             Display version information and exit.

Other options:

  --wrapper WRAPPER --  Run WRAPPER to start new programs.
  --disable-randomization
                        Run PROG with address space randomization disabled.
  --no-disable-randomization
                        Don't disable address space randomization when
                        starting PROG.

Debug options:

  --debug               Enable general debugging output.
  --debug-format=opt1[,opt2,...]
                        Specify extra content in debugging output.
                          Options:
                            all
                            none
                            timestamp
  --remote-debug        Enable remote protocol debugging output.
  --disable-packet=opt1[,opt2,...]
                        Disable support for RSP packets or features.
                          Options:
                            vCont, Tthread, qC, qfThreadInfo and
                            threads (disable all threading packets).

For more information, consult the GDB manual (available as on-line
info or a printed manual).
Report bugs to "<http://www.gnu.org/software/gdb/bugs/>".
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

gdb/gdbserver/ChangeLog:
2015-03-20  Pedro Alves  <palves@redhat.com>
	    Cleber Rosa  <crosa@redhat.com>

	* server.c (gdbserver_usage): Reorganize and extend the usage
	message.
---
 gdb/gdbserver/server.c | 40 +++++++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 96b31b8..3408ef7 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2996,10 +2996,32 @@ gdbserver_usage (FILE *stream)
 	   "\tgdbserver [OPTIONS] --attach COMM PID\n"
 	   "\tgdbserver [OPTIONS] --multi COMM\n"
 	   "\n"
-	   "COMM may either be a tty device (for serial debugging), or \n"
-	   "HOST:PORT to listen for a TCP connection.\n"
+	   "COMM may either be a tty device (for serial debugging),\n"
+	   "HOST:PORT to listen for a TCP connection, or '-' or 'stdio' to use \n"
+	   "stdin/stdout of gdbserver.\n"
+	   "PROG is the executable program.  ARGS are arguments passed to inferior.\n"
+	   "PID is the process ID to attach to, when --attach is specified.\n"
+	   "\n"
+	   "Operating modes:\n"
+	   "\n"
+	   "  --attach              Attach to running process PID.\n"
+	   "  --multi               Start server without a specific program, and\n"
+	   "                        only quit when explicitly commanded.\n"
+	   "  --once                Exit after the first connection has closed.\n"
+	   "  --help                Print this message and then exit.\n"
+	   "  --version             Display version information and exit.\n"
+	   "\n"
+	   "Other options:\n"
+	   "\n"
+	   "  --wrapper WRAPPER --  Run WRAPPER to start new programs.\n"
+	   "  --disable-randomization\n"
+	   "                        Run PROG with address space randomization disabled.\n"
+	   "  --no-disable-randomization\n"
+	   "                        Don't disable address space randomization when\n"
+	   "                        starting PROG.\n"
+	   "\n"
+	   "Debug options:\n"
 	   "\n"
-	   "Options:\n"
 	   "  --debug               Enable general debugging output.\n"
 	   "  --debug-format=opt1[,opt2,...]\n"
 	   "                        Specify extra content in debugging output.\n"
@@ -3008,10 +3030,14 @@ gdbserver_usage (FILE *stream)
 	   "                            none\n"
 	   "                            timestamp\n"
 	   "  --remote-debug        Enable remote protocol debugging output.\n"
-	   "  --version             Display version information and exit.\n"
-	   "  --wrapper WRAPPER --  Run WRAPPER to start new programs.\n"
-	   "  --once                Exit after the first connection has "
-								  "closed.\n");
+	   "  --disable-packet=opt1[,opt2,...]\n"
+	   "                        Disable support for RSP packets or features.\n"
+	   "                          Options:\n"
+	   "                            vCont, Tthread, qC, qfThreadInfo and \n"
+	   "                            threads (disable all threading packets).\n"
+	   "\n"
+	   "For more information, consult the GDB manual (available as on-line \n"
+	   "info or a printed manual).\n");
   if (REPORT_BUGS_TO[0] && stream == stdout)
     fprintf (stream, "Report bugs to \"%s\".\n", REPORT_BUGS_TO);
 }
-- 
1.9.3


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/4] GDBServer: introduce --server-stderr command line option
  2015-03-21  8:26   ` Eli Zaretskii
@ 2015-03-23 18:51     ` Cleber Rosa
  2015-03-23 19:12       ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Cleber Rosa @ 2015-03-23 18:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches, areis

On 03/21/2015 05:26 AM, Eli Zaretskii wrote:
>> From: Cleber Rosa <crosa@redhat.com>
>> Cc: crosa@redhat.com, areis@redhat.com
>> Date: Fri, 20 Mar 2015 23:34:24 -0300
>>
>> This command line option will redirect all of the gdbserver's own
>> output (always sent to stderr) to a separate file. This feature
>> makes it possible to distinguish between the inferior process stderr
>> and gdbserver's own stderr.
>> [...]
>> +@cindex @option{--server-stderr}, @code{gdbserver} option
>> +The @option{--server-stderr} option tells @code{gdbserver} that any content
>> +that it would normally generate itself to its own @code{stderr} should be
>> +redirected to another file. This is useful if you want to keep the
>> +inferior @code{stderr} separate from the one generated by @code{gdbserver}.
> Note how the text description of the rationale for the change you
> posted to explain it to us is much more useful than the technical
> description in the manual.  Why not say in the manual what you told
> us?
>
>  From the user POV, the fact that gdbserver uses stderr for its own
> output is an implementation detail that is almost unimportant.  (It
> could be important for redirection purposes, but the command-line
> option you introduce eliminates the need to redirect in most cases,
> right?)  What _is_ important is that gdbserver's own output will be
> redirected to that file, and that important information gets lost in
> the confusing "it would normally generate itself to its own 'stderr'",
> which leaves unanswered the question what part of gdbserver's output
> is "normally" excluded from that.

Your comments make a lot of sense. Indeed using "normally" introduces 
unnecessary doubt and confusion. Also, the user friendlier text in the 
commit message won't reach users. Good catches!

>
> For this reason, I suggest to name the option "--server-output".

Agreed.

BTW, to keep a more obvious correlation between variable name 
(server_stderr) and command line option (now --server-output), I'm going 
to rename the former unless someone feels strongly against it.

>
> And I think we want a NEWS entry for this change.

OK.

>
>> +@item --server-stderr
>> +Instruct @code{gdbserver} to redirect its own @code{stderr} to another
>> +file.
> The option requires an argument, so the argument should be mentioned
> with the option and referenced in the text that describes it.

Sure, I also feel an example could help. How do you feel about this:

@cindex @option{--server-output}, @code{gdbserver} option
The @option{--server-output=path} option tells @code{gdbserver} to send
all its output to a file given by @var{path}. This can be useful, for 
instance,
if you need to collect the server output and/or the inferior output, but 
want
to keep them separate:

@smallexample
$ gdbserver --server-output=server.log :2222 testprog >test.out 2>test.err
@end smallexample

>
>> +static int
>> +set_server_stderr (char *path)
>> +{
>> +  FILE *tmp;
>> +
>> +  tmp = fopen (path, "w");
>> +  if (tmp == NULL)
>> +    {
>> +      fprintf (stderr,
>> +	       "Could not open server stderr file '%s': %s\n",
>> +	       path, strerror(errno));
>> +      return -1;
>> +    }
>> +  server_stderr = tmp;
>> +  return 0;
>> +}
>> +
>>   void
>>   monitor_show_help (void)
>>   {
>> @@ -3017,6 +3036,7 @@ gdbserver_usage (FILE *stream)
>>   	   "                            none\n"
>>   	   "                            timestamp\n"
>>   	   "  --remote-debug        Enable remote protocol debugging output.\n"
>> +	   "  --server-stderr=PATH  Redirect server's STDERR to file at PATH.\n"
>>   	   "  --version             Display version information and exit.\n"
>>   	   "  --wrapper WRAPPER --  Run WRAPPER to start new programs.\n"
>>   	   "  --once                Exit after the first connection has "
>> @@ -3186,7 +3206,13 @@ captured_main (int argc, char *argv[])
>>   
>>     while (*next_arg != NULL && **next_arg == '-')
>>       {
>> -      if (strcmp (*next_arg, "--version") == 0)
>> +      if (strncmp (*next_arg, "--server-stderr=",
>> +		   sizeof ("--server-stderr=") - 1) == 0)
>> +	{
>> +	  char *path = *next_arg + (sizeof ("--server-stderr=") - 1);
>> +	  set_server_stderr (path);
>> +	}
>> +      else if (strcmp (*next_arg, "--version") == 0)
> AFAIK, GNU Coding Standards frown on using "path" for anything that is
> not PATH-style list of directories.  So please use "file" or "file
> name" here.

I could not find a mention on the GNU Coding Standards manual itself, 
but yeah, it may be (informally?) frowned upon as it's indeed confusing.

>
> Thanks.

Thank you for the comments and very valid points!

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/4] GDBServer: introduce --server-stderr command line option
  2015-03-23 18:51     ` Cleber Rosa
@ 2015-03-23 19:12       ` Eli Zaretskii
  2015-03-23 20:35         ` Cleber Rosa
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2015-03-23 19:12 UTC (permalink / raw)
  To: Cleber Rosa; +Cc: gdb-patches, areis

> Date: Mon, 23 Mar 2015 15:48:29 -0300
> From: Cleber Rosa <crosa@redhat.com>
> CC: gdb-patches@sourceware.org, areis@redhat.com
> 
> >> +@item --server-stderr
> >> +Instruct @code{gdbserver} to redirect its own @code{stderr} to another
> >> +file.
> > The option requires an argument, so the argument should be mentioned
> > with the option and referenced in the text that describes it.
> 
> Sure, I also feel an example could help. How do you feel about this:
> 
> @cindex @option{--server-output}, @code{gdbserver} option
> The @option{--server-output=path} option tells @code{gdbserver} to send

@option{--server-output=@var{path}} (and once again, please use
"file" or "filename", not "path").

Also, what happened to the @item?

> all its output to a file given by @var{path}. This can be useful, for 
                                              ^^
Two spaces between sentences.

> @smallexample
> $ gdbserver --server-output=server.log :2222 testprog >test.out 2>test.err
> @end smallexample

This line is too long; either try to make it shorter, e.g., by using
shorter file/program names, or break it into 2 lines.

Otherwise, this is fine, thanks.
> > AFAIK, GNU Coding Standards frown on using "path" for anything that is
> > not PATH-style list of directories.  So please use "file" or "file
> > name" here.
> 
> I could not find a mention on the GNU Coding Standards manual itself, 

It's in the node "GNU Manuals":

     Please do not use the term "pathname" that is used in Unix
  documentation; use "file name" (two words) instead.  We use the term
  "path" only for search paths, which are lists of directory names.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/4] GDBServer: add 'monitor set server-stderr' command
  2015-03-21  8:29   ` Eli Zaretskii
@ 2015-03-23 20:09     ` Cleber Rosa
  0 siblings, 0 replies; 19+ messages in thread
From: Cleber Rosa @ 2015-03-23 20:09 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches, areis

On 03/21/2015 05:29 AM, Eli Zaretskii wrote:
>> From: Cleber Rosa <crosa@redhat.com>
>> Cc: crosa@redhat.com, areis@redhat.com
>> Date: Fri, 20 Mar 2015 23:34:25 -0300
>>
>> gdb/doc/Changelog:
>> 2015-03-20  Cleber Rosa  <crosa@redhat.com>
>>
>> 	* gdb.texinfo (info): Add documentation about the 'monitor set
>> 	server-stderr' command
> Don't forget the period at the end of the sentence.
>
>> +@item monitor set server-stderr [PATH]
> You want "[@var{path}]" instead.  And please don't call this "path",
> for the reasons I explained in my other message.

Sure! BTW, I mistakenly added [PATH] as an optional parameter, and it 
should be mandatory. Because of that, this is the new version:

@item monitor set server-output @var{output_filename}
Redirect the server output to a file given by @var{output_filename}.

Also, FIY, the server behavior when the "output_filename" parameter is 
not passed in, is currently the same as when a parameter is missing from 
another commands such as "monitor set remote-debug":

(gdb) monitor set server-output
Unknown monitor command.
...
Protocol error with Rcmd


(gdb) monitor set remote-debug
Unknown monitor command.
...
Protocol error with Rcmd

It looks like this is an area that can receive some improvement, but 
it's obviously outside the scope of the changes proposed/worked here.

>
>> +Redirect the server stderr to a file given by @var{path}.
> Same comments as in my other message: instead of referencing "stderr",
> reference the server's output.
>
> Thanks.

Sure! I should really have learned that lesson by now!

Thanks a million!

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/4] GDBServer: introduce --server-stderr command line option
  2015-03-23 19:12       ` Eli Zaretskii
@ 2015-03-23 20:35         ` Cleber Rosa
  2015-03-23 20:43           ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Cleber Rosa @ 2015-03-23 20:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches, areis

On 03/23/2015 04:12 PM, Eli Zaretskii wrote:
>> Date: Mon, 23 Mar 2015 15:48:29 -0300
>> From: Cleber Rosa <crosa@redhat.com>
>> CC: gdb-patches@sourceware.org, areis@redhat.com
>>
>>>> +@item --server-stderr
>>>> +Instruct @code{gdbserver} to redirect its own @code{stderr} to another
>>>> +file.
>>> The option requires an argument, so the argument should be mentioned
>>> with the option and referenced in the text that describes it.
>> Sure, I also feel an example could help. How do you feel about this:
>>
>> @cindex @option{--server-output}, @code{gdbserver} option
>> The @option{--server-output=path} option tells @code{gdbserver} to send
> @option{--server-output=@var{path}} (and once again, please use
> "file" or "filename", not "path").

Sorry, I missed that in the first reply but it's covered in the updated 
patches (and inline for information purposes):

@cindex @option{--server-output}, @code{gdbserver} option
The @option{--server-output=@var{file}} option tells @code{gdbserver} to 
send
all its output to a file given by @var{file}.  This can be useful, for 
instance,
if you need to collect the server output and/or the inferior output, but 
want
to keep them separate:

@smallexample
$ gdbserver --server-output=log :2222 bin >bin.out 2>bin.err
@end smallexample

>
> Also, what happened to the @item?

@item --server-output=file
Instruct @code{gdbserver} to redirect its own output to @var{file}.

Which renders as:

        --server-output=file
            Instruct "gdbserver" to redirect its own output to file.

Too simplistic or is that OK?

>
>> all its output to a file given by @var{path}. This can be useful, for
>                                                ^^
> Two spaces between sentences.
>
>> @smallexample
>> $ gdbserver --server-output=server.log :2222 testprog >test.out 2>test.err
>> @end smallexample
> This line is too long; either try to make it shorter, e.g., by using
> shorter file/program names, or break it into 2 lines.

OK, how about (repeated from earlier):

@smallexample
$ gdbserver --server-output=log :2222 bin >bin.out 2>bin.err
@end smallexample

>
> Otherwise, this is fine, thanks.
>>> AFAIK, GNU Coding Standards frown on using "path" for anything that is
>>> not PATH-style list of directories.  So please use "file" or "file
>>> name" here.
>> I could not find a mention on the GNU Coding Standards manual itself,
> It's in the node "GNU Manuals":
>
>       Please do not use the term "pathname" that is used in Unix
>    documentation; use "file name" (two words) instead.  We use the term
>    "path" only for search paths, which are lists of directory names.

Oh, thanks for the pointer and for having the manual on (brain) cache 
and catching that!

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/4] GDBServer: introduce --server-stderr command line option
  2015-03-23 20:35         ` Cleber Rosa
@ 2015-03-23 20:43           ` Eli Zaretskii
  0 siblings, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2015-03-23 20:43 UTC (permalink / raw)
  To: Cleber Rosa; +Cc: gdb-patches, areis

> Date: Mon, 23 Mar 2015 17:34:46 -0300
> From: Cleber Rosa <crosa@redhat.com>
> CC: gdb-patches@sourceware.org, areis@redhat.com
> 
> > Also, what happened to the @item?
> 
> @item --server-output=file
> Instruct @code{gdbserver} to redirect its own output to @var{file}.
> 
> Which renders as:
> 
>         --server-output=file
>             Instruct "gdbserver" to redirect its own output to file.
> 
> Too simplistic or is that OK?

OK.

> >> @smallexample
> >> $ gdbserver --server-output=server.log :2222 testprog >test.out 2>test.err
> >> @end smallexample
> > This line is too long; either try to make it shorter, e.g., by using
> > shorter file/program names, or break it into 2 lines.
> 
> OK, how about (repeated from earlier):
> 
> @smallexample
> $ gdbserver --server-output=log :2222 bin >bin.out 2>bin.err
> @end smallexample

That's fine.

Thanks.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/4] GDBServer: give more complete usage information
  2015-03-21 17:05   ` Pedro Alves
@ 2015-03-24 14:15     ` Cleber Rosa
  2015-03-31 14:44       ` Cleber Rosa
  2015-04-01 10:10       ` Pedro Alves
  0 siblings, 2 replies; 19+ messages in thread
From: Cleber Rosa @ 2015-03-24 14:15 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches; +Cc: areis

On 03/21/2015 02:05 PM, Pedro Alves wrote:
> I went ahead and did that, along with a few more changes.  Let me
> know what you think.

Pedro,

This is so much better than my (knowingly limited) improvements! Please 
go ahead with your version, and I'll happily drop that patch from my series.

Thanks!
Cleber Rosa.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] GDBServer: introduce a dedicated stderr stream
  2015-03-21 15:05 ` [PATCH 0/4] GDBServer: introduce a dedicated stderr stream Pedro Alves
@ 2015-03-24 17:07   ` Cleber Rosa
  2015-04-01 11:17     ` Pedro Alves
  0 siblings, 1 reply; 19+ messages in thread
From: Cleber Rosa @ 2015-03-24 17:07 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches; +Cc: areis, Sergio Durigan Junior

On 03/21/2015 12:05 PM, Pedro Alves wrote:
> On 03/21/2015 02:34 AM, Cleber Rosa wrote:
>> This  patch series add command line options and monitor commands that
>> will redirect all of the gdbserver's own output (always sent to stderr)
>> to a separate file. This feature makes it possible to distinguish between
>> the inferior process stderr and gdbserver's own stderr.
> A specific FILE* is a fragile approach; libraries that gdbserver loads
> may well print to stdout/stderr or write to file descriptors 1 or
> 2 directly, for example.  If we're doing this, redirection is best done
> at the lower OS file descriptor layer, not at C-runtime stdio (stdout/stderr)
> layer, with e.g., dup/dup2.

I do agree with the fragility of the method chosen.  The truth is that 
all other approaches I considered turned out to be, IMHO, excessively 
complex and cumbersome for what was trying to be achieved.

>
> And, gdbserver itself may print to stdout/stderr _before_ the redirection
> command-line option is processed.  Thus it's safer/better to just start gdbserver
> with its input/output redirected already.  Of course, then because new
> inferiors inherit the input/output from gdbserver, we'd need a way to
> start the inferior with input/output redirected somewhere instead.

You're absolutely right that loaded libraries can write to the file 
descriptor this patch is trying to "protect", and so can instrumented 
commands during a debug session and possibly many others, other than 
gdbserver itself.  Even new code that slips into gdbserver itself may 
end up breaking this "contract".

At this point, it looks like I'm advocating, or even proving, that the 
chosen approach is too fragile.  But, my real point is that given the 
nature of the application itself and its typical users, this looked like 
the best balance between "simple enough" and "safe enough".  I also 
believe that if this feature gets enough users, violations of this 
"contract" would be readily caught and fixed. This could even evolve 
towards a unified way for gdbserver and libraries to announce errors 
(instead of printf()s).

I tried to map the code flow, and looking at server.c:main() and 
captured_main(), it looks pretty safe to assume that gdbserver itself 
won't write to stderr. This is a snippet from the proposed 
server.c:captured_main():

   server_output = stderr;

   while (*next_arg != NULL && **next_arg == '-')
     {
       if (strncmp (*next_arg, "--server-output=",
            sizeof ("--server-output=") - 1) == 0)
     {
       char *out_filename = *next_arg + (sizeof ("--server-output=") - 1);
       set_server_output (out_filename);
     }

Note that the output swap (set_server_output()) happens very early, and 
this increases my confidence of not having "bad" output from gdbserver 
itself.

>
> When native debugging, we can already do exactly that: we can
> tell gdb to starts inferior with input/output redirected, using the
> "set inferior-tty" command.  I'd be very desirable to be able to do that
> with gdbserver as well, in the context of local/remote parity too.  That makes
> it possible to have one single gdbserver start multiple programs on separate
> ttys, for example.
>
> And I think that would cover your use case too.
> You'd start gdbserver with input/output redirected to a pipe, like you
> seem to already do (for example), and pass it --inferior-tty=`tty` so
> that new inferiors start with input/output connected to that tty.

I actually tried that approach many months ago[1]. I was actually 
expecting the feature parity between gdb and gdbserver, and it kind of 
let me down. But, even with an exclusive TTY for the inferior, one big 
gap would remain: no clean way to differentiate between an application 
STDOUT and STDERR simply by reading from its TTY.

>
> What do you think?
>
> The code to do this in gdb is in fork-child.c and inflow.c.  Ideally
> we'd share it with gdbserver...  Sergio has been on and off working
> on exactly sharing that code, for startup-with-shell.

My dream feature set would be gdb supporting redirection *including* 
STDERR (doesn't seem to be the case right now[2]), and gdbserver with 
complete feature parity. Sorry if this is too selfish (but it's honest): 
this dream feature set looks too expensive at this time.

So, to sum it up, here's my plea: the proposed changes, fragile indeed 
but effective, have a very clear use case. The overlap is minimal to 
non-existing with future work getting gdbserver in feature parity with gdb.

>
> Thanks,
> Pedro Alves
>

Thanks again for the very fertile comments and review!
Cleber Rosa.

[1] - 
https://github.com/clebergnu/avocado/commit/fc813a5d047c7741ee603e00e638ef342c7997b6
[2] - 
https://sourceware.org/gdb/current/onlinedocs/gdb/Input_002fOutput.html#Input_002fOutput

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/4] GDBServer: give more complete usage information
  2015-03-24 14:15     ` Cleber Rosa
@ 2015-03-31 14:44       ` Cleber Rosa
  2015-04-01 10:10       ` Pedro Alves
  1 sibling, 0 replies; 19+ messages in thread
From: Cleber Rosa @ 2015-03-31 14:44 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches; +Cc: areis

On 03/24/2015 11:15 AM, Cleber Rosa wrote:
> On 03/21/2015 02:05 PM, Pedro Alves wrote:
>> I went ahead and did that, along with a few more changes.  Let me
>> know what you think.
>
> Pedro,
>
> This is so much better than my (knowingly limited) improvements! 
> Please go ahead with your version, and I'll happily drop that patch 
> from my series.
>
> Thanks!
> Cleber Rosa.

Hi Pedro,

Just as a reminder: I believe this one (your version) can be pushed 
without any side effects.

Thanks,
CR.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/4] GDBServer: give more complete usage information
  2015-03-24 14:15     ` Cleber Rosa
  2015-03-31 14:44       ` Cleber Rosa
@ 2015-04-01 10:10       ` Pedro Alves
  1 sibling, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2015-04-01 10:10 UTC (permalink / raw)
  To: Cleber Rosa, gdb-patches; +Cc: areis

On 03/24/2015 02:15 PM, Cleber Rosa wrote:
> On 03/21/2015 02:05 PM, Pedro Alves wrote:
>> I went ahead and did that, along with a few more changes.  Let me
>> know what you think.
> 
> Pedro,
> 
> This is so much better than my (knowingly limited) improvements! Please 
> go ahead with your version, and I'll happily drop that patch from my series.

Now pushed.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] GDBServer: introduce a dedicated stderr stream
  2015-03-24 17:07   ` Cleber Rosa
@ 2015-04-01 11:17     ` Pedro Alves
  0 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2015-04-01 11:17 UTC (permalink / raw)
  To: Cleber Rosa, gdb-patches; +Cc: areis, Sergio Durigan Junior

On 03/24/2015 05:07 PM, Cleber Rosa wrote:
> On 03/21/2015 12:05 PM, Pedro Alves wrote:
>> On 03/21/2015 02:34 AM, Cleber Rosa wrote:
>>> This  patch series add command line options and monitor commands that
>>> will redirect all of the gdbserver's own output (always sent to stderr)
>>> to a separate file. This feature makes it possible to distinguish between
>>> the inferior process stderr and gdbserver's own stderr.
>> A specific FILE* is a fragile approach; libraries that gdbserver loads
>> may well print to stdout/stderr or write to file descriptors 1 or
>> 2 directly, for example.  If we're doing this, redirection is best done
>> at the lower OS file descriptor layer, not at C-runtime stdio (stdout/stderr)
>> layer, with e.g., dup/dup2.
> 
> I do agree with the fragility of the method chosen.  The truth is that 
> all other approaches I considered turned out to be, IMHO, excessively 
> complex and cumbersome for what was trying to be achieved.
> 
>>
>> And, gdbserver itself may print to stdout/stderr _before_ the redirection
>> command-line option is processed.  Thus it's safer/better to just start gdbserver
>> with its input/output redirected already.  Of course, then because new
>> inferiors inherit the input/output from gdbserver, we'd need a way to
>> start the inferior with input/output redirected somewhere instead.
> 
> You're absolutely right that loaded libraries can write to the file 
> descriptor this patch is trying to "protect", and so can instrumented 
> commands during a debug session and possibly many others, other than 
> gdbserver itself.  Even new code that slips into gdbserver itself may 
> end up breaking this "contract".
> 

Why not do it with dup/dup2 instead then?  Something around this:

int inferior_fds[3] = { -1, -1, -1 };

set_server_output ()
{
  for (i = 0; i < 3; i++)
    inferior_fds[i] = dup (i);

  for (i = 0; i < 3; i++)
    dup2 (i, redirect_fd);
}

and then when the inferior is created, in linux_create_inferior,
around where we close most of the fork child's fds, make the child
re-redirect its output to the original file descriptors:

for (i = 0; i < 3; i++)
  if (inferior_fds[i] != -1)
    dup2 (i, inferior_fds[i]);

Note this linux_create_inferior change wouldn't be that
different from an option that would make gdbserver redirect
the inferior's stdin/stdout/stderr instead of its own without
shell involvement, like an hypothetical
"set inferior-stdin|stdout|stderr" feature to complement
"set inferior-tty".

> I actually tried that approach many months ago[1]. I was actually 
> expecting the feature parity between gdb and gdbserver, and it kind of 
> let me down. But, even with an exclusive TTY for the inferior, one big 
> gap would remain: no clean way to differentiate between an application 
> STDOUT and STDERR simply by reading from its TTY.
> 

> My dream feature set would be gdb supporting redirection *including* 
> STDERR (doesn't seem to be the case right now[2]),

If you're referring to the "run > outfile" example, GDB just creates
the inferior using the shell, like "sh -c program > outfile" behind
the scenes.  So "2 > stderr.txt" should work fine too.

Making gdbserver start the inferior with a shell so these things
work with gdbserver too (with target extended-remote and "run") is
exactly the gdb/gdbserver feature parity point that Sergio is
working on.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2015-04-01 11:17 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-21  2:35 [PATCH 0/4] GDBServer: introduce a dedicated stderr stream Cleber Rosa
2015-03-21  2:35 ` [PATCH 4/4] GDBServer: add 'monitor set server-stderr' command Cleber Rosa
2015-03-21  8:29   ` Eli Zaretskii
2015-03-23 20:09     ` Cleber Rosa
2015-03-21  2:35 ` [PATCH 1/4] GDBServer: introduce a stderr stream dedicated to the server Cleber Rosa
2015-03-21  2:35 ` [PATCH 2/4] GDBServer: give more complete usage information Cleber Rosa
2015-03-21 17:05   ` Pedro Alves
2015-03-24 14:15     ` Cleber Rosa
2015-03-31 14:44       ` Cleber Rosa
2015-04-01 10:10       ` Pedro Alves
2015-03-21  2:35 ` [PATCH 3/4] GDBServer: introduce --server-stderr command line option Cleber Rosa
2015-03-21  8:26   ` Eli Zaretskii
2015-03-23 18:51     ` Cleber Rosa
2015-03-23 19:12       ` Eli Zaretskii
2015-03-23 20:35         ` Cleber Rosa
2015-03-23 20:43           ` Eli Zaretskii
2015-03-21 15:05 ` [PATCH 0/4] GDBServer: introduce a dedicated stderr stream Pedro Alves
2015-03-24 17:07   ` Cleber Rosa
2015-04-01 11:17     ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).