From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x62c.google.com (mail-pl1-x62c.google.com [IPv6:2607:f8b0:4864:20::62c]) by sourceware.org (Postfix) with ESMTPS id 1C7193858D33 for ; Sun, 24 Apr 2022 15:56:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1C7193858D33 Received: by mail-pl1-x62c.google.com with SMTP id n8so21836999plh.1 for ; Sun, 24 Apr 2022 08:56:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=E+is4qU6gvQEWZzxA3uvxTZUbKOS44Q+XSHZGwcHN4w=; b=l6Gf/KzRLszZqnOj2exbBszZ1/awb9ZtRE3kI2FHHwOBnadmpLyGJAFEfSi9aO6gAx d1uZMZ/t7STk7xwnN3blEXgQeC0c1hKxzyYdtgRSjPR4K5tQLD+xQMvbR4t0tBrU6vge 7y5oQFb5GiTuRBOSyeHjCRymw6AzaaaXx2iZIWwXJj8Xtyg8y/+eltCYaPGbNZPv1VU9 TDHJfUtirsHXjpMOCWnVEZKqjQZ+9hbCwaMt9ttkE66sQiItLc2Cg+7FdaxHgH6GCNSF skacd4dsa902KwrR46ZMc4qYBOdfPd37GoV5l+KvgJKG/1e+g6yq+HhdtDwwVSIqlAbR 0Tlw== X-Gm-Message-State: AOAM532EwbIfe6I8xF7/cGJlAwA1ynU8Jxt4JtTAvieAwtPpSCDBJ7Np xMotycoX3sqXxsQs/j27OPfC X-Google-Smtp-Source: ABdhPJx//4oxkKPQWqYAXW9ong36QpotSSOfcnsHwlRrHa2FsKyQY6N3vDlIGS667zsA6zBMNUrsnw== X-Received: by 2002:a17:902:6b44:b0:154:4bee:c434 with SMTP id g4-20020a1709026b4400b001544beec434mr14074705plt.43.1650815818125; Sun, 24 Apr 2022 08:56:58 -0700 (PDT) Received: from takamaka.home ([184.69.131.86]) by smtp.gmail.com with ESMTPSA id l13-20020a056a00140d00b004e13da93eaasm8887236pfu.62.2022.04.24.08.56.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 24 Apr 2022 08:56:57 -0700 (PDT) Received: by takamaka.home (Postfix, from userid 1000) id 72B39A4D6E; Sun, 24 Apr 2022 08:56:56 -0700 (PDT) Date: Sun, 24 Apr 2022 08:56:56 -0700 From: Joel Brobecker To: Andrew Burgess Cc: gdb-patches@sourceware.org, Eli Zaretskii , Joel Brobecker , pedro@palves.net Subject: Re: GDB 12.0.90 available for testing Message-ID: References: <1379565857.1750775.1648990951974@mail.yahoo.com> <1113857996.1779276.1648999598328@mail.yahoo.com> <83czhyfa8i.fsf@gnu.org> <83wng1b161.fsf@gnu.org> <83tuaz6e3x.fsf@gnu.org> <874k2prr1c.fsf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <874k2prr1c.fsf@redhat.com> X-Spam-Status: No, score=-10.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 24 Apr 2022 15:57:01 -0000 Hi everyone, > Sorry for the slow reply, I've been off for the last week, so I'm only > just catching up with my GDB emails. > > How does the patch below look? This moves the setbuf call into a new > function, and includes the comment from gdb_readline_no_editing_callback. Given the feedback received on this patch, and the fact that it was tagged as important for the GDB 12 release, I performed one last round of testing on Windows (using AdaCore's testsuite), and pushed Andrew's patch below to both master and gdb-12-branch. The one change I made was to the commit message, to add a reference to the PR created by Eli. Thanks everyone who contributed to finding, analyzing, fixing and reviewing! > commit 3ee791edccae840e11b9ebe70e5547dfbec04e00 > Author: Andrew Burgess > Date: Tue Apr 19 17:00:04 2022 +0100 > > gdb: move setbuf calls out of gdb_readline_no_editing_callback > > After this commit: > > commit d08cbc5d3203118da5583296e49273cf82378042 > Date: Wed Dec 22 12:57:44 2021 +0000 > > gdb: unbuffer all input streams when not using readline > > Issues were reported with some MS-Windows hosts, see the thread > starting here: > > https://sourceware.org/pipermail/gdb-patches/2022-March/187004.html > > The problem seems to be that calling setbuf on terminal file handles > is not always acceptable, see this mail for more details: > > https://sourceware.org/pipermail/gdb-patches/2022-April/187310.html > > This commit does two things, first moving the setbuf calls out of > gdb_readline_no_editing_callback so that we don't end up calling > setbuf so often. > > Then, for MS-Windows hosts, we don't call setbuf for terminals, this > appears to resolve the issues that have been reported. > > 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 1cfffbeee7e..263a2ce8f43 100644 > --- a/gdb/top.c > +++ b/gdb/top.c > @@ -257,6 +257,41 @@ void (*deprecated_context_hook) (int id); > /* The highest UI number ever assigned. */ > static int highest_ui_num; > > +/* Unbuffer STREAM. This is a wrapper around setbuf(STREAM, nullptr) > + which applies some special rules for MS-Windows hosts. */ > + > +static void > +unbuffer_stream (FILE *stream) > +{ > + /* Unbuffer the input stream so that in gdb_readline_no_editing_callback, > + the calls to fgetc fetch only one char at the time from STREAM. > + > + This is important because gdb_readline_no_editing_callback will read > + from STREAM up to the first '\n' character, after this GDB returns to > + the event loop and relies on a select on STREAM indicating that more > + input is pending. > + > + If STREAM is buffered then the fgetc calls may have moved all the > + pending input from the kernel into a local buffer, after which the > + select will not indicate that more input is pending, and input after > + the first '\n' will not be processed immediately. > + > + Please ensure that any changes in this area run the MI tests with the > + FORCE_SEPARATE_MI_TTY=1 flag being passed. */ > + > +#ifdef __MINGW32__ > + /* With MS-Windows runtime, making stdin unbuffered when it's > + connected to the terminal causes it to misbehave. */ > + if (!ISATTY (stream)) > + setbuf (stream, nullptr); > +#else > + /* On GNU/Linux the issues described above can impact GDB even when > + dealing with input from a terminal. For now we unbuffer the input > + stream for everyone except MS-Windows. */ > + setbuf (stream, nullptr); > +#endif > +} > + > /* See top.h. */ > > ui::ui (FILE *instream_, FILE *outstream_, FILE *errstream_) > @@ -283,6 +318,8 @@ ui::ui (FILE *instream_, FILE *outstream_, FILE *errstream_) > { > buffer_init (&line_buffer); > > + unbuffer_stream (instream_); > + > if (ui_list == NULL) > ui_list = this; > else > @@ -412,6 +449,8 @@ read_command_file (FILE *stream) > { > struct ui *ui = current_ui; > > + unbuffer_stream (stream); > + > scoped_restore save_instream > = make_scoped_restore (&ui->instream, stream); > > -- Joel