Wrong ABI used for small C++ classes on Linux.

[vadimcn]

Input C/C++ Header

#include <cstdint>

struct Foo {
    size_t data;
};

struct Bar {
    size_t data;
    ~Bar() { data = 0; }
};

Foo MakeFoo(); // { return Foo(); }

Bar MakeBar(); // { return Bar(); }

Bindgen Invocation

bindgen::Builder::default()
    .clang_args(&["-x","c++", "-std=c++11"])
    .header("input.h")
    .generate()
    .unwrap()

Actual Results

#[repr(C)]
#[derive(Debug, Copy)]
pub struct Foo {
    pub data: usize,
}
impl Clone for Foo {
    fn clone(&self) -> Self { *self }
}
#[repr(C)]
#[derive(Debug)]
pub struct Bar {
    pub data: usize,
}
extern "C" {
    #[link_name = "_ZN3BarD1Ev"]
    pub fn Bar_Bar_destructor(this: *mut Bar);
}
impl Bar {
    #[inline]
    pub unsafe fn destruct(&mut self) { Bar_Bar_destructor(self) }
}
extern "C" {
    #[link_name = "_Z7MakeFoov"]
    pub fn MakeFoo() -> Foo;
}
extern "C" {
    #[link_name = "_Z7MakeBarv"]
    pub fn MakeBar() -> Bar;
}

Expected Results

Not sure...

Discussion

MakeFoo and MakeBar look very similar, however in C++ they use different ABIs, at least on Linux.
Here's a quote from System V AMD64 ABI spec (page 20):

If a C++ object has either a non-trivial copy constructor or a non-trivial
destructor ¹¹, it is passed by invisible reference (the object is replaced in the
parameter list by a pointer that has class POINTER) ¹²

This means that while Foo get returned by-value in the rax register, Bar must be returned by-pointer to a caller allocated memory. To Rust compiler, however, these look identical, so it expects both to be returned in rax.

[emilio]

Yeah, this is somewhat annoying, but passing non-trivial arguments by value needs rustc support, and last time I checked with them they really weren't interested in implementing some kind of extern "C++".

[emilio]

(The same happens for arguments, btw, we've run into this before, see https://bugzilla.mozilla.org/show_bug.cgi?id=1366247)

[vadimcn]

In my specific case, the struct was requested to be opaque, so maybe bindgen could use some other "filler" type? Looking at cabi_x86_64.rs, it seems that some kind on weird enum (one that ends up with StructWrappedNullablePointer layout) might do the trick.

Or, maybe there should be a sub-option of #[repr(C)] to specify this explicitly? E.g. #[repr(C, by_pointer)].

Overall, I think it would be preferable to emit an error rather than generate bindings that are known produce incorrect code.

[vadimcn]

Looks like this does the trick (i.e. some struct member must be misaligned):

#[repr(C, packed)]
struct Bar {
    data1: u8,
    data2: u16,
    data3: [u8; 5],
}

Would only be useful for opaque structs, of course...

[vadimcn]

On second thought, this

An object with either a non-trivial copy constructor or a non-trivial destructor cannot be
passed by value because such objects must have well defined addresses. Similar issues apply
when returning an object from a function.

is pretty much the distinction between Copy and !Copy types. Perhaps the rule should be that all non-Copy types are passed by-pointer?

[comex]

Huh, seems very similar to an MSVC ABI issue. And here I thought the madness of changing ABI based on the presence of methods was Windows-specific...

[retep998]

Perhaps the rule should be that all non-Copy types are passed by-pointer?

Please no. This would wreak so much havoc when ABIs mysteriously change due to the addition or removal of Copy impls. I'd much rather this be explicitly handled via a new repr to specify that a type is not POD.

[vadimcn]

As I've said in IRLO thread, I think there are two ways we can go here:

• Add support for "significant-address" c++ types in rustc via an attribute.
• Have bindgen change such arguments/return values to be explicitly by-pointer. Of course, I may turn out that different targets require slightly different transforms (for example, I think some targets require function to return address of memory location of the return value, while others don't), but I guess bindgen is already in that boat because it needs to know about vtable layout and mangling.

@emilio, do you have an opinion on the best way to proceed?

[retep998]

Right now the instant solution which works on stable is to transform the bindings to pass certain types by pointer and handle certain return values correctly. In the future Rust definitely should add more calling conventions and attributes however so that Rust can perform these transformations internally.

For example in windows API there is a COM method that looks like this in C++:

D3D12_HEAP_PROPERTIES GetCustomHeapProperties(
    UINT nodeMask,
    D3D12_HEAP_TYPE heapType
);

In order to get the ABI correct I have to make it look like this in Rust. Note that this already works in stable Rust, so it is a solution people can implement immediately in their projects.

fn GetCustomHeapProperties(
    nodeMask: UINT,
    heapType: D3D12_HEAP_TYPE,
    __ret_val: *mut D3D12_HEAP_PROPERTIES,
) -> *mut D3D12_HEAP_PROPERTIES

However, in the future I'd really love for Rust to have calling conventions for C++ methods as well as attributes to indicate that a type is not POD, so in some future version I could simplify the signature down to:

fn GetCustomHeapProperties(
    nodeMask: UINT,
    heapType: D3D12_HEAP_TYPE,
) -> D3D12_HEAP_PROPERTIES

So basically, do the bindgen transform regardless because it is needed for correctness now while in the future Rust really should add attributes and calling conventions to facilitate this and bindgen can be updated later to take advantage of those new language features if and when they do get implemented and stabilized.

[vadimcn]

@retep998: It seems to me that you are describing a different problem. Is there already an open issue for it? Let's take discussion there.

[retep998]

@vadimcn The two Rust issues are:

1. Types which are not POD are sometimes handled differently in function ABIs and need a way to mark them as such. issues with ABIs and struct return types rust#38258
2. C++ methods on windows have a slightly different calling convention than C and C++ functions and need a way to mark them as such. Need calling convention for stdcall methods (different from functions!) rfcs#1342

While this issue here is specifically for bindgen and what it should do about these functions whose ABI is changed slightly by things that Rust is currently incapable of representing.

[vadimcn]

bindgen could also fix up function signatures if it generated a small wrapper function. For example:

extern "C" { 
    #[link_name = "..."]
    fn raw_ClassMethod(this: *mut Class, ret_value: *mut SomeStruct) -> *mut SomeStruct; 
}

unsafe fn ClassMethod(this: *mut Class) -> SomeStruct {
    let s: SomeStruct = mem::uninitialized();
    raw_ClassMethod(this, &s);
    s
}

[emilio]

That'd yield also similar problems as #607, though.