summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--base/memory/scoped_ptr.h17
-rw-r--r--base/memory/scoped_ptr_unittest.cc38
-rw-r--r--crypto/scoped_nss_types.h2
-rw-r--r--crypto/scoped_openssl_types.h1
-rw-r--r--net/ssl/openssl_client_key_store.cc18
-rw-r--r--net/ssl/openssl_client_key_store.h4
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;