Attachment 360383 Details for Bug 193920 – proposed patch.

[patch] proposed patch.

bug-193920.patch (text/plain), 6.21 KB, created by Mark Lam on 2019-01-28 15:14:14 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Lam

Created: 2019-01-28 15:14:14 PST

Size: 6.21 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 240605)
>+++ JSTests/ChangeLog	(working copy)
>@@ -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 NOBODY (OOPS!).
>+
>+        * 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
>Index: JSTests/stress/dfg-to-string-on-int-does-gc.js
>===================================================================
>--- JSTests/stress/dfg-to-string-on-int-does-gc.js	(nonexistent)
>+++ JSTests/stress/dfg-to-string-on-int-does-gc.js	(working copy)
>@@ -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";
>Index: JSTests/stress/dfg-to-string-on-string-object-does-not-gc.js
>===================================================================
>--- JSTests/stress/dfg-to-string-on-string-object-does-not-gc.js	(nonexistent)
>+++ JSTests/stress/dfg-to-string-on-string-object-does-not-gc.js	(working copy)
>@@ -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";
>Index: JSTests/stress/dfg-to-string-on-string-or-string-object-does-not-gc.js
>===================================================================
>--- JSTests/stress/dfg-to-string-on-string-or-string-object-does-not-gc.js	(nonexistent)
>+++ JSTests/stress/dfg-to-string-on-string-or-string-object-does-not-gc.js	(working copy)
>@@ -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: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 240604)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -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 NOBODY (OOPS!).
>+
>+        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: Source/JavaScriptCore/dfg/DFGDoesGC.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGDoesGC.cpp	(revision 240604)
>+++ Source/JavaScriptCore/dfg/DFGDoesGC.cpp	(working copy)
>@@ -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;

Actions: View | Formatted Diff | Diff

Attachments on bug 193920: 360383