Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C++ MaybeUninit bypasses class-specific member operator new #553

Closed
dtolnay opened this issue Dec 9, 2020 · 0 comments · Fixed by #558
Closed

C++ MaybeUninit bypasses class-specific member operator new #553

dtolnay opened this issue Dec 9, 2020 · 0 comments · Fixed by #558
Labels

Comments

@dtolnay
Copy link
Owner

dtolnay commented Dec 9, 2020

For an input like this:

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:

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.

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:

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();
}
new(size_t)
Type
~Type
delete(void *)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant