JohnTitor / mach2

Apache License 2.0
32 stars 12 forks source link

Add support for exceptions ports + aarch64 #8

Closed bnjbvr closed 1 year ago

bnjbvr commented 2 years ago

This adds enough support for https://github.com/bytecodealliance/wasmtime/pull/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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

bnjbvr commented 2 years ago

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 :slightly_smiling_face:

bnjbvr commented 2 years ago

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

bnjbvr commented 2 years ago

Tests now passing on my aarch64 darwin machine :partying_face: