Title: [101639] trunk/Source/_javascript_Core
Revision
101639
Author
fpi...@apple.com
Date
2011-12-01 01:05:16 -0800 (Thu, 01 Dec 2011)

Log Message

BitVector isInline check could fail
https://bugs.webkit.org/show_bug.cgi?id=70691

Reviewed by Gavin Barraclough.
        
Switch back to using the high bit as the inline marker, to make
all of the bit indexing operations simpler. Computing the size in
words and in bytes of a bitvector, using the number of bits as
input is error-prone enough; and with the current approach to
solving the X86 bug we end up getting it wrong. Making it right
seems hard.
        
So instead, to solve the original problem (the high bit may be
meaningful on 32-bit systems), the out-of-line storage pointer is
right-shifted by 1. Compared to the original BitVector code, this
is a much smaller change (just three lines).
        
This solves a bug where the DFG was corrupting its call frame
because BitVector lost track of some bits.

* wtf/BitVector.cpp:
(WTF::BitVector::setSlow):
(WTF::BitVector::resizeOutOfLine):
* wtf/BitVector.h:
(WTF::BitVector::quickGet):
(WTF::BitVector::quickSet):
(WTF::BitVector::quickClear):
(WTF::BitVector::makeInlineBits):
(WTF::BitVector::isInline):
(WTF::BitVector::outOfLineBits):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (101638 => 101639)


--- trunk/Source/_javascript_Core/ChangeLog	2011-12-01 09:02:53 UTC (rev 101638)
+++ trunk/Source/_javascript_Core/ChangeLog	2011-12-01 09:05:16 UTC (rev 101639)
@@ -1,3 +1,36 @@
+2011-12-01  Filip Pizlo  <fpi...@apple.com>
+
+        BitVector isInline check could fail
+        https://bugs.webkit.org/show_bug.cgi?id=70691
+
+        Reviewed by Gavin Barraclough.
+        
+        Switch back to using the high bit as the inline marker, to make
+        all of the bit indexing operations simpler. Computing the size in
+        words and in bytes of a bitvector, using the number of bits as
+        input is error-prone enough; and with the current approach to
+        solving the X86 bug we end up getting it wrong. Making it right
+        seems hard.
+        
+        So instead, to solve the original problem (the high bit may be
+        meaningful on 32-bit systems), the out-of-line storage pointer is
+        right-shifted by 1. Compared to the original BitVector code, this
+        is a much smaller change (just three lines).
+        
+        This solves a bug where the DFG was corrupting its call frame
+        because BitVector lost track of some bits.
+
+        * wtf/BitVector.cpp:
+        (WTF::BitVector::setSlow):
+        (WTF::BitVector::resizeOutOfLine):
+        * wtf/BitVector.h:
+        (WTF::BitVector::quickGet):
+        (WTF::BitVector::quickSet):
+        (WTF::BitVector::quickClear):
+        (WTF::BitVector::makeInlineBits):
+        (WTF::BitVector::isInline):
+        (WTF::BitVector::outOfLineBits):
+
 2011-11-30  Filip Pizlo  <fpi...@apple.com>
 
         DFG should make it easier to notice node boundaries in disassembly

Modified: trunk/Source/_javascript_Core/wtf/BitVector.cpp (101638 => 101639)


--- trunk/Source/_javascript_Core/wtf/BitVector.cpp	2011-12-01 09:02:53 UTC (rev 101638)
+++ trunk/Source/_javascript_Core/wtf/BitVector.cpp	2011-12-01 09:05:16 UTC (rev 101639)
@@ -42,7 +42,7 @@
     else {
         OutOfLineBits* newOutOfLineBits = OutOfLineBits::create(other.size());
         memcpy(newOutOfLineBits->bits(), other.bits(), byteCount(other.size()));
-        newBitsOrPointer = bitwise_cast<uintptr_t>(newOutOfLineBits);
+        newBitsOrPointer = bitwise_cast<uintptr_t>(newOutOfLineBits) >> 1;
     }
     if (!isInline())
         OutOfLineBits::destroy(outOfLineBits());
@@ -91,7 +91,7 @@
     OutOfLineBits* newOutOfLineBits = OutOfLineBits::create(numBits);
     if (isInline()) {
         // Make sure that all of the bits are zero in case we do a no-op resize.
-        *newOutOfLineBits->bits() = m_bitsOrPointer & ~(static_cast<uintptr_t>(1));
+        *newOutOfLineBits->bits() = m_bitsOrPointer & ~(static_cast<uintptr_t>(1) << maxInlineBits());
     } else {
         if (numBits > size()) {
             size_t oldNumWords = outOfLineBits()->numWords();
@@ -102,8 +102,7 @@
             memcpy(newOutOfLineBits->bits(), outOfLineBits()->bits(), newOutOfLineBits->numWords() * sizeof(void*));
         OutOfLineBits::destroy(outOfLineBits());
     }
-    m_bitsOrPointer = bitwise_cast<uintptr_t>(newOutOfLineBits);
-    ASSERT(!isInline());
+    m_bitsOrPointer = bitwise_cast<uintptr_t>(newOutOfLineBits) >> 1;
 }
 
 #ifndef NDEBUG

Modified: trunk/Source/_javascript_Core/wtf/BitVector.h (101638 => 101639)


--- trunk/Source/_javascript_Core/wtf/BitVector.h	2011-12-01 09:02:53 UTC (rev 101638)
+++ trunk/Source/_javascript_Core/wtf/BitVector.h	2011-12-01 09:05:16 UTC (rev 101639)
@@ -114,19 +114,19 @@
     bool quickGet(size_t bit) const
     {
         ASSERT(bit < size());
-        return !!(bits()[bit / bitsInPointer()] & (static_cast<uintptr_t>(1) << ((bit & (bitsInPointer() - 1)) + 1)));
+        return !!(bits()[bit / bitsInPointer()] & (static_cast<uintptr_t>(1) << (bit & (bitsInPointer() - 1))));
     }
     
     void quickSet(size_t bit)
     {
         ASSERT(bit < size());
-        bits()[bit / bitsInPointer()] |= (static_cast<uintptr_t>(1) << ((bit & (bitsInPointer() - 1)) + 1));
+        bits()[bit / bitsInPointer()] |= (static_cast<uintptr_t>(1) << (bit & (bitsInPointer() - 1)));
     }
     
     void quickClear(size_t bit)
     {
         ASSERT(bit < size());
-        bits()[bit / bitsInPointer()] &= ~(static_cast<uintptr_t>(1) << ((bit & (bitsInPointer() - 1)) + 1));
+        bits()[bit / bitsInPointer()] &= ~(static_cast<uintptr_t>(1) << (bit & (bitsInPointer() - 1)));
     }
     
     void quickSet(size_t bit, bool value)
@@ -187,8 +187,8 @@
     
     static uintptr_t makeInlineBits(uintptr_t bits)
     {
-        ASSERT(!(bits & static_cast<uintptr_t>(1)));
-        return bits | static_cast<uintptr_t>(1);
+        ASSERT(!(bits & (static_cast<uintptr_t>(1) << maxInlineBits())));
+        return bits | (static_cast<uintptr_t>(1) << maxInlineBits());
     }
     
     class OutOfLineBits {
@@ -211,10 +211,10 @@
         size_t m_numBits;
     };
     
-    bool isInline() const { return m_bitsOrPointer & static_cast<uintptr_t>(1); }
+    bool isInline() const { return m_bitsOrPointer >> maxInlineBits(); }
     
-    const OutOfLineBits* outOfLineBits() const { return bitwise_cast<const OutOfLineBits*>(m_bitsOrPointer); }
-    OutOfLineBits* outOfLineBits() { return bitwise_cast<OutOfLineBits*>(m_bitsOrPointer); }
+    const OutOfLineBits* outOfLineBits() const { return bitwise_cast<const OutOfLineBits*>(m_bitsOrPointer << 1); }
+    OutOfLineBits* outOfLineBits() { return bitwise_cast<OutOfLineBits*>(m_bitsOrPointer << 1); }
     
     void resizeOutOfLine(size_t numBits);
     void setSlow(const BitVector& other);
@@ -232,10 +232,7 @@
             return &m_bitsOrPointer;
         return outOfLineBits()->bits();
     }
-
-    // The low bit of m_bitsOrPointer is a flag indicating whether this field is
-    // inline bits or a pointer to out of line bits. If the flag is set, the field
-    // is inline bits. This works because the low bit in a pointer is always unset.
+    
     uintptr_t m_bitsOrPointer;
 };
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to