From bebb53fb341c21f48e698512e581bf201eef216e Mon Sep 17 00:00:00 2001
From: "antti@apple.com" <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed, 10 Dec 2014 20:46:15 +0000
Subject: [PATCH] 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
---
 LayoutTests/ChangeLog                                   | 11 +++++++++++
 .../fast/css/calc-binary-operation-crash-expected.txt   |  3 +++
 LayoutTests/fast/css/calc-binary-operation-crash.html   |  6 ++++++
 Source/WebCore/ChangeLog                                | 17 +++++++++++++++++
 Source/WebCore/css/CSSCalculationValue.cpp              | 14 ++++++++------
 5 files changed, 45 insertions(+), 6 deletions(-)
 create mode 100644 LayoutTests/fast/css/calc-binary-operation-crash-expected.txt
 create mode 100644 LayoutTests/fast/css/calc-binary-operation-crash.html

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index e4af9bd..f65b646 100644
--- 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
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
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
--- 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
--- 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);
 
-- 
1.8.3.1