From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4231 invoked by alias); 4 Jul 2012 08:19:30 -0000 Received: (qmail 4218 invoked by uid 22791); 4 Jul 2012 08:19:27 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,MSGID_MULTIPLE_AT,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_NO X-Spam-Check-By: sourceware.org Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 04 Jul 2012 08:19:08 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Wed, 04 Jul 2012 09:19:05 +0100 Received: from shawin053 ([10.164.6.42]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.0); Wed, 4 Jul 2012 09:19:59 +0100 From: "Terry Guo" To: "'Eli Zaretskii'" Cc: , "Joey Ye" References: <000001cd5338$ded61b20$9c825160$%guo@arm.com> <83hatw8zn1.fsf@gnu.org> In-Reply-To: <83hatw8zn1.fsf@gnu.org> Subject: RE: [PATCH]Fix that GDB will get hang on Windows when using pipe to get stdout and stderr from stub Date: Wed, 04 Jul 2012 08:19:00 -0000 Message-ID: <000301cd59bd$ce1c8900$6a559b00$@guo@arm.com> MIME-Version: 1.0 X-MC-Unique: 112070409190511101 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-07/txt/msg00053.txt.bz2 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 >=20 > > From: "Terry Guo" > > Cc: , > > "Joey Ye" , > > "Matthew Gretton-Dann" , > > "'Pedro Alves'" , > > > > 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. >=20 > I only read it superficially, but it looked fine to me. >=20 > 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 * 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 #ifdef USE_WIN32API #include @@ -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 !=3D -1) + { + ssize_t s; + char buf[GDB_MI_MSG_WIDTH + 1]; + + for (;;) + { + char *current; + char *newline; + int to_read =3D GDB_MI_MSG_WIDTH; + int num_bytes =3D -1; + + if (scb->ops->avail) + num_bytes =3D (scb->ops->avail)(scb, scb->error_fd); + + if (num_bytes !=3D -1) + to_read =3D (num_bytes < to_read) ? num_bytes : to_read; + + if (to_read =3D=3D 0) + break; + + s =3D read (scb->error_fd, &buf, to_read); + if ((s =3D=3D -1) || (s =3D=3D 0 && !close_fd)) + break; + + if (s =3D=3D 0 && close_fd) + { + /* End of file. */ + close (scb->error_fd); + scb->error_fd =3D -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 <=3D GDB_MI_MSG_WIDTH); + buf[s] =3D '\0'; + current =3D buf; + while ((newline =3D strstr (current, "\n")) !=3D NULL) + { + *newline =3D '\0'; + fputs_unfiltered (current, gdb_stderr); + fputs_unfiltered ("\n", gdb_stderr); + current =3D 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 =3D 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 !=3D -1) - { - ssize_t s; - char buf[81]; - - for (;;) - { - char *current; - char *newline; - int to_read =3D 80; - - int num_bytes =3D -1; - if (scb->ops->avail) - num_bytes =3D (scb->ops->avail)(scb, scb->error_fd); - if (num_bytes !=3D -1) - to_read =3D (num_bytes < to_read) ? num_bytes : to_read; - - if (to_read =3D=3D 0) - break; - - s =3D read (scb->error_fd, &buf, to_read); - if (s =3D=3D -1) - break; - if (s =3D=3D 0) - { - /* EOF */ - close (scb->error_fd); - scb->error_fd =3D -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] =3D '\0'; - current =3D buf; - while ((newline =3D strstr (current, "\n")) !=3D NULL) - { - *newline =3D '\0'; - fputs_unfiltered (current, gdb_stderr); - fputs_unfiltered ("\n", gdb_stderr); - current =3D newline + 1; - } - fputs_unfiltered (current, gdb_stderr); - } - } + ser_base_read_error_fd (scb, 1); reschedule (scb); return ch;