ToString node actually does GC.
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Jan 2019 00:41:45 +0000 (00:41 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Jan 2019 00:41:45 +0000 (00:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193920
<rdar://problem/46695900>

Reviewed by Yusuke Suzuki.

JSTests:

* stress/dfg-to-string-on-int-does-gc.js: Added.
* stress/dfg-to-string-on-string-object-does-not-gc.js: Added.
* stress/dfg-to-string-on-string-or-string-object-does-not-gc.js: Added.

Source/JavaScriptCore:

Other than for StringObjectUse and StringOrStringObjectUse, ToString and
CallStringConstructor can allocate new JSStrings, and hence, can GC.

* dfg/DFGDoesGC.cpp:
(JSC::DFG::doesGC):

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

JSTests/ChangeLog
JSTests/stress/dfg-to-string-on-int-does-gc.js [new file with mode: 0644]
JSTests/stress/dfg-to-string-on-string-object-does-not-gc.js [new file with mode: 0644]
JSTests/stress/dfg-to-string-on-string-or-string-object-does-not-gc.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGDoesGC.cpp

index 4dc7172..bbd96a1 100644 (file)
@@ -1,3 +1,15 @@
+2019-01-28  Mark Lam  <mark.lam@apple.com>
+
+        ToString node actually does GC.
+        https://bugs.webkit.org/show_bug.cgi?id=193920
+        <rdar://problem/46695900>
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/dfg-to-string-on-int-does-gc.js: Added.
+        * stress/dfg-to-string-on-string-object-does-not-gc.js: Added.
+        * stress/dfg-to-string-on-string-or-string-object-does-not-gc.js: Added.
+
 2019-01-25  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] NativeErrorConstructor should not have own IsoSubspace
diff --git a/JSTests/stress/dfg-to-string-on-int-does-gc.js b/JSTests/stress/dfg-to-string-on-int-does-gc.js
new file mode 100644 (file)
index 0000000..f48663a
--- /dev/null
@@ -0,0 +1,26 @@
+//@ requireOptions("--exceptionStackTraceLimit=0", "--defaultErrorStackTraceLimit=0", "--forceRAMSize=1000000", "--forceDebuggerBytecodeGeneration=1", "--useZombieMode=1", "--jitPolicyScale=0", "--collectContinuously=1", "--useConcurrentJIT=0")
+
+function assert(b) {
+    if (!b)
+        throw new Error('aa');
+}
+
+var exception;
+try {
+    let target = function (x, y) {
+        const actual = '' + x;
+        target(x);
+    };
+    let handler = {
+        apply: function (theTarget, thisArg, argArray) {
+            return theTarget.apply([], argArray);
+        }
+    };
+    let proxy = new Proxy(target, handler);
+    assert(proxy(10, 20) === 'foo');
+} catch(e) {
+    exception = e;
+}
+
+if (exception != "RangeError: Maximum call stack size exceeded.")
+    throw "FAILED";
diff --git a/JSTests/stress/dfg-to-string-on-string-object-does-not-gc.js b/JSTests/stress/dfg-to-string-on-string-object-does-not-gc.js
new file mode 100644 (file)
index 0000000..1e35533
--- /dev/null
@@ -0,0 +1,26 @@
+//@ requireOptions("--exceptionStackTraceLimit=0", "--defaultErrorStackTraceLimit=0", "--forceRAMSize=1000000", "--forceDebuggerBytecodeGeneration=1", "--useZombieMode=1", "--jitPolicyScale=0", "--collectContinuously=1", "--useConcurrentJIT=0")
+
+function assert(b) {
+    if (!b)
+        throw new Error('aa');
+}
+
+var exception;
+try {
+    let target = function (x, y) {
+        const actual = '' + x;
+        target(x);
+    };
+    let handler = {
+        apply: function (theTarget, thisArg, argArray) {
+            return theTarget.apply([], argArray);
+        }
+    };
+    let proxy = new Proxy(target, handler);
+    assert(proxy(new String("10"), new String("20")) === 'foo');
+} catch(e) {
+    exception = e;
+}
+
+if (exception != "RangeError: Maximum call stack size exceeded.")
+    throw "FAILED";
diff --git a/JSTests/stress/dfg-to-string-on-string-or-string-object-does-not-gc.js b/JSTests/stress/dfg-to-string-on-string-or-string-object-does-not-gc.js
new file mode 100644 (file)
index 0000000..0dae94d
--- /dev/null
@@ -0,0 +1,34 @@
+//@ requireOptions("--exceptionStackTraceLimit=0", "--defaultErrorStackTraceLimit=0", "--forceRAMSize=1000000", "--forceDebuggerBytecodeGeneration=1", "--useZombieMode=1", "--jitPolicyScale=0", "--collectContinuously=1", "--useConcurrentJIT=0")
+
+function assert(b) {
+    if (!b)
+        throw new Error('aa');
+}
+
+let alternate = true;
+var exception;
+try {
+    function alter(x) {
+        alternate = !alternate;
+        if (alternate)
+            return new String(x);
+        return x;
+    }
+    noInline(alter);
+    let target = function (x, y) {
+        const actual = '' + alter(x);
+        target(x);
+    };
+    let handler = {
+        apply: function (theTarget, thisArg, argArray) {
+            return theTarget.apply([], argArray);
+        }
+    };
+    let proxy = new Proxy(target, handler);
+    assert(proxy("10", "20") === 'foo');
+} catch(e) {
+    exception = e;
+}
+
+if (exception != "RangeError: Maximum call stack size exceeded.")
+    throw "FAILED";
index d5f663d..c2a70f5 100644 (file)
@@ -1,3 +1,17 @@
+2019-01-28  Mark Lam  <mark.lam@apple.com>
+
+        ToString node actually does GC.
+        https://bugs.webkit.org/show_bug.cgi?id=193920
+        <rdar://problem/46695900>
+
+        Reviewed by Yusuke Suzuki.
+
+        Other than for StringObjectUse and StringOrStringObjectUse, ToString and
+        CallStringConstructor can allocate new JSStrings, and hence, can GC.
+
+        * dfg/DFGDoesGC.cpp:
+        (JSC::DFG::doesGC):
+
 2019-01-28  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] RegExpConstructor should not have own IsoSubspace
index 5418127..191f26b 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -191,8 +191,6 @@ bool doesGC(Graph& graph, Node* node)
     case LogicalNot:
     case ToPrimitive:
     case ToNumber:
-    case ToString:
-    case CallStringConstructor:
     case NumberToStringWithRadix:
     case NumberToStringWithValidRadixConstant:
     case InByVal:
@@ -384,6 +382,17 @@ bool doesGC(Graph& graph, Node* node)
     case ValueDiv:
         return true;
 
+    case CallStringConstructor:
+    case ToString:
+        switch (node->child1().useKind()) {
+        case StringObjectUse:
+        case StringOrStringObjectUse:
+            return false;
+        default:
+            break;
+        }
+        return true;
+
     case GetIndexedPropertyStorage:
         if (node->arrayMode().type() == Array::String)
             return true;