From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 68C7B3857BB2 for ; Thu, 22 Sep 2022 16:02:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 68C7B3857BB2 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 28MG2G6M003882 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 22 Sep 2022 12:02:21 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 28MG2G6M003882 Received: from simark.localdomain (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 8FB9B1E0D3; Thu, 22 Sep 2022 12:02:16 -0400 (EDT) From: Simon Marchi To: gdb-patches@sourceware.org Subject: [PATCH] gdb/testsuite/tui: start GDB with "set filename-display basename" Date: Thu, 22 Sep 2022 12:02:15 -0400 Message-Id: <20220922160215.742183-1-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.37.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Thu, 22 Sep 2022 16:02:16 +0000 X-Spam-Status: No, score=-3189.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_ASCII_DIVIDERS, KAM_SHORT, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Thu, 22 Sep 2022 16:02:32 -0000 The test gdb.tui/tui-missing-src.exp fails on my CI machine, and I concluded that it is caused by the long source directory name: /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/src/binutils-gdb The long name causes some particular redrawing that doesn't happen for shorter directories, and causes a Term::command call to return too early. This can be reproduced by cloning the binutils-gdb repo in a directory with a name similar to the one shown above. $ pwd /home/simark/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd64/target_board/unix/src/binutils-gdb/build/gdb $ make check-read1 TESTS="gdb.tui/tui-missing-src.exp" FAIL: gdb.tui/tui-missing-src.exp: checking if inside f2 () FAIL: gdb.tui/tui-missing-src.exp: f2.c must be displayed in source window FAIL: gdb.tui/tui-missing-src.exp: check source box is empty after return FAIL: gdb.tui/tui-missing-src.exp: Back in main Note that using "make check" instead of "make check-read1" only shows the last 2 failures for me. When running gdb.tui/tui-missing-src.exp in a directory with a shorter name, the terminal looks like this by the time the "checking if inside f2" test runs: Screen Dump (size 80 columns x 24 rows, cursor at column 6, row 23): 0 +-...ld/binutils-gdb-noasan/gdb/testsuite/outputs/gdb.tui/tui-missing-src/f2.c-+ 1 | 1 | 2 | 2 int | 3 | 3 f2 (int x) | 4 | 4 { | 5 | > 5 x <<= 1; | 6 | 6 return x+5; | 7 | 7 } | 8 | 8 | 9 | 9 | 10 | 10 | 11 | 11 | 12 | 12 | 13 | 13 | 14 +------------------------------------------------------------------------------+ 15 multi-thre Thread 0x7ffff7cc07 In: f2 L5 PC: 0x555555555143 16 at /home/simark/build/binutils-gdb-noasan/gdb/testsuite/outputs/gdb.tui/tui- 17 missing-src/main.c:6 18 (gdb) next 19 (gdb) step 20 f2 (x=4) 21 at /home/simark/build/binutils-gdb-noasan/gdb/testsuite/outputs/gdb.tui/tui- 22 missing-src/f2.c:5 23 (gdb) PASS: gdb.tui/tui-missing-src.exp: checking if inside f2 () When running the `Term::command "step"` just before, GDB writes the "step", which makes the `wait_for` proc go in the "looking for the prompt" mode, to know when the command's execution is complete. As some new output appears, lines that must disappear are deleted using the "Delete Line" operation [1] and some new ones are drawn. The source window gets redrawn with the contents of the f2.c file. Then, GDB writes the prompt (at line 23 above), which satisfies `wait_for`, which then returns. The state of the terminal is therefore correct for the "check if inside f2" and "f2.c must be displayed in the source window" tests. In the non-working case, the terminal looks like this by the time the "check if inside f2" test runs: Screen Dump (size 80 columns x 24 rows, cursor at column 6, row 17): 0 +------------------------------------------------------------------------------+ 1 | | 2 | | 3 | | 4 | | 5 | | 6 | | 7 | [ No Source Available ] | 8 | | 9 | | 10 | | 11 | | 12 | | 13 | | 14 +------------------------------------------------------------------------------+ 15 multi-thre Thread 0x7ffff7cc1b In: main L7 PC: 0x555555555128 16 sing-src/main.c:6 17 (gdb) ary breakpoint 1, main () 18 at /home/simark/workspace/binutils-gdb_master_linuxbuild/platform/jammy-amd6 19 4/target_board/unix/src/binutils-gdb/build/gdb/testsuite/outputs/gdb.tui/tui-mis 20 sing-src/main.c:6 21 (gdb) next 22 (gdb) step 23 FAIL: gdb.tui/tui-missing-src.exp: checking if inside f2 () What happened is: GDB wrote the "step" command, which make the `wait_for` proc go in its "looking for the prompt" mode. However, curses decided to redraw whatever scrolled up to line 17 using some standard character insertion operations: +++ Cursor Down (1), cursor: (16, 0) -> (17, 0) +++ Inserting string '(' +++ Inserted char '(', cursor: (17, 0) -> (17, 1) +++ Inserted string '(', cursor: (17, 0) -> (17, 1) +++ Inserting string 'g' +++ Inserted char 'g', cursor: (17, 1) -> (17, 2) +++ Inserted string 'g', cursor: (17, 1) -> (17, 2) +++ Inserting string 'd' +++ Inserted char 'd', cursor: (17, 2) -> (17, 3) +++ Inserted string 'd', cursor: (17, 2) -> (17, 3) +++ Inserting string 'b' +++ Inserted char 'b', cursor: (17, 3) -> (17, 4) +++ Inserted string 'b', cursor: (17, 3) -> (17, 4) +++ Inserting string ')' +++ Inserted char ')', cursor: (17, 4) -> (17, 5) +++ Inserted string ')', cursor: (17, 4) -> (17, 5) +++ Inserting string ' ' +++ Inserted char ' ', cursor: (17, 5) -> (17, 6) +++ Inserted string ' ', cursor: (17, 5) -> (17, 6) And that causes `wait_for` to think the "step" command is complete. This is wrong, as the prompt at line 17 isn't the prompt drawn after the completion of the "step" command. The subsequent tests now run with a partially updated screen (what is shown above) and obviously fail. The ideal way to fix this would be for `wait_for` to be smarter, to avoid it confusing the different prompts drawn. However, I would also like to reduce the variations in TUI test results due to the directories (source and build) in which tests are ran. TUI tests are more prone to differences in test results due to variations in directory names than other tests, as it makes curses take different redrawing decisions. So in this patch, I propose to make TUI tests use "set filename-display basename", which makes GDB omit directory names when it prints file names. This way, regardless of where you run the tests, you should get the same results (all other things being equal). Doing this happens to fix my failures and makes my CI happy (which in turns makes me happy). To be clear, I understand that this does not fix the root issue of `proc wait_for` being confused. However, it makes TUI test runs be more similar for everyone, such that there's less chance of TUI tests randomly failing for somebody. If some other change triggers the `wait_for` problem again in the future, hopefully everybody will see the problem and we can work on getting it fixed more easily than if just one unlucky person sees the problem. Note that there are other reasons why TUI tests could vary, like different curses library versions taking different re-drawing decisions. However, I think my change is a good step towards more stable test results. [1] https://vt100.net/docs/vt510-rm/DL.html Change-Id: Ib18da83317e7b78a46f77892af0d2e39bd261bf5 --- gdb/testsuite/lib/tuiterm.exp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp index bf094131eac0..6ca113fdd8ef 100644 --- a/gdb/testsuite/lib/tuiterm.exp +++ b/gdb/testsuite/lib/tuiterm.exp @@ -757,9 +757,15 @@ namespace eval Term { # clean_restart. proc clean_restart {rows cols {executable {}}} { global env stty_init - save_vars {env(TERM) stty_init} { + save_vars {env(TERM) stty_init ::GDBFLAGS} { setenv TERM ansi _setup $rows $cols + + # Make GDB not print the directory names. Use this setting to + # remove the differences in test runs due to varying directory + # names. + append ::GDBFLAGS " -ex \"set filename-display basename\"" + if {$executable == ""} { ::clean_restart } else { base-commit: aaf3f3f3bb38a59125ea34afa0ef7e0e14c2e916 -- 2.37.3