Skip to content

Stabilize ARM and AArch64 support on both Linux and Android, add support for x86-64 on Android#81

Merged
gabrielesvelto merged 2 commits intorust-minidump:mainfrom
gabrielesvelto:android-support-take-2
Jun 13, 2023
Merged

Stabilize ARM and AArch64 support on both Linux and Android, add support for x86-64 on Android#81
gabrielesvelto merged 2 commits intorust-minidump:mainfrom
gabrielesvelto:android-support-take-2

Conversation

@gabrielesvelto
Copy link
Copy Markdown
Contributor

No description provided.

…ort for x86-64 on Android

This *does not* add support for i686 on Android as it would require
significant work for almost no benefit. Android emulators usually use
x86-64 and only very old versions run on i686.
This fixes a regression introduced in
e706902 which prevented stack mappings
from being correctly merged on Android.
Copy link
Copy Markdown
Collaborator

@Jake-Shadle Jake-Shadle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks reasonable, but we really should find a nice way of adding tests, or at least sanity checks of some of this stuff to make reviews easier.

@gabrielesvelto
Copy link
Copy Markdown
Contributor Author

This change looks reasonable, but we really should find a nice way of adding tests, or at least sanity checks of some of this stuff to make reviews easier.

Is it possible to run tests on Android hosts in GitHub CI? I suppose we could use an emulator, that'd cover the x86-64 part at least. Once I merge this into Firefox we'll be running end-to-end tests on both Android hardware and the emulator but it'll still be nicer to have tests here.

@gabrielesvelto gabrielesvelto merged commit 76021df into rust-minidump:main Jun 13, 2023
@Jake-Shadle
Copy link
Copy Markdown
Collaborator

Well tests are harder for Android, when saying sanity checks I was more thinking just dumping out byte buffers and ensuring they can be interpreted correctly across changes in the code/libc/nix, but obviously has much lower value. But might be worth at least mentioning that Firefox is running more tests in addition to those in the repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants