Intensify security checks for WTF::Optional.

Source/wtf/Optional.h

Index: Source/wtf/Optional.h
diff --git a/Source/wtf/Optional.h b/Source/wtf/Optional.h
index 0b453b53415a240799a8f0957fd869b2c0e5f7a3..264b45ed616936fddd1284997a7c3ff21520e711 100644
--- a/Source/wtf/Optional.h
+++ b/Source/wtf/Optional.h
@@ -32,34 +32,31 @@ template <typename T>
 class Optional {
     WTF_MAKE_NONCOPYABLE(Optional);
 public:
-    Optional() : m_engaged(false) { }
+    Optional() : m_ptr(nullptr) { }
     ~Optional()
     {
-        if (m_engaged)
-            storage()->~T();
+        if (m_ptr)
+            m_ptr->~T();
     }
 
-    typedef bool Optional::*UnspecifiedBoolType;
-    operator UnspecifiedBoolType() const { return m_engaged ? &Optional::m_engaged : nullptr; }
+    typedef T* Optional::*UnspecifiedBoolType;
+    operator UnspecifiedBoolType() const { return m_ptr ? &Optional::m_ptr : nullptr; }

[esprehn, 2015/06/02 22:13:08]

This doesn't need the branch in it, you can just return m_ptr.

return m_ptr;

should be enough, you also don't need the Optional:: part.

[jbroman, 2015/06/02 23:40:17]

This is the pattern used in OwnPtr::operator UnspecifiedBoolType(), which also
has an m_ptr member.

m_ptr isn't of type UnspecifiedBoolType, so returning that would not compile; I
would have to return something constructible from T*. Using "operator bool" is
the obvious choice, but that would make this implicitly convertible to all
integer types (see comment in OwnPtr.h).

The modern fix for the is to use "explicit operator bool", but our Windows
toolchain doesn't yet support that (see "explicit conversion operator" on
http://chromium-cpp.appspot.com/).

So this is the hack/pattern we currently use elsewhere in WTF, ugly though it
is.

(Aside: I suspect, without having checked, that after inlining, the compiler can
eliminate this branch. Not sure though.)

[esprehn, 2015/06/02 23:57:08]

Right, just return a void* or something. Having a branch inside the boolean
operator is not needed and just makes it slower.

 
-    T& operator*() { ASSERT(m_engaged); return *storage(); }
-    const T& operator*() const { ASSERT(m_engaged); return *storage(); }
-    T* operator->() { ASSERT(m_engaged); return storage(); }
-    const T* operator->() const { ASSERT(m_engaged); return storage(); }
+    T& operator*() { ASSERT_WITH_SECURITY_IMPLICATION(m_ptr); return *m_ptr; }
+    const T& operator*() const { ASSERT_WITH_SECURITY_IMPLICATION(m_ptr); return *m_ptr; }
+    T* operator->() { ASSERT_WITH_SECURITY_IMPLICATION(m_ptr); return m_ptr; }
+    const T* operator->() const { ASSERT_WITH_SECURITY_IMPLICATION(m_ptr); return m_ptr; }
 
     template <typename... Args>
     void emplace(Args&&... args)
     {
-        ASSERT(!m_engaged);
-        new (storage()) T(forward<Args>(args)...);
-        m_engaged = true;
+        RELEASE_ASSERT(!m_ptr);
+        m_ptr = reinterpret_cast_ptr<T*>(&m_storage.buffer);
+        new (m_ptr) T(forward<Args>(args)...);
     }
 
 private:
-    T* storage() { return reinterpret_cast_ptr<T*>(&m_storage.buffer); }
-    const T* storage() const { return reinterpret_cast_ptr<const T*>(&m_storage.buffer); }
-
-    bool m_engaged;
+    T* m_ptr;
     AlignedBuffer<sizeof(T), WTF_ALIGN_OF(T)> m_storage;
 };