StephenChong / google-breakpad

Automatically exported from code.google.com/p/google-breakpad
0 stars 0 forks source link

Handling old style signal() calls in Breakpad #661

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
There is a problem in how Breakpad is dealing with old style signal calls. Here 
is an excerpt from 
https://code.google.com/p/google-breakpad/source/browse/trunk/src/client/linux/h
andler/exception_handler.cc#347

struct sigaction cur_handler;
  if (sigaction(sig, NULL, &cur_handler) == 0 &&
      (cur_handler.sa_flags & SA_SIGINFO) == 0) {
    // Reset signal handler with the right flags.
    sigemptyset(&cur_handler.sa_mask);
    sigaddset(&cur_handler.sa_mask, sig);

    cur_handler.sa_sigaction = SignalHandler;
    cur_handler.sa_flags = SA_ONSTACK | SA_SIGINFO;

    if (sigaction(sig, &cur_handler, NULL) == -1) {
      // When resetting the handler fails, try to reset the
      // default one to avoid an infinite loop here.
      InstallDefaultHandler(sig);
    }
    pthread_mutex_unlock(&g_handler_stack_mutex_);
    return;
  }

Is *returning* from signal handler always the right thing to do here? Not all 
signals will be re-thrown (e.g., SIGABRT, SIGFPE).

Original issue reported on code.google.com by sy...@google.com on 15 Jul 2015 at 6:10

GoogleCodeExporter commented 9 years ago
The problem is that returning is safer than re-throwing the signal. Either you 
know that you should re-throw the signal because it came via kill or similar, 
or you can end up triggering some signal handler loops.

The authoritative information lies in SI_FROMUSER (info->si_code <= 0). But the 
problem is that if you are in a handler registered via old-style signal, you 
don't have |info|, so you don't have the piece of information that tells you 
whether you should or should not re-trigger manually the signal after 
reinstalling. So, in practice, there isn't really anything too much you can do 
here. And speculating on the nature of the signal (e.g., assuming that SIGABRT 
comes always from the kernel) is unreliable.

Original comment by primi...@chromium.org on 16 Jul 2015 at 11:30

GoogleCodeExporter commented 9 years ago
>> you can end up triggering some signal handler loops.
I don't get it, the if block can be executed at most once. Is there any 
scenario you have in mind where there can be a loop?

Original comment by sy...@google.com on 20 Jul 2015 at 6:17