Add Arm64 version of breakpad_getcontext for Android.

Add Arm64 version of breakpad_getcontext for Android.

This CL adds breakpad_getcontext support for Arm64 to Android. The assembly
is based on getcontext.S in glibc.

BUG=354405,335641
R=mark@chromium.org

Committed: 1302

[rmcilroy]

Part 2 of the Arm64 work - implementing breakpad_getcontext.S.  PTAL.

https://breakpad.appspot.com/1384002/diff/1/common/android/breakpad_getcontext.S
File common/android/breakpad_getcontext.S (right):

https://breakpad.appspot.com/1384002/diff/1/common/android/breakpad_getcontex...
common/android/breakpad_getcontext.S:131: /* Write the context extension fpsimd
header.  */
Do we need to save the fpsimd context for aarch64?  The comment in Arm suggests
it's not required for arm, but I'm not sure for arm64, and don't know how
breakpad uses the context well enough to know if we need the fpsimd context too.
 Any idea Mark?

[Mark Mentovai]

LGTM in that it looks reasonable, but ARM64 is pretty new to me.

https://breakpad.appspot.com/1384002/diff/1/common/android/breakpad_getcontext.S
File common/android/breakpad_getcontext.S (right):

https://breakpad.appspot.com/1384002/diff/1/common/android/breakpad_getcontex...
common/android/breakpad_getcontext.S:37: #ifdef __arm__
Change to #if defined(…) to match all of the #elif defined(…)s that follow.

https://breakpad.appspot.com/1384002/diff/1/common/android/breakpad_getcontex...
common/android/breakpad_getcontext.S:116: already arrange to return the
appropriate return value in x0
arranged

https://breakpad.appspot.com/1384002/diff/1/common/android/breakpad_getcontex...
common/android/breakpad_getcontext.S:131: /* Write the context extension fpsimd
header.  */
rmcilroy wrote:
> Do we need to save the fpsimd context for aarch64?  The comment in Arm
suggests
> it's not required for arm, but I'm not sure for arm64, and don't know how
> breakpad uses the context well enough to know if we need the fpsimd context
too.
>  Any idea Mark?

Breakpad won’t use it for the stack walk, but the data might still come in
handy. There have been a couple of cases in the past where having FP or SIMD
register state handy in a dump file has helped pinpoint a problem.

[rmcilroy]

Updated CrashContext for aarch64.  PTAL.

https://breakpad.appspot.com/1384002/diff/1/common/android/breakpad_getcontext.S
File common/android/breakpad_getcontext.S (right):

https://breakpad.appspot.com/1384002/diff/1/common/android/breakpad_getcontex...
common/android/breakpad_getcontext.S:37: #ifdef __arm__
On 2014/04/02 16:22:18, Mark Mentovai wrote:
> Change to #if defined(…) to match all of the #elif defined(…)s that follow.

Done.

https://breakpad.appspot.com/1384002/diff/1/common/android/breakpad_getcontex...
common/android/breakpad_getcontext.S:116: already arrange to return the
appropriate return value in x0
On 2014/04/02 16:22:18, Mark Mentovai wrote:
> arranged

Done.

https://breakpad.appspot.com/1384002/diff/1/common/android/breakpad_getcontex...
common/android/breakpad_getcontext.S:131: /* Write the context extension fpsimd
header.  */
On 2014/04/02 16:22:18, Mark Mentovai wrote:
> rmcilroy wrote:
> > Do we need to save the fpsimd context for aarch64?  The comment in Arm
> suggests
> > it's not required for arm, but I'm not sure for arm64, and don't know how
> > breakpad uses the context well enough to know if we need the fpsimd context
> too.
> >  Any idea Mark?
> 
> Breakpad won’t use it for the stack walk, but the data might still come in
> handy. There have been a couple of cases in the past where having FP or SIMD
> register state handy in a dump file has helped pinpoint a problem.

Sounds good, I've updated exception_handler appropriately now.

[Mark Mentovai]

https://breakpad.appspot.com/1384002/diff/30001/client/linux/minidump_writer/...
File client/linux/minidump_writer/minidump_writer.cc (right):

https://breakpad.appspot.com/1384002/diff/30001/client/linux/minidump_writer/...
client/linux/minidump_writer/minidump_writer.cc:1760: const struct
fpsimd_context* const float_state_;  // ditto
There are a few spots where you switch between _libc_fpstate and fpsimd_context
depending on the aarch64-iness of the target. Does a typedef make sense?

https://breakpad.appspot.com/1384002/diff/30001/client/linux/minidump_writer/...
client/linux/minidump_writer/minidump_writer.cc:1762: const struct
_libc_fpstate* const float_state_;  // ditto
Maybe this should be excluded on 32-bit ARM and MIPS, per the comment in
exception_handler.h and the #if in this file on line 486. This will also require
an #if at line 851. (The initializer list in the constructor already has the
#if.)

[rmcilroy]

https://breakpad.appspot.com/1384002/diff/30001/client/linux/minidump_writer/...
File client/linux/minidump_writer/minidump_writer.cc (right):

https://breakpad.appspot.com/1384002/diff/30001/client/linux/minidump_writer/...
client/linux/minidump_writer/minidump_writer.cc:1760: const struct
fpsimd_context* const float_state_;  // ditto
On 2014/04/03 00:26:24, Mark Mentovai wrote:
> There are a few spots where you switch between _libc_fpstate and
fpsimd_context
> depending on the aarch64-iness of the target. Does a typedef make sense?

Yes, that would probably be cleaner - done (in minidump_writer.h)

https://breakpad.appspot.com/1384002/diff/30001/client/linux/minidump_writer/...
client/linux/minidump_writer/minidump_writer.cc:1762: const struct
_libc_fpstate* const float_state_;  // ditto
On 2014/04/03 00:26:24, Mark Mentovai wrote:
> Maybe this should be excluded on 32-bit ARM and MIPS, per the comment in
> exception_handler.h and the #if in this file on line 486. This will also
require
> an #if at line 851. (The initializer list in the constructor already has the
> #if.)

Done.

[Mark Mentovai]

LGTM

[rmcilroy]

Committed patchset #3 manually as r1302 (presubmit successful).