Getting context information from the kernel when catching a SIGABRT on iOS.

Getting context information from the kernel when catching a SIGABRT on iOS.

 Until now, the context information was the current one when receiving a
SIGABRT. This is mainly wrong because the signal handler start in a new
context. This instead use the context passed to the signal handler.
Committed: https://code.google.com/p/google-breakpad/source/detail?r=1015

[qsr]

Hi Mark,

 This change use the context from the signal instead of getting the current
signal when the dump is due to a signal.

 Now, it is in fact not that useful, the reason being that creating a dump in a
signal handler crash 1 time out of 2 because it uses libc call to
open/seek/write/close files.

 Linux solve this issue by directly using syscall. I might try to go that way
later for iOS too, but I need to find out if that will be considered a private
API call first.

[Mark Mentovai]

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/exception_h...
File src/client/mac/handler/exception_handler.cc (right):

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/exception_h...
src/client/mac/handler/exception_handler.cc:370:
md.SetTaskContext(task_context);
MinidumpGenerator::SetTaskContext asserts that it’s running on ARM. I don’t
believe that there’s anything that prevents this line from executing on other
CPUs. This will trip that assertion.

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/exception_h...
File src/client/mac/handler/exception_handler.h (right):

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/exception_h...
src/client/mac/handler/exception_handler.h:189: ucontext_t *task_context,
This could use some documentation about the expected use here. task_context is
allowed to be NULL. You should definitely say that. If the meanings of any of
the other fields change or are ignored depending on whether task_context is
NULL, you should also point that out in the comment.

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/minidump_ge...
File src/client/mac/handler/minidump_generator.cc (right):

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/minidump_ge...
src/client/mac/handler/minidump_generator.cc:781: if (target_thread !=
mach_task_self() ||
You’re comparing a thread to a task (process) here?

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/minidump_ge...
src/client/mac/handler/minidump_generator.cc:817: final_size =
sizeof(arm_thread_state_t);
final_size = std::min(*count, sizeof(arm_thread_state_t) ?

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/minidump_ge...
File src/client/mac/handler/minidump_generator.h (right):

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/minidump_ge...
src/client/mac/handler/minidump_generator.h:105: // Specify the task context.
You could probably use a little more documentation here about how this changes
things.

[qsr]

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/exception_h...
File src/client/mac/handler/exception_handler.cc (right):

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/exception_h...
src/client/mac/handler/exception_handler.cc:370:
md.SetTaskContext(task_context);
 Changed SetTaskContent to not trigger assertion. Instead I only use
task_context when I know what to do depending on the processor.

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/exception_h...
File src/client/mac/handler/exception_handler.h (right):

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/exception_h...
src/client/mac/handler/exception_handler.h:189: ucontext_t *task_context,
On 2012/08/17 17:44:07, Mark Mentovai wrote:
> This could use some documentation about the expected use here. task_context is
> allowed to be NULL. You should definitely say that. If the meanings of any of
> the other fields change or are ignored depending on whether task_context is
> NULL, you should also point that out in the comment.

Done.

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/minidump_ge...
File src/client/mac/handler/minidump_generator.cc (right):

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/minidump_ge...
src/client/mac/handler/minidump_generator.cc:781: if (target_thread !=
mach_task_self() ||
 Done, thanks.

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/minidump_ge...
src/client/mac/handler/minidump_generator.cc:817: final_size =
sizeof(arm_thread_state_t);
On 2012/08/17 17:44:07, Mark Mentovai wrote:
> final_size = std::min(*count, sizeof(arm_thread_state_t) ?

Done.

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/minidump_ge...
File src/client/mac/handler/minidump_generator.h (right):

https://breakpad.appspot.com/435002/diff/1/src/client/mac/handler/minidump_ge...
src/client/mac/handler/minidump_generator.h:105: // Specify the task context.
On 2012/08/17 17:44:07, Mark Mentovai wrote:
> You could probably use a little more documentation here about how this changes
> things.

Done.

[Mark Mentovai]

LGTM