Title: [278224] trunk
Revision
278224
Author
rmoris...@apple.com
Date
2021-05-28 13:17:41 -0700 (Fri, 28 May 2021)

Log Message

Fix LikelyDenseUnsignedIntegerSet::clear()
https://bugs.webkit.org/show_bug.cgi?id=226388
JSTests:

rdar://78607433

Reviewed by Mark Lam.

* stress/stack-allocation-regression.js: Added.
(foo):

Source/WTF:

Reviewed by Mark Lam.

There are two problems with it:
1) It calls BitVector::clearAll(), which does not free any memory.
Instead, it should call BitVector::~BitVector(), then do a placement new of a fresh BitVector (to get it back to its inline condition)
2) More problematically, it changes m_size before calling isBitVector() which relies crucially on the value of m_size.
So it is going to believe that it is in BitVector mode even when it is actually in HashSet mode.

* wtf/LikelyDenseUnsignedIntegerSet.h:
(WTF::LikelyDenseUnsignedIntegerSet::clear):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (278223 => 278224)


--- trunk/JSTests/ChangeLog	2021-05-28 20:13:35 UTC (rev 278223)
+++ trunk/JSTests/ChangeLog	2021-05-28 20:17:41 UTC (rev 278224)
@@ -1,3 +1,14 @@
+2021-05-28  Robin Morisset  <rmoris...@apple.com>
+
+        Fix LikelyDenseUnsignedIntegerSet::clear()
+        https://bugs.webkit.org/show_bug.cgi?id=226388
+        rdar://78607433
+
+        Reviewed by Mark Lam.
+
+        * stress/stack-allocation-regression.js: Added.
+        (foo):
+
 2021-05-28  Saam Barati  <sbar...@apple.com>
 
         Don't sink arguments past the context of the inline call frame they were created in

Added: trunk/JSTests/stress/stack-allocation-regression.js (0 => 278224)


--- trunk/JSTests/stress/stack-allocation-regression.js	                        (rev 0)
+++ trunk/JSTests/stress/stack-allocation-regression.js	2021-05-28 20:17:41 UTC (rev 278224)
@@ -0,0 +1,8 @@
+//@ requireOptions("--forceEagerCompilation=1")
+
+function foo() {
+for (let a0 in 'a')
+for (let a1 in 'a')
+for (let i=0; i<1000000000; i++);
+}
+foo();

Modified: trunk/Source/WTF/ChangeLog (278223 => 278224)


--- trunk/Source/WTF/ChangeLog	2021-05-28 20:13:35 UTC (rev 278223)
+++ trunk/Source/WTF/ChangeLog	2021-05-28 20:17:41 UTC (rev 278224)
@@ -1,3 +1,19 @@
+2021-05-28  Robin Morisset  <rmoris...@apple.com>
+
+        Fix LikelyDenseUnsignedIntegerSet::clear()
+        https://bugs.webkit.org/show_bug.cgi?id=226388
+
+        Reviewed by Mark Lam.
+
+        There are two problems with it:
+        1) It calls BitVector::clearAll(), which does not free any memory.
+        Instead, it should call BitVector::~BitVector(), then do a placement new of a fresh BitVector (to get it back to its inline condition)
+        2) More problematically, it changes m_size before calling isBitVector() which relies crucially on the value of m_size.
+        So it is going to believe that it is in BitVector mode even when it is actually in HashSet mode.
+
+        * wtf/LikelyDenseUnsignedIntegerSet.h:
+        (WTF::LikelyDenseUnsignedIntegerSet::clear):
+
 2021-05-28  Sam Weinig  <wei...@apple.com>
 
         Add stub implementation of CA separated portal bits for GraphicsLayer

Modified: trunk/Source/WTF/wtf/LikelyDenseUnsignedIntegerSet.h (278223 => 278224)


--- trunk/Source/WTF/wtf/LikelyDenseUnsignedIntegerSet.h	2021-05-28 20:13:35 UTC (rev 278223)
+++ trunk/Source/WTF/wtf/LikelyDenseUnsignedIntegerSet.h	2021-05-28 20:17:41 UTC (rev 278224)
@@ -77,15 +77,14 @@
 
     void clear()
     {
+        if (isBitVector())
+            m_inline.bitVector.~BitVector();
+        else
+            m_inline.hashSet.~HashSet();
+        new (NotNull, &m_inline.bitVector) BitVector;
         m_size = 0;
         m_min = 0;
         m_max = 0;
-        if (isBitVector())
-            m_inline.bitVector.clearAll();
-        else {
-            m_inline.hashSet.~HashSet();
-            new (NotNull, &m_inline.bitVector) BitVector;
-        }
     }
 
     bool contains(IndexType value) const
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to