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

Heads up about serious bug in Cranelift (Windows) #1179

Closed
hrydgard opened this issue Jan 28, 2020 · 7 comments
Closed

Heads up about serious bug in Cranelift (Windows) #1179

hrydgard opened this issue Jan 28, 2020 · 7 comments
Labels
bug Something isn't working

Comments

@hrydgard
Copy link

Just wanted to point you guys to https://github.com/bytecodealliance/cranelift/issues/1366 .

Cranelift fails to save/restore the XMM registers as the Windows x64 calling convention requires, so the host can suffer corruption of floating point variables (or even other variables if the compiler does certain optimizations).

So if you've ever seen mysterious corruption issues on Windows, this might be it...

@Hywan
Copy link
Contributor

Hywan commented Jan 29, 2020

Thanks for the heads-up!

@hrydgard
Copy link
Author

hrydgard commented Feb 10, 2020

Cranelift maintainers are working on a fix, bytecodealliance/cranelift#1378 (moved to bytecodealliance/wasmtime#1216 )

@hrydgard
Copy link
Author

hrydgard commented Mar 3, 2020

The cranelift codebase has now moved into the wasmtime repo. Not sure what that means, they don't seem to have transferred the pull requests...

@hrydgard
Copy link
Author

hrydgard commented Mar 4, 2020

Actually it seems to be fine.

The PR has now been moved to here: bytecodealliance/wasmtime#1216
Justification is here: bytecodealliance/wasmtime#1185 (comment)

@hrydgard
Copy link
Author

The upstream fix is merged finally! bytecodealliance/wasmtime#1216

@syrusakbary
Copy link
Member

Awesome. We upgraded to latest cranelift in the refactor. We should be able to close this issue once it lands in master

@syrusakbary
Copy link
Member

The refactor has now landed in master, and the cranelift issue should be now solved (since we are using the latest one).

Closing the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants