Revised deallocation of alternate signal stack

* Make sure the alternate signal stack for domain 0 is turned off and freed
  when the main OCaml code stops.
  (Adaptation of ocaml#10726)

* Be more robust against C libraries that install their own alternate signal
  stack.  Don't try to free their stack, only the stack block we allocated.
  (Adaptation of ocaml#11496)

runtime/caml/signals.h

@@ -74,8 +74,10 @@ value caml_process_pending_actions_with_root (value extra_root); // raises
 value caml_process_pending_actions_with_root_exn (value extra_root);
 
 void caml_init_signal_handling(void);
-int caml_init_signal_stack(void);
-void caml_free_signal_stack(void);
+void caml_init_signals();
+void caml_terminate_signals();
+void * caml_init_signal_stack(void);
+void caml_free_signal_stack(void *);
 
 /* These hooks are not modified after other threads are spawned. */
 CAMLextern void (*caml_enter_blocking_section_hook)(void);

runtime/domain.c

@@ -607,10 +607,6 @@ static void create_domain(uintnat initial_minor_heap_wsize) {
   domain_state->dependent_size = 0;
   domain_state->dependent_allocated = 0;
 
-  if (caml_init_signal_stack() < 0) {
-    goto init_signal_stack_failure;
-  }
-
   /* the minor heap will be initialized by
      [caml_reallocate_minor_heap] below. */
   domain_state->young_start =
@@ -707,8 +703,6 @@ static void create_domain(uintnat initial_minor_heap_wsize) {
   caml_free_minor_tables(domain_state->minor_tables);
   domain_state->minor_tables = NULL;
 alloc_minor_tables_failure:
-  caml_free_signal_stack();
-init_signal_stack_failure:
   domain_self = NULL;
 
 
@@ -1064,6 +1058,13 @@ static void* domain_thread_func(void* v)
 #ifndef _WIN32
   sigset_t mask = *(p->mask);
 #endif
+  void * signal_stack;
+
+  signal_stack = caml_init_signal_stack();
+  if (signal_stack == NULL) {
+    caml_gc_log("Failed to create domain: signal stack");
+    return 0;
+  }
 
   create_domain(caml_params->init_minor_heap_wsz);
   /* this domain is now part of the STW participant set */
@@ -1110,6 +1111,7 @@ static void* domain_thread_func(void* v)
   } else {
     caml_gc_log("Failed to create domain");
   }
+  caml_free_signal_stack(signal_stack);
   return 0;
 }
 
@@ -1731,7 +1733,6 @@ static void domain_terminate (void)
   domain_state->shared_heap = 0;
   caml_free_minor_tables(domain_state->minor_tables);
   domain_state->minor_tables = 0;
-  caml_free_signal_stack();
 
   caml_orphan_alloc_stats(domain_state);
   /* Heap stats were orphaned by caml_teardown_shared_heap above.

runtime/fail_byt.c

@@ -41,7 +41,10 @@ CAMLexport void caml_raise(value v)
   if (Is_exception_result(v))
     v = Extract_exception(v);
 
-  if (Caml_state->external_raise == NULL) caml_fatal_uncaught_exception(v);
+  if (Caml_state->external_raise == NULL) {
+    caml_terminate_signals();
+    caml_fatal_uncaught_exception(v);
+  }
   *Caml_state->external_raise->exn_bucket = v;
 
   Caml_state->local_roots = Caml_state->external_raise->local_roots;

runtime/fail_nat.c

@@ -73,7 +73,10 @@ void caml_raise(value v)
 
   exception_pointer = (char*)Caml_state->c_stack;
 
-  if (exception_pointer == NULL) caml_fatal_uncaught_exception(v);
+  if (exception_pointer == NULL) {
+    caml_terminate_signals();
+    caml_fatal_uncaught_exception(v);
+  }
 
   while (Caml_state->local_roots != NULL &&
          (char *) Caml_state->local_roots < exception_pointer) {

runtime/signals.c

@@ -463,7 +463,7 @@ CAMLexport int caml_rev_convert_signal_number(int signo)
   return signo;
 }
 
-int caml_init_signal_stack(void)
+void * caml_init_signal_stack(void)
 {
 #ifdef POSIX_SIGNALS
   stack_t stk;
@@ -476,11 +476,11 @@ int caml_init_signal_stack(void)
      nasty piece of undefined behaviour forced on the caller. */
   stk.ss_sp = malloc(stk.ss_size);
   if(stk.ss_sp == NULL) {
-    return -1;
+    return NULL;
   }
   if (sigaltstack(&stk, NULL) < 0) {
     free(stk.ss_sp);
-    return -1;
+    return NULL;
   }
 
   /* gprof installs a signal handler for SIGPROF.
@@ -497,23 +497,53 @@ int caml_init_signal_stack(void)
       }
     }
   }
+  return stk.ss_sp;
+#else
+  return NULL;
 #endif
-  return 0;
 }
 
-void caml_free_signal_stack(void)
+void caml_free_signal_stack(void * signal_stack)
 {
 #ifdef POSIX_SIGNALS
-  stack_t stk, disable = {0};
+  stack_t stk, disable;
   disable.ss_flags = SS_DISABLE;
-  /* POSIX says ss_size is ignored when SS_DISABLE is set,
-     but OSX/Darwin fails if the size isn't set. */
-  disable.ss_size = SIGSTKSZ;
+  disable.ss_sp = NULL;  /* not required but avoids a valgrind false alarm */
+  disable.ss_size = SIGSTKSZ; /* macOS wants a valid size here */
   if (sigaltstack(&disable, &stk) < 0) {
     caml_fatal_error("Failed to reset signal stack (err %d)", errno);
   }
+  /* Check whether someone else installed their own signal stack */
+  if (!(stk.ss_flags & SS_DISABLE) && stk.ss_sp != signal_stack) {
+    /* Re-activate their signal stack. */
+    sigaltstack(&stk, NULL);
+  }
   /* Memory was allocated with malloc directly; see caml_init_signal_stack */
-  free(stk.ss_sp);
+  free(signal_stack);
+#endif
+}
+
+/* This is the alternate signal stack block for domain 0 */
+static void * caml_signal_stack_0 = NULL;
+
+void caml_init_signals(void)
+{
+  /* Bound-check trap handling for Power and S390x will go here eventually. */
+
+  /* Set up alternate signal stack for domain 0 */
+#ifdef POSIX_SIGNALS
+  caml_signal_stack_0 = caml_init_signal_stack();
+  if (caml_signal_stack_0 == NULL) {
+    caml_fatal_error("Failed to allocate signal stack for domain 0");
+  }
+#endif
+}
+
+void caml_terminate_signals(void)
+{
+#ifdef POSIX_SIGNALS
+  caml_free_signal_stack(caml_signal_stack_0);
+  caml_signal_stack_0 = NULL;
 #endif
 }
 

runtime/startup_aux.c

@@ -170,7 +170,7 @@ CAMLexport void caml_shutdown(void)
   caml_free_shared_libs();
 #endif
   caml_stat_destroy_pool();
-  caml_free_signal_stack();
+  caml_terminate_signals();
 #if defined(_WIN32) && defined(NATIVE_CODE)
   caml_win32_unregister_overflow_detection();
 #endif

runtime/startup_byt.c

@@ -540,6 +540,8 @@ CAMLexport void caml_main(char_os **argv)
   CAML_RUNTIME_EVENTS_INIT();
 
   Caml_state->external_raise = NULL;
+  /* Setup signal handling */
+  caml_init_signals();
   /* Initialize the interpreter */
   caml_interprete(NULL, 0);
   /* Initialize the debugger, if needed */
@@ -587,6 +589,7 @@ CAMLexport void caml_main(char_os **argv)
     }
     caml_fatal_uncaught_exception(exn);
   }
+  caml_terminate_signals();
 }
 
 /* Main entry point when code is linked in as initialized data */
@@ -599,6 +602,7 @@ CAMLexport value caml_startup_code_exn(
            char_os **argv)
 {
   char_os * exe_name;
+  value res;
 
   /* Initialize the domain */
   CAML_INIT_DOMAIN_STATE;
@@ -638,6 +642,8 @@ CAMLexport value caml_startup_code_exn(
   Caml_state->external_raise = NULL;
   /* Initialize the interpreter */
   caml_interprete(NULL, 0);
+  /* Setup signal handling */
+  caml_init_signals();
   /* Initialize the debugger, if needed */
   caml_debugger_init();
   /* Load the code */
@@ -662,7 +668,9 @@ CAMLexport value caml_startup_code_exn(
   caml_load_main_debug_info();
   /* Execute the program */
   caml_debugger(PROGRAM_START, Val_unit);
-  return caml_interprete(caml_start_code, caml_code_size);
+  res = caml_interprete(caml_start_code, caml_code_size);
+  caml_terminate_signals();
+  return res;
 }
 
 CAMLexport void caml_startup_code(

runtime/startup_nat.c

@@ -88,6 +88,7 @@ extern void caml_install_invalid_parameter_handler(void);
 value caml_startup_common(char_os **argv, int pooling)
 {
   char_os * exe_name, * proc_self_exe;
+  value res;
 
   /* Initialize the domain */
   CAML_INIT_DOMAIN_STATE;
@@ -117,6 +118,7 @@ value caml_startup_common(char_os **argv, int pooling)
   CAML_RUNTIME_EVENTS_INIT();
 
   init_segments();
+  caml_init_signals();
 #ifdef _WIN32
   caml_win32_overflow_detection();
 #endif
@@ -130,7 +132,9 @@ value caml_startup_common(char_os **argv, int pooling)
     exe_name = caml_search_exe_in_path(exe_name);
   caml_sys_init(exe_name, argv);
   caml_maybe_expand_stack();
-  return caml_start_program(Caml_state);
+  res = caml_start_program(Caml_state);
+  caml_terminate_signals();
+  return res;
 }
 
 value caml_startup_exn(char_os **argv)

runtime/sys.c

@@ -196,6 +196,7 @@ CAML_RUNTIME_EVENTS_DESTROY();
 #ifdef _WIN32
   caml_restore_win32_terminal();
 #endif
+  caml_terminate_signals();
   exit(retcode);
 }
 

testsuite/tests/unwind/unwind_test.reference

@@ -4,6 +4,7 @@ Mylib.baz
 Driver.entry
 caml_program
 caml_start_program
+caml_startup_common
 caml_main
 main
 ml_perform_stack_walk
@@ -12,5 +13,6 @@ Mylib.bob
 Driver.entry
 caml_program
 caml_start_program
+caml_startup_common
 caml_main
 main