Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Android support to gimli #415

Merged
merged 1 commit into from
Apr 26, 2021
Merged

Add Android support to gimli #415

merged 1 commit into from
Apr 26, 2021

Conversation

kjvalencik
Copy link
Contributor

@kjvalencik kjvalencik commented Apr 9, 2021 •
edited
Loading

Depends on rust-lang/libc#2144
Resolves #351

dl_iterate_phdr was added in API Version 21; however, it's very old and I couldn't figure out how to build for it. I believe this will fail to link on API version 20 and below, but I'm not certain.

Any suggestions for the best way to handle it? Any suggestions for testing would be appreciated as well.

@kjvalencik kjvalencik mentioned this pull request Apr 9, 2021
Merged
bors added a commit to rust-lang/libc that referenced this pull request Apr 12, 2021
@bors
Auto merge of #2144 - kjvalencik:master, r=JohnTitor
d3ca6a7 
Add dl_iterate_phdr to Android

Adds the `dl_iterate_phdr` function for Android targets. This is required for Android support in `gimli` and by proxy, `backtrace`.

I tested this in [`backtrace`](rust-lang/backtrace-rs#415) both in an i686 emulator and a physical arm64 device.

This API is only available on Version 21+. I'm not sure how that's typically handled in `libc`, so I added a doc comment.

Let me know what else is needed!
@kjvalencik
Copy link
Contributor Author

@alexcrichton Do you have any suggestions for handling the Android API version check? Options that I've thought of:

  • Require users to enable it with a feature flag
  • Port the existing check that uses cc in a build.rs
  • Don't use libc and instead load the symbol dynamically with libloading

@alexcrichton
Copy link
Member

Thanks for this! I'd probably say that the second option, using the same build.rs check to see what the compiler api level reports, is the best for now. That way folks who want backwards-compat can get it and otherwise we'll automatically enable backtraces for more recent android versions (which should be basically all of them)

@kjvalencik kjvalencik force-pushed the kv/android-gimli branch 3 times, most recently from 4d8cc0d to b75d868 Compare April 16, 2021 19:46
@alexcrichton
Copy link
Member

Looks great to me! I'm happy to merge once libc is published with the update.

@kjvalencik kjvalencik mentioned this pull request Apr 19, 2021
Closed
@kjvalencik kjvalencik changed the title wip: Android support in gimli Add Android support to gimli Apr 26, 2021
@kjvalencik kjvalencik marked this pull request as ready for review April 26, 2021 14:53
@kjvalencik
Copy link
Contributor Author

Thanks again for the review @alexcrichton. libc has been published with the required function and the PR has been updated to point at the latest version.

@alexcrichton alexcrichton merged commit 6b13969 into rust-lang:master Apr 26, 2021
@alexcrichton
Copy link
Member

Looks great to me, thanks!

@s1341
Copy link

s1341 commented Apr 27, 2021

Awesome. Can we get a release so we can integrate this?

@alexcrichton
Copy link
Member

Sure thing, done now!

@s1341
Copy link

s1341 commented Apr 28, 2021

Thanks!

@s1341
Copy link

s1341 commented Apr 28, 2021

Confirmed working! Thanks all!

@bjorn3 bjorn3 mentioned this pull request Oct 13, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Gimli doesn't support Android
3 participants
@kjvalencik @alexcrichton @s1341