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.

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

Aarch64 macOS: ucontext_t is not FFI-Safe #2812

Closed
domenukk opened this issue May 31, 2022 · 5 comments · Fixed by #2817 or #3312
Closed

Aarch64 macOS: ucontext_t is not FFI-Safe #2812

domenukk opened this issue May 31, 2022 · 5 comments · Fixed by #2817 or #3312
Labels
C-bug Category: bug

Comments

@domenukk
Copy link

When using ucontext_t in getcontext (manually added, the API seems not to be exported on aarch64 mac in libc), like so:

extern "C" {
    fn getcontext(ucp: *mut ucontext_t) -> c_int;
}

the compiler complains that ucontext_t is not ffi-safe, as 128 bit integers are not supported in FFI.
I did not find any use of 128 bit ints, hence, I assume the compiler is overly smart for the neon state(?) which is:

    #[repr(align(16))]
    pub struct __darwin_arm_neon_state64 {
        pub __v: [[u64; 2]; 32],
        pub __fpsr: u32,
        pub __fpcr: u32,
    }

When I hand-roll the ucontext_t struct, including a neon-state like this, the compiler no longer complains.

#[repr(C, align(16))]
pub struct arm_neon_state64 {
    pub opaque: [u8; (32 * 16) + (2 * mem::size_of::<u32>())],
}

Coincidentally, my testcase would segfault after the original getcontext, which may be related.

This is on macos, aarch64, with libc 0.2.121 and rust 1.59.0

@devnexen
Copy link
Contributor

devnexen commented Jun 4, 2022

Thanks for the report. isn't it because ucontext_t type has a missing field in libc ? mcontext_data. If I hack the cached libc version test_windows_get_core_ids test no longer segfault, only core_affinity fails now.

@devnexen
Copy link
Contributor

devnexen commented Jun 4, 2022

What I can propose eventually :

  • adding the "missing" field here (and tests still pass with) but you keep the getcontext prototype in your side. Indeed the *context api on macos is deprecated and the aforementioned field exists solely when used with the *context api (means setting _XOPEN_SOURCE but that create a lot of build warning in all other parts).
  • keeping all the definition in your side as you are already doing.

Let me know what you think.

@domenukk
Copy link
Author

domenukk commented Jun 4, 2022

As far as I can tell, there is not really any good alternative to these APIs, and of course it would be great to have them somehow officially supported by the libc create (?)
I guess there is no concept of feature flags to enable deprecated APIs by any chance?

devnexen added a commit to devnexen/libc that referenced this issue Jun 4, 2022
@devnexen
Copy link
Contributor

devnexen commented Jun 4, 2022

I ve prepared a change, is it possible for you to temporarily tried out this repo/branch ?

devnexen added a commit to devnexen/libc that referenced this issue Jun 4, 2022
devnexen added a commit to devnexen/libc that referenced this issue Jun 4, 2022
devnexen added a commit to devnexen/libc that referenced this issue Jun 5, 2022
bors added a commit that referenced this issue Jun 7, 2022
ucontext usage update on macos 64 bits.

closes #2812
bors added a commit that referenced this issue Jun 7, 2022
ucontext usage update on macos 64 bits.

closes #2812
@bors bors closed this as completed in 2d11246 Jun 7, 2022
tatref pushed a commit to tatref/libc that referenced this issue Mar 20, 2023
@sug0
Copy link

sug0 commented Jul 30, 2023

I think this change broke the sigaction API on Apple silicon. Related to wasmerio/wasmer#4072

bors pushed a commit that referenced this issue Aug 15, 2023
This commit effectively reverts #2817. Currently `ucontext_t` has both
the wrong size and the wrong alignment for aarch64-apple-darwin which
causes problems for users referencing the structure [1]. The issue
linked from #2817 claimed that it fixed #2812 but that's still an issue
where FFI warnings are still emitted for usage of `ucontext_t` due to
its transitive usage of `u128`. I'm not sure how to fix #2812 myself,
but given that #2817 doesn't appear to solve its original intent and
additionally the size/align are currently wrong this commit reverts in
the meantime.

[1]: bytecodealliance/wasmtime#6785 (comment)
bors added a commit that referenced this issue Aug 15, 2023
…ohnTitor

Fix size/align of `ucontext_t` on aarch64-apple-darwin

This commit effectively reverts #2817. Currently `ucontext_t` has both the wrong size and the wrong alignment for aarch64-apple-darwin which causes problems for users referencing the structure [1]. The issue linked from #2817 claimed that it fixed #2812 but that's still an issue where FFI warnings are still emitted for usage of `ucontext_t` due to its transitive usage of `u128`. I'm not sure how to fix #2812 myself, but given that #2817 doesn't appear to solve its original intent and additionally the size/align are currently wrong this commit reverts in the meantime.

[1]: bytecodealliance/wasmtime#6785 (comment)
bors added a commit that referenced this issue Aug 17, 2023
…ohnTitor

Fix size/align of `ucontext_t` on aarch64-apple-darwin

This commit effectively reverts #2817. Currently `ucontext_t` has both the wrong size and the wrong alignment for aarch64-apple-darwin which causes problems for users referencing the structure [1]. The issue linked from #2817 claimed that it fixed #2812 but that's still an issue where FFI warnings are still emitted for usage of `ucontext_t` due to its transitive usage of `u128`. I'm not sure how to fix #2812 myself, but given that #2817 doesn't appear to solve its original intent and additionally the size/align are currently wrong this commit reverts in the meantime.

[1]: bytecodealliance/wasmtime#6785 (comment)
xevor11 pushed a commit to xevor11/libc that referenced this issue Aug 25, 2023
This commit effectively reverts rust-lang#2817. Currently `ucontext_t` has both
the wrong size and the wrong alignment for aarch64-apple-darwin which
causes problems for users referencing the structure [1]. The issue
linked from rust-lang#2817 claimed that it fixed rust-lang#2812 but that's still an issue
where FFI warnings are still emitted for usage of `ucontext_t` due to
its transitive usage of `u128`. I'm not sure how to fix rust-lang#2812 myself,
but given that rust-lang#2817 doesn't appear to solve its original intent and
additionally the size/align are currently wrong this commit reverts in
the meantime.

[1]: bytecodealliance/wasmtime#6785 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
3 participants