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

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: Introduce gcGenerationUnchecked 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..a0842102d01069ce2037adec43801048788074a8 100644
--- a/third_party/WebKit/Source/platform/heap/Handle.h
+++ b/third_party/WebKit/Source/platform/heap/Handle.h
@@ -771,10 +771,32 @@ 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 = HeapObjectHeaderTrait<T>::heapObjectHeader(m_raw);
+ ASSERT(!header || (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:
void checkPointer()
@@ -789,26 +811,33 @@ 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 gcGenerationForFreeListEntry or m_raw is
+ // nullptr, the verification is skipped.
+ // For technical reason, we set gcGenerationForFreeListEntry in
+ // m_gcGeneration in case that this method is called in a constructor
+ // of a GCMixin object.
+ // TODO(peria): Set m_gcGeneration even if this is called in a
+ // GCMixin constructor.
+ m_gcGeneration = gcGenerationUnchecked;
+ if (IsFullyDefined<T>::value && !ThreadState::current()->isConstructingGCMixin()) {
+ HeapObjectHeader* header = HeapObjectHeaderTrait<T>::heapObjectHeader(m_raw);
+ if (header) {
+ 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 +901,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 +983,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 +1451,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