-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
Fix alignment of uc_mcontext in ucontext_t on arm64 android #3894
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @tgross35 (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
Thanks! Does |
Apparently it does! Also fixed the visibility, although you still won't be able to construct a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Since this is a fix to a tier 2 target, it should be okay to backport.
@rustbot label +stable-nominated
Hey @maurer would you mind sanity checking this? I was going to backport it but figure it is worth double checking since this is a breaking change. |
tl;dr: Change is fine, but I think we have this bug for This both seems correct and should be safe to backport, but I think it's actually incomplete. For Android, this is also the case in riscv - we should probably add it to the rust-side riscv However, I think this may be even worse - I checked the Linux headers for an unmodified upstream kernel - they also have this padding, but our |
(backport <rust-lang#3894>) (cherry picked from commit 707d32c)
(backport <rust-lang#3894>) (cherry picked from commit 707d32c)
(backport <rust-lang#3894>) (cherry picked from commit 707d32c)
(backport <rust-lang#3894>) (cherry picked from commit 707d32c)
(backport <rust-lang#3894>) (cherry picked from commit 707d32c)
[ change `core` to `::core` for backport - Trevor ] (backport <rust-lang#3894>) (cherry picked from commit 707d32c)
[ change `core` to `::core` and move ucontext_t to s_no_extra_traits for backport - Trevor ] (backport <rust-lang#3894>) (cherry picked from commit 707d32c)
[ change `core` to `::core` and move ucontext_t to s_no_extra_traits for backport - Trevor ] (backport <rust-lang#3894>) (cherry picked from commit 707d32c)
[ change `core` to `::core` and move ucontext_t to s_no_extra_traits for backport - Trevor ] (backport <rust-lang#3894>) (cherry picked from commit 707d32c)
On ARM64 Android there should be padding between
uc_sigmask
anduc_mcontext
inlibc::ucontext_t
, see hereNow
core::mem::offset_of!(libc::ucontext_t, uc_mcontext)
is the expected value of0xB0
fixes #3655