From 72994b6028360eccb5d25b39d2e18b386d091426 Mon Sep 17 00:00:00 2001 From: Lancelot SIX Date: Mon, 13 Dec 2021 07:48:48 -0600 Subject: [PATCH] gdb/tui: install SIGWINCH only when connected to a TTY PR26056 reports that when GDB is connected to non-TTY stdin/stdout, it crashes when it receives a SIGWINCH signal. This can be reproduced as follows: $ gdb/gdb -nx -batch -ex 'run' --args sleep 60 &1 | cat # from another terminal: $ kill -WINCH %(pidof gdb) When doing so, the process crashes in a call to rl_resize_terminal: void rl_resize_terminal (void) { _rl_get_screen_size (fileno (rl_instream), 1); ... } The problem is that at this point rl_instream has the value NULL. The rl_instream variable is supposed to be initialized during a call to readline_initialize_everything, which in a normal startup sequence is called under this call chain: tui_interp::init tui_ensure_readline_initialized rl_initialize readline_initialize_everything In tui_interp::init, we have the following sequence: tui_initialize_io (); tui_initialize_win (); // <- Installs SIGWINCH if (gdb_stdout->isatty ()) tui_ensure_readline_initialized (); // <- Initializes rl_instream This function unconditionally installs the SIGWINCH signal handler (this is done by tui_initialize_win), and then if gdb_stdout is a TTY it initializes readline. Therefore, if stdout is not a TTY, SIGWINCH is installed but readline is not initialized. In such situation rl_instream stays NULL, and when GDB receives a SIGWINCH it calls its handler and in fine tries to access rl_instream leading to the crash. This patch proposes to fix this issue by installing the SIGWINCH signal handler only if GDB is connected to a TTY. Given that this initialization it the only task of tui_initialize_win, this patch moves tui_initialize_win just after the call to tui_ensure_readline_initialized. Tested on x86_64-linux. Co-authored-by: Pedro Alves Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26056 Change-Id: I6458acef7b0d9beda2a10715d0345f02361076d9 --- gdb/testsuite/gdb.base/sigwinch-notty.exp | 70 +++++++++++++++++++++++ gdb/testsuite/lib/gdb.exp | 4 +- gdb/testsuite/lib/notty-wrap | 24 ++++++++ gdb/tui/tui-interp.c | 10 +++- 4 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 gdb/testsuite/gdb.base/sigwinch-notty.exp create mode 100755 gdb/testsuite/lib/notty-wrap diff --git a/gdb/testsuite/gdb.base/sigwinch-notty.exp b/gdb/testsuite/gdb.base/sigwinch-notty.exp new file mode 100644 index 00000000000..9084c258388 --- /dev/null +++ b/gdb/testsuite/gdb.base/sigwinch-notty.exp @@ -0,0 +1,70 @@ +# Copyright 2021 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 . + +# Test that GDB does not crash when it is started without a terminal / +# without readline, and, it receives a SIGWINCH. Regression test for +# PR gdb/26056. + +if [target_info exists gdb,nosignals] { + verbose "Skipping $subdir/$gdb_test_file_name.exp because of nosignals." + continue +} + +# The testfile relies on "run" from the command line, so only works +# with "target native". +if { [target_info gdb_protocol] != "" } { + continue +} + +gdb_exit + +# Start GDB without a terminal, running sleep for a while. Before the +# sleep exits, we'll send a SIGWINCH. "show editing" to double check +# that readline is disabled. +save_vars { GDB GDBFLAGS } { + set GDB "$srcdir/lib/notty-wrap $GDB" + set GDBFLAGS "$GDBFLAGS -ex \"show editing\" -ex run --args sleep 3" + + gdb_spawn +} + +set gdb_pid [exp_pid -i $gdb_spawn_id] + +verbose -log "gdb_spawn_id=$gdb_spawn_id" +verbose -log "gdb_pid=$gdb_pid" + +after 1000 { + # Note, GDB is started under a shell, so PID is actually the + # shell's pid, not GDB's. Use "-PID" to send the signal to the + # whole process group and reach GDB, instead of just to the shell. + remote_exec host "kill -SIGWINCH -${gdb_pid}" +} + +# If GDB mishandles the SIGWINCH and crashes, that happens before we +# see the "inferior exited normally" message, so this will ERROR with +# eof. +gdb_test_multiple "" "wait for sleep exit" { + -re "Editing of command lines as they are typed is off.*$inferior_exited_re normally.*$gdb_prompt " { + pass $gdb_test_name + } +} + +gdb_test_multiple "" "wait for gdb exit" { + eof { + set wait_status [wait -i $gdb_spawn_id] + verbose -log "GDB process exited with wait status $wait_status" + pass $gdb_test_name + } +} diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 95220f6fc8d..b952c93c0d5 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -2096,7 +2096,9 @@ proc default_gdb_spawn { } { exit 1 } } - set res [remote_spawn host "$GDB $INTERNAL_GDBFLAGS $GDBFLAGS [host_info gdb_opts]"] + + # Put GDBFLAGS last so that tests can put "--args ..." in it. + set res [remote_spawn host "$GDB $INTERNAL_GDBFLAGS [host_info gdb_opts] $GDBFLAGS"] if { $res < 0 || $res == "" } { perror "Spawning $GDB failed." return 1 diff --git a/gdb/testsuite/lib/notty-wrap b/gdb/testsuite/lib/notty-wrap new file mode 100755 index 00000000000..899c2ddfdd5 --- /dev/null +++ b/gdb/testsuite/lib/notty-wrap @@ -0,0 +1,24 @@ +#!/bin/sh + +# Copyright (C) 2021 Free Software Foundation, Inc. +# +# This file is part of GDB. +# +# 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 +# . + +# Wrap any passed-in program and args in a pipe, so that the program +# is started without a terminal. + +exec "$@" &1 | cat diff --git a/gdb/tui/tui-interp.c b/gdb/tui/tui-interp.c index 0cd5b125919..16859cb5745 100644 --- a/gdb/tui/tui-interp.c +++ b/gdb/tui/tui-interp.c @@ -242,9 +242,15 @@ tui_interp::init (bool top_level) atexit (tui_exit); tui_initialize_io (); - tui_initialize_win (); if (gdb_stdout->isatty ()) - tui_ensure_readline_initialized (); + { + tui_ensure_readline_initialized (); + + /* This installs the SIGWINCH signal handler. The handler needs to do + readline calls (to rl_resize_terminal), so it must not be installed + unless readline is properly initialized. */ + tui_initialize_win (); + } } /* Used as the command handler for the tui. */ -- 2.30.2