Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(942)

Unified Diff: third_party/WebKit/Source/platform/heap/Handle.h

Issue 1411603007: [Oilpan] Add use-after-free detector in Member<> Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Rebase Created 5 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/platform/heap/Handle.h
diff --git a/third_party/WebKit/Source/platform/heap/Handle.h b/third_party/WebKit/Source/platform/heap/Handle.h
index 64e43fd71243011277fb2052e26dfe146148b0dc..1615b6b2ae8151e535175c58a2a0845805d04ebf 100644
--- a/third_party/WebKit/Source/platform/heap/Handle.h
+++ b/third_party/WebKit/Source/platform/heap/Handle.h
@@ -771,12 +771,44 @@ public:
checkPointer();
}
- T* get() const { return m_raw; }
+ T* get() const
+ {
+#if ENABLE(ASSERT) && defined(ADDRESS_SANITIZER)
+ // Verify that this handle points to a live on-heap object, if a pointer
+ // has been assigned to this handle out of GarbageCollectedMixin constructions.
+ if (m_raw && m_gcGeneration != gcGenerationUnchecked && !ThreadState::current()->isConstructingGCMixin()) {
+ HeapObjectHeader* header = heapObjectHeader();
+ ASSERT(m_gcGeneration == header->gcGeneration());
+ }
+#endif
+ return m_raw;
+ }
- void clear() { m_raw = nullptr; }
+ // This method is an accessor without any security checks, and it is
+ // expected to be used under some limited condition.
+ // So do NOT use it, if you do not know what it means.
+ // TODO(peria): We should remove this method.
+ T* unsafeGet() const
+ {
+ return m_raw;
+ }
+ void clear() { m_raw = nullptr; }
protected:
+#if ENABLE(ASSERT) && defined(ADDRESS_SANITIZER)
+ HeapObjectHeader* heapObjectHeader() const
+ {
+ BasePage* page = pageFromObject(m_raw);
+ if (page->isLargeObjectPage()) {
+ LargeObjectPage* large = static_cast<LargeObjectPage*>(page);
+ return large->heapObjectHeader();
+ }
+ NormalPage* normal = static_cast<NormalPage*>(page);
+ return normal->findHeaderFromObject(m_raw);
+ }
+#endif
+
void checkPointer()
{
#if ENABLE(ASSERT) && defined(ADDRESS_SANITIZER)
@@ -789,26 +821,31 @@ protected:
if (reinterpret_cast<intptr_t>(m_raw) % allocationGranularity)
return;
- // TODO(haraken): What we really want to check here is that the pointer
- // is a traceable object. In other words, the pointer is either of:
- //
- // (a) a pointer to the head of an on-heap object.
- // (b) a pointer to the head of an on-heap mixin object.
- //
- // We can check it by calling Heap::isHeapObjectAlive(m_raw),
- // but we cannot call it here because it requires to include T.h.
- // So we currently only try to implement the check for (a), but do
- // not insist that T's definition is in scope.
- if (IsFullyDefined<T>::value && !IsGarbageCollectedMixin<T>::value)
- ASSERT(HeapObjectHeader::fromPayload(m_raw)->checkHeader());
+ // m_gcGeneration verifies use-after-free of this Member handle.
+ // If m_gcGeneration is gcGenerationUnchecked or m_raw is nullptr,
+ // the verification is skipped.
+ // We set gcGenerationUnchecked while constructing a GC mixin object
+ // (and skip the verification unfortunately) because
+ // HeapObjectHeaderTrait<T>::heapObjectHeader doesn't work before
+ // the vtable of the object is complete.
+ // FIXME(Oilpan): We want to check them, too.
+ m_gcGeneration = gcGenerationUnchecked;
+ if (!ThreadState::current()->isConstructingGCMixin()) {
+ HeapObjectHeader* header = heapObjectHeader();
+ m_gcGeneration = header->gcGeneration();
+ ASSERT(m_gcGeneration != gcGenerationForFreeListEntry);
+ }
#endif
}
T* m_raw;
+#if ENABLE(ASSERT) && defined(ADDRESS_SANITIZER)
+ uint32_t m_gcGeneration;
+#endif
+
template<bool x, WTF::WeakHandlingFlag y, WTF::ShouldWeakPointersBeMarkedStrongly z, typename U, typename V> friend struct CollectionBackingTraceTrait;
friend class Visitor;
-
};
// WeakMember is similar to Member in that it is used to point to other oilpan
@@ -872,6 +909,14 @@ public:
return *this;
}
+ // TODO(peria): Remove this get() and use unsafeGet() at only
+ // where it is required.
+ T* get() const
+ {
+ // WeakMember may point to a dead object, so we skip the verification.
+ return Member<T>::unsafeGet();
+ }
+
private:
T** cell() const { return const_cast<T**>(&this->m_raw); }
@@ -946,15 +991,15 @@ public:
};
// Comparison operators between (Weak)Members, Persistents, and UntracedMembers.
-template<typename T, typename U> inline bool operator==(const Member<T>& a, const Member<U>& b) { return a.get() == b.get(); }
-template<typename T, typename U> inline bool operator!=(const Member<T>& a, const Member<U>& b) { return a.get() != b.get(); }
+template<typename T, typename U> inline bool operator==(const Member<T>& a, const Member<U>& b) { return a.unsafeGet() == b.unsafeGet(); }
+template<typename T, typename U> inline bool operator!=(const Member<T>& a, const Member<U>& b) { return a.unsafeGet() != b.unsafeGet(); }
template<typename T, typename U> inline bool operator==(const Persistent<T>& a, const Persistent<U>& b) { return a.get() == b.get(); }
template<typename T, typename U> inline bool operator!=(const Persistent<T>& a, const Persistent<U>& b) { return a.get() != b.get(); }
-template<typename T, typename U> inline bool operator==(const Member<T>& a, const Persistent<U>& b) { return a.get() == b.get(); }
-template<typename T, typename U> inline bool operator!=(const Member<T>& a, const Persistent<U>& b) { return a.get() != b.get(); }
-template<typename T, typename U> inline bool operator==(const Persistent<T>& a, const Member<U>& b) { return a.get() == b.get(); }
-template<typename T, typename U> inline bool operator!=(const Persistent<T>& a, const Member<U>& b) { return a.get() != b.get(); }
+template<typename T, typename U> inline bool operator==(const Member<T>& a, const Persistent<U>& b) { return a.unsafeGet() == b.get(); }
+template<typename T, typename U> inline bool operator!=(const Member<T>& a, const Persistent<U>& b) { return a.unsafeGet() != b.get(); }
+template<typename T, typename U> inline bool operator==(const Persistent<T>& a, const Member<U>& b) { return a.get() == b.unsafeGet(); }
+template<typename T, typename U> inline bool operator!=(const Persistent<T>& a, const Member<U>& b) { return a.get() != b.unsafeGet(); }
template<typename T>
class DummyBase {
@@ -1414,11 +1459,9 @@ template<typename T> struct PtrHash<blink::Member<T>> : PtrHash<T*> {
static bool equal(const U& a, const V& b) { return a == b; }
};
-template<typename T> struct PtrHash<blink::WeakMember<T>> : PtrHash<blink::Member<T>> {
-};
+template<typename T> struct PtrHash<blink::WeakMember<T>> : PtrHash<blink::Member<T>> { };
-template<typename T> struct PtrHash<blink::UntracedMember<T>> : PtrHash<blink::Member<T>> {
-};
+template<typename T> struct PtrHash<blink::UntracedMember<T>> : PtrHash<blink::Member<T>> { };
// PtrHash is the default hash for hash tables with members.
template<typename T> struct DefaultHash<blink::Member<T>> {

Powered by Google App Engine
This is Rietveld 408576698