From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 1C7833858403 for ; Mon, 25 Apr 2022 08:48:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1C7833858403 Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-97-cPgZsCblNEibOK7GQlVo6A-1; Mon, 25 Apr 2022 04:48:34 -0400 X-MC-Unique: cPgZsCblNEibOK7GQlVo6A-1 Received: by mail-wm1-f69.google.com with SMTP id v184-20020a1cacc1000000b00393e492a398so3408639wme.5 for ; Mon, 25 Apr 2022 01:48:34 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version; bh=CaxTw0F87wGGhMAkSsqM6fb/z4zC5NPzyULxYcq/AJc=; b=heS8l/Jp4XLsSI0VOl401WkHolTPFQH3EhO03+ZOyhUXOpMJ/EyXLS1DCvKMnz6Nz1 36MON/xdh0yAQGtIP2IvuFP2q/N25gXmK2npCzDXr/fcSj5mu/zOEt1pTzBk3J+XCAs3 6wf1ow693x4BoyAuhQfoR2HaZj1RaRSlEuSLHyAV5sCpu/GWTiHr1ncgyNle5zBi+9he dRraoKB7bh5TXsKZXcspNhqUuKEhvvVmuY2Z2z7u2PtnDSin/1T5hgzoK1is8CYKftVX TnyATbHvNTttpS56WoTaPAGNsQTRkPG8edr/WrXLZyRyRxLkz46wKbkKhCMo1asqG00V 9wFA== X-Gm-Message-State: AOAM530/JGhGbw7+Eg05SSHzzeBqFlLAKuNbbYSOrLYcN72mG1gsBJXu HXkG+pQhe209RO0xZvFvqJcQwP00F/KNSyvHIIcMIGZBLUwZakjK5NhmyT4Lcqx6y4+lXBUoIms gISm8tL+OPnLxmbAleA7KZQF07IwfpG4zGPP2KLJXLrzPeO9pnM6BBVuSZOnocMJjtNUzl3uLIQ == X-Received: by 2002:a05:6000:1f11:b0:20a:c899:a77a with SMTP id bv17-20020a0560001f1100b0020ac899a77amr12109788wrb.561.1650876512883; Mon, 25 Apr 2022 01:48:32 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzGzvH2X6VJoTlSbCn/avYn+68CuEgbAbyTKy5UREap2h0shn+lFYIXaeJSSRjoQOKkRGZDHw== X-Received: by 2002:a05:6000:1f11:b0:20a:c899:a77a with SMTP id bv17-20020a0560001f1100b0020ac899a77amr12109768wrb.561.1650876512472; Mon, 25 Apr 2022 01:48:32 -0700 (PDT) Received: from localhost (host81-136-113-48.range81-136.btcentralplus.com. [81.136.113.48]) by smtp.gmail.com with ESMTPSA id l8-20020a5d6d88000000b0020a98986534sm8799425wrs.43.2022.04.25.01.48.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 25 Apr 2022 01:48:31 -0700 (PDT) From: Andrew Burgess To: Joel Brobecker via Gdb-patches Cc: Joel Brobecker , gdb-patches@sourceware.org, pedro@palves.net Subject: Re: GDB 12.0.90 available for testing In-Reply-To: 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> Date: Mon, 25 Apr 2022 09:48:31 +0100 Message-ID: <87y1ztpmzk.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, 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: Mon, 25 Apr 2022 08:48:37 -0000 Joel Brobecker via Gdb-patches writes: > 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! Thanks for merging this. Andrew > >> 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