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.

Sign up for GitHub

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

Jump to bottom

Fix wasm pointer misalignment issues on Apple silicon #1778

Merged
merged 4 commits into from
Sep 6, 2023
Merged

Fix wasm pointer misalignment issues on Apple silicon #1778

merged 4 commits into from
Sep 6, 2023

Conversation

sug0
Copy link
Contributor

@sug0 sug0 commented Jul 30, 2023 •
edited
Loading

Switch to a wasmer revision based on 2.3.0 that fixes pointer misalignment issues on Apple silicon

Describe your changes

Fixes wasmerio/wasmer#4072

Indicate on which release or other PRs this topic is based on

v0.21.1

Checklist before merging to draft

  • I have added a changelog
  • Git history is in acceptable state

@sug0 sug0 requested review from tzemanovic and yito88 July 30, 2023 01:26
@sug0
Copy link
Contributor Author

sug0 commented Jul 30, 2023

The stack limiter tests are still failing locally, but not because of pointer misalignment issues. It complains about hitting the stack limit... 🤔 At least signals are correctly caught, now.

@sug0 sug0 added bug Something isn't working wasm labels Jul 30, 2023
@sug0 sug0 requested a review from brentstone July 30, 2023 10:33
@sug0 sug0 force-pushed the tiago/fix-wasm-vm-aarch64-mac branch from 81cdb58 to ab73d6c Compare July 31, 2023 10:10
@sug0
Copy link
Contributor Author

sug0 commented Jul 31, 2023 •
edited
Loading

Fixed the stack limiter unit tests on Apple silicon by reducing the number of loops before tripping the limiter. For some reason, we have ~200 bytes less stack space at our disposal. 😬

sug0 added a commit that referenced this pull request Jul 31, 2023
@sug0
Add changelog for #1778
3aa2eac
@sug0
Copy link
Contributor Author

sug0 commented Jul 31, 2023

CI failed to look-up my custom revision. The url on cargo probably needs to point to https://github.com/heliaxdev/wasmer, but I don't have write access on that repository.

@tzemanovic
Copy link
Member

CI failed to look-up my custom revision. The url on cargo probably needs to point to https://github.com/heliaxdev/wasmer, but I don't have write access on that repository.

Sry, should be fixed now

@sug0
Copy link
Contributor Author

sug0 commented Jul 31, 2023

@tzemanovic I'm still not entirely sure if the changes introduced by this PR are correct, given the issues with the stack limiter unit tests. Regardless, it's probably better to have something that works vs. something that is broken. lmk what you think

sug0 added a commit that referenced this pull request Jul 31, 2023
@sug0
Add changelog for #1778
5c16c97
@sug0 sug0 force-pushed the tiago/fix-wasm-vm-aarch64-mac branch from 3aa2eac to 5c16c97 Compare July 31, 2023 14:36
sug0 added a commit that referenced this pull request Jul 31, 2023
@sug0
Add changelog for #1778
1f4387b
@sug0 sug0 force-pushed the tiago/fix-wasm-vm-aarch64-mac branch from 5c16c97 to 1f4387b Compare July 31, 2023 14:38
@sug0 sug0 marked this pull request as draft August 29, 2023 13:42
sug0 added a commit that referenced this pull request Aug 29, 2023
@sug0
Add changelog for #1778
c430d37
@sug0 sug0 force-pushed the tiago/fix-wasm-vm-aarch64-mac branch from 1f4387b to c430d37 Compare August 29, 2023 14:58
@sug0 sug0 marked this pull request as ready for review August 29, 2023 14:58
tzemanovic
tzemanovic previously approved these changes Aug 30, 2023
View reviewed changes
sug0 and others added 2 commits August 30, 2023 13:21
@sug0 sug0 force-pushed the tiago/fix-wasm-vm-aarch64-mac branch from c430d37 to 9f0f946 Compare August 30, 2023 12:21
sug0 added a commit that referenced this pull request Aug 30, 2023
@sug0
Merge branch 'tiago/fix-wasm-vm-aarch64-mac' (#1778)
cce8315
@sug0 sug0 mentioned this pull request Aug 30, 2023
Closed
Fraccaman added a commit that referenced this pull request Sep 6, 2023
@Fraccaman
Merge branch 'tiago/fix-wasm-vm-aarch64-mac' (#1778)
e0cb295 
* origin/tiago/fix-wasm-vm-aarch64-mac:
  Add changelog for #1778
  Document reason for disabling wasm stack limiter tests
  Temporarily disable the stack limiter tests on Apple silicon
  Replace wasmer with a heliaxdev fork of v2.3.0
Fraccaman added a commit that referenced this pull request Sep 6, 2023
@Fraccaman
Merge branch 'tiago/fix-wasm-vm-aarch64-mac' (#1778)
c918929 
* origin/tiago/fix-wasm-vm-aarch64-mac:
  Add changelog for #1778
  Document reason for disabling wasm stack limiter tests
  Temporarily disable the stack limiter tests on Apple silicon
  Replace wasmer with a heliaxdev fork of v2.3.0
Fraccaman added a commit that referenced this pull request Sep 6, 2023
@Fraccaman
Merge branch 'tiago/fix-wasm-vm-aarch64-mac' (#1778)
bcdc400 
* origin/tiago/fix-wasm-vm-aarch64-mac:
  Add changelog for #1778
  Document reason for disabling wasm stack limiter tests
  Temporarily disable the stack limiter tests on Apple silicon
  Replace wasmer with a heliaxdev fork of v2.3.0
@Fraccaman Fraccaman merged commit 08f07ac into main Sep 6, 2023
12 checks passed
@Fraccaman Fraccaman deleted the tiago/fix-wasm-vm-aarch64-mac branch September 6, 2023 21:03
@tzemanovic tzemanovic mentioned this pull request Sep 13, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tzemanovic tzemanovic tzemanovic left review comments

@yito88 yito88 Awaiting requested review from yito88

@brentstone brentstone Awaiting requested review from brentstone

Assignees
No one assigned
Labels
bug Something isn't working wasm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Misaligned pointer dereference on Mac M1 Pro & Mac M2 Pro
3 participants
@sug0 @tzemanovic @Fraccaman