[PING][PATCH v3] [gdb] Fix segfault in sig_write

Tom de Vries <[email protected]> Thu, 30 Jul 2026 10:17:14 +0200
Newsgroups gmane.comp.gdb.patches
Message-ID <[email protected]>
On 4/23/26 4:43 PM, Tom de Vries wrote:
> I ran into a segfault in sig_write:
> ...
>    if (gdb_stderr == nullptr || gdb_stderr->fd () == -1)
> ...
> 
> [ A regression since commit 817003ed469 ("Rewrite output redirection and
> logging"). ]
> 
> This happened as follows.
> 
> First, we get gdb_stderr by calling current_gdb_stderr, which returns
> current_ui.current_interpreter.m_stderr.get (), which is not nullptr.
> 
> Then we do "gdb_stderr->fd ()", which gets us to
> wrapped_file<ui::ui_file_ptr<&ui::m_ui_stderr> >::fd:
> ...
>    int fd () const override
>    { return m_stream->fd (); }
> ...
> 
> The "m_stream->" part brings us to:
> ...
>    /* A "smart pointer" that references a particular member of the
>       current UI.  */
>    template<ui_file *ui::* F>
>    struct ui_file_ptr
>    {
>      ui_file *operator-> () const
>      {
>        return current_ui->*F;
>      }
>    };
> ...
> which does "current_ui.m_ui_stderr" which indeed is nullptr.
> 
> So why was current_ui.m_ui_stderr nullptr?  This was due a problem fixed by
> commit b171f68e945 ("[gdb/tui] Make tui_setup_io more robust").
> 
> So, atm as far as we know current_ui.m_ui_stderr is never nullptr.
> 
> However, sig_write has special requirements because it's called when
> "things go wrong", so we want it to work reliably in case things going wrong
> means that again current_ui.m_ui_stderr is nullptr.
> 
> Fix this in wrapped_file::fd by checking for a nullptr m_stream.
> 

Ping.

Thanks,
- Tom

> A v1 was submitted here [1].
> 
> Changes in v2 [2]:
> - changed approach of fixing this: rather than trying to fix this in sig_write
>    by avoiding the use of gdb_stderr, fix this in wrapped_file::fd.
> - added unit test
> 
> Changes in v3:
> - extended commit message to be more clear about how current_ui.m_ui_stderr
>    got to be a nullptr
> 
> Tested on x86_64-linux.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33918
> 
> [1] v1 https://sourceware.org/pipermail/gdb-patches/2026-February/225155.html
> [2] v2 https://sourceware.org/pipermail/gdb-patches/2026-April/226295.html
> ---
>   gdb/ui-file.h                     |  2 +-
>   gdb/ui.h                          |  4 ++++
>   gdb/unittests/ui-file-selftests.c | 14 ++++++++++++++
>   3 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/ui-file.h b/gdb/ui-file.h
> index 6a1d3964335..b7d3aebda9a 100644
> --- a/gdb/ui-file.h
> +++ b/gdb/ui-file.h
> @@ -420,7 +420,7 @@ class wrapped_file : public ui_file
>     { m_stream->emit_style_escape (style); }
>   
>     int fd () const override
> -  { return m_stream->fd (); }
> +  { return m_stream == nullptr ? -1 : m_stream->fd (); }
>   
>     void puts_unfiltered (const char *str) override
>     { m_stream->puts_unfiltered (str); }
> diff --git a/gdb/ui.h b/gdb/ui.h
> index 891660896ef..ef977470302 100644
> --- a/gdb/ui.h
> +++ b/gdb/ui.h
> @@ -163,6 +163,10 @@ struct ui
>       {
>         return current_ui->*F;
>       }
> +    bool operator== (nullptr_t p) const
> +    {
> +      return current_ui->*F == nullptr;
> +    }
>     };
>   
>     /* A ui_file that simply forwards.  */
> diff --git a/gdb/unittests/ui-file-selftests.c b/gdb/unittests/ui-file-selftests.c
> index 69e48735001..b9c1aca3774 100644
> --- a/gdb/unittests/ui-file-selftests.c
> +++ b/gdb/unittests/ui-file-selftests.c
> @@ -48,6 +48,20 @@ run_tests ()
>     scoped_restore save_7 = make_scoped_restore (&sevenbit_strings, true);
>     check_one ("more weird stuff: \xa5", '\\',
>   	     "more weird stuff: \\245");
> +
> +  {
> +    /* There's a bug that has the effect "*redirectable_stderr () == nullptr".
> +       In that case, gdb_stderr is not nullptr, but the underlying pointer is
> +       a nullptr.  Check that "gdb_stderr->fd ()" doesn't dereference the
> +       underlying nullptr.
> +       This allows us to check for a usable gdb_stderr using
> +       "gdb_stderr != nullptr && gdb_stderr->fd () == -1", as we do in
> +       sig_write.  */
> +    scoped_restore restore_stderr
> +      = make_scoped_restore (redirectable_stderr (), nullptr);
> +    SELF_CHECK (gdb_stderr != nullptr);
> +    SELF_CHECK (gdb_stderr->fd () == -1);
> +  }
>   }
>   
>   } /* namespace file*/
> 
> base-commit: 8dc535c59fdbcf99e28703425e5c1c711e63a1f1