Repository: dtolnay/cxx (language: rust)

Issue:
Some Box constructors are not exception safe
The copy constructor leaks an allocation if T's copy constructor throws.

https://github.com/dtolnay/cxx/blob/faf22e81122d5813a966a589a3b30ea683e7308d/include/cxx.h#L556-L560

Similarly, the move construct leaks an allocation if T's move constructor throws.

https://github.com/dtolnay/cxx/blob/faf22e81122d5813a966a589a3b30ea683e7308d/include/cxx.h#L562-L566

And the in_place factory function causes a Rust Drop impl to run on an uninitialized T if T's constructor throws.

https://github.com/dtolnay/cxx/blob/faf22e81122d5813a966a589a3b30ea683e7308d/include/cxx.h#L618-L625


Discussion / hints:

Fix patch:
diff --git a/gen/src/write.rs b/gen/src/write.rs
index fc0872fe2..44978f1bf 100644
--- a/gen/src/write.rs
+++ b/gen/src/write.rs
@@ -1351,6 +1351,11 @@ fn write_rust_box_extern(out: &mut OutFile, ident: &Pair) {
         "{} *cxxbridge1$box${}$alloc() noexcept;",
         inner, instance,
     );
+    writeln!(
+        out,
+        "void cxxbridge1$box${}$dealloc({} *) noexcept;",
+        instance, inner,
+    );
     writeln!(
         out,
         "void cxxbridge1$box${}$drop(::rust::Box<{}> *ptr) noexcept;",
@@ -1415,10 +1420,23 @@ fn write_rust_box_impl(out: &mut OutFile, ident: &Pair) {
     let instance = ident.to_symbol();
 
     writeln!(out, "template <>");
-    writeln!(out, "{} *Box<{}>::alloc() noexcept {{", inner, inner);
+    writeln!(
+        out,
+        "{} *Box<{}>::allocation::alloc() noexcept {{",
+        inner, inner,
+    );
     writeln!(out, "  return cxxbridge1$box${}$alloc();", instance);
     writeln!(out, "}}");
 
+    writeln!(out, "template <>");
+    writeln!(
+        out,
+        "void Box<{}>::allocation::dealloc({} *ptr) noexcept {{",
+        inner, inner,
+    );
+    writeln!(out, "  cxxbridge1$box${}$dealloc(ptr);", instance);
+    writeln!(out, "}}");
+
     writeln!(out, "template <>");
     writeln!(out, "void Box<{}>::drop() noexcept {{", inner);
     writeln!(out, "  cxxbridge1$box${}$drop(this);", instance);
diff --git a/include/cxx.h b/include/cxx.h
index 34b8771aa..0a9df91a7 100644
--- a/include/cxx.h
+++ b/include/cxx.h
@@ -227,8 +227,8 @@ class Box final {
 
 private:
   class uninit;
+  class allocation;
   Box(uninit) noexcept;
-  static T *alloc() noexcept;
   void drop() noexcept;
   T *ptr;
 };
@@ -550,6 +550,20 @@ typename Slice<T>::iterator Slice<T>::end() const noexcept {
 template <typename T>
 class Box<T>::uninit {};
 
+template <typename T>
+class Box<T>::allocation {
+  static T *alloc() noexcept;
+  static void dealloc(T *) noexcept;
+public:
+  allocation() noexcept : ptr(alloc()) {}
+  ~allocation() noexcept {
+    if (this->ptr) {
+      dealloc(this->ptr);
+    }
+  }
+  T *ptr;
+};
+
 template <typename T>
 Box<T>::Box(const Box &other) : Box(*other) {}
 
@@ -559,13 +573,19 @@ Box<T>::Box(Box &&other) noexcept : ptr(other.ptr) {
 }
 
 template <typename T>
-Box<T>::Box(const T &val) : ptr(alloc()) {
-  ::new (this->ptr) T(val);
+Box<T>::Box(const T &val) {
+  allocation alloc;
+  ::new (alloc.ptr) T(val);
+  this->ptr = alloc.ptr;
+  alloc.ptr = nullptr;
 }
 
 template <typename T>
-Box<T>::Box(T &&val) : ptr(alloc()) {
-  ::new (this->ptr) T(std::move(val));
+Box<T>::Box(T &&val) {
+  allocation alloc;
+  ::new (alloc.ptr) T(std::move(val));
+  this->ptr = alloc.ptr;
+  alloc.ptr = nullptr;
 }
 
 template <typename T>
@@ -581,8 +601,10 @@ Box<T> &Box<T>::operator=(const Box &other) {
     if (this->ptr) {
       **this = *other;
     } else {
-      this->ptr = alloc();
-      ::new (this->ptr) T(*other);
+      allocation alloc;
+      ::new (alloc.ptr) T(*other);
+      this->ptr = alloc.ptr;
+      alloc.ptr = nullptr;
     }
   }
   return *this;
@@ -621,7 +643,11 @@ T &Box<T>::operator*() noexcept {
 template <typename T>
 template <typename... Fields>
 Box<T> Box<T>::in_place(Fields &&... fields) {
-  return from_raw(::new (alloc()) T{std::forward<Fields>(fields)...});
+  allocation alloc;
+  auto ptr = alloc.ptr;
+  ::new (ptr) T{std::forward<Fields>(fields)...};
+  alloc.ptr = nullptr;
+  return from_raw(ptr);
 }
 
 template <typename T>
diff --git a/macro/src/expand.rs b/macro/src/expand.rs
index 4a17161d9..433eca623 100644
--- a/macro/src/expand.rs
+++ b/macro/src/expand.rs
@@ -952,10 +952,12 @@ fn type_id(name: &Pair) -> TokenStream {
 fn expand_rust_box(ident: &RustName, types: &Types) -> TokenStream {
     let link_prefix = format!("cxxbridge1$box${}$", types.resolve(ident).to_symbol());
     let link_alloc = format!("{}alloc", link_prefix);
+    let link_dealloc = format!("{}dealloc", link_prefix);
     let link_drop = format!("{}drop", link_prefix);
 
     let local_prefix = format_ident!("{}__box_", &ident.rust);
     let local_alloc = format_ident!("{}alloc", local_prefix);
+    let local_dealloc = format_ident!("{}dealloc", local_prefix);
     let local_drop = format_ident!("{}drop", local_prefix);
 
     let span = ident.span();
@@ -968,6 +970,11 @@ fn expand_rust_box(ident: &RustName, types: &Types) -> TokenStream {
             ::std::boxed::Box::into_raw(::std::boxed::Box::new(::std::mem::MaybeUninit::uninit()))
         }
         #[doc(hidden)]
+        #[export_name = #link_dealloc]
+        unsafe extern "C" fn #local_dealloc(ptr: *mut ::std::mem::MaybeUninit<#ident>) {
+            ::std::boxed::Box::from_raw(ptr);
+        }
+        #[doc(hidden)]
         #[export_name = #link_drop]
         unsafe extern "C" fn #local_drop(this: *mut ::std::boxed::Box<#ident>) {
             ::std::ptr::drop_in_place(this);


Test patch: