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

[Pedro Alves <palves@redhat.com>]

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