Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH 0/3] [gdb] Fix backtrace in log file
@ 2026-10-23  7:38 Tom de Vries
  2026-10-23  7:38 ` [PATCH 1/3] [gdb] Fix backtrace in log file (libbacktrace case) Tom de Vries
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Tom de Vries @ 2026-10-23  7:38 UTC (permalink / raw)
  To: gdb-patches

This series contains three patches.

The first and last fix a problem with the backtrace not showing up in log
files, one for libbacktrace and one for execinfo.

The second drops an unnecessary flush.

An earlier attempt was submitted here [1], which tried to fix the problem by
adding a logging_file::fd.  This series tries to follow up on review comments.

Tested on x86_64-linux.

Also tested the added test-case with --disable-libbacktrace.

[1] https://sourceware.org/pipermail/gdb-patches/2026-October/231239.html

Tom de Vries (3):
  [gdb] Fix backtrace in log file (libbacktrace case)
  [gdb] Don't flush in handle_fatal_signal
  [gdb] Fix backtrace in log file (execinfo case)

 gdb/bt-utils.c                                | 21 ++++--
 gdb/cli/cli-logging.c                         | 22 +++++-
 gdb/event-top.c                               |  8 --
 gdb/logging-file.h                            |  3 +-
 gdb/mi/mi-console.c                           |  7 +-
 gdb/mi/mi-console.h                           |  2 +-
 .../gdb.base/logging-internal-error.exp       | 75 +++++++++++++++++++
 gdb/ui-file.c                                 | 17 +----
 gdb/ui-file.h                                 | 36 +++++++--
 9 files changed, 147 insertions(+), 44 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/logging-internal-error.exp


base-commit: 21cffe7c0955c6bdb0c14ded96939d409f3c21f0
-- 
2.51.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/3] [gdb] Fix backtrace in log file (libbacktrace case)
  2026-10-23  7:38 [PATCH 0/3] [gdb] Fix backtrace in log file Tom de Vries
@ 2026-10-23  7:38 ` Tom de Vries
  2026-10-23  7:38 ` [PATCH 2/3] [gdb] Don't flush in handle_fatal_signal Tom de Vries
  2026-10-23  7:38 ` [PATCH 3/3] [gdb] Fix backtrace in log file (execinfo case) Tom de Vries
  2 siblings, 0 replies; 4+ messages in thread
From: Tom de Vries @ 2026-10-23  7:38 UTC (permalink / raw)
  To: gdb-patches

Consider test-case gdb.base/logging-internal-error.exp (added by this patch).

It enables logging to a file log.txt, and then does "maint internal-error".

In gdb.log we find the expected output:
...
(gdb) maint internal-error
gdb/maint.c:82: internal-error:
A problem internal to GDB has been detected,
further debugging may prove unreliable.
----- Backtrace -----
0x659d42 gdb_internal_backtrace_1
        gdb/bt-utils.c:122
  ...
0x419554 main
        gdb/gdb.c:38
---------------------

This is a bug, please report it.  For instructions, see:
<https://www.gnu.org/software/gdb/bugs/>.

Command aborted.
(gdb)
...

But in log.txt, the backtrace is missing:
...
gdb/maint.c:82: internal-error:
A problem internal to GDB has been detected,
further debugging may prove unreliable.

This is a bug, please report it.  For instructions, see:
<https://www.gnu.org/software/gdb/bugs/>.

Command aborted.
...

The backtrace is printed using sig_write:
...
void
sig_write (const char *msg)
{
  if (gdb_stderr == nullptr || gdb_stderr->fd () == -1)
    std::ignore = ::write (2, msg, strlen (msg));
  else
    gdb_stderr->write_async_safe (msg, strlen (msg));
}
...

Because logging_file::fd () == -1, plain write is used instead of
write_async_safe.  Consequently, the message is printed, but not
recorded in the log file.

Presumably the fd () == -1 check in sig_write is there to avoid running into
the default implementation of write_async_safe:
...
  virtual void write_async_safe (const char *buf, long length_buf)
  { gdb_assert_not_reached ("write_async_safe"); }
...
when gdb_stderr is temporarily set to a string_file.

Fix this by:
- making the default write_async_safe call do something more useful than
  asserting: a write to fd (),
- making write_async_safe return true if the write was handled, and
- using that return value in sig_write to decide whether to call a fallback.
---
 gdb/bt-utils.c                                |  9 ++-
 gdb/cli/cli-logging.c                         |  8 +-
 gdb/logging-file.h                            |  2 +-
 gdb/mi/mi-console.c                           |  7 +-
 gdb/mi/mi-console.h                           |  2 +-
 .../gdb.base/logging-internal-error.exp       | 75 +++++++++++++++++++
 gdb/ui-file.c                                 | 17 +----
 gdb/ui-file.h                                 | 20 +++--
 8 files changed, 108 insertions(+), 32 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/logging-internal-error.exp

diff --git a/gdb/bt-utils.c b/gdb/bt-utils.c
index 21a291c3df3..fcb912c8021 100644
--- a/gdb/bt-utils.c
+++ b/gdb/bt-utils.c
@@ -45,10 +45,11 @@ gdb_internal_backtrace_set_cmd (const char *args, int from_tty,
 void
 sig_write (const char *msg)
 {
-  if (gdb_stderr == nullptr || gdb_stderr->fd () == -1)
-    std::ignore = ::write (2, msg, strlen (msg));
-  else
-    gdb_stderr->write_async_safe (msg, strlen (msg));
+  size_t len = strlen (msg);
+
+  if (gdb_stderr == nullptr
+      || !gdb_stderr->write_async_safe (msg, len))
+    std::ignore = ::write (2, msg, len);
 }
 
 #ifdef GDB_PRINT_INTERNAL_BACKTRACE
diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c
index c9482d59199..d8212de4abb 100644
--- a/gdb/cli/cli-logging.c
+++ b/gdb/cli/cli-logging.c
@@ -174,13 +174,15 @@ logging_file<T>::write (const char *buf, long length_buf)
 /* See logging-file.h.  */
 
 template<typename T>
-void
+bool
 logging_file<T>::write_async_safe (const char *buf, long length_buf)
 {
+  bool handled = false;
   if (log_file != nullptr)
-    log_file->write_async_safe (buf, length_buf);
+    handled = log_file->write_async_safe (buf, length_buf);
   if (ordinary_output ())
-    m_out->write_async_safe (buf, length_buf);
+    handled = m_out->write_async_safe (buf, length_buf);
+  return handled;
 }
 
 /* See logging-file.h.  */
diff --git a/gdb/logging-file.h b/gdb/logging-file.h
index 5dfe1118494..f8b9f101150 100644
--- a/gdb/logging-file.h
+++ b/gdb/logging-file.h
@@ -41,7 +41,7 @@ class logging_file : public ui_file
   }
 
   void write (const char *buf, long length_buf) override;
-  void write_async_safe (const char *buf, long length_buf) override;
+  bool write_async_safe (const char *buf, long length_buf) override;
   void puts (const char *) override;
   void flush () override;
   bool can_page () const override;
diff --git a/gdb/mi/mi-console.c b/gdb/mi/mi-console.c
index 1a10bfa21fb..a5f69691277 100644
--- a/gdb/mi/mi-console.c
+++ b/gdb/mi/mi-console.c
@@ -47,10 +47,12 @@ mi_console_file::write (const char *buf, long length_buf)
     this->flush ();
 }
 
-void
+bool
 mi_console_file::write_async_safe (const char *buf, long length_buf)
 {
-  m_raw->write_async_safe (m_prefix, strlen (m_prefix));
+  if (!m_raw->write_async_safe (m_prefix, strlen (m_prefix)))
+    return false;
+
   if (m_quote)
     {
       m_raw->write_async_safe (&m_quote, 1);
@@ -62,6 +64,7 @@ mi_console_file::write_async_safe (const char *buf, long length_buf)
 
   char nl = '\n';
   m_raw->write_async_safe (&nl, 1);
+  return true;
 }
 
 void
diff --git a/gdb/mi/mi-console.h b/gdb/mi/mi-console.h
index 8dea558fb9b..3e5618495ee 100644
--- a/gdb/mi/mi-console.h
+++ b/gdb/mi/mi-console.h
@@ -36,7 +36,7 @@ class mi_console_file : public ui_file
 
   void write (const char *buf, long length_buf) override;
 
-  void write_async_safe (const char *buf, long length_buf) override;
+  bool write_async_safe (const char *buf, long length_buf) override;
 
 private:
   /* The wrapped raw output stream.  */
diff --git a/gdb/testsuite/gdb.base/logging-internal-error.exp b/gdb/testsuite/gdb.base/logging-internal-error.exp
new file mode 100644
index 00000000000..b121ca42612
--- /dev/null
+++ b/gdb/testsuite/gdb.base/logging-internal-error.exp
@@ -0,0 +1,75 @@
+# Copyright 2026 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that the backtrace generated by "maint internal-error", if any, also
+# ends up in the log file.
+
+require {!isremote host}
+
+# Do not run if gdb debug is enabled as it will interfere with logging.
+require !gdb_debug_enabled
+
+clean_restart
+
+# Don't quit, don't produce a core file.
+gdb_test_no_output "maint set internal-error corefile no"
+gdb_test_no_output "maint set internal-error quit no"
+
+set log_name [standard_output_file log.txt]
+gdb_test_no_output "set logging file $log_name" \
+    "set logging filename"
+gdb_test_no_output "set logging overwrite on"
+gdb_test "set logging enabled on" "Copying output to .*"
+
+set seen_bt 0
+set re_bt [string_to_regexp "----- Backtrace -----"]
+
+set cmd "maint internal-error"
+send_gdb "$cmd\n"
+set re \
+    [multi_line \
+	 [string_to_regexp "Command aborted."] \
+	 "$gdb_prompt $"]
+gdb_expect {
+    -re $re_bt {
+	set seen_bt 1
+	exp_continue
+    }
+    -re $re {
+	pass $cmd
+    }
+    -re "$gdb_prompt $" {
+	fail $cmd
+    }
+    timeout {
+	fail "$cmd (timeout)"
+    }
+    eof {
+	fail "$cmd (eof)"
+    }
+}
+
+gdb_test "set logging enabled off" "Done logging to .*"
+
+set fd [open $log_name]
+set data [read -nonewline $fd]
+close $fd
+
+set test "Logged backtrace"
+if {$seen_bt} {
+    gdb_assert {[regexp $re_bt $data]} $test
+} else {
+    unsupported $test
+}
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index 006bdc923e4..85de5440f0c 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -157,10 +157,11 @@ null_file::puts (const char *)
   /* Discard the request.  */
 }
 
-void
+bool
 null_file::write_async_safe (const char *buf, long sizeof_buf)
 {
-  /* Discard the request.  */
+  /* Discard the request.  We consider it handled though.  */
+  return true;
 }
 
 \f
@@ -271,18 +272,6 @@ stdio_file::write (const char *buf, long length_buf)
     }
 }
 
-void
-stdio_file::write_async_safe (const char *buf, long length_buf)
-{
-  /* This is written the way it is to avoid a warning from gcc about not using the
-     result of write (since it can be declared with attribute warn_unused_result).
-     Alas casting to void doesn't work for this.  */
-  if (::write (m_fd, buf, length_buf))
-    {
-      /* Nothing.  */
-    }
-}
-
 void
 stdio_file::puts (const char *linebuffer)
 {
diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index f5c23b9b6d9..23cbd2c7e78 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -20,6 +20,7 @@
 #define GDB_UI_FILE_H
 
 #include <string>
+#include <tuple>
 #include "ui-style.h"
 
 /* The abstract ui_file base class.  */
@@ -67,9 +68,16 @@ class ui_file
      first.  Implementations are also free to ignore some or all of
      the request.  puts_async is not provided as the async versions
      are rarely used, no point in having both for a rarely used
-     interface.  */
-  virtual void write_async_safe (const char *buf, long length_buf)
-  { gdb_assert_not_reached ("write_async_safe"); }
+     interface.  This default implementation writes to fd ().  Return true if
+     the write was handled.  */
+  virtual bool write_async_safe (const char *buf, long length_buf)
+  {
+    int fd = this->fd ();
+    if (fd == -1)
+      return false;
+    std::ignore = ::write (fd, buf, length_buf);
+    return true;
+  }
 
   /* Some ui_files override this to provide a efficient implementation
      that avoids a strlen.  */
@@ -163,7 +171,7 @@ class null_file : public ui_file
 {
 public:
   void write (const char *buf, long length_buf) override;
-  void write_async_safe (const char *buf, long sizeof_buf) override;
+  bool write_async_safe (const char *buf, long sizeof_buf) override;
   void puts (const char *str) override;
 };
 
@@ -267,8 +275,6 @@ class stdio_file : public ui_file
 
   void write (const char *buf, long length_buf) override;
 
-  void write_async_safe (const char *buf, long length_buf) override;
-
   void puts (const char *) override;
 
   long read (char *buf, long length_buf) override;
@@ -436,7 +442,7 @@ class wrapped_file : public ui_file
   void write (const char *buf, long length_buf) override
   { return m_stream->write (buf, length_buf); }
 
-  void write_async_safe (const char *buf, long length_buf) override
+  bool write_async_safe (const char *buf, long length_buf) override
   { return m_stream->write_async_safe (buf, length_buf); }
 
   bool can_page () const override
-- 
2.51.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/3] [gdb] Don't flush in handle_fatal_signal
  2026-10-23  7:38 [PATCH 0/3] [gdb] Fix backtrace in log file Tom de Vries
  2026-10-23  7:38 ` [PATCH 1/3] [gdb] Fix backtrace in log file (libbacktrace case) Tom de Vries
@ 2026-10-23  7:38 ` Tom de Vries
  2026-10-23  7:38 ` [PATCH 3/3] [gdb] Fix backtrace in log file (execinfo case) Tom de Vries
  2 siblings, 0 replies; 4+ messages in thread
From: Tom de Vries @ 2026-10-23  7:38 UTC (permalink / raw)
  To: gdb-patches

In handle_fatal_signal we use "gdb_stderr->flush ()".

In this function, we only want to use async-safe output, and flushing isn't.

Furthermore, all output is done using sig_write, which is unbuffered, so
there's nothing to flush.

Drop it.
---
 gdb/event-top.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 0fb6eafec57..78683ec364f 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -1016,14 +1016,6 @@ handle_fatal_signal (int sig)
 	  sig_write (".");
 	}
       sig_write ("\n\n");
-
-      if (gdb_stderr == nullptr || gdb_stderr->fd () == -1)
-	{
-	  /* Writing to file descriptor instead of stream, no flush
-	     required.  */
-	}
-      else
-	gdb_stderr->flush ();
     }
 #endif
 
-- 
2.51.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 3/3] [gdb] Fix backtrace in log file (execinfo case)
  2026-10-23  7:38 [PATCH 0/3] [gdb] Fix backtrace in log file Tom de Vries
  2026-10-23  7:38 ` [PATCH 1/3] [gdb] Fix backtrace in log file (libbacktrace case) Tom de Vries
  2026-10-23  7:38 ` [PATCH 2/3] [gdb] Don't flush in handle_fatal_signal Tom de Vries
@ 2026-10-23  7:38 ` Tom de Vries
  2 siblings, 0 replies; 4+ messages in thread
From: Tom de Vries @ 2026-10-23  7:38 UTC (permalink / raw)
  To: gdb-patches

Consider test-case gdb.base/logging-internal-error.exp.

It enables logging to a file log.txt, and then does "maint internal-error".

Now say we build gdb with --disable-libbacktrace, and run the test-case.

In gdb.log we find the expected output:
...
(gdb) maint internal-error
gdb/maint.c:82: internal-error:
A problem internal to GDB has been detected,
further debugging may prove unreliable.
----- Backtrace -----
gdb/gdb() [0x659b60]
  ...
gdb/gdb() [0xbb2f72]
Backtrace might be incomplete.
---------------------

This is a bug, please report it.  For instructions, see:
<https://www.gnu.org/software/gdb/bugs/>.

Command aborted.
(gdb)
...

But in log.txt, we have:
...
gdb/maint.c:82: internal-error:
A problem internal to GDB has been detected,
further debugging may prove unreliable.
----- Backtrace -----
Backtrace might be incomplete.
---------------------

This is a bug, please report it.  For instructions, see:
<https://www.gnu.org/software/gdb/bugs/>.

Command aborted.
...

Make the test-case detect this by making a regexp a bit stricter.

The problem is in gdb_internal_backtrace_1, where we do:
...
  int fd = ((gdb_stderr == nullptr || gdb_stderr->fd () == -1)
            ? 2
            : gdb_stderr->fd ());
  backtrace_symbols_fd (buffer, frames, fd);
...

We want to write to two files, but there's just one file descriptor.

Fix this by adding a new member function ui_file::for_each_fd, and using it in
gdb_internal_backtrace_1 to call backtrace_symbols_fd.
---
 gdb/bt-utils.c                                   | 12 ++++++++----
 gdb/cli/cli-logging.c                            | 14 ++++++++++++++
 gdb/logging-file.h                               |  1 +
 .../gdb.base/logging-internal-error.exp          |  2 +-
 gdb/ui-file.h                                    | 16 ++++++++++++++++
 5 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/gdb/bt-utils.c b/gdb/bt-utils.c
index fcb912c8021..b0000e92da2 100644
--- a/gdb/bt-utils.c
+++ b/gdb/bt-utils.c
@@ -134,10 +134,14 @@ gdb_internal_backtrace_1 ()
   void *buffer[25];
   int frames = backtrace (buffer, ARRAY_SIZE (buffer));
 
-  int fd = ((gdb_stderr == nullptr || gdb_stderr->fd () == -1)
-	    ? 2
-	    : gdb_stderr->fd ());
-  backtrace_symbols_fd (buffer, frames, fd);
+  bool handled = (gdb_stderr != nullptr
+		  && gdb_stderr->for_each_fd ([&] (int fd)
+		  {
+		    backtrace_symbols_fd (buffer, frames, fd);
+		  }));
+  if (!handled)
+    backtrace_symbols_fd (buffer, frames, 2);
+
   if (frames == ARRAY_SIZE (buffer))
     sig_write (_("Backtrace might be incomplete.\n"));
 }
diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c
index d8212de4abb..53fc66dda56 100644
--- a/gdb/cli/cli-logging.c
+++ b/gdb/cli/cli-logging.c
@@ -187,6 +187,20 @@ logging_file<T>::write_async_safe (const char *buf, long length_buf)
 
 /* See logging-file.h.  */
 
+template<typename T>
+bool
+logging_file<T>::for_each_fd (gdb::function_view<void (int)> f)
+{
+  bool handled = false;
+  if (log_file != nullptr)
+    handled = log_file->for_each_fd (f);
+  if (ordinary_output ())
+    handled = m_out->for_each_fd (f);
+  return handled;
+}
+
+/* See logging-file.h.  */
+
 template<typename T>
 void
 logging_file<T>::puts (const char *linebuffer)
diff --git a/gdb/logging-file.h b/gdb/logging-file.h
index f8b9f101150..7a4c265ed08 100644
--- a/gdb/logging-file.h
+++ b/gdb/logging-file.h
@@ -47,6 +47,7 @@ class logging_file : public ui_file
   bool can_page () const override;
   void emit_style_escape (const ui_file_style &style) override;
   void puts_unfiltered (const char *str) override;
+  bool for_each_fd (gdb::function_view<void (int)> f) override;
 
   bool isatty () override
   {
diff --git a/gdb/testsuite/gdb.base/logging-internal-error.exp b/gdb/testsuite/gdb.base/logging-internal-error.exp
index b121ca42612..9a46ed7c3e2 100644
--- a/gdb/testsuite/gdb.base/logging-internal-error.exp
+++ b/gdb/testsuite/gdb.base/logging-internal-error.exp
@@ -69,7 +69,7 @@ close $fd
 
 set test "Logged backtrace"
 if {$seen_bt} {
-    gdb_assert {[regexp $re_bt $data]} $test
+    gdb_assert {[regexp $re_bt.*$hex $data]} $test
 } else {
     unsupported $test
 }
diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index 23cbd2c7e78..6ef9722ead8 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -22,6 +22,7 @@
 #include <string>
 #include <tuple>
 #include "ui-style.h"
+#include "gdbsupport/function-view.h"
 
 /* The abstract ui_file base class.  */
 
@@ -117,6 +118,18 @@ class ui_file
   virtual int fd () const
   { return -1; }
 
+  /* Use this function to write to a file descriptor, rather than using
+     write (fd (), ...).  In some cases, multiple descriptors need to be
+     written.  Return true if the write was handled.  */
+  virtual bool for_each_fd (gdb::function_view<void (int)> f)
+  {
+    int fd = this->fd ();
+    if (fd == -1)
+      return false;
+    f (fd);
+    return true;
+  }
+
   /* Return true if this object supports paging, false otherwise.  */
   virtual bool can_page () const
   {
@@ -436,6 +449,9 @@ class wrapped_file : public ui_file
   int fd () const override
   { return m_stream->fd (); }
 
+  bool for_each_fd (gdb::function_view<void (int)> f) override
+  { return m_stream->for_each_fd (f); }
+
   void puts_unfiltered (const char *str) override
   { m_stream->puts_unfiltered (str); }
 
-- 
2.51.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-23  7:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-23  7:38 [PATCH 0/3] [gdb] Fix backtrace in log file Tom de Vries
2026-10-23  7:38 ` [PATCH 1/3] [gdb] Fix backtrace in log file (libbacktrace case) Tom de Vries
2026-10-23  7:38 ` [PATCH 2/3] [gdb] Don't flush in handle_fatal_signal Tom de Vries
2026-10-23  7:38 ` [PATCH 3/3] [gdb] Fix backtrace in log file (execinfo case) Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox