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

Cleber Rosa
2015-03-24T17:07:43Z

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] -