From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 76510 invoked by alias); 24 Mar 2015 17:07:43 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 76498 invoked by uid 89); 24 Mar 2015 17:07:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,T_RP_MATCHES_RCVD,UNSUBSCRIBE_BODY autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 24 Mar 2015 17:07:41 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t2OH7dCg008467 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Tue, 24 Mar 2015 13:07:39 -0400 Received: from x220.homelab.tallawa.org (ovpn-112-52.phx2.redhat.com [10.3.112.52]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t2OH7U5Q032200 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Tue, 24 Mar 2015 13:07:35 -0400 Message-ID: <551199CF.2000400@redhat.com> Date: Tue, 24 Mar 2015 17:07:00 -0000 From: Cleber Rosa User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Pedro Alves , gdb-patches@sourceware.org CC: areis@redhat.com, Sergio Durigan Junior Subject: Re: [PATCH 0/4] GDBServer: introduce a dedicated stderr stream References: <1426905265-8495-1-git-send-email-crosa@redhat.com> <550D889C.3030900@redhat.com> In-Reply-To: <550D889C.3030900@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2015-03/txt/msg00801.txt.bz2 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