Changeset 244480 in webkit

Timestamp:

Apr 19, 2019 5:37:43 PM (5 years ago)

Author:

sbarati@apple.com

Message:

AbstractValue can represent more than int52
​https://bugs.webkit.org/show_bug.cgi?id=197118
<rdar://problem/49969960>

Reviewed by Michael Saboff.

JSTests:

• stress/abstract-value-can-include-int52.js: Added.

(foo):
(index.index.8.index.60.index.65.index.1234.index.1234.parseInt.string_appeared_here.String.fromCharCode):

Source/JavaScriptCore:

Let's analyze this control flow diamond:

#0
branch #1, #2

#1:
PutStack(JSValue, loc42)
Jump #3

#2:
PutStack(Int52, loc42)
Jump #3

#3:
...

Our abstract value for loc42 at the head of #3 will contain an abstract
value that us the union of Int52 with other things. Obviously in the
above program, a GetStack for loc42 would be inavlid, since it might
be loading either JSValue or Int52. However, the abstract interpreter
just tracks what the value could be, and it could be Int52 or JSValue.

When I did the Int52 refactoring, I expected such things to never happen,
but it turns out it does. We should just allow for this instead of asserting
against it since it's valid IR to do the above.

• bytecode/SpeculatedType.cpp:

(JSC::dumpSpeculation):

• dfg/DFGAbstractValue.cpp:

(JSC::DFG::AbstractValue::checkConsistency const):

• dfg/DFGAbstractValue.h:

(JSC::DFG::AbstractValue::validateTypeAcceptingBoxedInt52 const):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/abstract-value-can-include-int52.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/bytecode/SpeculatedType.cpp (modified) (2 diffs)
• Source/JavaScriptCore/dfg/DFGAbstractValue.cpp (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGAbstractValue.h (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2019-04-19  Saam Barati  <sbarati@apple.com>
+
+        AbstractValue can represent more than int52
+        https://bugs.webkit.org/show_bug.cgi?id=197118
+        <rdar://problem/49969960>
+
+        Reviewed by Michael Saboff.
+
+        * stress/abstract-value-can-include-int52.js: Added.
+        (foo):
+        (index.index.8.index.60.index.65.index.1234.index.1234.parseInt.string_appeared_here.String.fromCharCode):
+
 2019-04-18  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-04-19  Saam Barati  <sbarati@apple.com>
+
+        AbstractValue can represent more than int52
+        https://bugs.webkit.org/show_bug.cgi?id=197118
+        <rdar://problem/49969960>
+
+        Reviewed by Michael Saboff.
+
+        Let's analyze this control flow diamond:
+        
+        #0
+        branch #1, #2
+        
+        #1:
+        PutStack(JSValue, loc42)
+        Jump #3
+        
+        #2:
+        PutStack(Int52, loc42)
+        Jump #3
+        
+        #3:
+        ...
+        
+        Our abstract value for loc42 at the head of #3 will contain an abstract
+        value that us the union of Int52 with other things. Obviously in the
+        above program, a GetStack for loc42 would be inavlid, since it might
+        be loading either JSValue or Int52. However, the abstract interpreter
+        just tracks what the value could be, and it could be Int52 or JSValue.
+        
+        When I did the Int52 refactoring, I expected such things to never happen,
+        but it turns out it does. We should just allow for this instead of asserting
+        against it since it's valid IR to do the above.
+
+        * bytecode/SpeculatedType.cpp:
+        (JSC::dumpSpeculation):
+        * dfg/DFGAbstractValue.cpp:
+        (JSC::DFG::AbstractValue::checkConsistency const):
+        * dfg/DFGAbstractValue.h:
+        (JSC::DFG::AbstractValue::validateTypeAcceptingBoxedInt52 const):
+
 2019-04-19  Tadeu Zagallo  <tzagallo@apple.com>
 

trunk/Source/JavaScriptCore/bytecode/SpeculatedType.cpp

@@ -249 +249 @@
             isTop = false;
     }
-    
-    if (value & SpecInt32AsInt52)
-        strOut.print("Int32AsInt52");
-
-    if (value & SpecNonInt32AsInt52)
-        strOut.print("NonInt32AsInt52");
-        
+
     if ((value & SpecBytecodeDouble) == SpecBytecodeDouble)
         strOut.print("BytecodeDouble");
@@ -288 +282 @@
         isTop = false;
     
-    if (isTop)
+    if (value & SpecEmpty)
+        strOut.print("Empty");
+    else
+        isTop = false;
+
+    if (value & SpecInt52Any) {
+        if ((value & SpecInt52Any) == SpecInt52Any)
+            strOut.print("Int52Any");
+        else if (value & SpecInt32AsInt52)
+            strOut.print("Int32AsInt52");
+        else if (value & SpecNonInt32AsInt52)
+            strOut.print("NonInt32AsInt52");
+    } else
+        isTop = false;
+    
+    if (value == SpecBytecodeTop)
+        out.print("BytecodeTop");
+    else if (value == SpecHeapTop)
+        out.print("HeapTop");
+    else if (value == SpecFullTop)
+        out.print("FullTop");
+    else if (isTop)
         out.print("Top");
     else
         out.print(strStream.toCString());
-    
-    if (value & SpecEmpty)
-        out.print("Empty");
 }
 

trunk/Source/JavaScriptCore/dfg/DFGAbstractValue.cpp

@@ -453 +453 @@
         RELEASE_ASSERT(!m_value);
     
-    if (m_type & SpecInt52Any) {
-        if (m_type != SpecFullTop)
-            RELEASE_ASSERT(isAnyInt52Speculation(m_type));
-    }
-
     if (!!m_value)
         RELEASE_ASSERT(validateTypeAcceptingBoxedInt52(m_value));

trunk/Source/JavaScriptCore/dfg/DFGAbstractValue.h

@@ -527 +527 @@
         
         if (m_type & SpecInt52Any) {
-            ASSERT(!(m_type & ~SpecInt52Any));
-
-            if (mergeSpeculations(m_type, int52AwareSpeculationFromValue(value)) != m_type)
-                return false;
-            return true;
+            if (mergeSpeculations(m_type, int52AwareSpeculationFromValue(value)) == m_type)
+                return true;
         }