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

Add support for exceptions ports + aarch64 #8

Merged
merged 7 commits into from
Sep 12, 2022

Conversation

bnjbvr
Copy link
Contributor

@bnjbvr bnjbvr commented Apr 11, 2022

This adds enough support for bytecodealliance/wasmtime#2632 to not add its own bindings, plus it brings up support for Apple Silicon (aarch64) by tweaking/adding a few data structures.

One note for reviewers: yes it's weird, but the count() implementations are indeed different when I look at my local mach header files. They are reflected as such in this PR.

@bnjbvr
Copy link
Contributor Author

bnjbvr commented Apr 13, 2022

Sorry I don't have a MacOS machine with x64 around, so I can't debug these tests, especially as they seem to be failing on a C file that's generated by the mach-test crate. Any help or pointers would be appreciated here, please!

@JohnTitor
Copy link
Owner

We have two errors:

  cargo:warning=/Users/runner/work/mach2/mach2/target/x86_64-apple-darwin/debug/build/mach-test-58d335b19de925b4/out/all.c:1892:28: error: incompatible pointer types returning 'integer_t (*)[2]' from a function with result type 'int64_t (*)[2]' [-Werror,-Wincompatible-pointer-types]
  cargo:warning=                    return &b->code;
  cargo:warning=                           ^~~~~~~~
  cargo:warning=/Users/runner/work/mach2/mach2/target/x86_64-apple-darwin/debug/build/mach-test-58d335b19de925b4/out/all.c:10917:24: error: incompatible pointer types returning 'kern_return_t (thread_act_t, exception_mask_t, mach_port_t, exception_behavior_t, thread_state_flavor_t)' (aka 'int (unsigned int, unsigned int, unsigned int, int, int)') from a function with result type 'kern_return_t (*)(thread_port_t, exception_mask_t, mach_port_t, unsigned int, thread_state_flavor_t)' (aka 'int (*)(unsigned int, unsigned int, unsigned int, unsigned int, int)') [-Werror,-Wincompatible-pointer-types]
  cargo:warning=                return thread_set_exception_ports;
  cargo:warning=                       ^~~~~~~~~~~~~~~~~~~~~~~~~~

Both are type-mismatch errors,

On x64 xcode 11.7.0-13.1.0, the code field of the __Request__exception_raise_t sturct is integer_t instead of int64_t, it seems. Most source code on GitHub declares it as integer_t so the declaration seems different with aarch64.
The second is about thread_set_exception_ports, I googled it and the 4th arg is exception_behavior_t (a.k.a. int). Maybe it's also different with aarch64?
If they're valid on aarch64 at all, we could separate the declaration with a cfg, like:

pub struct __Request__exception_raise_t {
    pub Head: mach_msg_header_t,
    /* start of the kernel processed data */
    pub msgh_body: mach_msg_body_t,
    pub thread: mach_msg_port_descriptor_t,
    pub task: mach_msg_port_descriptor_t,
    /* end of the kernel processed data */
    pub NDR: NDR_record_t,
    pub exception: exception_type_t,
    pub codeCnt: mach_msg_type_number_t,
	#[cfg(target_arch = "aarch64")]
    pub code: [i64; 2],
	#[cfg(not(target_arch = "aarch64"))]
	pub code: [i32; 2],
}

@JohnTitor
Copy link
Owner

Friendly-ping @bnjbvr, do you have some time to address the above failure?

@bnjbvr
Copy link
Contributor Author

bnjbvr commented Jun 19, 2022

Yep, I could get back to it but likely not before July. I don't have any x86 mac hardware to use for testing so this makes it a bit frustrating and inefficient to try, but I can play along with CI I guess. In any case, anyone can feel free to take over this PR and finish the remaining bits if they'd like 🙂

@bnjbvr bnjbvr force-pushed the aarch64-support branch from ba19761 to 3b95e79 Compare July 26, 2022 09:01
@bnjbvr
Copy link
Contributor Author

bnjbvr commented Jul 26, 2022

Alright, this passes tests on CI but not on a local aarch64 mac machine; investigating.

@bnjbvr
Copy link
Contributor Author

bnjbvr commented Jul 26, 2022

Tests now passing on my aarch64 darwin machine 🥳

@bnjbvr bnjbvr requested a review from JohnTitor July 26, 2022 10:01
Copy link
Owner

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, thanks!

@JohnTitor JohnTitor merged commit 0437e6d into JohnTitor:main Sep 12, 2022
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