Changeset 121391 in webkit for trunk/Source/JavaScriptCore

Timestamp:

Jun 27, 2012, 5:49:55 PM (13 years ago)

Author:

[email protected]

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/JavaScriptCore:

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):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• dfg/DFGSpeculativeJIT.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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]>
 

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -1398 +1398 @@
             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:
@@ -1432 +1430 @@
                 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;
+                }
             }
         
@@ -1483 +1483 @@
                 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;
+                    }
                 }
             }