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

issues with ABIs and struct return types #38258

Open
vvuk opened this issue Dec 9, 2016 · 6 comments
Open

issues with ABIs and struct return types #38258

vvuk opened this issue Dec 9, 2016 · 6 comments
Labels
A-FFI Area: Foreign function interface (FFI) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@vvuk
Copy link
Contributor

vvuk commented Dec 9, 2016

With the MSVC x64 ABI, structs are returned in RAX if and only if they are <= 8 bytes in size, and are effectively a POD type. See https://msdn.microsoft.com/en-us/library/7572ztz4.aspx . Unfortunately, I can't figure out how to tell Rust this. In C++, sk_sp<T> has a user-defined constructor, destructor, etc. In Rust, I couldn't figure out any way to do this. It's defined as..

    #[repr(C)]
    #[derive(Debug, Copy, Clone)]
    pub struct sk_sp<T> {
        pub ptr: *mut T,
    }

This causes segfaults because the C++ calling convention isn't upheld. A temporary workaround is to just add another dummy field, making this type bigger than 8 bytes. This happens to work in this case since this type is only ever used as a smart pointer return type; params are passed as basic T*.

Is there any way to tell rustc to treat this type as non-POD when giving it to LLVM? If not, can such a mechanism be added?

@retep998
Copy link
Member

retep998 commented Dec 9, 2016

Actually, this difference extends to 32bit as well. C++ methods have that slight difference in how they handle return types compared to C/C++ functions. A case where I encountered this issue was actually in binding COM interfaces. 99% of the time they'd just be returning HRESULT which is the same for both methods and functions. However every once in a while a COM method would return an aggregate type that is affected by this slight difference and Rust would have the wrong calling convention. Hell, even C compiled by cl.exe using the official C bindings to the COM interface gets it wrong, so this issue is so subtle that not even Microsoft has done anything about it.

Related issue rust-lang/rfcs#1342

@Mark-Simulacrum Mark-Simulacrum added the O-windows-msvc Toolchain: MSVC, Operating system: Windows label May 18, 2017
@steveklabnik
Copy link
Member

steveklabnik commented Jun 9, 2017

Someone ran into this today https://users.rust-lang.org/t/returning-values-from-rust-to-c/11256

EDIT: actually reading more, maybe not...

@comex
Copy link
Contributor

comex commented Jun 10, 2017

@steveklabnik (Turns out that was indeed this issue: the user added a constructor on the C++ side, making it non-POD, and didn't mention it originally.)

@retep998
Copy link
Member

In order to fully solve this we would need two things:

  1. Calling conventions for C++ methods rather than functions. The difference in how they handle struct returns is absolutely crucial for code to get right.
  2. A new repr for structs to indicate that they are not POD.

@emilio
Copy link
Contributor

emilio commented Jun 24, 2017

This is also an issue for Linux btw, see rust-lang/rust-bindgen#778.

@Mark-Simulacrum Mark-Simulacrum added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed O-windows-msvc Toolchain: MSVC, Operating system: Windows labels Jun 24, 2017
@Mark-Simulacrum Mark-Simulacrum changed the title issues with MSVC ABI and struct return types issues with ABIs and struct return types Jun 24, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 26, 2017
sdao added a commit to sdao/rust-bindgen that referenced this issue Jan 21, 2018
… Rust compiler supports it natively

Instance methods like `ReturnType myMethod(T arg1, U arg2)` where ReturnType is non trivial are passed in MSVC as:
rcx - this
rdx - address of return value
r8, r9 - arg1, arg2

But the extern "C" abi passes arguments to a function `ReturnType myFunction(ThisType* this, T arg1, U arg2)` as follows:
rcx - address of return value
rdx - this
r8, r9 - arg1, arg2

The MSVC convention for x64 is similar to x86 thiscall. Thus, we can rewrite the methods returning non-trivial values to be functions returning void, passing the return value via the second argument (after the this pointer). This will enable them to be called correctly with the new signature using extern "C" abi.

See <rust-lang/rust#38258>
@Enselic Enselic added the A-FFI Area: Foreign function interface (FFI) label Nov 18, 2023
@Ralith
Copy link
Contributor

Ralith commented Apr 18, 2024

A new repr for structs to indicate that they are not POD.

The notion of "POD" here is idiosyncratic to the MSVC ABI (e.g. it does not match the C++ notion of "standard-layout"), so care should be taking with naming such a repr to not cause further confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-FFI Area: Foreign function interface (FFI) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants