2011-04-22 Oliver Hunt <oliver@apple.com>
authoroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Apr 2011 18:26:34 +0000 (18:26 +0000)
committeroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Apr 2011 18:26:34 +0000 (18:26 +0000)
        Reviewed by Geoffrey Garen.

        Make it harder to use HandleSlot incorrectly
        https://bugs.webkit.org/show_bug.cgi?id=59205

        Just add a little type fudging to make it harder to
        incorrectly assign through a HandleSlot.

        * API/JSCallbackObjectFunctions.h:
        (JSC::::init):
        * JavaScriptCore.exp:
        * heap/Handle.h:
        (JSC::HandleBase::operator!):
        (JSC::HandleBase::operator UnspecifiedBoolType*):
        (JSC::HandleTypes::getFromSlot):
        * heap/HandleHeap.cpp:
        (JSC::HandleHeap::markStrongHandles):
        (JSC::HandleHeap::markWeakHandles):
        (JSC::HandleHeap::finalizeWeakHandles):
        (JSC::HandleHeap::writeBarrier):
        (JSC::HandleHeap::protectedGlobalObjectCount):
        (JSC::HandleHeap::isValidWeakNode):
        * heap/HandleHeap.h:
        (JSC::HandleHeap::copyWeak):
        (JSC::HandleHeap::makeWeak):
        (JSC::HandleHeap::Node::slot):
        * heap/HandleStack.cpp:
        (JSC::HandleStack::mark):
        (JSC::HandleStack::grow):
        * heap/HandleStack.h:
        (JSC::HandleStack::zapTo):
        (JSC::HandleStack::push):
        * heap/Heap.cpp:
        (JSC::HandleHeap::protectedObjectTypeCounts):
        * heap/Local.h:
        (JSC::::set):
        * heap/Strong.h:
        (JSC::Strong::set):
        * heap/Weak.h:
        (JSC::Weak::set):
        * runtime/StructureTransitionTable.h:
        (JSC::StructureTransitionTable::singleTransition):
        (JSC::StructureTransitionTable::setSingleTransition):
        * runtime/WeakGCMap.h:
        (JSC::WeakGCMap::add):
        (JSC::WeakGCMap::set):
        * runtime/WriteBarrier.h:
        (JSC::OpaqueJSValue::toJSValue):
        (JSC::OpaqueJSValue::toJSValueRef):
        (JSC::OpaqueJSValue::fromJSValue):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@84650 268f45cc-cd09-0410-ab3c-d52691b4dbfc

15 files changed:
Source/JavaScriptCore/API/JSCallbackObjectFunctions.h
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/JavaScriptCore.exp
Source/JavaScriptCore/heap/Handle.h
Source/JavaScriptCore/heap/HandleHeap.cpp
Source/JavaScriptCore/heap/HandleHeap.h
Source/JavaScriptCore/heap/HandleStack.cpp
Source/JavaScriptCore/heap/HandleStack.h
Source/JavaScriptCore/heap/Heap.cpp
Source/JavaScriptCore/heap/Local.h
Source/JavaScriptCore/heap/Strong.h
Source/JavaScriptCore/heap/Weak.h
Source/JavaScriptCore/runtime/StructureTransitionTable.h
Source/JavaScriptCore/runtime/WeakGCMap.h
Source/JavaScriptCore/runtime/WriteBarrier.h

index 8639e1a..6ffb51d 100644 (file)
@@ -96,7 +96,7 @@ void JSCallbackObject<Base>::init(ExecState* exec)
         HandleSlot slot = exec->globalData().allocateGlobalHandle();
         HandleHeap::heapFor(slot)->makeWeak(slot, m_callbackObjectData.get(), classRef());
         HandleHeap::heapFor(slot)->writeBarrier(slot, this);
-        *slot = this;
+        slot->fromJSValue(this);
     }
 }
 
index 005ffd5..9d76b03 100644 (file)
@@ -1,3 +1,56 @@
+2011-04-22  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Geoffrey Garen.
+
+        Make it harder to use HandleSlot incorrectly
+        https://bugs.webkit.org/show_bug.cgi?id=59205
+
+        Just add a little type fudging to make it harder to
+        incorrectly assign through a HandleSlot.
+
+        * API/JSCallbackObjectFunctions.h:
+        (JSC::::init):
+        * JavaScriptCore.exp:
+        * heap/Handle.h:
+        (JSC::HandleBase::operator!):
+        (JSC::HandleBase::operator UnspecifiedBoolType*):
+        (JSC::HandleTypes::getFromSlot):
+        * heap/HandleHeap.cpp:
+        (JSC::HandleHeap::markStrongHandles):
+        (JSC::HandleHeap::markWeakHandles):
+        (JSC::HandleHeap::finalizeWeakHandles):
+        (JSC::HandleHeap::writeBarrier):
+        (JSC::HandleHeap::protectedGlobalObjectCount):
+        (JSC::HandleHeap::isValidWeakNode):
+        * heap/HandleHeap.h:
+        (JSC::HandleHeap::copyWeak):
+        (JSC::HandleHeap::makeWeak):
+        (JSC::HandleHeap::Node::slot):
+        * heap/HandleStack.cpp:
+        (JSC::HandleStack::mark):
+        (JSC::HandleStack::grow):
+        * heap/HandleStack.h:
+        (JSC::HandleStack::zapTo):
+        (JSC::HandleStack::push):
+        * heap/Heap.cpp:
+        (JSC::HandleHeap::protectedObjectTypeCounts):
+        * heap/Local.h:
+        (JSC::::set):
+        * heap/Strong.h:
+        (JSC::Strong::set):
+        * heap/Weak.h:
+        (JSC::Weak::set):
+        * runtime/StructureTransitionTable.h:
+        (JSC::StructureTransitionTable::singleTransition):
+        (JSC::StructureTransitionTable::setSingleTransition):
+        * runtime/WeakGCMap.h:
+        (JSC::WeakGCMap::add):
+        (JSC::WeakGCMap::set):
+        * runtime/WriteBarrier.h:
+        (JSC::OpaqueJSValue::toJSValue):
+        (JSC::OpaqueJSValue::toJSValueRef):
+        (JSC::OpaqueJSValue::fromJSValue):
+
 2011-04-22  Patrick Gansterer  <paroga@webkit.org>
 
         Unreviewed. Build fix for ENABLE(INTERPRETER) after r84556.
index f33a38c..f41c5e4 100644 (file)
@@ -97,7 +97,7 @@ _WTFReportBacktrace
 _WTFReportError
 _WTFReportFatalError
 __ZN14OpaqueJSString6createERKN3JSC7UStringE
-__ZN3JSC10HandleHeap12writeBarrierEPNS_7JSValueERKS1_
+__ZN3JSC10HandleHeap12writeBarrierEPNS_13OpaqueJSValueERKNS_7JSValueE
 __ZN3JSC10HandleHeap4growEv
 __ZN3JSC10Identifier11addSlowCaseEPNS_12JSGlobalDataEPN3WTF10StringImplE
 __ZN3JSC10Identifier11addSlowCaseEPNS_9ExecStateEPN3WTF10StringImplE
index 531d535..a7900f7 100644 (file)
@@ -53,11 +53,11 @@ class HandleBase {
     template <typename KeyType, typename MappedType, typename FinalizerCallback, typename HashArg, typename KeyTraitsArg> friend class WeakGCMap;
 
 public:
-    bool operator!() const { return !m_slot || !*m_slot; }
+    bool operator!() const { return !m_slot || !m_slot->toJSValue(); }
 
     // This conversion operator allows implicit conversion to bool but not to other integer types.
     typedef JSValue (HandleBase::*UnspecifiedBoolType);
-    operator UnspecifiedBoolType*() const { return (m_slot && *m_slot) ? reinterpret_cast<UnspecifiedBoolType*>(1) : 0; }
+    operator UnspecifiedBoolType*() const { return (m_slot && m_slot->toJSValue()) ? reinterpret_cast<UnspecifiedBoolType*>(1) : 0; }
 
 protected:
     HandleBase(HandleSlot slot)
@@ -79,14 +79,14 @@ private:
 
 template <typename T> struct HandleTypes {
     typedef T* ExternalType;
-    static ExternalType getFromSlot(HandleSlot slot) { return (slot && *slot) ? reinterpret_cast<ExternalType>(slot->asCell()) : 0; }
+    static ExternalType getFromSlot(HandleSlot slot) { return (slot && slot->toJSValue()) ? reinterpret_cast<ExternalType>(slot->toJSValue().asCell()) : 0; }
     static JSValue toJSValue(T* cell) { return reinterpret_cast<JSCell*>(cell); }
     template <typename U> static void validateUpcast() { T* temp; temp = (U*)0; }
 };
 
 template <> struct HandleTypes<Unknown> {
     typedef JSValue ExternalType;
-    static ExternalType getFromSlot(HandleSlot slot) { return slot ? *slot : JSValue(); }
+    static ExternalType getFromSlot(HandleSlot slot) { return slot ? slot->toJSValue() : JSValue(); }
     static JSValue toJSValue(const JSValue& v) { return v; }
     template <typename U> static void validateUpcast() {}
 };
index 9b05db2..9627b28 100644 (file)
@@ -64,7 +64,7 @@ void HandleHeap::markStrongHandles(HeapRootVisitor& heapRootMarker)
 {
     Node* end = m_strongList.end();
     for (Node* node = m_strongList.begin(); node != end; node = node->next())
-        heapRootMarker.mark(node->slot());
+        heapRootMarker.mark(node->slot()->toJSValueRef());
 }
 
 void HandleHeap::markWeakHandles(HeapRootVisitor& heapRootVisitor)
@@ -74,7 +74,7 @@ void HandleHeap::markWeakHandles(HeapRootVisitor& heapRootVisitor)
     Node* end = m_weakList.end();
     for (Node* node = m_weakList.begin(); node != end; node = node->next()) {
         ASSERT(isValidWeakNode(node));
-        JSCell* cell = node->slot()->asCell();
+        JSCell* cell = node->slot()->toJSValue().asCell();
         if (Heap::isMarked(cell))
             continue;
 
@@ -85,7 +85,7 @@ void HandleHeap::markWeakHandles(HeapRootVisitor& heapRootVisitor)
         if (!weakOwner->isReachableFromOpaqueRoots(Handle<Unknown>::wrapSlot(node->slot()), node->weakOwnerContext(), visitor))
             continue;
 
-        heapRootVisitor.mark(node->slot());
+        heapRootVisitor.mark(node->slot()->toJSValueRef());
     }
 }
 
@@ -96,7 +96,7 @@ void HandleHeap::finalizeWeakHandles()
         m_nextToFinalize = node->next();
 
         ASSERT(isValidWeakNode(node));
-        JSCell* cell = node->slot()->asCell();
+        JSCell* cell = node->slot()->toJSValue().asCell();
         if (Heap::isMarked(cell))
             continue;
 
@@ -106,7 +106,7 @@ void HandleHeap::finalizeWeakHandles()
                 continue;
         }
 
-        *node->slot() = JSValue();
+        node->slot()->fromJSValue(JSValue());
         SentinelLinkedList<Node>::remove(node);
         m_immediateList.push(node);
     }
@@ -118,7 +118,7 @@ void HandleHeap::writeBarrier(HandleSlot slot, const JSValue& value)
 {
     ASSERT(!m_nextToFinalize); // Forbid assignment to handles during the finalization phase, since it would violate many GC invariants.
 
-    if (!value == !*slot && slot->isCell() == value.isCell())
+    if (!value == !slot->toJSValue() && slot->toJSValue().isCell() == value.isCell())
         return;
 
     Node* node = toNode(slot);
@@ -141,7 +141,7 @@ unsigned HandleHeap::protectedGlobalObjectCount()
     unsigned count = 0;
     Node* end = m_strongList.end();
     for (Node* node = m_strongList.begin(); node != end; node = node->next()) {
-        JSValue value = *node->slot();
+        JSValue value = node->slot()->toJSValue();
         if (value.isObject() && asObject(value.asCell())->isGlobalObject())
             count++;
     }
@@ -154,7 +154,7 @@ bool HandleHeap::isValidWeakNode(Node* node)
     if (!node->isWeak())
         return false;
 
-    JSValue value = *node->slot();
+    JSValue value = node->slot()->toJSValue();
     if (!value || !value.isCell())
         return false;
 
index 5c151c3..4d33d46 100644 (file)
@@ -173,8 +173,8 @@ inline HandleSlot HandleHeap::copyWeak(HandleSlot other)
 {
     Node* node = toNode(allocate());
     node->makeWeak(toNode(other)->weakOwner(), toNode(other)->weakOwnerContext());
-    writeBarrier(node->slot(), *other);
-    *node->slot() = *other;
+    writeBarrier(node->slot(), other->toJSValue());
+    node->slot()->fromJSValue(other->toJSValue());
     return toHandle(node);
 }
 
@@ -184,7 +184,7 @@ inline void HandleHeap::makeWeak(HandleSlot handle, WeakHandleOwner* weakOwner,
     node->makeWeak(weakOwner, context);
 
     SentinelLinkedList<Node>::remove(node);
-    if (!*handle || !handle->isCell()) {
+    if (!handle->toJSValue() || !handle->toJSValue().isCell()) {
         m_immediateList.push(node);
         return;
     }
@@ -215,7 +215,7 @@ inline HandleHeap::Node::Node(WTF::SentinelTag)
 
 inline HandleSlot HandleHeap::Node::slot()
 {
-    return &m_value;
+    return reinterpret_cast<HandleSlot>(&m_value);
 }
 
 inline HandleHeap* HandleHeap::Node::handleHeap()
index ada4f99..fadb368 100644 (file)
@@ -41,21 +41,21 @@ HandleStack::HandleStack()
 
 void HandleStack::mark(HeapRootVisitor& heapRootMarker)
 {
-    const Vector<HandleSlot>& blocks = m_blockStack.blocks();
+    const Vector<JSValue*>& blocks = m_blockStack.blocks();
     size_t blockLength = m_blockStack.blockLength;
 
     int end = blocks.size() - 1;
     for (int i = 0; i < end; ++i) {
-        HandleSlot block = blocks[i];
+        JSValue* block = blocks[i];
         heapRootMarker.mark(block, blockLength);
     }
-    HandleSlot block = blocks[end];
+    JSValue* block = blocks[end];
     heapRootMarker.mark(block, m_frame.m_next - block);
 }
 
 void HandleStack::grow()
 {
-    HandleSlot block = m_blockStack.grow();
+    JSValue* block = m_blockStack.grow();
     m_frame.m_next = block;
     m_frame.m_end = block + m_blockStack.blockLength;
 }
index 115784a..19785fc 100644 (file)
@@ -41,8 +41,8 @@ class HandleStack {
 public:
     class Frame {
     public:
-        HandleSlot m_next;
-        HandleSlot m_end;
+        JSValue* m_next;
+        JSValue* m_end;
     };
 
     HandleStack();
@@ -82,7 +82,7 @@ inline void HandleStack::zapTo(Frame& lastFrame)
 #ifdef NDEBUG
     UNUSED_PARAM(lastFrame);
 #else
-    const Vector<HandleSlot>& blocks = m_blockStack.blocks();
+    const Vector<JSValue*>& blocks = m_blockStack.blocks();
     
     if (lastFrame.m_end != m_frame.m_end) { // Zapping to a frame in a different block.
         int i = blocks.size() - 1;
@@ -91,13 +91,13 @@ inline void HandleStack::zapTo(Frame& lastFrame)
                 blocks[i][j] = JSValue();
         }
         
-        for (HandleSlot it = blocks[i] + m_blockStack.blockLength - 1; it != lastFrame.m_next - 1; --it)
+        for (JSValue* it = blocks[i] + m_blockStack.blockLength - 1; it != lastFrame.m_next - 1; --it)
             *it = JSValue();
         
         return;
     }
     
-    for (HandleSlot it = m_frame.m_next - 1; it != lastFrame.m_next - 1; --it)
+    for (JSValue* it = m_frame.m_next - 1; it != lastFrame.m_next - 1; --it)
         *it = JSValue();
 #endif
 }
@@ -121,7 +121,7 @@ inline HandleSlot HandleStack::push()
     ASSERT(m_scopeDepth); // Creating a Local outside of a LocalScope is a memory leak.
     if (m_frame.m_next == m_frame.m_end)
         grow();
-    return m_frame.m_next++;
+    return reinterpret_cast<HandleSlot>(m_frame.m_next++);
 }
 
 }
index 6375b14..a690e64 100644 (file)
@@ -363,7 +363,7 @@ void HandleHeap::protectedObjectTypeCounts(TypeCounter& typeCounter)
 {
     Node* end = m_strongList.end();
     for (Node* node = m_strongList.begin(); node != end; node = node->next()) {
-        JSValue value = *node->slot();
+        JSValue value = node->slot()->toJSValue();
         if (value && value.isCell())
             typeCounter(value.asCell());
     }
index ac7d136..c116926 100644 (file)
@@ -95,7 +95,7 @@ template <typename T> inline void Local<T>::set(ExternalType externalType)
 {
     ASSERT(slot());
     ASSERT(!HandleTypes<T>::toJSValue(externalType) || !HandleTypes<T>::toJSValue(externalType).isCell() || Heap::isMarked(HandleTypes<T>::toJSValue(externalType).asCell()));
-    *slot() = externalType;
+    slot()->fromJSValue(externalType);
 }
 
 
index 9f2aa05..8beface 100644 (file)
@@ -140,7 +140,7 @@ private:
         ASSERT(slot());
         JSValue value = HandleTypes<T>::toJSValue(externalType);
         HandleHeap::heapFor(slot())->writeBarrier(slot(), value);
-        *slot() = value;
+        slot()->fromJSValue(value);
     }
 };
 
index 62e2596..fb14891 100644 (file)
@@ -131,7 +131,7 @@ private:
         JSValue value = HandleTypes<T>::toJSValue(externalType);
         ASSERT(!value || !value.isCell() || Heap::isMarked(value.asCell()));
         HandleHeap::heapFor(slot())->writeBarrier(slot(), value);
-        *slot() = value;
+        slot()->fromJSValue(value);
     }
 };
 
index adebad2..658a8cc 100644 (file)
@@ -139,8 +139,8 @@ private:
     {
         ASSERT(isUsingSingleSlot());
         if (HandleSlot slot = this->slot()) {
-            if (*slot)
-                return reinterpret_cast<Structure*>(slot->asCell());
+            if (slot->toJSValue())
+                return reinterpret_cast<Structure*>(slot->toJSValue().asCell());
         }
         return 0;
     }
@@ -162,7 +162,7 @@ private:
             m_data = reinterpret_cast<intptr_t>(slot) | UsingSingleSlotFlag;
         }
         HandleHeap::heapFor(slot)->writeBarrier(slot, reinterpret_cast<JSCell*>(structure));
-        *slot = reinterpret_cast<JSCell*>(structure);
+        slot->fromJSValue(reinterpret_cast<JSCell*>(structure));
     }
 
     intptr_t m_data;
index 5ad1c62..aa0a71f 100644 (file)
@@ -129,7 +129,7 @@ public:
             iter.first->second = slot;
             HandleHeap::heapFor(slot)->makeWeak(slot, this, FinalizerCallback::finalizerContextFor(key));
             HandleHeap::heapFor(slot)->writeBarrier(slot, value);
-            *slot = value;
+            slot->fromJSValue(value);
         }
         return iter;
     }
@@ -139,7 +139,7 @@ public:
         HandleSlot slot = iter.m_iterator->second;
         ASSERT(slot);
         HandleHeap::heapFor(slot)->writeBarrier(slot, value);
-        *slot = value;
+        slot->fromJSValue(value);
     }
 
     void set(JSGlobalData& globalData, const KeyType& key, ExternalType value)
@@ -152,7 +152,7 @@ public:
             iter.first->second = slot;
         }
         HandleHeap::heapFor(slot)->writeBarrier(slot, value);
-        *slot = value;
+        slot->fromJSValue(value);
     }
 
     ExternalType take(const KeyType& key)
index 32cb968..37f7296 100644 (file)
@@ -41,7 +41,13 @@ inline void writeBarrier(JSGlobalData&, const JSCell*, JSCell*)
 }
 
 typedef enum { } Unknown;
-typedef JSValue* HandleSlot;
+class OpaqueJSValue : private JSValue {
+public:
+    JSValue& toJSValue() { return *this; }
+    JSValue* toJSValueRef() { return this; }
+    void fromJSValue(const JSValue& value) { *this = static_cast<const OpaqueJSValue&>(value); }
+};
+typedef OpaqueJSValue* HandleSlot;
 
 template <typename T> struct JSValueChecker {
     static const bool IsJSValue = false;