From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id F41683858D1E for ; Tue, 2 May 2023 13:32:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F41683858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 2BB7C21D47 for ; Tue, 2 May 2023 13:32:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1683034354; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=mBekDnfQbcCmAU2ljykMi7hFKSF9lV9L+9IkEI5Df8E=; b=bpVdJJnwCCfTT4/lzb9HrxrhrmFpUgwjwIq46MyCwNPBQQZ8lvnsS5gQLN2ok4adAojRVQ aEJ0uCAj0ZqtTRcAF9sIjd/lP6KZ/vqkiQaFtSnJ6Q824fztPcGoWnpiZUlmhCpgymSkw1 mPQKDun+9rcP6YVHA4cQzBqlBlyfGhw= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1683034354; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=mBekDnfQbcCmAU2ljykMi7hFKSF9lV9L+9IkEI5Df8E=; b=gJi1xMDzTgizuiWh1spc/wRpkureMsKqQjJ2LHwMtLDJvIqU52w8O9SffPiXIbHxj2yEVL S9ndd3dLWM1GYRDg== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 1472B134FB for ; Tue, 2 May 2023 13:32:34 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id K4LNA/IQUWRqXgAAMHmgww (envelope-from ) for ; Tue, 02 May 2023 13:32:34 +0000 From: Tom de Vries To: gdb-patches@sourceware.org Subject: [PATCH v2] [gdb/cli] Fix wrapping for TERM=ansi Date: Tue, 2 May 2023 15:32:58 +0200 Message-Id: <20230502133258.8380-1-tdevries@suse.de> X-Mailer: git-send-email 2.35.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: I. Auto-detected width (xterm vs. ansi) Say we have a terminal with a width of 40 chars: ... $ echo $COLUMNS 40 ... With TERM=xterm, we report a width of 40 chars: ... $ TERM=xterm gdb (gdb) show width Number of characters gdb thinks are in a line is 40. ... And with TERM=ansi, a width of 39 chars: ... $ TERM=ansi gdb (gdb) show width Number of characters gdb thinks are in a line is 39. ... Gdb uses readline to auto-detect screen size, and readline decides in the TERM=ansi case that the terminal does not have reliable auto-wrap, and consequently decides to hide the last terminal column from the readline user (in other words GDB), hence we get 39 instead of 40. II. Documented semantics of "set width" Reading the documentation, the width setting is described in different ways. There's the role of determining screen width: ... Normally GDB knows the size of the screen from the terminal driver software. For example, on Unix GDB uses the termcap data base together with the value of the TERM environment variable and the stty rows and stty cols settings. If this is not correct, you can override it with the set height and set width commands. ... Also, there's something related to wrapping (using "when", AFAIU it could mean either "whether" or "where" in this context): ... the screen width setting determines when to wrap lines of output ... The "set width" help text is more specific (using "where"): ... $ gdb -ex "help set width" Set number of characters where GDB should wrap lines of its output. This affects where GDB wraps its output to fit the screen width. Setting this to "unlimited" or zero prevents GDB from wrapping its output. (gdb) ... FWIW, in older gdb (7.5 and before) it used to be just: ... Set number of characters gdb thinks are in a line. ... Anyway, I'm confused. Does set width: - override auto-detected screen width, or does it - determine where GDB should wrap lines of its output? III. Types of wrapping Looking a bit more in detail inside gdb, it seems there are two types of wrapping: - readline wrapping (in other words, prompt edit wrapping), and - gdb output wrapping (can be observed by issuing "info sources"). This type of wrapping attempts to wrap some of the gdb output earlier than the indicated width, to not break lines in inconvenient places. IV. Readline wrapping, setting width to less than screen size So, how does readline wrapping behave when setting width to less than screen size, say 20? Let's try that out. First, let's note that the prompt prefix "(gdb) " is 6 chars long: ... 123456 (gdb) ... so we'll tag on starting with 7. With TERM=xterm, after typing the last 0, the cursor goes to the '(' at the start of the line: ... (gdb) set width 20 (gdb) 78901234567890 ... and after typing 1, we have: ... (gdb) set width 20 1gdb) 78901234567890 ... Things go even worse after backspacing, which clears the entire line and then goes into the "set width 20" line. Alright, now with TERM=ansi: ... (gdb) set width 20 (gdb) 7890123456789 0 ... Things do wrap, just after 19 instead of 20 chars. So what happened here? With TERM=xterm, readline decides that the terminal has reliable auto-wrap, and does insert an '\r' at the point of wrapping but leaves it to the terminal to insert the '\n'. But because the terminal is actually 40 wide, that doesn't happen, and so the '\r' brings the cursor to the start of the same line. In other words, the terminal is 40 wide, but we're telling the gdb that it's 20 wide, which is incorrect, and we get into trouble. With TERM=ansi, readline decides that the terminal does not have reliable auto-wrap, and consequently decides to hide the last terminal column from the user (hence 19 instead of 20), and inserts the '\n' itself. Consequently, the wrapping does work. But, there's no need to reduce the number of columns. The reason this is done is to avoid may-or-may-not auto-wrap in the last column, but there's reliably no auto-wrap in column 20, because the the actual screen width is 40. In other words, the terminal is 40 wide, but we're telling gdb that it's 20 wide, which is incorrect, and we get less usable width than possible. So, this doesn't work as expected in both cases. The ansi case could be fixed by working around the off-by-one error. The xterm case, not really, unless we trick readline into thinking that auto-wrap is not supported. We could do that by overriding _rl_term_autowrap to 0, but as per PR build/10723 we try to get rid of dependencies on readlines internal variables. And even if we would use that, we'd have to avoid overriding when setting width to the actual screen width, otherwise you'd get '\n' both from readline and the terminal. So, you'd need the actual screen width, which "set width" also determines. This just goes to show that the idea of administering two separate concepts of "screen width" and "wrapping location" in a single variable is not a good idea. V. gdb output wrapping, setting width to less than screen size So, how does the gdb output wrapping fare if we set width to less than screen size? Let's try in an 80 COLUMNS terminal with TERM=xterm: ... (gdb) info sources ... /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/../sysdeps/x86_64/crtn.S, /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/elf-init.c, /data/vries/hello.c, /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/../sysdeps/x86_64/crti.S, /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/init.c, /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/../sysdeps/x86_64/start.S ... Ok, now let's set width to 70, and try again: ... /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/../sysdeps/x86_64/crtn.S, /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/elf-init.c, /data/vries/hello.c, /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/../sysdeps/x86_64/crti.S, /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/init.c, /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/../sysdeps/x86_64/start.S ... We can see the effect in the placement of /data/vries/hello.c, which now is on its own line. We have the same at 60, but at 50, we're back at: ... /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/../sysdeps/x86_64/crtn.S, /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/elf-init.c, /data/vries/hello.c, /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/../sysdeps/x86_64/crti.S, /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/init.c, /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/../sysdeps/x86_64/start.S ... Apparantly GDB assumes that the screen size is 50, and that the terminal breaks the line at 50. We can see this by resizing the terminal to a width of 50, which gives us: ... /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/elf-ini t.c, /data/vries/hello.c, ... And that explains why gdb doesn't put /data/vries/hello.c on its own line. So it seems that also for this type of wrapping, the width setting is assumed to be set according to the screen size. VI. Conclusion Based on the above, I think that the notion that width can be set to determine where GDB wraps is incorrect. For both types of wrapping, it's essential that the width setting is set to the width of the screen. FWIW, we could introduce some "set wrap-width" setting, and then: - for "readline wrapping", do the _rl_term_autowrap overriding as described above (and ask readline maintainer to change status to public), and - for "gdb output wrapping" correctly detect (by comparing wrap-width with width) whether gdb can rely on the terminal to insert a linebreak, or whether gdb needs to do that itself. But I consider this outside the scope of this patch. VII. Readline wrapping, auto-detected screen size Alright, so then how do we fare with the auto-detected screen widths? [ COLUMNS == 40, TERM=xterm reporting width 40 and TERM=ansi reporting width 39. ] Let's try with xterm: ... $ TERM=xterm gdb (gdb) 7890123456789012345678901234567890 123 ... That looks as expected, wrapping occurs after 40 chars. Now, let's try with ansi: ... $ TERM=ansi gdb (gdb) 78901234567890123456789012345678 90123 ... It looks like wrapping occurred after 38, while readline should be capable of wrapping after 39 chars. This is caused by readline hiding the last column, twice. In more detail: - readline detects the screen width: 40, - readline hides the last column, setting the readline screen width to 39, - readline reports 39 to gdb as screen width, - gdb sets its width setting to 39, - gdb sets readline screen width to 39, - readline hides the last column, again, setting the readline screen width to 38. This is reported as PR cli/30346. VIII. gdb output wrapping, auto-detected screen size Say we set the terminal width to 56. With TERM=xterm, we have: ... /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/elf-init.c, /data/vries/hello.c, ... but with TERM=ansi: ... /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/elf-init.c, / data/vries/hello.c, ... So what happened here? With TERM=ansi, the width setting is auto-detected to 55, and gdb assumes the terminal inserts a line break there, which it doesn't because the terminal width is 56. This is reported as PR cli/30411. IX. Fix documented semantics of "set width" Fix the confusion about semantics of set width by updating doc and help text to make it more clear that "set width" means "set screen width". For the doc, we now have (using "whether" instead of "where"): ... the screen width setting determines whether to wrap lines of output. ... And for the help text: ... (gdb) help set width Set number of characters gdb thinks are in a line. This indicates the screen width. Auto-detected by default. Setting this to "unlimited" or zero prevents GDB from wrapping its output. (gdb) help show width Show number of characters gdb thinks are in a line. This indicates the screen width. Auto-detected by default. Setting this to "unlimited" or zero prevents GDB from wrapping its output. ... X. Fix PRs Fix both mentioned PRs by taking into account the hidden column when readline reports the screen width in init_page_info, and updating chars_per_line accordingly. Note that now we report the same width for both TERM=xterm and TERM=ansi, which is much clearer. The point where readline respectively expects or ensures wrapping is still indicated by "maint info screen", for xterm: ... Number of characters readline reports are in a line is 40. ... and ansi: ... Number of characters readline reports are in a line is 39. ... XI. Testing The new test-case gdb/testsuite/gdb.base/wrap-line.exp functions as regression test for PR cli/30346. I didn't manage to come up with a regression test for PR cli/30411. Perhaps that would be easier if we had a maintenance command that echoes its arguments while applying gdb output wrapping. Tested on x86_64-linux. PR cli/30346 PR cli/30411 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30346 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30411 --- gdb/doc/gdb.texinfo | 2 +- gdb/testsuite/gdb.base/wrap-line.exp | 114 +++++++++++++++++++++++++++ gdb/utils.c | 8 +- 3 files changed, 119 insertions(+), 5 deletions(-) create mode 100644 gdb/testsuite/gdb.base/wrap-line.exp diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index b21823d966d..8d89c8fb9bf 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -27059,7 +27059,7 @@ information output to the screen. To help you read all of it, output. Type @key{RET} when you want to see one more page of output, @kbd{q} to discard the remaining output, or @kbd{c} to continue without paging for the rest of the current command. Also, the screen -width setting determines when to wrap lines of output. Depending on +width setting determines whether to wrap lines of output. Depending on what is being printed, @value{GDBN} tries to break the line at a readable place, rather than simply letting it overflow onto the following line. diff --git a/gdb/testsuite/gdb.base/wrap-line.exp b/gdb/testsuite/gdb.base/wrap-line.exp new file mode 100644 index 00000000000..735b589dc45 --- /dev/null +++ b/gdb/testsuite/gdb.base/wrap-line.exp @@ -0,0 +1,114 @@ +# Copyright 2023 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Check that wrapping occurs at the expected location. + +# We set TERM on build, but we need to set it on host. That only works if +# build == host. +require {!is_remote host} + +# Test both ansi (no auto-wrap) and xterm (auto-wrap). +set terms {ansi xterm} + +foreach_with_prefix term $terms { + save_vars { env(TERM) INTERNAL_GDBFLAGS } { + + setenv TERM $term + + # Let's use the width as determined by readline to get correct + # wrapping in the auto-wrap case. Avoid "set width" argument. + set INTERNAL_GDBFLAGS \ + [string map {{-iex "set width 0"} ""} $INTERNAL_GDBFLAGS] + + # Avoid "set width" in default_gdb_start. + gdb_exit + gdb_spawn + } + + set test "initial prompt" + gdb_test_multiple "" $test { + -re "^$gdb_prompt $" { + pass "$test" + } + } + + if { ! [readline_is_used] } { + continue + } + + set gdb_width 0 + set readline_width 0 + set re1 "Number of characters gdb thinks are in a line is ($decimal)\\." + set re2 \ + "Number of characters readline reports are in a line is ($decimal)\\." + + gdb_test_multiple "maint info screen" "" { + -re -wrap "$re1\r\n$re2\r\n.*" { + set gdb_width $expect_out(1,string) + set readline_width $expect_out(2,string) + pass $gdb_test_name + } + } + + if { $gdb_width == 0 || $readline_width == 0 } { + continue + } + + if { $term == "xterm" } { + gdb_assert { $gdb_width == $readline_width } + } else { + gdb_assert { $gdb_width == [expr $readline_width + 1] } + } + + # New prompt, but avoid emitting a pass in order to avoid ending the line + # after the prompt in gdb.log. This make it a bit easier in gdb.log to + # understand where wrapping occurred. + gdb_test_multiple "print 1" "" { + -re -wrap " = 1" { + } + } + + # Take into account that the prompt also takes space. + set prefix [string length "(gdb) "] + set start [expr $prefix + 1] + + # Print chars without wrapping. + set i $start + while { 1 } { + send_gdb [expr $i % 10] + if { $i == $readline_width } { + break + } + incr i + } + + # Now print the first char we expect to wrap. + send_gdb "W" + + # Generate a prompt. + send_gdb "\003" + + # Note the difference between autowrap and no autowrap. In the autowrap + # case, readline doesn't emit a '\n', the terminal takes care of that. + if { $term == "xterm" } { + # xterm, autowrap. + set re "^$start\[^\r\n\]* \rWQuit" + } else { + # ansi, no autowrap. + set re "^$start\[^\r\n\]*\r\n\rWQuit" + } + + gdb_test "" $re "wrap" +} diff --git a/gdb/utils.c b/gdb/utils.c index e10198accd0..d16158eead8 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -1166,7 +1166,7 @@ init_page_info (void) readline_hidden_cols = _rl_term_autowrap ? 0 : 1; lines_per_page = rows; - chars_per_line = cols; + chars_per_line = cols + readline_hidden_cols; /* Readline should have fetched the termcap entry for us. Only try to use tgetnum function if rl_get_screen_size @@ -3686,9 +3686,9 @@ void _initialize_utils () { add_setshow_uinteger_cmd ("width", class_support, &chars_per_line, _("\ -Set number of characters where GDB should wrap lines of its output."), _("\ -Show number of characters where GDB should wrap lines of its output."), _("\ -This affects where GDB wraps its output to fit the screen width.\n\ +Set number of characters gdb thinks are in a line."), _("\ +Show number of characters gdb thinks are in a line."),_("\ +This indicates the screen width. Auto-detected by default.\n\ Setting this to \"unlimited\" or zero prevents GDB from wrapping its output."), set_width_command, show_chars_per_line, base-commit: 077a1f08485e88f3b234af1dbb8b907b16045e6a -- 2.35.3