[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