Title: [121391] trunk
Revision
121391
Author
[email protected]
Date
2012-06-27 17:49:55 -0700 (Wed, 27 Jun 2012)

Log Message

_javascript_ SHA-512 gives wrong hash on second and subsequent runs unless Web Inspector _javascript_ Debugging is on
https://bugs.webkit.org/show_bug.cgi?id=90053
<rdar://problem/11764613>

Source/_javascript_Core: 

Reviewed by Mark Hahnenberg.
        
The problem is that the code was assuming that the recovery should be Undefined if the source of
the SetLocal was !shouldGenerate(). But that's wrong, since the DFG optimizer may skip around a
UInt32ToNumber node (hence making it !shouldGenerate()) and keep the source of that node alive.
In that case we should base the recovery on the source of the UInt32ToNumber. The logic for this
was already in place but the fast check for !shouldGenerate() broke it.

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::computeValueRecoveryFor):

LayoutTests: 

Reviewed by Mark Hahnenberg.

* fast/js/dfg-uint32-to-number-skip-then-exit-expected.txt: Added.
* fast/js/dfg-uint32-to-number-skip-then-exit.html: Added.
* fast/js/script-tests/dfg-uint32-to-number-skip-then-exit.js: Added.
(foo):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (121390 => 121391)


--- trunk/LayoutTests/ChangeLog	2012-06-28 00:48:05 UTC (rev 121390)
+++ trunk/LayoutTests/ChangeLog	2012-06-28 00:49:55 UTC (rev 121391)
@@ -1,3 +1,16 @@
+2012-06-27  Filip Pizlo  <[email protected]>
+
+        _javascript_ SHA-512 gives wrong hash on second and subsequent runs unless Web Inspector _javascript_ Debugging is on
+        https://bugs.webkit.org/show_bug.cgi?id=90053
+        <rdar://problem/11764613>
+
+        Reviewed by Mark Hahnenberg.
+
+        * fast/js/dfg-uint32-to-number-skip-then-exit-expected.txt: Added.
+        * fast/js/dfg-uint32-to-number-skip-then-exit.html: Added.
+        * fast/js/script-tests/dfg-uint32-to-number-skip-then-exit.js: Added.
+        (foo):
+
 2012-06-27  Alpha Lam  <[email protected]>
 
         [chromium] Rebase image mismatch caused by 121371.

Added: trunk/LayoutTests/fast/js/dfg-uint32-to-number-skip-then-exit-expected.txt (0 => 121391)


--- trunk/LayoutTests/fast/js/dfg-uint32-to-number-skip-then-exit-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-uint32-to-number-skip-then-exit-expected.txt	2012-06-28 00:49:55 UTC (rev 121391)
@@ -0,0 +1,209 @@
+This tests that a skipped conversion of uint32 to number does not confuse OSR exit into thinking that the conversion is dead.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS foo(i, 1, o) is 42
+PASS foo(i, 1, o) is 42
+PASS foo(i, 1, o) is 43
+PASS foo(i, 1, o) is 43
+PASS foo(i, 1, o) is 44
+PASS foo(i, 1, o) is 44
+PASS foo(i, 1, o) is 45
+PASS foo(i, 1, o) is 45
+PASS foo(i, 1, o) is 46
+PASS foo(i, 1, o) is 46
+PASS foo(i, 1, o) is 47
+PASS foo(i, 1, o) is 47
+PASS foo(i, 1, o) is 48
+PASS foo(i, 1, o) is 48
+PASS foo(i, 1, o) is 49
+PASS foo(i, 1, o) is 49
+PASS foo(i, 1, o) is 50
+PASS foo(i, 1, o) is 50
+PASS foo(i, 1, o) is 51
+PASS foo(i, 1, o) is 51
+PASS foo(i, 1, o) is 52
+PASS foo(i, 1, o) is 52
+PASS foo(i, 1, o) is 53
+PASS foo(i, 1, o) is 53
+PASS foo(i, 1, o) is 54
+PASS foo(i, 1, o) is 54
+PASS foo(i, 1, o) is 55
+PASS foo(i, 1, o) is 55
+PASS foo(i, 1, o) is 56
+PASS foo(i, 1, o) is 56
+PASS foo(i, 1, o) is 57
+PASS foo(i, 1, o) is 57
+PASS foo(i, 1, o) is 58
+PASS foo(i, 1, o) is 58
+PASS foo(i, 1, o) is 59
+PASS foo(i, 1, o) is 59
+PASS foo(i, 1, o) is 60
+PASS foo(i, 1, o) is 60
+PASS foo(i, 1, o) is 61
+PASS foo(i, 1, o) is 61
+PASS foo(i, 1, o) is 62
+PASS foo(i, 1, o) is 62
+PASS foo(i, 1, o) is 63
+PASS foo(i, 1, o) is 63
+PASS foo(i, 1, o) is 64
+PASS foo(i, 1, o) is 64
+PASS foo(i, 1, o) is 65
+PASS foo(i, 1, o) is 65
+PASS foo(i, 1, o) is 66
+PASS foo(i, 1, o) is 66
+PASS foo(i, 1, o) is 67
+PASS foo(i, 1, o) is 67
+PASS foo(i, 1, o) is 68
+PASS foo(i, 1, o) is 68
+PASS foo(i, 1, o) is 69
+PASS foo(i, 1, o) is 69
+PASS foo(i, 1, o) is 70
+PASS foo(i, 1, o) is 70
+PASS foo(i, 1, o) is 71
+PASS foo(i, 1, o) is 71
+PASS foo(i, 1, o) is 72
+PASS foo(i, 1, o) is 72
+PASS foo(i, 1, o) is 73
+PASS foo(i, 1, o) is 73
+PASS foo(i, 1, o) is 74
+PASS foo(i, 1, o) is 74
+PASS foo(i, 1, o) is 75
+PASS foo(i, 1, o) is 75
+PASS foo(i, 1, o) is 76
+PASS foo(i, 1, o) is 76
+PASS foo(i, 1, o) is 77
+PASS foo(i, 1, o) is 77
+PASS foo(i, 1, o) is 78
+PASS foo(i, 1, o) is 78
+PASS foo(i, 1, o) is 79
+PASS foo(i, 1, o) is 79
+PASS foo(i, 1, o) is 80
+PASS foo(i, 1, o) is 80
+PASS foo(i, 1, o) is 81
+PASS foo(i, 1, o) is 81
+PASS foo(i, 1, o) is 82
+PASS foo(i, 1, o) is 82
+PASS foo(i, 1, o) is 83
+PASS foo(i, 1, o) is 83
+PASS foo(i, 1, o) is 84
+PASS foo(i, 1, o) is 84
+PASS foo(i, 1, o) is 85
+PASS foo(i, 1, o) is 85
+PASS foo(i, 1, o) is 86
+PASS foo(i, 1, o) is 86
+PASS foo(i, 1, o) is 87
+PASS foo(i, 1, o) is 87
+PASS foo(i, 1, o) is 88
+PASS foo(i, 1, o) is 88
+PASS foo(i, 1, o) is 89
+PASS foo(i, 1, o) is 89
+PASS foo(i, 1, o) is 90
+PASS foo(i, 1, o) is 90
+PASS foo(i, 1, o) is 91
+PASS foo(i, 1, o) is 91
+PASS foo(i, 1, o) is 92
+PASS foo(i, 1, o) is 92
+PASS foo(i, 1, o) is 93
+PASS foo(i, 1, o) is 93
+PASS foo(i, 1, o) is 94
+PASS foo(i, 1, o) is 94
+PASS foo(i, 1, o) is 95
+PASS foo(i, 1, o) is 95
+PASS foo(i, 1, o) is 96
+PASS foo(i, 1, o) is 96
+PASS foo(i, 1, o) is 97
+PASS foo(i, 1, o) is 97
+PASS foo(i, 1, o) is 98
+PASS foo(i, 1, o) is 98
+PASS foo(i, 1, o) is 99
+PASS foo(i, 1, o) is 99
+PASS foo(i, 1, o) is 100
+PASS foo(i, 1, o) is 100
+PASS foo(i, 1, o) is 101
+PASS foo(i, 1, o) is 101
+PASS foo(i, 1, o) is 102
+PASS foo(i, 1, o) is 102
+PASS foo(i, 1, o) is 103
+PASS foo(i, 1, o) is 103
+PASS foo(i, 1, o) is 104
+PASS foo(i, 1, o) is 104
+PASS foo(i, 1, o) is 105
+PASS foo(i, 1, o) is 105
+PASS foo(i, 1, o) is 106
+PASS foo(i, 1, o) is 106
+PASS foo(i, 1, o) is 107
+PASS foo(i, 1, o) is 107
+PASS foo(i, 1, o) is 108
+PASS foo(i, 1, o) is 108
+PASS foo(i, 1, o) is 109
+PASS foo(i, 1, o) is 109
+PASS foo(i, 1, o) is 110
+PASS foo(i, 1, o) is 110
+PASS foo(i, 1, o) is 111
+PASS foo(i, 1, o) is 111
+PASS foo(i, 1, o) is 112
+PASS foo(i, 1, o) is 112
+PASS foo(i, 1, o) is 113
+PASS foo(i, 1, o) is 113
+PASS foo(i, 1, o) is 114
+PASS foo(i, 1, o) is 114
+PASS foo(i, 1, o) is 115
+PASS foo(i, 1, o) is 115
+PASS foo(i, 1, o) is 116
+PASS foo(i, 1, o) is 116
+PASS foo(i, 1, o) is 118
+PASS foo(i, 1, o) is 118
+PASS foo(i, 1, o) is 119
+PASS foo(i, 1, o) is 119
+PASS foo(i, 1, o) is 120
+PASS foo(i, 1, o) is 120
+PASS foo(i, 1, o) is 121
+PASS foo(i, 1, o) is 121
+PASS foo(i, 1, o) is 122
+PASS foo(i, 1, o) is 122
+PASS foo(i, 1, o) is 123
+PASS foo(i, 1, o) is 123
+PASS foo(i, 1, o) is 124
+PASS foo(i, 1, o) is 124
+PASS foo(i, 1, o) is 125
+PASS foo(i, 1, o) is 125
+PASS foo(i, 1, o) is 126
+PASS foo(i, 1, o) is 126
+PASS foo(i, 1, o) is 127
+PASS foo(i, 1, o) is 127
+PASS foo(i, 1, o) is 128
+PASS foo(i, 1, o) is 128
+PASS foo(i, 1, o) is 129
+PASS foo(i, 1, o) is 129
+PASS foo(i, 1, o) is 130
+PASS foo(i, 1, o) is 130
+PASS foo(i, 1, o) is 131
+PASS foo(i, 1, o) is 131
+PASS foo(i, 1, o) is 132
+PASS foo(i, 1, o) is 132
+PASS foo(i, 1, o) is 133
+PASS foo(i, 1, o) is 133
+PASS foo(i, 1, o) is 134
+PASS foo(i, 1, o) is 134
+PASS foo(i, 1, o) is 135
+PASS foo(i, 1, o) is 135
+PASS foo(i, 1, o) is 136
+PASS foo(i, 1, o) is 136
+PASS foo(i, 1, o) is 137
+PASS foo(i, 1, o) is 137
+PASS foo(i, 1, o) is 138
+PASS foo(i, 1, o) is 138
+PASS foo(i, 1, o) is 139
+PASS foo(i, 1, o) is 139
+PASS foo(i, 1, o) is 140
+PASS foo(i, 1, o) is 140
+PASS foo(i, 1, o) is 141
+PASS foo(i, 1, o) is 141
+PASS foo(i, 1, o) is 142
+PASS foo(i, 1, o) is 142
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/js/dfg-uint32-to-number-skip-then-exit.html (0 => 121391)


--- trunk/LayoutTests/fast/js/dfg-uint32-to-number-skip-then-exit.html	                        (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-uint32-to-number-skip-then-exit.html	2012-06-28 00:49:55 UTC (rev 121391)
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>

Added: trunk/LayoutTests/fast/js/script-tests/dfg-uint32-to-number-skip-then-exit.js (0 => 121391)


--- trunk/LayoutTests/fast/js/script-tests/dfg-uint32-to-number-skip-then-exit.js	                        (rev 0)
+++ trunk/LayoutTests/fast/js/script-tests/dfg-uint32-to-number-skip-then-exit.js	2012-06-28 00:49:55 UTC (rev 121391)
@@ -0,0 +1,22 @@
+description(
+"This tests that a skipped conversion of uint32 to number does not confuse OSR exit into thinking that the conversion is dead."
+);
+
+function foo(a, b, o) {
+    var x = a >>> b;
+    return o.f + (x | 0);
+}
+
+for (var i = 0; i < 200; ++i) {
+    var o;
+    var expected;
+    if (i < 150) {
+        o = {f:42};
+        expected = 42 + ((i / 2) | 0);
+    } else {
+        o = {f:43, g:44};
+        expected = 43 + ((i / 2) | 0);
+    }
+    shouldBe("foo(i, 1, o)", "" + expected);
+}
+

Modified: trunk/Source/_javascript_Core/ChangeLog (121390 => 121391)


--- trunk/Source/_javascript_Core/ChangeLog	2012-06-28 00:48:05 UTC (rev 121390)
+++ trunk/Source/_javascript_Core/ChangeLog	2012-06-28 00:49:55 UTC (rev 121391)
@@ -1,5 +1,22 @@
 2012-06-27  Filip Pizlo  <[email protected]>
 
+        _javascript_ SHA-512 gives wrong hash on second and subsequent runs unless Web Inspector _javascript_ Debugging is on
+        https://bugs.webkit.org/show_bug.cgi?id=90053
+        <rdar://problem/11764613>
+
+        Reviewed by Mark Hahnenberg.
+        
+        The problem is that the code was assuming that the recovery should be Undefined if the source of
+        the SetLocal was !shouldGenerate(). But that's wrong, since the DFG optimizer may skip around a
+        UInt32ToNumber node (hence making it !shouldGenerate()) and keep the source of that node alive.
+        In that case we should base the recovery on the source of the UInt32ToNumber. The logic for this
+        was already in place but the fast check for !shouldGenerate() broke it.
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::computeValueRecoveryFor):
+
+2012-06-27  Filip Pizlo  <[email protected]>
+
         DFG disassembly should be easier to read
         https://bugs.webkit.org/show_bug.cgi?id=90106
 

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (121390 => 121391)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2012-06-28 00:48:05 UTC (rev 121390)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2012-06-28 00:49:55 UTC (rev 121391)
@@ -1397,14 +1397,12 @@
         if (nodePtr->hasConstant())
             return ValueRecovery::constant(valueOfJSConstant(valueSource.nodeIndex()));
         
-        if (!nodePtr->shouldGenerate()) {
-            // It's legitimately dead. As in, nobody will ever use this node, or operand,
-            // ever. Set it to Undefined to make the GC happy after the OSR.
-            return ValueRecovery::constant(jsUndefined());
-        }
-    
-        GenerationInfo* infoPtr = &m_generationInfo[nodePtr->virtualRegister()];
-        if (!infoPtr->alive() || infoPtr->nodeIndex() != valueSource.nodeIndex()) {
+        GenerationInfo* infoPtr;
+        if (nodePtr->shouldGenerate())
+            infoPtr = &m_generationInfo[nodePtr->virtualRegister()];
+        else
+            infoPtr = 0;
+        if (!infoPtr || !infoPtr->alive() || infoPtr->nodeIndex() != valueSource.nodeIndex()) {
             // Try to see if there is an alternate node that would contain the value we want.
             // There are four possibilities:
             //
@@ -1431,9 +1429,11 @@
             if (nodePtr->op() == UInt32ToNumber || nodePtr->op() == DoubleAsInt32) {
                 NodeIndex nodeIndex = nodePtr->child1().index();
                 nodePtr = &at(nodeIndex);
-                infoPtr = &m_generationInfo[nodePtr->virtualRegister()];
-                if (infoPtr->alive() && infoPtr->nodeIndex() == nodeIndex)
-                    found = true;
+                if (nodePtr->shouldGenerate()) {
+                    infoPtr = &m_generationInfo[nodePtr->virtualRegister()];
+                    if (infoPtr->alive() && infoPtr->nodeIndex() == nodeIndex)
+                        found = true;
+                }
             }
         
             if (!found) {
@@ -1482,9 +1482,11 @@
             
                 if (nodeIndexToUse != NoNode) {
                     nodePtr = &at(nodeIndexToUse);
-                    infoPtr = &m_generationInfo[nodePtr->virtualRegister()];
-                    ASSERT(infoPtr->alive() && infoPtr->nodeIndex() == nodeIndexToUse);
-                    found = true;
+                    if (nodePtr->shouldGenerate()) {
+                        infoPtr = &m_generationInfo[nodePtr->virtualRegister()];
+                        ASSERT(infoPtr->alive() && infoPtr->nodeIndex() == nodeIndexToUse);
+                        found = true;
+                    }
                 }
             }
         
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to