* [PATCH 0/4] Fixes and tests related to new boxed hint startup text
@ 2025-12-04 16:38 Andrew Burgess
2025-12-04 16:38 ` [PATCH 1/4] gdb: remove some unnecessary code from print_gdb_hints Andrew Burgess
` (5 more replies)
0 siblings, 6 replies; 26+ messages in thread
From: Andrew Burgess @ 2025-12-04 16:38 UTC (permalink / raw)
To: gdb-patches; +Cc: Guinevere Larsen, Andrew Burgess
Started out with a minor fix for the new boxed hint start up text.
Then I wrote some tests. Which exposed some bugs. Which required
some additional fixes.
First 2 patches are clean up.
Final 2 are actual fixes.
Thanks,
Andrew
---
Andrew Burgess (4):
gdb: remove some unnecessary code from print_gdb_hints
gdb: small white space fix in print_gdb_hints
gdb: make get_chars_per_line return an unsigned value
gdb: fix crashes and weird output from new boxed hint text
gdb/cli-out.c | 4 +-
gdb/main.c | 5 +-
gdb/testsuite/gdb.base/startup-hints.exp | 144 +++++++++++++++++++++++
gdb/top.c | 28 +++--
gdb/utils.c | 3 +-
gdb/utils.h | 5 +-
6 files changed, 170 insertions(+), 19 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/startup-hints.exp
base-commit: a9304819899e9164c9550c601ed81d807df2c794
--
2.47.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 1/4] gdb: remove some unnecessary code from print_gdb_hints 2025-12-04 16:38 [PATCH 0/4] Fixes and tests related to new boxed hint startup text Andrew Burgess @ 2025-12-04 16:38 ` Andrew Burgess 2025-12-04 17:14 ` Patrick Monnerat 2025-12-04 16:38 ` [PATCH 2/4] gdb: small white space fix in print_gdb_hints Andrew Burgess ` (4 subsequent siblings) 5 siblings, 1 reply; 26+ messages in thread From: Andrew Burgess @ 2025-12-04 16:38 UTC (permalink / raw) To: gdb-patches; +Cc: Guinevere Larsen, Andrew Burgess I noticed some code in print_gdb_hints that is unused. There's a std::string that is never used, and a 'return' that is unnecessary, we can just drop out of the 'if' block and return at the end of the function. There should be no user visible changes after this commit. --- gdb/top.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/gdb/top.c b/gdb/top.c index 0798fea7a3b..27ea3d95aec 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -1444,11 +1444,8 @@ print_gdb_hints (struct ui_file *stream) lines, but URLs can't. */ if (width - 3 <= docs_url.length ()) { - std::string sep (width, '-'); for (string_file &msg : styled_msg) gdb_printf (stream, "%s\n", msg.c_str ()); - - return; } else { -- 2.47.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/4] gdb: remove some unnecessary code from print_gdb_hints 2025-12-04 16:38 ` [PATCH 1/4] gdb: remove some unnecessary code from print_gdb_hints Andrew Burgess @ 2025-12-04 17:14 ` Patrick Monnerat 0 siblings, 0 replies; 26+ messages in thread From: Patrick Monnerat @ 2025-12-04 17:14 UTC (permalink / raw) To: gdb-patches On 12/4/25 5:38 PM, Andrew Burgess wrote: > I noticed some code in print_gdb_hints that is unused. There's a > std::string that is never used, and a 'return' that is unnecessary, we > can just drop out of the 'if' block and return at the end of the > function. > > There should be no user visible changes after this commit. > --- > gdb/top.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/gdb/top.c b/gdb/top.c > index 0798fea7a3b..27ea3d95aec 100644 > --- a/gdb/top.c > +++ b/gdb/top.c > @@ -1444,11 +1444,8 @@ print_gdb_hints (struct ui_file *stream) > lines, but URLs can't. */ > if (width - 3 <= docs_url.length ()) > { > - std::string sep (width, '-'); The above line has already been removed by https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=06e470d8fc0ae0e83fe0977fdf8c011998980891 > for (string_file &msg : styled_msg) > gdb_printf (stream, "%s\n", msg.c_str ()); > - > - return; > } > else > { You're right for the return :-) ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 2/4] gdb: small white space fix in print_gdb_hints 2025-12-04 16:38 [PATCH 0/4] Fixes and tests related to new boxed hint startup text Andrew Burgess 2025-12-04 16:38 ` [PATCH 1/4] gdb: remove some unnecessary code from print_gdb_hints Andrew Burgess @ 2025-12-04 16:38 ` Andrew Burgess 2025-12-04 16:38 ` [PATCH 3/4] gdb: make get_chars_per_line return an unsigned value Andrew Burgess ` (3 subsequent siblings) 5 siblings, 0 replies; 26+ messages in thread From: Andrew Burgess @ 2025-12-04 16:38 UTC (permalink / raw) To: gdb-patches; +Cc: Guinevere Larsen, Andrew Burgess A small white space fix in print_gdb_hints. There should be no user visible changes after this commit. --- gdb/top.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gdb/top.c b/gdb/top.c index 27ea3d95aec..a8ce1a5321d 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -1449,7 +1449,7 @@ print_gdb_hints (struct ui_file *stream) } else { - std::string sep (width-2, '-'); + std::string sep (width - 2, '-'); if (emojis_ok ()) { -- 2.47.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 3/4] gdb: make get_chars_per_line return an unsigned value 2025-12-04 16:38 [PATCH 0/4] Fixes and tests related to new boxed hint startup text Andrew Burgess 2025-12-04 16:38 ` [PATCH 1/4] gdb: remove some unnecessary code from print_gdb_hints Andrew Burgess 2025-12-04 16:38 ` [PATCH 2/4] gdb: small white space fix in print_gdb_hints Andrew Burgess @ 2025-12-04 16:38 ` Andrew Burgess 2025-12-04 16:38 ` [PATCH 4/4] gdb: fix crashes and weird output from new boxed hint text Andrew Burgess ` (2 subsequent siblings) 5 siblings, 0 replies; 26+ messages in thread From: Andrew Burgess @ 2025-12-04 16:38 UTC (permalink / raw) To: gdb-patches; +Cc: Guinevere Larsen, Andrew Burgess I noticed that get_chars_per_line currently returns an 'int', but the value it is returning, from utils.c, is an 'unsigned int'. In some cases this can cause weird behaviour as an unlimited width terminal will have UINT_MAX characters per line, which will appear as negative when returned from get_chars_per_line. This has been the case since get_chars_per_line was added in commit: commit 2f2287318b33ddf855a692fcc191f6b25caf4644 Date: Wed Dec 16 18:18:40 2020 +0100 [gdb/cli] Add a progress meter Lets make get_chars_per_line return an unsigned value, and update all the uses of this function to hold the result in an unsigned variable. I ran into this issue when looking at print_gdb_hints (from top.c) where a very large get_chars_per_line() value would appear negative, and so the startup hints would be printed without a box when really they should have been boxed. There's no tests with this commit yet as print_gdb_hints has other bugs which will be fixed in the next commit. At this point I'll add some tests. --- gdb/cli-out.c | 4 ++-- gdb/top.c | 2 +- gdb/utils.c | 3 ++- gdb/utils.h | 5 +++-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/gdb/cli-out.c b/gdb/cli-out.c index 27a82f607eb..1e52734802c 100644 --- a/gdb/cli-out.c +++ b/gdb/cli-out.c @@ -298,7 +298,7 @@ cli_ui_out::do_progress_notify (const std::string &msg, const char *unit, double howmuch, double total) { - int chars_per_line = get_chars_per_line (); + unsigned int chars_per_line = get_chars_per_line (); struct ui_file *stream = get_unbuffered (m_streams.back ()); cli_progress_info &info (m_progress_info.back ()); @@ -385,7 +385,7 @@ void cli_ui_out::clear_progress_notify () { struct ui_file *stream = get_unbuffered (m_streams.back ()); - int chars_per_line = get_chars_per_line (); + unsigned int chars_per_line = get_chars_per_line (); scoped_restore save_pagination = make_scoped_restore (&pagination_enabled, false); diff --git a/gdb/top.c b/gdb/top.c index a8ce1a5321d..1aefe74b394 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -1414,7 +1414,7 @@ box_one_message (ui_file *stream, std::string message, int width) void print_gdb_hints (struct ui_file *stream) { - int width = get_chars_per_line (); + unsigned int width = get_chars_per_line (); /* Arbitrarily setting maximum width to 80 characters, so that things are big and visible but not overwhelming. */ diff --git a/gdb/utils.c b/gdb/utils.c index ffe0d639e3a..d322843925b 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -1548,9 +1548,10 @@ gdb_flush (struct ui_file *stream) /* See utils.h. */ -int +unsigned int get_chars_per_line () { + gdb_assert (chars_per_line > 0); return chars_per_line; } diff --git a/gdb/utils.h b/gdb/utils.h index 0e28f9424e5..6316547653e 100644 --- a/gdb/utils.h +++ b/gdb/utils.h @@ -157,9 +157,10 @@ extern void wrap_here (int); extern void reinitialize_more_filter (void); -/* Return the number of characters in a line. */ +/* Return the number of characters in a line. Will never be zero, but can + be UINT_MAX, which indicates unlimited characters per line. */ -extern int get_chars_per_line (); +extern unsigned int get_chars_per_line (); extern bool pagination_enabled; -- 2.47.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 4/4] gdb: fix crashes and weird output from new boxed hint text 2025-12-04 16:38 [PATCH 0/4] Fixes and tests related to new boxed hint startup text Andrew Burgess ` (2 preceding siblings ...) 2025-12-04 16:38 ` [PATCH 3/4] gdb: make get_chars_per_line return an unsigned value Andrew Burgess @ 2025-12-04 16:38 ` Andrew Burgess 2025-12-04 19:02 ` [PATCH 0/4] Fixes and tests related to new boxed hint startup text Guinevere Larsen 2025-12-04 19:47 ` [PATCH 0/2] " Andrew Burgess 5 siblings, 0 replies; 26+ messages in thread From: Andrew Burgess @ 2025-12-04 16:38 UTC (permalink / raw) To: gdb-patches; +Cc: Guinevere Larsen, Andrew Burgess After the commit: commit f6df8aa48f120b78f0670b429f8a3363020a47dc Date: Mon Sep 15 11:56:17 2025 -0300 gdb: Make startup message more user friendly I noticed, that when I start GDB with a file on the command line, I was seeing some stray '..'. Like this: $ gdb -nw -nh /tmp/hello.x GNU gdb (GDB) 18.0.50.20251202-git Copyright (C) 2025 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-pc-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <https://www.gnu.org/software/gdb/bugs/>. ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ Find the GDB manual online at: ┃ ┃ http://www.gnu.org/software/gdb/documentation/. ┃ ┃ For help, type "help". ┃ ┃ Type "apropos <word>" to search for commands related to <word> ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ .. Reading symbols from /tmp/hello.x... Notice the '..' after the boxed hint text, that's what I'm complaining about. Before the above commit these '..' used to appear after the line: Type "apropos <word>" to search for commands related to <word> The '..' are added to show that a file is being loaded, and that this might take some time. But we have the 'Reading symbols from ...' text to indicate this now, so I think these extra '..' are redundant. Lets just drop them. Looking at these I noticed that the line: Type "apropos <word>" to search for commands related to <word> is missing a period at the end, so I've added that in too. The above commit unfortunately, didn't include any tests, so I thought I'd write some to cover this fix.... and that uncovered a bug where the box around the startup hints could be corrupted: $ gdb -eiex 'set width 50' -nw -nh GNU gdb (GDB) 18.0.50.20251202-git Copyright (C) 2025 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-pc-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <https://www.gnu.org/software/gdb/bugs/>. ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ Find the GDB manual online at: ┃ ┃ http://www.gnu.org/software/gdb/documentation/. ┃ ┃ For help, type "help". ┃ ┃ Type "apropos <word>" to ┃ ┃ search for commands related to <word> ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ (gdb) This was caused by a mistake on the line where we choose whether to box or not. The line is currently: if (width - 3 <= docs_url.length ()) There are two problems here, the '3' should be '4'. The box adds 4 characters '| ' and ' |'. But also, the width can be very small, less than 4 even, which means that the width will wrap around and become very large. I plan to rewrite the line to: if (width < docs_url.length () + 1 + 4) The '+ 1' accounts for the period at the end of the URL line (which was previously handled by the '<=', and the '+ 4' accounts for the box borders. By making it a '+ 4' on the URL, rather than '- 4' from the width, we avoid underflow on the width. This is fine so long as the URL to our documentation doesn't approach UINT_MAX in length. Which I hope it never does. I've added a couple of asserts to print_gdb_hints to reflect things that must be true. The first is that get_chars_per_line never returns 0. And later on, I assert that 'width > 4' in a place where we are about to do 'width - 4'. If the assert triggers then underflow would have occurred. --- gdb/main.c | 5 +- gdb/testsuite/gdb.base/startup-hints.exp | 144 +++++++++++++++++++++++ gdb/top.c | 21 +++- 3 files changed, 161 insertions(+), 9 deletions(-) create mode 100644 gdb/testsuite/gdb.base/startup-hints.exp diff --git a/gdb/main.c b/gdb/main.c index 0fa2b0ecbe4..e4da715134b 100644 --- a/gdb/main.c +++ b/gdb/main.c @@ -1197,13 +1197,10 @@ captured_main_1 (struct captured_main_args *context) if (!quiet) { - /* Print all the junk at the top, with trailing "..." if we are - about to read a symbol file (possibly slowly). */ + /* Print the version, copyright information, and hint text. */ print_gdb_version (gdb_stdout, true); gdb_printf ("\n"); print_gdb_hints (gdb_stdout); - if (symarg) - gdb_printf (".."); gdb_printf ("\n"); gdb_flush (gdb_stdout); /* Force to screen during slow operations. */ diff --git a/gdb/testsuite/gdb.base/startup-hints.exp b/gdb/testsuite/gdb.base/startup-hints.exp new file mode 100644 index 00000000000..0addf3769a2 --- /dev/null +++ b/gdb/testsuite/gdb.base/startup-hints.exp @@ -0,0 +1,144 @@ +# Copyright 2025 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 <http://www.gnu.org/licenses/>. + +# Test the start up hint text. Check that it is boxed (or not) +# correctly. + +# Test assumes host == build. +require {!is_remote host} + +# Just use a simple empty 'main' function. +standard_testfile main.c + +if { [build_executable "build" $testfile $srcfile] != 0 } { + return +} + +# Return a single regexp string for the startup hint text when the +# terminal width is WIDTH. For narrow terminals GDB will just print +# the hint text. For wider terminals GDB places the hint into a box. +proc build_hint_re { width } { + # GDB imposes a maximum width of 80. + if { $width > 80 || $width == 0 } { + set width 80 + } + + if { $width > 50 } { + + # These lines are shorter than 50 characters, and so are never + # wrapped. + set msg { + {Find the GDB manual online at:} + {http://www.gnu.org/software/gdb/documentation/.} + {For help, type "help".} + } + + # The final line is wrapped based on the terminal width. + if { $width > 66 } { + lappend msg {Type "apropos <word>" to search for commands related to <word>.} + } elseif { $width > 58 } { + lappend msg {Type "apropos <word>" to search for commands related to} {<word>.} + } elseif { $width > 55 } { + lappend msg {Type "apropos <word>" to search for commands related} {to <word>.} + } elseif { $width > 47 } { + lappend msg {Type "apropos <word>" to search for commands} { related to <word>.} + } + + # Place the lines into a box, padding with whitespace so that + # the sides are correctly aligned. + set top_bottom "+[string repeat "-" [expr $width - 2]]+" + set lines [list $top_bottom] + foreach m $msg { + set space_count [expr $width - 4 - [string length $m]] + set spaces [string repeat " " $space_count] + lappend lines "| $m$spaces |" + } + lappend lines $top_bottom + } else { + # We tell GDB that the terminal width is WIDTH, but in reality + # the actual terminal width is unlimited. As such, GDB drops + # the box around the hint, but doesn't inject any additional + # newlines (it assumes the terminal will break the lines for + # us). This is why, despite the narrow terminal, we expect + # each line without any line breaks. + set lines {{Find the GDB manual online at:} \ + {http://www.gnu.org/software/gdb/documentation/.} \ + {For help, type "help".} \ + {Type "apropos <word>" to search for commands related to <word>.}} + } + + # Add blank line before and after current lines. + set lines [linsert $lines 0 ""] + lappend lines "" + + # Convert every line to a regexp. Also log the expected output + # for debugging. + set lines_re {} + verbose -log -- "Expected message format:" + foreach l $lines { + verbose -log -- "$l" + lappend lines_re [string_to_regexp $l] + } + + # Join the regexp together. + return [multi_line {*}$lines_re] +} + +# Tell GDB to start with a terminal width of WIDTH, then start GDB. +# Check that the hint text is formatted correctly. +proc_with_prefix test_for_hint_with_width { width load_exec } { + global GDBFLAGS + + save_vars { GDBFLAGS } { + append GDBFLAGS " -eiex \"set width $width\" -eiex \"set height 0\"" + if { $load_exec } { + append GDBFLAGS " \"$::binfile\"" + } + gdb_exit + gdb_spawn + } + + set hint_re [build_hint_re $width] + + if { $load_exec } { + append hint_re "\r\nReading symbols from [string_to_regexp $::binfile]\\.\\.\\." + } + + gdb_test "" $hint_re \ + "check for hint with width $width" +} + +save_vars { INTERNAL_GDBFLAGS } { + set INTERNAL_GDBFLAGS [string map {"-q" ""} $INTERNAL_GDBFLAGS] + + foreach_with_prefix load_exec { false true } { + + # Width 0 actually means unlimited. The other small sizes + # check that GDB doesn't trigger undefined behaviour by trying + # to create strings with a negative length. + for { set width 0 } { $width <= 5 } { incr width } { + test_for_hint_with_width $width $load_exec + } + + # These widths cover the point where we transition from using + # an unboxed hint to a boxed hint. + for { set width 45 } { $width <= 55 } { incr width } { + test_for_hint_with_width $width $load_exec + } + + # Very large widths are treated like a width of 80. + test_for_hint_with_width 100 $load_exec + } +} diff --git a/gdb/top.c b/gdb/top.c index 1aefe74b394..10366f98df1 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -1421,6 +1421,8 @@ print_gdb_hints (struct ui_file *stream) if (80 < width) width = 80; + gdb_assert (width > 0); + std::string docs_url = "http://www.gnu.org/software/gdb/documentation/"; std::array<string_file, 4> styled_msg { string_file (true), @@ -1435,20 +1437,29 @@ print_gdb_hints (struct ui_file *stream) gdb_printf (&styled_msg[2], _("For help, type \"%ps\"."), styled_string (command_style.style (), "help")); gdb_printf (&styled_msg[3], - _("Type \"%ps\" to search for commands related to <word>"), + _("Type \"%ps\" to search for commands related to <word>."), styled_string (command_style.style (), "apropos <word>")); /* If there isn't enough space to display the longest URL in a boxed - style, use the simple styling of a singular visual break. The longest - URL is used because the other messages may be broken into multiple - lines, but URLs can't. */ - if (width - 3 <= docs_url.length ()) + style, then don't use the box, the terminal will break the output + where needed. The longest URL is used because the other messages may + be broken into multiple lines, but URLs can't. + + The ' + 1' after the URL accounts for the period that is placed after + the URL. + + The '+ 4' accounts for the box and inner white space. We add the 4 to + the string length rather than subtract from the width as the width + could be less than 4, and we want to avoid wrap around. */ + if (width < docs_url.length () + 1 + 4) { for (string_file &msg : styled_msg) gdb_printf (stream, "%s\n", msg.c_str ()); } else { + gdb_assert (width > 4); + std::string sep (width - 2, '-'); if (emojis_ok ()) -- 2.47.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/4] Fixes and tests related to new boxed hint startup text 2025-12-04 16:38 [PATCH 0/4] Fixes and tests related to new boxed hint startup text Andrew Burgess ` (3 preceding siblings ...) 2025-12-04 16:38 ` [PATCH 4/4] gdb: fix crashes and weird output from new boxed hint text Andrew Burgess @ 2025-12-04 19:02 ` Guinevere Larsen 2025-12-04 19:32 ` Andrew Burgess 2025-12-04 19:47 ` [PATCH 0/2] " Andrew Burgess 5 siblings, 1 reply; 26+ messages in thread From: Guinevere Larsen @ 2025-12-04 19:02 UTC (permalink / raw) To: Andrew Burgess, gdb-patches On 12/4/25 1:38 PM, Andrew Burgess wrote: > Started out with a minor fix for the new boxed hint start up text. > Then I wrote some tests. Which exposed some bugs. Which required > some additional fixes. > > First 2 patches are clean up. > > Final 2 are actual fixes. > > Thanks, > Andrew Hi Andrew! I took a look over the patches, and everything looks right. Thanks for the quick fixes! Reviewed-By: Guinevere Larsen <guinevere@redhat.com> -- Cheers, Guinevere Larsen It/she > > --- > > Andrew Burgess (4): > gdb: remove some unnecessary code from print_gdb_hints > gdb: small white space fix in print_gdb_hints > gdb: make get_chars_per_line return an unsigned value > gdb: fix crashes and weird output from new boxed hint text > > gdb/cli-out.c | 4 +- > gdb/main.c | 5 +- > gdb/testsuite/gdb.base/startup-hints.exp | 144 +++++++++++++++++++++++ > gdb/top.c | 28 +++-- > gdb/utils.c | 3 +- > gdb/utils.h | 5 +- > 6 files changed, 170 insertions(+), 19 deletions(-) > create mode 100644 gdb/testsuite/gdb.base/startup-hints.exp > > > base-commit: a9304819899e9164c9550c601ed81d807df2c794 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/4] Fixes and tests related to new boxed hint startup text 2025-12-04 19:02 ` [PATCH 0/4] Fixes and tests related to new boxed hint startup text Guinevere Larsen @ 2025-12-04 19:32 ` Andrew Burgess 0 siblings, 0 replies; 26+ messages in thread From: Andrew Burgess @ 2025-12-04 19:32 UTC (permalink / raw) To: Guinevere Larsen, gdb-patches Guinevere Larsen <guinevere@redhat.com> writes: > On 12/4/25 1:38 PM, Andrew Burgess wrote: >> Started out with a minor fix for the new boxed hint start up text. >> Then I wrote some tests. Which exposed some bugs. Which required >> some additional fixes. >> >> First 2 patches are clean up. >> >> Final 2 are actual fixes. >> >> Thanks, >> Andrew > > Hi Andrew! > > I took a look over the patches, and everything looks right. Thanks for > the quick fixes! > > Reviewed-By: Guinevere Larsen <guinevere@redhat.com> I pushed the first two patches. I need to figure the best way to integrate these fixes with the patch Patrick pushed, so there might be a v2 tomorrow. Thanks, Andrew ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 0/2] Fixes and tests related to new boxed hint startup text 2025-12-04 16:38 [PATCH 0/4] Fixes and tests related to new boxed hint startup text Andrew Burgess ` (4 preceding siblings ...) 2025-12-04 19:02 ` [PATCH 0/4] Fixes and tests related to new boxed hint startup text Guinevere Larsen @ 2025-12-04 19:47 ` Andrew Burgess 2025-12-04 19:47 ` [PATCH 1/2] gdb: make get_chars_per_line return an unsigned value Andrew Burgess ` (2 more replies) 5 siblings, 3 replies; 26+ messages in thread From: Andrew Burgess @ 2025-12-04 19:47 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess Merged the first two patches from V1. Here are the second two patches rebased. In v2: - Resolved conflicts with an upstream patch that touched the same area. - Work around a regression in debuginfod tests caused by changing get_chars_per_line returning unsigned. Thanks, Andrew --- Andrew Burgess (2): gdb: make get_chars_per_line return an unsigned value gdb: fix crashes and weird output from new boxed hint text gdb/cli-out.c | 10 +- gdb/main.c | 5 +- gdb/testsuite/gdb.base/startup-hints.exp | 144 +++++++++++++++++++++++ gdb/top.c | 23 +++- gdb/utils.c | 3 +- gdb/utils.h | 5 +- 6 files changed, 173 insertions(+), 17 deletions(-) create mode 100644 gdb/testsuite/gdb.base/startup-hints.exp base-commit: c54f3d5be665d325fb37cf486282fc1916976618 -- 2.47.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/2] gdb: make get_chars_per_line return an unsigned value 2025-12-04 19:47 ` [PATCH 0/2] " Andrew Burgess @ 2025-12-04 19:47 ` Andrew Burgess 2025-12-04 19:47 ` [PATCH 2/2] gdb: fix crashes and weird output from new boxed hint text Andrew Burgess 2025-12-05 19:53 ` [PATCHv3 0/3] Fixes and tests related to new boxed hint startup text Andrew Burgess 2 siblings, 0 replies; 26+ messages in thread From: Andrew Burgess @ 2025-12-04 19:47 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess, Guinevere Larsen I noticed that get_chars_per_line currently returns an 'int', but the value it is returning, from utils.c, is an 'unsigned int'. In some cases this can cause weird behaviour as an unlimited width terminal will have UINT_MAX characters per line, which will appear as negative when returned from get_chars_per_line. This has been the case since get_chars_per_line was added in commit: commit 2f2287318b33ddf855a692fcc191f6b25caf4644 Date: Wed Dec 16 18:18:40 2020 +0100 [gdb/cli] Add a progress meter Lets make get_chars_per_line return an unsigned value, and update all the uses of this function to hold the result in an unsigned variable. I ran into this issue when looking at print_gdb_hints (from top.c) where a very large get_chars_per_line() value would appear negative, and so the startup hints would be printed without a box when really they should have been boxed. Someone else noticed this problem while I was building this patch, and pushed commit: commit 06e470d8fc0ae0e83fe0977fdf8c011998980891 Date: Sat Nov 29 15:48:55 2025 +0100 gdb: handle unlimited screen width case in print_gdb_hints this commit reverts that earlier commit in favour of fixing the type of 'width'. There are other bugs in this code relating to how 'width' is handled, but they are fixed in the next commit. Additionally, in cli-out.c I've had to add some additional code so that the progress bars are disabled when the screen width is unlimited. Previously, an unlimited screen width would appear as a negative value, which was less than MIN_CHARS_PER_LINE, so the progress bars would be disabled. Our existing debuginfod tests rely on this. To minimise disruption to the tests, for now, I've updated the code in cli-out.c so that we specifically disable progress bars when the screen width is unlimited, as it is during our tests. It would be better, in the future, if we had a separate setting to disable progress bars, and the testsuite could use this. There's no tests with this commit yet as print_gdb_hints has other bugs which will be fixed in the next commit. At this point I'll add some tests. Reviewed-By: Guinevere Larsen <guinevere@redhat.com> --- gdb/cli-out.c | 10 ++++++---- gdb/top.c | 10 +++++----- gdb/utils.c | 3 ++- gdb/utils.h | 5 +++-- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/gdb/cli-out.c b/gdb/cli-out.c index 27a82f607eb..137ff13ef5d 100644 --- a/gdb/cli-out.c +++ b/gdb/cli-out.c @@ -298,7 +298,7 @@ cli_ui_out::do_progress_notify (const std::string &msg, const char *unit, double howmuch, double total) { - int chars_per_line = get_chars_per_line (); + unsigned int chars_per_line = get_chars_per_line (); struct ui_file *stream = get_unbuffered (m_streams.back ()); cli_progress_info &info (m_progress_info.back ()); @@ -322,7 +322,8 @@ cli_ui_out::do_progress_notify (const std::string &msg, } if (info.state != progress_update::BAR - || chars_per_line < MIN_CHARS_PER_LINE) + || chars_per_line < MIN_CHARS_PER_LINE + || chars_per_line == UINT_MAX) return; if (total > 0 && howmuch >= 0 && howmuch <= 1.0) @@ -385,14 +386,15 @@ void cli_ui_out::clear_progress_notify () { struct ui_file *stream = get_unbuffered (m_streams.back ()); - int chars_per_line = get_chars_per_line (); + unsigned int chars_per_line = get_chars_per_line (); scoped_restore save_pagination = make_scoped_restore (&pagination_enabled, false); if (!stream->isatty () || !current_ui->input_interactive_p () - || chars_per_line < MIN_CHARS_PER_LINE) + || chars_per_line < MIN_CHARS_PER_LINE + || chars_per_line == UINT_MAX) return; if (chars_per_line > MAX_CHARS_PER_LINE) diff --git a/gdb/top.c b/gdb/top.c index 1214101ad18..1aefe74b394 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -1414,7 +1414,7 @@ box_one_message (ui_file *stream, std::string message, int width) void print_gdb_hints (struct ui_file *stream) { - int width = get_chars_per_line (); + unsigned int width = get_chars_per_line (); /* Arbitrarily setting maximum width to 80 characters, so that things are big and visible but not overwhelming. */ @@ -1439,10 +1439,10 @@ print_gdb_hints (struct ui_file *stream) styled_string (command_style.style (), "apropos <word>")); /* If there isn't enough space to display the longest URL in a boxed - style or if screen width is unlimited, use the simple styling of a - singular visual break. The longest URL is used because the other - messages may be broken into multiple lines, but URLs can't. */ - if (width - 3 <= (int) docs_url.length ()) + style, use the simple styling of a singular visual break. The longest + URL is used because the other messages may be broken into multiple + lines, but URLs can't. */ + if (width - 3 <= docs_url.length ()) { for (string_file &msg : styled_msg) gdb_printf (stream, "%s\n", msg.c_str ()); diff --git a/gdb/utils.c b/gdb/utils.c index ffe0d639e3a..d322843925b 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -1548,9 +1548,10 @@ gdb_flush (struct ui_file *stream) /* See utils.h. */ -int +unsigned int get_chars_per_line () { + gdb_assert (chars_per_line > 0); return chars_per_line; } diff --git a/gdb/utils.h b/gdb/utils.h index 0e28f9424e5..6316547653e 100644 --- a/gdb/utils.h +++ b/gdb/utils.h @@ -157,9 +157,10 @@ extern void wrap_here (int); extern void reinitialize_more_filter (void); -/* Return the number of characters in a line. */ +/* Return the number of characters in a line. Will never be zero, but can + be UINT_MAX, which indicates unlimited characters per line. */ -extern int get_chars_per_line (); +extern unsigned int get_chars_per_line (); extern bool pagination_enabled; -- 2.47.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 2/2] gdb: fix crashes and weird output from new boxed hint text 2025-12-04 19:47 ` [PATCH 0/2] " Andrew Burgess 2025-12-04 19:47 ` [PATCH 1/2] gdb: make get_chars_per_line return an unsigned value Andrew Burgess @ 2025-12-04 19:47 ` Andrew Burgess 2025-12-05 7:06 ` Eli Zaretskii 2025-12-05 19:53 ` [PATCHv3 0/3] Fixes and tests related to new boxed hint startup text Andrew Burgess 2 siblings, 1 reply; 26+ messages in thread From: Andrew Burgess @ 2025-12-04 19:47 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess, Guinevere Larsen After the commit: commit f6df8aa48f120b78f0670b429f8a3363020a47dc Date: Mon Sep 15 11:56:17 2025 -0300 gdb: Make startup message more user friendly I noticed, that when I start GDB with a file on the command line, I was seeing some stray '..'. Like this: $ gdb -nw -nh /tmp/hello.x GNU gdb (GDB) 18.0.50.20251202-git Copyright (C) 2025 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-pc-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <https://www.gnu.org/software/gdb/bugs/>. ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ Find the GDB manual online at: ┃ ┃ http://www.gnu.org/software/gdb/documentation/. ┃ ┃ For help, type "help". ┃ ┃ Type "apropos <word>" to search for commands related to <word> ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ .. Reading symbols from /tmp/hello.x... Notice the '..' after the boxed hint text, that's what I'm complaining about. Before the above commit these '..' used to appear after the line: Type "apropos <word>" to search for commands related to <word> The '..' are added to show that a file is being loaded, and that this might take some time. But we have the 'Reading symbols from ...' text to indicate this now, so I think these extra '..' are redundant. Lets just drop them. Looking at these I noticed that the line: Type "apropos <word>" to search for commands related to <word> is missing a period at the end, so I've added that in too. The above commit unfortunately, didn't include any tests, so I thought I'd write some to cover this fix.... and that uncovered a bug where the box around the startup hints could be corrupted: $ gdb -eiex 'set width 50' -nw -nh GNU gdb (GDB) 18.0.50.20251202-git Copyright (C) 2025 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-pc-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <https://www.gnu.org/software/gdb/bugs/>. ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ Find the GDB manual online at: ┃ ┃ http://www.gnu.org/software/gdb/documentation/. ┃ ┃ For help, type "help". ┃ ┃ Type "apropos <word>" to ┃ ┃ search for commands related to <word> ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ (gdb) This was caused by a mistake on the line where we choose whether to box or not. The line is currently: if (width - 3 <= docs_url.length ()) There are two problems here, the '3' should be '4'. The box adds 4 characters '| ' and ' |'. But also, the width can be very small, less than 4 even, which means that the width will wrap around and become very large. I plan to rewrite the line to: if (width < docs_url.length () + 1 + 4) The '+ 1' accounts for the period at the end of the URL line (which was previously handled by the '<=', and the '+ 4' accounts for the box borders. By making it a '+ 4' on the URL, rather than '- 4' from the width, we avoid underflow on the width. This is fine so long as the URL to our documentation doesn't approach UINT_MAX in length. Which I hope it never does. I've added a couple of asserts to print_gdb_hints to reflect things that must be true. The first is that get_chars_per_line never returns 0. And later on, I assert that 'width > 4' in a place where we are about to do 'width - 4'. If the assert triggers then underflow would have occurred. Reviewed-By: Guinevere Larsen <guinevere@redhat.com> --- gdb/main.c | 5 +- gdb/testsuite/gdb.base/startup-hints.exp | 144 +++++++++++++++++++++++ gdb/top.c | 21 +++- 3 files changed, 161 insertions(+), 9 deletions(-) create mode 100644 gdb/testsuite/gdb.base/startup-hints.exp diff --git a/gdb/main.c b/gdb/main.c index 0fa2b0ecbe4..e4da715134b 100644 --- a/gdb/main.c +++ b/gdb/main.c @@ -1197,13 +1197,10 @@ captured_main_1 (struct captured_main_args *context) if (!quiet) { - /* Print all the junk at the top, with trailing "..." if we are - about to read a symbol file (possibly slowly). */ + /* Print the version, copyright information, and hint text. */ print_gdb_version (gdb_stdout, true); gdb_printf ("\n"); print_gdb_hints (gdb_stdout); - if (symarg) - gdb_printf (".."); gdb_printf ("\n"); gdb_flush (gdb_stdout); /* Force to screen during slow operations. */ diff --git a/gdb/testsuite/gdb.base/startup-hints.exp b/gdb/testsuite/gdb.base/startup-hints.exp new file mode 100644 index 00000000000..0addf3769a2 --- /dev/null +++ b/gdb/testsuite/gdb.base/startup-hints.exp @@ -0,0 +1,144 @@ +# Copyright 2025 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 <http://www.gnu.org/licenses/>. + +# Test the start up hint text. Check that it is boxed (or not) +# correctly. + +# Test assumes host == build. +require {!is_remote host} + +# Just use a simple empty 'main' function. +standard_testfile main.c + +if { [build_executable "build" $testfile $srcfile] != 0 } { + return +} + +# Return a single regexp string for the startup hint text when the +# terminal width is WIDTH. For narrow terminals GDB will just print +# the hint text. For wider terminals GDB places the hint into a box. +proc build_hint_re { width } { + # GDB imposes a maximum width of 80. + if { $width > 80 || $width == 0 } { + set width 80 + } + + if { $width > 50 } { + + # These lines are shorter than 50 characters, and so are never + # wrapped. + set msg { + {Find the GDB manual online at:} + {http://www.gnu.org/software/gdb/documentation/.} + {For help, type "help".} + } + + # The final line is wrapped based on the terminal width. + if { $width > 66 } { + lappend msg {Type "apropos <word>" to search for commands related to <word>.} + } elseif { $width > 58 } { + lappend msg {Type "apropos <word>" to search for commands related to} {<word>.} + } elseif { $width > 55 } { + lappend msg {Type "apropos <word>" to search for commands related} {to <word>.} + } elseif { $width > 47 } { + lappend msg {Type "apropos <word>" to search for commands} { related to <word>.} + } + + # Place the lines into a box, padding with whitespace so that + # the sides are correctly aligned. + set top_bottom "+[string repeat "-" [expr $width - 2]]+" + set lines [list $top_bottom] + foreach m $msg { + set space_count [expr $width - 4 - [string length $m]] + set spaces [string repeat " " $space_count] + lappend lines "| $m$spaces |" + } + lappend lines $top_bottom + } else { + # We tell GDB that the terminal width is WIDTH, but in reality + # the actual terminal width is unlimited. As such, GDB drops + # the box around the hint, but doesn't inject any additional + # newlines (it assumes the terminal will break the lines for + # us). This is why, despite the narrow terminal, we expect + # each line without any line breaks. + set lines {{Find the GDB manual online at:} \ + {http://www.gnu.org/software/gdb/documentation/.} \ + {For help, type "help".} \ + {Type "apropos <word>" to search for commands related to <word>.}} + } + + # Add blank line before and after current lines. + set lines [linsert $lines 0 ""] + lappend lines "" + + # Convert every line to a regexp. Also log the expected output + # for debugging. + set lines_re {} + verbose -log -- "Expected message format:" + foreach l $lines { + verbose -log -- "$l" + lappend lines_re [string_to_regexp $l] + } + + # Join the regexp together. + return [multi_line {*}$lines_re] +} + +# Tell GDB to start with a terminal width of WIDTH, then start GDB. +# Check that the hint text is formatted correctly. +proc_with_prefix test_for_hint_with_width { width load_exec } { + global GDBFLAGS + + save_vars { GDBFLAGS } { + append GDBFLAGS " -eiex \"set width $width\" -eiex \"set height 0\"" + if { $load_exec } { + append GDBFLAGS " \"$::binfile\"" + } + gdb_exit + gdb_spawn + } + + set hint_re [build_hint_re $width] + + if { $load_exec } { + append hint_re "\r\nReading symbols from [string_to_regexp $::binfile]\\.\\.\\." + } + + gdb_test "" $hint_re \ + "check for hint with width $width" +} + +save_vars { INTERNAL_GDBFLAGS } { + set INTERNAL_GDBFLAGS [string map {"-q" ""} $INTERNAL_GDBFLAGS] + + foreach_with_prefix load_exec { false true } { + + # Width 0 actually means unlimited. The other small sizes + # check that GDB doesn't trigger undefined behaviour by trying + # to create strings with a negative length. + for { set width 0 } { $width <= 5 } { incr width } { + test_for_hint_with_width $width $load_exec + } + + # These widths cover the point where we transition from using + # an unboxed hint to a boxed hint. + for { set width 45 } { $width <= 55 } { incr width } { + test_for_hint_with_width $width $load_exec + } + + # Very large widths are treated like a width of 80. + test_for_hint_with_width 100 $load_exec + } +} diff --git a/gdb/top.c b/gdb/top.c index 1aefe74b394..10366f98df1 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -1421,6 +1421,8 @@ print_gdb_hints (struct ui_file *stream) if (80 < width) width = 80; + gdb_assert (width > 0); + std::string docs_url = "http://www.gnu.org/software/gdb/documentation/"; std::array<string_file, 4> styled_msg { string_file (true), @@ -1435,20 +1437,29 @@ print_gdb_hints (struct ui_file *stream) gdb_printf (&styled_msg[2], _("For help, type \"%ps\"."), styled_string (command_style.style (), "help")); gdb_printf (&styled_msg[3], - _("Type \"%ps\" to search for commands related to <word>"), + _("Type \"%ps\" to search for commands related to <word>."), styled_string (command_style.style (), "apropos <word>")); /* If there isn't enough space to display the longest URL in a boxed - style, use the simple styling of a singular visual break. The longest - URL is used because the other messages may be broken into multiple - lines, but URLs can't. */ - if (width - 3 <= docs_url.length ()) + style, then don't use the box, the terminal will break the output + where needed. The longest URL is used because the other messages may + be broken into multiple lines, but URLs can't. + + The ' + 1' after the URL accounts for the period that is placed after + the URL. + + The '+ 4' accounts for the box and inner white space. We add the 4 to + the string length rather than subtract from the width as the width + could be less than 4, and we want to avoid wrap around. */ + if (width < docs_url.length () + 1 + 4) { for (string_file &msg : styled_msg) gdb_printf (stream, "%s\n", msg.c_str ()); } else { + gdb_assert (width > 4); + std::string sep (width - 2, '-'); if (emojis_ok ()) -- 2.47.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: fix crashes and weird output from new boxed hint text 2025-12-04 19:47 ` [PATCH 2/2] gdb: fix crashes and weird output from new boxed hint text Andrew Burgess @ 2025-12-05 7:06 ` Eli Zaretskii 2025-12-05 10:17 ` Andrew Burgess 0 siblings, 1 reply; 26+ messages in thread From: Eli Zaretskii @ 2025-12-05 7:06 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches, guinevere > From: Andrew Burgess <aburgess@redhat.com> > Cc: Andrew Burgess <aburgess@redhat.com>, > Guinevere Larsen <guinevere@redhat.com> > Date: Thu, 4 Dec 2025 19:47:25 +0000 > > After the commit: > > commit f6df8aa48f120b78f0670b429f8a3363020a47dc > Date: Mon Sep 15 11:56:17 2025 -0300 > > gdb: Make startup message more user friendly > > I noticed, that when I start GDB with a file on the command line, I > was seeing some stray '..'. Like this: > > $ gdb -nw -nh /tmp/hello.x > GNU gdb (GDB) 18.0.50.20251202-git > Copyright (C) 2025 Free Software Foundation, Inc. > License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> > This is free software: you are free to change and redistribute it. > There is NO WARRANTY, to the extent permitted by law. > Type "show copying" and "show warranty" for details. > This GDB was configured as "x86_64-pc-linux-gnu". > Type "show configuration" for configuration details. > For bug reporting instructions, please see: > <https://www.gnu.org/software/gdb/bugs/>. > > ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ > ┃ Find the GDB manual online at: ┃ > ┃ http://www.gnu.org/software/gdb/documentation/. ┃ > ┃ For help, type "help". ┃ > ┃ Type "apropos <word>" to search for commands related to <word> ┃ > ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ > .. > Reading symbols from /tmp/hello.x... > > Notice the '..' after the boxed hint text, that's what I'm complaining > about. > > Before the above commit these '..' used to appear after the line: > > Type "apropos <word>" to search for commands related to <word> > > The '..' are added to show that a file is being loaded, and that this > might take some time. But we have the 'Reading symbols from ...' text > to indicate this now, so I think these extra '..' are redundant. Lets > just drop them. But the single period will still be there, right? IOW, that line will, after applying the patch, look like this: Type "apropos word" to search for commands related to "word". with the single period at its end, right? What you wrote above could be interpreted as meaning there will be no periods at all, which IMO would be sub-optimal, and inconsistent with the line we show before, which says For help, type "help". and correctly ends in a period. Thanks. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: fix crashes and weird output from new boxed hint text 2025-12-05 7:06 ` Eli Zaretskii @ 2025-12-05 10:17 ` Andrew Burgess 2025-12-05 11:19 ` Eli Zaretskii 0 siblings, 1 reply; 26+ messages in thread From: Andrew Burgess @ 2025-12-05 10:17 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gdb-patches, guinevere Eli Zaretskii <eliz@gnu.org> writes: >> From: Andrew Burgess <aburgess@redhat.com> >> Cc: Andrew Burgess <aburgess@redhat.com>, >> Guinevere Larsen <guinevere@redhat.com> >> Date: Thu, 4 Dec 2025 19:47:25 +0000 >> >> After the commit: >> >> commit f6df8aa48f120b78f0670b429f8a3363020a47dc >> Date: Mon Sep 15 11:56:17 2025 -0300 >> >> gdb: Make startup message more user friendly >> >> I noticed, that when I start GDB with a file on the command line, I >> was seeing some stray '..'. Like this: >> >> $ gdb -nw -nh /tmp/hello.x >> GNU gdb (GDB) 18.0.50.20251202-git >> Copyright (C) 2025 Free Software Foundation, Inc. >> License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> >> This is free software: you are free to change and redistribute it. >> There is NO WARRANTY, to the extent permitted by law. >> Type "show copying" and "show warranty" for details. >> This GDB was configured as "x86_64-pc-linux-gnu". >> Type "show configuration" for configuration details. >> For bug reporting instructions, please see: >> <https://www.gnu.org/software/gdb/bugs/>. >> >> ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ >> ┃ Find the GDB manual online at: ┃ >> ┃ http://www.gnu.org/software/gdb/documentation/. ┃ >> ┃ For help, type "help". ┃ >> ┃ Type "apropos <word>" to search for commands related to <word> ┃ >> ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ >> .. >> Reading symbols from /tmp/hello.x... >> >> Notice the '..' after the boxed hint text, that's what I'm complaining >> about. >> >> Before the above commit these '..' used to appear after the line: >> >> Type "apropos <word>" to search for commands related to <word> >> >> The '..' are added to show that a file is being loaded, and that this >> might take some time. But we have the 'Reading symbols from ...' text >> to indicate this now, so I think these extra '..' are redundant. Lets >> just drop them. > > But the single period will still be there, right? IOW, that line > will, after applying the patch, look like this: > > Type "apropos word" to search for commands related to "word". > > with the single period at its end, right? What you wrote above could > be interpreted as meaning there will be no periods at all, which IMO > would be sub-optimal, and inconsistent with the line we show before, > which says > > For help, type "help". > > and correctly ends in a period. Yes, I'm adding the period back. The next part of the commit message says: Looking at these I noticed that the line: Type "apropos <word>" to search for commands related to <word> is missing a period at the end, so I've added that in too. I'll reword these two parts of the commit message to make it clearer that I'm removing the stray '..' and adding a single '.' that is missing from the line in question. The final result looks like this: ... snip ... <https://www.gnu.org/software/gdb/bugs/>. ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ Find the GDB manual online at: ┃ ┃ http://www.gnu.org/software/gdb/documentation/. ┃ ┃ For help, type "help". ┃ ┃ Type "apropos <word>" to search for commands related to <word>. ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ Reading symbols from /tmp/hello.x... (gdb) Thanks, Andrew ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] gdb: fix crashes and weird output from new boxed hint text 2025-12-05 10:17 ` Andrew Burgess @ 2025-12-05 11:19 ` Eli Zaretskii 0 siblings, 0 replies; 26+ messages in thread From: Eli Zaretskii @ 2025-12-05 11:19 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches, guinevere > From: Andrew Burgess <aburgess@redhat.com> > Cc: gdb-patches@sourceware.org, guinevere@redhat.com > Date: Fri, 05 Dec 2025 10:17:18 +0000 > > The final result looks like this: > > ... snip ... > <https://www.gnu.org/software/gdb/bugs/>. > > ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ > ┃ Find the GDB manual online at: ┃ > ┃ http://www.gnu.org/software/gdb/documentation/. ┃ > ┃ For help, type "help". ┃ > ┃ Type "apropos <word>" to search for commands related to <word>. ┃ > ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ > > Reading symbols from /tmp/hello.x... > (gdb) LGTM, thanks. ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCHv3 0/3] Fixes and tests related to new boxed hint startup text 2025-12-04 19:47 ` [PATCH 0/2] " Andrew Burgess 2025-12-04 19:47 ` [PATCH 1/2] gdb: make get_chars_per_line return an unsigned value Andrew Burgess 2025-12-04 19:47 ` [PATCH 2/2] gdb: fix crashes and weird output from new boxed hint text Andrew Burgess @ 2025-12-05 19:53 ` Andrew Burgess 2025-12-05 19:53 ` [PATCHv3 1/3] gdb: make get_chars_per_line return an unsigned value Andrew Burgess ` (2 more replies) 2 siblings, 3 replies; 26+ messages in thread From: Andrew Burgess @ 2025-12-05 19:53 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess Some fixes for the new boxed hint text. But this grew slightly as one of the issues is caused by a bug elsewhere in GDB. And fixing that exposed problem in a third area of GDB. Sigh. In v3: - Rewrote commit messages on first 2 patches. - Another minor tweak in patch #1, the debuginfod tests actually pass with this patch now. - Added a third patch, this adds a new switch to disable progress bars, which is then used by the debuginfod tests. Thanks, Andrew --- Andrew Burgess (3): gdb: make get_chars_per_line return an unsigned value gdb: fix crashes and weird output from new boxed hint text WIP: disable progress bars setting gdb/NEWS | 6 + gdb/cli-out.c | 81 ++++++++-- gdb/doc/gdb.texinfo | 15 ++ gdb/main.c | 5 +- gdb/testsuite/gdb.base/startup-hints.exp | 144 ++++++++++++++++++ .../build-id-no-debug-warning.exp | 1 + .../gdb.debuginfod/corefile-mapped-file.exp | 1 + gdb/testsuite/gdb.debuginfod/crc_mismatch.exp | 1 + .../gdb.debuginfod/fetch_src_and_symbols.exp | 4 + .../gdb.debuginfod/solib-with-soname.exp | 1 + gdb/top.c | 23 ++- gdb/utils.c | 3 +- gdb/utils.h | 5 +- 13 files changed, 268 insertions(+), 22 deletions(-) create mode 100644 gdb/testsuite/gdb.base/startup-hints.exp base-commit: 36a4bbcec4b31ff7a5fb8e5a2e20acef9344c810 -- 2.47.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCHv3 1/3] gdb: make get_chars_per_line return an unsigned value 2025-12-05 19:53 ` [PATCHv3 0/3] Fixes and tests related to new boxed hint startup text Andrew Burgess @ 2025-12-05 19:53 ` Andrew Burgess 2025-12-10 16:48 ` Tom Tromey 2025-12-05 19:53 ` [PATCHv3 2/3] gdb: fix crashes and weird output from new boxed hint text Andrew Burgess 2025-12-05 19:53 ` [PATCHv3 3/3] WIP: disable progress bars setting Andrew Burgess 2 siblings, 1 reply; 26+ messages in thread From: Andrew Burgess @ 2025-12-05 19:53 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess, Guinevere Larsen I noticed that get_chars_per_line currently returns an 'int', but the value it is returning, from utils.c, is an 'unsigned int'. In some cases this can cause weird behaviour as an unlimited width terminal will have UINT_MAX characters per line, which will appear as negative when returned from get_chars_per_line. This has been the case since get_chars_per_line was added in commit: commit 2f2287318b33ddf855a692fcc191f6b25caf4644 Date: Wed Dec 16 18:18:40 2020 +0100 [gdb/cli] Add a progress meter Lets make get_chars_per_line return an unsigned value, and update all the uses of this function to hold the result in an unsigned variable. I ran into this issue when looking at print_gdb_hints (from top.c) where a very large get_chars_per_line() value would appear negative, and so the startup hints would be printed without a box when really they should have been boxed. Someone else noticed this problem while I was building this patch, and pushed commit: commit 06e470d8fc0ae0e83fe0977fdf8c011998980891 Date: Sat Nov 29 15:48:55 2025 +0100 gdb: handle unlimited screen width case in print_gdb_hints This commit works around the signed / unsigned confusion entirely within print_gdb_hints by adding a case to 'int' in one place. The change I present here reverts parts of 06e470d8fc0a in favour of fixing the type of WIDTH within print_gdb_hints. It is worth nothing that there are other bugs in print_gdb_hints relating to how WIDTH is handled, but these are fixed in the next commit. By updating the return type of get_chars_per_line, I ran into some issues in cli-out.c relating to how progress bars are handled. The existing code includes code like: if (!stream->isatty () || !current_ui->input_interactive_p () || chars_per_line < MIN_CHARS_PER_LINE) return; The early return here triggers when progress bars should not be printed. Notice that when the terminal width is unlimited, CHARS_PER_LINE will appear negative, and so the early return will trigger. It turns out that our testsuite depends on this; the debuginfod tests don't expect to see a progress bar, and they don't because within the tests we set the width to unlimited. By "fixing" the type of CHARS_PER_LINE to 'unsigned int', an unlimited width terminal no longer triggers the early return, and GDB starts trying to print a progress bar in our debuginfod tests, which cause the tests to fail. I think the real fix is to add a new flag to allow progress bars to be disabled, the tests can then use this. I will add just such a flag at the end of this series. For now though, I propose adding a bodge. If CHARS_PER_LINE is UINT_MAX, then we should act as if progress bars are disabled. The above code now becomes: if (!stream->isatty () || !current_ui->input_interactive_p () || chars_per_line < MIN_CHARS_PER_LINE || chars_per_line == UINT_MAX) return; This change is in cli_ui_out::clear_progress_notify. There is a similar change in cli_ui_out::do_progress_notify. With these two changes, the debuginfod tests are working again. This bodge will be removed by the last patch in this series. There's no tests with this commit yet as print_gdb_hints has other bugs which will be fixed in the next commit. At this point I'll add some tests. Reviewed-By: Guinevere Larsen <guinevere@redhat.com> --- gdb/cli-out.c | 19 ++++++++++--------- gdb/top.c | 10 +++++----- gdb/utils.c | 3 ++- gdb/utils.h | 5 +++-- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/gdb/cli-out.c b/gdb/cli-out.c index 27a82f607eb..5aa13a64271 100644 --- a/gdb/cli-out.c +++ b/gdb/cli-out.c @@ -298,18 +298,16 @@ cli_ui_out::do_progress_notify (const std::string &msg, const char *unit, double howmuch, double total) { - int chars_per_line = get_chars_per_line (); + unsigned int chars_per_line = get_chars_per_line (); struct ui_file *stream = get_unbuffered (m_streams.back ()); cli_progress_info &info (m_progress_info.back ()); - if (chars_per_line > MAX_CHARS_PER_LINE) - chars_per_line = MAX_CHARS_PER_LINE; - if (info.state == progress_update::START) { if (stream->isatty () && current_ui->input_interactive_p () - && chars_per_line >= MIN_CHARS_PER_LINE) + && chars_per_line >= MIN_CHARS_PER_LINE + && chars_per_line != UINT_MAX) { gdb_printf (stream, "%s\n", msg.c_str ()); info.state = progress_update::BAR; @@ -321,10 +319,12 @@ cli_ui_out::do_progress_notify (const std::string &msg, } } - if (info.state != progress_update::BAR - || chars_per_line < MIN_CHARS_PER_LINE) + if (info.state != progress_update::BAR) return; + if (chars_per_line > MAX_CHARS_PER_LINE) + chars_per_line = MAX_CHARS_PER_LINE; + if (total > 0 && howmuch >= 0 && howmuch <= 1.0) { std::string progress = string_printf (" %3.f%% (%.2f %s)", @@ -385,14 +385,15 @@ void cli_ui_out::clear_progress_notify () { struct ui_file *stream = get_unbuffered (m_streams.back ()); - int chars_per_line = get_chars_per_line (); + unsigned int chars_per_line = get_chars_per_line (); scoped_restore save_pagination = make_scoped_restore (&pagination_enabled, false); if (!stream->isatty () || !current_ui->input_interactive_p () - || chars_per_line < MIN_CHARS_PER_LINE) + || chars_per_line < MIN_CHARS_PER_LINE + || chars_per_line == UINT_MAX) return; if (chars_per_line > MAX_CHARS_PER_LINE) diff --git a/gdb/top.c b/gdb/top.c index 1214101ad18..1aefe74b394 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -1414,7 +1414,7 @@ box_one_message (ui_file *stream, std::string message, int width) void print_gdb_hints (struct ui_file *stream) { - int width = get_chars_per_line (); + unsigned int width = get_chars_per_line (); /* Arbitrarily setting maximum width to 80 characters, so that things are big and visible but not overwhelming. */ @@ -1439,10 +1439,10 @@ print_gdb_hints (struct ui_file *stream) styled_string (command_style.style (), "apropos <word>")); /* If there isn't enough space to display the longest URL in a boxed - style or if screen width is unlimited, use the simple styling of a - singular visual break. The longest URL is used because the other - messages may be broken into multiple lines, but URLs can't. */ - if (width - 3 <= (int) docs_url.length ()) + style, use the simple styling of a singular visual break. The longest + URL is used because the other messages may be broken into multiple + lines, but URLs can't. */ + if (width - 3 <= docs_url.length ()) { for (string_file &msg : styled_msg) gdb_printf (stream, "%s\n", msg.c_str ()); diff --git a/gdb/utils.c b/gdb/utils.c index ffe0d639e3a..d322843925b 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -1548,9 +1548,10 @@ gdb_flush (struct ui_file *stream) /* See utils.h. */ -int +unsigned int get_chars_per_line () { + gdb_assert (chars_per_line > 0); return chars_per_line; } diff --git a/gdb/utils.h b/gdb/utils.h index 0e28f9424e5..6316547653e 100644 --- a/gdb/utils.h +++ b/gdb/utils.h @@ -157,9 +157,10 @@ extern void wrap_here (int); extern void reinitialize_more_filter (void); -/* Return the number of characters in a line. */ +/* Return the number of characters in a line. Will never be zero, but can + be UINT_MAX, which indicates unlimited characters per line. */ -extern int get_chars_per_line (); +extern unsigned int get_chars_per_line (); extern bool pagination_enabled; -- 2.47.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv3 1/3] gdb: make get_chars_per_line return an unsigned value 2025-12-05 19:53 ` [PATCHv3 1/3] gdb: make get_chars_per_line return an unsigned value Andrew Burgess @ 2025-12-10 16:48 ` Tom Tromey 0 siblings, 0 replies; 26+ messages in thread From: Tom Tromey @ 2025-12-10 16:48 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches, Guinevere Larsen >>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes: Andrew> This commit works around the signed / unsigned confusion entirely Andrew> within print_gdb_hints by adding a case to 'int' in one place. The Andrew> change I present here reverts parts of 06e470d8fc0a in favour of Andrew> fixing the type of WIDTH within print_gdb_hints. Andrew> It is worth nothing that there are other bugs in print_gdb_hints Andrew> relating to how WIDTH is handled, but these are fixed in the next Andrew> commit. [...] I think this patch is ok. Approved-By: Tom Tromey <tom@tromey.com> Tom ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCHv3 2/3] gdb: fix crashes and weird output from new boxed hint text 2025-12-05 19:53 ` [PATCHv3 0/3] Fixes and tests related to new boxed hint startup text Andrew Burgess 2025-12-05 19:53 ` [PATCHv3 1/3] gdb: make get_chars_per_line return an unsigned value Andrew Burgess @ 2025-12-05 19:53 ` Andrew Burgess 2025-12-10 16:51 ` Tom Tromey 2025-12-05 19:53 ` [PATCHv3 3/3] WIP: disable progress bars setting Andrew Burgess 2 siblings, 1 reply; 26+ messages in thread From: Andrew Burgess @ 2025-12-05 19:53 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess, Guinevere Larsen After the commit: commit f6df8aa48f120b78f0670b429f8a3363020a47dc Date: Mon Sep 15 11:56:17 2025 -0300 gdb: Make startup message more user friendly I noticed, that when I start GDB with a file on the command line, I was seeing some stray '..'. Like this: $ gdb -nw -nh /tmp/hello.x GNU gdb (GDB) 18.0.50.20251202-git Copyright (C) 2025 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-pc-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <https://www.gnu.org/software/gdb/bugs/>. ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ Find the GDB manual online at: ┃ ┃ http://www.gnu.org/software/gdb/documentation/. ┃ ┃ For help, type "help". ┃ ┃ Type "apropos <word>" to search for commands related to <word> ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ .. Reading symbols from /tmp/hello.x... Notice the '..' after the boxed hint text, that's what I'm complaining about. Also, notice that the last line within the box is missing its period. Before the above commit the last line would appear like this when no file was loaded: Type "apropos <word>" to search for commands related to <word>. And like this when a file was being loaded: Type "apropos <word>" to search for commands related to <word>... The extra '..' are added to show that a file is being loaded, and that this might take some time. But we have the 'Reading symbols from ...' text to indicate this now, so I think the extra '..' are redundant. Lets just drop them. This will leave just a single period at the end of the sentence. The above commit unfortunately, didn't include any tests, so I thought I'd write some to cover this fix.... and that uncovered a bug where the box around the startup hints could be corrupted: $ gdb -eiex 'set width 50' -nw -nh GNU gdb (GDB) 18.0.50.20251202-git Copyright (C) 2025 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-pc-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <https://www.gnu.org/software/gdb/bugs/>. ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ Find the GDB manual online at: ┃ ┃ http://www.gnu.org/software/gdb/documentation/. ┃ ┃ For help, type "help". ┃ ┃ Type "apropos <word>" to ┃ ┃ search for commands related to <word> ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ (gdb) This was caused by a mistake on the line where we choose whether to box or not. The line is currently: if (width - 3 <= docs_url.length ()) There are two problems here, the '3' should be '4'. The box adds 4 characters '| ' and ' |'. But also, the WIDTH can be very small, less than 4 even, which means that the subtraction can underflow, wrapping around and giving a very large value. I plan to rewrite the line to: if (width < docs_url.length () + 1 + 4) The '+ 1' accounts for the period at the end of the URL line (which was previously handled by the '<=', and the '+ 4' accounts for the box borders. By making it a '+ 4' on the URL, rather than '- 4' from the width, we avoid underflow. This is fine so long as the URL to our documentation doesn't approach UINT_MAX in length. Which I hope it never does. I've added a couple of asserts to print_gdb_hints to reflect things that must be true. The first is that get_chars_per_line never returns 0. And later on, I assert that 'width > 4' in a place where we are about to do 'width - 4'. If the assert triggers then underflow would have occurred. Reviewed-By: Guinevere Larsen <guinevere@redhat.com> --- gdb/main.c | 5 +- gdb/testsuite/gdb.base/startup-hints.exp | 144 +++++++++++++++++++++++ gdb/top.c | 21 +++- 3 files changed, 161 insertions(+), 9 deletions(-) create mode 100644 gdb/testsuite/gdb.base/startup-hints.exp diff --git a/gdb/main.c b/gdb/main.c index 0fa2b0ecbe4..e4da715134b 100644 --- a/gdb/main.c +++ b/gdb/main.c @@ -1197,13 +1197,10 @@ captured_main_1 (struct captured_main_args *context) if (!quiet) { - /* Print all the junk at the top, with trailing "..." if we are - about to read a symbol file (possibly slowly). */ + /* Print the version, copyright information, and hint text. */ print_gdb_version (gdb_stdout, true); gdb_printf ("\n"); print_gdb_hints (gdb_stdout); - if (symarg) - gdb_printf (".."); gdb_printf ("\n"); gdb_flush (gdb_stdout); /* Force to screen during slow operations. */ diff --git a/gdb/testsuite/gdb.base/startup-hints.exp b/gdb/testsuite/gdb.base/startup-hints.exp new file mode 100644 index 00000000000..0addf3769a2 --- /dev/null +++ b/gdb/testsuite/gdb.base/startup-hints.exp @@ -0,0 +1,144 @@ +# Copyright 2025 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 <http://www.gnu.org/licenses/>. + +# Test the start up hint text. Check that it is boxed (or not) +# correctly. + +# Test assumes host == build. +require {!is_remote host} + +# Just use a simple empty 'main' function. +standard_testfile main.c + +if { [build_executable "build" $testfile $srcfile] != 0 } { + return +} + +# Return a single regexp string for the startup hint text when the +# terminal width is WIDTH. For narrow terminals GDB will just print +# the hint text. For wider terminals GDB places the hint into a box. +proc build_hint_re { width } { + # GDB imposes a maximum width of 80. + if { $width > 80 || $width == 0 } { + set width 80 + } + + if { $width > 50 } { + + # These lines are shorter than 50 characters, and so are never + # wrapped. + set msg { + {Find the GDB manual online at:} + {http://www.gnu.org/software/gdb/documentation/.} + {For help, type "help".} + } + + # The final line is wrapped based on the terminal width. + if { $width > 66 } { + lappend msg {Type "apropos <word>" to search for commands related to <word>.} + } elseif { $width > 58 } { + lappend msg {Type "apropos <word>" to search for commands related to} {<word>.} + } elseif { $width > 55 } { + lappend msg {Type "apropos <word>" to search for commands related} {to <word>.} + } elseif { $width > 47 } { + lappend msg {Type "apropos <word>" to search for commands} { related to <word>.} + } + + # Place the lines into a box, padding with whitespace so that + # the sides are correctly aligned. + set top_bottom "+[string repeat "-" [expr $width - 2]]+" + set lines [list $top_bottom] + foreach m $msg { + set space_count [expr $width - 4 - [string length $m]] + set spaces [string repeat " " $space_count] + lappend lines "| $m$spaces |" + } + lappend lines $top_bottom + } else { + # We tell GDB that the terminal width is WIDTH, but in reality + # the actual terminal width is unlimited. As such, GDB drops + # the box around the hint, but doesn't inject any additional + # newlines (it assumes the terminal will break the lines for + # us). This is why, despite the narrow terminal, we expect + # each line without any line breaks. + set lines {{Find the GDB manual online at:} \ + {http://www.gnu.org/software/gdb/documentation/.} \ + {For help, type "help".} \ + {Type "apropos <word>" to search for commands related to <word>.}} + } + + # Add blank line before and after current lines. + set lines [linsert $lines 0 ""] + lappend lines "" + + # Convert every line to a regexp. Also log the expected output + # for debugging. + set lines_re {} + verbose -log -- "Expected message format:" + foreach l $lines { + verbose -log -- "$l" + lappend lines_re [string_to_regexp $l] + } + + # Join the regexp together. + return [multi_line {*}$lines_re] +} + +# Tell GDB to start with a terminal width of WIDTH, then start GDB. +# Check that the hint text is formatted correctly. +proc_with_prefix test_for_hint_with_width { width load_exec } { + global GDBFLAGS + + save_vars { GDBFLAGS } { + append GDBFLAGS " -eiex \"set width $width\" -eiex \"set height 0\"" + if { $load_exec } { + append GDBFLAGS " \"$::binfile\"" + } + gdb_exit + gdb_spawn + } + + set hint_re [build_hint_re $width] + + if { $load_exec } { + append hint_re "\r\nReading symbols from [string_to_regexp $::binfile]\\.\\.\\." + } + + gdb_test "" $hint_re \ + "check for hint with width $width" +} + +save_vars { INTERNAL_GDBFLAGS } { + set INTERNAL_GDBFLAGS [string map {"-q" ""} $INTERNAL_GDBFLAGS] + + foreach_with_prefix load_exec { false true } { + + # Width 0 actually means unlimited. The other small sizes + # check that GDB doesn't trigger undefined behaviour by trying + # to create strings with a negative length. + for { set width 0 } { $width <= 5 } { incr width } { + test_for_hint_with_width $width $load_exec + } + + # These widths cover the point where we transition from using + # an unboxed hint to a boxed hint. + for { set width 45 } { $width <= 55 } { incr width } { + test_for_hint_with_width $width $load_exec + } + + # Very large widths are treated like a width of 80. + test_for_hint_with_width 100 $load_exec + } +} diff --git a/gdb/top.c b/gdb/top.c index 1aefe74b394..10366f98df1 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -1421,6 +1421,8 @@ print_gdb_hints (struct ui_file *stream) if (80 < width) width = 80; + gdb_assert (width > 0); + std::string docs_url = "http://www.gnu.org/software/gdb/documentation/"; std::array<string_file, 4> styled_msg { string_file (true), @@ -1435,20 +1437,29 @@ print_gdb_hints (struct ui_file *stream) gdb_printf (&styled_msg[2], _("For help, type \"%ps\"."), styled_string (command_style.style (), "help")); gdb_printf (&styled_msg[3], - _("Type \"%ps\" to search for commands related to <word>"), + _("Type \"%ps\" to search for commands related to <word>."), styled_string (command_style.style (), "apropos <word>")); /* If there isn't enough space to display the longest URL in a boxed - style, use the simple styling of a singular visual break. The longest - URL is used because the other messages may be broken into multiple - lines, but URLs can't. */ - if (width - 3 <= docs_url.length ()) + style, then don't use the box, the terminal will break the output + where needed. The longest URL is used because the other messages may + be broken into multiple lines, but URLs can't. + + The ' + 1' after the URL accounts for the period that is placed after + the URL. + + The '+ 4' accounts for the box and inner white space. We add the 4 to + the string length rather than subtract from the width as the width + could be less than 4, and we want to avoid wrap around. */ + if (width < docs_url.length () + 1 + 4) { for (string_file &msg : styled_msg) gdb_printf (stream, "%s\n", msg.c_str ()); } else { + gdb_assert (width > 4); + std::string sep (width - 2, '-'); if (emojis_ok ()) -- 2.47.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv3 2/3] gdb: fix crashes and weird output from new boxed hint text 2025-12-05 19:53 ` [PATCHv3 2/3] gdb: fix crashes and weird output from new boxed hint text Andrew Burgess @ 2025-12-10 16:51 ` Tom Tromey 0 siblings, 0 replies; 26+ messages in thread From: Tom Tromey @ 2025-12-10 16:51 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches, Guinevere Larsen >>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes: Andrew> The extra '..' are added to show that a file is being loaded, and that Andrew> this might take some time. But we have the 'Reading symbols from ...' Andrew> text to indicate this now, so I think the extra '..' are redundant. Andrew> Lets just drop them. This will leave just a single period at the end Andrew> of the sentence. In all these years I never knew about this little detail. Andrew> This is fine so long as the URL to our Andrew> documentation doesn't approach UINT_MAX in length. Which I hope it Andrew> never does. There goes my plan to ship the entire manual in a data: URL. /s And anyway it would still fit. Approved-By: Tom Tromey <tom@tromey.com> Tom ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCHv3 3/3] WIP: disable progress bars setting 2025-12-05 19:53 ` [PATCHv3 0/3] Fixes and tests related to new boxed hint startup text Andrew Burgess 2025-12-05 19:53 ` [PATCHv3 1/3] gdb: make get_chars_per_line return an unsigned value Andrew Burgess 2025-12-05 19:53 ` [PATCHv3 2/3] gdb: fix crashes and weird output from new boxed hint text Andrew Burgess @ 2025-12-05 19:53 ` Andrew Burgess 2025-12-06 9:12 ` Eli Zaretskii ` (2 more replies) 2 siblings, 3 replies; 26+ messages in thread From: Andrew Burgess @ 2025-12-05 19:53 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess --- gdb/NEWS | 6 ++ gdb/cli-out.c | 66 ++++++++++++++++++- gdb/doc/gdb.texinfo | 15 +++++ .../build-id-no-debug-warning.exp | 1 + .../gdb.debuginfod/corefile-mapped-file.exp | 1 + gdb/testsuite/gdb.debuginfod/crc_mismatch.exp | 1 + .../gdb.debuginfod/fetch_src_and_symbols.exp | 4 ++ .../gdb.debuginfod/solib-with-soname.exp | 1 + 8 files changed, 93 insertions(+), 2 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index 01c998f4ea0..0e43de89e8a 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -59,6 +59,12 @@ maintenance test-remote-args ARGS Test splitting and joining of inferior arguments ARGS as they would be split and joined when being passed to a remote target. +set progress-bars enabled on|off +show progress-bars enabled + Allows the progress bars, used when debuginfod is downloading + content, to be disabled (the set command), or to see if + progress-bars are currently enabled or not (the show command). + * Changed commands maintenance info program-spaces diff --git a/gdb/cli-out.c b/gdb/cli-out.c index 5aa13a64271..ae60b65064b 100644 --- a/gdb/cli-out.c +++ b/gdb/cli-out.c @@ -26,6 +26,7 @@ #include "readline/readline.h" #include "cli/cli-style.h" #include "ui.h" +#include "cli/cli-cmds.h" /* These are the CLI output functions */ @@ -275,6 +276,31 @@ cli_ui_out::do_progress_start () #define MIN_CHARS_PER_LINE 50 #define MAX_CHARS_PER_LINE 4096 +/* When this is false no progress bars will be displayed. When true, + progress bars can be displayed if the output stream supports them. */ + +static bool progress_bars_enabled = true; + +/* The "show progress-bars enabled" command. */ + +static void +show_progress_bars_enabled (struct ui_file *file, int from_tty, + struct cmd_list_element *c, + const char *value) +{ + if (progress_bars_enabled && get_chars_per_line () < MIN_CHARS_PER_LINE) + gdb_printf (file, _("Progress bars are currently \"off\". " + "The terminal is too narrow.\n")); + else if (progress_bars_enabled && (!gdb_stdout->isatty () + || !current_ui->input_interactive_p ())) + gdb_printf (file, _("Progress bars are currently \"off\". " + "The terminal doesn't support them.\n")); + else + gdb_printf (file, + _("Progress bars are currently \"%s\".\n"), + value); +} + /* Print a progress update. MSG is a string to be printed on the line above the progress bar. TOTAL is the size of the download whose progress is being displayed. UNIT should be the unit of TOTAL (ex. "K"). If HOWMUCH @@ -307,7 +333,7 @@ cli_ui_out::do_progress_notify (const std::string &msg, if (stream->isatty () && current_ui->input_interactive_p () && chars_per_line >= MIN_CHARS_PER_LINE - && chars_per_line != UINT_MAX) + && progress_bars_enabled) { gdb_printf (stream, "%s\n", msg.c_str ()); info.state = progress_update::BAR; @@ -393,7 +419,7 @@ cli_ui_out::clear_progress_notify () if (!stream->isatty () || !current_ui->input_interactive_p () || chars_per_line < MIN_CHARS_PER_LINE - || chars_per_line == UINT_MAX) + || !progress_bars_enabled) return; if (chars_per_line > MAX_CHARS_PER_LINE) @@ -542,3 +568,39 @@ cli_display_match_list (char **matches, int len, int max) gdb_display_match_list (matches, len, max, &displayer); rl_forced_update_display (); } + +/* Set/show progress-bars commands. */ +static cmd_list_element *set_progress_bars_prefix_list; +static cmd_list_element *show_progress_bars_prefix_list; + +/* Initialization for this file. */ + +INIT_GDB_FILE (cli_out) +{ + /* set/show debuginfod */ + add_setshow_prefix_cmd ("progress-bars", class_obscure, + _("Set progress-bars options."), + _("Show progress-bars options."), + &set_progress_bars_prefix_list, + &show_progress_bars_prefix_list, + &setlist, &showlist); + + /* Adds 'set|show progress-bars enabled'. */ + add_setshow_boolean_cmd ("enabled", class_obscure, + &progress_bars_enabled, _("\ +Set whether progress bars should be displayed."), _("\ +Show whether progress bars should be displayed."),_("\ +During some slow operations, for example, fetching debug information\n\ +from debuginfod, GDB will display an animated progress bar when this\n\ +setting is \"on\". When this setting is \"off\", no progress bars\n\ +will be displayed.\n\ +\n\ +Even when \"on\", progress bars can be disabled if the output terminal\n\ +doesn't support them."), + nullptr, + show_progress_bars_enabled, + &set_progress_bars_prefix_list, + &show_progress_bars_prefix_list); + + +} diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index e4469227a9e..4a6bc7355b5 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -51323,6 +51323,21 @@ Debuginfod Settings @item show debuginfod verbose Show the current verbosity setting. +@kindex set progress-bars enabled +@cindex progress bars, disabling +@item set progress-bars enabled @r{[}on@r{|}off@r{]} +Set whether @value{GDBN} can display a progress bar when downloading a +file from debuginfod. When @value{off}, @value{GDBN} will not display +a progress bar. When @value{on}, @value{GDBN} will display a progress +bar if @value{GDBN}'s output console supports it. + +@kindex show progress-bars enabled +@item show progress-bars enabled +Shows whether progress bars are currently enabled or not. Progress +bars can be automatically disabled if @value{GDBN}'s output console +doesn't support them, or if the terminal width is too small +(@pxref{Screen Size,,@kbd{set width} command}). + @end table @node Man Pages diff --git a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp index 7a0cfda627c..eb4d0589478 100644 --- a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp +++ b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp @@ -132,6 +132,7 @@ proc_with_prefix local_debuginfod { } { # Enable debuginfod and fetch the debuginfo. gdb_test_no_output "set debuginfod enabled on" + gdb_test_no_output "set progress-bars enabled off" # "separate debug info file has no debug info" warning should not be # reported now because the correct debuginfo should be fetched from diff --git a/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp index 83472f00bb0..df84a1dcb98 100644 --- a/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp +++ b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp @@ -371,6 +371,7 @@ with_debuginfod_env $cache { clean_restart gdb_test_no_output "set debuginfod enabled on" \ "enabled debuginfod for initial test" + gdb_test_no_output "set progress-bars enabled off" gdb_load $binfile load_core_file "load corefile, download library from debuginfod" \ diff --git a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp index e44748f8205..92de3ee2167 100644 --- a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp +++ b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp @@ -113,6 +113,7 @@ proc_with_prefix local_debuginfod { } { # Enable debuginfod and fetch the debuginfo. gdb_test_no_output "set debuginfod enabled on" + gdb_test_no_output "set progress-bars enabled off" gdb_test "file $binfile" ".*Reading symbols from.*debuginfo.*" \ "file [file tail $binfile] cmd on" diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp index 9078068c8fe..e3d9c36777b 100644 --- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp +++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp @@ -218,6 +218,7 @@ proc_with_prefix local_url { } { clean_restart gdb_test_no_output "set debuginfod enabled on" \ "enabled debuginfod for initial test" + gdb_test_no_output "set progress-bars enabled off" gdb_load $binfile gdb_test_no_output "set substitute-path $outputdir /dev/null" \ "set substitute-path" @@ -243,11 +244,13 @@ proc_with_prefix local_url { } { set enable_debuginfod_question \ "Enable debuginfod for this session. \\(y or \\\[n\\\]\\) " clean_restart + gdb_test_no_output -nopass "set progress-bars enabled off" gdb_test "core $::corefile" ".*return 0.*" "file [file tail $::corefile]" \ $enable_debuginfod_question "y" # GDB should now find the debugaltlink file. clean_restart + gdb_test_no_output -nopass "set progress-bars enabled off" gdb_test "file ${binfile}_alt.o" \ ".*Downloading.*separate debug info.*" \ "file [file tail ${binfile}_alt.o]" \ @@ -269,6 +272,7 @@ proc_with_prefix local_url { } { # Enable debuginfod and fetch the debuginfo. gdb_test_no_output "set debuginfod enabled on" + gdb_test_no_output -nopass "set progress-bars enabled off" gdb_test "file $binfile" ".*Reading symbols from.*debuginfo.*" \ "file [file tail $binfile] cmd on" diff --git a/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp b/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp index a22fa597602..5ff65e8f769 100644 --- a/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp +++ b/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp @@ -281,6 +281,7 @@ with_debuginfod_env $cache { save_vars { GDBFLAGS } { append GDBFLAGS " -ex \"set debuginfod enabled on\"" + append GDBFLAGS " -ex \"set progress-bars enabled off\"" # Reload the executable and core file. GDB should download # the file libfoo_1.so using debuginfod during the mapped file -- 2.47.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv3 3/3] WIP: disable progress bars setting 2025-12-05 19:53 ` [PATCHv3 3/3] WIP: disable progress bars setting Andrew Burgess @ 2025-12-06 9:12 ` Eli Zaretskii 2025-12-09 17:32 ` Andrew Burgess 2025-12-16 7:40 ` Tom de Vries 2 siblings, 0 replies; 26+ messages in thread From: Eli Zaretskii @ 2025-12-06 9:12 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches > From: Andrew Burgess <aburgess@redhat.com> > Cc: Andrew Burgess <aburgess@redhat.com> > Date: Fri, 5 Dec 2025 19:53:17 +0000 > > --- > gdb/NEWS | 6 ++ > gdb/cli-out.c | 66 ++++++++++++++++++- > gdb/doc/gdb.texinfo | 15 +++++ > .../build-id-no-debug-warning.exp | 1 + > .../gdb.debuginfod/corefile-mapped-file.exp | 1 + > gdb/testsuite/gdb.debuginfod/crc_mismatch.exp | 1 + > .../gdb.debuginfod/fetch_src_and_symbols.exp | 4 ++ > .../gdb.debuginfod/solib-with-soname.exp | 1 + > 8 files changed, 93 insertions(+), 2 deletions(-) Thanks, the documentation parts are okay. Reviewed-By: Eli Zaretskii <eliz@gnu.org> ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv3 3/3] WIP: disable progress bars setting 2025-12-05 19:53 ` [PATCHv3 3/3] WIP: disable progress bars setting Andrew Burgess 2025-12-06 9:12 ` Eli Zaretskii @ 2025-12-09 17:32 ` Andrew Burgess 2025-12-10 16:55 ` Tom Tromey 2025-12-16 7:40 ` Tom de Vries 2 siblings, 1 reply; 26+ messages in thread From: Andrew Burgess @ 2025-12-09 17:32 UTC (permalink / raw) To: gdb-patches Andrew Burgess <aburgess@redhat.com> writes: > --- > gdb/NEWS | 6 ++ > gdb/cli-out.c | 66 ++++++++++++++++++- > gdb/doc/gdb.texinfo | 15 +++++ > .../build-id-no-debug-warning.exp | 1 + > .../gdb.debuginfod/corefile-mapped-file.exp | 1 + > gdb/testsuite/gdb.debuginfod/crc_mismatch.exp | 1 + > .../gdb.debuginfod/fetch_src_and_symbols.exp | 4 ++ > .../gdb.debuginfod/solib-with-soname.exp | 1 + > 8 files changed, 93 insertions(+), 2 deletions(-) I realise I posted this without writing an actual commit message. That's pretty poor form. Apologies. I think I was having a bad day. Anyway, here's the same patch (no code or doc changes) but with an actual commit message. Thanks, Andrew --- commit d60de316ab832dcfdbd8cef11ff22a95ef27baa4 Author: Andrew Burgess <aburgess@redhat.com> Date: Fri Dec 5 11:33:29 2025 +0000 gdb: new setting to disable progress bars Two commits ago, in the commit titled: gdb: make get_chars_per_line return an unsigned value A bodge was added in cli-out.c so that progress bars (as seen when debuginfod downloads a file) would be disabled when the output terminal had unlimited width. The hack was added because this previous commit fixed a bug such that progress bars could now be displayed in very wide, or even on unlimited width output terminals. By fixing this bug, progress bars were now being displayed when running the testsuite, as the testsuite sets the output terminal to unlimited width. To avoid breaking the tests, this previous commit added a bodge such that on unlimited width output terminals, progress bars would always be disabled. This got the tests passing again, but isn't an ideal solution. This commit cleans things up. We now have a new setting: set progress-bars enabled on|off show progress-bars enabled This setting allows progress bars to be turned off. The tests are then updated to explicitly turn off progress bars. The bodge from the earlier commit is then removed. Now, progress bars should display correctly on any width of output terminal over 50 characters, the minimum required. And the debuginfod tests should all pass as they turn off progress bars. Reviewed-By: Eli Zaretskii <eliz@gnu.org> diff --git a/gdb/NEWS b/gdb/NEWS index 01c998f4ea0..0e43de89e8a 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -59,6 +59,12 @@ maintenance test-remote-args ARGS Test splitting and joining of inferior arguments ARGS as they would be split and joined when being passed to a remote target. +set progress-bars enabled on|off +show progress-bars enabled + Allows the progress bars, used when debuginfod is downloading + content, to be disabled (the set command), or to see if + progress-bars are currently enabled or not (the show command). + * Changed commands maintenance info program-spaces diff --git a/gdb/cli-out.c b/gdb/cli-out.c index 5aa13a64271..ae60b65064b 100644 --- a/gdb/cli-out.c +++ b/gdb/cli-out.c @@ -26,6 +26,7 @@ #include "readline/readline.h" #include "cli/cli-style.h" #include "ui.h" +#include "cli/cli-cmds.h" /* These are the CLI output functions */ @@ -275,6 +276,31 @@ cli_ui_out::do_progress_start () #define MIN_CHARS_PER_LINE 50 #define MAX_CHARS_PER_LINE 4096 +/* When this is false no progress bars will be displayed. When true, + progress bars can be displayed if the output stream supports them. */ + +static bool progress_bars_enabled = true; + +/* The "show progress-bars enabled" command. */ + +static void +show_progress_bars_enabled (struct ui_file *file, int from_tty, + struct cmd_list_element *c, + const char *value) +{ + if (progress_bars_enabled && get_chars_per_line () < MIN_CHARS_PER_LINE) + gdb_printf (file, _("Progress bars are currently \"off\". " + "The terminal is too narrow.\n")); + else if (progress_bars_enabled && (!gdb_stdout->isatty () + || !current_ui->input_interactive_p ())) + gdb_printf (file, _("Progress bars are currently \"off\". " + "The terminal doesn't support them.\n")); + else + gdb_printf (file, + _("Progress bars are currently \"%s\".\n"), + value); +} + /* Print a progress update. MSG is a string to be printed on the line above the progress bar. TOTAL is the size of the download whose progress is being displayed. UNIT should be the unit of TOTAL (ex. "K"). If HOWMUCH @@ -307,7 +333,7 @@ cli_ui_out::do_progress_notify (const std::string &msg, if (stream->isatty () && current_ui->input_interactive_p () && chars_per_line >= MIN_CHARS_PER_LINE - && chars_per_line != UINT_MAX) + && progress_bars_enabled) { gdb_printf (stream, "%s\n", msg.c_str ()); info.state = progress_update::BAR; @@ -393,7 +419,7 @@ cli_ui_out::clear_progress_notify () if (!stream->isatty () || !current_ui->input_interactive_p () || chars_per_line < MIN_CHARS_PER_LINE - || chars_per_line == UINT_MAX) + || !progress_bars_enabled) return; if (chars_per_line > MAX_CHARS_PER_LINE) @@ -542,3 +568,39 @@ cli_display_match_list (char **matches, int len, int max) gdb_display_match_list (matches, len, max, &displayer); rl_forced_update_display (); } + +/* Set/show progress-bars commands. */ +static cmd_list_element *set_progress_bars_prefix_list; +static cmd_list_element *show_progress_bars_prefix_list; + +/* Initialization for this file. */ + +INIT_GDB_FILE (cli_out) +{ + /* set/show debuginfod */ + add_setshow_prefix_cmd ("progress-bars", class_obscure, + _("Set progress-bars options."), + _("Show progress-bars options."), + &set_progress_bars_prefix_list, + &show_progress_bars_prefix_list, + &setlist, &showlist); + + /* Adds 'set|show progress-bars enabled'. */ + add_setshow_boolean_cmd ("enabled", class_obscure, + &progress_bars_enabled, _("\ +Set whether progress bars should be displayed."), _("\ +Show whether progress bars should be displayed."),_("\ +During some slow operations, for example, fetching debug information\n\ +from debuginfod, GDB will display an animated progress bar when this\n\ +setting is \"on\". When this setting is \"off\", no progress bars\n\ +will be displayed.\n\ +\n\ +Even when \"on\", progress bars can be disabled if the output terminal\n\ +doesn't support them."), + nullptr, + show_progress_bars_enabled, + &set_progress_bars_prefix_list, + &show_progress_bars_prefix_list); + + +} diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index e4469227a9e..4a6bc7355b5 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -51323,6 +51323,21 @@ Debuginfod Settings @item show debuginfod verbose Show the current verbosity setting. +@kindex set progress-bars enabled +@cindex progress bars, disabling +@item set progress-bars enabled @r{[}on@r{|}off@r{]} +Set whether @value{GDBN} can display a progress bar when downloading a +file from debuginfod. When @value{off}, @value{GDBN} will not display +a progress bar. When @value{on}, @value{GDBN} will display a progress +bar if @value{GDBN}'s output console supports it. + +@kindex show progress-bars enabled +@item show progress-bars enabled +Shows whether progress bars are currently enabled or not. Progress +bars can be automatically disabled if @value{GDBN}'s output console +doesn't support them, or if the terminal width is too small +(@pxref{Screen Size,,@kbd{set width} command}). + @end table @node Man Pages diff --git a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp index 7a0cfda627c..eb4d0589478 100644 --- a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp +++ b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp @@ -132,6 +132,7 @@ proc_with_prefix local_debuginfod { } { # Enable debuginfod and fetch the debuginfo. gdb_test_no_output "set debuginfod enabled on" + gdb_test_no_output "set progress-bars enabled off" # "separate debug info file has no debug info" warning should not be # reported now because the correct debuginfo should be fetched from diff --git a/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp index 83472f00bb0..df84a1dcb98 100644 --- a/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp +++ b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp @@ -371,6 +371,7 @@ with_debuginfod_env $cache { clean_restart gdb_test_no_output "set debuginfod enabled on" \ "enabled debuginfod for initial test" + gdb_test_no_output "set progress-bars enabled off" gdb_load $binfile load_core_file "load corefile, download library from debuginfod" \ diff --git a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp index e44748f8205..92de3ee2167 100644 --- a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp +++ b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp @@ -113,6 +113,7 @@ proc_with_prefix local_debuginfod { } { # Enable debuginfod and fetch the debuginfo. gdb_test_no_output "set debuginfod enabled on" + gdb_test_no_output "set progress-bars enabled off" gdb_test "file $binfile" ".*Reading symbols from.*debuginfo.*" \ "file [file tail $binfile] cmd on" diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp index 9078068c8fe..e3d9c36777b 100644 --- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp +++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp @@ -218,6 +218,7 @@ proc_with_prefix local_url { } { clean_restart gdb_test_no_output "set debuginfod enabled on" \ "enabled debuginfod for initial test" + gdb_test_no_output "set progress-bars enabled off" gdb_load $binfile gdb_test_no_output "set substitute-path $outputdir /dev/null" \ "set substitute-path" @@ -243,11 +244,13 @@ proc_with_prefix local_url { } { set enable_debuginfod_question \ "Enable debuginfod for this session. \\(y or \\\[n\\\]\\) " clean_restart + gdb_test_no_output -nopass "set progress-bars enabled off" gdb_test "core $::corefile" ".*return 0.*" "file [file tail $::corefile]" \ $enable_debuginfod_question "y" # GDB should now find the debugaltlink file. clean_restart + gdb_test_no_output -nopass "set progress-bars enabled off" gdb_test "file ${binfile}_alt.o" \ ".*Downloading.*separate debug info.*" \ "file [file tail ${binfile}_alt.o]" \ @@ -269,6 +272,7 @@ proc_with_prefix local_url { } { # Enable debuginfod and fetch the debuginfo. gdb_test_no_output "set debuginfod enabled on" + gdb_test_no_output -nopass "set progress-bars enabled off" gdb_test "file $binfile" ".*Reading symbols from.*debuginfo.*" \ "file [file tail $binfile] cmd on" diff --git a/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp b/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp index a22fa597602..5ff65e8f769 100644 --- a/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp +++ b/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp @@ -281,6 +281,7 @@ with_debuginfod_env $cache { save_vars { GDBFLAGS } { append GDBFLAGS " -ex \"set debuginfod enabled on\"" + append GDBFLAGS " -ex \"set progress-bars enabled off\"" # Reload the executable and core file. GDB should download # the file libfoo_1.so using debuginfod during the mapped file ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv3 3/3] WIP: disable progress bars setting 2025-12-09 17:32 ` Andrew Burgess @ 2025-12-10 16:55 ` Tom Tromey 2025-12-15 15:33 ` Andrew Burgess 0 siblings, 1 reply; 26+ messages in thread From: Tom Tromey @ 2025-12-10 16:55 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches >>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes: Andrew> Now, progress bars should display correctly on any width of output Andrew> terminal over 50 characters, the minimum required. And the debuginfod Andrew> tests should all pass as they turn off progress bars. Thanks. I have one nit but it's ok with this fixed. Andrew> diff --git a/gdb/cli-out.c b/gdb/cli-out.c Andrew> index 5aa13a64271..ae60b65064b 100644 Andrew> --- a/gdb/cli-out.c Andrew> +++ b/gdb/cli-out.c ...] Andrew> +INIT_GDB_FILE (cli_out) Andrew> +{ ... Andrew> + nullptr, Andrew> + show_progress_bars_enabled, Andrew> + &set_progress_bars_prefix_list, Andrew> + &show_progress_bars_prefix_list); Andrew> + Andrew> + Andrew> +} There's a couple extraneous empty lines here. Approved-By: Tom Tromey <tom@tromey.com> Tom ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv3 3/3] WIP: disable progress bars setting 2025-12-10 16:55 ` Tom Tromey @ 2025-12-15 15:33 ` Andrew Burgess 0 siblings, 0 replies; 26+ messages in thread From: Andrew Burgess @ 2025-12-15 15:33 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches Tom Tromey <tom@tromey.com> writes: >>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes: > > Andrew> Now, progress bars should display correctly on any width of output > Andrew> terminal over 50 characters, the minimum required. And the debuginfod > Andrew> tests should all pass as they turn off progress bars. > > Thanks. I have one nit but it's ok with this fixed. > > Andrew> diff --git a/gdb/cli-out.c b/gdb/cli-out.c > Andrew> index 5aa13a64271..ae60b65064b 100644 > Andrew> --- a/gdb/cli-out.c > Andrew> +++ b/gdb/cli-out.c > ...] > Andrew> +INIT_GDB_FILE (cli_out) > Andrew> +{ > ... > Andrew> + nullptr, > Andrew> + show_progress_bars_enabled, > Andrew> + &set_progress_bars_prefix_list, > Andrew> + &show_progress_bars_prefix_list); > Andrew> + > Andrew> + > Andrew> +} > > There's a couple extraneous empty lines here. > > Approved-By: Tom Tromey <tom@tromey.com> I cleaned up the empty lines, and pushed this series. Thanks, Andrew ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv3 3/3] WIP: disable progress bars setting 2025-12-05 19:53 ` [PATCHv3 3/3] WIP: disable progress bars setting Andrew Burgess 2025-12-06 9:12 ` Eli Zaretskii 2025-12-09 17:32 ` Andrew Burgess @ 2025-12-16 7:40 ` Tom de Vries 2025-12-16 10:05 ` Andrew Burgess 2 siblings, 1 reply; 26+ messages in thread From: Tom de Vries @ 2025-12-16 7:40 UTC (permalink / raw) To: Andrew Burgess, gdb-patches; +Cc: Eli Zaretskii On 12/5/25 8:53 PM, Andrew Burgess wrote: > +file from debuginfod. When @value{off}, @value{GDBN} will not display > +a progress bar. When @value{on}, @value{GDBN} will display a progress Hi, I noticed in the build: ... gdb.texinfo:51356: warning: undefined flag: off gdb.texinfo:51357: warning: undefined flag: on ... and indeed in the online documentation: ... set progress-bars enabled [on|off] Set whether GDB can display a progress bar when downloading a file from debuginfod. When {No value for ‘off’}, GDB will not display a progress bar. When {No value for ‘on’}, GDB will display a progress bar if GDB’s output console supports it. ... Thanks, - Tom ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv3 3/3] WIP: disable progress bars setting 2025-12-16 7:40 ` Tom de Vries @ 2025-12-16 10:05 ` Andrew Burgess 0 siblings, 0 replies; 26+ messages in thread From: Andrew Burgess @ 2025-12-16 10:05 UTC (permalink / raw) To: Tom de Vries, gdb-patches; +Cc: Eli Zaretskii Tom de Vries <tdevries@suse.de> writes: > On 12/5/25 8:53 PM, Andrew Burgess wrote: >> +file from debuginfod. When @value{off}, @value{GDBN} will not display >> +a progress bar. When @value{on}, @value{GDBN} will display a progress > > Hi, > > I noticed in the build: > ... > gdb.texinfo:51356: warning: undefined flag: off > gdb.texinfo:51357: warning: undefined flag: on > ... > and indeed in the online documentation: > ... > set progress-bars enabled [on|off] > > Set whether GDB can display a progress bar when downloading a file > from debuginfod. When {No value for ‘off’}, GDB will not display a > progress bar. When {No value for ‘on’}, GDB will display a progress bar > if GDB’s output console supports it. > ... Sorry for that. I pushed the patch below to resolve this problem. Thanks, Andrew -- commit 9529188a184f69d804c9c1c974c51d3b3460e676 Author: Andrew Burgess <aburgess@redhat.com> Date: Tue Dec 16 10:00:38 2025 +0000 gdb/doc: fix incorrect use of @value In commit: commit 255a9c42709958d0925d1d0d1d39d262972fd2a6 Date: Fri Dec 5 11:33:29 2025 +0000 gdb: new setting to disable progress bars I incorrectly used @value{on} and @value{off} to reference the on/off settings of a new flag. This caused the following warnings when building the docs: gdb.texinfo:51356: warning: undefined flag: off gdb.texinfo:51357: warning: undefined flag: on Looking through the docs there seems to be a split, in some cases we use @code and in others we use @samp. I'm not sure @code is correct, so I've switched to use @samp. The warnings are gone after this patch. diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 4c012393ff4..f77fe17c81f 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -51353,8 +51353,8 @@ Debuginfod Settings @cindex progress bars, disabling @item set progress-bars enabled @r{[}on@r{|}off@r{]} Set whether @value{GDBN} can display a progress bar when downloading a -file from debuginfod. When @value{off}, @value{GDBN} will not display -a progress bar. When @value{on}, @value{GDBN} will display a progress +file from debuginfod. When @samp{off}, @value{GDBN} will not display +a progress bar. When @samp{on}, @value{GDBN} will display a progress bar if @value{GDBN}'s output console supports it. @kindex show progress-bars enabled ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2025-12-16 10:06 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-12-04 16:38 [PATCH 0/4] Fixes and tests related to new boxed hint startup text Andrew Burgess 2025-12-04 16:38 ` [PATCH 1/4] gdb: remove some unnecessary code from print_gdb_hints Andrew Burgess 2025-12-04 17:14 ` Patrick Monnerat 2025-12-04 16:38 ` [PATCH 2/4] gdb: small white space fix in print_gdb_hints Andrew Burgess 2025-12-04 16:38 ` [PATCH 3/4] gdb: make get_chars_per_line return an unsigned value Andrew Burgess 2025-12-04 16:38 ` [PATCH 4/4] gdb: fix crashes and weird output from new boxed hint text Andrew Burgess 2025-12-04 19:02 ` [PATCH 0/4] Fixes and tests related to new boxed hint startup text Guinevere Larsen 2025-12-04 19:32 ` Andrew Burgess 2025-12-04 19:47 ` [PATCH 0/2] " Andrew Burgess 2025-12-04 19:47 ` [PATCH 1/2] gdb: make get_chars_per_line return an unsigned value Andrew Burgess 2025-12-04 19:47 ` [PATCH 2/2] gdb: fix crashes and weird output from new boxed hint text Andrew Burgess 2025-12-05 7:06 ` Eli Zaretskii 2025-12-05 10:17 ` Andrew Burgess 2025-12-05 11:19 ` Eli Zaretskii 2025-12-05 19:53 ` [PATCHv3 0/3] Fixes and tests related to new boxed hint startup text Andrew Burgess 2025-12-05 19:53 ` [PATCHv3 1/3] gdb: make get_chars_per_line return an unsigned value Andrew Burgess 2025-12-10 16:48 ` Tom Tromey 2025-12-05 19:53 ` [PATCHv3 2/3] gdb: fix crashes and weird output from new boxed hint text Andrew Burgess 2025-12-10 16:51 ` Tom Tromey 2025-12-05 19:53 ` [PATCHv3 3/3] WIP: disable progress bars setting Andrew Burgess 2025-12-06 9:12 ` Eli Zaretskii 2025-12-09 17:32 ` Andrew Burgess 2025-12-10 16:55 ` Tom Tromey 2025-12-15 15:33 ` Andrew Burgess 2025-12-16 7:40 ` Tom de Vries 2025-12-16 10:05 ` Andrew Burgess
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox