Compare commits

...

4 Commits

Author SHA1 Message Date
Pedro Alves
c73fc9439a Add a unit test for scoped_ignore_sigpipe
gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* Makefile.in (SELFTESTS_SRCS): Add
	unittests/scoped_ignore_signal-selftests.c.
	* unittests/scoped_ignore_signal-selftests.c: New.

Change-Id: Idce24aa9432a3f1eb7065bc9aa030b1d0d7dcad5
2021-06-15 12:00:11 +01:00
Pedro Alves
5e9f71e1cd scoped_ignore_signal: Use sigprocmask+sigtimedwait instead of signal
The problem with using signal(...) to temporarily ignore a signal, is
that that changes the the signal disposition for the whole process.
If multiple threads do it at the same time, you have a race.

Fix this by using sigprocmask + sigtimedwait to implement the ignoring
instead, if available, which I think probably means everywhere except
Windows nowadays.  This way, we only change the signal mask for the
current thread, so there's no race.

Change-Id: Idfe3fb08327ef8cae926f3de9ee81c56a83b1738

gdbsupport/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* scoped_ignore_signal.h
	(scoped_ignore_signal::scoped_ignore_signal)
	[HAVE_SIGPROCMASK]: Use sigprocmask to block the signal instead of
	changing the signal disposition for the whole process.
	(scoped_ignore_signal::~scoped_ignore_signal) [HAVE_SIGPROCMASK]:
	Use sigtimedwait and sigprocmask to flush and unblock the signal.
2021-06-15 12:00:11 +01:00
Pedro Alves
68e41281e8 Introduce scoped_restore_signal
We currently have scoped_restore_sigttou and scoped_restore_sigpipe
doing basically the same thing -- temporarily ignoring a specific
signal.

This patch introduce a scoped_restore_signal type that can be used for
both.  This will become more important for the next patch which
changes how the signal-ignoring is implemented.

scoped_restore_sigpipe is a straight alias to
scoped_restore_signal<SIGPIPE> on systems that define SIGPIPE, and an
alias to scoped_restore_signal_nop (a no-op version of
scoped_restore_signal) otherwise.

scoped_restore_sigttou is not a straight alias because it wants to
check the job_control global.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* gdbsupport/scoped_ignore_signal.h: New.
	* compile/compile.c: Include gdbsupport/scoped_ignore_signal.h
	instead of <signal.h>.  Don't include <unistd.h>.
	(scoped_ignore_sigpipe): Remove.
	* gdbsupport/scoped_ignore_sigttou.h: Include gdbsupport/scoped_ignore_signal.h
	instead of <signal.h>.  Don't include <unistd.h>.
	(lazy_init): New.
	(scoped_ignore_sigttou): Reimplement using scoped_ignore_signal
	and lazy_init.

Change-Id: Ibb44d0bd705e96df03ef0787c77358a4a7b7086c
2021-06-15 12:00:10 +01:00
Pedro Alves
148e7536c6 Move scoped_ignore_sigttou to gdbsupport/
A following patch will want to use scoped_ignore_sigttou in code
shared between GDB and GDBserver.  Move it under gdbsupport/.

Note that despite what inflow.h/inflow.c's first line says, inflow.c
is no longer about ptrace, it is about terminal management.  Some
other files were unnecessarily including inflow.h, I guess a leftover
from the days when inflow.c really was about ptrace.  Those inclusions
are simply dropped.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* Makefile.in (HFILES_NO_SRCDIR): Remove inflow.h.
	* inf-ptrace.c, inflow.c, procfs.c: Don't include "inflow.h".
	* inflow.h: Delete, moved to gdbsupport/ under a different name.
	* ser-unix.c: Don't include "inflow.h".  Include
	"gdbsupport/scoped_ignore_sigttou.h".

gdbsupport/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* scoped_ignore_sigttou.h: New file, moved from gdb/ and renamed.

Change-Id: Ie390abf42c3a78bec6d282ad2a63edd3e623559a
2021-06-15 12:00:10 +01:00
9 changed files with 266 additions and 49 deletions

View File

@@ -461,6 +461,7 @@ SELFTESTS_SRCS = \
unittests/mkdir-recursive-selftests.c \ unittests/mkdir-recursive-selftests.c \
unittests/rsp-low-selftests.c \ unittests/rsp-low-selftests.c \
unittests/scoped_fd-selftests.c \ unittests/scoped_fd-selftests.c \
unittests/scoped_ignore_signal-selftests.c \
unittests/scoped_mmap-selftests.c \ unittests/scoped_mmap-selftests.c \
unittests/scoped_restore-selftests.c \ unittests/scoped_restore-selftests.c \
unittests/search-memory-selftests.c \ unittests/search-memory-selftests.c \
@@ -1336,7 +1337,6 @@ HFILES_NO_SRCDIR = \
inf-ptrace.h \ inf-ptrace.h \
infcall.h \ infcall.h \
inferior.h \ inferior.h \
inflow.h \
inline-frame.h \ inline-frame.h \
interps.h \ interps.h \
jit.h \ jit.h \

View File

@@ -43,7 +43,7 @@
#include "gdbsupport/gdb_optional.h" #include "gdbsupport/gdb_optional.h"
#include "gdbsupport/gdb_unlinker.h" #include "gdbsupport/gdb_unlinker.h"
#include "gdbsupport/pathstuff.h" #include "gdbsupport/pathstuff.h"
#include <signal.h> #include "gdbsupport/scoped_ignore_signal.h"
@@ -634,33 +634,6 @@ print_callback (void *ignore, const char *message)
fputs_filtered (message, gdb_stderr); fputs_filtered (message, gdb_stderr);
} }
/* RAII class used to ignore SIGPIPE in a scope. */
class scoped_ignore_sigpipe
{
public:
scoped_ignore_sigpipe ()
{
#ifdef SIGPIPE
m_osigpipe = signal (SIGPIPE, SIG_IGN);
#endif
}
~scoped_ignore_sigpipe ()
{
#ifdef SIGPIPE
signal (SIGPIPE, m_osigpipe);
#endif
}
DISABLE_COPY_AND_ASSIGN (scoped_ignore_sigpipe);
private:
#ifdef SIGPIPE
sighandler_t m_osigpipe = NULL;
#endif
};
/* Process the compilation request. On success it returns the object /* Process the compilation request. On success it returns the object
and source file names. On an error condition, error () is and source file names. On an error condition, error () is
called. */ called. */

View File

@@ -20,7 +20,6 @@
#include "defs.h" #include "defs.h"
#include "command.h" #include "command.h"
#include "inferior.h" #include "inferior.h"
#include "inflow.h"
#include "terminal.h" #include "terminal.h"
#include "gdbcore.h" #include "gdbcore.h"
#include "regcache.h" #include "regcache.h"

View File

@@ -29,12 +29,12 @@
#include <fcntl.h> #include <fcntl.h>
#include "gdbsupport/gdb_select.h" #include "gdbsupport/gdb_select.h"
#include "inflow.h"
#include "gdbcmd.h" #include "gdbcmd.h"
#ifdef HAVE_TERMIOS_H #ifdef HAVE_TERMIOS_H
#include <termios.h> #include <termios.h>
#endif #endif
#include "gdbsupport/job-control.h" #include "gdbsupport/job-control.h"
#include "gdbsupport/scoped_ignore_sigttou.h"
#ifdef HAVE_SYS_IOCTL_H #ifdef HAVE_SYS_IOCTL_H
#include <sys/ioctl.h> #include <sys/ioctl.h>

View File

@@ -40,7 +40,6 @@
#include <signal.h> #include <signal.h>
#include <ctype.h> #include <ctype.h>
#include "gdb_bfd.h" #include "gdb_bfd.h"
#include "inflow.h"
#include "auxv.h" #include "auxv.h"
#include "procfs.h" #include "procfs.h"
#include "observable.h" #include "observable.h"

View File

@@ -32,7 +32,7 @@
#include "gdbcmd.h" #include "gdbcmd.h"
#include "gdbsupport/filestuff.h" #include "gdbsupport/filestuff.h"
#include <termios.h> #include <termios.h>
#include "inflow.h" #include "gdbsupport/scoped_ignore_sigttou.h"
struct hardwire_ttystate struct hardwire_ttystate
{ {

View File

@@ -0,0 +1,125 @@
/* Self tests for scoped_ignored_signal for GDB, the GNU debugger.
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 <http://www.gnu.org/licenses/>. */
#include "defs.h"
#include "gdbsupport/scoped_ignore_signal.h"
#include "gdbsupport/selftest.h"
#include "gdbsupport/scope-exit.h"
#include <unistd.h>
#include <signal.h>
namespace selftests {
namespace scoped_ignore_sig {
#ifdef SIGPIPE
/* True if the SIGPIPE handler ran. */
static sig_atomic_t got_sigpipe = 0;
/* SIGPIPE handler for testing. */
static void
handle_sigpipe (int)
{
got_sigpipe = 1;
}
/* Test scoped_ignore_sigpipe. */
static void
test_sigpipe ()
{
auto *osig = signal (SIGPIPE, handle_sigpipe);
SCOPE_EXIT { signal (SIGPIPE, osig); };
#ifdef HAVE_SIGPROCMASK
/* Make sure SIGPIPE isn't blocked. */
sigset_t set, old_state;
sigemptyset (&set);
sigaddset (&set, SIGPIPE);
sigprocmask (SIG_UNBLOCK, &set, &old_state);
SCOPE_EXIT { sigprocmask (SIG_SETMASK, &old_state, nullptr); };
#endif
/* Create pipe, and close read end so that writes to the pipe fail
with EPIPE. */
int fd[2];
char c = 0xff;
int r;
r = pipe (fd);
SELF_CHECK (r == 0);
close (fd[0]); SCOPE_EXIT { close (fd[1]); };
/* Check that writing to the pipe results in EPIPE. EXPECT_SIG
indicates whether a SIGPIPE signal is expected. */
auto check_pipe_write = [&] (bool expect_sig)
{
got_sigpipe = 0;
errno = 0;
r = write (fd[1], &c, 1);
SELF_CHECK (r == -1 && errno == EPIPE
&& got_sigpipe == expect_sig);
};
/* Check that without a scoped_ignore_sigpipe in scope we indeed get
a SIGPIPE signal. */
check_pipe_write (true);
/* Now check that with a scoped_ignore_sigpipe in scope, SIGPIPE is
ignored/blocked. */
{
scoped_ignore_sigpipe ignore1;
check_pipe_write (false);
/* Check that scoped_ignore_sigpipe nests correctly. */
{
scoped_ignore_sigpipe ignore2;
check_pipe_write (false);
}
/* If nesting works correctly, this write results in no
SIGPIPE. */
check_pipe_write (false);
}
/* No scoped_ignore_sigpipe is is scope anymore, so this should
result in a SIGPIPE signal. */
check_pipe_write (true);
}
#endif /* SIGPIPE */
} /* namespace scoped_ignore_sig */
} /* namespace selftests */
void _initialize_scoped_ignore_signal_selftests ();
void
_initialize_scoped_ignore_signal_selftests ()
{
#ifdef SIGPIPE
selftests::register_test ("scoped_ignore_sigpipe",
selftests::scoped_ignore_sig::test_sigpipe);
#endif
}

View File

@@ -0,0 +1,97 @@
/* Support for ignoring signals.
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 <http://www.gnu.org/licenses/>. */
#ifndef SCOPED_IGNORE_SIGNAL_H
#define SCOPED_IGNORE_SIGNAL_H
#include <signal.h>
/* RAII class used to ignore a signal in a scope. If sigprocmask is
supported, then the signal is only ignored by the calling thread.
Otherwise, the signal disposition is set to SIG_IGN, which affects
the whole process. */
template <int Sig>
class scoped_ignore_signal
{
public:
scoped_ignore_signal ()
{
#ifdef HAVE_SIGPROCMASK
sigset_t set, old_state;
sigemptyset (&set);
sigaddset (&set, Sig);
sigprocmask (SIG_BLOCK, &set, &old_state);
m_was_blocked = sigismember (&old_state, Sig);
#else
m_osig = signal (Sig, SIG_IGN);
#endif
}
~scoped_ignore_signal ()
{
#ifdef HAVE_SIGPROCMASK
if (!m_was_blocked)
{
sigset_t set;
const timespec zero_timeout = {};
sigemptyset (&set);
sigaddset (&set, Sig);
/* If we got a pending Sig signal, consume it before
unblocking. */
sigtimedwait (&set, nullptr, &zero_timeout);
sigprocmask (SIG_UNBLOCK, &set, nullptr);
}
#else
signal (Sig, m_osig);
#endif
}
DISABLE_COPY_AND_ASSIGN (scoped_ignore_signal);
private:
#ifdef HAVE_SIGPROCMASK
bool m_was_blocked;
#else
sighandler_t m_osig;
#endif
};
struct scoped_ignore_signal_nop
{
/* Note, these can't both be "= default", because otherwise the
compiler warns that variables of this type are not used. */
scoped_ignore_signal_nop ()
{}
~scoped_ignore_signal_nop ()
{}
DISABLE_COPY_AND_ASSIGN (scoped_ignore_signal_nop);
};
#ifdef SIGPIPE
using scoped_ignore_sigpipe = scoped_ignore_signal<SIGPIPE>;
#else
using scoped_ignore_sigpipe = scoped_ignore_signal_nop;
#endif
#endif /* SCOPED_IGNORE_SIGNAL_H */

View File

@@ -1,4 +1,4 @@
/* Low level interface to ptrace, for GDB when running under Unix. /* Support for signoring SIGTTOU.
Copyright (C) 2003-2021 Free Software Foundation, Inc. Copyright (C) 2003-2021 Free Software Foundation, Inc.
@@ -17,40 +17,64 @@
You should have received a copy of the GNU General Public License You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */ along with this program. If not, see <http://www.gnu.org/licenses/>. */
#ifndef INFLOW_H #ifndef SCOPED_IGNORE_SIGTTOU_H
#define INFLOW_H #define SCOPED_IGNORE_SIGTTOU_H
#include <unistd.h> #include "gdbsupport/scoped_ignore_signal.h"
#include <signal.h>
#include "gdbsupport/job-control.h" #include "gdbsupport/job-control.h"
/* RAII class used to ignore SIGTTOU in a scope. */ #ifdef SIGTTOU
/* Simple wrapper that allows lazy initialization / destruction of T.
Slightly more efficient than gdb::optional, because it doesn't
carry storage to track whether the object has been initialized. */
template<typename T>
class lazy_init
{
public:
void emplace ()
{
new (m_buf) T ();
}
void reset ()
{
reinterpret_cast <T *> (m_buf)->~T ();
}
private:
alignas (T) gdb_byte m_buf[sizeof (T)];
};
/* RAII class used to ignore SIGTTOU in a scope. This isn't simply
scoped_ignore_signal<SIGTTOU> because we want to check the
`job_control' global. */
class scoped_ignore_sigttou class scoped_ignore_sigttou
{ {
public: public:
scoped_ignore_sigttou () scoped_ignore_sigttou ()
{ {
#ifdef SIGTTOU
if (job_control) if (job_control)
m_osigttou = signal (SIGTTOU, SIG_IGN); m_ignore_signal.emplace ();
#endif
} }
~scoped_ignore_sigttou () ~scoped_ignore_sigttou ()
{ {
#ifdef SIGTTOU
if (job_control) if (job_control)
signal (SIGTTOU, m_osigttou); m_ignore_signal.reset ();
#endif
} }
DISABLE_COPY_AND_ASSIGN (scoped_ignore_sigttou); DISABLE_COPY_AND_ASSIGN (scoped_ignore_sigttou);
private: private:
#ifdef SIGTTOU lazy_init<scoped_ignore_signal<SIGTTOU>> m_ignore_signal;
sighandler_t m_osigttou = NULL;
#endif
}; };
#endif /* inflow.h */ #else
using scoped_ignore_sigttou = scoped_ignore_signal_nop;
#endif
#endif /* SCOPED_IGNORE_SIGTTOU_H */