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