Skip to content

WebAssembly calling convention impl incorrectly uses make_indirect_byval() #130442

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
bjorn3 opened this issue Sep 16, 2024 · 2 comments · Fixed by #130450
Closed

WebAssembly calling convention impl incorrectly uses make_indirect_byval() #130442

bjorn3 opened this issue Sep 16, 2024 · 2 comments · Fixed by #130450
Labels
A-ABI Area: Concerning the application binary interface (ABI) C-bug Category: This is a bug. O-wasm Target: WASM (WebAssembly), http://webassembly.org/ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@bjorn3
Copy link
Member

bjorn3 commented Sep 16, 2024

It should use make_indirect() instead. It seems like LLVM ignores sarg on WebAssembly, so it happens to work out fine in the end with the LLVM backend at least, but it is still incorrect per https://github.com/WebAssembly/tool-conventions/blob/main/BasicCABI.md.

For those unaware the difference between the two is that make_indirect() will pass a pointer to the argument somewhere, while make_indirect_byval() is supposed to store the argument at a fixed location on the stack.

@bjorn3 bjorn3 added O-wasm Target: WASM (WebAssembly), http://webassembly.org/ C-bug Category: This is a bug. A-ABI Area: Concerning the application binary interface (ABI) labels Sep 16, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 16, 2024
@workingjubilee
Copy link
Member

For those unaware the difference between the two is that make_indirect() will pass a pointer to the argument somewhere, while make_indirect_byval() is supposed to store the argument at a fixed location on the stack.

So one is pass_by_pointer and the one is pass_by_stack_location?

@bjorn3
Copy link
Member Author

bjorn3 commented Sep 16, 2024

Yes. The former sets the pass mode to PassMode::Indirect { on_stack: false, ... } while the latter sets it to PassMode::Indirect { on_stack: true, ... }. I believe the on_stack field used to be called byval too, but I changed it a couple of years ago as byval is pretty confusing.

@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Sep 17, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-ABI Area: Concerning the application binary interface (ABI) C-bug Category: This is a bug. O-wasm Target: WASM (WebAssembly), http://webassembly.org/ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants