From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x435.google.com (mail-wr1-x435.google.com [IPv6:2a00:1450:4864:20::435]) by sourceware.org (Postfix) with ESMTPS id 2B2EA397EC0A for ; Tue, 27 Jul 2021 10:40:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2B2EA397EC0A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=embecosm.com Received: by mail-wr1-x435.google.com with SMTP id e2so14621913wrq.6 for ; Tue, 27 Jul 2021 03:40:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Tomux/KVmTEo3n3iHlen0S1gaPFXYDAkh8NM9916mXw=; b=AkbbZYY+yOYjDZJ1RoN7JXxm1a1A5vqTbwHC3bA29sAK6bGUqfoM5fdl5GbDJsE03f 692o304qK+USu74RhIn7DJc9H4O2I/UDq+xh49LysXqVrlJIr5B4GB9dJrqQG9wJgRpA agt4ACvO+LhHaVfiYAVI0DNCd4lfg3IMNq/VwBW5MtiYyEX9gURGb7wWm4A2tY/544+v xSmDCpNJsX0uApANFDsp77V3QBb1RcTPs5LwmUHXlwqnlaKppXOz7FfHu5uZbJzJiExP NO+Arswln2QQnurjqkmZuUo7Iffw3/lFOURZi7vgEdVavKbHG989oB85S+e46lg84l5Z a/7Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=Tomux/KVmTEo3n3iHlen0S1gaPFXYDAkh8NM9916mXw=; b=BZv9dHrztRP6/lOFDB1p+COxRvv61b4IyV97j9GvUntIgO2mAMIKs2p9CHq1A6+CmB T/UgTPGNnrwzTROlGERGDyA640Lz19oXz/wM9ugQWc/YZTWy+f0SNGFyCF4x8xB4CcVu XQZbuikb5mS/Fkw+Z7Rv9zALOottWjfPY7iFgMe0b9s57SiIfPmLbcF3o7H4ITWxTOZ+ GHAwr5BAXfj6a/SeOHQ6d5YN68Ml1OksYkRd+drS8YkUHrKA+YJumiAkdaQdQMTgqMRQ XcE/4QFWBO8A6YtIAid7gZQCcHrZekpZcFwcOhEhTBShyxxmmmlg5nkFYce7P8h3VjdO qX8A== X-Gm-Message-State: AOAM532HfW7SWnP6u47HAMi11txNx/vFMhnmIcNhdZl2ZKSH4JPzfQi8 7UdCbACCuQoYZDrOeRTyRVWfEg== X-Google-Smtp-Source: ABdhPJwz6j+zJ/b2JBBWX0lYYM4mjnvaGymcz5r81L7l/GuH0Q8F7Pqibpbnz0/dg5FqkZi9mB0obw== X-Received: by 2002:a05:6000:10d0:: with SMTP id b16mr24149752wrx.332.1627382441265; Tue, 27 Jul 2021 03:40:41 -0700 (PDT) Received: from localhost (host86-161-16-194.range86-161.btcentralplus.com. [86.161.16.194]) by smtp.gmail.com with ESMTPSA id q14sm2911251wrs.8.2021.07.27.03.40.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 27 Jul 2021 03:40:40 -0700 (PDT) Date: Tue, 27 Jul 2021 11:40:39 +0100 From: Andrew Burgess To: Carl Love Cc: gdb-patches@sourceware.org, Will Schmidt , Rogerio Alves , Ulrich Weigand Subject: Re: [PATCH] Fix for gdb.tui/tui-layout-asm.exp Message-ID: <20210727104039.GM1872618@embecosm.com> References: <20210726123320.GJ1872618@embecosm.com> <538c169dedebcd3987eb603a0bc0c13376827c9c.camel@us.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <538c169dedebcd3987eb603a0bc0c13376827c9c.camel@us.ibm.com> X-Operating-System: Linux/5.8.18-100.fc31.x86_64 (x86_64) X-Uptime: 11:37:48 up 10 days, 21:11, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_BARRACUDACENTRAL, 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: Tue, 27 Jul 2021 10:40:44 -0000 * Carl Love [2021-07-26 09:59:11 -0700]: > Andrew: > > On Mon, 2021-07-26 at 13:33 +0100, Andrew Burgess wrote: > > I can't think of a better way to solve this issue. However, I wonder > > if there is more we could do to help anyone in the future who might > > hit this issue again? > > > > Consider the patch I've attached below. In this version the width is > > still 80 (for now), so it should fail for you. However, my hope is > > that when you look at the log file you should see a big message > > printed right next to the failure. This message will direct you to > > the test script, which (I hope) should then explain what's gone > > wrong. > > > > My expectation is that you would still need to adjust the window > > width > > from 80 to 90 in a final version of this patch. > > > > How would you feel with this? > > Yes, I like it. The issue was really not obvious to me at first. As > embaressing as it is to admit, it took me way too long to realize what > the error was. I looked at a number of other things that I thought > could be the cause of the failure, timeout, read failure etc. > > I wasn't all that happy with just changing the window from 80 to 90 as > it was a bit of an arbitrary choice. There is no way to ensure that > will cover all possible cases in the future. The additional checks and > messages should really help in the future if the issue occurs again. I > did note in the patch that PPC needs it to be at least 90 characters > wide so someone would know why 90 was picked. Otherwise, it is just an > arbitrary looking setting. > > I tested the patch with a width of 80 to verify the test fails and the > warning messages are printed to the log. I then changed the width to > 90 and verified the test now works correctly. I have attached you > patch with the minor PPC64 changes. Please let me know if it looks ok. > Thanks for all the help. > > Carl Love > > ------------------------------------------------------------ > Fix for gdb.tui/tui-layout-asm.exp > > The width of the window is too narrow to display the entire assembly line. > The width of the columns in the window changes as the test walks thru the > terminal window output. The column change results in the first and second > reads of the same line to differ thus causing the test to fail. Increasing > the width of the window keeps the column width consistent thru the test. > > If the test fails, the added check prints an message to the log file if > the failure may be due to the window being too narrow. > > gdb/testsuite/ChangeLog > > *gdb.tui/tui-layout-asm.exp: Replace window width of 80 with the > tui_asm_window_width variable for the width. > Add count_whitespace proceedure. > Add if count_whitespace check. The ChangeLog block should be formatted as: * gdb.tui/tui-layout-asm.exp: Replace window width of 80 with the tui_asm_window_width variable for the width. Add if count_whitespace check. (count_whitespace): New proc. Otherwise, this looks good to me. I think it would be a good idea though to wait for 48hrs before pushing this, just in case someone else wants to comment. Thanks, Andrew > --- > gdb/testsuite/gdb.tui/tui-layout-asm.exp | 32 ++++++++++++++++++++++-- > 1 file changed, 30 insertions(+), 2 deletions(-) > > diff --git a/gdb/testsuite/gdb.tui/tui-layout-asm.exp b/gdb/testsuite/gdb.tui/tui-layout-asm.exp > index 19ce333ca9e..3ff5347f5ed 100644 > --- a/gdb/testsuite/gdb.tui/tui-layout-asm.exp > +++ b/gdb/testsuite/gdb.tui/tui-layout-asm.exp > @@ -24,15 +24,23 @@ if {[build_executable "failed to prepare" ${testfile} ${srcfile}] == -1} { > return -1 > } > > -Term::clean_restart 24 80 $testfile > +# PPC currently needs a minimum window width of 90 to work correctly. > +set tui_asm_window_width 90 > + > +Term::clean_restart 24 ${tui_asm_window_width} $testfile > if {![Term::prepare_for_tui]} { > unsupported "TUI not supported" > return > } > > +# Helper proc, returns a count of the ' ' characters in STRING. > +proc count_whitespace { string } { > + return [expr {[llength [split $string { }]] - 1}] > +} > + > # This puts us into TUI mode, and should display the ASM window. > Term::command_no_prompt_prefix "layout asm" > -Term::check_box_contents "check asm box contents" 0 0 80 15 "
" > +Term::check_box_contents "check asm box contents" 0 0 ${tui_asm_window_width} 15 "
" > > # Scroll the ASM window down using the down arrow key. In an ideal > # world we'd like to use PageDown here, but currently our terminal > @@ -65,6 +73,26 @@ while (1) { > && [regexp $re_line [Term::get_line 1]]} { > # We scrolled successfully. > } else { > + if {[count_whitespace ${line}] != \ > + [count_whitespace [Term::get_line 1]]} { > + # GDB's TUI assembler display will widen columns based on > + # the longest item that appears in a column on any line. > + # As we have just scrolled, and so revealed a new line, it > + # is possible that the width of some columns has changed. > + # > + # As a result it is possible that part of the line we were > + # expected to see in the output is now off the screen. And > + # this test will fail. > + # > + # This is unfortunate, but, right now, there's no easy way > + # to "lock" the format of the TUI assembler window. The > + # only option appears to be making the window width wider, > + # this can be done by adjusting TUI_ASM_WINDOW_WIDTH. > + verbose -log "WARNING: The following failure is probably due to the TUI window" > + verbose -log " width. See the comments in the test script for more" > + verbose -log " details." > + } > + > fail "$testname (scroll failed)" > Term::dump_screen > break > -- > 2.17.1 > >