- Revision
- 116914
- Author
- [email protected]
- Date
- 2012-05-13 20:30:46 -0700 (Sun, 13 May 2012)
Log Message
Heap-use-after-free in WTF::HashMap<int, WTF::RefPtr<WebCore::CalculationValue>, WTF::IntHash<unsigned int>, WTF::HashTrait
https://bugs.webkit.org/show_bug.cgi?id=85195
Source/WebCore:
This bug was caused by Length not understanding that calc expressions shouldn't be
blended - a Length with a calc _expression_ handle was created without incrementing
the ref count of the _expression_. Length no longer attempts to blend calc expressions,
http://webkit.org/b/86160 has been filed to track _expression_ blending. Fixing this fixed
the crash.
Once this was fixed, the RenderStyle diff checker thought the style was changing,
as Length didn't know how to compare calc expressions, resulting in an infinite
loop of style recalcs. Expressions can now compare themselves.
Reviewed by Darin Adler.
Tests: css3/calc/transition-crash.html
css3/calc/transition-crash2.html
* platform/CalculationValue.h:
(WebCore::CalcExpressionNode::CalcExpressionNode):
(CalcExpressionNode):
(WebCore::CalcExpressionNode::type):
(CalculationValue):
(WebCore::CalculationValue::operator==):
(WebCore::CalcExpressionNumber::CalcExpressionNumber):
(WebCore::CalcExpressionNumber::operator==):
(CalcExpressionNumber):
(WebCore::CalcExpressionLength::CalcExpressionLength):
(WebCore::CalcExpressionLength::operator==):
(CalcExpressionLength):
(WebCore::CalcExpressionBinaryOperation::CalcExpressionBinaryOperation):
(WebCore::CalcExpressionBinaryOperation::operator==):
(CalcExpressionBinaryOperation):
* platform/Length.cpp:
(WebCore::Length::isCalculatedEqual):
(WebCore):
* platform/Length.h:
(WebCore::Length::operator==):
(Length):
(WebCore::Length::blend):
LayoutTests:
Reviewed by Darin Adler.
* css3/calc/transition-crash-expected.txt: Added.
* css3/calc/transition-crash.html: Added.
* css3/calc/transition-crash2-expected.txt: Added.
* css3/calc/transition-crash2.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (116913 => 116914)
--- trunk/LayoutTests/ChangeLog 2012-05-14 03:23:54 UTC (rev 116913)
+++ trunk/LayoutTests/ChangeLog 2012-05-14 03:30:46 UTC (rev 116914)
@@ -1,3 +1,15 @@
+2012-05-13 Mike Lawther <[email protected]>
+
+ Heap-use-after-free in WTF::HashMap<int, WTF::RefPtr<WebCore::CalculationValue>, WTF::IntHash<unsigned int>, WTF::HashTrait
+ https://bugs.webkit.org/show_bug.cgi?id=85195
+
+ Reviewed by Darin Adler.
+
+ * css3/calc/transition-crash-expected.txt: Added.
+ * css3/calc/transition-crash.html: Added.
+ * css3/calc/transition-crash2-expected.txt: Added.
+ * css3/calc/transition-crash2.html: Added.
+
2012-05-13 Philippe Normand <[email protected]>
Unreviewed GTK test_expectations update.
Added: trunk/LayoutTests/css3/calc/transition-crash-expected.txt (0 => 116914)
--- trunk/LayoutTests/css3/calc/transition-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/css3/calc/transition-crash-expected.txt 2012-05-14 03:30:46 UTC (rev 116914)
@@ -0,0 +1,2 @@
+This test checks class changes affecting sibling selectors happening during transitions over calculated lengths. The test passes if it does not crash.
+
Added: trunk/LayoutTests/css3/calc/transition-crash.html (0 => 116914)
--- trunk/LayoutTests/css3/calc/transition-crash.html (rev 0)
+++ trunk/LayoutTests/css3/calc/transition-crash.html 2012-05-14 03:30:46 UTC (rev 116914)
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<style>
+.transition {
+ height: -webkit-calc(100% - 10px);
+ -webkit-transition: height 50ms;
+}
+.flim + .sibling {
+}
+</style>
+
+<body>
+ This test checks class changes affecting sibling selectors happening during transitions over calculated lengths.
+ The test passes if it does not crash.
+</body>
+
+<script>
+ if (window.layoutTestController) {
+ layoutTestController.dumpAsText(true);
+ layoutTestController.waitUntilDone();
+ }
+
+ div = document.createElement('div');
+ div.setAttribute('class', 'sibling');
+ document.body.appendChild(div);
+
+ th = document.createElement('th'); // td, tr also cause test to fail
+ th.setAttribute('class', 'transition');
+ document.body.appendChild(th);
+
+ function boom() {
+ div.setAttribute('class', 'stix');
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+ }
+ setTimeout(boom, 1);
+</script>
Added: trunk/LayoutTests/css3/calc/transition-crash2-expected.txt (0 => 116914)
--- trunk/LayoutTests/css3/calc/transition-crash2-expected.txt (rev 0)
+++ trunk/LayoutTests/css3/calc/transition-crash2-expected.txt 2012-05-14 03:30:46 UTC (rev 116914)
@@ -0,0 +1,2 @@
+This tests transitioning of elements containing a calc _expression_. The test passes if it does not crash.
+
Added: trunk/LayoutTests/css3/calc/transition-crash2.html (0 => 116914)
--- trunk/LayoutTests/css3/calc/transition-crash2.html (rev 0)
+++ trunk/LayoutTests/css3/calc/transition-crash2.html 2012-05-14 03:30:46 UTC (rev 116914)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+This tests transitioning of elements containing a calc _expression_. The test passes if it does not crash.
+<div>
+ <div style="width: -webkit-calc(-100px + 100%);"></div>
+</div>
+<script>
+ if (window.layoutTestController) {
+ layoutTestController.dumpAsText(true);
+ layoutTestController.waitUntilDone();
+ }
+
+ function boom() {
+ head = document.getElementsByTagName("head")[0];
+ var style = document.createElement("style");
+ style.innerHTML=":last-child {-webkit-transition-duration:.1s;}";
+ head.appendChild(style);
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+ }
+
+ setTimeout(boom, 10);
+</script>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (116913 => 116914)
--- trunk/Source/WebCore/ChangeLog 2012-05-14 03:23:54 UTC (rev 116913)
+++ trunk/Source/WebCore/ChangeLog 2012-05-14 03:30:46 UTC (rev 116914)
@@ -1,3 +1,46 @@
+2012-05-13 Mike Lawther <[email protected]>
+
+ Heap-use-after-free in WTF::HashMap<int, WTF::RefPtr<WebCore::CalculationValue>, WTF::IntHash<unsigned int>, WTF::HashTrait
+ https://bugs.webkit.org/show_bug.cgi?id=85195
+
+ This bug was caused by Length not understanding that calc expressions shouldn't be
+ blended - a Length with a calc _expression_ handle was created without incrementing
+ the ref count of the _expression_. Length no longer attempts to blend calc expressions,
+ http://webkit.org/b/86160 has been filed to track _expression_ blending. Fixing this fixed
+ the crash.
+
+ Once this was fixed, the RenderStyle diff checker thought the style was changing,
+ as Length didn't know how to compare calc expressions, resulting in an infinite
+ loop of style recalcs. Expressions can now compare themselves.
+
+ Reviewed by Darin Adler.
+
+ Tests: css3/calc/transition-crash.html
+ css3/calc/transition-crash2.html
+
+ * platform/CalculationValue.h:
+ (WebCore::CalcExpressionNode::CalcExpressionNode):
+ (CalcExpressionNode):
+ (WebCore::CalcExpressionNode::type):
+ (CalculationValue):
+ (WebCore::CalculationValue::operator==):
+ (WebCore::CalcExpressionNumber::CalcExpressionNumber):
+ (WebCore::CalcExpressionNumber::operator==):
+ (CalcExpressionNumber):
+ (WebCore::CalcExpressionLength::CalcExpressionLength):
+ (WebCore::CalcExpressionLength::operator==):
+ (CalcExpressionLength):
+ (WebCore::CalcExpressionBinaryOperation::CalcExpressionBinaryOperation):
+ (WebCore::CalcExpressionBinaryOperation::operator==):
+ (CalcExpressionBinaryOperation):
+ * platform/Length.cpp:
+ (WebCore::Length::isCalculatedEqual):
+ (WebCore):
+ * platform/Length.h:
+ (WebCore::Length::operator==):
+ (Length):
+ (WebCore::Length::blend):
+
2012-05-13 Darin Adler <[email protected]>
Roll out local changes accidentally landed in r116905.
Modified: trunk/Source/WebCore/platform/CalculationValue.h (116913 => 116914)
--- trunk/Source/WebCore/platform/CalculationValue.h 2012-05-14 03:23:54 UTC (rev 116913)
+++ trunk/Source/WebCore/platform/CalculationValue.h 2012-05-14 03:30:46 UTC (rev 116914)
@@ -51,20 +51,43 @@
CalculationRangeAll,
CalculationRangeNonNegative
};
+
+enum CalcExpressionNodeType {
+ CalcExpressionNodeUndefined,
+ CalcExpressionNodeNumber,
+ CalcExpressionNodeLength,
+ CalcExpressionNodeBinaryOperation
+};
class CalcExpressionNode {
public:
+ CalcExpressionNode()
+ : m_type(CalcExpressionNodeUndefined)
+ {
+ }
+
virtual ~CalcExpressionNode()
{
}
virtual float evaluate(float maxValue) const = 0;
+ virtual bool operator==(const CalcExpressionNode&) const = 0;
+
+ CalcExpressionNodeType type() const { return m_type; }
+
+protected:
+ CalcExpressionNodeType m_type;
};
class CalculationValue : public RefCounted<CalculationValue> {
public:
static PassRefPtr<CalculationValue> create(PassOwnPtr<CalcExpressionNode> value, CalculationPermittedValueRange);
float evaluate(float maxValue) const;
+
+ bool operator==(const CalculationValue& o) const
+ {
+ return m_value == o.m_value || *(m_value.get()) == *(o.m_value.get());
+ }
private:
CalculationValue(PassOwnPtr<CalcExpressionNode> value, CalculationPermittedValueRange range)
@@ -82,8 +105,19 @@
explicit CalcExpressionNumber(float value)
: m_value(value)
{
+ m_type = CalcExpressionNodeNumber;
}
+ bool operator==(const CalcExpressionNumber& o) const
+ {
+ return m_value == o.m_value;
+ }
+
+ virtual bool operator==(const CalcExpressionNode& o) const
+ {
+ return type() == o.type() && *this == static_cast<const CalcExpressionNumber&>(o);
+ }
+
virtual float evaluate(float) const
{
return m_value;
@@ -98,8 +132,19 @@
explicit CalcExpressionLength(Length length)
: m_length(length)
{
+ m_type = CalcExpressionNodeLength;
}
+ bool operator==(const CalcExpressionLength& o) const
+ {
+ return m_length == o.m_length;
+ }
+
+ virtual bool operator==(const CalcExpressionNode& o) const
+ {
+ return type() == o.type() && *this == static_cast<const CalcExpressionLength&>(o);
+ }
+
virtual float evaluate(float maxValue) const
{
return floatValueForLength(m_length, maxValue);
@@ -116,8 +161,20 @@
, m_rightSide(rightSide)
, m_operator(op)
{
+ m_type = CalcExpressionNodeBinaryOperation;
}
+ bool operator==(const CalcExpressionBinaryOperation& o) const
+ {
+ return m_operator == o.m_operator && *m_leftSide == *o.m_leftSide && *m_rightSide == *o.m_rightSide;
+ }
+
+ virtual bool operator==(const CalcExpressionNode& o) const
+ {
+ return type() == o.type() && *this == static_cast<const CalcExpressionBinaryOperation&>(o);
+ }
+
+
virtual float evaluate(float) const;
private:
Modified: trunk/Source/WebCore/platform/Length.cpp (116913 => 116914)
--- trunk/Source/WebCore/platform/Length.cpp 2012-05-14 03:23:54 UTC (rev 116913)
+++ trunk/Source/WebCore/platform/Length.cpp 2012-05-14 03:30:46 UTC (rev 116914)
@@ -232,6 +232,11 @@
return result;
}
+bool Length::isCalculatedEqual(const Length& o) const
+{
+ return isCalculated() && (calculationValue() == o.calculationValue() || *calculationValue() == *o.calculationValue());
+}
+
class SameSizeAsLength {
int32_t value;
int32_t metaData;
Modified: trunk/Source/WebCore/platform/Length.h (116913 => 116914)
--- trunk/Source/WebCore/platform/Length.h 2012-05-14 03:23:54 UTC (rev 116913)
+++ trunk/Source/WebCore/platform/Length.h 2012-05-14 03:30:46 UTC (rev 116914)
@@ -49,21 +49,25 @@
Length(LengthType t)
: m_intValue(0), m_quirk(false), m_type(t), m_isFloat(false)
{
+ ASSERT(t != Calculated);
}
Length(int v, LengthType t, bool q = false)
: m_intValue(v), m_quirk(q), m_type(t), m_isFloat(false)
{
+ ASSERT(t != Calculated);
}
Length(FractionalLayoutUnit v, LengthType t, bool q = false)
: m_floatValue(v.toFloat()), m_quirk(q), m_type(t), m_isFloat(true)
{
+ ASSERT(t != Calculated);
}
Length(float v, LengthType t, bool q = false)
- : m_floatValue(v), m_quirk(q), m_type(t), m_isFloat(true)
+ : m_floatValue(v), m_quirk(q), m_type(t), m_isFloat(true)
{
+ ASSERT(t != Calculated);
}
Length(double v, LengthType t, bool q = false)
@@ -91,7 +95,7 @@
decrementCalculatedRef();
}
- bool operator==(const Length& o) const { return (m_type == o.m_type) && (m_quirk == o.m_quirk) && (isUndefined() || (getFloatValue() == o.getFloatValue())); }
+ bool operator==(const Length& o) const { return (m_type == o.m_type) && (m_quirk == o.m_quirk) && (isUndefined() || (getFloatValue() == o.getFloatValue()) || isCalculatedEqual(o)); }
bool operator!=(const Length& o) const { return !(*this == o); }
const Length& operator*=(float v)
@@ -212,6 +216,7 @@
bool isIntrinsicOrAuto() const { return type() == Auto || type() == MinIntrinsic || type() == Intrinsic; }
bool isSpecified() const { return type() == Fixed || type() == Percent || type() == Calculated || isViewportPercentage(); }
bool isCalculated() const { return type() == Calculated; }
+ bool isCalculatedEqual(const Length&) const;
Length blend(const Length& from, double progress) const
{
@@ -221,6 +226,10 @@
if (from.isZero() && isZero())
return *this;
+
+ // FIXME http://webkit.org/b/86160 - Blending doesn't work with calculated expressions
+ if (type() == Calculated)
+ return *this;
LengthType resultType = type();
if (isZero())
