Re: [PATCH 0/4] GDBServer: introduce a dedicated stderr stream

[Cleber Rosa <crosa@redhat.com>]

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