Avoid UB on opaque types.

autocxx generates opaque types of a given length, represented by an array of
bytes. Unless autocxx is configured to use CppRef, references exist to those
types, and as those references pass through C++ they may not obey Rust aliasing
rules. This is the main motivation behind adopting CppRef<T>, but meanwhile, to
reduce such theoretical UB, we mark the data as both MaybeUninit and UnsafeCell
such that Rust makes fewer assumptions about the underlying data.

Per comments, this doesn't eliminate the chance of UB.

Author: adetaylor

engine/src/conversion/codegen_rs/non_pod_struct.rs

@@ -38,6 +38,25 @@ pub(crate) fn make_non_pod(s: &mut ItemStruct, layout: Option<Layout>) {
     // (if we have layout information from bindgen we use that instead)
     // (2) We want to ensure the type is !Unpin
     // (3) We want to ensure it's not Send or Sync
+    // In addition, we want to avoid UB:
+    // (4) By marking the data as MaybeUninit we ensure there's no UB
+    //     by Rust assuming it's initialized
+    // (5) By marking it as UnsafeCell we perhaps help reduce aliasing UB.
+    //     This is on the assumption that references to this type may pass
+    //     through C++ and get duplicated, so there may be multiple Rust
+    //     references to the same underlying data.
+    //     The correct solution to this is to put autocxx into the mode
+    //     where it uses CppRef<T> instead of Rust references, but otherwise,
+    //     using UnsafeCell here may help a bit. It definitely does not
+    //     eliminate the UB here for the following reasons:
+    //     a) The references floating around are to the outer type, not the
+    //        data stored within the UnsafeCell.
+    //     b) C++ may have multiple mutable references, or may have mutable
+    //        references coexisting with immutable references, and no amount
+    //        of UnsafeCell can make that safe.
+    //     Nevertheless the use of UnsafeCell here may (*may*) reduce the
+    //     opportunities for aliasing UB. Again, the only actual way to
+    //     eliminate UB is to use CppRef<T> everywhere instead of &T and &mut T.
     //
     // For opaque types, the Rusty opaque structure could in fact be generated
     // by three different things:
@@ -51,6 +70,13 @@ pub(crate) fn make_non_pod(s: &mut ItemStruct, layout: Option<Layout>) {
     // much more difficult.
     // We use (c) for abstract types. For everything else, we do it ourselves
     // for maximal control. See codegen_rs/mod.rs generate_type for more notes.
+    //
+    // It is worth noting that our constraints here are a bit more severe than
+    // for cxx. In the case of cxx, C++ types are usually represented as
+    // zero-sized types within Rust. Zero-sized types, by definition, can't
+    // have overlapping references and thus can't have aliasing UB. We can't
+    // do that because we want C++ types to be representable on the Rust stack,
+    // and thus we need to tell Rust their real size and alignment.
     // First work out attributes.
     let doc_attr = s
         .attrs
@@ -98,7 +124,7 @@ pub(crate) fn make_non_pod(s: &mut ItemStruct, layout: Option<Layout>) {
         Some(
             syn::Field::parse_named
                 .parse2(quote! {
-                    _data: [u8; #size]
+                    _data: ::core::cell::UnsafeCell<::core::mem::MaybeUninit<[u8; #size]>>
                 })
                 .unwrap(),
         )