[Bug tui/33794] [gdb/tui] Recursive readline use

https://sourceware.org/bugzilla/show_bug.cgi?id=33794

--- Comment #8 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <[email protected]>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=96d843689b6b5532ebaf616c63c059ba1136bb67

commit 96d843689b6b5532ebaf616c63c059ba1136bb67
Author: Andrew Burgess <[email protected]>
Date:   Thu Apr 30 09:29:38 2026 +0100

    gdb/tui: fix debuginfod prompt using 'C-x C-a' to enter TUI

    This commit ties closely into the previous commit.  The previous
    commit looks at issues that can arise when using 'tui enable' to enter
    TUI mode if a debuginfod prompt is triggered.  This commit looks at
    the problems that can arise when a multi-key combination is used to
    enter TUI mode, e.g. 'C-x C-a'.  Bug PR gdb/33794 discusses this
    issue.

    There has been a previous attempt to address this issue here:

     
https://inbox.sourceware.org/gdb-patches/[email protected]

    The approach taken in that patch was to prevent switching to TUI mode
    if debuginfod is still in ASK mode, this means the switch could
    potentially trigger a secondary prompt.

    While the previous commit is relatively simple, the complexity in this
    case arises from how multi-key combinations are handled by readline.
    Currently global readline state is used to track the multi-key press
    situation, and when the multi-key is dispatched back to application
    (GDB) code, the globals are still live.

    If GDB then triggers reentry into readline, e.g. by triggering a
    secondary prompt, the call into readline for this prompt will cause
    the global state to be released.  When the secondary prompt is
    finished and we return back to readline the global state will be
    accessed, and undefined behaviour occurs, including crashes.

    The core idea of my proposed solution to this is to move handling of
    the multi-key actions out of the readline callback, and into GDB's
    normal event loop.

    When the user presses a combination like 'C-x C-a' this will call a
    templated tui_rl_keybinding function as it currently does, but instead
    of immediately forwarding to another function to carry out the TUI
    changes, we instead schedule a callback with the event loop and then
    return.

    As far as readline is concerned the multi-key action has now been
    dealt with, however, no interface changes have yet occurred.  As
    readline has now finished handling this key press, readline returns to
    the event loop to get the next user input.

    At the event loop the pending callback is seen and dispatched.  This
    callback triggers the actual UI changes, e.g. entering TUI mode.  As
    we are not inside readline at this point we are free to create
    secondary prompts if needed.

    In tui_rl_keybinding we use run_on_main_thread to schedule a callback
    with the event loop, but there are some additional changes needed:

    1. If we changed the tui_active state then we need to call
       reinitialize_more_filter.  Previously tui_rl_switch_mode would call
       rl_newline which would make readline think that a command had been
       fully entered, this would trigger a call to GDB's
       command_line_handler, which calls command_handler, which then calls
       reinitialize_more_filter.

       For reasons explained below tui_rl_switch_mode can no longer call
       rl_newline, so the reinitialize_more_filter is never reached.  This
       means that especially when switching CLI to TUI, when the `cmd`
       window is smaller than the CLI terminal, GDB might enter TUI mode
       thinking that the TUI is already full.  This leads to incorrect
       pager prompts appearing.  Resolve this by explicitly resetting the
       pager.

    2. After changing the tui_active state (i.e. entering or leaving TUI
       mode), there will not be a GDB prompt displayed.  Under the old
       scheme, the rl_newline call in tui_rl_switch_mode would trick GDB
       into thinking an empty command had just been completed, this would
       then trigger a prompt redisplay.

       Under the new scheme we need to explicitly call display_gdb_prompt
       or tui_redisplay_readline to redraw the prompt.  However, as we
       were at a GDB prompt already when the user pressed a key like 'C-x
       C-a', the current_ui will not think that a prompt is needed, if we
       plan to call display_gdb_prompt then we'll need to change the
       prompt_state to PROMPT_NEEDED before calling display_gdb_prompt.

       When possible we prefer calling tui_redisplay_readline, as this
       preserves the current readline input line buffer contents, so if
       the user types something at the prompt and then does 'C-x o' to
       change window focus, the partially typed text is preserved.

    Both of these additional actions need to be performed for both the
    normal exit path, and the exception path in order that the prompt be
    correctly displayed, so this code is done in a SCOPE_EXIT block.

    The other set of changes are in tui_rl_switch_mode:

    1. The calls to rl_prep_terminal are no longer needed as
       display_gdb_prompt will take care of calling this for us if
       appropriate (e.g. we are not in TUI mode).

    2. The gdb_exception_forced_quit handling can now just propagate the
       exception, We are no longer within a readline callback, and so can
       throw this exception further up the stack.

    3. Likewise with gdb_exception, we can re-throw this.  As the
       run_on_main_thread mechanism silently swallows all gdb_exceptions
       except the gdb_exception_forced_quit sub-class, we do need to print
       the exception ourselves first though.  This is why we had to
       separate out the gdb_exception_forced_quit handling.

    4. The rl_kill_text call is no longer needed as the following
       rl_newline call is going to be removed.

    5. The rl_newline call was a neat trick to force a prompt redisplay,
       but this only works when we are within a readline callback, it
       injects a newline so that when we return from this callback
       readline will see the pending newline character, process the now
       empty line (thanks to the rl_kill_text call), and the print the
       prompt.  This is replaced by the display_gdb_prompt call that was
       added to tui_rl_keybinding.

    6. The dont_repeat call was needed because the rl_kill_text and
       rl_newline calls were tricking readline into thinking the user had
       pressed Enter on an empty line, this was done to force a prompt
       redisplay.

       However, pressing Enter on an empty line repeats the previous
       command unless dont_repeat has been called.  Now we don't use the
       rl_newline trick, the dont_repeat call is not needed.

    The gdb.tui/debuginfod-query.exp test is updated to include tests that
    switch using multi-key combinations.

    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33794

-- 
You are receiving this mail because:
You are on the CC list for the bug.