From: Andrew Burgess <aburgess@redhat.com>
Date: Thu, 4 Dec 2025 10:38:23 +0000 (+0000)
Subject: gdb: fix crashes and weird output from new boxed hint text
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=19a76d2bc7ddebc7ae89251c87ca1d7830e6cc18;p=thirdparty%2Fbinutils-gdb.git

gdb: fix crashes and weird output from new boxed hint text

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>
Approved-By: Tom Tromey <tom@tromey.com>
---

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 adc0596a504..0ab9b609e45 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1420,6 +1420,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),
@@ -1434,20 +1436,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 ())