Fix crash in crash handler during C++ exception reporting #219
Conversation
Our __cxa_throw override is only called if the exception is thrown from the same image as our __cxa_throw lives in, e.g. the main app if KSCrash is linked in as a static library. In cases where it's not called the advanceCursor and resetCursor members of the KSStackCursor are 0, resulting in a crash during crash handling. By always initializing the stackCursor we at least ensure that we don't crash during reporting, even if the result is an empty stack trace for that thread.
torarnv
force-pushed
the
cpp-exception-crash-fix
branch
from
60d493b
to
42f580c
Compare
| if(!isInitialized) | ||
| { | ||
| isInitialized = true; | ||
| kssc_initCursor(&g_stackCursor, NULL, NULL); |
There was a problem hiding this comment.
Perhaps a better place is inside CPPExceptionTerminate(), where we can look at g_stackCursor's advanceCursor to see if we need an init? Don't know if there's any way we can mark the backtrace as explicitly missing, so that it looks better in the reports? Either way we should probably KSLOG_WARN about the situation. Are any of the KSStackCursor fields appropriate to detect that nothing was filled out? currentDepth == 0? address == 0?
There was a problem hiding this comment.
By the time terminate is called, the stack trace has been obliterated by the C++ runtime, so it has to be initialized before an exception occurs.
There was a problem hiding this comment.
Ah, yes, I didn't mean to call kssc_initSelfThread(&g_stackCursor, 1); to init the actual stack trace from CPPExceptionTerminate(), I was talking about whether to delay the fallback-init of kssc_initCursor(&g_stackCursor, NULL, NULL); to CPPExceptionTerminate(), based on detecting that kssc_initSelfThreadwas never called. As opposed to an unconditional initialize from setEnabled.
There was a problem hiding this comment.
Eg something like:
if (!g_stackCursor.advanceCursor) {
KSLOG_WARN("Failed to resolve stack trace for C++ exception");
kssc_initCursor(&g_stackCursor, NULL, NULL);
}There was a problem hiding this comment.
I think it's fine to leave this init to the very start of the app, so that you don't need to keep checking it. If cxa_throw gets intercepted, the stack cursor vtable will change.
What we can do, however, is add a warning to the empty implementation of advanceCursor() in KSStackCursor.c to include a warning message.
See issue #205