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

Terry Guo terry.guo@arm.com
Wed Jul 4 08:19:00 GMT 2012


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;





More information about the Gdb-patches mailing list