Fix signal propagation logic for Linux/Android exception handler.

The current code is relying on info->si_pid to figure out whether
the exception handler was triggered by a signal coming from the kernel
(that will re-trigger until the cause that triggered the signal has
been cleared) or from user-space e.g., kill -SIGNAL pid, which will NOT
automatically re-trigger in the next signal handler in the chain.
While the intentions are good (manually re-triggering user-space
signals), the current implementation mistakenly looks at the si_pid
field in siginfo_t, assuming that it is coming from the kernel if
si_pid == 0.
This is wrong. siginfo_t, in fact, is a union and si_pid is meaningful
only for userspace signals. For signals originated by the kernel,
instead, si_pid overlaps with si_addr (the faulting address).
As a matter of facts, the current implementation is mistakenly
re-triggering the signal using tgkill for most of the kernel-space
signals (unless the fault address is exactly 0x0).
This is not completelly correct for the case of SIGSEGV/SIGBUS. The
next handler in the chain will stil see the signal, but the |siginfo|
and the |context| arguments of the handler will be meaningless
(retriggering a signal with tgkill doesn't preserve them).
Therefore, if the next handler in the chain expects those arguments
to be set, it will fail.
Concretelly, this is causing problems to WebView. In some rare
circumstances, the next handler in the chain is a user-space runtime
which does SIGSEGV handling to implement speculative null pointer
managed exceptions (see as an example
http://www.mono-project.com/docs/advanced/runtime/docs/exception-handling/)

The fix herein proposed consists in using the si_code (see SI_FROMUSER
macros) to determine whether a signal is coming form the kernel
(and therefore just re-establish the next signal handler) or from
userspace (and use the tgkill logic).

Repro case:
This issue is visible in Chrome for Android with this simple repro case:
- Add a non-null pointer dereference in the codebase:
  *((volatile int*)0xbeef) = 42
Without this change: the next handler (the libc trap) prints:
  F/libc  (  595): Fatal signal 11 (SIGSEGV), code 1, fault addr 0x487
  where 0x487 is actually the PID of the process (which is wrong).
With this change: the next handler prints:
  F/libc  (  595): Fatal signal 11 (SIGSEGV), code 1, fault addr 0xbeef
  which is the correct answer.

BUG=chromium:481937
R=mark@chromium.org

Review URL: https://breakpad.appspot.com/6844002.

git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1461 4c0a9323-5329-0410-9bdc-e9ce6186880e

1 files changed, 15 insertions, 2 deletions

diff --git a/src/processor/microdump.cc b/src/processor/microdump.cc
index bf628552..3cc618f6 100644
--- a/src/processor/microdump.cc
+++ b/src/processor/microdump.cc
@@ -76,6 +76,19 @@ std::vector<uint8_t> ParseHexBuf(const string& str) {
   return buf;
 }
 
+bool GetLine(std::istringstream* istream, string* str) {
+  if (std::getline(*istream, *str)) {
+    // Trim any trailing newline from the end of the line. Allows us
+    // to seamlessly handle both Windows/DOS and Unix formatted input. The
+    // adb tool generally writes logcat dumps in Windows/DOS format.
+    if (!str->empty() && str->at(str->size() - 1) == '\r') {
+      str->erase(str->size() - 1);
+    }
+    return true;
+  }
+  return false;
+}
+
 }  // namespace
 
 namespace google_breakpad {
@@ -183,7 +196,7 @@ Microdump::Microdump(const string& contents)
   string arch;
 
   std::istringstream stream(contents);
-  while (std::getline(stream, line)) {
+  while (GetLine(&stream, &line)) {
     if (line.find(kGoogleBreakpadKey) == string::npos) {
       continue;
     }
@@ -214,7 +227,7 @@ Microdump::Microdump(const string& contents)
       os_tokens >> arch;
       os_tokens >> num_cpus;
       os_tokens >> hw_arch;
-      std::getline(os_tokens, os_version);
+      GetLine(&os_tokens, &os_version);
       os_version.erase(0, 1);  // remove leading space.
 
       system_info_->cpu = hw_arch;