[patch] Crashed cross gdb/MinGW host

[patch] Crashed cross gdb/MinGW host

[Masaki Muranaka]

My sh-elf-gdb on MinGW host was crashed
when I tried to debug using the serial connection.
Here is a patch.

It's possible the another plan to remove writefds
check. There is no support yet for writefds.


- - - -
2006-05-19	Masaki Muranaka <monaka@monami-software.com>
    * mingw-hdep.c (gdb_select)
      Add check if writefds is not NULL.
- - - -
Index: mingw-hdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mingw-hdep.c,v
retrieving revision 1.3
diff -u -p -r1.3 mingw-hdep.c
--- mingw-hdep.c        24 Apr 2006 21:00:13 -0000      1.3
+++ mingw-hdep.c        19 May 2006 09:35:32 -0000
@@ -169,7 +169,7 @@ gdb_select (int n, fd_set *readfds, fd_s
        HANDLE fd_h;
        struct serial *scb;
-      if (!FD_ISSET (fd, readfds) && !FD_ISSET (fd, writefds))
+      if (!FD_ISSET (fd, readfds) && !(writefds && FD_ISSET (fd,  
writefds)))
         continue;
        if (FD_ISSET (fd, readfds))

--
Masaki Muranaka
Monami software

Re: [patch] Crashed cross gdb/MinGW host

[Christopher Faylor]

On Fri, May 19, 2006 at 06:47:04PM +0900, Masaki Muranaka wrote:
>My sh-elf-gdb on MinGW host was crashed when I tried to debug using the
>serial connection.  Here is a patch.
>
>It's possible the another plan to remove writefds check.  There is no
>support yet for writefds.

If this is truly supposed to be an emulation of the system select(), I
think it would make sense to check both !readfds and !writefds.

cgf

Re: [patch] Crashed cross gdb/MinGW host

[Masaki Muranaka]

On 2006/05/20, at 3:52, Christopher Faylor wrote:
> If this is truly supposed to be an emulation of the system select(), I
> think it would make sense to check both !readfds and !writefds.

If someone want truly emulation, he/she shuold write
the code around writefds, too.

That patch is just enough to avoid crash.
It seems there is no path readfds becomes NULL in HEAD branch.

--
Masaki Muranaka
Monami software

Re: [patch] Crashed cross gdb/MinGW host

[Daniel Jacobowitz]

On Fri, May 19, 2006 at 02:52:57PM -0400, Christopher Faylor wrote:
> On Fri, May 19, 2006 at 06:47:04PM +0900, Masaki Muranaka wrote:
> >My sh-elf-gdb on MinGW host was crashed when I tried to debug using the
> >serial connection.  Here is a patch.
> >
> >It's possible the another plan to remove writefds check.  There is no
> >support yet for writefds.
> 
> If this is truly supposed to be an emulation of the system select(), I
> think it would make sense to check both !readfds and !writefds.

Sorry for letting this sit so long.  I just encountered this bug today. 
I think I tested console and pipe when I last touched this code
but failed to test ser-base.

The underlying problem is actually different.  The code looks like
this:
      if (!FD_ISSET (fd, readfds) && !FD_ISSET (fd, writefds))
        continue;

      if (FD_ISSET (fd, readfds))
	...

      if (FD_ISSET (fd, exceptfds))
	...

See the mismatch? :-(

I have committed the attached patch as obvious.  It adds the missing
NULL checks, and also corrects the loop check for exceptfds.

-- 
Daniel Jacobowitz
CodeSourcery

2006-06-10  Daniel Jacobowitz  <dan@codesourcery.com>

	* mingw-hdep.c (gdb_select): Always check for NULL fd sets
	before calling FD_ISSET.  Correct check for exceptfds which
	previously tested writefds.

Index: mingw-hdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mingw-hdep.c,v
retrieving revision 1.3
diff -u -p -r1.3 mingw-hdep.c
--- mingw-hdep.c	24 Apr 2006 21:00:13 -0000	1.3
+++ mingw-hdep.c	10 Jun 2006 16:37:55 -0000
@@ -105,8 +105,8 @@ gdb_select (int n, fd_set *readfds, fd_s
 	 if something starts using it.  */
       gdb_assert (!writefds || !FD_ISSET (fd, writefds));
 
-      if (!FD_ISSET (fd, readfds)
-	  && !FD_ISSET (fd, exceptfds))
+      if ((!readfds || !FD_ISSET (fd, readfds))
+	  && (!exceptfds || !FD_ISSET (fd, exceptfds)))
 	continue;
       h = (HANDLE) _get_osfhandle (fd);
 
@@ -124,13 +124,13 @@ gdb_select (int n, fd_set *readfds, fd_s
 	  except = never_handle;
 	}
 
-      if (FD_ISSET (fd, readfds))
+      if (readfds && FD_ISSET (fd, readfds))
 	{
 	  gdb_assert (num_handles < MAXIMUM_WAIT_OBJECTS);
 	  handles[num_handles++] = read;
 	}
 
-      if (FD_ISSET (fd, exceptfds))
+      if (exceptfds && FD_ISSET (fd, exceptfds))
 	{
 	  gdb_assert (num_handles < MAXIMUM_WAIT_OBJECTS);
 	  handles[num_handles++] = except;
@@ -169,10 +169,11 @@ gdb_select (int n, fd_set *readfds, fd_s
       HANDLE fd_h;
       struct serial *scb;
 
-      if (!FD_ISSET (fd, readfds) && !FD_ISSET (fd, writefds))
+      if ((!readfds || !FD_ISSET (fd, readfds))
+	  && (!exceptfds || !FD_ISSET (fd, exceptfds)))
 	continue;
 
-      if (FD_ISSET (fd, readfds))
+      if (readfds && FD_ISSET (fd, readfds))
 	{
 	  fd_h = handles[indx++];
 	  /* This handle might be ready, even though it wasn't the handle
@@ -183,7 +184,7 @@ gdb_select (int n, fd_set *readfds, fd_s
 	    num_ready++;
 	}
 
-      if (FD_ISSET (fd, exceptfds))
+      if (exceptfds && FD_ISSET (fd, exceptfds))
 	{
 	  fd_h = handles[indx++];
 	  /* This handle might be ready, even though it wasn't the handle

Re: [patch] Crashed cross gdb/MinGW host

[Christopher Faylor]

On Sat, Jun 10, 2006 at 02:25:07PM -0400, Daniel Jacobowitz wrote:
>On Fri, May 19, 2006 at 02:52:57PM -0400, Christopher Faylor wrote:
>> On Fri, May 19, 2006 at 06:47:04PM +0900, Masaki Muranaka wrote:
>> >My sh-elf-gdb on MinGW host was crashed when I tried to debug using the
>> >serial connection.  Here is a patch.
>> >
>> >It's possible the another plan to remove writefds check.  There is no
>> >support yet for writefds.
>> 
>> If this is truly supposed to be an emulation of the system select(), I
>> think it would make sense to check both !readfds and !writefds.
>
>Sorry for letting this sit so long.  I just encountered this bug today. 
>I think I tested console and pipe when I last touched this code
>but failed to test ser-base.
>
>The underlying problem is actually different.  The code looks like
>this:
>      if (!FD_ISSET (fd, readfds) && !FD_ISSET (fd, writefds))
>        continue;
>
>      if (FD_ISSET (fd, readfds))
>	...
>
>      if (FD_ISSET (fd, exceptfds))
>	...
>
>See the mismatch? :-(

I saw it and was wondering about it.  I'm glad to see consistency restored.

:-)

cgf

Re: [patch] Crashed cross gdb/MinGW host

[Masaki Muranaka]

On 2006/06/11, at 3:25, Daniel Jacobowitz wrote:
> The underlying problem is actually different.  The code looks like
> this:
(snip)
> See the mismatch? :-(

At that time, I didn't check consistency.
I think your fix is correct.

In mainline, it seems there is no path which become
readfs == NULL | expectfds == NULL.
So it might we have no need to check NULL.
(Of course, sanity checks are always agreeable.)

Thanks for your fix.


--
Masaki Muranaka
Monami software