Re: [PATCH: toplev.c] PR jit/64721: don't install signal-handlers when running within libgccjit

Re: [PATCH: toplev.c] PR jit/64721: don't install signal-handlers when running within libgccjit

[Jakub Jelinek]

On Wed, Jan 21, 2015 at 05:44:03PM -0500, David Malcolm wrote:
> Don't install signal handlers in toplev.c if we're called from libgccjit,
> only install them if we're called from main.c, thus avoiding
> touching process-wide state from a shared library (see the PR for
> details of how this is currently leading to SIGSEGV on errors in
> "make check-jit").
> 
> Is this one sufficiently obvious to still be applicable in stage 4?
> (assuming bootstrap?)
> 
> gcc/ChangeLog:
> 	PR jit/64721
> 	* main.c (main): Construct toplev instances with init_signals=true.
> 	* toplev.c (general_init): Add param "init_signals", and use it to
> 	conditionalize the calls to signal and host_hooks.extra_signals.
> 	(toplev::toplev): Add param "init_signals".
> 	(toplev::main): When invoking general_init, pass m_init_signals
> 	to control whether signal-handlers are installed.
> 	* toplev.h (toplev::toplev): Add param "init_signals".
> 	(toplev::m_init_signals): New field.
> 
> gcc/jit/ChangeLog:
> 	PR jit/64721
> 	* jit-playback.c (gcc::jit::playback::context::compile): Construct
> 	toplev instances with init_signals=false.

Ok for stage4.

	Jakub

[PATCH: toplev.c] PR jit/64721: don't install signal-handlers when running within libgccjit

[David Malcolm]

Don't install signal handlers in toplev.c if we're called from libgccjit,
only install them if we're called from main.c, thus avoiding
touching process-wide state from a shared library (see the PR for
details of how this is currently leading to SIGSEGV on errors in
"make check-jit").

Is this one sufficiently obvious to still be applicable in stage 4?
(assuming bootstrap?)

gcc/ChangeLog:
	PR jit/64721
	* main.c (main): Construct toplev instances with init_signals=true.
	* toplev.c (general_init): Add param "init_signals", and use it to
	conditionalize the calls to signal and host_hooks.extra_signals.
	(toplev::toplev): Add param "init_signals".
	(toplev::main): When invoking general_init, pass m_init_signals
	to control whether signal-handlers are installed.
	* toplev.h (toplev::toplev): Add param "init_signals".
	(toplev::m_init_signals): New field.

gcc/jit/ChangeLog:
	PR jit/64721
	* jit-playback.c (gcc::jit::playback::context::compile): Construct
	toplev instances with init_signals=false.
---
 gcc/jit/jit-playback.c |  3 ++-
 gcc/main.c             |  3 ++-
 gcc/toplev.c           | 32 ++++++++++++++++++--------------
 gcc/toplev.h           |  4 +++-
 4 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
index b4f2073..d2549a0 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -1723,7 +1723,8 @@ compile ()
   acquire_mutex ();
 
   /* This runs the compiler.  */
-  toplev toplev (false);
+  toplev toplev (false, /* use_TV_TOTAL */
+		 false); /* init_signals */
   enter_scope ("toplev::main");
   if (get_logger ())
     for (unsigned i = 0; i < fake_args.length (); i++)
diff --git a/gcc/main.c b/gcc/main.c
index 7a02983..79baf0d 100644
--- a/gcc/main.c
+++ b/gcc/main.c
@@ -33,7 +33,8 @@ int main (int argc, char **argv);
 int
 main (int argc, char **argv)
 {
-  toplev toplev (true);
+  toplev toplev (true, /* use_TV_TOTAL */
+		 true /* init_signals */);
 
   return toplev.main (argc, argv);
 }
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 7bf177f..09f41ba 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -137,7 +137,7 @@ along with GCC; see the file COPYING3.  If not see
 
 #include <new>
 
-static void general_init (const char *);
+static void general_init (const char *, bool);
 static void do_compile ();
 static void process_options (void);
 static void backend_init (void);
@@ -1149,7 +1149,7 @@ open_auxiliary_file (const char *ext)
    options are parsed.  Signal handlers, internationalization etc.
    ARGV0 is main's argv[0].  */
 static void
-general_init (const char *argv0)
+general_init (const char *argv0, bool init_signals)
 {
   const char *p;
 
@@ -1188,28 +1188,31 @@ general_init (const char *argv0)
   global_dc->option_state = &global_options;
   global_dc->option_name = option_name;
 
-  /* Trap fatal signals, e.g. SIGSEGV, and convert them to ICE messages.  */
+  if (init_signals)
+    {
+      /* Trap fatal signals, e.g. SIGSEGV, and convert them to ICE messages.  */
 #ifdef SIGSEGV
-  signal (SIGSEGV, crash_signal);
+      signal (SIGSEGV, crash_signal);
 #endif
 #ifdef SIGILL
-  signal (SIGILL, crash_signal);
+      signal (SIGILL, crash_signal);
 #endif
 #ifdef SIGBUS
-  signal (SIGBUS, crash_signal);
+      signal (SIGBUS, crash_signal);
 #endif
 #ifdef SIGABRT
-  signal (SIGABRT, crash_signal);
+      signal (SIGABRT, crash_signal);
 #endif
 #if defined SIGIOT && (!defined SIGABRT || SIGABRT != SIGIOT)
-  signal (SIGIOT, crash_signal);
+      signal (SIGIOT, crash_signal);
 #endif
 #ifdef SIGFPE
-  signal (SIGFPE, crash_signal);
+      signal (SIGFPE, crash_signal);
 #endif
 
-  /* Other host-specific signal setup.  */
-  (*host_hooks.extra_signals)();
+      /* Other host-specific signal setup.  */
+      (*host_hooks.extra_signals)();
+  }
 
   /* Initialize the garbage-collector, string pools and tree type hash
      table.  */
@@ -2057,8 +2060,9 @@ do_compile ()
     }
 }
 
-toplev::toplev (bool use_TV_TOTAL)
-  : m_use_TV_TOTAL (use_TV_TOTAL)
+toplev::toplev (bool use_TV_TOTAL, bool init_signals)
+  : m_use_TV_TOTAL (use_TV_TOTAL),
+    m_init_signals (init_signals)
 {
   if (!m_use_TV_TOTAL)
     start_timevars ();
@@ -2095,7 +2099,7 @@ toplev::main (int argc, char **argv)
   expandargv (&argc, &argv);
 
   /* Initialization of GCC's environment, and diagnostics.  */
-  general_init (argv[0]);
+  general_init (argv[0], m_init_signals);
 
   /* One-off initialization of options that does not need to be
      repeated when options are added for particular functions.  */
diff --git a/gcc/toplev.h b/gcc/toplev.h
index 5503795..6b423be 100644
--- a/gcc/toplev.h
+++ b/gcc/toplev.h
@@ -28,7 +28,8 @@ extern unsigned int save_decoded_options_count;
 class toplev
 {
 public:
-  toplev (bool use_TV_TOTAL);
+  toplev (bool use_TV_TOTAL,
+	  bool init_signals);
   ~toplev ();
 
   int main (int argc, char **argv);
@@ -40,6 +41,7 @@ private:
   void start_timevars ();
 
   bool m_use_TV_TOTAL;
+  bool m_init_signals;
 };
 
 extern void rest_of_decl_compilation (tree, int, int);
-- 
1.8.5.3