Stream: git-wasmtime

Topic: wasmtime / PR #7506 iOS target support


view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2023 at 21:54):

jmp-0x7C0 opened PR #7506 from jmp-0x7C0:aarch64-apple-ios to bytecodealliance:main:

As discussed with @alexcrichton on zulip, there appears to be some interest in supporting iOS as a platform. This PR makes the necessary changes for wasmtime to build when targeting aarch64-apple-ios.

A few notes about this PR:

As this is the first time contributing, please let me know if this PR makes sense or would need any changes to align with the contributing guidelines for this project.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2023 at 21:59):

jmp-0x7C0 edited PR #7506:

As discussed with @alexcrichton on zulip, there appears to be some interest in supporting iOS as a platform. This PR makes the necessary changes for wasmtime to build when targeting aarch64-apple-ios.

A few notes about this PR:

As this is the first time contributing, please let me know if this PR makes sense or would need any changes to align with the contributing guidelines for this project.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2023 at 22:11):

jmp-0x7C0 updated PR #7506.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2023 at 22:28):

alexcrichton submitted PR review:

Thanks for this! If you're ok with it though because this is a pretty small patch I'd personally recommend not merging this just yet and moving on to the next steps of trying to get code running (if you're interested in doing those next steps). I'm a bit worried about how there's a number of open questions and I find it easier to remember the questions if they're highlighted in a diff here rather than hunting around a preexisting codebase.

If you'd prefer though I also think it'd be reasonable to land ahead of time.

One suggestion I'd have is that you can probably void updating any tests at this time. I think we're probably still aways out from running tests on iOS so I think it'd be ok to cross that bridge when we get there.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2023 at 22:28):

alexcrichton submitted PR review:

Thanks for this! If you're ok with it though because this is a pretty small patch I'd personally recommend not merging this just yet and moving on to the next steps of trying to get code running (if you're interested in doing those next steps). I'm a bit worried about how there's a number of open questions and I find it easier to remember the questions if they're highlighted in a diff here rather than hunting around a preexisting codebase.

If you'd prefer though I also think it'd be reasonable to land ahead of time.

One suggestion I'd have is that you can probably void updating any tests at this time. I think we're probably still aways out from running tests on iOS so I think it'd be ok to cross that bridge when we get there.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2023 at 22:28):

alexcrichton created PR review comment:

This relates to my comment about pointer authentication above, because if iOS uses the A key instead of the B key then this block would need to change.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2023 at 22:28):

alexcrichton created PR review comment:

This is also something to watch out for once you're at the point you can run some code. This block is true for macOS but iOS may very well be different in its calling convention. I'm not sure either way (or where to look it up), so it's mostly something to watch out for to see if apps crash when you load them.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2023 at 22:28):

alexcrichton created PR review comment:

This is something that I think may need to be confirmed. The comment below for example is incorrect since not all iOS devices will run on Apple Silicon, but I'm sure all recent ones do. That being said I don't know the story for return-address-signing on iOS and whether or not it's different than macOS.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2023 at 22:28):

alexcrichton created PR review comment:

I think it's ok to skip iOS in this file, it's unlikely these tests will ever complete successfully on iOS (and they're disabled by default too)

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2023 at 22:28):

alexcrichton created PR review comment:

(ditto for all other tests/all/debug/*.rs files)

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2023 at 22:28):

alexcrichton created PR review comment:

As a heads up I've posted https://github.com/bytecodealliance/wasmtime/pull/7509 which will conflict with this, although what you're doing here of "where macos is needed also allow ios" would also apply to that PR.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2023 at 22:28):

alexcrichton created PR review comment:

If possible you'll want to use libc::ucontext_t instead of this. This structure is one that has a high risk of being different between iOS and macOS. I posted https://github.com/bytecodealliance/wasmtime/pull/7507 to remove this structure definition entirely as it's not longer necessary (the PR referenced in the above comment has landed and has been published). I think you should be able to use libc::ucontext_t for iOS, but that depends on the libc crate which I can't say with certainty.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 06 2024 at 22:55):

turbolent submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 06 2024 at 22:55):

turbolent created PR review comment:

From what I can see from https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms, it doesn't look like macOS and iOS have different calling conventions on arm64.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 07 2024 at 16:45):

turbolent submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 07 2024 at 16:45):

turbolent created PR review comment:

Looking at https://support.apple.com/en-ng/guide/security/sec8b776536b/1/web/1#sec0167b469d, there does not seem to be any indication there are any differences across iOS and macOS regarding which key is used, it always seems to be IB for return addresses.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 07 2024 at 22:47):

turbolent commented on PR #7506:

@alexcrichton @jmp-0x7C0 I just wanted to report that making similar changes on main, i.e., extending cases of target_os = "macos" with target_os = "ios", let me run the GCD example in an iPhone 15 simulator and on a real iPhone 11 Pro.

Even if some details, like trap and signal handling, might still need to be fully tested, it would be great to allow building wasmtime for iOS upstream. I'm happy to open a PR on top of main

view this post on Zulip Wasmtime GitHub notifications bot (Sep 07 2024 at 22:49):

turbolent edited a comment on PR #7506:

@alexcrichton @jmp-0x7C0 I just wanted to report that making similar changes on main, i.e., extending cases of target_os = "macos" with target_os = "ios", let me run the GCD example in an iPhone 15 simulator and on a real iPhone 11 Pro.

![IMG_6331](https://github.com/user-attachments/assets/f2356f61-e464-40d8-9b6c-50c80038edf7)

Even if some details, like trap and signal handling, might still need to be fully tested, it would be great to allow building wasmtime for iOS upstream. I'm happy to open a PR on top of main

view this post on Zulip Wasmtime GitHub notifications bot (Sep 07 2024 at 22:50):

turbolent edited a comment on PR #7506:

@alexcrichton @jmp-0x7C0 I just wanted to report that making similar changes on main, i.e., extending cases of target_os = "macos" with target_os = "ios", let me run the GCD example in an iPhone 15 simulator and on a real iPhone 11 Pro.

<img src="https://github.com/user-attachments/assets/f2356f61-e464-40d8-9b6c-50c80038edf7" width="300"/>

Even if some details, like trap and signal handling, might still need to be fully tested, it would be great to allow building wasmtime for iOS upstream. I'm happy to open a PR on top of main

view this post on Zulip Wasmtime GitHub notifications bot (Sep 08 2024 at 18:20):

alexcrichton commented on PR #7506:

@turbolent oh that's awesome, thanks for testing! If you'd like I think it would be reasonable to open an PR against main, likely starting from this PR to retain the original authorship and then adding bits and pieces on top as you've tested them.


Last updated: Nov 22 2024 at 16:03 UTC