[2/4] Introduce scoped_restore_signal

Message ID 20210615111429.1879286-3-pedro@palves.net
State New
Headers show
Series
  • Introduce scoped_ignore_signal & make it thread safe
Related show

Commit Message

Pedro Alves June 15, 2021, 11:14 a.m.
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
---
 gdb/compile/compile.c              | 29 +-------------
 gdbsupport/scoped_ignore_signal.h  | 64 ++++++++++++++++++++++++++++++
 gdbsupport/scoped_ignore_sigttou.h | 48 ++++++++++++++++------
 3 files changed, 101 insertions(+), 40 deletions(-)
 create mode 100644 gdbsupport/scoped_ignore_signal.h

-- 
2.26.2

Comments

Pedro Alves June 17, 2021, 2:57 p.m. | #1
On 2021-06-15 12:14 p.m., Pedro Alves wrote:
> +/* 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)];

> +};

> +


I was looking at this again, and realized that a raw buffer like that isn't super
convenient when you're debugging gdb and want to inspect T (you'll need to cast).

So I made it a union instead (see below).  It has the added advantage that no cast
is necessary.

I'm going to push the series with this change.

diff --git c/gdbsupport/scoped_ignore_sigttou.h w/gdbsupport/scoped_ignore_sigttou.h
index 7b29f2b79a3..1fc8f80d7fd 100644
--- c/gdbsupport/scoped_ignore_sigttou.h
+++ w/gdbsupport/scoped_ignore_sigttou.h
@@ -34,16 +34,23 @@ class lazy_init
 public:
   void emplace ()
   {
-    new (m_buf) T ();
+    new (&m_u.obj) T ();
   }
 
   void reset ()
   {
-    reinterpret_cast <T *> (m_buf)->~T ();
+    m_u.obj.~T ();
   }
 
 private:
-  alignas (T) gdb_byte m_buf[sizeof (T)];
+  union u
+  {
+    /* Must define ctor/dtor if T has non-trivial ctor/dtor.  */
+    u () {}
+    ~u () {}
+
+    T obj;
+  } m_u;
 };

Patch

diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index abbb72a393c..e815348ff07 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -43,7 +43,7 @@ 
 #include "gdbsupport/gdb_optional.h"
 #include "gdbsupport/gdb_unlinker.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);
 }
 
-/* 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
    and source file names.  On an error condition, error () is
    called.  */
diff --git a/gdbsupport/scoped_ignore_signal.h b/gdbsupport/scoped_ignore_signal.h
new file mode 100644
index 00000000000..cccd390529b
--- /dev/null
+++ b/gdbsupport/scoped_ignore_signal.h
@@ -0,0 +1,64 @@ 
+/* 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.  */
+
+template <int Sig>
+class scoped_ignore_signal
+{
+public:
+  scoped_ignore_signal ()
+  {
+    m_osig = signal (Sig, SIG_IGN);
+  }
+
+  ~scoped_ignore_signal ()
+  {
+    signal (Sig, m_osig);
+  }
+
+  DISABLE_COPY_AND_ASSIGN (scoped_ignore_signal);
+
+private:
+  sighandler_t m_osig = nullptr;
+};
+
+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 */
diff --git a/gdbsupport/scoped_ignore_sigttou.h b/gdbsupport/scoped_ignore_sigttou.h
index a31316460b4..7b29f2b79a3 100644
--- a/gdbsupport/scoped_ignore_sigttou.h
+++ b/gdbsupport/scoped_ignore_sigttou.h
@@ -20,37 +20,61 @@ 
 #ifndef SCOPED_IGNORE_SIGTTOU_H
 #define SCOPED_IGNORE_SIGTTOU_H
 
-#include <unistd.h>
-#include <signal.h>
+#include "gdbsupport/scoped_ignore_signal.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
 {
 public:
   scoped_ignore_sigttou ()
   {
-#ifdef SIGTTOU
     if (job_control)
-      m_osigttou = signal (SIGTTOU, SIG_IGN);
-#endif
+      m_ignore_signal.emplace ();
   }
 
   ~scoped_ignore_sigttou ()
   {
-#ifdef SIGTTOU
     if (job_control)
-      signal (SIGTTOU, m_osigttou);
-#endif
+      m_ignore_signal.reset ();
   }
 
   DISABLE_COPY_AND_ASSIGN (scoped_ignore_sigttou);
 
 private:
-#ifdef SIGTTOU
-  sighandler_t m_osigttou = NULL;
-#endif
+  lazy_init<scoped_ignore_signal<SIGTTOU>> m_ignore_signal;
 };
 
+#else
+
+using scoped_ignore_sigttou = scoped_ignore_signal_nop;
+
+#endif
+
 #endif /* SCOPED_IGNORE_SIGTTOU_H */