cxx icon indicating copy to clipboard operation
cxx copied to clipboard

Generate default constructors for shared types

Open edsrzf opened this issue 4 years ago • 3 comments

CXX does not emit a default constructor for shared types, which means that the C++ struct's member variables are uninitialized after construction. This makes behavior on the C++ side less deterministic and safe.

As a solution, I suggest generating a default constructor that default-initializes all member variables using an initialization list. This effectively zero-initializes everything.

It would be nice to eventually have finer-grained control over the constructor generation (for example, if I want a member initialized to something other than zero), but zero-initializing everything seems strictly better than leaving things uninitialized.

Example

I have a simple struct I'm trying to move from C++ to Rust. I define it like this:

#[cxx::bridge]
mod ffi {
    pub struct Point {
        pub x: i32,
        pub y: i32,
    }
}

CXX generates this C++ struct:

struct Point final {
  ::std::int32_t x;
  ::std::int32_t y;

  using IsRelocatable = ::std::true_type;
};

CXX does not define a default constructor for this struct, so it has an implicitly-defined one which does not initialize x or y when called. This means that when C++ code writes:

Point p;

...then p.x and p.y are uninitialized.

It also happens that in my case, the original C++ struct did provide a default constructor that zero-initialized x and y. Now I have many places in the code that are using uninitialized memory. The code base is fairly large and it would be very difficult to try to search out every single place that calls the default constructor for this struct.

With my proposed solution, CXX would generate:


struct Point final {
  ::std::int32_t x;
  ::std::int32_t y;

  Point(): x(), y() {}

  using IsRelocatable = ::std::true_type;
};

edsrzf avatar Feb 14 '21 18:02 edsrzf

After trying to implement this, I've run into a couple challenges:

First, it doesn't work when a member variable has a deleted default constructor, such as rust::Box. Maybe we can generate a constructor only when trivial types (i32, bool, f64, etc.) are involved?

Second, and more critically, adding a constructor causes Clang (haven't tried GCC) to emit -Wreturn-type-c-linkage warnings on extern "C" functions. I don't know what the actual risks are, since the struct is still standard layout (std::is_standard_layout<Point>() returns true), but it doesn't feel great to trigger that warning.

edsrzf avatar Feb 14 '21 20:02 edsrzf

After exploring my options a bit more I think in my case it makes more sense to use a handwritten ExternType impl as described here.

I'll leave this issue open for discussion though as I do feel like there's something missing in this area, whether it's a generated default constructor or some more general way of generating constructors for shared types.

edsrzf avatar Feb 15 '21 00:02 edsrzf

I don't know what the actual risks are, since the struct is still standard layout

It mostly seems to be that while UDTs with constructors are still StandardLayoutType, the constructor makes them not a TrivialType. The x64 calling convention places small enough TrivialType returns into registers, but any type which is not TrivialType, no matter the size, is never returned in a register.

C has no concept of types which are not TrivialType, thus you get a mismatch in linkage when you return a type which is not a TrivialType.

As such, in order to have an FFI-shared type with any user-defined constructor, you either need to create a subclass and add the constructor there, or otherwise marshal the type through a TrivialType or outptr on the FFI boundary. Thankfully, custom cxx::kind::Trivial ExternType do use pass-by-pointer, avoiding this issue.

However, this would suggest and interesting option for StandardLayout types. If a type is StandardLayout and cxx::kind::Trivial, then "all" you need to do in order to actually pass it by-value over FFI is accurately define a bit-equivalent TrivialType, then reinterpret the bits as necessary.

References:

  • https://stackoverflow.com/a/57436920/3019990
  • https://stackoverflow.com/a/23666579/3019990

CAD97 avatar May 09 '21 04:05 CAD97