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 3a7f7d0be3
  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>
This commit is contained in:
Andrew Burgess 2025-12-12 11:27:53 +00:00
parent 647c23de32
commit f42bd4f05d
5 changed files with 72 additions and 4 deletions

View file

@ -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

View file

@ -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);
}

View file

@ -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);

View file

@ -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

View file

@ -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
}
}