Title: [248185] trunk
Revision
248185
Author
[email protected]
Date
2019-08-02 15:20:54 -0700 (Fri, 02 Aug 2019)

Log Message

[JSC] LazyJSValue should be robust for empty JSValue
https://bugs.webkit.org/show_bug.cgi?id=200388

Reviewed by Saam Barati.

JSTests:

* stress/switch-constant-child-becomes-empty.js: Added.
(foo):

Source/_javascript_Core:

If the Switch DFG node is preceded by ForceOSRExit or something that invalidates the basic block,
it can take a FrozenValue as a child which includes empty value instead of string, number etc.
If this Switch node is kept and we reached to DFGCFGSimplificationPhase, it will use this FrozenValue.
However, LazyJSValue using this FrozenValue strongly assumes that FrozenValue is never holding empty value.
But this assumption is wrong. This patch makes LazyJSValue robust for empty value.

* dfg/DFGLazyJSValue.cpp:
(JSC::DFG::LazyJSValue::tryGetStringImpl const):
(JSC::DFG::LazyJSValue::tryGetString const):
(JSC::DFG::LazyJSValue::strictEqual const):
(JSC::DFG::LazyJSValue::switchLookupValue const):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (248184 => 248185)


--- trunk/JSTests/ChangeLog	2019-08-02 22:14:30 UTC (rev 248184)
+++ trunk/JSTests/ChangeLog	2019-08-02 22:20:54 UTC (rev 248185)
@@ -1,3 +1,13 @@
+2019-08-02  Yusuke Suzuki  <[email protected]>
+
+        [JSC] LazyJSValue should be robust for empty JSValue
+        https://bugs.webkit.org/show_bug.cgi?id=200388
+
+        Reviewed by Saam Barati.
+
+        * stress/switch-constant-child-becomes-empty.js: Added.
+        (foo):
+
 2019-08-01  Yusuke Suzuki  <[email protected]>
 
         GetterSetter type confusion during DFG compilation

Added: trunk/JSTests/stress/switch-constant-child-becomes-empty.js (0 => 248185)


--- trunk/JSTests/stress/switch-constant-child-becomes-empty.js	                        (rev 0)
+++ trunk/JSTests/stress/switch-constant-child-becomes-empty.js	2019-08-02 22:20:54 UTC (rev 248185)
@@ -0,0 +1,17 @@
+//@ runDefault("--useConcurrentJIT=0")
+function foo(x) {
+    switch (x) {
+    case "a":
+    case "a":
+    case "a":
+        for (let j = 0; j <100; j++) {
+            let j=foo(j);
+        }
+    default:
+        return 2;
+    }
+}
+
+for (let i = 0; i <100000; i++) {
+    foo("ab");
+}

Modified: trunk/Source/_javascript_Core/ChangeLog (248184 => 248185)


--- trunk/Source/_javascript_Core/ChangeLog	2019-08-02 22:14:30 UTC (rev 248184)
+++ trunk/Source/_javascript_Core/ChangeLog	2019-08-02 22:20:54 UTC (rev 248185)
@@ -1,3 +1,22 @@
+2019-08-02  Yusuke Suzuki  <[email protected]>
+
+        [JSC] LazyJSValue should be robust for empty JSValue
+        https://bugs.webkit.org/show_bug.cgi?id=200388
+
+        Reviewed by Saam Barati.
+
+        If the Switch DFG node is preceded by ForceOSRExit or something that invalidates the basic block,
+        it can take a FrozenValue as a child which includes empty value instead of string, number etc.
+        If this Switch node is kept and we reached to DFGCFGSimplificationPhase, it will use this FrozenValue.
+        However, LazyJSValue using this FrozenValue strongly assumes that FrozenValue is never holding empty value.
+        But this assumption is wrong. This patch makes LazyJSValue robust for empty value.
+
+        * dfg/DFGLazyJSValue.cpp:
+        (JSC::DFG::LazyJSValue::tryGetStringImpl const):
+        (JSC::DFG::LazyJSValue::tryGetString const):
+        (JSC::DFG::LazyJSValue::strictEqual const):
+        (JSC::DFG::LazyJSValue::switchLookupValue const):
+
 2019-08-02  Devin Rousso  <[email protected]>
 
         Web Inspector: Storage: disable related agents when the tab is closed

Modified: trunk/Source/_javascript_Core/dfg/DFGLazyJSValue.cpp (248184 => 248185)


--- trunk/Source/_javascript_Core/dfg/DFGLazyJSValue.cpp	2019-08-02 22:14:30 UTC (rev 248184)
+++ trunk/Source/_javascript_Core/dfg/DFGLazyJSValue.cpp	2019-08-02 22:20:54 UTC (rev 248185)
@@ -100,9 +100,11 @@
             return string->tryGetValueImpl();
         return nullptr;
 
-    default:
+    case SingleCharacterString:
         return nullptr;
     }
+    RELEASE_ASSERT_NOT_REACHED();
+    return nullptr;
 }
 
 String LazyJSValue::tryGetString(Graph& graph) const
@@ -114,7 +116,8 @@
     case SingleCharacterString:
         return String(&u.character, 1);
 
-    default:
+    case KnownValue:
+    case KnownStringImpl:
         if (const StringImpl* string = tryGetStringImpl(graph.m_vm)) {
             unsigned ginormousStringLength = 10000;
             if (string->length() > ginormousStringLength)
@@ -128,6 +131,8 @@
         
         return String();
     }
+    RELEASE_ASSERT_NOT_REACHED();
+    return String();
 }
 
 TriState LazyJSValue::strictEqual(const LazyJSValue& other) const
@@ -135,14 +140,23 @@
     switch (m_kind) {
     case KnownValue:
         switch (other.m_kind) {
-        case KnownValue:
+        case KnownValue: {
+            if (!value()->value() || !other.value()->value())
+                return value()->value() == other.value()->value() ? TrueTriState : FalseTriState;
             return JSValue::pureStrictEqual(value()->value(), other.value()->value());
-        case SingleCharacterString:
+        }
+        case SingleCharacterString: {
+            if (!value()->value())
+                return FalseTriState;
             return equalToSingleCharacter(value()->value(), other.character());
+        }
         case KnownStringImpl:
-        case NewStringImpl:
+        case NewStringImpl: {
+            if (!value()->value())
+                return FalseTriState;
             return equalToStringImpl(value()->value(), other.stringImpl());
         }
+        }
         break;
     case SingleCharacterString:
         switch (other.m_kind) {
@@ -153,7 +167,7 @@
             if (other.stringImpl()->length() != 1)
                 return FalseTriState;
             return triState(other.stringImpl()->at(0) == character());
-        default:
+        case KnownValue:
             return other.strictEqual(*this);
         }
         break;
@@ -163,7 +177,8 @@
         case KnownStringImpl:
         case NewStringImpl:
             return triState(WTF::equal(stringImpl(), other.stringImpl()));
-        default:
+        case SingleCharacterString:
+        case KnownValue:
             return other.strictEqual(*this);
         }
         break;
@@ -181,9 +196,13 @@
     case KnownValue:
         switch (kind) {
         case SwitchImm:
-            return value()->value().asInt32();
+            if (value()->value())
+                return value()->value().asInt32();
+            return 0;
         case SwitchCell:
-            return bitwise_cast<uintptr_t>(value()->value().asCell());
+            if (value()->value())
+                return bitwise_cast<uintptr_t>(value()->value().asCell());
+            return 0;
         default:
             RELEASE_ASSERT_NOT_REACHED();
             return 0;
@@ -196,10 +215,13 @@
             RELEASE_ASSERT_NOT_REACHED();
             return 0;
         }
-    default:
+    case KnownStringImpl:
+    case NewStringImpl:
         RELEASE_ASSERT_NOT_REACHED();
         return 0;
     }
+    RELEASE_ASSERT_NOT_REACHED();
+    return 0;
 }
 
 void LazyJSValue::emit(CCallHelpers& jit, JSValueRegs result) const
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to