diff options
-rw-r--r-- | base/memory/scoped_ptr.h | 17 | ||||
-rw-r--r-- | base/memory/scoped_ptr_unittest.cc | 38 | ||||
-rw-r--r-- | crypto/scoped_nss_types.h | 2 | ||||
-rw-r--r-- | crypto/scoped_openssl_types.h | 1 | ||||
-rw-r--r-- | net/ssl/openssl_client_key_store.cc | 18 | ||||
-rw-r--r-- | net/ssl/openssl_client_key_store.h | 4 |
6 files changed, 67 insertions, 13 deletions
diff --git a/base/memory/scoped_ptr.h b/base/memory/scoped_ptr.h index d93a8b4..f3bbd12 100644 --- a/base/memory/scoped_ptr.h +++ b/base/memory/scoped_ptr.h @@ -184,6 +184,17 @@ template <typename T> struct IsNotRefCounted { }; }; +template <typename T> +struct ShouldAbortOnSelfReset { + template <typename U> + static NoType Test(const typename U::AllowSelfReset*); + + template <typename U> + static YesType Test(...); + + static const bool value = sizeof(Test<T>(0)) == sizeof(YesType); +}; + // Minimal implementation of the core logic of scoped_ptr, suitable for // reuse in both scoped_ptr and its specializations. template <class T, class D> @@ -222,9 +233,9 @@ class scoped_ptr_impl { } void reset(T* p) { - // This is a self-reset, which is no longer allowed: http://crbug.com/162971 - if (p != nullptr && p == data_.ptr) - abort(); + // This is a self-reset, which is no longer allowed for default deleters: + // https://crbug.com/162971 + assert(!ShouldAbortOnSelfReset<D>::value || p == nullptr || p != data_.ptr); // Note that running data_.ptr = p can lead to undefined behavior if // get_deleter()(get()) deletes this. In order to prevent this, reset() diff --git a/base/memory/scoped_ptr_unittest.cc b/base/memory/scoped_ptr_unittest.cc index 2ea44e9..3da8d3b 100644 --- a/base/memory/scoped_ptr_unittest.cc +++ b/base/memory/scoped_ptr_unittest.cc @@ -656,3 +656,41 @@ TEST(ScopedPtrTest, Conversion) { scoped_ptr<Super> super2 = SubClassReturn(); super2 = SubClassReturn(); } + +// Android death tests don't work properly with assert(). Yay. +#if !defined(NDEBUG) && defined(GTEST_HAS_DEATH_TEST) && !defined(OS_ANDROID) +TEST(ScopedPtrTest, SelfResetAbortsWithDefaultDeleter) { + scoped_ptr<int> x(new int); + EXPECT_DEATH(x.reset(x.get()), ""); +} + +TEST(ScopedPtrTest, SelfResetAbortsWithDefaultArrayDeleter) { + scoped_ptr<int[]> y(new int[4]); + EXPECT_DEATH(y.reset(y.get()), ""); +} + +TEST(ScopedPtrTest, SelfResetAbortsWithDefaultFreeDeleter) { + scoped_ptr<int, base::FreeDeleter> z(static_cast<int*>(malloc(sizeof(int)))); + EXPECT_DEATH(z.reset(z.get()), ""); +} + +// A custom deleter that doesn't opt out should still crash. +TEST(ScopedPtrTest, SelfResetAbortsWithCustomDeleter) { + struct CustomDeleter { + inline void operator()(int* x) { delete x; } + }; + scoped_ptr<int, CustomDeleter> x(new int); + EXPECT_DEATH(x.reset(x.get()), ""); +} +#endif + +TEST(ScopedPtrTest, SelfResetWithCustomDeleterOptOut) { + // A custom deleter should be able to opt out of self-reset abort behavior. + struct NoOpDeleter { + typedef void AllowSelfReset; + inline void operator()(int*) {} + }; + scoped_ptr<int> owner(new int); + scoped_ptr<int, NoOpDeleter> x(owner.get()); + x.reset(x.get()); +} diff --git a/crypto/scoped_nss_types.h b/crypto/scoped_nss_types.h index 8e96e8d..fdfb83c 100644 --- a/crypto/scoped_nss_types.h +++ b/crypto/scoped_nss_types.h @@ -16,6 +16,7 @@ namespace crypto { template <typename Type, void (*Destroyer)(Type*)> struct NSSDestroyer { + typedef void AllowSelfReset; void operator()(Type* ptr) const { Destroyer(ptr); } @@ -23,6 +24,7 @@ struct NSSDestroyer { template <typename Type, void (*Destroyer)(Type*, PRBool), PRBool freeit> struct NSSDestroyer1 { + typedef void AllowSelfReset; void operator()(Type* ptr) const { Destroyer(ptr, freeit); } diff --git a/crypto/scoped_openssl_types.h b/crypto/scoped_openssl_types.h index d0428fb..cc056e4 100644 --- a/crypto/scoped_openssl_types.h +++ b/crypto/scoped_openssl_types.h @@ -22,6 +22,7 @@ namespace crypto { // base::internal::RunnableAdapter<>, but that's far too heavy weight. template <typename Type, void (*Destroyer)(Type*)> struct OpenSSLDestroyer { + typedef void AllowSelfReset; void operator()(Type* ptr) const { Destroyer(ptr); } }; diff --git a/net/ssl/openssl_client_key_store.cc b/net/ssl/openssl_client_key_store.cc index b0b5eb3..de65cd9 100644 --- a/net/ssl/openssl_client_key_store.cc +++ b/net/ssl/openssl_client_key_store.cc @@ -6,6 +6,7 @@ #include <openssl/evp.h> #include <openssl/x509.h> +#include <algorithm> #include "base/memory/scoped_ptr.h" #include "base/memory/singleton.h" @@ -50,15 +51,14 @@ OpenSSLClientKeyStore::KeyPair::KeyPair(const KeyPair& other) private_key(EVP_PKEY_dup(other.private_key.get())) { } -void OpenSSLClientKeyStore::KeyPair::operator=(const KeyPair& other) { - // Use a temporary ScopedEVP_PKEY because scoped_ptr does not allow resetting - // to the current value, even though it's safe here. - crypto::ScopedEVP_PKEY public_key_tmp(EVP_PKEY_dup(other.public_key.get())); - crypto::ScopedEVP_PKEY private_key_tmp(EVP_PKEY_dup(other.private_key.get())); - public_key.reset(); - public_key = public_key_tmp.Pass(); - private_key.reset(); - private_key = private_key_tmp.Pass(); +void OpenSSLClientKeyStore::KeyPair::operator=(KeyPair other) { + swap(other); +} + +void OpenSSLClientKeyStore::KeyPair::swap(KeyPair& other) { + using std::swap; + swap(public_key, other.public_key); + swap(private_key, other.private_key); } int OpenSSLClientKeyStore::FindKeyPairIndex(EVP_PKEY* public_key) { diff --git a/net/ssl/openssl_client_key_store.h b/net/ssl/openssl_client_key_store.h index 0cbb23a..6c06148 100644 --- a/net/ssl/openssl_client_key_store.h +++ b/net/ssl/openssl_client_key_store.h @@ -73,7 +73,9 @@ class NET_EXPORT OpenSSLClientKeyStore { public: explicit KeyPair(EVP_PKEY* pub_key, EVP_PKEY* priv_key); KeyPair(const KeyPair& other); - void operator=(const KeyPair& other); + // Intentionally pass by value, in order to use the copy-and-swap idiom. + void operator=(KeyPair other); + void swap(KeyPair& other); ~KeyPair(); crypto::ScopedEVP_PKEY public_key; |