Re: GDB 12.0.90 available for testing

[Andrew Burgess <aburgess@redhat.com>]

Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org> writes:

>> Date: Thu, 31 Mar 2022 10:48:18 +0100
>> Cc: gdb-patches@sourceware.org, Andrew Burgess <aburgess@redhat.com>
>> From: Pedro Alves <pedro@palves.net>
>> 
>> >>   https://sourceware.org/bugzilla/show_bug.cgi?id=29002
>> > 
>> > Ping!  The two issues described in this PR are IMO serious and should
>> > be fixed before GDB 12.1 is released.  Could someone please respond to
>> > what I wrote there, and propose how to fix this without adversely
>> > affecting GNU/Linux (and possibly other non-Windows platforms)?
>> 
>> So from the bugzilla discussion, it sounds like calling setvbuf all the
>> time is a bad idea.  So we should call it once for a given stream early
>> on, just once.  That sounds reasonable to me.  However, the patch you
>> attached in bugzilla doesn't seem to be doing that correctly:
>> 
>> 
>> 836	  if (!done_once && !ISATTY (stream))
>> 837	    {
>> 838	      setbuf (stream, NULL);
>> 839	      done_once = 1;
>> 840	    }
>> 836	
>> 841	
>> 
>> because that will only do it once for all streams, and we need to do this
>> once per stream.
>
> The above was what we did in GDB 11 and before.

But what we used to do wasn't good enough.  On Linux we have to turn off
buffering even when 'ISATTY (stream)' is true, otherwise glibc/kernel
will conspire to hide input from us.

Could you give the patch below a try, please.  It tries to move the
setbuf calls out more so (I hope) they only get done once per stream,
and before we've started reading anything from the stream.

Thanks,
Andrew

---

diff --git a/gdb/event-top.c b/gdb/event-top.c
index b628f0397cb..a55338d78a2 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -818,19 +818,6 @@ gdb_readline_no_editing_callback (gdb_client_data client_data)
   FILE *stream = ui->instream != nullptr ? ui->instream : ui->stdin_stream;
   gdb_assert (stream != nullptr);
 
-  /* Unbuffer the input stream, so that, later on, the calls to fgetc
-     fetch only one char at the time from the stream.  The fgetc's will
-     get up to the first newline, but there may be more chars in the
-     stream after '\n'.  If we buffer the input and fgetc drains the
-     stream, getting stuff beyond the newline as well, a select, done
-     afterwards will not trigger.
-
-     This unbuffering was, at one point, not applied if the input stream
-     was a tty, however, the buffering can cause problems, even for a tty,
-     in some cases.  Please ensure that any changes in this area run the MI
-     tests with the FORCE_SEPARATE_MI_TTY=1 flag being passed.  */
-  setbuf (stream, NULL);
-
   /* We still need the while loop here, even though it would seem
      obvious to invoke gdb_readline_no_editing_callback at every
      character entered.  If not using the readline library, the
diff --git a/gdb/top.c b/gdb/top.c
index ecd31456f03..456130856e5 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -283,6 +283,8 @@ ui::ui (FILE *instream_, FILE *outstream_, FILE *errstream_)
 {
   buffer_init (&line_buffer);
 
+  setbuf (instream_, NULL);
+
   if (ui_list == NULL)
     ui_list = this;
   else
@@ -412,6 +414,8 @@ read_command_file (FILE *stream)
 {
   struct ui *ui = current_ui;
 
+  setbuf (stream, nullptr);
+
   scoped_restore save_instream
     = make_scoped_restore (&ui->instream, stream);