[PATCH] [gdb/tui] Make tui_setup_io more robust

[PATCH] [gdb/tui] Make tui_setup_io more robust

[Tom de Vries]

The following sequence of events may happen when enabling TUI:
- tui_enable () is called,
- a SIGFPE happens, before tui_setup_io (1) is called,
- the SIGFPE triggers handle_fatal_signal, which calls tui_disable (),
- during tui_disable (), tui_setup_io (0) is called, and
- tui_setup_io (0) tries to restore things that were saved during a previous
  tui_setup_io (1) call, but tui_setup_io (1) was never called so the saving
  never happened.

This can cause current_ui.m_ui_stderr to be nullptr, which then can cause a
crash in sig_write (PR33918).

Fix this by:
- adding a variable tui_io_mode, and
- using the variable to bail out of tui_setup_io in case the current mode
  doesn't change.

Tested on x86_64-linux.
---
 gdb/tui/tui-io.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index f673fbf36f6..5a3bc604592 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -824,6 +824,10 @@ tui_rl_display_match_list (char **matches, int len, int max)
   gdb_display_match_list (matches, len, max, &displayer);
 }
 
+/* Whether the IO is setup for curses (1) or non-curses (0).  */
+
+static int tui_io_mode;
+
 /* Setup the IO for curses or non-curses mode.
    - In non-curses mode, readline and gdb use the standard input and
    standard output/error directly.
@@ -837,6 +841,14 @@ tui_setup_io (int mode)
 {
   extern int _rl_echoing_p;
 
+  if (tui_io_mode == mode)
+    {
+      /* Nothing to do.  Also, this makes sure that we don't try to restore
+	 things before having saved them. */
+      return;
+    }
+  tui_io_mode = mode;
+
   if (mode)
     {
       /* Ensure that readline has been initialized before saving any

base-commit: 646982f4295bf0a1e64867606d496c34c9a15a0c
-- 
2.51.0

Re: [PATCH] [gdb/tui] Make tui_setup_io more robust

[Andrew Burgess]

Tom de Vries <tdevries@suse.de> writes:

> The following sequence of events may happen when enabling TUI:
> - tui_enable () is called,
> - a SIGFPE happens, before tui_setup_io (1) is called,
> - the SIGFPE triggers handle_fatal_signal, which calls tui_disable (),
> - during tui_disable (), tui_setup_io (0) is called, and
> - tui_setup_io (0) tries to restore things that were saved during a previous
>   tui_setup_io (1) call, but tui_setup_io (1) was never called so the saving
>   never happened.
>
> This can cause current_ui.m_ui_stderr to be nullptr, which then can cause a
> crash in sig_write (PR33918).

If this bug number is worth mentioning, then we should add a 'Bug:...'
tag to the commit message.  I like them if for nothing else they are
clickable in my terminal :)

>
> Fix this by:
> - adding a variable tui_io_mode, and
> - using the variable to bail out of tui_setup_io in case the current mode
>   doesn't change.

What's one more global, the tui already has so many!

This seems a reasonable change to me.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


>
> Tested on x86_64-linux.
> ---
>  gdb/tui/tui-io.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
> index f673fbf36f6..5a3bc604592 100644
> --- a/gdb/tui/tui-io.c
> +++ b/gdb/tui/tui-io.c
> @@ -824,6 +824,10 @@ tui_rl_display_match_list (char **matches, int len, int max)
>    gdb_display_match_list (matches, len, max, &displayer);
>  }
>  
> +/* Whether the IO is setup for curses (1) or non-curses (0).  */
> +
> +static int tui_io_mode;
> +
>  /* Setup the IO for curses or non-curses mode.
>     - In non-curses mode, readline and gdb use the standard input and
>     standard output/error directly.
> @@ -837,6 +841,14 @@ tui_setup_io (int mode)
>  {
>    extern int _rl_echoing_p;
>  
> +  if (tui_io_mode == mode)
> +    {
> +      /* Nothing to do.  Also, this makes sure that we don't try to restore
> +	 things before having saved them. */
> +      return;
> +    }
> +  tui_io_mode = mode;
> +
>    if (mode)
>      {
>        /* Ensure that readline has been initialized before saving any
>
> base-commit: 646982f4295bf0a1e64867606d496c34c9a15a0c
> -- 
> 2.51.0

Re: [PATCH] [gdb/tui] Make tui_setup_io more robust

[Tom de Vries]

On 4/2/26 11:18 AM, Andrew Burgess wrote:
> Tom de Vries <tdevries@suse.de> writes:
> 
>> The following sequence of events may happen when enabling TUI:
>> - tui_enable () is called,
>> - a SIGFPE happens, before tui_setup_io (1) is called,
>> - the SIGFPE triggers handle_fatal_signal, which calls tui_disable (),
>> - during tui_disable (), tui_setup_io (0) is called, and
>> - tui_setup_io (0) tries to restore things that were saved during a previous
>>    tui_setup_io (1) call, but tui_setup_io (1) was never called so the saving
>>    never happened.
>>
>> This can cause current_ui.m_ui_stderr to be nullptr, which then can cause a
>> crash in sig_write (PR33918).
> 
> If this bug number is worth mentioning, then we should add a 'Bug:...'
> tag to the commit message.  I like them if for nothing else they are
> clickable in my terminal :)
Hi Andrew,

thanks for the review.

I didn't add the tag, because I usually only add the tag if the commit 
claims to fix the PR, and I considered the problem fixed by this patch 
to be different from the one in the PR (I could have filed a another PR, 
but didn't bother).   But that is a bit of a strict approach, so anyway, 
added.

>> Fix this by:
>> - adding a variable tui_io_mode, and
>> - using the variable to bail out of tui_setup_io in case the current mode
>>    doesn't change.
> 
> What's one more global, the tui already has so many!

I tried to explore a scenario where this mattered, and ran into a 
problem, for which I filed a PR ( 
https://sourceware.org/pipermail/gdb-prs/2026q2/047033.html ).
But I might also file a PR for the generic problem.

> This seems a reasonable change to me.
> 
> Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks, pushed.

- Tom

> Thanks,
> Andrew
> 
> 
>>
>> Tested on x86_64-linux.
>> ---
>>   gdb/tui/tui-io.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
>> index f673fbf36f6..5a3bc604592 100644
>> --- a/gdb/tui/tui-io.c
>> +++ b/gdb/tui/tui-io.c
>> @@ -824,6 +824,10 @@ tui_rl_display_match_list (char **matches, int len, int max)
>>     gdb_display_match_list (matches, len, max, &displayer);
>>   }
>>   
>> +/* Whether the IO is setup for curses (1) or non-curses (0).  */
>> +
>> +static int tui_io_mode;
>> +
>>   /* Setup the IO for curses or non-curses mode.
>>      - In non-curses mode, readline and gdb use the standard input and
>>      standard output/error directly.
>> @@ -837,6 +841,14 @@ tui_setup_io (int mode)
>>   {
>>     extern int _rl_echoing_p;
>>   
>> +  if (tui_io_mode == mode)
>> +    {
>> +      /* Nothing to do.  Also, this makes sure that we don't try to restore
>> +	 things before having saved them. */
>> +      return;
>> +    }
>> +  tui_io_mode = mode;
>> +
>>     if (mode)
>>       {
>>         /* Ensure that readline has been initialized before saving any
>>
>> base-commit: 646982f4295bf0a1e64867606d496c34c9a15a0c
>> -- 
>> 2.51.0
> 

Re: [PATCH] [gdb/tui] Make tui_setup_io more robust

[Tom de Vries]

On 4/2/26 11:22 PM, Tom de Vries wrote:
> But I might also file a PR for the generic problem.

I ended up filing this PR ( 
https://sourceware.org/bugzilla/show_bug.cgi?id=34039 ).

Thanks,
- Tom

Re: [PATCH] [gdb/tui] Make tui_setup_io more robust

[Tom de Vries]

On 4/2/26 11:18 AM, Andrew Burgess wrote:
> The following sequence of events may happen when enabling TUI:
> - tui_enable () is called,
> - a SIGFPE happens, before tui_setup_io (1) is called,
> - the SIGFPE triggers handle_fatal_signal, which calls tui_disable (),
> - during tui_disable (), tui_setup_io (0) is called, and
> - tui_setup_io (0) tries to restore things that were saved during a previous
>    tui_setup_io (1) call, but tui_setup_io (1) was never called so the saving
>    never happened.
> 
> This can cause current_ui.m_ui_stderr to be nullptr

I've written a selftest for this ( 
https://sourceware.org/pipermail/gdb-patches/2026-April/226319.html ).

Thanks,
- Tom