Commit 28ef22ea authored by Mingtao Yang's avatar Mingtao Yang Committed by Facebook GitHub Bot

fibers: Invoke old sigsegv handler synchronously

Summary:
folly::fibers uses a SIGSEGV handler to detect fiber overflows, by checking if
the faulting address resides within a guard page range.

In the case when it does not, prior to this diff, the folly::fibers signal
handler would reraise the signal and allow the previous sigsegv handler handle
the exception.

The problem with this is that reraising the signal will discard the original
`siginfo` context sent by the kernel. The old signal handler would lose
visibility on the machine state, faulting condition, etc.

This does not play nicely with folly::symbolizer. Typically,
folly::symbolizer's signal handler will print information on the faulting
signal, such as:

```
*** Signal 11 (SIGSEGV) (0xdeadbeef) received by PID 3140647 (pthread TID 0x7fe2cddf1380) (linux TID 3140647) (code: address not mapped to object), stack trace: ***
    @ 00000000002f4d3d folly::symbolizer::(anonymous namespace)::signalHandler(int, siginfo_t*, void*)
                       ./folly/experimental/symbolizer/SignalHandler.cpp:449
    @ 0000000000000000 (unknown)
    @ 00000000002dd0f9 doCrashWildRead()
                       ./scripts/mingtao/oneoffs/crash.cc:9
    @ 00000000002dd0b1 main
                       ./scripts/mingtao/oneoffs/crash.cc:47
    @ 0000000000025dc4 __libc_start_main
    @ 00000000002dcfb9 _start
                       /home/engshare/third-party2/glibc/2.30/src/glibc-2.30/csu/../sysdeps/x86_64/start.S:120
Segmentation fault (core dumped)
```

From the error message, you can see that:
* the faulting address was 0xdeadbeef
* the reason for SIGSEGV was because the application accessed a region of memory outside of its virtual address space

If you happened to have used `folly::fibers` somewhere within your application, though, you instead would see this:

```
*** Aborted at 1639532805 (Unix time, try 'date -d 1639532805') ***
*** Signal 11 (SIGSEGV) (0x1c85b0032a0f8) received by PID 3318008 (pthread TID 0x7f5573e8d380) (linux TID 3318008) (maybe from PID 3318008, UID 116827) (code: -6), stack trace: ***
    @ 00000000002f4e0d folly::symbolizer::(anonymous namespace)::signalHandler(int, siginfo_t*, void*)
                       ./folly/experimental/symbolizer/SignalHandler.cpp:449
    @ 0000000000000000 (unknown)
    @ 00000000002dd169 doCrashWildRead()
                       ./scripts/mingtao/oneoffs/crash.cc:9
    @ 00000000002dd124 main
                       ./scripts/mingtao/oneoffs/crash.cc:48
    @ 0000000000025dc4 __libc_start_main
    @ 00000000002dcfb9 _start
                       /home/engshare/third-party2/glibc/2.30/src/glibc-2.30/csu/../sysdeps/x86_64/start.S:120
```

`code: -6` corresponds to `SI_TKILL`, indicating that the signal was a result of some process invoking `tgkill` (this is the fiber handler re-raising the signal)

And `0x1c85b0032a0f8` is some bogus value, since signals raised by `tgkill` do not have a valid si_addr field.

Furthermore, when debugging the core file, `p $_siginfo` will no longer give the relevant machine state at the time of the crash, since `$_siginfo` now would correspond to the
raised signal by folly::fibers, rather than the original fault.

This diff changes the `raise` to a synchronous invocation of the old signal handler. This allows the original signal context to be properly preserved (and visible within core).

Reviewed By: Gownta

Differential Revision: D33165801

fbshipit-source-id: 4526d5a9a93cf1eba13e9f5a16b13d00b3c8c85b
parent 47eda458
......@@ -230,8 +230,14 @@ void sigsegvSignalHandler(int signum, siginfo_t* info, void* ucontext) {
return;
}
// Let the old signal handler handle the signal.
raise(signum);
// Let the old signal handler handle the signal. Invoke this synchronously
// within our own signal handler to ensure that the kernel siginfo context
// is not lost.
if (oldSigsegvAction.sa_flags & SA_SIGINFO) {
oldSigsegvAction.sa_sigaction(signum, info, ucontext);
} else {
oldSigsegvAction.sa_handler(signum);
}
}
bool isInJVM() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment