Windows semihosting (remote-fileio) fix for console reads

Windows semihosting (remote-fileio) fix for console reads

[Daniel Jacobowitz]

First of all, a disclaimer.  I will absolutely understand if someone wants
to draw the line on broken system support on the wrong side of this patch.
It's incredibly bizarre.  I'm going to let it sit for comments for a bit; if
you object, please let me know.  I don't really know how I could move the
ugly bit to a Windows-specific file, but maybe we can come up with
something.

Here's the problem: a customer tried to use the remote file I/O protocol to
read from standard input.  Trivial little program, just a read() from stdin
followed by a write() to stdout.  Read was returning -1.  A well-placed
printf discovered that this was actually ENOMEM.

By trial and error, we worked out that on the system we were using a read of
26609 bytes from the console, or more, would always return ENOMEM.  Given
26608 bytes or less, it would succeed normally.  Please don't ask me what
that number means.  No, it does not vary with the size of the malloced
buffer.

Read is always allowed to return a short value, and it was already
hardcoding a buffer size of 32767.  So this changes that buffer to 8K,
with a comment about the arbitrary number.

I don't believe there's anywhere else we do big reads from stdin.
Normally we want one character at a time.

-- 
Daniel Jacobowitz
CodeSourcery

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

	gdb/
	* remote-fileio.c (remote_fileio_func_read): Limit console
	reads to 8K.

Index: remote-fileio.c
===================================================================
RCS file: /cvs/src/src/gdb/remote-fileio.c,v
retrieving revision 1.18
diff -u -p -r1.18 remote-fileio.c
--- remote-fileio.c	8 Jun 2006 19:08:22 -0000	1.18
+++ remote-fileio.c	10 Jun 2006 18:11:51 -0000
@@ -729,7 +729,7 @@ remote_fileio_func_read (char *buf)
 	  static char *remaining_buf = NULL;
 	  static int remaining_length = 0;
 
-	  buffer = (gdb_byte *) xmalloc (32768);
+	  buffer = (gdb_byte *) xmalloc (8192);
 	  if (remaining_buf)
 	    {
 	      remote_fio_no_longjmp = 1;
@@ -751,7 +751,16 @@ remote_fileio_func_read (char *buf)
 	    }
 	  else
 	    {
-	      ret = ui_file_read (gdb_stdtargin, (char *) buffer, 32767);
+	      /* NOTE drow/2006-06-10: Windows (mingw32) has a truly
+		 bizarre bug.  If a handle is backed by a real console
+		 device, overly large reads from the console will fail
+		 and set errno == ENOMEM.  On a Windows Server 2003
+		 system where I tested, reading 26608 bytes from the
+		 console was OK, but anything about 26609 bytes would
+		 fail.  So, we limit this read to something smaller
+		 than that - by a safe margin, in case the limit
+		 depends on system resources or version.  */
+	      ret = ui_file_read (gdb_stdtargin, (char *) buffer, 8191);
 	      remote_fio_no_longjmp = 1;
 	      if (ret > 0 && (size_t)ret > length)
 		{

Re: Windows semihosting (remote-fileio) fix for console reads

[Eli Zaretskii]

> Date: Sat, 10 Jun 2006 14:21:31 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> Here's the problem: a customer tried to use the remote file I/O protocol to
> read from standard input.  Trivial little program, just a read() from stdin
> followed by a write() to stdout.  Read was returning -1.  A well-placed
> printf discovered that this was actually ENOMEM.
> 
> By trial and error, we worked out that on the system we were using a read of
> 26609 bytes from the console, or more, would always return ENOMEM.  Given
> 26608 bytes or less, it would succeed normally.  Please don't ask me what
> that number means.  No, it does not vary with the size of the malloced
> buffer.

That rings a bell, but I'll need to dig deep into old files to find
out the details.  In the meantime, could you please post a short test
program that demonstrates this failure?

Thanks.

Re: Windows semihosting (remote-fileio) fix for console reads

[Daniel Jacobowitz]

On Sun, Jun 11, 2006 at 12:28:33AM +0300, Eli Zaretskii wrote:
> That rings a bell, but I'll need to dig deep into old files to find
> out the details.  In the meantime, could you please post a short test
> program that demonstrates this failure?

I'm afraid I don't have one; I tested by changing the size of the read
performed by remote-fileio, which requires a remote target and stub to
reproduce.

It turned out to be trivial to reproduce it manually though.  Here.
Running this under Cygwin works (there, stdin is a pipe); this includes
running it from a Cygwin bash in a command terminal.  But if you
Start->Run cmd.exe, and then run the test program, read will fail.

[Interestingly, if I use a stack buffer and it's far too small for the
read request, Cygwin returns EINVAL... nice trick.]

#include <unistd.h>
#include <stdio.h>
#include <errno.h>
#include <string.h>

static char buf[32768];

int
main (int argc, char **argv)
{
  int ret, err;

  sprintf (buf, "Type something:\n");
  write (1, buf, strlen (buf));
  errno = 0;
  ret = read (0, buf, 26609);
  err = errno;

  sprintf (buf, "read returned %d, errno %d\n", ret, err);
  write (1, buf, strlen (buf));
  return 0;
}

-- 
Daniel Jacobowitz
CodeSourcery

Re: Windows semihosting (remote-fileio) fix for console reads

[Eli Zaretskii]

> Date: Sat, 10 Jun 2006 19:43:57 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> #include <unistd.h>
> #include <stdio.h>
> #include <errno.h>
> #include <string.h>
> 
> static char buf[32768];
> 
> int
> main (int argc, char **argv)
> {
>   int ret, err;
> 
>   sprintf (buf, "Type something:\n");
>   write (1, buf, strlen (buf));
>   errno = 0;
>   ret = read (0, buf, 26609);
>   err = errno;
> 
>   sprintf (buf, "read returned %d, errno %d\n", ret, err);
>   write (1, buf, strlen (buf));
>   return 0;
> }

I tried this on two XP boxes: on one, 26609 was the smallest value
that failed, on the other it started to fail with 31005.  Go figure.

Anyway, I suggest to go for 16K; 8K sounds too conservative.

Re: Windows semihosting (remote-fileio) fix for console reads

[Jim Blandy]

Eli Zaretskii <eliz@gnu.org> writes:
>> Date: Sat, 10 Jun 2006 19:43:57 -0400
>> From: Daniel Jacobowitz <drow@false.org>
>> 
>> #include <unistd.h>
>> #include <stdio.h>
>> #include <errno.h>
>> #include <string.h>
>> 
>> static char buf[32768];
>> 
>> int
>> main (int argc, char **argv)
>> {
>>   int ret, err;
>> 
>>   sprintf (buf, "Type something:\n");
>>   write (1, buf, strlen (buf));
>>   errno = 0;
>>   ret = read (0, buf, 26609);
>>   err = errno;
>> 
>>   sprintf (buf, "read returned %d, errno %d\n", ret, err);
>>   write (1, buf, strlen (buf));
>>   return 0;
>> }
>
> I tried this on two XP boxes: on one, 26609 was the smallest value
> that failed, on the other it started to fail with 31005.  Go figure.
>
> Anyway, I suggest to go for 16K; 8K sounds too conservative.

The patch looks okay to me; I agree with Eli's suggestion.