Repository: dtolnay/cxx (language: rust)

Issue:
C++ MaybeUninit bypasses class-specific member operator new
For an input like this:

```rust
mod bindgen {
    use cxx::{type_id, ExternType};

    #[repr(C)]
    pub struct Type {...}

    unsafe impl ExternType for Type {
        type Id = type_id!("Type");
        type Kind = cxx::kind::Trivial;
    }
}

#[cxx::bridge]
mod ffi {
    extern "C++" {
        type Type = crate::bindgen::Type;
    }
    impl UniquePtr<Type> {}
}
```

the generated C++ side of the UniquePtr impl currently contains something like the following as the implementation backing UniquePtr::new and UniquePtr::drop:

```cpp
template <typename T>
union MaybeUninit {
  T value;
  MaybeUninit() {}
  ~MaybeUninit() {}
};

::Type *cxxbridge1$unique_ptr$Type$uninit(::std::unique_ptr<::Type> *ptr) noexcept {
  ::Type *uninit = reinterpret_cast<::Type *>(new ::rust::MaybeUninit<::Type>);
  new (ptr) ::std::unique_ptr<::Type>(uninit);
  return uninit;
}
void cxxbridge1$unique_ptr$Type$drop(::std::unique_ptr<::Type> *ptr) noexcept {
  ptr->~unique_ptr();
}
```

This is Bad because in the case that ::Type has a class-specific member `operator new` and `operator delete`, we'd allocate without using its `operator new` and delete *with* its `operator delete`, which is UB.

Relevant to e.g. `std::unique_ptr<folly::IOBuf>`: https://github.com/facebook/folly/blob/cd39f451c749ff502cf5e062ab094762d1806f00/folly/io/IOBuf.h#L1314-L1323

Fix is to SFINAE detect a class-specific member `operator new` on MaybeUninit's `T` and apply that during the MaybeUninit allocation.

```cpp
template <typename T, typename = void *>
struct operator_new {
  void *operator()(size_t size) { return ::operator new(size); }
};

template <typename T>
struct operator_new<T, decltype(T::operator new(sizeof(T)))> {
  void *operator()(size_t size) { return T::operator new(size); }
};

template <typename T>
union MaybeUninit {
  T value;
  void *operator new(size_t size) { return operator_new<T>{}(size); }
  MaybeUninit() {}
  ~MaybeUninit() {}
};
```

Demo showing the correct in-place construction and destruction sequence:

```cpp
struct Type {
  Type() { std::cout << "Type\n"; }
  ~Type() { std::cout << "~Type\n"; }
  void *operator new(size_t sz) {
    std::cout << "new(size_t)\n";
    return ::operator new(sz);
  }
  void *operator new(size_t, void *p) {
    std::cout << "new(size_t, void *)\n";
    return p;
  }
  void operator delete(void *p) {
    std::cout << "delete(void *)\n";
    ::operator delete(p);
  }
  void operator delete(void *, void *) {
    std::cout << "delete(void *, void *)\n";
  }
};

int main() {
  union place {
    place() {}
    ~place() {}
    std::unique_ptr<Type> type;
  } place;

  Type *uninit = reinterpret_cast<Type *>(new MaybeUninit<Type>);
  ::new (&place.type) std::unique_ptr<Type>(uninit);
  ::new (&*place.type) Type;

  place.type.~unique_ptr();
}
```

```console
new(size_t)
Type
~Type
delete(void *)
```


Discussion / hints:

Fix patch:
diff --git a/gen/src/builtin.rs b/gen/src/builtin.rs
index 62c352717..be70a7126 100644
--- a/gen/src/builtin.rs
+++ b/gen/src/builtin.rs
@@ -134,6 +134,34 @@ pub(super) fn write(out: &mut OutFile) {
     ifndef::write(out, builtin.opaque, "CXXBRIDGE1_RUST_OPAQUE");
     ifndef::write(out, builtin.relocatable, "CXXBRIDGE1_RELOCATABLE");
 
+    out.begin_block(Block::Namespace("detail"));
+
+    if builtin.maybe_uninit {
+        include.cstddef = true;
+        include.new = true;
+        out.next_section();
+        writeln!(out, "template <typename T, typename = void *>");
+        writeln!(out, "struct operator_new {{");
+        writeln!(
+            out,
+            "  void *operator()(size_t sz) {{ return ::operator new(sz); }}",
+        );
+        writeln!(out, "}};");
+        out.next_section();
+        writeln!(out, "template <typename T>");
+        writeln!(
+            out,
+            "struct operator_new<T, decltype(T::operator new(sizeof(T)))> {{",
+        );
+        writeln!(
+            out,
+            "  void *operator()(size_t sz) {{ return T::operator new(sz); }}",
+        );
+        writeln!(out, "}};");
+    }
+
+    out.end_block(Block::Namespace("detail"));
+
     if builtin.manually_drop {
         out.next_section();
         include.utility = true;
@@ -149,10 +177,15 @@ pub(super) fn write(out: &mut OutFile) {
     }
 
     if builtin.maybe_uninit {
+        include.cstddef = true;
         out.next_section();
         writeln!(out, "template <typename T>");
         writeln!(out, "union MaybeUninit {{");
         writeln!(out, "  T value;");
+        writeln!(
+            out,
+            "  void *operator new(size_t sz) {{ return detail::operator_new<T>{{}}(sz); }}",
+        );
         writeln!(out, "  MaybeUninit() {{}}");
         writeln!(out, "  ~MaybeUninit() {{}}");
         writeln!(out, "}};");
diff --git a/gen/src/write.rs b/gen/src/write.rs
index 03ebeb5ad..851b95373 100644
--- a/gen/src/write.rs
+++ b/gen/src/write.rs
@@ -1540,7 +1540,7 @@ fn write_unique_ptr_common(out: &mut OutFile, ty: UniquePtr) {
         "void cxxbridge1$unique_ptr${}$null(::std::unique_ptr<{}> *ptr) noexcept {{",
         instance, inner,
     );
-    writeln!(out, "  new (ptr) ::std::unique_ptr<{}>();", inner);
+    writeln!(out, "  ::new (ptr) ::std::unique_ptr<{}>();", inner);
     writeln!(out, "}}");
     if can_construct_from_value {
         out.builtin.maybe_uninit = true;
@@ -1554,7 +1554,7 @@ fn write_unique_ptr_common(out: &mut OutFile, ty: UniquePtr) {
             "  {} *uninit = reinterpret_cast<{} *>(new ::rust::MaybeUninit<{}>);",
             inner, inner, inner,
         );
-        writeln!(out, "  new (ptr) ::std::unique_ptr<{}>(uninit);", inner);
+        writeln!(out, "  ::new (ptr) ::std::unique_ptr<{}>(uninit);", inner);
         writeln!(out, "  return uninit;");
         writeln!(out, "}}");
     }
@@ -1563,7 +1563,7 @@ fn write_unique_ptr_common(out: &mut OutFile, ty: UniquePtr) {
         "void cxxbridge1$unique_ptr${}$raw(::std::unique_ptr<{}> *ptr, {} *raw) noexcept {{",
         instance, inner, inner,
     );
-    writeln!(out, "  new (ptr) ::std::unique_ptr<{}>(raw);", inner);
+    writeln!(out, "  ::new (ptr) ::std::unique_ptr<{}>(raw);", inner);
     writeln!(out, "}}");
     writeln!(
         out,
@@ -1630,7 +1630,7 @@ fn write_shared_ptr(out: &mut OutFile, ident: &RustName) {
         "void cxxbridge1$shared_ptr${}$null(::std::shared_ptr<{}> *ptr) noexcept {{",
         instance, inner,
     );
-    writeln!(out, "  new (ptr) ::std::shared_ptr<{}>();", inner);
+    writeln!(out, "  ::new (ptr) ::std::shared_ptr<{}>();", inner);
     writeln!(out, "}}");
     if can_construct_from_value {
         out.builtin.maybe_uninit = true;
@@ -1644,7 +1644,7 @@ fn write_shared_ptr(out: &mut OutFile, ident: &RustName) {
             "  {} *uninit = reinterpret_cast<{} *>(new ::rust::MaybeUninit<{}>);",
             inner, inner, inner,
         );
-        writeln!(out, "  new (ptr) ::std::shared_ptr<{}>(uninit);", inner);
+        writeln!(out, "  ::new (ptr) ::std::shared_ptr<{}>(uninit);", inner);
         writeln!(out, "  return uninit;");
         writeln!(out, "}}");
     }
@@ -1653,7 +1653,7 @@ fn write_shared_ptr(out: &mut OutFile, ident: &RustName) {
         "void cxxbridge1$shared_ptr${}$clone(const ::std::shared_ptr<{}>& self, ::std::shared_ptr<{}> *ptr) noexcept {{",
         instance, inner, inner,
     );
-    writeln!(out, "  new (ptr) ::std::shared_ptr<{}>(self);", inner);
+    writeln!(out, "  ::new (ptr) ::std::shared_ptr<{}>(self);", inner);
     writeln!(out, "}}");
     writeln!(
         out,


Test patch: