* Re: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub
[not found] <000001cd5338$ded61b20$9c825160$%guo@arm.com>
[not found] ` <000501cd5404$fbf210c0$f3d63240$%guo@arm.com>
@ 2012-06-27 16:54 ` Eli Zaretskii
2012-07-04 8:19 ` Terry Guo
[not found] ` <000301cd59bd$ce1c8900$6a559b00$%guo@arm.com>
1 sibling, 2 replies; 19+ messages in thread
From: Eli Zaretskii @ 2012-06-27 16:54 UTC (permalink / raw)
To: Terry Guo
Cc: gdb-patches, Joey.Ye, Matthew.Gretton-Dann, palves, daniel.jacobowitz
> 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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub
2012-06-27 16:54 ` Eli Zaretskii
@ 2012-07-04 8:19 ` Terry Guo
2012-07-11 5:00 ` Terry Guo
[not found] ` <000301cd59bd$ce1c8900$6a559b00$%guo@arm.com>
1 sibling, 1 reply; 19+ messages in thread
From: Terry Guo @ 2012-07-04 8:19 UTC (permalink / raw)
To: 'Eli Zaretskii'; +Cc: gdb-patches, Joey Ye
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;
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub
2012-07-04 8:19 ` Terry Guo
@ 2012-07-11 5:00 ` Terry Guo
0 siblings, 0 replies; 19+ messages in thread
From: Terry Guo @ 2012-07-11 5:00 UTC (permalink / raw)
To: 'Eli Zaretskii'; +Cc: gdb-patches, Joey Ye
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;
>
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <000301cd59bd$ce1c8900$6a559b00$%guo@arm.com>]
* [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub
@ 2012-06-26 1:12 Terry Guo
2012-06-27 1:33 ` Terry Guo
0 siblings, 1 reply; 19+ messages in thread
From: Terry Guo @ 2012-06-26 1:12 UTC (permalink / raw)
To: gdb-patches
Cc: eliz, Joey Ye, Matthew Gretton-Dann, 'Pedro Alves',
daniel.jacobowitz
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;
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub
2012-06-26 1:12 Terry Guo
@ 2012-06-27 1:33 ` Terry Guo
0 siblings, 0 replies; 19+ messages in thread
From: Terry Guo @ 2012-06-27 1:33 UTC (permalink / raw)
To: eliz; +Cc: gdb-patches
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;
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2012-07-17 17:21 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <000001cd5338$ded61b20$9c825160$%guo@arm.com>
[not found] ` <000501cd5404$fbf210c0$f3d63240$%guo@arm.com>
2012-06-27 3:06 ` [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub Eli Zaretskii
2012-06-27 16:54 ` Eli Zaretskii
2012-07-04 8:19 ` Terry Guo
2012-07-11 5:00 ` Terry Guo
[not found] ` <000301cd59bd$ce1c8900$6a559b00$%guo@arm.com>
[not found] ` <000101cd5f22$2371cdc0$6a556940$%guo@arm.com>
2012-07-11 18:36 ` Eli Zaretskii
2012-07-12 1:41 ` Terry Guo
[not found] ` <000301cd5fcf$838d31b0$8aa79510$%guo@arm.com>
2012-07-12 5:20 ` Eli Zaretskii
2012-07-16 5:51 ` Terry Guo
2012-07-17 1:28 ` Sergio Durigan Junior
2012-07-17 3:05 ` Terry Guo
2012-07-17 7:51 ` Terry Guo
2012-07-17 17:21 ` Sergio Durigan Junior
2012-07-16 6:08 ` Terry Guo
2012-07-16 6:38 ` Yao Qi
2012-07-16 6:55 ` Terry Guo
2012-07-16 7:19 ` Yao Qi
2012-07-16 8:10 ` Terry Guo
2012-06-26 1:12 Terry Guo
2012-06-27 1:33 ` Terry Guo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).