Report inferior SIGSEGV/SIGILL/SIGBUS/SIGFPE as a signal instead of an exception on freebsd

[karnajitw]

Summary

This is the freebsd equivalent of rL238549.

The fix serves 2 purpose

1. LLDB should handle inferior process signals SIGSEGV/SIGILL/SIGBUS/SIGFPE the way it is suppose to be handled. Prior to this fix these signals will neither create a coredump, nor exit from the debugger or work for signal handling scenario.
2. eInvalidCrashReason need not report "unknown crash reason" if we have a valid si_signo

[karnajitw]

Adding the associated bugzilla link for this issue
https://bugs.llvm.org/show_bug.cgi?id=23699

[Before Fix]

[/home/karnajitw/llvm_50_public/build.symbol] $ bin/lldb ~/lldb_test_execs/sigsegv
(lldb) target create "/home/karnajitw/lldb_test_execs/sigsegv"
Current executable set to '/home/karnajitw/lldb_test_execs/sigsegv' (x86_64).
(lldb) run
Process 19312 launching
Process 19312 launched: '/home/karnajitw/lldb_test_execs/sigsegv' (x86_64)
Trying to write on a read only memory...
Process 19312 stopped
* thread #1, name = 'sigsegv', stop reason = signal SIGSEGV: address access protected (fault address: 0x4007e6)
    frame #0: 0x000000000040077c sigsegv`main at sigsegv.c:9
   6
   7      printf("Trying to write on a read only memory...\n");
   8
-> 9      *s = 'h';
   10
   11     printf("Exited normally...\n");
   12
(lldb) c
Process 19312 resuming
Process 19312 stopped
* thread #1, name = 'sigsegv', stop reason = signal SIGSEGV: address access protected (fault address: 0x4007e6)
    frame #0: 0x000000000040077c sigsegv`main at sigsegv.c:9
   6
   7      printf("Trying to write on a read only memory...\n");
   8
-> 9      *s = 'h';
   10
   11     printf("Exited normally...\n");
   12

[/home/karnajitw/llvm_50_public/build.symbol] $ bin/lldb ~/lldb_test_execs/sigill
(lldb) target create "/home/karnajitw/lldb_test_execs/sigill"
Current executable set to '/home/karnajitw/lldb_test_execs/sigill' (x86_64).
(lldb) run
Process 19141 launching
Process 19141 launched: '/home/karnajitw/lldb_test_execs/sigill' (x86_64)
Trying to trigger SIGILL
Process 19141 stopped
* thread #1, name = 'sigill', stop reason = unknown crash reason
    frame #0: 0x00000008008e45ca libc.so.7`__sys_thr_kill + 10
libc.so.7`__sys_thr_kill:
->  0x8008e45ca <+10>: jb     0x800975474
    0x8008e45d0 <+16>: retq
    0x8008e45d1:       nop
    0x8008e45d2:       nop

[After Fix]

[/home/karnajitw/llvm_50_public/build.symbol] $ bin/lldb ~/lldb_test_execs/sigsegv
(lldb) target create "/home/karnajitw/lldb_test_execs/sigsegv"
Current executable set to '/home/karnajitw/lldb_test_execs/sigsegv' (x86_64).
(lldb) run
Process 20220 launching
Process 20220 launched: '/home/karnajitw/lldb_test_execs/sigsegv' (x86_64)
Trying to write on a read only memory...
Process 20220 stopped
* thread #1, name = 'sigsegv', stop reason = signal SIGSEGV: address access protected (fault address: 0x4007e6)
    frame #0: 0x000000000040077c sigsegv`main at sigsegv.c:9
   6
   7      printf("Trying to write on a read only memory...\n");
   8
-> 9      *s = 'h';
   10
   11     printf("Exited normally...\n");
   12
(lldb) c
Process 20220 resuming
Process 20220 exited with status = -1 (0xffffffff)

[/home/karnajitw/llvm_50_public/build.symbol] $ bin/lldb ~/lldb_test_execs/sigill
(lldb) target create "/home/karnajitw/lldb_test_execs/sigill"
Current executable set to '/home/karnajitw/lldb_test_execs/sigill' (x86_64).
(lldb) run
Process 18529 launching
Process 18529 launched: '/home/karnajitw/lldb_test_execs/sigill' (x86_64)
Trying to trigger SIGILL
Process 18529 stopped
* thread #1, name = 'sigill', stop reason = signal SIGILL
    frame #0: 0x00000008008e45ca libc.so.7`__sys_thr_kill + 10
libc.so.7`__sys_thr_kill:
->  0x8008e45ca <+10>: jb     0x800975474
    0x8008e45d0 <+16>: retq
    0x8008e45d1:       nop
    0x8008e45d2:       nop

[labath]

Adding ed as freebsd owner.

The patch seems reasonable, we changed the behavior on linux in a similar way couple of releases back -- SEGV is not a "crash", because the application can generally handle it via a signal handler.

[karnajitw]

Updated the patch. While referring to the linux implementation, looks like I have added a reinterpret_cast on message.GetFaultAddress which is not required in the freebsd scenario.

[emaste]

Sorry I missed this when first uploaded, I will look at it shortly.

For future uploads can you kindly include full context, as described in https://llvm.org/docs/Phabricator.html?

[emaste]

The patch LGTM, I will test it soon.

[emaste]

With this patch I observed three new failures on FreeBSD and three new unexpected passes on FreeBSD. An example of a new failure:

======================================================================
FAIL: test_inferior_crashing_expr_step_and_expr_dwarf (TestInferiorCrashing.CrashingInferiorTestCase)
   Test that lldb expressions work before and after stepping after a crash.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1732, in dwarf_test_method
    return attrvalue(self)
  File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/decorators.py", line 110, in wrapper
    func(*args, **kwargs)
  File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/inferior-crashing/TestInferiorCrashing.py", line 82, in test_inferior_crashing_expr_step_and_expr
    self.inferior_crashing_expr_step_expr()
  File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/inferior-crashing/TestInferiorCrashing.py", line 249, in inferior_crashing_expr_step_expr
    self.check_stop_reason()
  File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/inferior-crashing/TestInferiorCrashing.py", line 93, in check_stop_reason
    STOPPED_DUE_TO_EXC_BAD_ACCESS)
AssertionError: 0 != 1 : Process should be stopped due to bad access exception
Config=x86_64-/usr/bin/cc
----------------------------------------------------------------------
Ran 7 tests in 4.320s

From lldbsuite/test/lldbutil.py:

def is_thread_crashed(test, thread):
    """In the test suite we dereference a null pointer to simulate a crash. The way this is
    reported depends on the platform."""
    if test.platformIsDarwin():
        return thread.GetStopReason(
        ) == lldb.eStopReasonException and "EXC_BAD_ACCESS" in thread.GetStopDescription(100)
    elif test.getPlatform() == "linux":
        return thread.GetStopReason() == lldb.eStopReasonSignal and thread.GetStopReasonDataAtIndex(
            0) == thread.GetProcess().GetUnixSignals().GetSignalNumberFromName("SIGSEGV")
    else:
        return "invalid address" in thread.GetStopDescription(100)

Presumably we want the second case to apply on FreeBSD as well when this patch is in, although I don't see why the existing else case shouldn't continue to work correctly.

[labath]

In D35223#834050, @emaste wrote:

With this patch I observed three new failures on FreeBSD and three new unexpected passes on FreeBSD. An example of a new failure:

======================================================================
FAIL: test_inferior_crashing_expr_step_and_expr_dwarf (TestInferiorCrashing.CrashingInferiorTestCase)
   Test that lldb expressions work before and after stepping after a crash.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1732, in dwarf_test_method
    return attrvalue(self)
  File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/decorators.py", line 110, in wrapper
    func(*args, **kwargs)
  File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/inferior-crashing/TestInferiorCrashing.py", line 82, in test_inferior_crashing_expr_step_and_expr
    self.inferior_crashing_expr_step_expr()
  File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/inferior-crashing/TestInferiorCrashing.py", line 249, in inferior_crashing_expr_step_expr
    self.check_stop_reason()
  File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/inferior-crashing/TestInferiorCrashing.py", line 93, in check_stop_reason
    STOPPED_DUE_TO_EXC_BAD_ACCESS)
AssertionError: 0 != 1 : Process should be stopped due to bad access exception
Config=x86_64-/usr/bin/cc
----------------------------------------------------------------------
Ran 7 tests in 4.320s

From lldbsuite/test/lldbutil.py:

def is_thread_crashed(test, thread):
    """In the test suite we dereference a null pointer to simulate a crash. The way this is
    reported depends on the platform."""
    if test.platformIsDarwin():
        return thread.GetStopReason(
        ) == lldb.eStopReasonException and "EXC_BAD_ACCESS" in thread.GetStopDescription(100)
    elif test.getPlatform() == "linux":
        return thread.GetStopReason() == lldb.eStopReasonSignal and thread.GetStopReasonDataAtIndex(
            0) == thread.GetProcess().GetUnixSignals().GetSignalNumberFromName("SIGSEGV")
    else:
        return "invalid address" in thread.GetStopDescription(100)

Presumably we want the second case to apply on FreeBSD as well when this patch is in, although I don't see why the existing else case shouldn't continue to work correctly.

Actually, I think you probably need to extend the @skipIfLinux to apply to freebsd as well. The reason is that the test is doing something which goes fundamentally against this patch -- it expects that "step" after a "crash" is a no-op, whereas the new behavior will be to let the process continue (and probably exit, unless it has a SEGV handler). That behavior may make sense for mach exceptions, but I don't think we should try to make posix signals emulate that behavior.

[emaste]

Actually, I think you probably need to extend the @skipIfLinux to apply to freebsd as well.

Oh, I mistook the source of the check_stop_reason assertion failure -- I thought it was asserting before the step. In this case I agree the test should be skipped on FreeBSD.

[karnajitw]

Updated the diff to include the changes that were required in the test part. Please verify.

[emaste]

Still would appreciate diffs uploaded with full context - i.e.,

Note that you can upload patches created through various diff tools, including git and svn. To make reviews easier, please always include as much context as possible with your diff! Don’t worry, Phabricator will automatically send a diff with a smaller context in the review email, but having the full file in the web interface will help the reviewer understand your code.

To get a full diff, use one of the following commands (or just use Arcanist to upload your patch):

git show HEAD -U999999 > mypatch.patch
git format-patch -U999999 @{u}
svn diff --diff-cmd=diff -x -U999999

However, patch looks good to me.

[karnajitw]

Thanks for the info. I will make sure of it in future.

lldb/trunk/packages/Python/lldbsuite/test/functionalities/inferior-crashing/TestInferiorCrashing.py

 from lldbsuite.test.lldbtest import *
 
 
 class CrashingInferiorTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     @expectedFailureAll(
-        oslist=['freebsd'],
-        bugnumber="llvm.org/pr23699 SIGSEGV is reported as exception, not signal")
-    @expectedFailureAll(
         oslist=["windows"],
         bugnumber="llvm.org/pr24778, This actually works, but the test relies on the output format instead of the API")
     def test_inferior_crashing(self):
         """Test that lldb reliably catches the inferior crashing (command)."""
         self.build()
         self.inferior_crashing()
 
     @expectedFailureAll(
class CrashingInferiorTestCase(TestBase):
     @expectedFailureAll(
         oslist=["windows"],
         bugnumber="llvm.org/pr24778, This actually works, but the test relies on the output format instead of the API")
     def test_inferior_crashing_step(self):
         """Test that stepping after a crash behaves correctly."""
         self.build()
         self.inferior_crashing_step()
 
-    @expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr24939')
     @expectedFailureAll(
         oslist=["windows"],
         bugnumber="llvm.org/pr24778, This actually works, but the test relies on the output format instead of the API")
     @skipIfTargetAndroid()  # debuggerd interferes with this test on Android
     def test_inferior_crashing_step_after_break(self):
         """Test that lldb functions correctly after stepping through a crash."""
         self.build()
         self.inferior_crashing_step_after_break()
 
     @expectedFailureAll(
         oslist=["windows"],
         bugnumber="llvm.org/pr24778, This actually works, but the test relies on the output format instead of the API")
     # Inferior exits after stepping after a segfault. This is working as
     # intended IMHO.
     @skipIfLinux
+    @skipIfFreeBSD
     def test_inferior_crashing_expr_step_and_expr(self):
         """Test that lldb expressions work before and after stepping after a crash."""
         self.build()
         self.inferior_crashing_expr_step_expr()
 
     def set_breakpoint(self, line):
         lldbutil.run_break_set_by_file_and_line(
             self, "main.c", line, num_expected_locations=1, loc_exact=True)
def inferior_crashing(self):
         """Inferior crashes upon launching; lldb should catch the event and stop."""
         exe = os.path.join(os.getcwd(), "a.out")
         self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
 
         self.runCmd("run", RUN_SUCCEEDED)
         # The exact stop reason depends on the platform
         if self.platformIsDarwin():
             stop_reason = 'stop reason = EXC_BAD_ACCESS'
-        elif self.getPlatform() == "linux":
+        elif self.getPlatform() == "linux" or self.getPlatform() == "freebsd":
             stop_reason = 'stop reason = signal SIGSEGV'
         else:
             stop_reason = 'stop reason = invalid address'
         self.expect("thread list", STOPPED_DUE_TO_EXC_BAD_ACCESS,
                     substrs=['stopped',
                              stop_reason])
 
         # And it should report the correct line number.

lldb/trunk/packages/Python/lldbsuite/test/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferior.py

 from lldbsuite.test import lldbplatformutil
 from lldbsuite.test import lldbutil
 
 
 class CrashingRecursiveInferiorTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
-    @expectedFailureAll(
-        oslist=['freebsd'],
-        bugnumber="llvm.org/pr23699 SIGSEGV is reported as exception, not signal")
     @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778")
     def test_recursive_inferior_crashing(self):
         """Test that lldb reliably catches the inferior crashing (command)."""
         self.build()
         self.recursive_inferior_crashing()
 
     @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778")
     def test_recursive_inferior_crashing_register(self):
def test_recursive_inferior_crashing_expr(self):
         self.recursive_inferior_crashing_expr()
 
     @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778")
     def test_recursive_inferior_crashing_step(self):
         """Test that stepping after a crash behaves correctly."""
         self.build()
         self.recursive_inferior_crashing_step()
 
-    @expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr24939')
     @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778")
     @skipIfTargetAndroid()  # debuggerd interferes with this test on Android
     def test_recursive_inferior_crashing_step_after_break(self):
         """Test that lldb functions correctly after stepping through a crash."""
         self.build()
         self.recursive_inferior_crashing_step_after_break()
 
     # Inferior exits after stepping after a segfault. This is working as
     # intended IMHO.
     @skipIfLinux
+    @skipIfFreeBSD
     @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778")
     def test_recursive_inferior_crashing_expr_step_and_expr(self):
         """Test that lldb expressions work before and after stepping after a crash."""
         self.build()
         self.recursive_inferior_crashing_expr_step_expr()
 
     def set_breakpoint(self, line):
         lldbutil.run_break_set_by_file_and_line(
def recursive_inferior_crashing(self):
         exe = os.path.join(os.getcwd(), "a.out")
         self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
 
         self.runCmd("run", RUN_SUCCEEDED)
 
         # The exact stop reason depends on the platform
         if self.platformIsDarwin():
             stop_reason = 'stop reason = EXC_BAD_ACCESS'
-        elif self.getPlatform() == "linux":
+        elif self.getPlatform() == "linux" or self.getPlatform() == "freebsd":
             stop_reason = 'stop reason = signal SIGSEGV'
         else:
             stop_reason = 'stop reason = invalid address'
         self.expect("thread list", STOPPED_DUE_TO_EXC_BAD_ACCESS,
                     substrs=['stopped',
                              stop_reason])
 
         # And it should report a backtrace that includes main and the crash

lldb/trunk/packages/Python/lldbsuite/test/functionalities/signal/handle-segv/TestHandleSegv.py

 
 
 class HandleSegvTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     @skipIfWindows  # signals do not exist on Windows
     @skipIfDarwin
-    @expectedFailureAll(
-        oslist=['freebsd'],
-        bugnumber="llvm.org/pr23699 SIGSEGV is reported as exception, not signal")
     def test_inferior_handle_sigsegv(self):
         self.build()
         exe = os.path.join(os.getcwd(), "a.out")
 
         # Create a target by the debugger.
         target = self.dbg.CreateTarget(exe)
         self.assertTrue(target, VALID_TARGET)
 

lldb/trunk/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp

void FreeBSDThread::Notify(const ProcessMessage &message) {
   case ProcessMessage::eExitMessage:
     // Nothing to be done.
     break;
 
   case ProcessMessage::eLimboMessage:
     LimboNotify(message);
     break;
 
+  case ProcessMessage::eCrashMessage:
   case ProcessMessage::eSignalMessage:
     SignalNotify(message);
     break;
 
   case ProcessMessage::eSignalDeliveredMessage:
     SignalDeliveredNotify(message);
     break;
 
   case ProcessMessage::eTraceMessage:
     TraceNotify(message);
     break;
 
   case ProcessMessage::eBreakpointMessage:
     BreakNotify(message);
     break;
 
   case ProcessMessage::eWatchpointMessage:
     WatchNotify(message);
     break;
 
-  case ProcessMessage::eCrashMessage:
-    CrashNotify(message);
-    break;
-
   case ProcessMessage::eExecMessage:
     ExecNotify(message);
     break;
   }
 }
 
 bool FreeBSDThread::EnableHardwareWatchpoint(Watchpoint *wp) {
   bool wp_set = false;

 }
 
 void FreeBSDThread::LimboNotify(const ProcessMessage &message) {
   SetStopInfo(lldb::StopInfoSP(new POSIXLimboStopInfo(*this)));
 }
 
 void FreeBSDThread::SignalNotify(const ProcessMessage &message) {
   int signo = message.GetSignal();
+  if (message.GetKind() == ProcessMessage::eCrashMessage) {
+    std::string stop_description = GetCrashReasonString(
+        message.GetCrashReason(), message.GetFaultAddress());
+    SetStopInfo(StopInfo::CreateStopReasonWithSignal(
+        *this, signo, stop_description.c_str()));
+  } else {
     SetStopInfo(StopInfo::CreateStopReasonWithSignal(*this, signo));
   }
+}
 
 void FreeBSDThread::SignalDeliveredNotify(const ProcessMessage &message) {
   int signo = message.GetSignal();
   SetStopInfo(StopInfo::CreateStopReasonWithSignal(*this, signo));
 }
 
-void FreeBSDThread::CrashNotify(const ProcessMessage &message) {
-  // FIXME: Update stop reason as per bugzilla 14598
-  int signo = message.GetSignal();
-
-  assert(message.GetKind() == ProcessMessage::eCrashMessage);
-
-  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD));
-  if (log)
-    log->Printf("FreeBSDThread::%s () signo = %i, reason = '%s'", __FUNCTION__,
-                signo, message.PrintCrashReason());
-
-  SetStopInfo(lldb::StopInfoSP(new POSIXCrashStopInfo(
-      *this, signo, message.GetCrashReason(), message.GetFaultAddress())));
-}
-
 unsigned FreeBSDThread::GetRegisterIndexFromOffset(unsigned offset) {
   unsigned reg = LLDB_INVALID_REGNUM;
   ArchSpec arch = HostInfo::GetArchitecture();
 
   switch (arch.GetMachine()) {
   default:
     llvm_unreachable("CPU type not supported!");
     break;

lldb/trunk/source/Plugins/Process/FreeBSD/POSIXStopInfo.h

public:
   const char *GetDescription();
 
   bool ShouldStop(lldb_private::Event *event_ptr);
 
   bool ShouldNotify(lldb_private::Event *event_ptr);
 };
 
 //===----------------------------------------------------------------------===//
-/// @class POSIXCrashStopInfo
-/// @brief Represents the stop state of process that is ready to crash.
-///
-class POSIXCrashStopInfo : public POSIXStopInfo {
-public:
-  POSIXCrashStopInfo(FreeBSDThread &thread, uint32_t status, CrashReason reason,
-                     lldb::addr_t fault_addr);
-  ~POSIXCrashStopInfo();
-
-  lldb::StopReason GetStopReason() const;
-};
-
-//===----------------------------------------------------------------------===//
 /// @class POSIXNewThreadStopInfo
 /// @brief Represents the stop state of process when a new thread is spawned.
 ///
 
 class POSIXNewThreadStopInfo : public POSIXStopInfo {
 public:
   POSIXNewThreadStopInfo(FreeBSDThread &thread) : POSIXStopInfo(thread, 0) {}
 

lldb/trunk/source/Plugins/Process/FreeBSD/POSIXStopInfo.cpp

 
 const char *POSIXLimboStopInfo::GetDescription() { return "thread exiting"; }
 
 bool POSIXLimboStopInfo::ShouldStop(Event *event_ptr) { return false; }
 
 bool POSIXLimboStopInfo::ShouldNotify(Event *event_ptr) { return false; }
 
 //===----------------------------------------------------------------------===//
-// POSIXCrashStopInfo
-
-POSIXCrashStopInfo::POSIXCrashStopInfo(FreeBSDThread &thread, uint32_t status,
-                                       CrashReason reason,
-                                       lldb::addr_t fault_addr)
-    : POSIXStopInfo(thread, status) {
-  m_description = ::GetCrashReasonString(reason, fault_addr);
-}
-
-POSIXCrashStopInfo::~POSIXCrashStopInfo() {}
-
-lldb::StopReason POSIXCrashStopInfo::GetStopReason() const {
-  return lldb::eStopReasonException;
-}
-
-//===----------------------------------------------------------------------===//
 // POSIXNewThreadStopInfo
 
 POSIXNewThreadStopInfo::~POSIXNewThreadStopInfo() {}
 
 lldb::StopReason POSIXNewThreadStopInfo::GetStopReason() const {
   return lldb::eStopReasonNone;
 }
 
 const char *POSIXNewThreadStopInfo::GetDescription() {
   return "thread spawned";
 }
 
 bool POSIXNewThreadStopInfo::ShouldStop(Event *event_ptr) { return false; }
 
 bool POSIXNewThreadStopInfo::ShouldNotify(Event *event_ptr) { return false; }

lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp

ProcessMessage ProcessMonitor::MonitorSignal(ProcessMonitor *monitor,
 
   switch (signo) {
   case SIGSEGV:
   case SIGILL:
   case SIGFPE:
   case SIGBUS:
     lldb::addr_t fault_addr = reinterpret_cast<lldb::addr_t>(info->si_addr);
     const auto reason = GetCrashReason(*info);
+    if (reason != CrashReason::eInvalidCrashReason) {
       return ProcessMessage::Crash(tid, reason, signo, fault_addr);
+    } // else; Use atleast si_signo info for other si_code
   }
 
   // Everything else is "normal" and does not require any special action on
   // our part.
   return ProcessMessage::Signal(tid, signo);
 }
 
 void ProcessMonitor::ServeOperation(OperationArgs *args) {