Crash when creating CSSCalcBinaryOperation

https://bugs.webkit.org/show_bug.cgi?id=134886
rdar://problem/17663561

Reviewed by Chris Dumez.

Source/WebCore:

Test: fast/css/calc-binary-operation-crash.html

* css/CSSCalculationValue.cpp:
(WebCore::determineCategory):

Ensure that both axis are within the addSubtractResult table.
Remove unneeded CalcOther test. The call site guarantees it doesn't happen and the normal cases would handle it anyway.
Also strengthen some asserts.

LayoutTests:

* fast/css/calc-binary-operation-crash-expected.txt: Added.
* fast/css/calc-binary-operation-crash.html: Added.

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index e4af9bd..f65b646 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2014-12-10  Antti Koivisto  <antti@apple.com>
+
+        Crash when creating CSSCalcBinaryOperation
+        https://bugs.webkit.org/show_bug.cgi?id=134886
+        rdar://problem/17663561
+
+        Reviewed by Chris Dumez.
+
+        * fast/css/calc-binary-operation-crash-expected.txt: Added.
+        * fast/css/calc-binary-operation-crash.html: Added.
+
 2014-12-10  Joanmarie Diggs  <jdiggs@igalia.com>
 
         AX: [ATK] MathML tokens with text fail to expose their text content via AtkText

diff --git a/LayoutTests/fast/css/calc-binary-operation-crash-expected.txt b/LayoutTests/fast/css/calc-binary-operation-crash-expected.txt
new file mode 100644 (file)
index 0000000..81630ed
--- /dev/null
+++ b/LayoutTests/fast/css/calc-binary-operation-crash-expected.txt
@@ -0,0 +1,3 @@
+This test passes if it doesn't crash.
+
+X

diff --git a/LayoutTests/fast/css/calc-binary-operation-crash.html b/LayoutTests/fast/css/calc-binary-operation-crash.html
new file mode 100644 (file)
index 0000000..fb451b5
--- /dev/null
+++ b/LayoutTests/fast/css/calc-binary-operation-crash.html
@@ -0,0 +1,6 @@
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+<p>This test passes if it doesn't crash.</p>
+<p style="text-indent: calc(25s - 3px + 12.5%*2)">X</p>

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 10497bc..f6aa200 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,20 @@
+2014-12-10  Antti Koivisto  <antti@apple.com>
+
+        Crash when creating CSSCalcBinaryOperation
+        https://bugs.webkit.org/show_bug.cgi?id=134886
+        rdar://problem/17663561
+
+        Reviewed by Chris Dumez.
+
+        Test: fast/css/calc-binary-operation-crash.html
+
+        * css/CSSCalculationValue.cpp:
+        (WebCore::determineCategory):
+
+        Ensure that both axis are within the addSubtractResult table.
+        Remove unneeded CalcOther test. The call site guarantees it doesn't happen and the normal cases would handle it anyway.
+        Also strengthen some asserts.
+
 2014-12-10  Anders Carlsson  <andersca@apple.com>
 
         Add WebStorageNamespaceProvider::closeLocalStorage

diff --git a/Source/WebCore/css/CSSCalculationValue.cpp b/Source/WebCore/css/CSSCalculationValue.cpp
index 5a9041d..fa995f6 100644 (file)
--- a/Source/WebCore/css/CSSCalculationValue.cpp
+++ b/Source/WebCore/css/CSSCalculationValue.cpp
@@ -242,6 +242,7 @@ private:
         case CalcOther:
             ASSERT_NOT_REACHED();
         }
+        ASSERT_NOT_REACHED();
         return nullptr;
     }
 
@@ -311,14 +312,13 @@ static CalculationCategory determineCategory(const CSSCalcExpressionNode& leftSi
 {
     CalculationCategory leftCategory = leftSide.category();
     CalculationCategory rightCategory = rightSide.category();
-
-    if (leftCategory == CalcOther || rightCategory == CalcOther)
-        return CalcOther;
+    ASSERT(leftCategory < CalcOther);
+    ASSERT(rightCategory < CalcOther);
 
     switch (op) {
     case CalcAdd:
     case CalcSubtract:
-        if (leftCategory < CalcAngle || rightCategory < CalcAngle)
+        if (leftCategory < CalcAngle && rightCategory < CalcAngle)
             return addSubtractResult[leftCategory][rightCategory];
         if (leftCategory == rightCategory)
             return leftCategory;
@@ -349,7 +349,8 @@ class CSSCalcBinaryOperation final : public CSSCalcExpressionNode {
 public:
     static PassRefPtr<CSSCalcBinaryOperation> create(CalcOperator op, PassRefPtr<CSSCalcExpressionNode> leftSide, PassRefPtr<CSSCalcExpressionNode> rightSide)
     {
-        ASSERT(leftSide->category() != CalcOther && rightSide->category() != CalcOther);
+        ASSERT(leftSide->category() < CalcOther);
+        ASSERT(rightSide->category() < CalcOther);
 
         CalculationCategory newCategory = determineCategory(*leftSide, *rightSide, op);
 
@@ -363,7 +364,8 @@ public:
     {
         CalculationCategory leftCategory = leftSide->category();
         CalculationCategory rightCategory = rightSide->category();
-        ASSERT(leftCategory != CalcOther && rightCategory != CalcOther);
+        ASSERT(leftCategory < CalcOther);
+        ASSERT(rightCategory < CalcOther);
 
         bool isInteger = isIntegerResult(op, *leftSide, *rightSide);