Bug 1036286 - Delay registration of the faulty.lib signal handler until when it's necessary. r=nfroyd, a=sledru
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 07 Aug 2014 02:51:03 +0900
changeset 208271 ec230387fad2
parent 208270 91f078c385f8
child 208272 1f96d584763a
push id3796
push userryanvm@gmail.com
push date2014-08-08 18:52 +0000
treeherdermozilla-beta@1cf7b5810eb5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnfroyd, sledru
bugs1036286
milestone32.0
Bug 1036286 - Delay registration of the faulty.lib signal handler until when it's necessary. r=nfroyd, a=sledru It's necessary to delay it because for the second part, we need to call dlopen, and until recently bionic's linker dead-locked when using dlopen from a static initializer.
mozglue/linker/ElfLoader.cpp
mozglue/linker/ElfLoader.h
--- a/mozglue/linker/ElfLoader.cpp
+++ b/mozglue/linker/ElfLoader.cpp
@@ -960,28 +960,28 @@ static uint64_t ProcessTimeStamp_Now()
 /* Data structure used to pass data to the temporary signal handler,
  * as well as triggering a test crash. */
 struct TmpData {
   volatile int crash_int;
   volatile uint64_t crash_timestamp;
 };
 
 SEGVHandler::SEGVHandler()
-: registeredHandler(false), signalHandlingBroken(false)
-, signalHandlingSlow(false)
+: initialized(false), registeredHandler(false), signalHandlingBroken(true)
+, signalHandlingSlow(true)
 {
   /* Initialize oldStack.ss_flags to an invalid value when used to set
    * an alternative stack, meaning we haven't got information about the
-   * original alternative stack and thus don't mean to restore it */
+   * original alternative stack and thus don't mean to restore it in
+   * the destructor. */
   oldStack.ss_flags = SS_ONSTACK;
-  if (!Divert(sigaction, __wrap_sigaction))
-    return;
 
   /* Get the current segfault signal handler. */
-  sys_sigaction(SIGSEGV, nullptr, &this->action);
+  struct sigaction old_action;
+  sys_sigaction(SIGSEGV, nullptr, &old_action);
 
   /* Some devices don't provide useful information to their SIGSEGV handlers,
    * making it impossible for on-demand decompression to work. To check if
    * we're on such a device, setup a temporary handler and deliberately
    * trigger a segfault. The handler will set signalHandlingBroken if the
    * provided information is bogus.
    * Some other devices have a kernel option enabled that makes SIGSEGV handler
    * have an overhead so high that it affects how on-demand decompression
@@ -1000,21 +1000,33 @@ SEGVHandler::SEGVHandler()
   if (sys_sigaction(SIGSEGV, &action, nullptr))
     return;
 
   TmpData *data = reinterpret_cast<TmpData*>(stackPtr.get());
   data->crash_timestamp = ProcessTimeStamp_Now();
   mprotect(stackPtr, stackPtr.GetLength(), PROT_NONE);
   data->crash_int = 123;
   /* Restore the original segfault signal handler. */
-  sys_sigaction(SIGSEGV, &this->action, nullptr);
+  sys_sigaction(SIGSEGV, &old_action, nullptr);
   stackPtr.Assign(MAP_FAILED, 0);
+}
+
+void
+SEGVHandler::FinishInitialization()
+{
+  /* Ideally, we'd need some locking here, but in practice, we're not
+   * going to race with another thread. */
+  initialized = true;
+
   if (signalHandlingBroken || signalHandlingSlow)
     return;
 
+  if (!Divert(sigaction, __wrap_sigaction))
+    return;
+
   /* Setup an alternative stack if the already existing one is not big
    * enough, or if there is none. */
   if (sigaltstack(nullptr, &oldStack) == 0) {
     if (oldStack.ss_flags == SS_ONSTACK)
       oldStack.ss_flags = 0;
     if (!oldStack.ss_sp || oldStack.ss_size < stackSize) {
       stackPtr.Assign(MemoryRange::mmap(nullptr, stackSize,
                                         PROT_READ | PROT_WRITE,
@@ -1028,17 +1040,17 @@ SEGVHandler::SEGVHandler()
       if (sigaltstack(&stack, nullptr) != 0)
         return;
     }
   }
   /* Register our own handler, and store the already registered one in
    * SEGVHandler's struct sigaction member */
   action.sa_sigaction = &SEGVHandler::handler;
   action.sa_flags = SA_SIGINFO | SA_NODEFER | SA_ONSTACK;
-  registeredHandler = !sys_sigaction(SIGSEGV, &action, nullptr);
+  registeredHandler = !sys_sigaction(SIGSEGV, &action, &this->action);
 }
 
 SEGVHandler::~SEGVHandler()
 {
   /* Restore alternative stack for signals */
   if (oldStack.ss_flags != SS_ONSTACK)
     sigaltstack(&oldStack, nullptr);
   /* Restore original signal handler */
@@ -1048,28 +1060,28 @@ SEGVHandler::~SEGVHandler()
 
 /* Test handler for a deliberately triggered SIGSEGV that determines whether
  * useful information is provided to signal handlers, particularly whether
  * si_addr is filled in properly, and whether the segfault handler is called
  * quickly enough. */
 void SEGVHandler::test_handler(int signum, siginfo_t *info, void *context)
 {
   SEGVHandler &that = ElfLoader::Singleton;
-  if (signum != SIGSEGV ||
-      info == nullptr || info->si_addr != that.stackPtr.get())
-    that.signalHandlingBroken = true;
+  if (signum == SIGSEGV && info &&
+      info->si_addr == that.stackPtr.get())
+    that.signalHandlingBroken = false;
   mprotect(that.stackPtr, that.stackPtr.GetLength(), PROT_READ | PROT_WRITE);
   TmpData *data = reinterpret_cast<TmpData*>(that.stackPtr.get());
   uint64_t latency = ProcessTimeStamp_Now() - data->crash_timestamp;
   DEBUG_LOG("SEGVHandler latency: %" PRIu64, latency);
   /* See bug 886736 for timings on different devices, 150 ┬Ás is reasonably above
-   * the latency on "working" devices and seems to be reasonably fast to incur
+   * the latency on "working" devices and seems to be short enough to not incur
    * a huge overhead to on-demand decompression. */
-  if (latency > 150000)
-    that.signalHandlingSlow = true;
+  if (latency <= 150000)
+    that.signalHandlingSlow = false;
 }
 
 /* TODO: "properly" handle signal masks and flags */
 void SEGVHandler::handler(int signum, siginfo_t *info, void *context)
 {
   //ASSERT(signum == SIGSEGV);
   DEBUG_LOG("Caught segmentation fault @%p", info->si_addr);
 
--- a/mozglue/linker/ElfLoader.h
+++ b/mozglue/linker/ElfLoader.h
@@ -307,32 +307,40 @@ private:
  * faults within the address space of the loaded libraries. It however
  * allows a handler to be set for faults in other places, and redispatches
  * to the handler set through signal() or sigaction().
  */
 class SEGVHandler
 {
 public:
   bool hasRegisteredHandler() {
+    if (! initialized)
+      FinishInitialization();
     return registeredHandler;
   }
 
   bool isSignalHandlingBroken() {
     return signalHandlingBroken;
   }
 
 protected:
   SEGVHandler();
   ~SEGVHandler();
 
 private:
   static int __wrap_sigaction(int signum, const struct sigaction *act,
                               struct sigaction *oldact);
 
   /**
+   * The constructor doesn't do all initialization, and the tail is done
+   * at a later time.
+   */
+  void FinishInitialization();
+
+  /**
    * SIGSEGV handler registered with __wrap_signal or __wrap_sigaction.
    */
   struct sigaction action;
   
   /**
    * ElfLoader SIGSEGV handler.
    */
   static void handler(int signum, siginfo_t *info, void *context);
@@ -354,16 +362,17 @@ private:
   stack_t oldStack;
 
   /**
    * Pointer to an alternative stack for signals. Only set if oldStack is
    * not set or not big enough.
    */
   MappedPtr stackPtr;
 
+  bool initialized;
   bool registeredHandler;
   bool signalHandlingBroken;
   bool signalHandlingSlow;
 };
 
 /**
  * Elf Loader class in charge of loading and bookkeeping libraries.
  */