diff mbox series

[3/6] gdb/tui: prevent TUI activation from a secondary prompt

Message ID 3b444291c235f24a4fdf34740293333270985745.1777645161.git.aburgess@redhat.com
State New
Headers
Series gdb/tui: fix debuginfod related crash |

Commit Message

Andrew Burgess May 1, 2026, 2:22 p.m. UTC 
  The TUI can be activated with key combinations like 'C-x C-a'.  This
is handled by readline calling the function tui_rl_switch_mode, or
various other functions which indirectly call that function.

These multi-key combinations can be used even at a secondary prompt,
e.g. the:

  Make breakpoint pending on future shared library load? (y or [n])

The problem with this is that when the TUI activates CLI content
doesn't carry over into the `cmd` window, so the secondary prompt is
not visible to the user after the mode switch.  Worse, because the
content doesn't carry over we clear the readline state, and this
involves sending a '\n' to readline.  This newline will select the
default action at the secondary prompt, which might not be what the
user actually wants.

Now, we could imagine trying to "fix" this so that the CLI content is
copied over into the `cmd` window, and the secondary prompt is
represented to the user, so they can then make the choice they want,
but implementing this fix would be a big job, for very little gain.

Easier I think, to just prevent the user switching to TUI mode while
at a secondary prompt.

I created a new templated wrapper function tui_rl_keybinding, which is
then used to wrap every function that is bound to a readline multi-key
combination.  The wrapper function checks if we are in a secondary
prompt, and if we are, performs an early return.

For completeness, I've added an assert that we are not in a secondary
prompt to all of the wrapped functions, this (hopefully) will help
catch cases where these functions are called directly without going
through the wrapper.

There's a new helper proc added to lib/gdb.exp, this will be used by
additional tests later in this series.

The user can still switch to TUI mode at the primary '(gdb)' prompt.
---
 .../gdb.tui/activate-from-secondary-prompt.c  |  22 +++
 .../activate-from-secondary-prompt.exp        | 154 ++++++++++++++++++
 gdb/testsuite/lib/gdb.exp                     |  28 ++++
 gdb/tui/tui.c                                 |  73 ++++++---
 4 files changed, 255 insertions(+), 22 deletions(-)
 create mode 100644 gdb/testsuite/gdb.tui/activate-from-secondary-prompt.c
 create mode 100644 gdb/testsuite/gdb.tui/activate-from-secondary-prompt.exp
diff mbox series

Patch

diff --git a/gdb/testsuite/gdb.tui/activate-from-secondary-prompt.c b/gdb/testsuite/gdb.tui/activate-from-secondary-prompt.c
new file mode 100644
index 00000000000..6a0e311ef41
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/activate-from-secondary-prompt.c
@@ -0,0 +1,22 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2026 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/>.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.tui/activate-from-secondary-prompt.exp b/gdb/testsuite/gdb.tui/activate-from-secondary-prompt.exp
new file mode 100644
index 00000000000..8de044e7b19
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/activate-from-secondary-prompt.exp
@@ -0,0 +1,154 @@ 
+# Copyright 2026 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/>.
+
+# Check that the TUI can NOT be activated using 'C-x C-a', and similar
+# key combinations, from a secondary prompt.  Activating from the
+# secondary prompt has the same effect as pressing 'Return' at the
+# prompt, which might not be what the user wants.
+#
+# We could imagine a world where, when activating the TUI from a
+# secondary prompt works as we might expect, the prompt is not
+# cancelled, but reprinted within the TUI, and the user can then make
+# their choice and continue with their debug session.  If we ever
+# implement this, then this test will need to change.
+#
+# For now though we take the easy way and just ban the user from
+# activating the TUI from a secondary prompt.
+
+load_lib debuginfod-support.exp
+
+tuiterm_env
+
+require allow_tui_tests
+
+standard_testfile
+
+if {[build_executable "build executable" ${testfile} ${srcfile}] == -1} {
+    return
+}
+
+# Check that readline is in use.
+clean_restart
+if { ![readline_is_used] } {
+    unsupported "readline is required"
+    return
+}
+
+# Assuming that GDB is at a secondary prompt, try various multi-key
+# combinations and check that the screen doesn't change, these should
+# all be ignored.
+proc test_multi_key_combos {} {
+    foreach mk { \030\001 \030a \030A \0302 \0301 \030o \030s } {
+	set before [Term::get_region 0 0 80 24 "\r\n"]
+	send_gdb $mk
+	set after [Term::get_region 0 0 80 24 "\r\n"]
+
+	gdb_assert { $before eq $after } \
+	    "no changes after sending '[unprintable_to_octal $mk]'"
+    }
+}
+
+# Send CHAR followed by a newline to GDB, this should quit the
+# currently active secondary prompt.  Wait for the GDB prompt to
+# present, then send a basic print command to GDB to ensure that GDB
+# is still interactive.
+proc quit_prompt { char } {
+    send_gdb "${char}\n"
+    Term::wait_for ""
+
+    gdb_test "print 1" " = 1"
+}
+
+# Trigger the pagination prompt, then check that the multi-key
+# combinations don't result in a change to TUI mode.  Finally, check
+# that GDB is still responsive.
+proc_with_prefix test_from_pager_prompt {} {
+    Term::clean_restart 24 80 $::testfile
+
+    set cmd "echo "
+    for { set i 0 } { $i < 100 } { incr i } {
+	set cmd "${cmd}${i}\\n"
+    }
+
+    gdb_test_no_output "set pagination on"
+    gdb_test_no_output "set height 24"
+    send_gdb "$cmd\n"
+
+    if {![Term::wait_for_region_contents 0 0 80 24 \
+	      [string_to_regexp $::pagination_prompt_str]]} {
+	fail "spot secondary prompt"
+	return
+    }
+
+    test_multi_key_combos
+    quit_prompt q
+}
+
+# Trigger the debuginfod prompt, then check that the multi-key
+# combinations don't result in a change to TUI mode.  Finally, check
+# GDB is still responsive.
+proc_with_prefix test_from_debuginfod_prompt {} {
+    if { ![allow_debuginfod_tests] } {
+	return
+    }
+
+    set stripped_binfile [standard_output_file ${::testfile}-stripped]
+    file copy -force $::binfile $stripped_binfile
+    if {[gdb_gnu_strip_debug $stripped_binfile no-debuglink]} {
+	unsupported "produce separate debug info for [file tail $stripped_binfile]"
+	return
+    }
+
+    if {[section_get $stripped_binfile ".gnu_debuglink"] ne ""} {
+	unsupported "debug information has already been split out"
+	return
+    }
+
+    save_vars { env(DEBUGINFOD_URLS) } {
+	setenv DEBUGINFOD_URLS "foo"
+	Term::clean_restart 24 80
+    }
+
+    send_gdb "file \"$stripped_binfile\"\n"
+    if {![Term::wait_for_region_contents 0 0 80 24 \
+	      [string_to_regexp {(y or [n]) }]]} {
+	fail "spot secondary prompt"
+	return
+    }
+
+    test_multi_key_combos
+    quit_prompt n
+}
+
+# Trigger the pending breakpoint prompt, then check that the multi-key
+# combinations don't result in a change to TUI mode.  Finally, check
+# that GDB is still responsive.
+proc_with_prefix test_from_breakpoint_prompt {} {
+    Term::clean_restart 24 80 $::testfile
+
+    send_gdb "break _a_function_that_hopefully_doesnt_exist_\n"
+    if {![Term::wait_for_region_contents 0 0 80 24 \
+	      [string_to_regexp {(y or [n]) }]]} {
+	fail "spot secondary prompt"
+	return
+    }
+
+    test_multi_key_combos
+    quit_prompt n
+}
+
+test_from_pager_prompt
+test_from_debuginfod_prompt
+test_from_breakpoint_prompt
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 28709004570..d777b25e45c 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -11808,6 +11808,34 @@  proc dwz_version_at_least { ver } {
     return [version_compare [split $ver .] <= [dwz_version]]
 }
 
+# Take a string containing unprintable characters and return a string
+# with the unprintable characters represented as 3 octal bytes,
+# e.g. "\030".
+#
+# To avoid having to escape backslashes, backslashes themselves are
+# also represented as octal.  Probably best not to use this for
+# strings containing backslashes.
+proc unprintable_to_octal { input_string } {
+    set result ""
+
+    foreach char [split $input_string ""] {
+	# Convert character to its integer ASCII value.
+	scan $char %c ascii_val
+
+	# Include printable characters as literals.  Exclude backslash
+	# to avoid needing to escape it, we don't actually expect to
+	# see that in INPUT_STRING though.
+	if {[string is print $char] && $char ne "\\"} {
+	    append result $char
+	} else {
+	    # Include non-printable characters as a 3-digit octal.
+	    append result [format "\\%03o" $ascii_val]
+	}
+    }
+
+    return $result
+}
+
 require {tcl_version_at_least 8 6 2}
 
 # Always load compatibility stuff.
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index ba4f6f68769..d2e9c1b4def 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -114,6 +114,7 @@  static Keymap tui_readline_standard_keymap;
 static int
 tui_rl_switch_mode (int notused1 = 0, int notused2 = 0)
 {
+  gdb_assert (!gdb_in_secondary_prompt_p (current_ui));
 
   /* Don't let exceptions escape.  We're in the middle of a readline
      callback that isn't prepared for that.  */
@@ -198,6 +199,8 @@  tui_try_activate ()
 static int
 tui_rl_change_windows (int notused1, int notused2)
 {
+  gdb_assert (!gdb_in_secondary_prompt_p (current_ui));
+
   if (tui_try_activate ())
     tui_next_layout ();
 
@@ -209,6 +212,8 @@  tui_rl_change_windows (int notused1, int notused2)
 static int
 tui_rl_delete_other_windows (int notused1, int notused2)
 {
+  gdb_assert (!gdb_in_secondary_prompt_p (current_ui));
+
   if (tui_try_activate ())
     tui_remove_some_windows ();
 
@@ -220,6 +225,8 @@  tui_rl_delete_other_windows (int notused1, int notused2)
 static int
 tui_rl_other_window (int count, int key)
 {
+  gdb_assert (!gdb_in_secondary_prompt_p (current_ui));
+
   if (tui_try_activate ())
     tui_set_win_focus_to (tui_next_win (tui_win_with_focus ()));
 
@@ -231,10 +238,10 @@  tui_rl_other_window (int count, int key)
 static int
 tui_rl_command_key (int count, int key)
 {
-  int i;
+  gdb_assert (!gdb_in_secondary_prompt_p (current_ui));
 
   reinitialize_more_filter ();
-  for (i = 0; tui_commands[i].cmd; i++)
+  for (int i = 0; tui_commands[i].cmd; i++)
     {
       if (tui_commands[i].key == key)
 	{
@@ -262,6 +269,7 @@  tui_rl_command_key (int count, int key)
 static int
 tui_rl_command_mode (int count, int key)
 {
+  gdb_assert (!gdb_in_secondary_prompt_p (current_ui));
   tui_set_key_mode (TUI_ONE_COMMAND_MODE);
   return rl_insert (count, key);
 }
@@ -271,6 +279,8 @@  tui_rl_command_mode (int count, int key)
 static int
 tui_rl_next_keymap (int notused1, int notused2)
 {
+  gdb_assert (!gdb_in_secondary_prompt_p (current_ui));
+
   if (!tui_try_activate ())
     return 0;
 
@@ -310,6 +320,20 @@  tui_set_key_mode (enum tui_key_mode mode)
   tui_show_status_content ();
 }
 
+/* Wrapper around function FPTR, used to add common checks before
+   functions that are bound to readline multi-key combinations.  */
+
+template<int (*FPTR) (int, int)>
+int
+tui_rl_keybinding (int count, int key)
+{
+  /* Don't allow TUI changes while we're at an interactive prompt.  */
+  if (gdb_in_secondary_prompt_p (current_ui))
+    return 0;
+
+  return FPTR (count, key);
+}
+
 /* Initialize readline and configure the keymap for the switching
    key shortcut.  */
 void
@@ -324,11 +348,16 @@  tui_ensure_readline_initialized ()
   int i;
   Keymap tui_ctlx_keymap;
 
-  rl_add_defun ("tui-switch-mode", tui_rl_switch_mode, -1);
-  rl_add_defun ("next-keymap", tui_rl_next_keymap, -1);
-  rl_add_defun ("tui-delete-other-windows", tui_rl_delete_other_windows, -1);
-  rl_add_defun ("tui-change-windows", tui_rl_change_windows, -1);
-  rl_add_defun ("tui-other-window", tui_rl_other_window, -1);
+  rl_add_defun ("tui-switch-mode",
+		tui_rl_keybinding<tui_rl_switch_mode>, -1);
+  rl_add_defun ("next-keymap",
+		tui_rl_keybinding<tui_rl_next_keymap>, -1);
+  rl_add_defun ("tui-delete-other-windows",
+		tui_rl_keybinding<tui_rl_delete_other_windows>, -1);
+  rl_add_defun ("tui-change-windows",
+		tui_rl_keybinding<tui_rl_change_windows>, -1);
+  rl_add_defun ("tui-other-window",
+		tui_rl_keybinding<tui_rl_other_window>, -1);
 
   tui_keymap = rl_make_bare_keymap ();
 
@@ -361,21 +390,21 @@  tui_ensure_readline_initialized ()
       rl_bind_key_in_map (i, tui_rl_command_mode, tui_keymap);
     }
 
-  rl_bind_key_in_map ('a', tui_rl_switch_mode, emacs_ctlx_keymap);
-  rl_bind_key_in_map ('a', tui_rl_switch_mode, tui_ctlx_keymap);
-  rl_bind_key_in_map ('A', tui_rl_switch_mode, emacs_ctlx_keymap);
-  rl_bind_key_in_map ('A', tui_rl_switch_mode, tui_ctlx_keymap);
-  rl_bind_key_in_map (c_ctrl ('A'), tui_rl_switch_mode, emacs_ctlx_keymap);
-  rl_bind_key_in_map (c_ctrl ('A'), tui_rl_switch_mode, tui_ctlx_keymap);
-  rl_bind_key_in_map ('1', tui_rl_delete_other_windows, emacs_ctlx_keymap);
-  rl_bind_key_in_map ('1', tui_rl_delete_other_windows, tui_ctlx_keymap);
-  rl_bind_key_in_map ('2', tui_rl_change_windows, emacs_ctlx_keymap);
-  rl_bind_key_in_map ('2', tui_rl_change_windows, tui_ctlx_keymap);
-  rl_bind_key_in_map ('o', tui_rl_other_window, emacs_ctlx_keymap);
-  rl_bind_key_in_map ('o', tui_rl_other_window, tui_ctlx_keymap);
-  rl_bind_key_in_map ('q', tui_rl_next_keymap, tui_keymap);
-  rl_bind_key_in_map ('s', tui_rl_next_keymap, emacs_ctlx_keymap);
-  rl_bind_key_in_map ('s', tui_rl_next_keymap, tui_ctlx_keymap);
+  rl_bind_key_in_map ('a', tui_rl_keybinding<tui_rl_switch_mode>, emacs_ctlx_keymap);
+  rl_bind_key_in_map ('a', tui_rl_keybinding<tui_rl_switch_mode>, tui_ctlx_keymap);
+  rl_bind_key_in_map ('A', tui_rl_keybinding<tui_rl_switch_mode>, emacs_ctlx_keymap);
+  rl_bind_key_in_map ('A', tui_rl_keybinding<tui_rl_switch_mode>, tui_ctlx_keymap);
+  rl_bind_key_in_map (c_ctrl ('A'), tui_rl_keybinding<tui_rl_switch_mode>, emacs_ctlx_keymap);
+  rl_bind_key_in_map (c_ctrl ('A'), tui_rl_keybinding<tui_rl_switch_mode>, tui_ctlx_keymap);
+  rl_bind_key_in_map ('1', tui_rl_keybinding<tui_rl_delete_other_windows>, emacs_ctlx_keymap);
+  rl_bind_key_in_map ('1', tui_rl_keybinding<tui_rl_delete_other_windows>, tui_ctlx_keymap);
+  rl_bind_key_in_map ('2', tui_rl_keybinding<tui_rl_change_windows>, emacs_ctlx_keymap);
+  rl_bind_key_in_map ('2', tui_rl_keybinding<tui_rl_change_windows>, tui_ctlx_keymap);
+  rl_bind_key_in_map ('o', tui_rl_keybinding<tui_rl_other_window>, emacs_ctlx_keymap);
+  rl_bind_key_in_map ('o', tui_rl_keybinding<tui_rl_other_window>, tui_ctlx_keymap);
+  rl_bind_key_in_map ('q', tui_rl_keybinding<tui_rl_next_keymap>, tui_keymap);
+  rl_bind_key_in_map ('s', tui_rl_keybinding<tui_rl_next_keymap>, emacs_ctlx_keymap);
+  rl_bind_key_in_map ('s', tui_rl_keybinding<tui_rl_next_keymap>, tui_ctlx_keymap);
 
   /* Initialize readline after the above.  */
   rl_initialize ();