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

Pedro Alves
2015-04-01T11:17:08Z

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