DFG::LazyJSValue::tryGetStringImpl() crashes for empty values
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 May 2016 22:29:02 +0000 (22:29 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 May 2016 22:29:02 +0000 (22:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=158170

Reviewed by Michael Saboff.

The problem here is that jsDynamicCast<>() is evil! It avoids checking for the empty
value, presumably because this makes it soooper fast. In DFG IR, empty values can appear
anywhere because of TDZ.

This patch doesn't change jsDynamicCast<>(), but it hardens our wrappers for it in the DFG
and it has the affected code use one of those wrappers.

* dfg/DFGFrozenValue.h:
(JSC::DFG::FrozenValue::dynamicCast): Harden this.
(JSC::DFG::FrozenValue::cast):
* dfg/DFGLazyJSValue.cpp:
(JSC::DFG::LazyJSValue::tryGetStringImpl): Use the hardened wrapper.
* tests/stress/strcat-emtpy.js: Added. This used to crash every time.
(foo):
(i.catch):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@201470 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGFrozenValue.h
Source/JavaScriptCore/dfg/DFGLazyJSValue.cpp
Source/JavaScriptCore/tests/stress/strcat-emtpy.js [new file with mode: 0644]

index 4689234..17d1501 100644 (file)
@@ -1,5 +1,28 @@
 2016-05-27  Filip Pizlo  <fpizlo@apple.com>
 
+        DFG::LazyJSValue::tryGetStringImpl() crashes for empty values
+        https://bugs.webkit.org/show_bug.cgi?id=158170
+
+        Reviewed by Michael Saboff.
+
+        The problem here is that jsDynamicCast<>() is evil! It avoids checking for the empty
+        value, presumably because this makes it soooper fast. In DFG IR, empty values can appear
+        anywhere because of TDZ.
+        
+        This patch doesn't change jsDynamicCast<>(), but it hardens our wrappers for it in the DFG
+        and it has the affected code use one of those wrappers.
+        
+        * dfg/DFGFrozenValue.h:
+        (JSC::DFG::FrozenValue::dynamicCast): Harden this.
+        (JSC::DFG::FrozenValue::cast):
+        * dfg/DFGLazyJSValue.cpp:
+        (JSC::DFG::LazyJSValue::tryGetStringImpl): Use the hardened wrapper.
+        * tests/stress/strcat-emtpy.js: Added. This used to crash every time.
+        (foo):
+        (i.catch):
+
+2016-05-27  Filip Pizlo  <fpizlo@apple.com>
+
         regExpProtoFuncSplitFast should OOM before it swaps
         https://bugs.webkit.org/show_bug.cgi?id=158157
 
index c59abeb..fa6eebe 100644 (file)
@@ -73,7 +73,10 @@ public:
     template<typename T>
     T dynamicCast()
     {
-        return jsDynamicCast<T>(value());
+        JSValue theValue = value();
+        if (!theValue)
+            return nullptr;
+        return jsDynamicCast<T>(theValue);
     }
     template<typename T>
     T cast()
index 0c2ed19..468053e 100644 (file)
@@ -95,7 +95,7 @@ const StringImpl* LazyJSValue::tryGetStringImpl() const
         return u.stringImpl;
 
     case KnownValue:
-        if (JSString* string = jsDynamicCast<JSString*>(value()->value()))
+        if (JSString* string = value()->dynamicCast<JSString*>())
             return string->tryGetValueImpl();
         return nullptr;
 
diff --git a/Source/JavaScriptCore/tests/stress/strcat-emtpy.js b/Source/JavaScriptCore/tests/stress/strcat-emtpy.js
new file mode 100644 (file)
index 0000000..bf68ea1
--- /dev/null
@@ -0,0 +1,14 @@
+function foo() {
+    "use strict";
+    let a = "hello" + a;
+    return a;
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+    try {
+        foo();
+    } catch (e) {
+    }
+}