Intensify security checks for WTF::Optional.

Source/wtf/Optional.h

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef Optional_h
 #define Optional_h
 
 #include "wtf/Alignment.h"
 #include "wtf/Assertions.h"
 #include "wtf/Noncopyable.h"
@@ skipping 14 matching lines @@
 //       recorder.emplace(constructor, args, here);
 //   // recorder destroyed at end of scope
 //
 // Note in particular that unlike a pointer, though, dereferencing a const
 // optional yields a const reference.
 
 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;
 };
 
 } // namespace WTF
 
 using WTF::Optional;
 
 #endif // Optional_h