diff mbox series

[v2] Fix segfault in sig_write

Message ID 20260402084914.946223-1-tdevries@suse.de
State New
Headers
Series [v2] Fix segfault in sig_write |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Tom de Vries April 2, 2026, 8:49 a.m. UTC 
  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 happens 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.

Fix this in wrapped_file::fd by checking for a nullptr m_stream.

A v1 was submitted here [1].

Changes in v2:
- 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

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
---
 gdb/ui-file.h                     |  2 +-
 gdb/ui.h                          |  4 ++++
 gdb/unittests/ui-file-selftests.c | 14 ++++++++++++++
 3 files changed, 19 insertions(+), 1 deletion(-)


base-commit: 8ee110dee1942a20fd206c7107a027e9b04ba56e
diff mbox series

Patch

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*/