summaryrefslogtreecommitdiff
path: root/core
diff options
context:
space:
mode:
authorTom Sepez <tsepez@chromium.org>2017-05-16 15:57:26 -0700
committerChromium commit bot <commit-bot@chromium.org>2017-05-17 20:06:13 +0000
commita3cf977f974c67bd2cef8dee5b810784bfc21336 (patch)
tree7877cf8df0f5b6a214e870448e8d2df27a280e94 /core
parentb48912f4f3c428c8b71bbb714faa03cb843d40b0 (diff)
downloadpdfium-a3cf977f974c67bd2cef8dee5b810784bfc21336.tar.xz
CFX_UnownedPtr: check during assignment time as well.
In particular, doing m_pPtr = nullptr; in your dtor to evade this check will not longer work. Fix slight mis-ordering observeds in CFX_Font and CPDFXFA_Context. Change-Id: I3e6137159430333b091364021283a54a13d916b5 Reviewed-on: https://pdfium-review.googlesource.com/5570 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Tom Sepez <tsepez@chromium.org>
Diffstat (limited to 'core')
-rw-r--r--core/fxcrt/cfx_unowned_ptr.h18
-rw-r--r--core/fxcrt/cfx_unowned_ptr_unittest.cpp32
-rw-r--r--core/fxge/ge/cfx_font.cpp3
3 files changed, 45 insertions, 8 deletions
diff --git a/core/fxcrt/cfx_unowned_ptr.h b/core/fxcrt/cfx_unowned_ptr.h
index 11f4e8e38b..bc58144335 100644
--- a/core/fxcrt/cfx_unowned_ptr.h
+++ b/core/fxcrt/cfx_unowned_ptr.h
@@ -26,17 +26,20 @@ class CFX_UnownedPtr {
CFX_UnownedPtr(std::nullptr_t ptr) {}
#if defined(MEMORY_TOOL_REPLACES_ALLOCATOR)
- ~CFX_UnownedPtr() {
- if (m_pObj)
- reinterpret_cast<volatile uint8_t*>(m_pObj)[0];
- }
+ ~CFX_UnownedPtr() { Probe(); }
#endif
CFX_UnownedPtr& operator=(T* that) {
+#if defined(MEMORY_TOOL_REPLACES_ALLOCATOR)
+ Probe();
+#endif
m_pObj = that;
return *this;
}
CFX_UnownedPtr& operator=(const CFX_UnownedPtr& that) {
+#if defined(MEMORY_TOOL_REPLACES_ALLOCATOR)
+ Probe();
+#endif
if (*this != that)
m_pObj = that.Get();
return *this;
@@ -58,6 +61,13 @@ class CFX_UnownedPtr {
T* operator->() const { return m_pObj; }
private:
+#if defined(MEMORY_TOOL_REPLACES_ALLOCATOR)
+ void Probe() {
+ if (m_pObj)
+ reinterpret_cast<volatile uint8_t*>(m_pObj)[0];
+ }
+#endif
+
T* m_pObj = nullptr;
};
diff --git a/core/fxcrt/cfx_unowned_ptr_unittest.cpp b/core/fxcrt/cfx_unowned_ptr_unittest.cpp
index 7d29faedf5..586c4bbb2e 100644
--- a/core/fxcrt/cfx_unowned_ptr_unittest.cpp
+++ b/core/fxcrt/cfx_unowned_ptr_unittest.cpp
@@ -25,6 +25,15 @@ void DeleteDangling() {
delete ptr2;
}
+void AssignDangling() {
+ Clink* ptr1 = new Clink();
+ Clink* ptr2 = new Clink();
+ ptr2->next_ = ptr1;
+ delete ptr1;
+ ptr2->next_ = nullptr;
+ delete ptr2;
+}
+
} // namespace
TEST(fxcrt, UnownedPtrOk) {
@@ -43,7 +52,24 @@ TEST(fxcrt, UnownedPtrNotOk) {
#endif
}
-TEST(fxcrt, OperatorEQ) {
+TEST(fxcrt, UnownedAssignOk) {
+ Clink* ptr1 = new Clink();
+ Clink* ptr2 = new Clink();
+ ptr2->next_ = ptr1;
+ ptr2->next_ = nullptr;
+ delete ptr2;
+ delete ptr1;
+}
+
+TEST(fxcrt, UnownedAssignNotOk) {
+#if defined(MEMORY_TOOL_REPLACES_ALLOCATOR)
+ EXPECT_DEATH(AssignDangling(), "");
+#else
+ AssignDangling();
+#endif
+}
+
+TEST(fxcrt, UnownedOperatorEQ) {
int foo;
CFX_UnownedPtr<int> ptr1;
EXPECT_TRUE(ptr1 == ptr1);
@@ -59,7 +85,7 @@ TEST(fxcrt, OperatorEQ) {
EXPECT_TRUE(ptr1 == ptr3);
}
-TEST(fxcrt, OperatorNE) {
+TEST(fxcrt, UnownedOperatorNE) {
int foo;
CFX_UnownedPtr<int> ptr1;
EXPECT_FALSE(ptr1 != ptr1);
@@ -75,7 +101,7 @@ TEST(fxcrt, OperatorNE) {
EXPECT_FALSE(ptr1 != ptr3);
}
-TEST(fxcrt, OperatorLT) {
+TEST(fxcrt, UnownedOperatorLT) {
int foos[2];
CFX_UnownedPtr<int> ptr1(&foos[0]);
CFX_UnownedPtr<int> ptr2(&foos[1]);
diff --git a/core/fxge/ge/cfx_font.cpp b/core/fxge/ge/cfx_font.cpp
index 18baed9d09..1a45fd3f5b 100644
--- a/core/fxge/ge/cfx_font.cpp
+++ b/core/fxge/ge/cfx_font.cpp
@@ -545,8 +545,9 @@ CFX_FaceCache* CFX_Font::GetFaceCache() const {
void CFX_Font::ClearFaceCache() {
if (!m_FaceCache)
return;
- CFX_GEModule::Get()->GetFontCache()->ReleaseCachedFace(this);
+
m_FaceCache = nullptr;
+ CFX_GEModule::Get()->GetFontCache()->ReleaseCachedFace(this);
}
int CFX_Font::GetULPos() const {