Re: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

Re: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Eli Zaretskii]

> From: "Terry Guo" <terry.guo@arm.com>
> Cc: <gdb-patches@sourceware.org>
> Date: Wed, 27 Jun 2012 09:34:22 +0800
> 
> Can you help to review this proposal? The GDB trunk also has this issue.

I will, when I have time.  You only posted this one day ago.

Re: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Eli Zaretskii]

> From: "Terry Guo" <terry.guo@arm.com>
> Cc: <eliz@gnu.org>,
> 	"Joey Ye" <Joey.Ye@arm.com>,
> 	"Matthew Gretton-Dann" <Matthew.Gretton-Dann@arm.com>,
> 	"'Pedro Alves'" <palves@redhat.com>,
> 	<daniel.jacobowitz@gmail.com>
> Date: Tue, 26 Jun 2012 09:13:14 +0800
> 
> I noticed a cross-built MINGW arm-none-eabi GDB will get hang on Windows
> when use pipe to get stderr and stdout from stub. The command used to start
> stub in GDB is "target extended-remote |
> stub-that-write-stderr-before-stdout". For my case, after send
> "$vFlashDone#ea" to stub, GDB get hang. The GDB source show that GDB will
> keep waiting for ACK message from stdout of stub, after send the packet.
> Unfortunately my stub will write some kind of log information into stderr
> and this action takes place before stub write ACK message to its stdout. So
> the only pipe is occupied by stderr which is waiting for GDB to consume,
> while GDB keep waiting for message from the stdout which hasn't pipe to use.
> We finally end up with a deadlock on pipe between GDB/stderr/stdout.
> 
> The following patch can avoid such deadlock by letting GDB also probe and
> consume stderr when waiting for stdout. Please review and comment.

I only read it superficially, but it looked fine to me.

Thanks.

RE: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Terry Guo]

Hi Eli,

> -----Original Message-----
> From: Eli Zaretskii [mailto:eliz@gnu.org]
> Sent: Thursday, June 28, 2012 12:54 AM
> To: Terry Guo
> Cc: gdb-patches@sourceware.org; Joey Ye; Matthew Gretton-Dann;
> palves@redhat.com; daniel.jacobowitz@gmail.com
> Subject: Re: [PATCH]Fix that GDB will get hang on Windows when using
> pipe to get stdout and stderr from stub
> 
> > From: "Terry Guo" <terry.guo@arm.com>
> > Cc: <eliz@gnu.org>,
> > 	"Joey Ye" <Joey.Ye@arm.com>,
> > 	"Matthew Gretton-Dann" <Matthew.Gretton-Dann@arm.com>,
> > 	"'Pedro Alves'" <palves@redhat.com>,
> > 	<daniel.jacobowitz@gmail.com>
> > Date: Tue, 26 Jun 2012 09:13:14 +0800
> >
> > I noticed a cross-built MINGW arm-none-eabi GDB will get hang on
> Windows
> > when use pipe to get stderr and stdout from stub. The command used to
> start
> > stub in GDB is "target extended-remote |
> > stub-that-write-stderr-before-stdout". For my case, after send
> > "$vFlashDone#ea" to stub, GDB get hang. The GDB source show that GDB
> will
> > keep waiting for ACK message from stdout of stub, after send the
> packet.
> > Unfortunately my stub will write some kind of log information into
> stderr
> > and this action takes place before stub write ACK message to its
> stdout. So
> > the only pipe is occupied by stderr which is waiting for GDB to
> consume,
> > while GDB keep waiting for message from the stdout which hasn't pipe
> to use.
> > We finally end up with a deadlock on pipe between GDB/stderr/stdout.
> >
> > The following patch can avoid such deadlock by letting GDB also probe
> and
> > consume stderr when waiting for stdout. Please review and comment.
> 
> I only read it superficially, but it looked fine to me.
> 
> Thanks.

Thanks for your review. Here I updated my patch to make it more robust. No
changes on its functions. Please help to review again. Thanks.

The original patch is at:
http://sourceware.org/ml/gdb-patches/2012-06/msg00790.html

BR,
Terry

2012-07-04  Terry Guo  <terry.guo@arm.com>

        * defs.h (GDB_MI_MSG_WIDTH): New.
        * ser_base (ser_base_read_error_fd): New function.
        (do_ser_base_readchar): Poll error file descriptor as well as
        standard output.
        (generic_readchar): Refactor error handling.

diff --git a/gdb/defs.h b/gdb/defs.h
index 9531c5a..303721b 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -1214,6 +1214,9 @@ extern int use_windows;
 #define ISATTY(FP)     (isatty (fileno (FP)))
 #endif

+/* A width that can achieve a better legibility for GDB MI mode.  */
+#define GDB_MI_MSG_WIDTH  80
+
 /* Ensure that V is aligned to an N byte boundary (B's assumed to be a
    power of 2).  Round up/down when necessary.  Examples of correct
    use include:
diff --git a/gdb/ser-base.c b/gdb/ser-base.c
index 368afa6..661c5a9 100644
--- a/gdb/ser-base.c
+++ b/gdb/ser-base.c
@@ -26,6 +26,7 @@

 #include "gdb_select.h"
 #include "gdb_string.h"
+#include "gdb_assert.h"
 #include <sys/time.h>
 #ifdef USE_WIN32API
 #include <winsock2.h>
@@ -223,6 +224,64 @@ ser_base_wait_for (struct serial *scb, int timeout)
     }
 }

+/* Read any error output we might have.  */
+
+void
+ser_base_read_error_fd (struct serial *scb, int close_fd)
+{
+  if (scb->error_fd != -1)
+    {
+      ssize_t s;
+      char buf[GDB_MI_MSG_WIDTH + 1];
+
+      for (;;)
+        {
+         char *current;
+         char *newline;
+         int to_read = GDB_MI_MSG_WIDTH;
+         int num_bytes = -1;
+
+         if (scb->ops->avail)
+           num_bytes = (scb->ops->avail)(scb, scb->error_fd);
+
+         if (num_bytes != -1)
+           to_read = (num_bytes < to_read) ? num_bytes : to_read;
+
+         if (to_read == 0)
+           break;
+
+         s = read (scb->error_fd, &buf, to_read);
+         if ((s == -1) || (s == 0 && !close_fd))
+           break;
+
+         if (s == 0 && close_fd)
+           {
+             /* End of file.  */
+             close (scb->error_fd);
+             scb->error_fd = -1;
+             break;
+           }
+
+         /* In theory, embedded newlines are not a problem.
+            But for MI, we want each output line to have just
+            one newline for legibility.  So output things
+            in newline chunks.  */
+         gdb_assert (s > 0 && s <= GDB_MI_MSG_WIDTH);
+         buf[s] = '\0';
+         current = buf;
+         while ((newline = strstr (current, "\n")) != NULL)
+           {
+             *newline = '\0';
+             fputs_unfiltered (current, gdb_stderr);
+             fputs_unfiltered ("\n", gdb_stderr);
+             current = newline + 1;
+           }
+
+         fputs_unfiltered (current, gdb_stderr);
+       }
+    }
+}
+
 /* Read a character with user-specified timeout.  TIMEOUT is number of
seconds
    to wait, or -1 to wait forever.  Use timeout of 0 to effect a poll.
Returns
    char if successful.  Returns -2 if timeout expired, EOF if line dropped
@@ -273,6 +332,11 @@ do_ser_base_readchar (struct serial *scb, int timeout)
          status = SERIAL_TIMEOUT;
          break;
        }
+
+      /* We also need to check and consume the stderr because it could
+         come before the stdout for some stubs.  If we just sit and wait
+         for stdout, we would hit a deadlock for that case.  */
+      ser_base_read_error_fd (scb, 0);
     }

   if (status < 0)
@@ -344,53 +408,7 @@ generic_readchar (struct serial *scb, int timeout,
        }
     }
   /* Read any error output we might have.  */
-  if (scb->error_fd != -1)
-    {
-      ssize_t s;
-      char buf[81];
-
-      for (;;)
-        {
-         char *current;
-         char *newline;
-         int to_read = 80;
-
-         int num_bytes = -1;
-         if (scb->ops->avail)
-           num_bytes = (scb->ops->avail)(scb, scb->error_fd);
-         if (num_bytes != -1)
-           to_read = (num_bytes < to_read) ? num_bytes : to_read;
-
-         if (to_read == 0)
-           break;
-
-         s = read (scb->error_fd, &buf, to_read);
-         if (s == -1)
-           break;
-         if (s == 0)
-           {
-             /* EOF */
-             close (scb->error_fd);
-             scb->error_fd = -1;
-             break;
-           }
-
-         /* In theory, embedded newlines are not a problem.
-            But for MI, we want each output line to have just
-            one newline for legibility.  So output things
-            in newline chunks.  */
-         buf[s] = '\0';
-         current = buf;
-         while ((newline = strstr (current, "\n")) != NULL)
-           {
-             *newline = '\0';
-             fputs_unfiltered (current, gdb_stderr);
-             fputs_unfiltered ("\n", gdb_stderr);
-             current = newline + 1;
-           }
-         fputs_unfiltered (current, gdb_stderr);
-       }
-    }
+  ser_base_read_error_fd (scb, 1);

   reschedule (scb);
   return ch;

RE: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Terry Guo]

Hi Eli,

Could you please help to review this updated patch when you have time?
Thanks.

BR,
Terry

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Terry Guo
> Sent: Wednesday, July 04, 2012 4:20 PM
> To: 'Eli Zaretskii'
> Cc: gdb-patches@sourceware.org; Joey Ye
> Subject: RE: [PATCH]Fix that GDB will get hang on Windows when using
> pipe to get stdout and stderr from stub
> 
> Hi Eli,
> 
> > -----Original Message-----
> > From: Eli Zaretskii [mailto:eliz@gnu.org]
> > Sent: Thursday, June 28, 2012 12:54 AM
> > To: Terry Guo
> > Cc: gdb-patches@sourceware.org; Joey Ye; Matthew Gretton-Dann;
> > palves@redhat.com; daniel.jacobowitz@gmail.com
> > Subject: Re: [PATCH]Fix that GDB will get hang on Windows when using
> > pipe to get stdout and stderr from stub
> >
> > > From: "Terry Guo" <terry.guo@arm.com>
> > > Cc: <eliz@gnu.org>,
> > > 	"Joey Ye" <Joey.Ye@arm.com>,
> > > 	"Matthew Gretton-Dann" <Matthew.Gretton-Dann@arm.com>,
> > > 	"'Pedro Alves'" <palves@redhat.com>,
> > > 	<daniel.jacobowitz@gmail.com>
> > > Date: Tue, 26 Jun 2012 09:13:14 +0800
> > >
> > > I noticed a cross-built MINGW arm-none-eabi GDB will get hang on
> > Windows
> > > when use pipe to get stderr and stdout from stub. The command used
> to
> > start
> > > stub in GDB is "target extended-remote |
> > > stub-that-write-stderr-before-stdout". For my case, after send
> > > "$vFlashDone#ea" to stub, GDB get hang. The GDB source show that
> GDB
> > will
> > > keep waiting for ACK message from stdout of stub, after send the
> > packet.
> > > Unfortunately my stub will write some kind of log information into
> > stderr
> > > and this action takes place before stub write ACK message to its
> > stdout. So
> > > the only pipe is occupied by stderr which is waiting for GDB to
> > consume,
> > > while GDB keep waiting for message from the stdout which hasn't
> pipe
> > to use.
> > > We finally end up with a deadlock on pipe between GDB/stderr/stdout.
> > >
> > > The following patch can avoid such deadlock by letting GDB also
> probe
> > and
> > > consume stderr when waiting for stdout. Please review and comment.
> >
> > I only read it superficially, but it looked fine to me.
> >
> > Thanks.
> 
> Thanks for your review. Here I updated my patch to make it more robust.
> No
> changes on its functions. Please help to review again. Thanks.
> 
> The original patch is at:
> http://sourceware.org/ml/gdb-patches/2012-06/msg00790.html
> 
> BR,
> Terry
> 
> 2012-07-04  Terry Guo  <terry.guo@arm.com>
> 
>         * defs.h (GDB_MI_MSG_WIDTH): New.
>         * ser_base (ser_base_read_error_fd): New function.
>         (do_ser_base_readchar): Poll error file descriptor as well as
>         standard output.
>         (generic_readchar): Refactor error handling.
> 
> diff --git a/gdb/defs.h b/gdb/defs.h
> index 9531c5a..303721b 100644
> --- a/gdb/defs.h
> +++ b/gdb/defs.h
> @@ -1214,6 +1214,9 @@ extern int use_windows;
>  #define ISATTY(FP)     (isatty (fileno (FP)))
>  #endif
> 
> +/* A width that can achieve a better legibility for GDB MI mode.  */
> +#define GDB_MI_MSG_WIDTH  80
> +
>  /* Ensure that V is aligned to an N byte boundary (B's assumed to be a
>     power of 2).  Round up/down when necessary.  Examples of correct
>     use include:
> diff --git a/gdb/ser-base.c b/gdb/ser-base.c
> index 368afa6..661c5a9 100644
> --- a/gdb/ser-base.c
> +++ b/gdb/ser-base.c
> @@ -26,6 +26,7 @@
> 
>  #include "gdb_select.h"
>  #include "gdb_string.h"
> +#include "gdb_assert.h"
>  #include <sys/time.h>
>  #ifdef USE_WIN32API
>  #include <winsock2.h>
> @@ -223,6 +224,64 @@ ser_base_wait_for (struct serial *scb, int timeout)
>      }
>  }
> 
> +/* Read any error output we might have.  */
> +
> +void
> +ser_base_read_error_fd (struct serial *scb, int close_fd)
> +{
> +  if (scb->error_fd != -1)
> +    {
> +      ssize_t s;
> +      char buf[GDB_MI_MSG_WIDTH + 1];
> +
> +      for (;;)
> +        {
> +         char *current;
> +         char *newline;
> +         int to_read = GDB_MI_MSG_WIDTH;
> +         int num_bytes = -1;
> +
> +         if (scb->ops->avail)
> +           num_bytes = (scb->ops->avail)(scb, scb->error_fd);
> +
> +         if (num_bytes != -1)
> +           to_read = (num_bytes < to_read) ? num_bytes : to_read;
> +
> +         if (to_read == 0)
> +           break;
> +
> +         s = read (scb->error_fd, &buf, to_read);
> +         if ((s == -1) || (s == 0 && !close_fd))
> +           break;
> +
> +         if (s == 0 && close_fd)
> +           {
> +             /* End of file.  */
> +             close (scb->error_fd);
> +             scb->error_fd = -1;
> +             break;
> +           }
> +
> +         /* In theory, embedded newlines are not a problem.
> +            But for MI, we want each output line to have just
> +            one newline for legibility.  So output things
> +            in newline chunks.  */
> +         gdb_assert (s > 0 && s <= GDB_MI_MSG_WIDTH);
> +         buf[s] = '\0';
> +         current = buf;
> +         while ((newline = strstr (current, "\n")) != NULL)
> +           {
> +             *newline = '\0';
> +             fputs_unfiltered (current, gdb_stderr);
> +             fputs_unfiltered ("\n", gdb_stderr);
> +             current = newline + 1;
> +           }
> +
> +         fputs_unfiltered (current, gdb_stderr);
> +       }
> +    }
> +}
> +
>  /* Read a character with user-specified timeout.  TIMEOUT is number of
> seconds
>     to wait, or -1 to wait forever.  Use timeout of 0 to effect a poll.
> Returns
>     char if successful.  Returns -2 if timeout expired, EOF if line
> dropped
> @@ -273,6 +332,11 @@ do_ser_base_readchar (struct serial *scb, int
> timeout)
>           status = SERIAL_TIMEOUT;
>           break;
>         }
> +
> +      /* We also need to check and consume the stderr because it could
> +         come before the stdout for some stubs.  If we just sit and
> wait
> +         for stdout, we would hit a deadlock for that case.  */
> +      ser_base_read_error_fd (scb, 0);
>      }
> 
>    if (status < 0)
> @@ -344,53 +408,7 @@ generic_readchar (struct serial *scb, int timeout,
>         }
>      }
>    /* Read any error output we might have.  */
> -  if (scb->error_fd != -1)
> -    {
> -      ssize_t s;
> -      char buf[81];
> -
> -      for (;;)
> -        {
> -         char *current;
> -         char *newline;
> -         int to_read = 80;
> -
> -         int num_bytes = -1;
> -         if (scb->ops->avail)
> -           num_bytes = (scb->ops->avail)(scb, scb->error_fd);
> -         if (num_bytes != -1)
> -           to_read = (num_bytes < to_read) ? num_bytes : to_read;
> -
> -         if (to_read == 0)
> -           break;
> -
> -         s = read (scb->error_fd, &buf, to_read);
> -         if (s == -1)
> -           break;
> -         if (s == 0)
> -           {
> -             /* EOF */
> -             close (scb->error_fd);
> -             scb->error_fd = -1;
> -             break;
> -           }
> -
> -         /* In theory, embedded newlines are not a problem.
> -            But for MI, we want each output line to have just
> -            one newline for legibility.  So output things
> -            in newline chunks.  */
> -         buf[s] = '\0';
> -         current = buf;
> -         while ((newline = strstr (current, "\n")) != NULL)
> -           {
> -             *newline = '\0';
> -             fputs_unfiltered (current, gdb_stderr);
> -             fputs_unfiltered ("\n", gdb_stderr);
> -             current = newline + 1;
> -           }
> -         fputs_unfiltered (current, gdb_stderr);
> -       }
> -    }
> +  ser_base_read_error_fd (scb, 1);
> 
>    reschedule (scb);
>    return ch;
> 
> 
> 

Re: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Eli Zaretskii]

> From: "Terry Guo" <terry.guo@arm.com>
> Cc: <gdb-patches@sourceware.org>,
> 	"Joey Ye" <Joey.Ye@arm.com>
> Date: Wed, 11 Jul 2012 13:00:46 +0800
> 
> Hi Eli,
> 
> Could you please help to review this updated patch when you have time?
> Thanks.

What exactly changed since the previous version?

RE: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Terry Guo]

> -----Original Message-----
> From: Eli Zaretskii [mailto:eliz@gnu.org]
> Sent: Thursday, July 12, 2012 2:36 AM
> To: Terry Guo
> Cc: gdb-patches@sourceware.org; Joey Ye
> Subject: Re: [PATCH]Fix that GDB will get hang on Windows when using
> pipe to get stdout and stderr from stub
> 
> > From: "Terry Guo" <terry.guo@arm.com>
> > Cc: <gdb-patches@sourceware.org>,
> > 	"Joey Ye" <Joey.Ye@arm.com>
> > Date: Wed, 11 Jul 2012 13:00:46 +0800
> >
> > Hi Eli,
> >
> > Could you please help to review this updated patch when you have time?
> > Thanks.
> 
> What exactly changed since the previous version?

Changes are about code secure and readability. No functionality change.

1). Previously I have code:

	char buf[81];
	int to_read = 80;

Now I change it to:

	#define GDB_MI_MSG_WIDTH  80

	char buf[GDB_MI_MSG_WIDTH + 1];
	int to_read = GDB_MI_MSG_WIDTH;

2). Another change is to add string length check. It is:

	s = read (scb->error_fd, &buf, to_read);

	if ((s == -1) || (s == 0 && !close_fd))
		break

	if (s == 0 && close_fd)
		{
			....
		}
	
	/* Current patch add this new line to check length  */
	gdb_assert (s > 0 && s <= GDB_MI_MSG_WIDTH);

	buf[s] = '\0';

The original patch is at 
http://sourceware.org/ml/gdb-patches/2012-06/msg00790.html

The updated patch is at
http://sourceware.org/ml/gdb-patches/2012-07/msg00053.html

BR,
Terry

Re: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Eli Zaretskii]

> From: "Terry Guo" <terry.guo@arm.com>
> Cc: <gdb-patches@sourceware.org>
> Date: Thu, 12 Jul 2012 09:41:51 +0800
> 
> Changes are about code secure and readability. No functionality change.
> 
> 1). Previously I have code:
> 
> 	char buf[81];
> 	int to_read = 80;
> 
> Now I change it to:
> 
> 	#define GDB_MI_MSG_WIDTH  80
> 
> 	char buf[GDB_MI_MSG_WIDTH + 1];
> 	int to_read = GDB_MI_MSG_WIDTH;
> 
> 2). Another change is to add string length check. It is:
> 
> 	s = read (scb->error_fd, &buf, to_read);
> 
> 	if ((s == -1) || (s == 0 && !close_fd))
> 		break
> 
> 	if (s == 0 && close_fd)
> 		{
> 			....
> 		}
> 	
> 	/* Current patch add this new line to check length  */
> 	gdb_assert (s > 0 && s <= GDB_MI_MSG_WIDTH);
> 
> 	buf[s] = '\0';

Thanks.  These are all no-brainers, so I think the patch is still good
to go in.

RE: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Terry Guo]

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

Hi Eli,

> 
> Thanks.  These are all no-brainers, so I think the patch is still good
> to go in.

Sorry to bother you again. I realized I haven't gdb write permission. Could
you please help me to commit it at your convenient time? The attachment is
the patch against the latest gdb trunk.

BR,
Terry

[-- Attachment #2: gdb-hang-trunk.patch --]
[-- Type: application/octet-stream, Size: 4514 bytes --]

diff --git a/gdb/defs.h b/gdb/defs.h
index 1c6fa79..bd556b0 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -1115,6 +1115,9 @@ extern int use_windows;
 #define ISATTY(FP)	(isatty (fileno (FP)))
 #endif
 
+/* A width that can achieve a better legibility for GDB MI mode.  */
+#define GDB_MI_MSG_WIDTH  80
+
 /* Ensure that V is aligned to an N byte boundary (B's assumed to be a
    power of 2).  Round up/down when necessary.  Examples of correct
    use include:
diff --git a/gdb/ser-base.c b/gdb/ser-base.c
index 2f12dfc..152b1aa 100644
--- a/gdb/ser-base.c
+++ b/gdb/ser-base.c
@@ -25,6 +25,7 @@
 
 #include "gdb_select.h"
 #include "gdb_string.h"
+#include "gdb_assert.h"
 #include <sys/time.h>
 #ifdef USE_WIN32API
 #include <winsock2.h>
@@ -242,6 +243,64 @@ ser_base_wait_for (struct serial *scb, int timeout)
     }
 }
 
+/* Read any error output we might have.  */
+
+static void
+ser_base_read_error_fd (struct serial *scb, int close_fd)
+{
+  if (scb->error_fd != -1)
+    {
+      ssize_t s;
+      char buf[GDB_MI_MSG_WIDTH + 1];
+
+      for (;;)
+        {
+         char *current;
+         char *newline;
+         int to_read = GDB_MI_MSG_WIDTH;
+         int num_bytes = -1;
+
+         if (scb->ops->avail)
+           num_bytes = (scb->ops->avail)(scb, scb->error_fd);
+
+         if (num_bytes != -1)
+           to_read = (num_bytes < to_read) ? num_bytes : to_read;
+
+         if (to_read == 0)
+           break;
+
+         s = read (scb->error_fd, &buf, to_read);
+         if ((s == -1) || (s == 0 && !close_fd))
+           break;
+
+         if (s == 0 && close_fd)
+           {
+             /* End of file.  */
+             close (scb->error_fd);
+             scb->error_fd = -1;
+             break;
+           }
+
+         /* In theory, embedded newlines are not a problem.
+            But for MI, we want each output line to have just
+            one newline for legibility.  So output things
+            in newline chunks.  */
+         gdb_assert (s > 0 && s <= GDB_MI_MSG_WIDTH);
+         buf[s] = '\0';
+         current = buf;
+         while ((newline = strstr (current, "\n")) != NULL)
+           {
+             *newline = '\0';
+             fputs_unfiltered (current, gdb_stderr);
+             fputs_unfiltered ("\n", gdb_stderr);
+             current = newline + 1;
+           }
+
+         fputs_unfiltered (current, gdb_stderr);
+       }
+    }
+}
+
 /* Read a character with user-specified timeout.  TIMEOUT is number of seconds
    to wait, or -1 to wait forever.  Use timeout of 0 to effect a poll.  Returns
    char if successful.  Returns -2 if timeout expired, EOF if line dropped
@@ -292,6 +351,11 @@ do_ser_base_readchar (struct serial *scb, int timeout)
 	  status = SERIAL_TIMEOUT;
 	  break;
 	}
+
+      /* We also need to check and consume the stderr because it could
+         come before the stdout for some stubs.  If we just sit and wait
+         for stdout, we would hit a deadlock for that case.  */
+      ser_base_read_error_fd (scb, 0);
     }
 
   if (status < 0)
@@ -363,53 +427,7 @@ generic_readchar (struct serial *scb, int timeout,
 	}
     }
   /* Read any error output we might have.  */
-  if (scb->error_fd != -1)
-    {
-      ssize_t s;
-      char buf[81];
-
-      for (;;)
-        {
- 	  char *current;
- 	  char *newline;
-	  int to_read = 80;
-
-	  int num_bytes = -1;
-	  if (scb->ops->avail)
-	    num_bytes = (scb->ops->avail)(scb, scb->error_fd);
-	  if (num_bytes != -1)
-	    to_read = (num_bytes < to_read) ? num_bytes : to_read;
-
-	  if (to_read == 0)
-	    break;
-
-	  s = read (scb->error_fd, &buf, to_read);
-	  if (s == -1)
-	    break;
-	  if (s == 0)
-	    {
-	      /* EOF */
-	      close (scb->error_fd);
-	      scb->error_fd = -1;
-	      break;
-	    }
-
-	  /* In theory, embedded newlines are not a problem.
-	     But for MI, we want each output line to have just
-	     one newline for legibility.  So output things
-	     in newline chunks.  */
-	  buf[s] = '\0';
-	  current = buf;
-	  while ((newline = strstr (current, "\n")) != NULL)
-	    {
-	      *newline = '\0';
-	      fputs_unfiltered (current, gdb_stderr);
-	      fputs_unfiltered ("\n", gdb_stderr);
-	      current = newline + 1;
-	    }
-	  fputs_unfiltered (current, gdb_stderr);
-	}
-    }
+  ser_base_read_error_fd (scb, 1);
 
   reschedule (scb);
   return ch;

RE: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Terry Guo]

Here are the ChangeLog:

2012-07-16  Terry Guo  <terry.guo@arm.com>

        * defs.h (GDB_MI_MSG_WIDTH): New.
        * ser_base (ser_base_read_error_fd): New function.
        (do_ser_base_readchar): Poll error file descriptor as well as
        standard output.
        (generic_readchar): Refactor error handling.

BR,
Terry

> -----Original Message-----
> From: Terry Guo [mailto:terry.guo@arm.com]
> Sent: Monday, July 16, 2012 1:52 PM
> To: Eli Zaretskii
> Cc: gdb-patches@sourceware.org
> Subject: RE: [PATCH]Fix that GDB will get hang on Windows when using
> pipe to get stdout and stderr from stub
> 
> Hi Eli,
> 
> >
> > Thanks.  These are all no-brainers, so I think the patch is still
> good
> > to go in.
> 
> Sorry to bother you again. I realized I haven't gdb write permission.
> Could you please help me to commit it at your convenient time? The
> attachment is the patch against the latest gdb trunk.
> 
> BR,
> Terry

Re: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Yao Qi]

On Monday, July 16, 2012 01:52:23 PM Terry Guo wrote:
> Sorry to bother you again. I realized I haven't gdb write permission. Could
> you please help me to commit it at your convenient time? The attachment is
> the patch against the latest gdb trunk.

Terry,

As your patch can't go in as an obvious change, you need a FSF copyright 
assignment.  Please contact to Tom or Joel to get the form, and fill in it.  
Usually, it takes weeks or one month, I think.  Once your assignment paper is 
ready, you or other people can check in your patch to CVS trunk.

In parallel, you can apply for a sourceware account for writing access.

 http://sourceware.org/cgi-bin/pdw/ps_form.cgi

You need a person (global maintainer, I think) to approve your request.

-- 
Yao (齐尧)

RE: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Terry Guo]

Hi Yao,

Thanks for reminding. I signed a FSF copyright assignment when I started to work for GCC. But I can't remember its content. Do you know whether it cover the GDB and Binutils by default?

BR,
Terry

> -----Original Message-----
> From: Yao Qi [mailto:yao@codesourcery.com]
> Sent: Monday, July 16, 2012 8:38 PM
> To: gdb-patches@sourceware.org
> Cc: Terry Guo; Eli Zaretskii
> Subject: Re: [PATCH]Fix that GDB will get hang on Windows when using
> pipe to get stdout and stderr from stub
> 
> On Monday, July 16, 2012 01:52:23 PM Terry Guo wrote:
> > Sorry to bother you again. I realized I haven't gdb write permission.
> Could
> > you please help me to commit it at your convenient time? The
> attachment is
> > the patch against the latest gdb trunk.
> 
> Terry,
> 
> As your patch can't go in as an obvious change, you need a FSF
> copyright
> assignment.  Please contact to Tom or Joel to get the form, and fill in
> it.
> Usually, it takes weeks or one month, I think.  Once your assignment
> paper is
> ready, you or other people can check in your patch to CVS trunk.
> 
> In parallel, you can apply for a sourceware account for writing access.
> 
>  http://sourceware.org/cgi-bin/pdw/ps_form.cgi
> 
> You need a person (global maintainer, I think) to approve your request.
> 
> --
> Yao (齐尧)

Re: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Yao Qi]

On Monday, July 16, 2012 02:56:27 PM Terry Guo wrote:
> Thanks for reminding. I signed a FSF copyright assignment when I started to
> work for GCC. But I can't remember its content. Do you know whether it
> cover the GDB and Binutils by default?

I checked /gd/gnuorg/copyright.list again, and find that ARM has the corporate 
assignment, if I read that correctly,

GDB     ARM Limited     2001-08-17
Assigns past and future changes.
(J. Brooks)

So the copyright assignment is not an issue to you any more :)  Sorry for the 
confusion.

-- 
Yao (齐尧)

RE: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Terry Guo]

> -----Original Message-----
> From: Yao Qi [mailto:yao@codesourcery.com]
> Sent: Monday, July 16, 2012 9:19 PM
> To: gdb-patches@sourceware.org
> Cc: Terry Guo; 'Eli Zaretskii'
> Subject: Re: [PATCH]Fix that GDB will get hang on Windows when using
> pipe to get stdout and stderr from stub
> 
> On Monday, July 16, 2012 02:56:27 PM Terry Guo wrote:
> > Thanks for reminding. I signed a FSF copyright assignment when I
> started to
> > work for GCC. But I can't remember its content. Do you know whether
> it
> > cover the GDB and Binutils by default?
> 
> I checked /gd/gnuorg/copyright.list again, and find that ARM has the
> corporate
> assignment, if I read that correctly,
> 
> GDB     ARM Limited     2001-08-17
> Assigns past and future changes.
> (J. Brooks)
> 
> So the copyright assignment is not an issue to you any more :)  Sorry
> for the
> confusion.
> 

I checked and think you read correctly. The ARM assignment does cover me. Never mind the "confusion", at least I learned how to find copyright.list :).

BR,
Terry

Re: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Sergio Durigan Junior]

On Monday, July 16 2012, Terry Guo wrote:

>> 
>> Thanks.  These are all no-brainers, so I think the patch is still good
>> to go in.
>
> Sorry to bother you again. I realized I haven't gdb write permission. Could
> you please help me to commit it at your convenient time? The attachment is
> the patch against the latest gdb trunk.

Hello Terry,

I only read the thread now, so sorry for coming with comments after
Eli's approval.  Anyway, it's just a small nit.

> diff --git a/gdb/ser-base.c b/gdb/ser-base.c
> index 2f12dfc..152b1aa 100644
> --- a/gdb/ser-base.c
> +++ b/gdb/ser-base.c
> @@ -25,6 +25,7 @@
>  
>  #include "gdb_select.h"
>  #include "gdb_string.h"
> +#include "gdb_assert.h"
>  #include <sys/time.h>
>  #ifdef USE_WIN32API
>  #include <winsock2.h>
> @@ -242,6 +243,64 @@ ser_base_wait_for (struct serial *scb, int timeout)
>      }
>  }
>  
> +/* Read any error output we might have.  */
> +
> +static void
> +ser_base_read_error_fd (struct serial *scb, int close_fd)
> +{
> +  if (scb->error_fd != -1)
> +    {
> +      ssize_t s;
> +      char buf[GDB_MI_MSG_WIDTH + 1];
> +
> +      for (;;)
> +        {
> +         char *current;

I notice you are not following the convention of converting 8 spaces to
a TAB character here and everywhere else.  Can you please fix it?  After
that, I can commit the patch for (if you don't have the permission yet).

-- 
Sergio

RE: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Terry Guo]

> > +/* Read any error output we might have.  */
> > +
> > +static void
> > +ser_base_read_error_fd (struct serial *scb, int close_fd)
> > +{
> > +  if (scb->error_fd != -1)
> > +    {
> > +      ssize_t s;
> > +      char buf[GDB_MI_MSG_WIDTH + 1];
> > +
> > +      for (;;)
> > +        {
> > +         char *current;
> 
> I notice you are not following the convention of converting 8 spaces to
> a TAB character here and everywhere else.  Can you please fix it?
> After
> that, I can commit the patch for (if you don't have the permission yet).
> 

Hi Sergio,

Thanks very much. I will fix them and re-send the patch for review.

BR,
Terry

RE: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Terry Guo]

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



> 
> > diff --git a/gdb/ser-base.c b/gdb/ser-base.c
> > index 2f12dfc..152b1aa 100644
> > --- a/gdb/ser-base.c
> > +++ b/gdb/ser-base.c
> > @@ -25,6 +25,7 @@
> >
> >  #include "gdb_select.h"
> >  #include "gdb_string.h"
> > +#include "gdb_assert.h"
> >  #include <sys/time.h>
> >  #ifdef USE_WIN32API
> >  #include <winsock2.h>
> > @@ -242,6 +243,64 @@ ser_base_wait_for (struct serial *scb, int
> timeout)
> >      }
> >  }
> >
> > +/* Read any error output we might have.  */
> > +
> > +static void
> > +ser_base_read_error_fd (struct serial *scb, int close_fd)
> > +{
> > +  if (scb->error_fd != -1)
> > +    {
> > +      ssize_t s;
> > +      char buf[GDB_MI_MSG_WIDTH + 1];
> > +
> > +      for (;;)
> > +        {
> > +         char *current;
> 
> I notice you are not following the convention of converting 8 spaces to
> a TAB character here and everywhere else.  Can you please fix it?
> After
> that, I can commit the patch for (if you don't have the permission yet).
> 

Hello Sergio,

Can you help to review the attached patch which should convert 8 spaces to a
TAB? Thanks.
BTW. I think I have write permission now.

BR,
Terry

[-- Attachment #2: gdb-hang-v2.patch --]
[-- Type: application/octet-stream, Size: 4356 bytes --]

diff --git a/gdb/defs.h b/gdb/defs.h
index 1c6fa79..bd556b0 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -1115,6 +1115,9 @@ extern int use_windows;
 #define ISATTY(FP)	(isatty (fileno (FP)))
 #endif
 
+/* A width that can achieve a better legibility for GDB MI mode.  */
+#define GDB_MI_MSG_WIDTH  80
+
 /* Ensure that V is aligned to an N byte boundary (B's assumed to be a
    power of 2).  Round up/down when necessary.  Examples of correct
    use include:
diff --git a/gdb/ser-base.c b/gdb/ser-base.c
index 2f12dfc..287e55d 100644
--- a/gdb/ser-base.c
+++ b/gdb/ser-base.c
@@ -25,6 +25,7 @@
 
 #include "gdb_select.h"
 #include "gdb_string.h"
+#include "gdb_assert.h"
 #include <sys/time.h>
 #ifdef USE_WIN32API
 #include <winsock2.h>
@@ -242,6 +243,64 @@ ser_base_wait_for (struct serial *scb, int timeout)
     }
 }
 
+/* Read any error output we might have.  */
+
+static void
+ser_base_read_error_fd (struct serial *scb, int close_fd)
+{
+  if (scb->error_fd != -1)
+    {
+      ssize_t s;
+      char buf[GDB_MI_MSG_WIDTH + 1];
+
+      for (;;)
+	{
+	  char *current;
+	  char *newline;
+	  int to_read = GDB_MI_MSG_WIDTH;
+	  int num_bytes = -1;
+
+	  if (scb->ops->avail)
+	    num_bytes = (scb->ops->avail)(scb, scb->error_fd);
+
+	  if (num_bytes != -1)
+	    to_read = (num_bytes < to_read) ? num_bytes : to_read;
+
+	  if (to_read == 0)
+	    break;
+
+	  s = read (scb->error_fd, &buf, to_read);
+	  if ((s == -1) || (s == 0 && !close_fd))
+	    break;
+
+	  if (s == 0 && close_fd)
+	    {
+	      /* End of file.  */
+	      close (scb->error_fd);
+	      scb->error_fd = -1;
+	      break;
+	    }
+
+	  /* In theory, embedded newlines are not a problem.
+	     But for MI, we want each output line to have just
+	     one newline for legibility.  So output things
+	     in newline chunks.  */
+	  gdb_assert (s > 0 && s <= GDB_MI_MSG_WIDTH);
+	  buf[s] = '\0';
+	  current = buf;
+	  while ((newline = strstr (current, "\n")) != NULL)
+	    {
+	      *newline = '\0';
+	      fputs_unfiltered (current, gdb_stderr);
+	      fputs_unfiltered ("\n", gdb_stderr);
+	      current = newline + 1;
+	    }
+
+	  fputs_unfiltered (current, gdb_stderr);
+       }
+    }
+}
+
 /* Read a character with user-specified timeout.  TIMEOUT is number of seconds
    to wait, or -1 to wait forever.  Use timeout of 0 to effect a poll.  Returns
    char if successful.  Returns -2 if timeout expired, EOF if line dropped
@@ -292,6 +351,11 @@ do_ser_base_readchar (struct serial *scb, int timeout)
 	  status = SERIAL_TIMEOUT;
 	  break;
 	}
+
+      /* We also need to check and consume the stderr because it could
+	 come before the stdout for some stubs.  If we just sit and wait
+	 for stdout, we would hit a deadlock for that case.  */
+      ser_base_read_error_fd (scb, 0);
     }
 
   if (status < 0)
@@ -362,54 +426,9 @@ generic_readchar (struct serial *scb, int timeout,
 	    }
 	}
     }
-  /* Read any error output we might have.  */
-  if (scb->error_fd != -1)
-    {
-      ssize_t s;
-      char buf[81];
-
-      for (;;)
-        {
- 	  char *current;
- 	  char *newline;
-	  int to_read = 80;
-
-	  int num_bytes = -1;
-	  if (scb->ops->avail)
-	    num_bytes = (scb->ops->avail)(scb, scb->error_fd);
-	  if (num_bytes != -1)
-	    to_read = (num_bytes < to_read) ? num_bytes : to_read;
-
-	  if (to_read == 0)
-	    break;
-
-	  s = read (scb->error_fd, &buf, to_read);
-	  if (s == -1)
-	    break;
-	  if (s == 0)
-	    {
-	      /* EOF */
-	      close (scb->error_fd);
-	      scb->error_fd = -1;
-	      break;
-	    }
 
-	  /* In theory, embedded newlines are not a problem.
-	     But for MI, we want each output line to have just
-	     one newline for legibility.  So output things
-	     in newline chunks.  */
-	  buf[s] = '\0';
-	  current = buf;
-	  while ((newline = strstr (current, "\n")) != NULL)
-	    {
-	      *newline = '\0';
-	      fputs_unfiltered (current, gdb_stderr);
-	      fputs_unfiltered ("\n", gdb_stderr);
-	      current = newline + 1;
-	    }
-	  fputs_unfiltered (current, gdb_stderr);
-	}
-    }
+  /* Read any error output we might have.  */
+  ser_base_read_error_fd (scb, 1);
 
   reschedule (scb);
   return ch;
-- 
1.7.9.5

Re: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Sergio Durigan Junior]

On Tuesday, July 17 2012, Terry Guo wrote:

> Can you help to review the attached patch which should convert 8 spaces to a
> TAB? Thanks.
> BTW. I think I have write permission now.

Yes, the patch is OK regarding this issue.  I guess the patch was
already approved, right?  In this case, you can commit it without
problems.

-- 
Sergio

RE: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Terry Guo]

Hi Eli,

Can you help to review this proposal? The GDB trunk also has this issue.
Thanks.

BR,
Terry

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Terry Guo
> Sent: Tuesday, June 26, 2012 9:13 AM
> To: gdb-patches@sourceware.org
> Cc: eliz@gnu.org; Joey Ye; Matthew Gretton-Dann; 'Pedro Alves';
> daniel.jacobowitz@gmail.com
> Subject: [PATCH]Fix that GDB will get hang on Windows when using pipe
> to get stdout and stderr from stub
> 
> Hi,
> 
> I noticed a cross-built MINGW arm-none-eabi GDB will get hang on
> Windows
> when use pipe to get stderr and stdout from stub. The command used to
> start
> stub in GDB is "target extended-remote |
> stub-that-write-stderr-before-stdout". For my case, after send
> "$vFlashDone#ea" to stub, GDB get hang. The GDB source show that GDB
> will
> keep waiting for ACK message from stdout of stub, after send the packet.
> Unfortunately my stub will write some kind of log information into
> stderr
> and this action takes place before stub write ACK message to its stdout.
> So
> the only pipe is occupied by stderr which is waiting for GDB to consume,
> while GDB keep waiting for message from the stdout which hasn't pipe to
> use.
> We finally end up with a deadlock on pipe between GDB/stderr/stdout.
> 
> The following patch can avoid such deadlock by letting GDB also probe
> and
> consume stderr when waiting for stdout. Please review and comment.
> 
> The Linux version GDB hasn't such issue. I think it's because we use
> different way to handle PIPE as stated in functions pipe_open and
> pipe_windows_open. For Linux we have two socketpair kind pipes, one for
> stdout and one for stderr. While for windows, we only have one pipe
> which is
> created by _pipe function.
> 
> BR,
> Terry
> 
> 2012-06-25  Terry Guo  <terry.guo@arm.com>
> 
> 	* ser_base (ser_base_read_error_fd): New function.
> 	(do_ser_base_readchar): Poll error file descriptor as well as
> 	standard output.
> 	(generic_readchar): Refactor error handling.
> 
> diff --git a/gdb/ser-base.c b/gdb/ser-base.c
> index 368afa6..ee6db54 100644
> --- a/gdb/ser-base.c
> +++ b/gdb/ser-base.c
> @@ -223,6 +223,63 @@ ser_base_wait_for (struct serial *scb, int timeout)
>      }
>  }
> 
> +/* Read any error output we might have.  */
> +
> +void
> +ser_base_read_error_fd (struct serial *scb, int close_fd)
> +{
> +  if (scb->error_fd != -1)
> +    {
> +      ssize_t s;
> +      char buf[81];
> +
> +      for (;;)
> +        {
> + 	  char *current;
> + 	  char *newline;
> +	  int to_read = 80;
> +	  int num_bytes = -1;
> +
> +	  if (scb->ops->avail)
> +	    num_bytes = (scb->ops->avail)(scb, scb->error_fd);
> +
> +	  if (num_bytes != -1)
> +	    to_read = (num_bytes < to_read) ? num_bytes : to_read;
> +
> +	  if (to_read == 0)
> +	    break;
> +
> +	  s = read (scb->error_fd, &buf, to_read);
> +	  if ((s == -1) || (s == 0 && !close_fd))
> +	    break;
> +
> +	  if (s == 0 && close_fd)
> +	    {
> +	      /* End of file.  */
> +	      close (scb->error_fd);
> +	      scb->error_fd = -1;
> +	      break;
> +	    }
> +
> +	  /* In theory, embedded newlines are not a problem.
> +	     But for MI, we want each output line to have just
> +	     one newline for legibility.  So output things
> +	     in newline chunks.  */
> +	  buf[s] = '\0';
> +	  current = buf;
> +	  while ((newline = strstr (current, "\n")) != NULL)
> +	    {
> +	      *newline = '\0';
> +	      fputs_unfiltered (current, gdb_stderr);
> +	      fputs_unfiltered ("\n", gdb_stderr);
> +	      current = newline + 1;
> +	    }
> +
> +	  fputs_unfiltered (current, gdb_stderr);
> +	}
> +    }
> +}
> +
>  /* Read a character with user-specified timeout.  TIMEOUT is number of
> seconds
>     to wait, or -1 to wait forever.  Use timeout of 0 to effect a poll.
> Returns
>     char if successful.  Returns -2 if timeout expired, EOF if line
> dropped
> @@ -273,6 +330,11 @@ do_ser_base_readchar (struct serial *scb, int
> timeout)
>  	  status = SERIAL_TIMEOUT;
>  	  break;
>  	}
> +
> +      /* We also need to check and consume the stderr because it could
> +         come before the stdout for some stubs.  If we just sit and
> wait
> +         for stdout, we would hit a deadlock for that case.  */
> +      ser_base_read_error_fd (scb, 0);
>      }
> 
>    if (status < 0)
> @@ -344,53 +406,7 @@ generic_readchar (struct serial *scb, int timeout,
>  	}
>      }
>    /* Read any error output we might have.  */
> -  if (scb->error_fd != -1)
> -    {
> -      ssize_t s;
> -      char buf[81];
> -
> -      for (;;)
> -        {
> - 	  char *current;
> - 	  char *newline;
> -	  int to_read = 80;
> -
> -	  int num_bytes = -1;
> -	  if (scb->ops->avail)
> -	    num_bytes = (scb->ops->avail)(scb, scb->error_fd);
> -	  if (num_bytes != -1)
> -	    to_read = (num_bytes < to_read) ? num_bytes : to_read;
> -
> -	  if (to_read == 0)
> -	    break;
> -
> -	  s = read (scb->error_fd, &buf, to_read);
> -	  if (s == -1)
> -	    break;
> -	  if (s == 0)
> -	    {
> -	      /* EOF */
> -	      close (scb->error_fd);
> -	      scb->error_fd = -1;
> -	      break;
> -	    }
> -
> -	  /* In theory, embedded newlines are not a problem.
> -	     But for MI, we want each output line to have just
> -	     one newline for legibility.  So output things
> -	     in newline chunks.  */
> -	  buf[s] = '\0';
> -	  current = buf;
> -	  while ((newline = strstr (current, "\n")) != NULL)
> -	    {
> -	      *newline = '\0';
> -	      fputs_unfiltered (current, gdb_stderr);
> -	      fputs_unfiltered ("\n", gdb_stderr);
> -	      current = newline + 1;
> -	    }
> -	  fputs_unfiltered (current, gdb_stderr);
> -	}
> -    }
> +  ser_base_read_error_fd (scb, 1);
> 
>    reschedule (scb);
>    return ch;
> 
> 

[PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub

[Terry Guo]

Hi,

I noticed a cross-built MINGW arm-none-eabi GDB will get hang on Windows
when use pipe to get stderr and stdout from stub. The command used to start
stub in GDB is "target extended-remote |
stub-that-write-stderr-before-stdout". For my case, after send
"$vFlashDone#ea" to stub, GDB get hang. The GDB source show that GDB will
keep waiting for ACK message from stdout of stub, after send the packet.
Unfortunately my stub will write some kind of log information into stderr
and this action takes place before stub write ACK message to its stdout. So
the only pipe is occupied by stderr which is waiting for GDB to consume,
while GDB keep waiting for message from the stdout which hasn't pipe to use.
We finally end up with a deadlock on pipe between GDB/stderr/stdout.

The following patch can avoid such deadlock by letting GDB also probe and
consume stderr when waiting for stdout. Please review and comment.

The Linux version GDB hasn't such issue. I think it's because we use
different way to handle PIPE as stated in functions pipe_open and
pipe_windows_open. For Linux we have two socketpair kind pipes, one for
stdout and one for stderr. While for windows, we only have one pipe which is
created by _pipe function.

BR,
Terry

2012-06-25  Terry Guo  <terry.guo@arm.com>

	* ser_base (ser_base_read_error_fd): New function.
	(do_ser_base_readchar): Poll error file descriptor as well as
	standard output.
	(generic_readchar): Refactor error handling.

diff --git a/gdb/ser-base.c b/gdb/ser-base.c
index 368afa6..ee6db54 100644
--- a/gdb/ser-base.c
+++ b/gdb/ser-base.c
@@ -223,6 +223,63 @@ ser_base_wait_for (struct serial *scb, int timeout)
     }
 }
 
+/* Read any error output we might have.  */
+
+void
+ser_base_read_error_fd (struct serial *scb, int close_fd)
+{
+  if (scb->error_fd != -1)
+    {
+      ssize_t s;
+      char buf[81];
+
+      for (;;)
+        {
+ 	  char *current;
+ 	  char *newline;
+	  int to_read = 80;
+	  int num_bytes = -1;
+
+	  if (scb->ops->avail)
+	    num_bytes = (scb->ops->avail)(scb, scb->error_fd);
+
+	  if (num_bytes != -1)
+	    to_read = (num_bytes < to_read) ? num_bytes : to_read;
+
+	  if (to_read == 0)
+	    break;
+
+	  s = read (scb->error_fd, &buf, to_read);
+	  if ((s == -1) || (s == 0 && !close_fd))
+	    break;
+
+	  if (s == 0 && close_fd)
+	    {
+	      /* End of file.  */
+	      close (scb->error_fd);
+	      scb->error_fd = -1;
+	      break;
+	    }
+
+	  /* In theory, embedded newlines are not a problem.
+	     But for MI, we want each output line to have just
+	     one newline for legibility.  So output things
+	     in newline chunks.  */
+	  buf[s] = '\0';
+	  current = buf;
+	  while ((newline = strstr (current, "\n")) != NULL)
+	    {
+	      *newline = '\0';
+	      fputs_unfiltered (current, gdb_stderr);
+	      fputs_unfiltered ("\n", gdb_stderr);
+	      current = newline + 1;
+	    }
+
+	  fputs_unfiltered (current, gdb_stderr);
+	}
+    }
+}
+
 /* Read a character with user-specified timeout.  TIMEOUT is number of
seconds
    to wait, or -1 to wait forever.  Use timeout of 0 to effect a poll.
Returns
    char if successful.  Returns -2 if timeout expired, EOF if line dropped
@@ -273,6 +330,11 @@ do_ser_base_readchar (struct serial *scb, int timeout)
 	  status = SERIAL_TIMEOUT;
 	  break;
 	}
+
+      /* We also need to check and consume the stderr because it could 
+         come before the stdout for some stubs.  If we just sit and wait
+         for stdout, we would hit a deadlock for that case.  */
+      ser_base_read_error_fd (scb, 0);
     }
 
   if (status < 0)
@@ -344,53 +406,7 @@ generic_readchar (struct serial *scb, int timeout,
 	}
     }
   /* Read any error output we might have.  */
-  if (scb->error_fd != -1)
-    {
-      ssize_t s;
-      char buf[81];
-
-      for (;;)
-        {
- 	  char *current;
- 	  char *newline;
-	  int to_read = 80;
-
-	  int num_bytes = -1;
-	  if (scb->ops->avail)
-	    num_bytes = (scb->ops->avail)(scb, scb->error_fd);
-	  if (num_bytes != -1)
-	    to_read = (num_bytes < to_read) ? num_bytes : to_read;
-
-	  if (to_read == 0)
-	    break;
-
-	  s = read (scb->error_fd, &buf, to_read);
-	  if (s == -1)
-	    break;
-	  if (s == 0)
-	    {
-	      /* EOF */
-	      close (scb->error_fd);
-	      scb->error_fd = -1;
-	      break;
-	    }
-
-	  /* In theory, embedded newlines are not a problem.
-	     But for MI, we want each output line to have just
-	     one newline for legibility.  So output things
-	     in newline chunks.  */
-	  buf[s] = '\0';
-	  current = buf;
-	  while ((newline = strstr (current, "\n")) != NULL)
-	    {
-	      *newline = '\0';
-	      fputs_unfiltered (current, gdb_stderr);
-	      fputs_unfiltered ("\n", gdb_stderr);
-	      current = newline + 1;
-	    }
-	  fputs_unfiltered (current, gdb_stderr);
-	}
-    }
+  ser_base_read_error_fd (scb, 1);
 
   reschedule (scb);
   return ch;