Title: [164268] trunk
- Revision
- 164268
- Author
- [email protected]
- Date
- 2014-02-17 20:10:04 -0800 (Mon, 17 Feb 2014)
Log Message
SelectorCompiler incorrectly saves a backtracking register for a child chain without descendant relation on the right
https://bugs.webkit.org/show_bug.cgi?id=128944
Patch by Benjamin Poulain <[email protected]> on 2014-02-17
Reviewed by Andreas Kling.
Source/WebCore:
When resolving the backtracking relations, the value of ancestorPositionSinceDescendantRelation was incorrect for the
rightmost child chain.
What was happenning is updateChainStates() would increment ancestorPositionSinceDescendantRelation even if there was
no descendant relation previously in the chain. As a result, the second SelectorFragment in the fragment chain would
save a backtracking register.
Previously this would just be a wasted register but since r163850, the number of registers available for compilation
is defined by SelectorCompiler::minimumRegisterRequirements(). Since we would have one less register available than computed,
we could run out of register and RegisterAllocator would invoke WTFCrash to avoid generating incorrect code.
This patch fixes the issue by not updating ancestorPositionSinceDescendantRelation until the first descendant relation
is seen. There was no need to fix the Adjacent relation because adjacentPositionSinceIndirectAdjacentTreeWalk already
had the correct guard.
Test: fast/selectors/querySelector-rightmost-child-chain-attribute-matching.html
* cssjit/SelectorCompiler.cpp:
(WebCore::SelectorCompiler::updateChainStates):
(WebCore::SelectorCompiler::isFirstAdjacent): The name was a bad copy-paste, fix it.
LayoutTests:
* fast/selectors/querySelector-rightmost-child-chain-attribute-matching-expected.txt: Added.
* fast/selectors/querySelector-rightmost-child-chain-attribute-matching.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (164267 => 164268)
--- trunk/LayoutTests/ChangeLog 2014-02-18 03:50:41 UTC (rev 164267)
+++ trunk/LayoutTests/ChangeLog 2014-02-18 04:10:04 UTC (rev 164268)
@@ -1,3 +1,13 @@
+2014-02-17 Benjamin Poulain <[email protected]>
+
+ SelectorCompiler incorrectly saves a backtracking register for a child chain without descendant relation on the right
+ https://bugs.webkit.org/show_bug.cgi?id=128944
+
+ Reviewed by Andreas Kling.
+
+ * fast/selectors/querySelector-rightmost-child-chain-attribute-matching-expected.txt: Added.
+ * fast/selectors/querySelector-rightmost-child-chain-attribute-matching.html: Added.
+
2014-02-17 Dean Jackson <[email protected]>
Constrain replaced element layout to from-intrinsic aspect ratio if specified
Added: trunk/LayoutTests/fast/selectors/querySelector-rightmost-child-chain-attribute-matching-expected.txt (0 => 164268)
--- trunk/LayoutTests/fast/selectors/querySelector-rightmost-child-chain-attribute-matching-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/selectors/querySelector-rightmost-child-chain-attribute-matching-expected.txt 2014-02-18 04:10:04 UTC (rev 164268)
@@ -0,0 +1,11 @@
+The CSS JIT was incorrectly saving a backtracking entry point for the rightmost chain of simple selectors with child relation.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.querySelectorAll("[data-webkit][data-rocks]>[data-when]>[data-it-does-not-crash]").length is 1
+PASS document.querySelectorAll("[data-webkit][data-rocks]>[data-when]>[data-it-does-not-crash]")[0].id is "target"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/selectors/querySelector-rightmost-child-chain-attribute-matching.html (0 => 164268)
--- trunk/LayoutTests/fast/selectors/querySelector-rightmost-child-chain-attribute-matching.html (rev 0)
+++ trunk/LayoutTests/fast/selectors/querySelector-rightmost-child-chain-attribute-matching.html 2014-02-18 04:10:04 UTC (rev 164268)
@@ -0,0 +1,42 @@
+<!doctype html>
+<html>
+<head>
+<style>
+[data-webkit][data-rocks]>[data-when]>[data-it-does-not-crash] {
+ background-color:green;
+}
+</style>
+<script src=""
+</head>
+<body>
+<div style="display:none">
+ <div data-rocks data-webkit>
+ <ul data-when>
+ <li></li>
+ <li data-it-does-not-crash id=target></li>
+ <li></li>
+ </ul>
+ <!-- Lacks the data-when -->
+ <ul>
+ <li data-it-does-not-crash></li>
+ <li></li>
+ </ul>
+ </div>
+ <!-- Lacks the data-webkit -->
+ <div data-rocks>
+ <ul data-when>
+ <li></li>
+ <li data-it-does-not-crash></li>
+ <li></li>
+ </ul>
+ </div>
+</div>
+</body>
+<script>
+description('The CSS JIT was incorrectly saving a backtracking entry point for the rightmost chain of simple selectors with child relation.');
+
+shouldBe('document.querySelectorAll("[data-webkit][data-rocks]>[data-when]>[data-it-does-not-crash]").length', '1');
+shouldBeEqualToString('document.querySelectorAll("[data-webkit][data-rocks]>[data-when]>[data-it-does-not-crash]")[0].id', 'target');
+</script>
+<script src=""
+</html>
Modified: trunk/Source/WebCore/ChangeLog (164267 => 164268)
--- trunk/Source/WebCore/ChangeLog 2014-02-18 03:50:41 UTC (rev 164267)
+++ trunk/Source/WebCore/ChangeLog 2014-02-18 04:10:04 UTC (rev 164268)
@@ -1,3 +1,30 @@
+2014-02-17 Benjamin Poulain <[email protected]>
+
+ SelectorCompiler incorrectly saves a backtracking register for a child chain without descendant relation on the right
+ https://bugs.webkit.org/show_bug.cgi?id=128944
+
+ Reviewed by Andreas Kling.
+
+ When resolving the backtracking relations, the value of ancestorPositionSinceDescendantRelation was incorrect for the
+ rightmost child chain.
+ What was happenning is updateChainStates() would increment ancestorPositionSinceDescendantRelation even if there was
+ no descendant relation previously in the chain. As a result, the second SelectorFragment in the fragment chain would
+ save a backtracking register.
+
+ Previously this would just be a wasted register but since r163850, the number of registers available for compilation
+ is defined by SelectorCompiler::minimumRegisterRequirements(). Since we would have one less register available than computed,
+ we could run out of register and RegisterAllocator would invoke WTFCrash to avoid generating incorrect code.
+
+ This patch fixes the issue by not updating ancestorPositionSinceDescendantRelation until the first descendant relation
+ is seen. There was no need to fix the Adjacent relation because adjacentPositionSinceIndirectAdjacentTreeWalk already
+ had the correct guard.
+
+ Test: fast/selectors/querySelector-rightmost-child-chain-attribute-matching.html
+
+ * cssjit/SelectorCompiler.cpp:
+ (WebCore::SelectorCompiler::updateChainStates):
+ (WebCore::SelectorCompiler::isFirstAdjacent): The name was a bad copy-paste, fix it.
+
2014-02-17 Dean Jackson <[email protected]>
Constrain replaced element layout to from-intrinsic aspect ratio if specified
Modified: trunk/Source/WebCore/cssjit/SelectorCompiler.cpp (164267 => 164268)
--- trunk/Source/WebCore/cssjit/SelectorCompiler.cpp 2014-02-18 03:50:41 UTC (rev 164267)
+++ trunk/Source/WebCore/cssjit/SelectorCompiler.cpp 2014-02-18 04:10:04 UTC (rev 164268)
@@ -446,7 +446,8 @@
hasIndirectAdjacentRelationOnTheRightOfDirectAdjacentChain = false;
break;
case FragmentRelation::Child:
- ++ancestorPositionSinceDescendantRelation;
+ if (hasDescendantRelationOnTheRight)
+ ++ancestorPositionSinceDescendantRelation;
hasIndirectAdjacentRelationOnTheRightOfDirectAdjacentChain = false;
break;
case FragmentRelation::DirectAdjacent:
@@ -464,9 +465,9 @@
return ancestorPositionSinceDescendantRelation == 1;
}
-static inline bool isFirstAdjacent(unsigned ancestorPositionSinceDescendantRelation)
+static inline bool isFirstAdjacent(unsigned adjacentPositionSinceIndirectAdjacentTreeWalk)
{
- return ancestorPositionSinceDescendantRelation == 1;
+ return adjacentPositionSinceIndirectAdjacentTreeWalk == 1;
}
static inline bool isAfterChildRelation(unsigned ancestorPositionSinceDescendantRelation)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
