Don't use std::is_trivially_destructable.

It returns false for incomplete types (or is undefined prior to C++14),
so other instantiations can get confused. Instead, require an explicit
kAllowUniquePtr toggle.

I tried using sizeof(T) to SFINAE-detect an incomplete type but ran into
MSVC issues, I think
https://connect.microsoft.com/VisualStudio/feedback/details/820390/vc-sizeof-doesnt-work-as-expected-in-sfinae-context
Though it seems this also may cause ODR violations if different
compilation units disagree on whether a type is complete. This is all a
mess, so just do the boring thing.

Bug: 132
Change-Id: I6f2d47499f16e75f62629c76f43a5329e91c6daf
Reviewed-on: https://boringssl-review.googlesource.com/18464
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
This commit is contained in:
David Benjamin 2017-07-25 22:26:44 -04:00
parent 2507d9e709
commit 9a89250285

View File

@ -201,13 +201,11 @@ void Delete(T *t) {
} }
} }
/* Register all types with non-trivial destructors with |UniquePtr|. Types with /* All types with kAllowUniquePtr set may be used with UniquePtr. Other types
* trivial destructors may be C structs which require a |BORINGSSL_MAKE_DELETER| * may be C structs which require a |BORINGSSL_MAKE_DELETER| registration. */
* registration. */
namespace internal { namespace internal {
template <typename T> template <typename T>
struct DeleterImpl<T, typename std::enable_if< struct DeleterImpl<T, typename std::enable_if<T::kAllowUniquePtr>::type> {
!std::is_trivially_destructible<T>::value>::type> {
static void Free(T *t) { Delete(t); } static void Free(T *t) { Delete(t); }
}; };
} }
@ -223,7 +221,7 @@ UniquePtr<T> MakeUnique(Args &&... args) {
#define HAS_VIRTUAL_DESTRUCTOR #define HAS_VIRTUAL_DESTRUCTOR
#define PURE_VIRTUAL = 0 #define PURE_VIRTUAL = 0
#else #else
/* HAS_VIRTUAL_DESTRUCTOR should be declared in any base class which defines a /* HAS_VIRTUAL_DESTRUCTOR should be declared in any base clas ~s which defines a
* virtual destructor. This avoids a dependency on |_ZdlPv| and prevents the * virtual destructor. This avoids a dependency on |_ZdlPv| and prevents the
* class from being used with |delete|. */ * class from being used with |delete|. */
#define HAS_VIRTUAL_DESTRUCTOR \ #define HAS_VIRTUAL_DESTRUCTOR \
@ -470,6 +468,8 @@ class SSLAEADContext {
public: public:
SSLAEADContext(uint16_t version, const SSL_CIPHER *cipher); SSLAEADContext(uint16_t version, const SSL_CIPHER *cipher);
~SSLAEADContext(); ~SSLAEADContext();
static constexpr bool kAllowUniquePtr = true;
SSLAEADContext(const SSLAEADContext &&) = delete; SSLAEADContext(const SSLAEADContext &&) = delete;
SSLAEADContext &operator=(const SSLAEADContext &&) = delete; SSLAEADContext &operator=(const SSLAEADContext &&) = delete;
@ -771,6 +771,7 @@ int custom_ext_add_serverhello(SSL_HANDSHAKE *hs, CBB *extensions);
class SSLKeyShare { class SSLKeyShare {
public: public:
virtual ~SSLKeyShare() {} virtual ~SSLKeyShare() {}
static constexpr bool kAllowUniquePtr = true;
HAS_VIRTUAL_DESTRUCTOR HAS_VIRTUAL_DESTRUCTOR
/* Create returns a SSLKeyShare instance for use with group |group_id| or /* Create returns a SSLKeyShare instance for use with group |group_id| or
@ -1072,6 +1073,7 @@ enum ssl_hs_wait_t {
struct SSL_HANDSHAKE { struct SSL_HANDSHAKE {
explicit SSL_HANDSHAKE(SSL *ssl); explicit SSL_HANDSHAKE(SSL *ssl);
~SSL_HANDSHAKE(); ~SSL_HANDSHAKE();
static constexpr bool kAllowUniquePtr = true;
/* ssl is a non-owning pointer to the parent |SSL| object. */ /* ssl is a non-owning pointer to the parent |SSL| object. */
SSL *ssl; SSL *ssl;