From f42bd4f05dedeaf2da9b5a5e91c9a66516c8da76 Mon Sep 17 00:00:00 2001
From: Andrew Burgess <aburgess@redhat.com>
Date: Fri, 12 Dec 2025 11:27:53 +0000
Subject: [PATCH] gdb: fix crash from 'shell' when GDB has no terminal

Bug PR gdb/33716 identifies a problem where GDB will crash when using
the 'shell' command if GDB is not attached to a terminal.  E.g.:

  $ gdb -nw -nh -q -batch -ex 'shell echo hello' </dev/null
  hello

  Fatal signal: Segmentation fault
  ----- Backtrace -----
  ... etc ...

This problem was introduced with commit:

  commit 3a7f7d0be3a2953fd110f084b34586ce9c649d66
  Date:   Fri Jul 25 19:07:59 2025 +0200

      [gdb/tui] Fix shell command terminal settings

This commit introduces 'class scoped_gdb_ttystate', a RAII class which
backs up and restores the current terminal state.  Unfortunately, the
implementation tries to backup and restore the state even when GDB is
not attached to a terminal.

If GDB is not attached to a terminal then serial_get_tty_state
will (on a Unix like system) return a NULL pointer.  Calling
serial_set_tty_state with a NULL state object will trigger the crash
seen above.

Elsewhere in GDB, calling serial_set_tty_state is guarded by calling
gdb_has_a_terminal().  I propose that we only try to backup the
terminal state if GDB is actually connected to a terminal.  The call
to serial_set_tty_state will only be made if the backed up tty state
is not NULL.

With this change in place the crash is resolved.  There's a new test
to cover this case.

While I'm here I added a gdb_assert in ser-unix.c which would have
triggered in this case before we saw the crash.  And I've extended the
comment in serial.h to document that serial_get_tty_state can return
NULL.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33716

Approved-By: Simon Marchi <simon.marchi@efficios.com>
---
 gdb/inflow.c                                 |  8 ++-
 gdb/ser-unix.c                               |  1 +
 gdb/serial.h                                 |  3 +-
 gdb/terminal.h                               |  4 +-
 gdb/testsuite/gdb.base/shell-no-terminal.exp | 60 ++++++++++++++++++++
 5 files changed, 72 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/shell-no-terminal.exp

diff --git a/gdb/inflow.c b/gdb/inflow.c
index 0296fdc172e..8423e330287 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -49,6 +49,8 @@
 
 static void pass_signal (int);
 
+static int gdb_has_a_terminal (void);
+
 static void child_terminal_ours_1 (target_terminal_state);
 
 /* Record terminal status separately for debugger and inferior.  */
@@ -59,14 +61,16 @@ static struct serial *stdin_serial;
 
 scoped_restore_tty_state::scoped_restore_tty_state ()
 {
-  m_ttystate = serial_get_tty_state (stdin_serial);
+  if (gdb_has_a_terminal ())
+    m_ttystate = serial_get_tty_state (stdin_serial);
 }
 
 /* See terminal.h.  */
 
 scoped_restore_tty_state::~scoped_restore_tty_state ()
 {
-  serial_set_tty_state (stdin_serial, m_ttystate);
+  if (m_ttystate != nullptr)
+    serial_set_tty_state (stdin_serial, m_ttystate);
 }
 
 /* Terminal related info we need to keep track of.  Each inferior
diff --git a/gdb/ser-unix.c b/gdb/ser-unix.c
index 553220e5b08..6f2766518be 100644
--- a/gdb/ser-unix.c
+++ b/gdb/ser-unix.c
@@ -150,6 +150,7 @@ hardwire_set_tty_state (struct serial *scb, serial_ttystate ttystate)
   struct hardwire_ttystate *state;
 
   state = (struct hardwire_ttystate *) ttystate;
+  gdb_assert (state != nullptr);
 
   return set_tty_state (scb, state);
 }
diff --git a/gdb/serial.h b/gdb/serial.h
index 1f26dc60210..9c9752f42a1 100644
--- a/gdb/serial.h
+++ b/gdb/serial.h
@@ -152,7 +152,8 @@ extern void serial_send_break (struct serial *scb);
 extern void serial_raw (struct serial *scb);
 
 /* Return a pointer to a newly malloc'd ttystate containing the state
-   of the tty.  */
+   of the tty.  Can return NULL if the current tty state could not be
+   read, for example, if GDB's stdin is not a terminal.  */
 
 extern serial_ttystate serial_get_tty_state (struct serial *scb);
 
diff --git a/gdb/terminal.h b/gdb/terminal.h
index 225554a60c3..892d43113d4 100644
--- a/gdb/terminal.h
+++ b/gdb/terminal.h
@@ -59,7 +59,9 @@ public:
   DISABLE_COPY_AND_ASSIGN (scoped_restore_tty_state);
 
 private:
-  serial_ttystate m_ttystate;
+  /* The saved tty state.  This can remain NULL even after the constructor
+     has run if serial_get_tty_state fails to fetch the tty state.  */
+  serial_ttystate m_ttystate = nullptr;
 };
 
 #ifdef USE_WIN32API
diff --git a/gdb/testsuite/gdb.base/shell-no-terminal.exp b/gdb/testsuite/gdb.base/shell-no-terminal.exp
new file mode 100644
index 00000000000..2a630fb1541
--- /dev/null
+++ b/gdb/testsuite/gdb.base/shell-no-terminal.exp
@@ -0,0 +1,60 @@
+# Copyright 2025 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/>.
+
+# Run GDB in batch mode, with stdin attached to a non-pty.  Use the
+# 'shell' command from the GDB command line.  Check that GDB doesn't
+# crash.  This checks for bug PR gdb/33716.
+
+# Remote boards override the 'remote_spawn' mechanism, and don't
+# support the 'readonly' argument that this test relies on.  Just
+# running this test on local hosts should be fine.
+require {!is_remote host}
+
+gdb_exit
+
+save_vars { GDBFLAGS } {
+    append GDBFLAGS " -batch -ex \"shell echo first\" -ex \"shell echo second\" </dev/null"
+
+    # Inlined default_gdb_spawn.
+    verbose -log "Spawning $GDB $INTERNAL_GDBFLAGS $GDBFLAGS"
+    gdb_write_cmd_file "$GDB $INTERNAL_GDBFLAGS $GDBFLAGS"
+
+    set use_gdb_stub [use_gdb_stub]
+    set res [remote_spawn host "$GDB $INTERNAL_GDBFLAGS [host_info gdb_opts] $GDBFLAGS" "readonly"]
+    if { $res < 0 || $res == "" } {
+	perror "Spawning $GDB failed."
+	return
+    }
+    set gdb_spawn_id $res
+}
+
+# Capture the output of the GDB process.  The above GDB is spawned
+# without a pty (so that we can replace its stdin), and so we don't
+# use '\r\n' as the end of line sequence, instead we expect '\n'.
+set saw_first false
+set saw_second false
+gdb_test_multiple "" "check shell command output" {
+    -re "^first\n" {
+	set saw_first true
+	exp_continue
+    }
+    -re "^second\n" {
+	set saw_second true
+	exp_continue
+    }
+    eof {
+	gdb_assert { $saw_first && $saw_second } $gdb_test_name
+    }
+}