[PATCH] Target stderr not displayed thru MI

[PATCH] Target stderr not displayed thru MI

[Denis PILAT]

[-- Attachment #1: Type: text/plain, Size: 523 bytes --]

Hi all,

When debugging a program thru Eclipse or any front end using MI,
the stderr of the debugged program is lost.

This problem occures only on remote targets since native ones don't
use MI stream for output.

In the file gdb/mi/mi-interp.c, the 'mi_interpreter_resume' function
redirects the gdb stream into MI streams. All streams are redirected
except gdb_stdtargerr.

The attached patch (from a ST colleague) redirects target error output
on the unique MI target output stream.

Do you have any comment ?

Denis




[-- Attachment #2: mi-interp.patch --]
[-- Type: text/plain, Size: 594 bytes --]

2005-11-20  Serge Chatroux  <serge.chatroux@st.com>

	* mi-interp.c (mi_interpreter_resume): Route target error
	through the MI.


Index: mi-interp.c
===================================================================
--- mi-interp.c	(revision 245)
+++ mi-interp.c	(working copy)
@@ -119,6 +119,8 @@
   gdb_stdlog = mi->log;
   /* Route target output through the MI. */
   gdb_stdtarg = mi->targ;
+  /* Route target error through the MI as well. */
+  gdb_stdtargerr = mi->targ;
 
   /* Replace all the hooks that we know about.  There really needs to
      be a better way of doing this... */

Re: [PATCH] Target stderr not displayed thru MI

[Jim Blandy]

On 11/29/05, Denis PILAT <denis.pilat@st.com> wrote:
> 2005-11-20  Serge Chatroux  <serge.chatroux@st.com>
>
>         * mi-interp.c (mi_interpreter_resume): Route target error
>         through the MI.

Hi, Denis.  This change looks right to me; let's give the MI fans a
chance to comment, but if nobody else has reservations, you can commit
this.

Since the ChangeLog entry is to appear in gdb/ChangeLog (there is no
gdb/mi/ChangeLog any more), the filename should be "mi/mi-interp.c".

Re:[PATCH] Target stderr not displayed thru MI

[Nick Roberts]

> +  /* Route target error through the MI as well. */
> +  gdb_stdtargerr = mi->targ;

I know nothing about remote debugging but shouldn't error output go to
mi->err so that you can distinguish it from target output?

Nick

Re: [PATCH] Target stderr not displayed thru MI

[Denis PILAT]

Nick Roberts wrote:

>>+  /* Route target error through the MI as well. */
>>+  gdb_stdtargerr = mi->targ;
>>    
>>
>
>I know nothing about remote debugging but shouldn't error output go to
>mi->err so that you can distinguish it from target output?
>
>  
>
Nick,

The mi->err is used for displaying debugger errors, not the error coming 
from the target execution.
The main problem with MI is that we can not distinguish target stdout 
from target stderr.

Denis

Re: [PATCH] Target stderr not displayed thru MI

[Bob Rossi]

On Wed, Nov 30, 2005 at 01:58:06PM +0100, Denis PILAT wrote:
> 
> Nick Roberts wrote:
> 
> >>+  /* Route target error through the MI as well. */
> >>+  gdb_stdtargerr = mi->targ;
> >>   
> >>
> >
> >I know nothing about remote debugging but shouldn't error output go to
> >mi->err so that you can distinguish it from target output?
> >
> > 
> >
> Nick,
> 
> The mi->err is used for displaying debugger errors, not the error coming 
> from the target execution.
> The main problem with MI is that we can not distinguish target stdout 
> from target stderr.

Hmm, that's interesting. Are you using the -inferior-tty-set option?

A program can write to stdout (1), stderr (2), but it can
also write to any other file descriptor it wants to. I understand
solving the stdout/stderr case would be helpful, but would it be good to
think of it in terms of file descriptor number? For instance, a program
could easily open another file descriptor and point it to the
controlling terminal and write data. Is this case relavent?

Thanks,
Bob Rossi

Re: [PATCH] Target stderr not displayed thru MI

[Daniel Jacobowitz]

On Wed, Nov 30, 2005 at 08:50:44AM -0500, Bob Rossi wrote:
> On Wed, Nov 30, 2005 at 01:58:06PM +0100, Denis PILAT wrote:
> > 
> > Nick Roberts wrote:
> > 
> > >>+  /* Route target error through the MI as well. */
> > >>+  gdb_stdtargerr = mi->targ;
> > >>   
> > >>
> > >
> > >I know nothing about remote debugging but shouldn't error output go to
> > >mi->err so that you can distinguish it from target output?
> > >
> > > 
> > >
> > Nick,
> > 
> > The mi->err is used for displaying debugger errors, not the error coming 
> > from the target execution.
> > The main problem with MI is that we can not distinguish target stdout 
> > from target stderr.
> 
> Hmm, that's interesting. Are you using the -inferior-tty-set option?
> 
> A program can write to stdout (1), stderr (2), but it can
> also write to any other file descriptor it wants to. I understand
> solving the stdout/stderr case would be helpful, but would it be good to
> think of it in terms of file descriptor number? For instance, a program
> could easily open another file descriptor and point it to the
> controlling terminal and write data. Is this case relavent?

I assume that Denis is talking about a remote target here, providing
I/O via their target protocol, rather than a native TTY.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] Target stderr not displayed thru MI

[Denis PILAT]

[-- Attachment #1: Type: text/plain, Size: 1152 bytes --]



Nick Roberts wrote:

 >
 > > Nick,
 > >
 > > The mi->err is used for displaying debugger errors, not the error coming
 > > from the target execution.
 > > The main problem with MI is that we can not distinguish target stdout
 > > from target stderr.
 >
 >In that case, can you create a new MI channel? e.g.
 >
 >
 >
I'm not sure creating a new mi stream is on fashion, but I'm not opposed
to that.

My original problem was the following:
  >   When debugging a program thru Eclipse or any front end using MI,
  >   the stderr of the debugged program is lost.
  >   This problem occures only on remote targets since native ones don't
  >   use MI stream for output.

Your solution won't fix my problem since current Eclipse uses MI 1 and will
never recognize the new prefix.
What we can do is creating a new error stream mi->targerr but with a prefix
that will be:
- the same than the mi->targ for MI 1 and 2, means "@"
- a new one like you proposed for MI versoin > 2: "#" for instance.


I attach a new patch in that sense. It does not include the documentation patch 
  about MI new stream but it's just to give you an idea.
Any comment ?

Denis



[-- Attachment #2: mi-interp.patch --]
[-- Type: text/plain, Size: 1481 bytes --]

2005-12-01  Denis Pilat  <denis.pilat@st.com>

	* mi-interp.c (mi_interpreter_resume): Route target error through the MI.
	(struct mi_interp): Add field targerr.
	(mi_interpreter_init): Initialize mi target error stream.

Index: mi/mi-interp.c
===================================================================
--- mi/mi-interp.c	(revision 224)
+++ mi/mi-interp.c	(working copy)
@@ -40,6 +40,7 @@
   struct ui_file *err;
   struct ui_file *log;
   struct ui_file *targ;
+  struct ui_file *targerr;
   struct ui_file *event_channel;
 
   /* This is the interpreter for the mi... */
@@ -86,6 +87,15 @@
   mi->err = mi_console_file_new (raw_stdout, "&", '"');
   mi->log = mi->err;
   mi->targ = mi_console_file_new (raw_stdout, "@", '"');
+
+  /* MI 1 and 2 target error stream use the same steam prefix "@" in oder
+     to ensure backward compatibility with old frondtend, MI 3 uses
+     a new prefix "#" */
+  if (mi_version (uiout) > 2)
+    mi->targerr = mi_console_file_new (raw_stdout, "#", '"');
+  else
+    mi->targerr = mi_console_file_new (raw_stdout, "@", '"');
+
   mi->event_channel = mi_console_file_new (raw_stdout, "=", 0);
 
   return mi;
@@ -119,6 +129,8 @@
   gdb_stdlog = mi->log;
   /* Route target output through the MI. */
   gdb_stdtarg = mi->targ;
+  /* Route target error through the MI error stream. */
+  gdb_stdtargerr = mi->targerr;
 
   /* Replace all the hooks that we know about.  There really needs to
      be a better way of doing this... */

Re: [PATCH] Target stderr not displayed thru MI

[Bob Rossi]

> I attach a new patch in that sense. It does not include the documentation 
> patch about MI new stream but it's just to give you an idea.
> Any comment ?

> +  /* MI 1 and 2 target error stream use the same steam prefix "@" in oder
> +     to ensure backward compatibility with old frondtend, MI 3 uses
> +     a new prefix "#" */
> +  if (mi_version (uiout) > 2)
> +    mi->targerr = mi_console_file_new (raw_stdout, "#", '"');
> +  else
> +    mi->targerr = mi_console_file_new (raw_stdout, "@", '"');
> +
>    mi->event_channel = mi_console_file_new (raw_stdout, "=", 0);

We have a lot of people that are confused about the target-stream-output
and it was recently brought up that maybe this wasn't used and should be
removed.

   http://sources.redhat.com/ml/gdb/2005-11/msg00391.html

However, you seem to be using it, and want to add another stream-output,
probably called something like target-stream-stderr-output.

I haven't seen MI use the target-stream-output yet. What is your
configuration that makes this happen? Is it still useful to you?
If so, it would be nice if we could improve the doco a bit, to let users
know when this would be useful for them.

Thanks,
Bob Rossi

Re: [PATCH] Target stderr not displayed thru MI

[Daniel Jacobowitz]

On Thu, Dec 01, 2005 at 09:04:36AM -0500, Bob Rossi wrote:
> > I attach a new patch in that sense. It does not include the documentation 
> > patch about MI new stream but it's just to give you an idea.
> > Any comment ?
> 
> > +  /* MI 1 and 2 target error stream use the same steam prefix "@" in oder
> > +     to ensure backward compatibility with old frondtend, MI 3 uses
> > +     a new prefix "#" */
> > +  if (mi_version (uiout) > 2)
> > +    mi->targerr = mi_console_file_new (raw_stdout, "#", '"');
> > +  else
> > +    mi->targerr = mi_console_file_new (raw_stdout, "@", '"');
> > +
> >    mi->event_channel = mi_console_file_new (raw_stdout, "=", 0);
> 
> We have a lot of people that are confused about the target-stream-output
> and it was recently brought up that maybe this wasn't used and should be
> removed.
> 
>    http://sources.redhat.com/ml/gdb/2005-11/msg00391.html
> 
> However, you seem to be using it, and want to add another stream-output,
> probably called something like target-stream-stderr-output.
> 
> I haven't seen MI use the target-stream-output yet. What is your
> configuration that makes this happen? Is it still useful to you?
> If so, it would be nice if we could improve the doco a bit, to let users
> know when this would be useful for them.

It's used for any (most?) remote or simulator targets that can provide
output.  I'm sorry if I was not adequately clear about that.  The ST
folks appear to have their own proprietary remote target, which also
generates these packets.

Nick, what's your reasoning for separating this from the stdout data?
When we put the inferior onto its own TTY, we don't get stdout and
stderr separated, either.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] Target stderr not displayed thru MI

[Denis PILAT]

>>
>>I haven't seen MI use the target-stream-output yet. What is your
>>configuration that makes this happen? Is it still useful to you?
>>If so, it would be nice if we could improve the doco a bit, to let users
>>know when this would be useful for them.
>>
>
>It's used for any (most?) remote or simulator targets that can provide
>output.  I'm sorry if I was not adequately clear about that.  The ST
>folks appear to have their own proprietary remote target, which also
>generates these packets.
>
>
>

Daniel, Nick,

You're right, we have remote targets that are accessed thru a GDB that runs
under Eclipse. These targets outputs error that are not displayed anywhere,
my initial patch was about to fix that
(http://sources.redhat.com/ml/gdb-patches/2005-11/msg00497.html).

Adding a new stream to MI (target error stream) with a prefix different
from the others, will offer a possibility for future Eclipse (or any 
front end)
- 1st to get the stderr of the inferior.
- 2nd to distinguish it from target stdout.

I'm not sticked to have one more MI stream today since nobody
will use it efficiently, I just need to have at least target error
reported to a stream, even if it's the only mi stream available (mi->targ).

Hope I'm not adding more confusion.

Denis

Re: [PATCH] Target stderr not displayed thru MI

[Nick Roberts]

 > Nick, what's your reasoning for separating this from the stdout data?
 > When we put the inferior onto its own TTY, we don't get stdout and
 > stderr separated, either.

I was just trying to interpret what Denis said:

DP> The main problem with MI is that we can not distinguish target stdout 
DP> from target stderr.

I don't see any harm as, in the remote case, it presumably just prepends parts
of a single stream with a different character.  This would be easy to merge
back if it wasn't needed.

More recently, he has said that one stream is enough for his purposes:

DP> I'm not sticked to have one more MI stream today since nobody
DP> will use it efficiently, I just need to have at least target error
DP> reported to a stream, even if it's the only mi stream available (mi->targ).

Since no-one else appears to need another MI stream, or possibly even use MI
for remote debugging, if we accept that adding a stream later may present
compatibility problems, his original patch seems appropriate.

Nick

Re: [PATCH] Target stderr not displayed thru MI

[Jim Blandy]

Adding a new MI output stream with its own magic character will break
existing MI clients.  I don't think that's worthwhile, especially
given that, when a program is talking to a TTY, you can't distinguish
the two either.

If, at some point in the future, the MI protocol does get extended to
distinguish the two streams, the code will be ready for it, since we
already distinguish gdb_stdtarg and gdb_stdtargerr internally.  This
change doesn't make future improvements difficult.

Re: [PATCH] Target stderr not displayed thru MI

[Denis PILAT]

Nick Roberts wrote:

>DP> I'm not sticked to have one more MI stream today since nobody
>DP> will use it efficiently, I just need to have at least target error
>DP> reported to a stream, even if it's the only mi stream available (mi->targ).
>
>Since no-one else appears to need another MI stream, or possibly even use MI
>for remote debugging, if we accept that adding a stream later may present
>compatibility problems, his original patch seems appropriate.
>
>  
>
Nick,

Can I consider that my original patch
(http://sources.redhat.com/ml/gdb-patches/2005-11/msg00497.html) is 
accepted ?

Denis

Re: [PATCH] Target stderr not displayed thru MI

[Nick Roberts]

 > Nick,
 > 
 > Can I consider that my original patch
 > (http://sources.redhat.com/ml/gdb-patches/2005-11/msg00497.html) is 
 > accepted ?
 > 
 > Denis

I don't have the authority to approve it.  I only have "Write After Approval"
and an interest in MI.  According to the rules in the MAINTAINERS file, since
there is no longer a maintainer for MI, the responsibility falls to a global
maintainer such as Daniel Jacobowitz or Jim Blandy.

Nick

Re: [PATCH] Target stderr not displayed thru MI

[Daniel Jacobowitz]

On Mon, Dec 05, 2005 at 11:31:59PM +1300, Nick Roberts wrote:
>  > Nick,
>  > 
>  > Can I consider that my original patch
>  > (http://sources.redhat.com/ml/gdb-patches/2005-11/msg00497.html) is 
>  > accepted ?
>  > 
>  > Denis
> 
> I don't have the authority to approve it.  I only have "Write After Approval"
> and an interest in MI.  According to the rules in the MAINTAINERS file, since
> there is no longer a maintainer for MI, the responsibility falls to a global
> maintainer such as Daniel Jacobowitz or Jim Blandy.

The patch is OK; Jim already approved it conditional on feedback, Nick
was the only one to suggest a change, and we've agreed that the change
is unnecessary for the moment.

-- 
Daniel Jacobowitz
CodeSourcery, LLC