Changeset 63747 in webkit

Timestamp:

Jul 20, 2010 9:16:10 AM (14 years ago)

Author:

commit-queue@webkit.org

Message:

2010-07-20 Hayato Ito <​hayato@chromium.org>

Reviewed by Darin Adler.

Fixed a crash when deeply nested CSS selector is used.
​https://bugs.webkit.org/show_bug.cgi?id=41129

This patch deletes CSSSelectors iteratively so that it doesn't cause stack overflow.

• fast/css/css-selector-deeply-nested-expected.txt: Added.
• fast/css/css-selector-deeply-nested.html: Added.

2010-07-20 Hayato Ito <​hayato@chromium.org>

Reviewed by Darin Adler.

Fixed a crash when deeply nested CSS selector is used.
​https://bugs.webkit.org/show_bug.cgi?id=41129

This patch deletes CSSSelectors iteratively so that it doesn't cause stack overflow.

Test: fast/css/css-selector-deeply-nested.html

• css/CSSSelector.cpp: (WebCore::CSSSelectorBag::~CSSSelectorBag): (WebCore::CSSSelectorBag::isEmpty): (WebCore::CSSSelectorBag::append): (WebCore::CSSSelectorBag::takeAny): (WebCore::CSSSelector::~CSSSelector): (WebCore::CSSSelector::specificity):
• css/CSSSelector.h:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css/css-selector-deeply-nested-expected.txt (added)
• LayoutTests/fast/css/css-selector-deeply-nested.html (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/css/CSSSelector.cpp (modified) (3 diffs)
• WebCore/css/CSSSelector.h (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2010-07-20  Hayato Ito  <hayato@chromium.org>
+
+        Reviewed by Darin Adler.
+
+        Fixed a crash when deeply nested CSS selector is used.
+        https://bugs.webkit.org/show_bug.cgi?id=41129
+
+        This patch deletes CSSSelectors iteratively so that it doesn't cause stack overflow.
+
+        * fast/css/css-selector-deeply-nested-expected.txt: Added.
+        * fast/css/css-selector-deeply-nested.html: Added.
+
 2010-07-20  Stephen White  <senorblanco@chromium.org>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-07-20  Hayato Ito  <hayato@chromium.org>
+
+        Reviewed by Darin Adler.
+
+        Fixed a crash when deeply nested CSS selector is used.
+        https://bugs.webkit.org/show_bug.cgi?id=41129
+
+        This patch deletes CSSSelectors iteratively so that it doesn't cause stack overflow.
+
+        Test: fast/css/css-selector-deeply-nested.html
+
+        * css/CSSSelector.cpp:
+        (WebCore::CSSSelectorBag::~CSSSelectorBag):
+        (WebCore::CSSSelectorBag::isEmpty):
+        (WebCore::CSSSelectorBag::append):
+        (WebCore::CSSSelectorBag::takeAny):
+        (WebCore::CSSSelector::~CSSSelector):
+        (WebCore::CSSSelector::specificity):
+        * css/CSSSelector.h:
+
 2010-07-20  Yury Semikhatsky  <yurys@chromium.org>
 

trunk/WebCore/css/CSSSelector.cpp

@@ -32 +32 @@
 #include <wtf/HashMap.h>
 #include <wtf/StdLibExtras.h>
+#include <wtf/Vector.h>
 
 namespace WebCore {
     
 using namespace HTMLNames;
+
+// A helper class to hold CSSSelectors.
+class CSSSelectorBag : public Noncopyable {
+public:
+    ~CSSSelectorBag()
+    {
+        deleteAllValues(m_stack);
+    }
+
+    bool isEmpty() const
+    {
+        return m_stack.isEmpty();
+    }
+
+    void append(PassOwnPtr<CSSSelector> selector)
+    {
+        if (selector)
+            m_stack.append(selector.leakPtr());
+    }
+
+    PassOwnPtr<CSSSelector> takeAny()
+    {
+        ASSERT(!isEmpty());
+        OwnPtr<CSSSelector> selector = adoptPtr(m_stack.last());
+        m_stack.removeLast();
+        return selector.release();
+    }
+
+private:
+    Vector<CSSSelector*, 16> m_stack;
+};
+
+CSSSelector::~CSSSelector()
+{
+    // We should avoid a recursive destructor call, which causes stack overflow
+    // if CSS Selectors are deeply nested.
+
+    // Early exit if we have already processed the children of this selector.
+    if (m_hasRareData) {
+        if (!m_data.m_rareData)
+            return;
+    } else if (!m_data.m_tagHistory)
+        return;
+
+    CSSSelectorBag selectorsToBeDeleted;
+    if (m_hasRareData) {
+        selectorsToBeDeleted.append(m_data.m_rareData->m_tagHistory.release());
+        selectorsToBeDeleted.append(m_data.m_rareData->m_simpleSelector.release());
+        delete m_data.m_rareData;
+    } else
+        selectorsToBeDeleted.append(adoptPtr(m_data.m_tagHistory));
+
+    // Traverse the tree of CSSSelector and delete each CSSSelector iteratively.
+    while (!selectorsToBeDeleted.isEmpty()) {
+        OwnPtr<CSSSelector> selector(selectorsToBeDeleted.takeAny());
+        ASSERT(selector);
+        if (selector->m_hasRareData) {
+            ASSERT(selector->m_data.m_rareData);
+            selectorsToBeDeleted.append(selector->m_data.m_rareData->m_tagHistory.release());
+            selectorsToBeDeleted.append(selector->m_data.m_rareData->m_simpleSelector.release());
+            delete selector->m_data.m_rareData;
+            // Clear the pointer so that a destructor of the selector, which is
+            // about to be called, can know the children are already processed.
+            selector->m_data.m_rareData = 0;
+        } else {
+            selectorsToBeDeleted.append(adoptPtr(selector->m_data.m_tagHistory));
+            // Clear the pointer for the same reason.
+            selector->m_data.m_tagHistory = 0;
+        }
+    }
+}
 
 unsigned int CSSSelector::specificity()
@@ -64 +136 @@
     }
 
+    // FIXME: Avoid recursive calls to prevent possible stack overflow.
     if (CSSSelector* tagHistory = this->tagHistory())
         s += tagHistory->specificity();
@@ -907 +980 @@
     }
 }
-    
+
 } // namespace WebCore

trunk/WebCore/css/CSSSelector.h

@@ -23 +23 @@
 #define CSSSelector_h
 
+#include "QualifiedName.h"
 #include "RenderStyleConstants.h"
-#include "QualifiedName.h"
 #include <wtf/Noncopyable.h>
 #include <wtf/OwnPtr.h>
@@ -58 +58 @@
         }
 
-        ~CSSSelector()
-        {
-            if (m_hasRareData)
-                delete m_data.m_rareData;
-            else
-                delete m_data.m_tagHistory;
-        }
+        ~CSSSelector();
 
         /**